Bump nim-eth and accompanying discv5 cleanup (#1081)

beacon_chain/eth2_discovery.nim

@@ -1,13 +1,9 @@
-# TODO Cannot use push here becaise it gets applied to PeerID.init (!)
+{.push raises: [Defect].}
-#      probably because it's a generic proc...
-# {.push raises: [Defect].}
 
 import
-  os, net, sequtils, strutils, strformat, parseutils,
+  os, net, sequtils, strutils,
-  chronicles, stew/[results, objects], eth/keys, eth/trie/db, eth/p2p/enode,
+  chronicles, stew/results, eth/keys, eth/trie/db,
-  eth/p2p/discoveryv5/[enr, protocol, discovery_db, types],
+  eth/p2p/discoveryv5/[enr, protocol, discovery_db, node],
-  libp2p/[multiaddress, peer],
-  libp2p/crypto/crypto as libp2pCrypto, libp2p/crypto/secp,
   conf
 
 type
@@ -16,86 +12,10 @@ type
   PublicKey = keys.PublicKey
 
 export
-  Eth2DiscoveryProtocol, open, start, close, results
+  Eth2DiscoveryProtocol, open, start, close, closeWait, randomNodes, results
 
-proc toENode*(a: MultiAddress): Result[ENode, cstring] {.raises: [Defect].} =
+proc parseBootstrapAddress*(address: TaintedString):
-  try:
+    Result[enr.Record, cstring] =
-    if not IPFS.match(a):
-      return err "Unsupported MultiAddress"
-
-    # TODO. This code is quite messy with so much string handling.
-    # MultiAddress can offer a more type-safe API?
-    var
-      peerId = PeerID.init(a[2].protoAddress())
-      addressFragments = split($a[0], "/")
-      portFragments = split($a[1], "/")
-      tcpPort: int
-
-    if addressFragments.len != 3 or
-       addressFragments[1] != "ip4" or
-       portFragments.len != 3 or
-       portFragments[1] notin ["tcp", "udp"] or
-       parseInt(portFragments[2], tcpPort) == 0:
-      return err "Only IPv4 MultiAddresses are supported"
-
-    let
-      ipAddress = parseIpAddress(addressFragments[2])
-
-      # TODO. The multiaddress will have either a TCP or a UDP value, but
-      # is it reasonable to assume that a client will use the same ports?
-      # Probably not, but how can we bootstrap then?
-      udpPort = tcpPort
-
-    var pubkey: libp2pCrypto.PublicKey
-    if peerId.extractPublicKey(pubkey):
-      if pubkey.scheme == Secp256k1:
-        return ok ENode(pubkey: PublicKey(pubkey.skkey),
-                        address: Address(ip: ipAddress,
-                                         tcpPort: Port tcpPort,
-                                         udpPort: Port udpPort))
-
-  except CatchableError:
-    # This will reach the error exit path below
-    discard
-  except Exception as e:
-    # TODO:
-    # nim-libp2p/libp2p/multiaddress.nim(616, 40) Error: can raise an unlisted exception: Exception
-    if e of Defect:
-      raise (ref Defect)(e)
-
-  return err "Invalid MultiAddress"
-
-proc toMultiAddressStr*(enode: ENode): string =
-  var peerId = PeerID.init(libp2pCrypto.PublicKey(
-    scheme: Secp256k1, skkey: secp.SkPublicKey(enode.pubkey)))
-  &"/ip4/{enode.address.ip}/tcp/{enode.address.tcpPort}/p2p/{peerId.pretty}"
-
-proc toENode*(enrRec: enr.Record): Result[ENode, cstring] {.raises: [Defect].} =
-  try:
-    # TODO: handle IPv6
-    let ipBytes = enrRec.get("ip", seq[byte])
-    if ipBytes.len != 4:
-      return err "Malformed ENR IP address"
-    let
-      ip = IpAddress(family: IpAddressFamily.IPv4,
-                     address_v4: toArray(4, ipBytes))
-      tcpPort = Port enrRec.get("tcp", uint16)
-      udpPort = Port enrRec.get("udp", uint16)
-    let pubkey = enrRec.get(PublicKey)
-    if pubkey.isNone:
-      return err "Failed to read public key from ENR record"
-    return ok ENode(pubkey: pubkey.get(),
-                    address: Address(ip: ip,
-                                     tcpPort: tcpPort,
-                                     udpPort: udpPort))
-  except CatchableError:
-    return err "Invalid ENR record"
-
-# TODO
-# This will be resoted to its more generalized form (returning ENode)
-# once we refactor the discv5 code to be more easily bootstrapped with
-# trusted, but non-signed bootstrap addresses.
-proc parseBootstrapAddress*(address: TaintedString): Result[enr.Record, cstring] =
   if address.len == 0:
     return err "an empty string is not a valid bootstrap node"
 
@@ -104,13 +24,6 @@ proc parseBootstrapAddress*(address: TaintedString): Result[enr.Record, cstring]
 
   if address[0] == '/':
     return err "MultiAddress bootstrap addresses are not supported"
-    #[
-    try:
-      let ma = MultiAddress.init(address)
-      return toENode(ma)
-    except CatchableError:
-      return err "Invalid bootstrap multiaddress"
-    ]#
   else:
     let lowerCaseAddress = toLowerAscii(string address)
     if lowerCaseAddress.startsWith("enr:"):
@@ -120,40 +33,41 @@ proc parseBootstrapAddress*(address: TaintedString): Result[enr.Record, cstring]
       return err "Invalid ENR bootstrap record"
     elif lowerCaseAddress.startsWith("enode:"):
       return err "ENode bootstrap addresses are not supported"
-      #[
-      try:
-        return ok initEnode(string address)
-      except CatchableError as err:
-        return err "Ignoring invalid enode bootstrap address"
-      ]#
     else:
       return err "Ignoring unrecognized bootstrap address type"
 
 proc addBootstrapNode*(bootstrapAddr: string,
-                       bootNodes: var seq[ENode],
+                       bootstrapEnrs: var seq[enr.Record],
-                       bootEnrs: var seq[enr.Record],
                        localPubKey: PublicKey) =
   let enrRes = parseBootstrapAddress(bootstrapAddr)
   if enrRes.isOk:
-    bootEnrs.add enrRes.value
+    bootstrapEnrs.add enrRes.value
   else:
     warn "Ignoring invalid bootstrap address",
           bootstrapAddr, reason = enrRes.error
 
 proc loadBootstrapFile*(bootstrapFile: string,
-                        bootNodes: var seq[ENode],
+                        bootstrapEnrs: var seq[enr.Record],
-                        bootEnrs: var seq[enr.Record],
                         localPubKey: PublicKey) =
   if bootstrapFile.len == 0: return
   let ext = splitFile(bootstrapFile).ext
   if cmpIgnoreCase(ext, ".txt") == 0:
-    for ln in lines(bootstrapFile):
+    try:
-      addBootstrapNode(ln, bootNodes, bootEnrs, localPubKey)
+      for ln in lines(bootstrapFile):
+        addBootstrapNode(ln, bootstrapEnrs, localPubKey)
+    except IOError as e:
+      error "Could not read bootstrap file", msg = e.msg
+      quit 1
+
   elif cmpIgnoreCase(ext, ".yaml") == 0:
     # TODO. This is very ugly, but let's try to negotiate the
     # removal of YAML metadata.
-    for ln in lines(bootstrapFile):
+    try:
-      addBootstrapNode(string(ln[3..^2]), bootNodes, bootEnrs, localPubKey)
+      for ln in lines(bootstrapFile):
+        addBootstrapNode(string(ln[3..^2]), bootstrapEnrs, localPubKey)
+    except IOError as e:
+      error "Could not read bootstrap file", msg = e.msg
+      quit 1
   else:
     error "Unknown bootstrap file format", ext
     quit 1
@@ -162,26 +76,26 @@ proc new*(T: type Eth2DiscoveryProtocol,
           conf: BeaconNodeConf,
           ip: Option[IpAddress], tcpPort, udpPort: Port,
           rawPrivKeyBytes: openarray[byte],
-          enrFields: openarray[(string, seq[byte])]): T =
+          enrFields: openarray[(string, seq[byte])]):
+          T {.raises: [Exception, Defect].} =
   # TODO
   # Implement more configuration options:
   # * for setting up a specific key
   # * for using a persistent database
-  var
+  let
-    pk = PrivateKey.fromRaw(rawPrivKeyBytes).tryGet()
+    pk = PrivateKey.fromRaw(rawPrivKeyBytes).expect("Valid private key")
-    ourPubKey = pk.toPublicKey().tryGet()
+    ourPubKey = pk.toPublicKey().expect("Public key from valid private key")
+    # TODO: `newMemoryDB()` causes raises: [Exception]
     db = DiscoveryDB.init(newMemoryDB())
 
-  var bootNodes: seq[ENode]
+  var bootstrapEnrs: seq[enr.Record]
-  var bootEnrs: seq[enr.Record]
   for node in conf.bootstrapNodes:
-    addBootstrapNode(node, bootNodes, bootEnrs, ourPubKey)
+    addBootstrapNode(node, bootstrapEnrs, ourPubKey)
-  loadBootstrapFile(string conf.bootstrapNodesFile, bootNodes, bootEnrs, ourPubKey)
+  loadBootstrapFile(string conf.bootstrapNodesFile, bootstrapEnrs, ourPubKey)
 
   let persistentBootstrapFile = conf.dataDir / "bootstrap_nodes.txt"
   if fileExists(persistentBootstrapFile):
-    loadBootstrapFile(persistentBootstrapFile, bootNodes, bootEnrs, ourPubKey)
+    loadBootstrapFile(persistentBootstrapFile, bootstrapEnrs, ourPubKey)
 
   let enrFieldPairs = mapIt(enrFields, toFieldPair(it[0], it[1]))
-  newProtocol(pk, db, ip, tcpPort, udpPort, enrFieldPairs, bootEnrs)
+  newProtocol(pk, db, ip, tcpPort, udpPort, enrFieldPairs, bootstrapEnrs)
-

beacon_chain/eth2_network.nim

@@ -17,15 +17,12 @@ import
   libp2p/protocols/pubsub/[pubsub, floodsub, rpc/messages],
   libp2p/transports/tcptransport,
   libp2p/stream/lpstream,
-  eth/[keys, async_utils], eth/p2p/[enode, p2p_protocol_dsl],
+  eth/[keys, async_utils], eth/p2p/p2p_protocol_dsl,
   eth/net/nat, eth/p2p/discoveryv5/[enr, node],
   # Beacon node modules
   version, conf, eth2_discovery, libp2p_json_serialization, conf, ssz,
   peer_pool, spec/[datatypes, network]
 
-import
-  eth/p2p/discoveryv5/protocol as discv5_protocol
-
 export
   version, multiaddress, peer_pool, peerinfo, p2pProtocol,
   libp2p_json_serialization, ssz, peer, results
@@ -681,14 +678,16 @@ proc runDiscoveryLoop*(node: Eth2Node) {.async.} =
           node.discovery.randomNodes(node.wantedPeers - currentPeerCount)
         for peer in discoveredPeers:
           try:
-            let peerInfo = peer.record.toTypedRecord.toPeerInfo
+            let peerRecord = peer.record.toTypedRecord
-            if peerInfo != nil:
+            if peerRecord.isOk:
-              if peerInfo.id notin node.switch.connections:
+              let peerInfo = peerRecord.value.toPeerInfo
-                debug "Discovered new peer", peer = $peer
+              if peerInfo != nil:
-                # TODO do this in parallel
+                if peerInfo.id notin node.switch.connections:
-                await node.dialPeer(peerInfo)
+                  debug "Discovered new peer", peer = $peer
-              else:
+                  # TODO do this in parallel
-                peerInfo.close()
+                  await node.dialPeer(peerInfo)
+                else:
+                  peerInfo.close()
           except CatchableError as err:
             debug "Failed to connect to peer", peer = $peer, err = err.msg
       except CatchableError as err:
@@ -731,7 +730,7 @@ proc init*(T: type Eth2Node, conf: BeaconNodeConf, enrForkId: ENRForkID,
 template publicKey*(node: Eth2Node): keys.PublicKey =
   node.discovery.privKey.toPublicKey.tryGet()
 
-template addKnownPeer*(node: Eth2Node, peer: ENode|enr.Record) =
+template addKnownPeer*(node: Eth2Node, peer: enr.Record) =
   node.discovery.addNode peer
 
 proc start*(node: Eth2Node) {.async.} =
@@ -1009,12 +1008,6 @@ proc announcedENR*(node: Eth2Node): enr.Record =
 proc shortForm*(id: KeyPair): string =
   $PeerID.init(id.pubkey)
 
-proc toPeerInfo(enode: ENode): PeerInfo =
-  let
-    peerId = PeerID.init enode.pubkey.asLibp2pKey
-    addresses = @[MultiAddress.init enode.toMultiAddressStr]
-  return PeerInfo.init(peerId, addresses)
-
 proc connectToNetwork*(node: Eth2Node) {.async.} =
   await node.start()
 

beacon_chain/inspector.nim

@@ -11,7 +11,7 @@ import libp2p/[switch, standard_setup, connection, multiaddress, multicodec,
 import libp2p/crypto/crypto as lcrypto
 import libp2p/crypto/secp as lsecp
 import eth/p2p/discoveryv5/enr as enr
-import eth/p2p/discoveryv5/[protocol, discovery_db, types]
+import eth/p2p/discoveryv5/[protocol, discovery_db, node]
 import eth/keys as ethkeys, eth/trie/db
 import stew/[results, objects]
 import stew/byteutils as bu
@@ -314,7 +314,7 @@ proc init*(p: typedesc[PeerInfo],
   var trec: enr.TypedRecord
   try:
     let trecOpt = enraddr.toTypedRecord()
-    if trecOpt.isSome():
+    if trecOpt.isOk():
       trec = trecOpt.get()
       if trec.secp256k1.isSome():
         let skpubkey = ethkeys.PublicKey.fromRaw(trec.secp256k1.get())
@@ -441,7 +441,7 @@ proc logEnrAddress(address: string) =
     var attnData = rec.tryGet("attnets", seq[byte])
     var optrec = rec.toTypedRecord()
 
-    if optrec.isSome():
+    if optrec.isOk():
       trec = optrec.get()
 
       if eth2Data.isSome():

tests/all_tests.nim

@@ -16,7 +16,6 @@ import # Unit test
   ./test_beacon_node,
   ./test_beaconstate,
   ./test_block_pool,
-  ./test_discovery_helpers,
   ./test_helpers,
   ./test_mocking,
   ./test_mainchain_monitor,

tests/test_discovery_helpers.nim

@@ -1,32 +0,0 @@
-{.used.}
-
-import
-  net, unittest, testutil,
-  eth/keys, eth/p2p/enode, libp2p/multiaddress,
-  ../beacon_chain/eth2_discovery
-
-suiteReport "Discovery v5 utilities":
-  timedTest "Multiaddress to ENode":
-    let addrStr = "/ip4/178.128.140.61/tcp/9000/p2p/16Uiu2HAmL5A5DAiiupFi6sUTF6Zq1TCKf6Pd5T8oFt9opQJqLqTQ"
-    let ma = MultiAddress.init addrStr
-    let enode = ma.toENode
-
-    check:
-      enode.isOk
-      enode.value.address.tcpPort == Port(9000)
-      $enode.value.address.ip == "178.128.140.61"
-      enode.value.toMultiAddressStr == addrStr
-
-  timedTest "ENR to ENode":
-    let enr = "enr:-Iu4QPONEndy6aWOJLWBaCLS1KRg7YPeK0qptnxJzuBW8OcFP9tLgA_ewmAvHBzn9zPG6XIgdH83Mq_5cyLF5yWRYmYBgmlkgnY0gmlwhDaZ6cGJc2VjcDI1NmsxoQK-9tWOso2Kco7L5L-zKoj-MwPfeBbEP12bxr9bqzwZV4N0Y3CCIyiDdWRwgiMo"
-    let enrParsed = parseBootstrapAddress(enr)
-    check enrParsed.isOk
-
-    let enode = enrParsed.value.toENode
-
-    check:
-      enode.isOk
-      $enode.value.address.ip == "54.153.233.193"
-      enode.value.address.udpPort == Port(9000)
-      $enode.value.pubkey == "bef6d58eb28d8a728ecbe4bfb32a88fe3303df7816c43f5d9bc6bf5bab3c19571012d3dd5ab492b1b0d2b42e32ce32f6bafc1075dbaaabe1fa6be711be7a992a"
-

vendor/nim-eth

@@ -1 +1 @@
-Subproject commit ff546d27c3e65df806e499a17e1918a545522094
+Subproject commit a110f091af38e070781de28fea400303d5b3cf43