Kademlia doesn't add peers automatically on connection established

[tqwewe]

When using Kademlia with mDNS, when peers are found I get a Discovered remote peer log message printed, and the Kademlia DHT gets shared with this new peer. However when I manually dial a peer which is not discoverable with mDNS using swarm.dial(...), the DHT isn't shared with this new peer and I need to manually call kad::Behaviour::add_address on SwarmEvent::ConnectionEstablished events.

Why must I call kad::Behaviour::add_address manually when dialing a peer, but not when mDNS auto discovers peers?

The event handling I had to add to fix this Kademlia dial issues is:

match event {
    SwarmEvent::ConnectionEstablished {
        peer_id, endpoint, ..
    } => {
        self.swarm
            .add_peer_address(peer_id, endpoint.get_remote_address().clone());
        self.swarm
            .behaviour_mut()
            .kademlia
            .add_address(&peer_id, endpoint.get_remote_address().clone());
    }
    SwarmEvent::ConnectionClosed {
        peer_id, endpoint, ..
    } => {
        self.swarm
            .remove_external_address(endpoint.get_remote_address());
        self.swarm
            .behaviour_mut()
            .kademlia
            .remove_address(&peer_id, endpoint.get_remote_address());
    }
}

I also want to check is code I added correct? My Behaviour struct includes:

#[derive(NetworkBehaviour)]
struct Behaviour {
    kademlia: kad::Behaviour<MemoryStore>,
    request_response: request_response::cbor::Behaviour<SwarmReq, SwarmResp>,
    mdns: mdns::tokio::Behaviour,
    autonat: autonat::Behaviour,
}

I'm not sure if I need to call add_address on any of these other behaviours or if its just Kademlia that needs special handling.

This looks related: #3877

[oblique]

I don't know why it doesn't add the address, maybe @thomaseizinger will know. However it will add them when #5313 is implemented, but with a different event - identify will generate NewExternalAddrOfPeer and which at the end it will be forwarded and consumed by kad.

Another thing about your code: If I'm not mistaken you shouldn't remove addresses on ConnectionClosed because kad tries to reestablish close connections to keep you connected to the network. It is a normal scenario for the remote peer to close the connection just because you don't send requests to it.

[tqwewe]

Great thanks for the reply, so perhaps for now I need to call add_address on kademlia, but after that PR is merged I'll no longer need it hopefully.

I also assume I don't really need this line?

self.swarm.add_peer_address(peer_id, endpoint.get_remote_address().clone());

[oblique]

add_peer_address only generates NewExternalAddrOfPeer, so if you use identify, you don't really need it. Unless if you want to explicitly add an address that is not advertised by identify of the remote peer.

Personally I use kademlia's add_address on identify::Event::Received event instead of ConnectionEstablished event. This is the equivalent of what #5313 will implement.