Add & update tests

eth/p2p/discoveryv5/protocol.nim

@@ -699,10 +699,10 @@ proc resolve*(d: Protocol, id: NodeId): Future[Option[Node]]
     {.async, raises: [Exception, Defect].} =
   ## Resolve a `Node` based on provided `NodeId`.
   ##
-  ## This will first look in the own DHT. If the node is known, it will try to
+  ## This will first look in the own routing table. If the node is known, it
-  ## contact if for newer information. If node is not known or it does not
+  ## will try to contact if for newer information. If node is not known or it
-  ## reply, a lookup is done to see if it can find a (newer) record of the node
+  ## does not reply, a lookup is done to see if it can find a (newer) record of
-  ## on the network.
+  ## the node on the network.
 
   let node = d.getNode(id)
   if node.isSome():
@@ -715,8 +715,6 @@ proc resolve*(d: Protocol, id: NodeId): Future[Option[Node]]
   let discovered = await d.lookup(id)
   for n in discovered:
     if n.id == id:
-      # TODO: Not getting any new seqNum here as in a lookup nodes in table with
-      # new seqNum don't get replaced.
       if node.isSome() and node.get().record.seqNum >= n.record.seqNum:
         return node
       else:

tests/p2p/test_discoveryv5.nim

@@ -313,8 +313,8 @@ procSuite "Discovery v5 Tests":
 
     var targetSeqNum = targetNode.localNode.record.seqNum
 
-    # Populate DHT with target through a ping. Next, close target and see
+    # Populate routing table with target through a ping. Next, close target and
-    # if resolve works (only local lookup)
+    # see if resolve works (only local getNode).
     block:
       let pong = await targetNode.ping(mainNode.localNode)
       check pong.isOk()
@@ -324,13 +324,14 @@ procSuite "Discovery v5 Tests":
         n.isSome()
         n.get().id == targetId
         n.get().record.seqNum == targetSeqNum
+    # Node will be removed because of failed findNode request.
 
     # Bring target back online, update seqNum in ENR, check if we get the
     # updated ENR.
     block:
       targetNode.open()
       # ping to node again to add as it was removed after failed findNode in
-      # resolve in previous test block
+      # resolve in previous test block.
       let pong = await targetNode.ping(mainNode.localNode)
       check pong.isOk()
 
@@ -339,13 +340,22 @@ procSuite "Discovery v5 Tests":
       let update = targetNode.updateRecord({"addsomefield": @[byte 1]})
       check update.isOk()
 
-      let n = await mainNode.resolve(targetId)
+      var n = mainNode.getNode(targetId)
+      check:
+        n.isSome()
+        n.get().id == targetId
+        n.get().record.seqNum == targetSeqNum - 1
+
+      n = await mainNode.resolve(targetId)
       check:
         n.isSome()
         n.get().id == targetId
         n.get().record.seqNum == targetSeqNum
 
-    # Update seqNum in ENR again, ping lookupNode to be added in DHT,
+      # Add the updated version
+      check mainNode.addNode(n.get())
+
+    # Update seqNum in ENR again, ping lookupNode to be added in routing table,
     # close targetNode, resolve should lookup, check if we get updated ENR.
     block:
       targetSeqNum.inc()
@@ -357,11 +367,8 @@ procSuite "Discovery v5 Tests":
       # findNode request
       check (await lookupNode.ping(targetNode.localNode)).isOk()
       await targetNode.closeWait()
-      # TODO: This step should eventually not be needed and ENRs with new seqNum
-      # should just get updated in the lookup.
-      await mainNode.revalidateNode(targetNode.localNode)
 
-      check mainNode.addNode(lookupNode.localNode)
+      check mainNode.addNode(lookupNode.localNode.record)
       let n = await mainNode.resolve(targetId)
       check:
         n.isSome()
@@ -419,6 +426,73 @@ procSuite "Discovery v5 Tests":
         db, ip, port, port, rng = rng,
         previousRecord = some(updatesNode.getRecord()))
 
+  asyncTest "Update node record with revalidate":
+    let
+      mainNode =
+        initDiscoveryNode(rng, PrivateKey.random(rng[]), localAddress(20301))
+      testNode =
+        initDiscoveryNode(rng, PrivateKey.random(rng[]), localAddress(20302))
+      testNodeId = testNode.localNode.id
+
+    check:
+      # Get node with current ENR in routing table.
+      # Handshake will get done here.
+      (await testNode.ping(mainNode.localNode)).isOk()
+      testNode.updateRecord({"test" : @[byte 1]}).isOk()
+      testNode.localNode.record.seqNum == 2
+
+    # Get the node from routing table, seqNum should still be 1.
+    var n = mainNode.getNode(testNodeId)
+    check:
+      n.isSome()
+      n.get.record.seqNum == 1
+
+    # This should not do a handshake and thus the new ENR must come from the
+    # findNode(0)
+    await mainNode.revalidateNode(n.get)
+
+    # Get the node from routing table, and check if record got updated.
+    n = mainNode.getNode(testNodeId)
+    check:
+      n.isSome()
+      n.get.record.seqNum == 2
+
+    await mainNode.closeWait()
+    await testNode.closeWait()
+
+  asyncTest "Update node record with handshake":
+    let
+      mainNode =
+        initDiscoveryNode(rng, PrivateKey.random(rng[]), localAddress(20301))
+      testNode =
+        initDiscoveryNode(rng, PrivateKey.random(rng[]), localAddress(20302))
+      testNodeId = testNode.localNode.id
+
+    # Add the node (from the record, so new node!) so no handshake is done yet.
+    check: mainNode.addNode(testNode.localNode.record)
+
+    check:
+      testNode.updateRecord({"test" : @[byte 1]}).isOk()
+      testNode.localNode.record.seqNum == 2
+
+    # Get the node from routing table, seqNum should still be 1.
+    var n = mainNode.getNode(testNodeId)
+    check:
+      n.isSome()
+      n.get.record.seqNum == 1
+
+    # This should do a handshake and update the ENR through that.
+    check (await testNode.ping(mainNode.localNode)).isOk()
+
+    # Get the node from routing table, and check if record got updated.
+    n = mainNode.getNode(testNodeId)
+    check:
+      n.isSome()
+      n.get.record.seqNum == 2
+
+    await mainNode.closeWait()
+    await testNode.closeWait()
+
   test "Verify records of nodes message":
     let
       port = Port(9000)

tests/p2p/test_discv5_encoding.nim

@@ -241,15 +241,27 @@ suite "Discovery v5 Additional":
       nodeId = pubKey.toNodeId()
       idNonce = hexToByteArray[idNonceSize](
         "0xa77e3aa0c144ae7c0a3af73692b7d6e5b7a2fdc0eda16e8d5e6cb0d08e88dd04")
-      whoareyou = Whoareyou(idNonce: idNonce, recordSeq: 0, pubKey: some(pubKey))
       c = Codec(localNode: node, privKey: privKey)
 
-    let (auth, _) = encodeAuthHeader(rng[], c, nodeId, nonce, whoareyou)
+    block: # With ENR
-    var rlp = rlpFromBytes(auth)
+      let
-    let authHeader = rlp.read(AuthHeader)
+        whoareyou = Whoareyou(idNonce: idNonce, recordSeq: 0, pubKey: some(pubKey))
-    var newNode: Node
+        (auth, _) = encodeAuthHeader(rng[], c, nodeId, nonce, whoareyou)
-    let secrets = c.decodeAuthResp(privKey.toPublicKey().toNodeId(),
+      var rlp = rlpFromBytes(auth)
-      authHeader, whoareyou, newNode)
+      let authHeader = rlp.read(AuthHeader)
+      var newNode: Node
+      let secrets = c.decodeAuthResp(privKey.toPublicKey().toNodeId(),
+        authHeader, whoareyou, newNode)
+
+    block: # Without ENR
+      let
+        whoareyou = Whoareyou(idNonce: idNonce, recordSeq: 1, pubKey: some(pubKey))
+        (auth, _) = encodeAuthHeader(rng[], c, nodeId, nonce, whoareyou)
+      var rlp = rlpFromBytes(auth)
+      let authHeader = rlp.read(AuthHeader)
+      var newNode: Node
+      let secrets = c.decodeAuthResp(privKey.toPublicKey().toNodeId(),
+        authHeader, whoareyou, newNode)
 
     # TODO: Test cases with invalid nodeId and invalid signature, the latter
     # is in the current code structure rather difficult and would need some