Fix idAtDistance (#219)

eth/p2p/discoveryv5/routing_table.nim

@@ -18,9 +18,17 @@ const
   BITS_PER_HOP = 8
   ID_SIZE = 256
 
-proc distanceTo(n: Node, id: NodeId): UInt256 = n.id xor id
+proc distanceTo(n: Node, id: NodeId): UInt256 =
+  ## Calculate the distance to a NodeId.
+  n.id xor id
 
 proc logDist*(a, b: NodeId): uint32 =
+  ## Calculate the logarithmic distance between two `NodeId`s.
+  ##
+  ## According the specification, this is the log base 2 of the distance. But it
+  ## is rather the log base 2 of the distance + 1, as else the 0 value can not
+  ## be used (e.g. by FindNode call to return peer its own ENR)
+  ## For NodeId of 256 bits, range is 0-256.
   let a = a.toBytes
   let b = b.toBytes
   var lz = 0
@@ -157,6 +165,8 @@ proc addNode*(r: var RoutingTable, n: Node): Node =
     # to 0 mod BITS_PER_HOP
 
     let depth = computeSharedPrefixBits(bucket.nodes)
+    # TODO: Shouldn't the adding to replacement cache be done only if the bucket
+    # doesn't get split?
     if bucket.inRange(r.thisNode) or (depth mod BITS_PER_HOP != 0 and depth != ID_SIZE):
       r.splitBucket(r.buckets.find(bucket))
       return r.addNode(n) # retry
@@ -191,10 +201,16 @@ proc neighbours*(r: RoutingTable, id: NodeId, k: int = BUCKET_SIZE): seq[Node] =
   if result.len > k:
     result.setLen(k)
 
-proc idAtDistance(id: NodeId, dist: uint32): NodeId =
+proc idAtDistance*(id: NodeId, dist: uint32): NodeId =
-  id and (Uint256.high shl dist.int)
+  ## Calculate the "lowest" `NodeId` for given logarithmic distance.
+  ## A logarithmic distance obviously covers a whole range of distances and thus
+  ## potential `NodeId`s.
+  # xor the NodeId with 2^(d - 1) or one could say, calculate back the leading
+  # zeroes and xor those` with the id.
+  id xor (1.stuint(256) shl (dist.int - 1))
 
 proc neighboursAtDistance*(r: RoutingTable, distance: uint32, k: int = BUCKET_SIZE): seq[Node] =
+  # TODO: Filter out nodes with not exact distance here?
   r.neighbours(idAtDistance(r.thisNode.id, distance), k)
 
 proc len*(r: RoutingTable): int =

tests/p2p/test_discoveryv5.nim

@@ -218,10 +218,15 @@ suite "Discovery v5 Tests":
         ("0x0002", 2'u32),
         ("0x0003", 2'u32),
         ("0x0004", 3'u32),
+        ("0x0007", 3'u32),
         ("0x0008", 4'u32),
+        ("0x000f", 4'u32),
+        ("0x0080", 8'u32),
         ("0x00ff", 8'u32),
         ("0x0100", 9'u32),
-        ("0xf000", 16'u32)
+        ("0x01ff", 9'u32),
+        ("0x8000", 16'u32),
+        ("0xffff", 16'u32)
       ]
 
     for (id, d) in testValues:
@@ -244,3 +249,36 @@ suite "Discovery v5 Tests":
     for (key, d) in testValues:
       let id = toNodeId(PrivateKey.fromHex(key)[].toPublicKey()[])
       check logDist(targetId, id) == d
+
+  test "Distance to id check":
+    const
+      targetId = "0x0000"
+      testValues = [ # possible id in that distance range
+        ("0x0001", 1'u32),
+        ("0x0002", 2'u32),
+        ("0x0004", 3'u32),
+        ("0x0008", 4'u32),
+        ("0x0080", 8'u32),
+        ("0x0100", 9'u32),
+        ("0x8000", 16'u32)
+      ]
+
+    for (id, d) in testValues:
+      check idAtDistance(parse(targetId, UInt256, 16), d) == parse(id, UInt256, 16)
+
+  test "Distance to id check with keys":
+    const
+      targetKey = "5d485bdcbe9bc89314a10ae9231e429d33853e3a8fa2af39f5f827370a2e4185e344ace5d16237491dad41f278f1d3785210d29ace76cd627b9147ee340b1125"
+      testValues = [ # possible id in that distance range
+        ("9e5b34809116e3790b2258a45e7ef03b11af786503fb1a6d4b4a8ca021ad653c", 251'u32),
+        ("925b34809116e3790b2258a45e7ef03b11af786503fb1a6d4b4a8ca021ad653c", 252'u32),
+        ("8a5b34809116e3790b2258a45e7ef03b11af786503fb1a6d4b4a8ca021ad653c", 253'u32),
+        ("ba5b34809116e3790b2258a45e7ef03b11af786503fb1a6d4b4a8ca021ad653c", 254'u32),
+        ("da5b34809116e3790b2258a45e7ef03b11af786503fb1a6d4b4a8ca021ad653c", 255'u32),
+        ("1a5b34809116e3790b2258a45e7ef03b11af786503fb1a6d4b4a8ca021ad653c", 256'u32)
+      ]
+
+    let targetId = toNodeId(PublicKey.fromHex(targetKey)[])
+
+    for (id, d) in testValues:
+      check idAtDistance(targetId, d) == parse(id, UInt256, 16)