Allow for discv4 chronos strict usage

And group p2p tests that can be run with strict usage along the way.

eth.nimble

@@ -49,13 +49,11 @@ task test_discv4, "Run discovery v4 tests":
 task test_p2p, "Run p2p tests":
   test_discv5_task()
 
+  runTest("tests/p2p/all_tests")
+
+  # Code that still requires chronosStrict = false
   for filename in [
       "les/test_flow_control",
-      "test_auth",
-      "test_crypt",
-      "test_discovery",
-      "test_ecies",
-      "test_enode",
       "test_rlpx_thunk",
       "test_shh",
       "test_shh_config",

eth/p2p/discovery.nim

@@ -103,14 +103,14 @@ proc expiration(): uint32 =
 
 # Wire protocol
 
-proc send(d: DiscoveryProtocol, n: Node, data: seq[byte]) =
+proc send(d: DiscoveryProtocol, n: Node, data: seq[byte]) {.raises: [Defect].} =
   let ta = initTAddress(n.node.address.ip, n.node.address.udpPort)
   let f = d.transp.sendTo(ta, data)
   f.callback = proc(data: pointer) {.gcsafe.} =
     if f.failed:
       debug "Discovery send failed", msg = f.readError.msg
 
-proc sendPing*(d: DiscoveryProtocol, n: Node): seq[byte] =
+proc sendPing*(d: DiscoveryProtocol, n: Node): seq[byte] {.raises: [Defect].} =
   let payload = rlp.encode((PROTO_VERSION, d.address, n.node.address,
                             expiration()))
   let msg = pack(cmdPing, payload, d.privKey)
@@ -233,8 +233,8 @@ proc expirationValid(cmdId: CommandId, rlpEncodedPayload: openArray[byte]):
   else:
     raise newException(DiscProtocolError, "Invalid RLP list for this packet id")
 
-proc receive*(d: DiscoveryProtocol, a: Address, msg: openArray[byte]) {.gcsafe.} =
+proc receive*(d: DiscoveryProtocol, a: Address, msg: openArray[byte])
-  ## Can raise `DiscProtocolError` and all of `RlpError`
+    {.raises: [DiscProtocolError, RlpError, ValueError, Defect].} =
   # Note: export only needed for testing
   let msgHash = validateMsgHash(msg)
   if msgHash.isOk():
@@ -260,23 +260,23 @@ proc receive*(d: DiscoveryProtocol, a: Address, msg: openArray[byte]) {.gcsafe.}
   else:
     notice "Wrong msg mac from ", a
 
-proc processClient(transp: DatagramTransport,
+proc processClient(transp: DatagramTransport, raddr: TransportAddress):
-                   raddr: TransportAddress): Future[void] {.async, gcsafe.} =
+    Future[void] {.async, raises: [Defect].} =
   var proto = getUserData[DiscoveryProtocol](transp)
-  try:
+  let buf = try: transp.getMessage()
-    # TODO: Maybe here better to use `peekMessage()` to avoid allocation,
+            except TransportOsError as e:
-    # but `Bytes` object is just a simple seq[byte], and `ByteRange` object
+              # This is likely to be local network connection issues.
-    # do not support custom length.
+              warn "Transport getMessage", exception = e.name, msg = e.msg
-    var buf = transp.getMessage()
+              return
   let a = Address(ip: raddr.address, udpPort: raddr.port, tcpPort: raddr.port)
+  try:
     proto.receive(a, buf)
   except RlpError as e:
     debug "Receive failed", exc = e.name, err = e.msg
   except DiscProtocolError as e:
     debug "Receive failed", exc = e.name, err = e.msg
-  except Exception as e:
+  except ValueError as e:
     debug "Receive failed", exc = e.name, err = e.msg
-    raise e
 
 proc open*(d: DiscoveryProtocol) =
   # TODO allow binding to specific IP / IPv6 / etc

eth/p2p/kademlia.nim

@@ -26,7 +26,7 @@ type
     routing: RoutingTable
     pongFutures: Table[seq[byte], Future[bool]]
     pingFutures: Table[Node, Future[bool]]
-    neighboursCallbacks: Table[Node, proc(n: seq[Node]) {.gcsafe.}]
+    neighboursCallbacks: Table[Node, proc(n: seq[Node]) {.gcsafe, raises: [Defect].}]
     rng: ref BrHmacDrbgContext
 
   NodeId* = UInt256
@@ -255,7 +255,7 @@ proc updateRoutingTable(k: KademliaProtocol, n: Node) {.gcsafe.} =
       # replacement cache.
       asyncCheck k.bond(evictionCandidate)
 
-proc doSleep(p: proc() {.gcsafe.}) {.async, gcsafe.} =
+proc doSleep(p: proc() {.gcsafe, raises: [Defect].}) {.async, gcsafe.} =
   await sleepAsync(REQUEST_TIMEOUT)
   p()
 
@@ -276,7 +276,7 @@ proc waitPong(k: KademliaProtocol, n: Node, pingid: seq[byte]): Future[bool] =
       k.pongFutures.del(pingid)
       fut.complete(false)
 
-proc ping(k: KademliaProtocol, n: Node): seq[byte] =
+proc ping(k: KademliaProtocol, n: Node): seq[byte] {.raises: [Defect].} =
   doAssert(n != k.thisNode)
   result = k.wire.sendPing(n)
 
@@ -290,12 +290,13 @@ proc waitPing(k: KademliaProtocol, n: Node): Future[bool] {.gcsafe.} =
       k.pingFutures.del(n)
       fut.complete(false)
 
-proc waitNeighbours(k: KademliaProtocol, remote: Node): Future[seq[Node]] =
+proc waitNeighbours(k: KademliaProtocol, remote: Node):
+    Future[seq[Node]] {.raises: [Defect].} =
   doAssert(remote notin k.neighboursCallbacks)
   result = newFuture[seq[Node]]("waitNeighbours")
   let fut = result
   var neighbours = newSeqOfCap[Node](BUCKET_SIZE)
-  k.neighboursCallbacks[remote] = proc(n: seq[Node]) =
+  k.neighboursCallbacks[remote] = proc(n: seq[Node]) {.gcsafe, raises: [Defect].} =
     # This callback is expected to be called multiple times because nodes usually
     # split the neighbours replies into multiple packets, so we only complete the
     # future event.set() we've received enough neighbours.

tests/p2p/all_tests.nim

@@ -0,0 +1,6 @@
+import
+  ./test_auth,
+  ./test_crypt,
+  ./test_discovery,
+  ./test_ecies,
+  ./test_enode

tests/p2p/p2p_test_helper.nim

@@ -10,12 +10,6 @@ proc localAddress*(port: int): Address =
   result = Address(udpPort: port, tcpPort: port,
                    ip: parseIpAddress("127.0.0.1"))
 
-proc startDiscoveryNode*(privKey: PrivateKey, address: Address,
-                        bootnodes: seq[ENode]): Future[DiscoveryProtocol] {.async.} =
-  result = newDiscoveryProtocol(privKey, address, bootnodes)
-  result.open()
-  await result.bootstrap()
-
 proc setupTestNode*(
     rng: ref BrHmacDrbgContext,
     capabilities: varargs[ProtocolInfo, `protocolInfo`]): EthereumNode {.gcsafe.} =
@@ -27,13 +21,6 @@ proc setupTestNode*(
   for capability in capabilities:
     result.addCapability capability
 
-proc packData*(payload: openArray[byte], pk: PrivateKey): seq[byte] =
-  let
-    payloadSeq = @payload
-    signature = @(pk.sign(payload).toRaw())
-    msgHash = keccak256.digest(signature & payloadSeq)
-  result = @(msgHash.data) & signature & payloadSeq
-
 template sourceDir*: string = currentSourcePath.rsplit(DirSep, 1)[0]
 
 proc recvMsgMock*(msg: openArray[byte]): tuple[msgId: int, msgData: Rlp] =

tests/p2p/test_auth.nim

@@ -7,6 +7,8 @@
 #    distribution, for details about the copyright.
 #
 
+{.used.}
+
 import
   std/unittest,
   nimcrypto/[utils, keccak],

tests/p2p/test_crypt.nim

@@ -7,6 +7,8 @@
 #    distribution, for details about the copyright.
 #
 
+{.used.}
+
 import
   std/unittest,
   nimcrypto/[utils, sysrand, keccak],

tests/p2p/test_discovery.nim

@@ -7,26 +7,46 @@
 #    distribution, for details about the copyright.
 #
 
+{.used.}
+
 import
-  std/[sequtils, unittest],
+  std/sequtils,
-  chronos, stew/byteutils,
+  chronos, stew/byteutils, nimcrypto, testutils/unittests,
-  ../../eth/[keys, rlp], ../../eth/p2p/[discovery, kademlia, enode],
+  ../../eth/keys, ../../eth/p2p/[discovery, kademlia, enode]
-  ./p2p_test_helper
+
+proc localAddress(port: int): Address =
+  let port = Port(port)
+  result = Address(udpPort: port, tcpPort: port,
+                   ip: parseIpAddress("127.0.0.1"))
+
+proc initDiscoveryNode(privKey: PrivateKey, address: Address,
+                        bootnodes: seq[ENode]): DiscoveryProtocol =
+  let node = newDiscoveryProtocol(privKey, address, bootnodes)
+  node.open()
+
+  return node
+
+proc packData(payload: openArray[byte], pk: PrivateKey): seq[byte] =
+  let
+    payloadSeq = @payload
+    signature = @(pk.sign(payload).toRaw())
+    msgHash = keccak256.digest(signature & payloadSeq)
+  result = @(msgHash.data) & signature & payloadSeq
 
 proc nodeIdInNodes(id: NodeId, nodes: openarray[Node]): bool =
   for n in nodes:
     if id == n.id: return true
 
-proc test() {.async.} =
+procSuite "Discovery Tests":
-  suite "Discovery Tests":
   let
     bootNodeKey = PrivateKey.fromHex(
       "a2b50376a79b1a8c8a3296485572bdfbf54708bb46d3c25d73d2723aaaf6a617")[]
     bootNodeAddr = localAddress(20301)
     bootENode = ENode(pubkey: bootNodeKey.toPublicKey(), address: bootNodeAddr)
-      bootNode = await startDiscoveryNode(bootNodeKey, bootNodeAddr, @[])
+    bootNode = initDiscoveryNode(bootNodeKey, bootNodeAddr, @[])
+  waitFor bootNode.bootstrap()
 
-    test "Discover nodes":
+  asyncTest "Discover nodes":
     let nodeKeys = [
       PrivateKey.fromHex(
         "a2b50376a79b1a8c8a3296485572bdfbf54708bb46d3c25d73d2723aaaf6a618")[],
@@ -35,12 +55,14 @@ proc test() {.async.} =
       PrivateKey.fromHex(
         "a2b50376a79b1a8c8a3296485572bdfbf54708bb46d3c25d73d2723aaaf6a620")[]
     ]
-      var nodeAddrs = newSeqOfCap[Address](nodeKeys.len)
-      for i in 0 ..< nodeKeys.len: nodeAddrs.add(localAddress(20302 + i))
 
-      var nodes = await all(zip(nodeKeys, nodeAddrs).mapIt(
+    var nodes: seq[DiscoveryProtocol]
-        startDiscoveryNode(it[0], it[1], @[bootENode]))
+    for i in 0..<nodeKeys.len:
-      )
+      let node = initDiscoveryNode(nodeKeys[i], localAddress(20302 + i),
+        @[bootENode])
+      nodes.add(node)
+
+    await allFutures(nodes.mapIt(it.bootstrap()))
     nodes.add(bootNode)
 
     for i in nodes:
@@ -97,7 +119,7 @@ proc test() {.async.} =
     # msg mac
     bootNode.receive(address, packData(@[], nodeKey))
 
-    test "Two findNode calls for the same peer in rapid succession":
+  asyncTest "Two findNode calls for the same peer in rapid succession":
     let targetKey = PrivateKey.fromHex(
         "a2b50376a79b1a8c8a3296485572bdfbf54708bb46d3c25d73d2723aaaf6a618")[]
     let peerKey = PrivateKey.fromHex(
@@ -121,5 +143,3 @@ proc test() {.async.} =
     # Just for completeness, wait for the first result out of order.
     # Max delay 5 seconds.
     discard await neighbours1Future
-
-waitFor test()

tests/p2p/test_ecies.nim

@@ -7,6 +7,8 @@
 #    distribution, for details about the copyright.
 #
 
+{.used.}
+
 import
   std/unittest,
   nimcrypto/[utils, sha2, hmac, rijndael],

tests/p2p/test_enode.nim

@@ -7,6 +7,8 @@
 #  Apache License, version 2.0, (LICENSE-APACHEv2)
 #            MIT license (LICENSE-MIT)
 
+{.used.}
+
 import
   std/[unittest, net, options],
   ../../eth/p2p/enode