From 4d7e773c429ba94563f88136375935dcd37420ea Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Wed, 16 Mar 2022 21:32:21 +0100 Subject: [PATCH 1/6] fix tests: make bootstrap nodes async --- tests/dht/test_providers.nim | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/dht/test_providers.nim b/tests/dht/test_providers.nim index e952923..748930a 100644 --- a/tests/dht/test_providers.nim +++ b/tests/dht/test_providers.nim @@ -28,9 +28,9 @@ import proc bootstrapNodes( nodecount: int, - bootnodes: openArray[SignedPeerRecord], + bootnodes: seq[SignedPeerRecord], rng = keys.newRng() - ) : seq[(discv5_protocol.Protocol, keys.PrivateKey)] = + ) : Future[seq[(discv5_protocol.Protocol, keys.PrivateKey)]] {.async.} = for i in 0.. Date: Wed, 16 Mar 2022 22:12:57 +0100 Subject: [PATCH 2/6] fix bootstrap: add node after successful handshake Previous version was very conservative about adding nodes to the routing table, requiring a full request/response message exchange initiated by us. Here we also add the node as 'seen' if the whoareyou/handshake exchange was successful. This significantly speeds up bootstreap from 0 Signed-off-by: Csaba Kiraly --- libp2pdht/private/eth/p2p/discoveryv5/node.nim | 3 ++- libp2pdht/private/eth/p2p/discoveryv5/transport.nim | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/libp2pdht/private/eth/p2p/discoveryv5/node.nim b/libp2pdht/private/eth/p2p/discoveryv5/node.nim index cf61e47..4e7a854 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/node.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/node.nim @@ -28,7 +28,8 @@ type address*: Option[Address] record*: SignedPeerRecord seen*: bool ## Indicates if there was at least one successful - ## request-response with this node. + ## request-response with this node, or if the nde was verified + ## through the underlying transport mechanisms. func toNodeId*(pk: keys.PublicKey): NodeId = ## Convert public key to a node identifier. diff --git a/libp2pdht/private/eth/p2p/discoveryv5/transport.nim b/libp2pdht/private/eth/p2p/discoveryv5/transport.nim index c49770c..adff276 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/transport.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/transport.nim @@ -144,6 +144,9 @@ proc receive*(t: Transport, a: Address, packet: openArray[byte]) = # on the next revalidation, one could spam these as the handshake # message occurs on (first) incoming messages. if node.address.isSome() and a == node.address.get(): + # TODO: maybe here we could verify that the address matches what we were + # sending the 'whoareyou' message to. In that case, we can set 'seen' + node.seen = true if t.client.addNode(node): trace "Added new node to routing table after handshake", node else: From 0431587765e9b3563e8e6bb033b32ad8b7411fc9 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Wed, 16 Mar 2022 22:14:01 +0100 Subject: [PATCH 3/6] test_providers: add optional delay during bootstrap Signed-off-by: Csaba Kiraly --- tests/dht/test_providers.nim | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/dht/test_providers.nim b/tests/dht/test_providers.nim index 748930a..0897971 100644 --- a/tests/dht/test_providers.nim +++ b/tests/dht/test_providers.nim @@ -29,15 +29,19 @@ import proc bootstrapNodes( nodecount: int, bootnodes: seq[SignedPeerRecord], - rng = keys.newRng() + rng = keys.newRng(), + delay: int = 0 ) : Future[seq[(discv5_protocol.Protocol, keys.PrivateKey)]] {.async.} = + debug "---- STARTING BOOSTRAPS ---" for i in 0.. 0: + await sleepAsync(chronos.milliseconds(delay)) + #await allFutures(result.mapIt(it.bootstrap())) # this waits for bootstrap based on bootENode, which includes bonding with all its ping pongs From d14222e8d1cfc34a0a24df1d2e9f7acbcc309bfa Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Wed, 16 Mar 2022 22:25:12 +0100 Subject: [PATCH 4/6] tests: add back test after bootnode dies Signed-off-by: Csaba Kiraly --- tests/dht/test_providers.nim | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/dht/test_providers.nim b/tests/dht/test_providers.nim index 0897971..9b9cfe4 100644 --- a/tests/dht/test_providers.nim +++ b/tests/dht/test_providers.nim @@ -236,14 +236,17 @@ suite "Providers Tests: 20 nodes": debug "Providers:", providers check (providers.len == 1 and providers[0].data.peerId == peerRec0.peerId) - # test "20 nodes, retieve after bootnode dies": - # # TODO: currently this is not working even with a 2 minute timeout - # debug "---- KILLING BOOTSTRAP NODE ---" - # await nodes[0].closeWait() + test "20 nodes, retieve after bootnode dies": + debug "---- KILLING BOOTSTRAP NODE ---" + let (node0, _) = nodes[0] + let (node18, _) = nodes[^2] + await node0.closeWait() + nodes.del(0) - # debug "---- STARTING PROVIDERS LOOKUP ---" - # let providers = await nodes[^2].getProviders(targetId) - # debug "Providers:", providers + debug "---- STARTING PROVIDERS LOOKUP ---" + let providersRes = await node18.getProviders(targetId) - # debug "---- STARTING CHECKS ---" - # check (providers.len == 1 and providers[0].peerId == nodes[0].toPeerRecord.peerId) + debug "---- STARTING CHECKS ---" + let providers = providersRes.get + debug "Providers:", providers + check (providers.len == 1 and providers[0].data.peerId == peerRec0.peerId) From 50df73da1b559355df881723c2c4b73390bd5879 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 18 Mar 2022 11:01:14 +0100 Subject: [PATCH 5/6] fixup: tests: expose startup delay parameter --- tests/dht/test_providers.nim | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/dht/test_providers.nim b/tests/dht/test_providers.nim index 9b9cfe4..27bff65 100644 --- a/tests/dht/test_providers.nim +++ b/tests/dht/test_providers.nim @@ -47,7 +47,8 @@ proc bootstrapNodes( proc bootstrapNetwork( nodecount: int, - rng = keys.newRng() + rng = keys.newRng(), + delay: int = 0 ) : Future[seq[(discv5_protocol.Protocol, keys.PrivateKey)]] {.async.} = let @@ -60,7 +61,8 @@ proc bootstrapNetwork( var res = await bootstrapNodes(nodecount - 1, @[bootnode.localNode.record], - rng) + rng, + delay) res.insert((bootNode, bootNodeKey), 0) return res From 4362c83a5680c2fd877b495d230b47653b0c4677 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 18 Mar 2022 11:02:16 +0100 Subject: [PATCH 6/6] log improvements --- libp2pdht/private/eth/p2p/discoveryv5/protocol.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim index 9cf3d6b..00670fa 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim @@ -375,7 +375,7 @@ proc replaceNode(d: Protocol, n: Node) = # For now we never remove bootstrap nodes. It might make sense to actually # do so and to retry them only in case we drop to a really low amount of # peers in the routing table. - debug "Message request to bootstrap node failed", spr = toURI(n.record) + debug "Message request to bootstrap node failed", src=d.localNode, dst=n proc waitMessage(d: Protocol, fromNode: Node, reqId: RequestId):