Skip to content

Comments

[ADR] 0003 - Adopt Libp2p PeerIDs#34

Open
bryanchriswhite wants to merge 7 commits intomainfrom
docs/ADR0003
Open

[ADR] 0003 - Adopt Libp2p PeerIDs#34
bryanchriswhite wants to merge 7 commits intomainfrom
docs/ADR0003

Conversation

@bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Apr 14, 2023

Summary

This pull request adds an Architecture Decision Record (ADR) for adopting Libp2p identity/IDs to simplify and consolidate node identity in our system. The ADR explains the context, problem statement, decision drivers, considered options, and the decision outcome, along with the positive and negative consequences of the chosen option.

The chosen option is to adopt Libp2p identity/IDs, as it provides a unique, verifiable, and secure identification system that is proven in the peer-to-peer ecosystem.

Please review the ADR and provide feedback on:

  • The clarity of the problem statement and context
  • The thoroughness of the decision drivers and considered options
  • The appropriateness of the chosen option given the decision drivers
  • Any additional considerations that may have been overlooked
  • Any changes in formatting, grammar, or language for better readability

Related Issues

@bryanchriswhite bryanchriswhite added p2p architecture decision Related to architecture decision records (ADRs) labels Apr 14, 2023
@bryanchriswhite bryanchriswhite self-assigned this Apr 14, 2023
@bryanchriswhite bryanchriswhite changed the base branch from main to docs/ADRs April 14, 2023 11:46
@bryanchriswhite bryanchriswhite force-pushed the docs/ADRs branch 2 times, most recently from 8e332c4 to 7e6934a Compare April 14, 2023 12:47
@bryanchriswhite bryanchriswhite changed the base branch from docs/ADRs to main April 17, 2023 11:14
@bryanchriswhite bryanchriswhite force-pushed the docs/ADR0003 branch 2 times, most recently from 0565bd5 to ab78a57 Compare April 17, 2023 11:17
@bryanchriswhite bryanchriswhite marked this pull request as ready for review April 17, 2023 11:20
@bryanchriswhite bryanchriswhite changed the title [ADR] 0003 - Adopting Libp2p Identity/IDs [ADR] 0003 - Adopt Libp2p PeerIDs Apr 17, 2023
@bryanchriswhite bryanchriswhite marked this pull request as draft April 18, 2023 17:34
@Olshansk Olshansk removed their request for review April 19, 2023 19:41
@bryanchriswhite bryanchriswhite marked this pull request as ready for review April 21, 2023 07:21
Copy link
Collaborator

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this makes sense to me and I approve.

My only question as a reader (which may be outside the scope of this ADR) is what a LibP2P PeerId ends up looking like

@bryanchriswhite
Copy link
Contributor Author

All of this makes sense to me and I approve.

My only question as a reader (which may be outside the scope of this ADR) is what a LibP2P PeerId ends up looking like

@Olshansk, I think this is related to https://github.com/pokt-network/pocket-network-protocol/pull/32/files#r1199028895.

These are supposed to be living documents and we have yet to update one. Wdyt about merging this as is and updating it with some examples / practical application, once we've decided what that looks like and updated the template once more?

@bryanchriswhite bryanchriswhite requested a review from Olshansk May 19, 2023 14:30
@Olshansk Olshansk removed the request for review from gokutheengineer May 20, 2023 01:31
Copy link
Collaborator

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No


Facing the need for a unique and verifiable identifier for nodes in the peer-to-peer network, we decided to adopt Libp2p identity/IDs and neglect alternative identity schemes, to achieve a reliable and secure identification system, accepting the additional dependency and integration work, because Libp2p provides a proven solution with robust security features.

## Problem Statement
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's worth mentioning that this ADR builds on top of #32 to adopt multiaddr and LibP2PPeerIDs use multiaddr underneath?

## Considered Options

1. Adopting Libp2p identity/IDs
2. Custom identity scheme
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth saying "A custom wrapper" around multiaddr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiaddr is orthogonal to peer IDs. It's used represent the network address of a node for the purpose of establishing a network transport connection. A peer ID can be encapsulated in a multiaddr to encode identity information in addition to the network address, for the convenience of the consumer. However, this ADR is coming from a different, more peer-ID-centric, perspective where multiaddr isn't specifically relevant.

Examples of "custom identity scheme"s would be something like what we talked about in the decentralized identity protocol hour. Ranging from something simple, like a self-signed public key (key plus signature) or something more sophisticated, like an X509 certificate chain (self-signed or otherwise). Perhaps these examples could be added using the structure you suggested (#32 (comment))?


Chosen option: "Adopting Libp2p identity/IDs", because it provides a unique, verifiable, and secure identification system that is proven in the peer-to-peer ecosystem.

### Positive Consequences
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, I was thinking of throwing a 🔧 into the mix after reading #32 right before this (which I fully support) and start hesitating a bit about this one. Going through these positive consequences convinced me otherwise.

With that said, calling adopt that doing this manually, even if we adopt LibP2P multiaddr, would be a non-trivial amount of work that does not enable anything Pocket might need in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure I followed the second part, can you say more?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had some typos so understand why you were confused...

Going to consolidate this with your comment in https://github.com/pokt-network/pocket-network-protocol/pull/34/files#r1206682770.


You said that a A peer ID can be encapsulated in a multiaddr but I think of it as a A single peer ID can map to multiple multiaddr since a multiaddr is lower on the network stack and used for network communication rather than authenticating/authorizing another networking peer.

As such, adopting multiaddr feels like a very easy decision for me to make, but adopting libp2p for peer IDs comes with more hurdles. We can easily build Pocket identity wrappers around multiaddr, but adopting PeerID from libp2p natively may give us less optionality in the future.

My argument is that "Because we have decided to adopt multiaddr, and libp2p PeerID incorporated multiaddr natively, this will reduce the workload and maintenance on Pocket's side.`


Does that make more sense in terms of:

  1. My concerns
  2. The arguments we need to consider to adopt/withdraw this?


Chosen option: "Adopting Libp2p identity/IDs", because it provides a unique, verifiable, and secure identification system that is proven in the peer-to-peer ecosystem.

### Positive Consequences
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had some typos so understand why you were confused...

Going to consolidate this with your comment in https://github.com/pokt-network/pocket-network-protocol/pull/34/files#r1206682770.


You said that a A peer ID can be encapsulated in a multiaddr but I think of it as a A single peer ID can map to multiple multiaddr since a multiaddr is lower on the network stack and used for network communication rather than authenticating/authorizing another networking peer.

As such, adopting multiaddr feels like a very easy decision for me to make, but adopting libp2p for peer IDs comes with more hurdles. We can easily build Pocket identity wrappers around multiaddr, but adopting PeerID from libp2p natively may give us less optionality in the future.

My argument is that "Because we have decided to adopt multiaddr, and libp2p PeerID incorporated multiaddr natively, this will reduce the workload and maintenance on Pocket's side.`


Does that make more sense in terms of:

  1. My concerns
  2. The arguments we need to consider to adopt/withdraw this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

architecture decision Related to architecture decision records (ADRs) p2p

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants