Merge pull request #172 from status-im/improved-exception-tracking

Reduce the use of the general Exception type

eth/p2p/discoveryv5/discovery_db.nim

@@ -27,15 +27,24 @@ proc makeKey(id: NodeId, address: Address): array[keySize, byte] =
     copyMem(addr result[sizeof(id) + 1], unsafeAddr address.ip.address_v6, sizeof(address.ip.address_v6))
   copyMem(addr result[sizeof(id) + 1 + sizeof(address.ip.address_v6)], unsafeAddr address.udpPort, sizeof(address.udpPort))
 
-method storeKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: array[16, byte]) =
-  var value: array[sizeof(r) + sizeof(w), byte]
-  value[0 .. 15] = r
-  value[16 .. ^1] = w
-  db.backend.put(makeKey(id, address), value)
+method storeKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: array[16, byte]): bool {.raises: [Defect].} =
+  try:
+    var value: array[sizeof(r) + sizeof(w), byte]
+    value[0 .. 15] = r
+    value[16 .. ^1] = w
+    db.backend.put(makeKey(id, address), value)
+    return true
+  except CatchableError:
+    return false
 
-method loadKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: var array[16, byte]): bool =
-  let res = db.backend.get(makeKey(id, address))
-  if res.len == sizeof(r) + sizeof(w):
+method loadKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: var array[16, byte]): bool {.raises: [Defect].} =
+  try:
+    let res = db.backend.get(makeKey(id, address))
+    if res.len != sizeof(r) + sizeof(w):
+      return false
     copyMem(addr r[0], unsafeAddr res[0], sizeof(r))
     copyMem(addr w[0], unsafeAddr res[sizeof(r)], sizeof(w))
-    result = true
+    return true
+  except CatchableError:
+    return false
+

eth/p2p/discoveryv5/encoding.nim

@@ -32,13 +32,14 @@ type
     ephemeralKey*: array[64, byte]
     response*: seq[byte]
 
+  RandomSourceDepleted* = object of CatchableError
 
 const
   gcmTagSize = 16
 
 proc randomBytes(v: var openarray[byte]) =
   if nimcrypto.randomBytes(v) != v.len:
-    raise newException(Exception, "Could not randomize bytes") # TODO:
+    raise newException(RandomSourceDepleted, "Could not randomize bytes")
 
 proc idNonceHash(nonce, ephkey: openarray[byte]): array[32, byte] =
   var ctx: sha256
@@ -50,12 +51,12 @@ proc idNonceHash(nonce, ephkey: openarray[byte]): array[32, byte] =
 
 proc signIDNonce(c: Codec, idNonce, ephKey: openarray[byte]): SignatureNR =
   if signRawMessage(idNonceHash(idNonce, ephKey), c.privKey, result) != EthKeysStatus.Success:
-    raise newException(Exception, "Could not sign idNonce")
+    raise newException(EthKeysException, "Could not sign idNonce")
 
 proc deriveKeys(n1, n2: NodeID, priv: PrivateKey, pub: PublicKey, challenge: Whoareyou, result: var HandshakeSecrets) =
   var eph: SharedSecretFull
   if ecdhAgree(priv, pub, eph) != EthKeysStatus.Success:
-    raise newException(Exception, "ecdhAgree failed")
+    raise newException(EthKeysException, "ecdhAgree failed")
 
   # TODO: Unneeded allocation here
   var info = newSeqOfCap[byte](idNoncePrefix.len + 32 * 2)
@@ -85,16 +86,12 @@ proc makeAuthHeader(c: Codec, toNode: Node, nonce: array[gcmNonceSize, byte],
   if challenge.recordSeq < ln.record.seqNum:
     resp.record = ln.record
 
-  var remotePubkey: PublicKey
-  if not toNode.record.get(remotePubkey):
-    raise newException(Exception, "Could not get public key from remote ENR") # Should not happen!
-
   let ephKey = newPrivateKey()
   let ephPubkey = ephKey.getPublicKey().getRaw
 
   resp.signature = c.signIDNonce(challenge.idNonce, ephPubkey).getRaw
 
-  deriveKeys(ln.id, toNode.id, ephKey, remotePubkey, challenge, handhsakeSecrets)
+  deriveKeys(ln.id, toNode.id, ephKey, toNode.node.pubKey, challenge, handhsakeSecrets)
 
   let respRlp = rlp.encode(resp)
 
@@ -134,8 +131,8 @@ proc encodeEncrypted*(c: Codec, toNode: Node, packetData: seq[byte], challenge:
     headEnc = c.makeAuthHeader(toNode, nonce, sec, challenge)
 
     writeKey = sec.writeKey
-
-    c.db.storeKeys(toNode.id, toNode.address, sec.readKey, sec.writeKey)
+    # TODO: is it safe to ignore the error here?
+    discard c.db.storeKeys(toNode.id, toNode.address, sec.readKey, sec.writeKey)
 
   var body = packetData
   let tag = packetTag(toNode.id, c.localNode.id)
@@ -228,7 +225,8 @@ proc decodeEncrypted*(c: var Codec, fromId: NodeID, fromAddr: Address, input: se
 
     # Swap keys to match remote
     swap(sec.readKey, sec.writeKey)
-    c.db.storeKeys(fromId, fromAddr, sec.readKey, sec.writeKey)
+    # TODO: is it safe to ignore the error here?
+    discard c.db.storeKeys(fromId, fromAddr, sec.readKey, sec.writeKey)
     readKey = sec.readKey
 
   else:
@@ -249,7 +247,7 @@ proc decodeEncrypted*(c: var Codec, fromId: NodeID, fromAddr: Address, input: se
 
 proc newRequestId*(): RequestId =
   if randomBytes(addr result, sizeof(result)) != sizeof(result):
-    raise newException(Exception, "Could not randomize bytes") # TODO:
+    raise newException(RandomSourceDepleted, "Could not randomize bytes") # TODO:
 
 proc numFields(T: typedesc): int =
   for k, v in fieldPairs(default(T)): inc result

eth/p2p/discoveryv5/enr.nim

@@ -84,7 +84,7 @@ proc makeEnrAux(seqNum: uint64, pk: PrivateKey, pairs: openarray[(string, Field)
 
   var sig: SignatureNR
   if signRawMessage(keccak256.digest(toSign).data, pk, sig) != EthKeysStatus.Success:
-    raise newException(Exception, "Could not sign ENR (internal error)")
+    raise newException(EthKeysException, "Could not sign ENR (internal error)")
 
   result.raw = block:
     var w = initRlpList(result.pairs.len * 2 + 2)
@@ -136,6 +136,10 @@ proc get*(r: Record, key: string, T: type): T =
     elif T is string:
       requireKind(f, kString)
       return f.str
+    elif T is PublicKey:
+      requireKind(f, kBytes)
+      if recoverPublicKey(f.bytes, result) != EthKeysStatus.Success:
+        raise newException(ValueError, "Invalid public key")
     elif T is array:
       when type(result[0]) is byte:
         requireKind(f, kBytes)
@@ -156,11 +160,11 @@ proc get*(r: Record, pubKey: var PublicKey): bool =
 
 proc tryGet*(r: Record, key: string, T: type): Option[T] =
   try:
-    return some r.get(key, T)
+    return some get(r, key, T)
   except CatchableError:
     discard
 
-func toTypedRecord*(r: Record): Option[TypedRecord] =
+proc toTypedRecord*(r: Record): Option[TypedRecord] =
   let id = r.tryGet("id", string)
   if id.isSome:
     var tr: TypedRecord

eth/p2p/discoveryv5/protocol.nim

@@ -33,9 +33,12 @@ proc whoareyouMagic(toNode: NodeId): array[32, byte] =
   for i, c in prefix: data[sizeof(toNode) + i] = byte(c)
   sha256.digest(data).data
 
-proc newProtocol*(privKey: PrivateKey, db: Database, port: Port): Protocol =
+proc newProtocol*(privKey: PrivateKey,
+                  db: Database,
+                  tcpPort, udpPort: Port): Protocol =
   result = Protocol(privateKey: privKey, db: db)
-  let a = Address(ip: parseIpAddress("127.0.0.1"), udpPort: port)
+  let a = Address(ip: parseIpAddress("127.0.0.1"),
+                  tcpPort: tcpPort, udpPort: udpPort)
 
   result.localNode = newNode(initENode(result.privateKey.getPublicKey(), a))
   result.localNode.record = enr.Record.init(12, result.privateKey, a)
@@ -63,7 +66,7 @@ proc send(d: Protocol, n: Node, data: seq[byte]) =
 
 proc randomBytes(v: var openarray[byte]) =
   if nimcrypto.randomBytes(v) != v.len:
-    raise newException(Exception, "Could not randomize bytes") # TODO:
+    raise newException(RandomSourceDepleted, "Could not randomize bytes") # TODO:
 
 proc `xor`[N: static[int], T](a, b: array[N, T]): array[N, T] =
   for i in 0 .. a.high:
@@ -123,9 +126,18 @@ proc handleFindNode(d: Protocol, fromNode: Node, fn: FindNodePacket, reqId: Requ
     let distance = min(fn.distance, 256)
     d.sendNodes(fromNode, reqId, d.routingTable.neighboursAtDistance(distance))
 
-proc receive(d: Protocol, a: Address, msg: Bytes)
-    {.gcsafe, raises:[RlpError, Exception].} =
-  # TODO: figure out where the general exception comes from and clean it up
+proc receive(d: Protocol, a: Address, msg: Bytes) {.gcsafe,
+  raises: [
+    Defect,
+    # TODO This is now coming from Chronos's callSoon
+    Exception,
+    # TODO All of these should probably be handled here
+    RlpError,
+    IOError,
+    TransportAddressError,
+    EthKeysException,
+    Secp256k1Exception,
+  ].} =
   if msg.len < 32:
     return # Invalid msg
 
@@ -136,9 +148,12 @@ proc receive(d: Protocol, a: Address, msg: Bytes)
     var pr: PendingRequest
     if d.pendingRequests.take(whoareyou.authTag, pr):
       let toNode = pr.node
-
-      let (data, _) = d.codec.encodeEncrypted(toNode, pr.packet, challenge = whoareyou)
-      d.send(toNode, data)
+      try:
+        let (data, _) = d.codec.encodeEncrypted(toNode, pr.packet, challenge = whoareyou)
+        d.send(toNode, data)
+      except RandomSourceDepleted as err:
+        debug "Failed to respond to a who-you-are msg " &
+              "due to randomness source depletion."
 
   else:
     var tag: array[32, byte]
@@ -169,7 +184,7 @@ proc receive(d: Protocol, a: Address, msg: Bytes)
         if d.awaitedPackets.take((node, packet.reqId), waiter):
           waiter.complete(packet.some)
         else:
-          debug "TODO: handle packet: ", packet = packet.kind, origin = node
+          debug "TODO: handle packet: ", packet = packet.kind, origin = $node
 
     else:
       debug "Could not decode, respond with whoareyou"
@@ -352,7 +367,8 @@ when isMainModule:
       else:
         pk = newPrivateKey()
 
-      let d = newProtocol(pk, DiscoveryDB.init(newMemoryDB()), Port(12001 + i))
+      let d = newProtocol(pk, DiscoveryDB.init(newMemoryDB()),
+                          Port(12001 + i), Port(12001 + i))
       d.open()
       result.add(d)
 

eth/p2p/discoveryv5/types.nim

@@ -69,8 +69,9 @@ template packetKind*(T: typedesc[SomePacket]): PacketKind =
   elif T is FindNodePacket: findNode
   elif T is NodesPacket: nodes
 
-method storeKeys*(db: Database, id: NodeId, address: Address, r, w: array[16, byte]) {.base.} = discard
-method loadKeys*(db: Database, id: NodeId, address: Address, r, w: var array[16, byte]): bool {.base.} = discard
+method storeKeys*(db: Database, id: NodeId, address: Address, r, w: array[16, byte]): bool {.base, raises: [Defect].} = discard
+
+method loadKeys*(db: Database, id: NodeId, address: Address, r, w: var array[16, byte]): bool {.base, raises: [Defect].} = discard
 
 proc toBytes*(id: NodeId): array[32, byte] {.inline.} =
   id.toByteArrayBE()

eth/trie/db.nim

@@ -21,10 +21,18 @@ type
     contains(DB, KeccakHash) is bool
 
   # XXX: poor's man vtref types
-  PutProc = proc (db: RootRef, key, val: openarray[byte]) {.gcsafe.}
-  GetProc = proc (db: RootRef, key: openarray[byte]): Bytes {.gcsafe.} # Must return empty seq if not found
-  DelProc = proc (db: RootRef, key: openarray[byte]) {.gcsafe.}
-  ContainsProc = proc (db: RootRef, key: openarray[byte]): bool {.gcsafe.}
+  PutProc = proc (db: RootRef, key, val: openarray[byte]) {.
+    gcsafe, raises: [Defect, CatchableError] .}
+
+  GetProc = proc (db: RootRef, key: openarray[byte]): Bytes {.
+    gcsafe, raises: [Defect, CatchableError] .}
+    ## The result will be empty seq if not found
+
+  DelProc = proc (db: RootRef, key: openarray[byte]) {.
+    gcsafe, raises: [Defect, CatchableError] .}
+
+  ContainsProc = proc (db: RootRef, key: openarray[byte]): bool {.
+    gcsafe, raises: [Defect, CatchableError] .}
 
   TrieDatabaseRef* = ref object
     obj: RootRef

tests/p2p/p2p_test_helper.nim

@@ -6,7 +6,8 @@ var nextPort = 30303
 
 proc localAddress*(port: int): Address =
   let port = Port(port)
-  result = Address(udpPort: port, tcpPort: port, ip: parseIpAddress("127.0.0.1"))
+  result = Address(udpPort: port, tcpPort: port,
+                   ip: parseIpAddress("127.0.0.1"))
 
 proc startDiscoveryNode*(privKey: PrivateKey, address: Address,
                         bootnodes: seq[ENode]): Future[DiscoveryProtocol] {.async.} =

tests/p2p/test_discoveryv5.nim

@@ -6,9 +6,9 @@ import
   ./p2p_test_helper
 
 proc startDiscoveryv5Node*(privKey: PrivateKey, address: Address,
-                        bootnodes: seq[Record]): discv5_protocol.Protocol =
+                           bootnodes: seq[Record]): discv5_protocol.Protocol =
   var db = DiscoveryDB.init(newMemoryDB())
-  result = newProtocol(privKey, db, address.udpPort)
+  result = newProtocol(privKey, db, address.tcpPort, address.udpPort)
 
   for node in bootnodes:
     result.addNode(node)