libp2p / go-libp2p-kad-dht

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

Increased ProviderAddrTTL from RFM17.1 isn't applied #868

Closed dennis-tra closed 11 months ago

dennis-tra commented 11 months ago

Looking through the code, I believe that the increased ProviderAddrTTL doesn’t have any effect. We increased that value as a result of RFM 17.1.

In this line we pass the new provider of a certain key to the ProviderManager:

dht.providerStore.AddProvider(ctx, key, peer.AddrInfo{ID: p})

The value that we're passing is just peer.AddrInfo{ID: p}, so there are no addresses attached to it. In the ProviderManger this value gets handled here:

if provInfo.ID != pm.self {
  pm.pstore.AddAddrs(provInfo.ID, provInfo.Addrs, ProviderAddrTTL)
}

provInfo is the peer.AddrInfo{ID: p} from above. This means the Addrs field will be always empty, so, I believe, ProviderAddrTTL won't have any effect.

However, we're still keeping the addresses around for a bit because the peer that stores the records with us gets added to the peerstore just because it connected to us - the TTL of their addresses will just be different. I think the values are ConnectedAddrTTL while we're connected to that peer (basically infinite TTL) and TempAddrTTL after we have disconnected. The latter value is set to 2 minutes.

cc @cortze

dennis-tra commented 11 months ago

This is how it worked initially: https://github.com/libp2p/go-libp2p-kad-dht/commit/3b37c43171714b2f3f4657b05d6cd95bc4be9a06

    for _, pi := range pinfos {
        if pi.ID != p {
            // we should ignore this provider reccord! not from originator.
            // (we chould sign them and check signature later...)
            log.Errorf("handleAddProvider received provider %s from %s. Ignore.", pi.ID, p)
            continue
        }

        if len(pi.Addrs) < 1 {
            log.Errorf("got no valid addresses for provider %s. Ignore.", p)
            continue
        }

        log.Infof("received provider %s for %s (addrs: %s)", p, key, pi.Addrs)
        for _, maddr := range pi.Addrs {
            // add the received addresses to our peerstore.
            dht.peerstore.AddAddress(p, maddr)
        }
        dht.providers.AddProvider(key, p)
    }

That's where it capsized: https://github.com/libp2p/go-libp2p-kad-dht/commit/7724838e46d1822b28fd20611ab9815d4ce2d94f