Skip to content
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.

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

avoid breaking network compatibility #74

Open
rkuhn opened this issue Dec 16, 2022 · 0 comments
Open

avoid breaking network compatibility #74

rkuhn opened this issue Dec 16, 2022 · 0 comments

Comments

@rkuhn
Copy link

rkuhn commented Dec 16, 2022

As witnessed in libp2p/rust-libp2p#3244 (comment) adding a new protocol to the multiaddr implementation breaks communication between nodes using the new feature and nodes using an older version of this library. This would be warranted if understanding the meaning of a multiaddr is always mandatory, but there are cases (like the identify protocol) where that is not the case.

Breakage for such cases could be avoided by adding a new layer of validation: besides syntactically invalid and fully understood there could be a class that is syntactically valid but not fully understood.

Due to the design of multiaddr syntax, this is not a trivial question: a protocol segment may have arguments, like /tcp/1234, and without understanding the protocol name it is impossible to know the number of arguments. It would have been possible to choose different separator characters (like /tcp=1324 or some such), but that ship has sailed. So the only way to express syntactically valid but not fully understood addresses is to add a variant like Protocol::Unknown(Cow<'a, str>), where /tcp2/1234 would lead to two unknown segments (with tcp2 and 1234 payloads, respectively).


The alternative to handling this in this library is to always deserialize a multiaddr property as String first and then check whether it can be fully parsed if needed. However, given that multiaddr aims to offer an abstraction over various addressing schemes, I think it is reasonable to expect that this scheme itself is extensible and handles extensions in a graceful fashion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant