diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index 3f4a35f..6b2d744 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/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 - ## contact if for newer information. If node is not known or it does not - ## reply, a lookup is done to see if it can find a (newer) record of the node - ## on the network. + ## This will first look in the own routing table. If the node is known, it + ## will try to contact if for newer information. If node is not known or it + ## does not reply, a lookup is done to see if it can find a (newer) record of + ## 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: diff --git a/tests/p2p/test_discoveryv5.nim b/tests/p2p/test_discoveryv5.nim index a03da9f..5275df0 100644 --- a/tests/p2p/test_discoveryv5.nim +++ b/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 - # if resolve works (only local lookup) + # Populate routing table with target through a ping. Next, close target and + # 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) diff --git a/tests/p2p/test_discv5_encoding.nim b/tests/p2p/test_discv5_encoding.nim index 11f52b7..5c153f0 100644 --- a/tests/p2p/test_discv5_encoding.nim +++ b/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) - var rlp = rlpFromBytes(auth) - let authHeader = rlp.read(AuthHeader) - var newNode: Node - let secrets = c.decodeAuthResp(privKey.toPublicKey().toNodeId(), - authHeader, whoareyou, newNode) + block: # With ENR + let + whoareyou = Whoareyou(idNonce: idNonce, recordSeq: 0, 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) + + 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