Skip to content

[ADR] 0001 - Adopt Multiaddr #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bryanchriswhite wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[ADR] 0001 - Adopt Multiaddr #32

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
baaf824
docs: add ADR0001
bryanchriswhite Apr 14, 2023
5dbf590
ADR0001: status "in review"
bryanchriswhite Apr 21, 2023
d54250b
refactor: reformat using udpated template (#40)
bryanchriswhite Apr 28, 2023
2736e9b
Merge remote-tracking branch 'pokt/main' into docs/ADR0001
bryanchriswhite May 19, 2023
8758154
chore: incorporate updated template & review feedback
bryanchriswhite May 19, 2023
c09e767
edit: adding detail
bryanchriswhite May 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions ADRs/0001-adopt-multiaddr.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# ADR-0001: Adopt Multiaddr
Copy link
Copy Markdown
Contributor

@dylanlott dylanlott Apr 21, 2023

Choose a reason for hiding this comment

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

💯


# Adopt Multiaddr for Addressing and Protocol Handling

* Status: in review
* Deciders: Pocket Network team
Comment thread
bryanchriswhite marked this conversation as resolved.
Outdated
* Date: 2023-04-17

Technical Story: GitHub discussion [#616: Should we go multiaddr](https://github.com/pokt-network/pocket/discussions/616)

## Context and Problem Statement

In the context of simplifying and consolidating node identity, facing the need for a standardized and extensible address format, we decided to adopt multiaddr and neglect alternative address conventions, to achieve a future-proof and composable addressing scheme, accepting the additional dependency and learning curve, because multiaddr provides a standard solution with many benefits.
Comment thread
bryanchriswhite marked this conversation as resolved.
Outdated

## Decision Drivers

* Standardized serialization format
* Supporting arbitrarily deeply nested protocols
* Supporting arbitrary protocol values, i.e., extensible
Comment thread
bryanchriswhite marked this conversation as resolved.
Outdated

## Considered Options

* Adopting multiaddr
* Custom address convention

## Decision Outcome

Chosen option: "Adopting multiaddr", because it provides a standardized, extensible, and future-proof solution for addressing nodes.
Comment thread
bryanchriswhite marked this conversation as resolved.
Outdated

### Positive Consequences

* Standardized and easily parsable node addresses
* Extensible and composable address format
* Future-proof addressing scheme

### Negative Consequences

* Additional dependency on the multiaddr library
* Learning curve for understanding multiaddr abstraction

## Pros and Cons of the Options

### Adopting multiaddr

* Good, because it provides a standardized and extensible address format
* Good, because it supports arbitrarily deeply nested protocols
* Good, because it is a widely adopted solution in the p2p ecosystem
* Bad, because it introduces an additional dependency
* Bad, because it requires learning a new abstraction

### Custom address convention

* Good, because it doesn't require an additional dependency
* Bad, because it may lead to a non-standard and less extensible solution
* Bad, because it requires development and maintenance efforts

Copy link
Copy Markdown
Collaborator

@Olshansk Olshansk Apr 27, 2023

Choose a reason for hiding this comment

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

I noticed that the ADR is very light, which comes with both pros & cons.

Was it an explicit decision to not, for example, provide examples of what a multiaddr or a custom URL looks like?

I can steelman both sides of the argument here but wanted to know if it was the intention or just a lack of time?

Copy link
Copy Markdown
Contributor Author

@bryanchriswhite bryanchriswhite May 19, 2023

Choose a reason for hiding this comment

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

I agree, I think it was more of a matter of structure.

Looking at it now, I'm not sure where it would make the most sense to put something like that. I feel like that would be something that I would expect to be covered by references. However, there isn't a great way to consistently be much more specific with a reference other than a link to a page (and maybe a named anchor on the page). I wonder if grouping them all together is the must useful approach.

We could consider including an "examples" or "practical application" section in "decision outcome" > "positive(/negative) consequences" and/or each option section. Pos./neg. consequences would provide a place to show (or reference) code and/or diagram(s) like this. Wdyt?

Copy link
Copy Markdown
Collaborator

@Olshansk Olshansk May 20, 2023

Choose a reason for hiding this comment

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

Let's add a section like so with the comments I have in place. I think we should keep it light weight but have a reminder to do this if we choose to.

I think if you copy-paste one of your tests in p2p, or link to them, it would be more than sufficient in this ADR.

### Supporting Examples/References <!-- optional -->
<!-- 
This is an optional section for you to link to or embed supporting examples.
These could come in the form of positive/negative consequences or just
examples for others to reference.
-->

### Links

* [`go-multiaddr` GitHub repo](https://github.com/multiformats/go-multiaddr)
* [`multiaddr` specification](https://github.com/multiformats/multiaddr)