-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add the possibility to negotiate fallback Kademlia protocol names #2837
Comments
This makes sense to me.
I don't mind the breaking change on the API. I am fine with One thing to keep in mind: Given node
I don't think this is a blocker. Just something to keep in mind when it comes to your role out strategy. |
Back when this was implemented, was the alternative considered of just adding The API of this module is about to change and I am wondering if we need to keep the functionality of specifying multiple names: #5006 |
No, I was not aware of such option. It would be easier to keep multiple protocol names for now to not incur the refactoring of the client code. CC @altonen |
Yes, for now the API is staying. But with #5006, we get the opportunity to introduce a new one and I think we should be fine to just handle a single protocol there. |
Description
Add the possibility to negotiate multiple Kademlia protocol names, like it's done, for example for request-response protocol.
Motivation
This would allow to change the protocol name in a backward-compatible manner. We've updated the protocol names in Polkadot/Substrate and the Kademlia protocol is the only one remaining due to this limit of one on-the-wire protocol name. Original discussion is in paritytech/substrate#7746 (comment).
Current Implementation
Currently only one protocol name is returned in
UpgardeInfo
forKademliaProtocolConfig
:rust-libp2p/protocols/kad/src/protocol.rs
Lines 177 to 184 in 4253080
Proposed Solution
We can add a field
fallback_names
toKademliaProtocolConfig
and a setterset_fallback_names()
. This way the change will be backward-compatible in terms of the API.Open Questions
Generally, it might worth reporting the negotiated protocol name for clients to be able to change the behavior accordingly, but this will require to change
KadStreamSink
and will be an API breaking change. I don't know whether it makes sense doing this, probably it doesn't. CC @tomakaAre you planning to do it yourself in a pull request?
Yes.
The text was updated successfully, but these errors were encountered: