dht icon indicating copy to clipboard operation
dht copied to clipboard

Possible Bug: `AddPeer` stores only IP as key, but `GetPeers` expects full NodeAddr binary

Open adgsm opened this issue 3 months ago • 1 comments

Hi, thanks for maintaining this project! While reading through the peer store code, I noticed what might be an inconsistency in how peers are added vs. retrieved. I’d like to confirm if my understanding is correct.

Flow I observed

AddPeer does the following:

  1. Uses string(na.IP) as the key
  2. Stores NodeAndTime{na, time.Now()} in the map

Meanwhile, GetPeers:

  • Iterates over the keys
  • Calls UnmarshalBinary([]byte(key)) on each key

Potential issue

  • AddPeer is only storing the IP address string as the key.
  • GetPeers appears to expect the key to be the full binary representation of a NodeAddr (i.e., IP + Port).

This means the port information is discarded on insert, but later expected during retrieval.

Suggested fix

Instead of using just the IP string, store the full binary representation of the NodeAddr as the key. For example:

// Use full binary representation (IP + Port) as key instead of just IP
binaryData, err := na.MarshalBinary()
if err != nil {
    panic(err) // Should never happen for valid NodeAddr
}
m[string(binaryData)] = NodeAndTime{na, time.Now()}

adgsm avatar Sep 28 '25 08:09 adgsm

Sorry, I haven't had time to look at this yet.

anacrolix avatar Nov 21 '25 02:11 anacrolix