search

libp2p / go-libp2p-kad-dht

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

GetClosestPeers returning the zero address. #117

Closed Stebalien closed 6 years ago

Stebalien commented 6 years ago

We should be judiciously stripping these out.

See https://github.com/libp2p/go-libp2p-kad-dht/issues/88#issuecomment-365496992

dirkmc commented 6 years ago

I'm not sure if I was clear in my explanation. I meant that for the peers returned from GetClosestPeers len(p.Addrs) == 0. I don't think this causes any issues but if you'd like to make the code more consistent I'd be happy to submit a fix.

Stebalien commented 6 years ago

I see. Yeah, it looks like we're filtering that info in some places but not others. I wonder if we should modify the PeerInfos function in the peerstore to omit peers about which we have no info.

dirkmc commented 6 years ago

This is what I was getting at: https://github.com/libp2p/go-libp2p-kad-dht/pull/118

dirkmc commented 6 years ago

I think we can close this as the PR has been merged

Stebalien commented 6 years ago

Good point.

upperwal commented 5 years ago

@Stebalien

Same is true for FindProviders. Is that by design?

going by this comment

// ProviderAddrTTL is the TTL of an address we've received from a provider.
// This is also a temporary address, but lasts longer. After this expires,
// the records we return will require an extra lookup.
ProviderAddrTTL = time.Minute * 10
Stebalien commented 5 years ago

Yes. After that time period elapses, the requester will have to go back to the DHT a second time to find new addresses for that provider.

Once we finish moving the peerstore to disk and improve the dialer a bit, we could consider keeping these records longer.