libp2p / go-libp2p-kad-dht

A Kademlia DHT implementation on go-libp2p
https://github.com/libp2p/specs/tree/master/kad-dht
MIT License
516 stars 220 forks source link

Bootstrap process not compatible with rust libp2p-kad #966

Closed xinaxu closed 2 months ago

xinaxu commented 3 months ago

In Pull Request #820, additional validation is added to check if the bootstrap node should be put into the routing table - if it fails to return itself for finding itself, then it won't be put into the routing table https://github.com/libp2p/go-libp2p-kad-dht/blob/1b5d0b69ec72752f651145f9f3a8aded499ab454/dht.go#L383

The logic indicates, when querying FIND_NODE for the destination peer, it is expecting the destination node to return at least themselves.

However, the logic in rust libp2p-kad does not include themselves in the response. The result will be, a golang DHT client cannot use a rust DHT client for bootstrapping. https://github.com/libp2p/rust-libp2p/blob/a579e691c46a8efd2e38ebafca1c59a9b58fc56c/protocols/kad/src/behaviour.rs#L1184

The error shown is

2024-03-22T12:10:47.251-0700    ERROR   dht     go-libp2p-kad-dht/dht.go:694    connected peer not answering DHT request as expected    {"peer": "12D3KooWPRvdxAyaoq6hqBxCZkFknNW7k4aogJtQqWrQENmBEZWx", "error": "peer 12D3KooWPRvdxAyaoq6hqBxCZkFknNW7k4aogJtQqWrQENmBEZWx failed to return its closest peers, got 0
"}
xinaxu commented 3 months ago

@guillaumemichel @Jorropo

Jorropo commented 3 months ago

Right I overlooked the situation where you creating a new DHT from thin air. Can this happen if rust-libp2p is bootstrapped ? (afait no because it would return other peers too)

I wanted to use the DHT Ping RPC query (for performance reasons), but it is deprecated and we didn't went that route, that could solve issues like this.

I also still think this kind of hardening and edge cases should be written down in a spec but there were no interest last time I had discussions about this.

dennis-tra commented 3 months ago

As I understand it, this is a general problem, even if rust-libp2p is bootstrapped. The issue is just that rust-libp2p never returns itself and hence the validation never passes.

As I see it, the peer itself is the only entry in its highest bucket, so it makes sense to return itself if asked for peers in that highest bucket. Therefore, I think, the better solution would be that rust-libp2p just returned itself.

guillaumemichel commented 3 months ago

The specs specify that FIND_NODE should return the closest nodes to the requested key. Hence, both go-libp2p-kad-dht and rust-libp2p don't follow the specs.

When requested for self's key:

Implicitly, a node wouldn't have to return itself (because the requester is already aware), but it should return a list of its closest peers.


@xinaxu is right, it is an issue that go-libp2p-kad-dht nodes don't accept rust-libp2p nodes in their buckets. A quick fix could be to modify the lookupCheck to request ~one's own key, instead of requesting the remote node's key~.

What really matters is that the remote node is able to provide at least 1 peer in response to FIND_NODE.

guillaumemichel commented 3 months ago

Just suggested a change in rust-libp2p to address the issue

See https://github.com/libp2p/rust-libp2p/issues/5269 and https://github.com/libp2p/rust-libp2p/pull/5270

guillaumemichel commented 2 months ago

https://github.com/libp2p/rust-libp2p/pull/5270 has been merged. It makes sure that rust-libp2p returns multiple peers when being requested for its own peer id. It used to return 0 peers in the past, which is why the rust-libp2p nodes were never added to go-libp2p-kad-dht buckets.

Also see: https://github.com/libp2p/go-libp2p-kad-dht/pull/968