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

Clarify expectations about supported protocols across connections and peers #525

Closed
thomaseizinger opened this issue Feb 27, 2023 · 5 comments · Fixed by #530
Closed

Clarify expectations about supported protocols across connections and peers #525

thomaseizinger opened this issue Feb 27, 2023 · 5 comments · Fixed by #530

Comments

@thomaseizinger
Copy link
Contributor

As part of implementing libp2p/rust-libp2p#2680, a discussion came up that I'd like to have on a cross-implementation basis.

What is everyone's take on the impact of supported protocols that are communicated via identify? Do these apply to all physical connections or just the one that identify ran over?

In rust-libp2p, it is technically possible for individually negotiated stream to support a completely different set of protocols. That is obviously not very useful.

However, I think it is useful to report a set of supported protocols perhaps per peer or per physical connection. One example where we do this is not allowing relay over an already relayed connection.

Would it make sense to specify that a libp2p application SHOULD support the same set of protocols over the lifetime of a physical connection with a given peer but MAY NOT support the same set of protocols over a different connection or to a different peer?

@marten-seemann
Copy link
Contributor

Not sure what the question is. Identify applies to the connection it is run on. The list of protocols is no more than advisory, there's no guarantee that the latest Identify information is up to date anyway.

@thomaseizinger
Copy link
Contributor Author

Identify applies to the connection it is run on.

That is what I am after.

there's no guarantee that the latest Identify information is up to date anyway.

This sounds like the supported protocols may also change across the lifetime of a connection?

For example, if node A advertises that they speak the kademlia protocol, node B should only add that specific connection to the routing table. If kademlia disappears from the list of supported protocols, the routing table entry should be removed.

Is this what is happening today in other implementations?

The motivation for this issue is kademlia's client-mode support in rust-libp2p: libp2p/rust-libp2p#2032

@MarcoPolo
Copy link
Contributor

Would it make sense to specify that a libp2p application SHOULD support the same set of protocols over the lifetime of a physical connection with a given peer ...

I think it's okay for a libp2p implementations to change their supported protocols over the lifetime of a connection. For example with Kademlia you may start as a client as a client and not advertise kademlia in your identify. Later you may learn you are public and thus add kademlia to your identify msg. Then push an update via Identify Push.

... but MAY NOT support the same set of protocols over a different connection or to a different peer?

maybe? What's the use case? A relay server won't be using a relayed connection any time soon (we don't support relay over relay). But I do see the benefit of a node saying "Please don't bitswap on this connection, I have limited bandwidth here. Please use this connection only to bootstrap a direct connection". So thinking about this a bit, this makes sense.

This sounds like the supported protocols may also change across the lifetime of a connection?

Yes.

... Is this what is happening today in other implementations?

Yes, https://github.com/libp2p/go-libp2p-kad-dht/blob/master/subscriber_notifee.go#L105

@thomaseizinger
Copy link
Contributor Author

Would it make sense to specify that a libp2p application SHOULD support the same set of protocols over the lifetime of a physical connection with a given peer ...

I think it's okay for a libp2p implementations to change their supported protocols over the lifetime of a connection. For example with Kademlia you may start as a client as a client and not advertise kademlia in your identify. Later you may learn you are public and thus add kademlia to your identify msg. Then push an update via Identify Push.

That makes sense!

Would we be overstating the obvious if we included the following somewhere in our specs (rewording welcome):

A libp2p node SHOULD scope its set of supported protocols to the underlying physical connection to a peer. It MAY only support a protocol based on properties of a physical connection to e.g. limit the use of bandwidth-heavy protocols over a relayed or metered connection. A libp2p node MAY offer different sets of protocols to different peers. It MAY revoke or add the support for a protocol at any time, for example to only offer certain services after learning its NAT status on a connection. Therefore, libp2p nodes SHOULD NOT assume that the set of protocols on a connection is static.

@mxinden
Copy link
Member

mxinden commented Mar 2, 2023

I am in favor of including the above in our specs.

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

Successfully merging a pull request may close this issue.

4 participants