DHT - better peers to query impl

I have a question about betterPeersToQuery(): There is a comment in the code for both the go and js versions of the DHT that says:
betterPeersToQuery returns nearestPeersToQuery, but if and only if closer than self.

However it seems that this is not strictly true. The code filters the local node out of the list of closer peers, but doesn’t check that the peers are actually closer than the local node.

In practice, it may not matter much, because

  • a node that is close to the target key will know about all the peers in that neighbourhood, so it will return close peers to the key (even if they’re not strictly closer than itself)
  • a node that is far from the target key will know about a lot of peers in between itself and the key, so probably the list of closer peers will all be closer than the local peer.

However it might make a difference if

  • the node is new, and doesn’t have much information about the network yet
  • a malicious or malfunctioning remote peer returns a list of far away nodes

You’re right, it does look like only the local node and the peer initiating the query are filtered out. It’s possible that nearestPeersToQuery already filtered out more distant nodes, but that’s not clear from the comments & I don’t know where the main implementation lives.

Have you observed more distant peers being returned? If so, that answers the question about whether neerestPeersToQuery is filtering them…

If it is returning more distant nodes, then it seems worth fixing, if only so the comment is telling the truth.

I haven’t dug into the go code - I tested the JS code and it seems like it is returning more distant nodes than the key

I agree both that the comment in the code about nearestPeersToQuery is , and that it shouldn’t matter.

The DHT generally, and especially this logic, needs a rewrite soon. See this discussion.