Kademlia Client mode
Context
Continuation of https://github.com/libp2p/rust-libp2p/pull/2184 and fixes https://github.com/libp2p/rust-libp2p/issues/2032.
See https://github.com/libp2p/specs/blob/master/kad-dht/README.md#client-and-server-mode for the specs on client mode.
Most of this diff is tests :)
Warning
This changes the behavior of Kademlia's handler's inject_fully_negotiated_inbound. Namely before this change we assumed the dialer spoke the Kademlia protocol, but now we don't. This is because we don't know if the dialer is a client and therefore would not respond to our request or a server and would.
Before we merge this change I think we should make some other change that would let a node know if another node supports kademlia. Currently you'd have to wire up the identify behavior and manually call add_address.
For those interested, this is now deployed on kademlia-exporter.max-inden.de/. See dashboard below to compare with vanilla libp2p v0.44.0.
https://kademlia-exporter.max-inden.de/d/Pfr0Fj6Mk/rust-libp2p?orgId=1&var-data_source=Prometheus&var-instance=kademlia-exporter-ipfs-client-mode:8080&var-instance=kademlia-exporter-ipfs:8080
What else shall we do for this pr merged into master?
I wrote in the PR description:
Before we merge this change I think we should make some other change that would let a node know if another node supports kademlia. Currently you'd have to wire up the identify behavior and manually call add_address.
I think this is still true. Right now if we merge this change we could negatively impact the routing table of servers. Since before they would add a peer when the peer reached out, now they would only do that on dial out or if the peer was manually added. Of course the peer could have been unreachable anyways (say behind a nat) so maybe it's a wash. Still it would be good to have a mechanism that lets the old behavior keep working, instead of changing it completely.
I have some ideas on the change that would be required to get this working. And I'll make an issue that outlines it so we can track the blocker properly.
I have some ideas on the change that would be required to get this working. And I'll make an issue that outlines it so we can track the blocker properly.
Corresponding issue based on a past meeting with @MarcoPolo: https://github.com/libp2p/rust-libp2p/issues/2680
@MarcoPolo and @ all additional input is welcome.
Thanks Max, that's exactly what I had in mind :)
Any updates on this or what would need need to be done?
We would want to do https://github.com/libp2p/rust-libp2p/issues/2680 first in order to address the "warning" section in the initial pull request description. @dariusc93 let me know if that (a) makes sense and (b) whether you would be interested working on any of it.
Setting this to draft until we can proceed.
This pull request has merge conflicts. Could you please resolve them @MarcoPolo? 🙏
Closing this as we will likely ship this in a different PR, thanks kicking it off @MarcoPolo !
See https://github.com/libp2p/rust-libp2p/pull/3651 and related PRs for further progress!