Adjust for chronosStrictException usage in rest of eth/p2p

eth.nimble

@@ -44,22 +44,11 @@ task test_discv5, "Run discovery v5 tests":
   runTest("tests/p2p/all_discv5_tests")
 
 task test_discv4, "Run discovery v4 tests":
-  runTest("tests/p2p/test_discovery", chronosStrict = false)
+  runTest("tests/p2p/test_discovery")
 
 task test_p2p, "Run p2p tests":
   runTest("tests/p2p/all_tests")
 
-  # Code that still requires chronosStrict = false
-  for filename in [
-      "les/test_flow_control",
-      "test_rlpx_thunk",
-      "test_shh",
-      "test_shh_config",
-      "test_shh_connect",
-      "test_protocol_handlers"
-    ]:
-    runTest("tests/p2p/" & filename, chronosStrict = false)
-
 task test_rlp, "Run rlp tests":
   # workaround for github action CI
   # mysterious crash on windows-2019 64bit mode

eth/common/eth_types.nim

@@ -368,10 +368,12 @@ template deref*(b: Blob): auto = b
 template deref*(o: Option): auto = o.get
 template deref*(r: EthResourceRefs): auto = r[]
 
-method genesisHash*(db: AbstractChainDB): KeccakHash {.base, gcsafe.} =
+method genesisHash*(db: AbstractChainDB): KeccakHash
+    {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
-method getBlockHeader*(db: AbstractChainDB, b: HashOrNum, output: var BlockHeader): bool {.base, gcsafe.} =
+method getBlockHeader*(db: AbstractChainDB, b: HashOrNum,
+    output: var BlockHeader): bool {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
 proc getBlockHeader*(db: AbstractChainDB, hash: KeccakHash): BlockHeaderRef {.gcsafe.} =
@@ -384,22 +386,29 @@ proc getBlockHeader*(db: AbstractChainDB, b: BlockNumber): BlockHeaderRef {.gcsa
   if not db.getBlockHeader(HashOrNum(isHash: false, number: b), result[]):
     return nil
 
-method getBestBlockHeader*(self: AbstractChainDB): BlockHeader {.base, gcsafe.} =
+method getBestBlockHeader*(self: AbstractChainDB): BlockHeader
+    {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
-method getSuccessorHeader*(db: AbstractChainDB, h: BlockHeader, output: var BlockHeader, skip = 0'u): bool {.base, gcsafe.} =
+method getSuccessorHeader*(db: AbstractChainDB, h: BlockHeader,
+    output: var BlockHeader, skip = 0'u): bool
+    {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
-method getAncestorHeader*(db: AbstractChainDB, h: BlockHeader, output: var BlockHeader, skip = 0'u): bool {.base, gcsafe.} =
+method getAncestorHeader*(db: AbstractChainDB, h: BlockHeader,
+    output: var BlockHeader, skip = 0'u): bool
+    {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
-method getBlockBody*(db: AbstractChainDB, blockHash: KeccakHash): BlockBodyRef {.base, gcsafe.} =
+method getBlockBody*(db: AbstractChainDB, blockHash: KeccakHash): BlockBodyRef
+    {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
 method getReceipt*(db: AbstractChainDB, hash: KeccakHash): ReceiptRef {.base, gcsafe.} =
   notImplemented()
 
-method getTrieDB*(db: AbstractChainDB): TrieDatabaseRef {.base, gcsafe.} =
+method getTrieDB*(db: AbstractChainDB): TrieDatabaseRef
+    {.base, gcsafe, raises: [Defect].} =
   notImplemented()
 
 method getCodeByHash*(db: AbstractChainDB, hash: KeccakHash): Blob {.base, gcsafe.} =

eth/common/state_accessors.nim

@@ -18,6 +18,7 @@ proc getContractCode*(chain: AbstractChainDB, req: ContractCodeRequest): Blob {.
     let acc = getAccount(chain.getTrieDB, b.stateRoot, req.key)
     result = chain.getCodeByHash(acc.codeHash)
 
-proc getStorageNode*(chain: AbstractChainDB, hash: KeccakHash): Blob =
+proc getStorageNode*(chain: AbstractChainDB, hash: KeccakHash): Blob
+    {.raises: [CatchableError, Defect].} =
   let db = chain.getTrieDB
   return db.get(hash.data)

eth/p2p.nim

@@ -17,7 +17,8 @@ import
 export
   p2p_types, rlpx, enode, kademlia
 
-proc addCapability*(node: var EthereumNode, p: ProtocolInfo) =
+proc addCapability*(node: var EthereumNode, p: ProtocolInfo)
+    {.raises: [Defect].} =
   doAssert node.connectionState == ConnectionState.None
 
   let pos = lowerBound(node.protocols, p, rlpx.cmp)
@@ -38,10 +39,10 @@ proc newEthereumNode*(keys: KeyPair,
                       addAllCapabilities = true,
                       useCompression: bool = false,
                       minPeers = 10,
-                      rng = newRng()): EthereumNode =
+                      rng = newRng()): EthereumNode {.raises: [Defect].} =
 
   if rng == nil: # newRng could fail
-    raise (ref CatchableError)(msg: "Cannot initialize RNG")
+    raise (ref Defect)(msg: "Cannot initialize RNG")
 
   new result
   result.keys = keys

eth/p2p/blockchain_utils.nim

@@ -3,8 +3,8 @@ import
 
 # TODO: Perhaps we can move this to eth-common
 
-proc getBlockHeaders*(db: AbstractChainDB,
-                      req: BlocksRequest): seq[BlockHeader] {.gcsafe.} =
+proc getBlockHeaders*(db: AbstractChainDB, req: BlocksRequest): seq[BlockHeader]
+    {.gcsafe, raises: [Defect].} =
   result = newSeqOfCap[BlockHeader](req.maxResults)
 
   var foundBlock: BlockHeader

eth/p2p/p2p_backends_helpers.nim

@@ -28,9 +28,12 @@ template networkState*(connection: Peer, Protocol: type): untyped =
   ## particular connection.
   protocolState(connection.network, Protocol)
 
-proc initProtocolState*[T](state: T, x: Peer|EthereumNode) {.gcsafe.} = discard
+proc initProtocolState*[T](state: T, x: Peer|EthereumNode)
+    {.gcsafe, raises: [Defect].} =
+  discard
 
-proc initProtocolStates(peer: Peer, protocols: openarray[ProtocolInfo]) =
+proc initProtocolStates(peer: Peer, protocols: openarray[ProtocolInfo])
+    {.raises: [Defect].} =
   # Initialize all the active protocol states
   newSeq(peer.protocolStates, allProtocols.len)
   for protocol in protocols:

eth/p2p/peer_pool.nim

@@ -108,7 +108,7 @@ proc getRandomBootnode(p: PeerPool): Option[Node] =
   if p.discovery.bootstrapNodes.len != 0:
     result = option(p.discovery.bootstrapNodes.sample())
 
-proc addPeer*(pool: PeerPool, peer: Peer) {.gcsafe.} =
+proc addPeer*(pool: PeerPool, peer: Peer) {.gcsafe, raises: [Defect].} =
   doAssert(peer.remote notin pool.connectedNodes)
   pool.connectedNodes[peer.remote] = peer
   connected_peers.inc()

eth/p2p/private/p2p_types.nim

@@ -61,8 +61,8 @@ type
     observers*: Table[int, PeerObserver]
 
   PeerObserver* = object
-    onPeerConnected*: proc(p: Peer) {.gcsafe.}
-    onPeerDisconnected*: proc(p: Peer) {.gcsafe.}
+    onPeerConnected*: proc(p: Peer) {.gcsafe, raises: [Defect].}
+    onPeerDisconnected*: proc(p: Peer) {.gcsafe, raises: [Defect].}
     protocol*: ProtocolInfo
 
   Capability* = object
@@ -138,16 +138,19 @@ type
 
   # Private types:
   MessageHandlerDecorator* = proc(msgId: int, n: NimNode): NimNode
-  ThunkProc* = proc(x: Peer, msgId: int, data: Rlp): Future[void] {.gcsafe.}
+  ThunkProc* = proc(x: Peer, msgId: int, data: Rlp): Future[void]
+    {.gcsafe, raises: [RlpError, Defect].}
   MessageContentPrinter* = proc(msg: pointer): string {.gcsafe.}
   RequestResolver* = proc(msg: pointer, future: FutureBase)
     {.gcsafe, raises: [Defect].}
-  NextMsgResolver* = proc(msgData: Rlp, future: FutureBase) {.gcsafe.}
-  PeerStateInitializer* = proc(peer: Peer): RootRef {.gcsafe.}
-  NetworkStateInitializer* = proc(network: EthereumNode): RootRef {.gcsafe.}
-  HandshakeStep* = proc(peer: Peer): Future[void] {.gcsafe.}
+  NextMsgResolver* = proc(msgData: Rlp, future: FutureBase)
+    {.gcsafe, raises: [RlpError, Defect].}
+  PeerStateInitializer* = proc(peer: Peer): RootRef {.gcsafe, raises: [Defect].}
+  NetworkStateInitializer* = proc(network: EthereumNode): RootRef
+    {.gcsafe, raises: [Defect].}
+  HandshakeStep* = proc(peer: Peer): Future[void] {.gcsafe, raises: [Defect].}
   DisconnectionHandler* = proc(peer: Peer, reason: DisconnectionReason):
-    Future[void] {.gcsafe.}
+    Future[void] {.gcsafe, raises: [Defect].}
 
   ConnectionState* = enum
     None,

eth/p2p/rlpx.nim

@@ -108,7 +108,7 @@ proc messagePrinter[MsgType](msg: pointer): string {.gcsafe.} =
   # result = $(cast[ptr MsgType](msg)[])
 
 proc disconnect*(peer: Peer, reason: DisconnectionReason,
-                 notifyOtherPeer = false) {.gcsafe, async.}
+                 notifyOtherPeer = false) {.gcsafe, raises: [Defect], async.}
 
 template raisePeerDisconnected(msg: string, r: DisconnectionReason) =
   var e = newException(PeerDisconnected, msg)
@@ -256,9 +256,11 @@ proc cmp*(lhs, rhs: ProtocolInfo): int =
       return int16(lhs.name[i]) - int16(rhs.name[i])
   return 0
 
-proc nextMsgResolver[MsgType](msgData: Rlp, future: FutureBase) {.gcsafe.} =
+proc nextMsgResolver[MsgType](msgData: Rlp, future: FutureBase)
+    {.gcsafe, raises: [RlpError, Defect].} =
   var reader = msgData
-  Future[MsgType](future).complete reader.readRecordType(MsgType, MsgType.rlpFieldsCount > 1)
+  Future[MsgType](future).complete reader.readRecordType(MsgType,
+    MsgType.rlpFieldsCount > 1)
 
 proc registerMsg(protocol: ProtocolInfo,
                  id: int, name: string,
@@ -307,7 +309,8 @@ proc supports*(peer: Peer, Protocol: type): bool {.inline.} =
 template perPeerMsgId(peer: Peer, MsgType: type): int =
   perPeerMsgIdImpl(peer, MsgType.msgProtocol.protocolInfo, MsgType.msgId)
 
-proc invokeThunk*(peer: Peer, msgId: int, msgData: var Rlp): Future[void] =
+proc invokeThunk*(peer: Peer, msgId: int, msgData: var Rlp): Future[void]
+    {.raises: [UnsupportedMessageError, RlpError, Defect].} =
   template invalidIdError: untyped =
     raise newException(UnsupportedMessageError,
       "RLPx message with an invalid id " & $msgId &
@@ -766,7 +769,10 @@ proc p2pProtocolBackendImpl*(protocol: P2PProtocol): Backend =
       thunkName = ident(msgName & "Thunk")
 
     msg.defineThunk quote do:
-      proc `thunkName`(`peerVar`: `Peer`, _: int, data: Rlp) {.async, gcsafe.} =
+      proc `thunkName`(`peerVar`: `Peer`, _: int, data: Rlp)
+          # Fun error if you just use `RlpError` instead of `rlp.RlpError`:
+          # "Error: type expected, but got symbol 'RlpError' of kind 'EnumField'"
+          {.async, gcsafe, raises: [rlp.RlpError, Defect].} =
         var `receivedRlp` = data
         var `receivedMsg` {.noinit.}: `msgRecName`
         `readParamsPrelude`
@@ -866,12 +872,13 @@ p2pProtocol DevP2P(version = 5, rlpxName = "p2p"):
   proc pong(peer: Peer, emptyList: EmptyList) =
     discard
 
-proc removePeer(network: EthereumNode, peer: Peer) =
+proc removePeer(network: EthereumNode, peer: Peer) {.raises: [Defect].} =
   # It is necessary to check if peer.remote still exists. The connection might
   # have been dropped already from the peers side.
   # E.g. when receiving a p2p.disconnect message from a peer, a race will happen
   # between which side disconnects first.
-  if network.peerPool != nil and not peer.remote.isNil and peer.remote in network.peerPool.connectedNodes:
+  if network.peerPool != nil and not peer.remote.isNil and
+      peer.remote in network.peerPool.connectedNodes:
     network.peerPool.connectedNodes.del(peer.remote)
     connected_peers.dec()
 
@@ -883,16 +890,23 @@ proc removePeer(network: EthereumNode, peer: Peer) =
           if observer.protocol.isNil or peer.supports(observer.protocol):
             observer.onPeerDisconnected(peer)
 
-proc callDisconnectHandlers(peer: Peer, reason: DisconnectionReason): Future[void] =
+proc callDisconnectHandlers(peer: Peer, reason: DisconnectionReason):
+    Future[void] {.async.} =
   var futures = newSeqOfCap[Future[void]](allProtocols.len)
 
   for protocol in peer.dispatcher.activeProtocols:
     if protocol.disconnectHandler != nil:
       futures.add((protocol.disconnectHandler)(peer, reason))
 
-  return all(futures)
+  await allFutures(futures)
 
-proc disconnect*(peer: Peer, reason: DisconnectionReason, notifyOtherPeer = false) {.async.} =
+  for f in futures:
+    doAssert(f.finished())
+    if f.failed():
+      trace "Disconnection handler ended with an error", err = f.error.msg
+
+proc disconnect*(peer: Peer, reason: DisconnectionReason,
+    notifyOtherPeer = false) {.async, raises: [Defect].} =
   if peer.connectionState notin {Disconnecting, Disconnected}:
     peer.connectionState = Disconnecting
     # Do this first so sub-protocols have time to clean up and stop sending
@@ -901,7 +915,7 @@ proc disconnect*(peer: Peer, reason: DisconnectionReason, notifyOtherPeer = fals
       # In case of `CatchableError` in any of the handlers, this will be logged.
       # Other handlers will still execute.
       # In case of `Defect` in any of the handlers, program will quit.
-      traceAwaitErrors callDisconnectHandlers(peer, reason)
+      await callDisconnectHandlers(peer, reason)
 
     if notifyOtherPeer and not peer.transport.closed:
       var fut = peer.sendDisconnectMsg(DisconnectionReasonList(value: reason))
@@ -931,7 +945,8 @@ proc checkUselessPeer(peer: Peer) {.inline.} =
     # XXX: Send disconnect + UselessPeer
     raise newException(UselessPeerError, "Useless peer")
 
-proc initPeerState*(peer: Peer, capabilities: openarray[Capability]) =
+proc initPeerState*(peer: Peer, capabilities: openarray[Capability])
+    {.raises: [UselessPeerError, Defect].} =
   peer.dispatcher = getDispatcher(peer.network, capabilities)
   checkUselessPeer(peer)
 

eth/p2p/rlpx_protocols/whisper_protocol.nim

@@ -96,8 +96,9 @@ proc allowed*(msg: Message, config: WhisperConfig): bool =
 
   return true
 
-proc run(peer: Peer) {.gcsafe, async.}
-proc run(node: EthereumNode, network: WhisperNetwork) {.gcsafe, async.}
+proc run(peer: Peer) {.gcsafe, async, raises: [Defect].}
+proc run(node: EthereumNode, network: WhisperNetwork)
+  {.gcsafe, async, raises: [Defect].}
 
 proc initProtocolState*(network: WhisperNetwork, node: EthereumNode) {.gcsafe.} =
   new(network.queue)
@@ -107,7 +108,7 @@ proc initProtocolState*(network: WhisperNetwork, node: EthereumNode) {.gcsafe.}
   network.config.powRequirement = defaultMinPow
   network.config.isLightNode = false
   network.config.maxMsgSize = defaultMaxMsgSize
-  asyncCheck node.run(network)
+  asyncSpawn node.run(network)
 
 p2pProtocol Whisper(version = whisperVersion,
                     rlpxName = "shh",
@@ -152,7 +153,7 @@ p2pProtocol Whisper(version = whisperVersion,
     whisperPeer.initialized = true
 
     if not whisperNet.config.isLightNode:
-      traceAsyncErrors peer.run()
+      asyncSpawn peer.run()
 
     debug "Whisper peer initialized", peer
 
@@ -249,7 +250,7 @@ p2pProtocol Whisper(version = whisperVersion,
 
 # 'Runner' calls ---------------------------------------------------------------
 
-proc processQueue(peer: Peer) =
+proc processQueue(peer: Peer) {.raises: [Defect].} =
   # Send to peer all valid and previously not send envelopes in the queue.
   var
     envelopes: seq[Envelope] = @[]
@@ -280,7 +281,7 @@ proc processQueue(peer: Peer) =
     # gets dropped
     traceAsyncErrors peer.messages(envelopes)
 
-proc run(peer: Peer) {.async.} =
+proc run(peer: Peer) {.async, raises: [Defect].} =
   while peer.connectionState notin {Disconnecting, Disconnected}:
     peer.processQueue()
     await sleepAsync(messageInterval)
@@ -298,7 +299,7 @@ proc pruneReceived(node: EthereumNode) {.raises: [].} =
       # the received sets.
       peer.received = intersection(peer.received, whisperNet.queue.itemHashes)
 
-proc run(node: EthereumNode, network: WhisperNetwork) {.async.} =
+proc run(node: EthereumNode, network: WhisperNetwork) {.async, raises: [Defect].} =
   while true:
     # prune message queue every second
     # TTL unit is in seconds, so this should be sufficient?

tests/p2p/all_tests.nim

@@ -4,4 +4,10 @@ import
   ./test_crypt,
   ./test_discovery,
   ./test_ecies,
-  ./test_enode
+  ./test_enode,
+  ./test_rlpx_thunk,
+  ./test_shh,
+  ./test_shh_config,
+  ./test_shh_connect,
+  ./test_protocol_handlers,
+  ./les/test_flow_control

tests/p2p/les/test_flow_control.nim

@@ -1,3 +1,5 @@
+{.used.}
+
 import
   ../../../eth/p2p/rlpx_protocols/les/flow_control
 

tests/p2p/test_protocol_handlers.nim

@@ -7,6 +7,8 @@
 #  Apache License, version 2.0, (LICENSE-APACHEv2)
 #            MIT license (LICENSE-MIT)
 
+{.used.}
+
 import
   std/tables,
   chronos, testutils/unittests,

tests/p2p/test_rlpx_thunk.nim

@@ -1,3 +1,5 @@
+{.used.}
+
 import
   std/[json, os, unittest],
   chronos, stew/byteutils,

tests/p2p/test_shh.nim

@@ -7,6 +7,8 @@
 #  Apache License, version 2.0, (LICENSE-APACHEv2)
 #            MIT license (LICENSE-MIT)
 
+{.used.}
+
 import
   std/[sequtils, options, unittest, tables],
   nimcrypto/hash,

tests/p2p/test_shh_config.nim

@@ -7,6 +7,8 @@
 #  Apache License, version 2.0, (LICENSE-APACHEv2)
 #            MIT license (LICENSE-MIT)
 
+{.used.}
+
 import
   std/[sequtils, options, unittest, times],
   ../../eth/p2p/rlpx_protocols/whisper_protocol as whisper

tests/p2p/test_shh_connect.nim

@@ -7,6 +7,8 @@
 #  Apache License, version 2.0, (LICENSE-APACHEv2)
 #            MIT license (LICENSE-MIT)
 
+{.used.}
+
 import
   std/[sequtils, options, tables],
   chronos, testutils/unittests, bearssl,