Unify the bootstrap nodes handling code

We no longer discriminate between ENR, MultiAddress or ENode
bootstrap records (all of them are remapped to ENodes).

The discovery loop will stochastically try to reconnect to
accidentally disconnected nodes.

beacon_chain/beacon_node.nim

@@ -6,12 +6,13 @@ import
   stew/[objects, bitseqs, byteutils],
   chronos, chronicles, confutils, metrics,
   json_serialization/std/[options, sets], serialization/errors,
-  kvstore, kvstore_lmdb, eth/async_utils, eth/p2p/discoveryv5/enr,
+  kvstore, kvstore_lmdb,
+  eth/p2p/enode, eth/[keys, async_utils], eth/p2p/discoveryv5/enr,
 
   # Local modules
   spec/[datatypes, digest, crypto, beaconstate, helpers, validator, network],
-  conf, time, state_transition, beacon_chain_db,
+  conf, time, state_transition, beacon_chain_db, validator_pool, extras,
-  validator_pool, extras, attestation_pool, block_pool, eth2_network,
+  attestation_pool, block_pool, eth2_network, eth2_discovery,
   beacon_node_types, mainchain_monitor, version, ssz, ssz/dynamic_navigator,
   sync_protocol, request_manager, validator_keygen, interop, statusbar
 
@@ -45,10 +46,9 @@ type
     nickname: string
     network: Eth2Node
     forkVersion: array[4, byte]
-    networkIdentity: Eth2NodeIdentity
+    netKeys: DiscKeyPair
     requestManager: RequestManager
-    bootstrapNodes: seq[BootstrapAddr]
+    bootstrapNodes: seq[ENode]
-    bootstrapEnrs: seq[enr.Record]
     db: BeaconChainDB
     config: BeaconNodeConf
     attachedValidators: ValidatorPool
@@ -121,55 +121,10 @@ proc getStateFromSnapshot(conf: BeaconNodeConf, state: var BeaconState): bool =
 
   result = true
 
-proc addBootstrapAddr(v: var seq[BootstrapAddr], add: TaintedString) =
-  try:
-    v.add BootstrapAddr.initAddress(string add)
-  except CatchableError as e:
-    warn "Skipping invalid address", err = e.msg
-
-proc loadBootstrapFile(bootstrapFile: string): seq[BootstrapAddr] =
-  if fileExists(bootstrapFile):
-    for line in lines(bootstrapFile):
-      result.addBootstrapAddr(line)
-
-proc addEnrBootstrapNode(enrBase64: string,
-                         bootNodes: var seq[BootstrapAddr],
-                         enrs: var seq[enr.Record]) =
-  var enrRec: enr.Record
-  if enrRec.fromURI(enrBase64):
-    try:
-      let
-        ip = IpAddress(family: IpAddressFamily.IPv4,
-                       address_v4: cast[array[4, uint8]](enrRec.get("ip", int)))
-        tcpPort = Port enrRec.get("tcp", int)
-        # udpPort = Port enrRec.get("udp", int)
-      bootNodes.add BootstrapAddr.initAddress(ip, tcpPort)
-      enrs.add enrRec
-    except CatchableError as err:
-      warn "Invalid ENR record", enrRec
-  else:
-    warn "Failed to parse ENR record", value = enrRec
-
-proc useEnrBootstrapFile(bootstrapFile: string,
-                         bootNodes: var seq[BootstrapAddr],
-                         enrs: var seq[enr.Record]) =
-  let ext = splitFile(bootstrapFile).ext
-  if cmpIgnoreCase(ext, ".txt") == 0:
-    for ln in lines(bootstrapFile):
-      addEnrBootstrapNode(string ln, bootNodes, enrs)
-  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):
-      addEnrBootstrapNode(string(ln[3..^2]), bootNodes, enrs)
-  else:
-    error "Unknown bootstrap file format", ext
-    quit 1
-
 proc init*(T: type BeaconNode, conf: BeaconNodeConf): Future[BeaconNode] {.async.} =
   let
-    networkId = getPersistentNetIdentity(conf)
+    netKeys = getPersistentNetKeys(conf)
-    nickname = if conf.nodeName == "auto": shortForm(networkId)
+    nickname = if conf.nodeName == "auto": shortForm(netKeys)
                else: conf.nodeName
     db = BeaconChainDB.init(kvStore LmdbStoreRef.init(conf.databaseDir))
 
@@ -222,34 +177,24 @@ proc init*(T: type BeaconNode, conf: BeaconNodeConf): Future[BeaconNode] {.async
     #      monitor
     mainchainMonitor.start()
 
-  var
+  var bootNodes: seq[ENode]
-    bootNodes: seq[BootstrapAddr]
+  for node in conf.bootstrapNodes: bootNodes.addBootstrapNode(node)
-    enrs: seq[enr.Record]
+  bootNodes.loadBootstrapFile(string conf.bootstrapNodesFile)
-
+  bootNodes.loadBootstrapFile(conf.dataDir / "bootstrap_nodes.txt")
-  for node in conf.bootstrapNodes: bootNodes.addBootstrapAddr(node)
+  bootNodes = filterIt(bootNodes, it.pubkey != netKeys.pubkey)
-  bootNodes.add(loadBootstrapFile(string conf.bootstrapNodesFile))
-  bootNodes.add(loadBootstrapFile(conf.dataDir / "bootstrap_nodes.txt"))
-
-  let enrBootstrapFile = string conf.enrBootstrapNodesFile
-  if enrBootstrapFile.len > 0:
-    useEnrBootstrapFile(enrBootstrapFile, bootNodes, enrs)
-
-  bootNodes = filterIt(bootNodes, not it.isSameNode(networkId))
 
   let
-    network = await createEth2Node(conf, bootNodes, enrs)
+    network = await createEth2Node(conf, bootNodes)
-
+    addressFile = string(conf.dataDir) / "beacon_node.address"
-  let addressFile = string(conf.dataDir) / "beacon_node.address"
   network.saveConnectionAddressFile(addressFile)
 
   var res = BeaconNode(
     nickname: nickname,
     network: network,
     forkVersion: blockPool.headState.data.data.fork.current_version,
-    networkIdentity: networkId,
+    netKeys: netKeys,
     requestManager: RequestManager.init(network),
     bootstrapNodes: bootNodes,
-    bootstrapEnrs: enrs,
     db: db,
     config: conf,
     attachedValidators: ValidatorPool.init(),
@@ -1027,7 +972,7 @@ when hasPrompt:
         # arbitrary expression that is resolvable through this API.
         case expr.toLowerAscii
         of "connected_peers":
-          $(sync_protocol.libp2p_peers.value.int)
+          $(libp2p_peers.value.int)
 
         of "last_finalized_epoch":
           var head = node.blockPool.finalizedHead

beacon_chain/block_pool.nim

@@ -712,9 +712,10 @@ proc updateStateData*(pool: BlockPool, state: var StateData, bs: BlockSlot) =
   # applied
   for i in countdown(ancestors.len - 1, 0):
     let ok =
-      skipAndUpdateState(state.data, ancestors[i].data.message, {skipValidation}) do(
+      skipAndUpdateState(state.data,
-        state: HashedBeaconState):
+                         ancestors[i].data.message,
-      pool.maybePutState(state, ancestors[i].refs)
+                         {skipValidation}) do (state: HashedBeaconState):
+        pool.maybePutState(state, ancestors[i].refs)
     doAssert ok, "Blocks in database should never fail to apply.."
 
   skipAndUpdateState(state.data, bs.slot) do(state: HashedBeaconState):

beacon_chain/conf.nim

@@ -90,11 +90,6 @@ type
         desc: "Specifies a line-delimited file of bootsrap Ethereum network addresses."
         name: "bootstrap-file" }: InputFile
 
-      enrBootstrapNodesFile* {.
-        defaultValue: ""
-        desc: "Specifies a line-delimited file of bootstrap ENR records"
-        name: "enr-bootstrap-file" }: InputFile
-
       tcpPort* {.
         defaultValue: defaultPort(config)
         desc: "TCP listening port."

beacon_chain/eth2_discovery.nim

@@ -1,7 +1,9 @@
 import
-  net,
+  os, net, strutils, strformat, parseutils,
-  eth/keys, eth/trie/db,
+  chronicles, stew/result, eth/keys, eth/trie/db, eth/p2p/enode,
-  eth/p2p/discoveryv5/[protocol, node, discovery_db, types],
+  eth/p2p/discoveryv5/[enr, protocol, node, discovery_db, types],
+  libp2p/[multiaddress, multicodec, peer],
+  libp2p/crypto/crypto as libp2pCrypto,
   conf
 
 type
@@ -9,7 +11,7 @@ type
   Eth2DiscoveryId* = NodeId
 
 export
-  Eth2DiscoveryProtocol, open, start, close
+  Eth2DiscoveryProtocol, open, start, close, result
 
 proc new*(T: type Eth2DiscoveryProtocol,
           conf: BeaconNodeConf,
@@ -24,3 +26,119 @@ proc new*(T: type Eth2DiscoveryProtocol,
 
   newProtocol(pk, db, Port conf.udpPort)
 
+proc toENode*(a: MultiAddress): Result[ENode, cstring] =
+  if not IPFS.match(a):
+    return err "Unsupported MultiAddress"
+
+  try:
+    # 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: pubkey.skkey,
+                        address: Address(ip: ipAddress,
+                                         tcpPort: Port tcpPort,
+                                         udpPort: Port udpPort))
+
+  except CatchableError:
+    # This will reach the error exit path below
+    discard
+
+  return err "Invalid MultiAddress"
+
+proc toMultiAddressStr*(enode: ENode): string =
+  var peerId = PeerID.init(libp2pCrypto.PublicKey(scheme: Secp256k1,
+                                                  skkey: enode.pubkey))
+  &"/ip4/{enode.address.ip}/tcp/{enode.address.tcpPort}/p2p/{peerId.pretty}"
+
+proc parseBootstrapAddress*(address: TaintedString): Result[ENode, cstring] =
+  if address.len == 0:
+    return err "an empty string is not a valid bootstrap node"
+
+  logScope:
+    address = string(address)
+
+  if address[0] == '/':
+    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:"):
+      var enrRec: enr.Record
+      if enrRec.fromURI(string address):
+        try:
+          let
+            ip = IpAddress(family: IpAddressFamily.IPv4,
+                           address_v4: cast[array[4, uint8]](enrRec.get("ip", int)))
+            tcpPort = Port enrRec.get("tcp", int)
+            udpPort = Port enrRec.get("udp", int)
+          var pubKey: keys.PublicKey
+          if not enrRec.get(pubKey):
+            return err "Failed to read public key from ENR record"
+          return ok ENode(pubkey: pubkey,
+                          address: Address(ip: ip,
+                                           tcpPort: tcpPort,
+                                           udpPort: udpPort))
+        except CatchableError:
+          # This will continue to the failure path below
+          discard
+      return err "Invalid ENR bootstrap record"
+    elif lowerCaseAddress.startsWith("enode:"):
+      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*(bootNodes: var seq[ENode],
+                       bootstrapAddr: string) =
+  let enodeRes = parseBootstrapAddress(bootstrapAddr)
+  if enodeRes.isOk:
+    bootNodes.add enodeRes.value
+  else:
+    warn "Ignoring invalid bootstrap address",
+          bootstrapAddr, reason = enodeRes.error
+
+proc loadBootstrapFile*(bootNodes: var seq[ENode],
+                        bootstrapFile: string) =
+  if bootstrapFile.len == 0: return
+  let ext = splitFile(bootstrapFile).ext
+  if cmpIgnoreCase(ext, ".txt") == 0:
+    for ln in lines(bootstrapFile):
+      bootNodes.addBootstrapNode(ln)
+  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):
+      bootNodes.addBootstrapNode(string(ln[3..^2]))
+  else:
+    error "Unknown bootstrap file format", ext
+    quit 1
+

beacon_chain/eth2_network.nim

@@ -1,8 +1,12 @@
 import
-  options, tables,
+  options, tables, strutils, sequtils,
-  chronos, json_serialization, strutils, chronicles, metrics,
+  chronos, json_serialization, chronicles, metrics, libp2p/crypto/secp,
-  eth/net/nat, eth/p2p/discoveryv5/enr,
+  eth/keys, eth/p2p/enode, eth/net/nat, eth/p2p/discoveryv5/enr,
-  version, conf
+  eth2_discovery, version, conf
+
+type
+  DiscKeyPair* = keys.KeyPair
+  DiscPrivKey* = keys.PrivateKey
 
 const
   clientId* = "Nimbus beacon node v" & fullVersionStr
@@ -61,7 +65,7 @@ when networkBackend in [libp2p, libp2pDaemon]:
   import
     os, random,
     stew/io, eth/async_utils,
-    libp2p/crypto/crypto, libp2p/[multiaddress, multicodec],
+    libp2p/crypto/crypto as libp2pCrypto, libp2p/[multiaddress, multicodec],
     ssz
 
   export
@@ -94,11 +98,14 @@ when networkBackend in [libp2p, libp2pDaemon]:
     netBackendName* = "libp2p"
     networkKeyFilename = "privkey.protobuf"
 
-  type
+  func asLibp2pKey*(key: DiscPrivKey): libp2pCrypto.PrivateKey =
-    BootstrapAddr* = MultiAddress
+    libp2pCrypto.PrivateKey(scheme: Secp256k1,
-    Eth2NodeIdentity* = KeyPair
+                            skkey: SkPrivateKey(data: key.data))
 
-  proc initAddress*(T: type BootstrapAddr, str: string): T =
+  func asLibp2pKey*(key: keys.PublicKey): libp2pCrypto.PublicKey =
+    libp2pCrypto.PublicKey(scheme: Secp256k1, skkey: key)
+
+  proc initAddress*(T: type MultiAddress, str: string): T =
     let address = MultiAddress.init(str)
     if IPFS.match(address) and matchPartial(multiaddress.TCP, address):
       result = address
@@ -109,36 +116,32 @@ when networkBackend in [libp2p, libp2pDaemon]:
   template tcpEndPoint(address, port): auto =
     MultiAddress.init(address, Protocol.IPPROTO_TCP, port)
 
-  proc initAddress*(T: type BootstrapAddr, ip: IpAddress, tcpPort: Port): T =
+  proc genRandomNetKey: DiscPrivKey =
-    tcpEndPoint(ip, tcpPort)
+    let skkey = SkPrivateKey.random
+    DiscPrivKey(data: skkey.data)
 
   proc ensureNetworkIdFile(conf: BeaconNodeConf): string =
     result = conf.dataDir / networkKeyFilename
     if not fileExists(result):
       createDir conf.dataDir.string
-      let pk = PrivateKey.random(Secp256k1)
+      let pk = genRandomNetKey()
-      writeFile(result, pk.getBytes)
+      writeFile(result, pk.data)
 
-  proc getPersistentNetIdentity*(conf: BeaconNodeConf): Eth2NodeIdentity =
+  proc getPersistentNetKeys*(conf: BeaconNodeConf): DiscKeyPair =
-    let privateKeyFile = conf.dataDir / networkKeyFilename
+    let privKeyPath = conf.dataDir / networkKeyFilename
-    var privKey: PrivateKey
+    var privKey: DiscPrivKey
-    if not fileExists(privateKeyFile):
+    if not fileExists(privKeyPath):
       createDir conf.dataDir.string
-      privKey = PrivateKey.random(Secp256k1)
+      privKey = genRandomNetKey()
-      writeFile(privateKeyFile, privKey.getBytes())
+      writeFile(privKeyPath, privKey.data)
     else:
-      let strdata = readFile(privateKeyFile)
+      let strdata = readFile(privKeyPath)
-      privKey = PrivateKey.init(cast[seq[byte]](strdata))
+      privKey = initPrivateKey(cast[seq[byte]](strdata))
 
-    result = KeyPair(seckey: privKey, pubkey: privKey.getKey())
+    DiscKeyPair(seckey: privKey, pubkey: privKey.getPublicKey())
-
-  proc allMultiAddresses(nodes: seq[BootstrapAddr]): seq[string] =
-    for node in nodes:
-      result.add $node
 
   proc createEth2Node*(conf: BeaconNodeConf,
-                       bootstrapNodes: seq[BootstrapAddr],
+                       bootstrapNodes: seq[ENode]): Future[Eth2Node] {.async.} =
-                       bootstrapEnrs: seq[enr.Record]): Future[Eth2Node] {.async.} =
     var
       (extIp, extTcpPort, _) = setupNat(conf)
       hostAddress = tcpEndPoint(globalListeningAddr, Port conf.tcpPort)
@@ -150,16 +153,13 @@ when networkBackend in [libp2p, libp2pDaemon]:
                                     bootstrapNodes
 
     when networkBackend == libp2p:
-      let keys = conf.getPersistentNetIdentity
+      let keys = conf.getPersistentNetKeys
       # TODO nim-libp2p still doesn't have support for announcing addresses
       # that are different from the host address (this is relevant when we
       # are running behind a NAT).
-      var switch = newStandardSwitch(some keys.seckey, hostAddress,
+      var switch = newStandardSwitch(some keys.seckey.asLibp2pKey, hostAddress,
                                      triggerSelf = true, gossip = false)
       result = Eth2Node.init(conf, switch, keys.seckey)
-      for enr in bootstrapEnrs:
-        result.addKnownPeer(enr)
-      await result.start()
     else:
       let keyFile = conf.ensureNetworkIdFile
 
@@ -173,7 +173,7 @@ when networkBackend in [libp2p, libp2pDaemon]:
                      id = keyFile,
                      hostAddresses = @[hostAddress],
                      announcedAddresses = announcedAddresses,
-                     bootstrapNodes = allMultiAddresses(bootstrapNodes),
+                     bootstrapNodes = mapIt(bootstrapNodes, it.toMultiAddressStr),
                      peersRequired = 1)
 
       mainDaemon = await daemonFut
@@ -185,52 +185,53 @@ when networkBackend in [libp2p, libp2pDaemon]:
       result = await Eth2Node.init(mainDaemon)
 
   proc getPersistenBootstrapAddr*(conf: BeaconNodeConf,
-                                  ip: IpAddress, port: Port): BootstrapAddr =
+                                  ip: IpAddress, port: Port): ENode =
-    let pair = getPersistentNetIdentity(conf)
+    let pair = getPersistentNetKeys(conf)
-    let pidma = MultiAddress.init(multiCodec("p2p"), PeerID.init(pair.pubkey))
+    initENode(pair.pubkey, Address(ip: ip, udpPort: port))
-    result = tcpEndPoint(ip, port) & pidma
 
-  proc isSameNode*(bootstrapNode: BootstrapAddr, id: Eth2NodeIdentity): bool =
+  proc shortForm*(id: DiscKeyPair): string =
-    if IPFS.match(bootstrapNode):
+    $PeerID.init(id.pubkey.asLibp2pKey)
-      let pid1 = PeerID.init(bootstrapNode[2].protoAddress())
-      let pid2 = PeerID.init(id.pubkey)
-      result = (pid1 == pid2)
 
-  proc shortForm*(id: Eth2NodeIdentity): string =
+  proc toPeerInfo(enode: ENode): PeerInfo =
-    $PeerID.init(id.pubkey)
+    let
-
+      peerId = PeerID.init enode.pubkey.asLibp2pKey
-  proc multiAddressToPeerInfo(a: MultiAddress): PeerInfo =
+      addresses = @[MultiAddress.init enode.toMultiAddressStr]
-    if IPFS.match(a):
+    when networkBackend == libp2p:
-      let
+      return PeerInfo.init(peerId, addresses)
-        peerId = PeerID.init(a[2].protoAddress())
+    else:
-        addresses = @[a[0] & a[1]]
+      return PeerInfo(peer: peerId, addresses: addresses)
-      when networkBackend == libp2p:
-        return PeerInfo.init(peerId, addresses)
-      else:
-        return PeerInfo(peer: peerId, addresses: addresses)
 
   proc connectToNetwork*(node: Eth2Node,
-                         bootstrapNodes: seq[MultiAddress]) {.async.} =
+                         bootstrapNodes: seq[ENode]) {.async.} =
-    # TODO: perhaps we should do these in parallel
+    when networkBackend == libp2pDaemon:
-    var connected = false
+      var connected = false
-    for bootstrapNode in bootstrapNodes:
+      for bootstrapNode in bootstrapNodes:
-      try:
+        try:
-        let peerInfo = multiAddressToPeerInfo(bootstrapNode)
+          let peerInfo = toPeerInfo(bootstrapNode)
-        when networkBackend == libp2p:
+          when networkBackend == libp2p:
-          discard await node.switch.dial(peerInfo)
+            discard await node.switch.dial(peerInfo)
-        else:
+          else:
-          await node.daemon.connect(peerInfo.peer, peerInfo.addresses)
+            await node.daemon.connect(peerInfo.peer, peerInfo.addresses)
-        var peer = node.getPeer(peerInfo)
+          var peer = node.getPeer(peerInfo)
-        peer.wasDialed = true
+          peer.wasDialed = true
-        await initializeConnection(peer)
+          await initializeConnection(peer)
-        connected = true
+          connected = true
-      except CatchableError as err:
+        except CatchableError as err:
-        error "Failed to connect to bootstrap node",
+          error "Failed to connect to bootstrap node",
-               node = bootstrapNode, err = err.msg
+                 node = bootstrapNode, err = err.msg
 
-    if bootstrapNodes.len > 0 and connected == false:
+      if bootstrapNodes.len > 0 and connected == false:
-      fatal "Failed to connect to any bootstrap node. Quitting."
+        fatal "Failed to connect to any bootstrap node. Quitting."
-      quit 1
+        quit 1
+    elif networkBackend == libp2p:
+      for bootstrapNode in bootstrapNodes:
+        node.addKnownPeer bootstrapNode
+      await node.start()
+
+      await sleepAsync(10.seconds)
+      if libp2p_peers.get == 0:
+        fatal "Failed to connect to any bootstrap node. Quitting"
+        quit 1
 
   proc saveConnectionAddressFile*(node: Eth2Node, filename: string) =
     when networkBackend == libp2p:

beacon_chain/libp2p_backend.nim

@@ -3,7 +3,7 @@ import
   stew/[varints,base58], stew/shims/[macros, tables], chronos, chronicles,
   stint, faststreams/output_stream, serialization,
   json_serialization/std/options, eth/p2p/p2p_protocol_dsl,
-  eth/p2p/discoveryv5/enr,
+  eth/p2p/discoveryv5/node,
   # TODO: create simpler to use libp2p modules that use re-exports
   libp2p/[switch, multistream, connection,
           multiaddress, peerinfo, peer,
@@ -177,22 +177,33 @@ proc toPeerInfo(r: Option[enr.TypedRecord]): PeerInfo =
   if r.isSome:
     return r.get.toPeerInfo
 
-proc dialPeer*(node: Eth2Node, enr: enr.Record) {.async.} =
+proc dialPeer*(node: Eth2Node, peerInfo: PeerInfo) {.async.} =
-  let peerInfo = enr.toTypedRecord.toPeerInfo
+  debug "Dialing peer", peer = $peerInfo
-  if peerInfo != nil:
+  discard await node.switch.dial(peerInfo)
-    discard await node.switch.dial(peerInfo)
+  var peer = node.getPeer(peerInfo)
-    var peer = node.getPeer(peerInfo)
+  peer.wasDialed = true
-    peer.wasDialed = true
+  await initializeConnection(peer)
-    await initializeConnection(peer)
+
+proc runDiscoveryLoop*(node: Eth2Node) {.async.} =
+  debug "Starting discovery loop"
 
-proc runDiscoveryLoop(node: Eth2Node) {.async.} =
   while true:
-    if node.peersByDiscoveryId.len < node.wantedPeers:
+    let currentPeerCount = node.switch.connections.len
-      let discoveredPeers = await node.discovery.lookupRandom()
+    libp2p_peers.set currentPeerCount.int64
-      for peer in discoveredPeers:
+    if currentPeerCount < node.wantedPeers:
-        if peer.id notin node.peersByDiscoveryId:
+      try:
-          # TODO do this in parallel
+        let discoveredPeers = await node.discovery.lookupRandom()
-          await node.dialPeer(peer.record)
+        for peer in discoveredPeers:
+          debug "Discovered peer", peer = $peer
+          try:
+            let peerInfo = peer.record.toTypedRecord.toPeerInfo
+            if peerInfo != nil and peerInfo.id notin node.switch.connections:
+              # TODO do this in parallel
+              await node.dialPeer(peerInfo)
+          except CatchableError as err:
+            debug "Failed to connect to peer", peer = $peer
+      except CatchableError as err:
+        debug "Failure in discovery", err = err.msg
 
     await sleepAsync seconds(1)
 
@@ -213,13 +224,14 @@ proc init*(T: type Eth2Node, conf: BeaconNodeConf,
       if msg.protocolMounter != nil:
         msg.protocolMounter result
 
-proc addKnownPeer*(node: Eth2Node, peerEnr: enr.Record) =
+proc addKnownPeer*(node: Eth2Node, peer: ENode) =
-  node.discovery.addNode peerEnr
+  node.discovery.addNode peer
 
 proc start*(node: Eth2Node) {.async.} =
   node.discovery.open()
   node.discovery.start()
   node.libp2pTransportLoops = await node.switch.start()
+  traceAsyncErrors node.runDiscoveryLoop()
 
 proc init*(T: type Peer, network: Eth2Node, info: PeerInfo): Peer =
   new result

beacon_chain/libp2p_backends_common.nim

@@ -1,3 +1,6 @@
+import
+  metrics
+
 type
   ResponseCode* = enum
     Success
@@ -22,6 +25,8 @@ const
 logScope:
   topics = "libp2p"
 
+declarePublicGauge libp2p_peers, "Number of libp2p peers"
+
 template libp2pProtocol*(name: string, version: int) {.pragma.}
 
 proc getRequestProtoName(fn: NimNode): NimNode =

beacon_chain/sync_protocol.nim

@@ -1,14 +1,12 @@
 import
   options, tables, sets, macros,
-  chronicles, chronos, metrics, stew/ranges/bitranges,
+  chronicles, chronos, stew/ranges/bitranges,
   spec/[datatypes, crypto, digest, helpers],
   beacon_node_types, eth2_network, block_pool, ssz
 
 when networkBackend == libp2p:
   import libp2p/switch
 
-declarePublicGauge libp2p_peers, "Number of libp2p peers"
-
 logScope:
   topics = "sync"
 
@@ -100,9 +98,6 @@ p2pProtocol BeaconSync(version = 1,
       else:
         warn "Status response not received in time"
 
-  onPeerDisconnected do (peer: Peer):
-    libp2p_peers.set peer.network.peers.len.int64
-
   requestResponse:
     proc status(peer: Peer, theirStatus: StatusMsg) {.libp2pProtocol("status", 1).} =
       let
@@ -181,8 +176,6 @@ proc handleInitialStatus(peer: Peer,
   # where it needs to sync and it should execute the sync algorithm with a certain
   # number of randomly selected peers. The algorithm itself must be extracted in a proc.
   try:
-    libp2p_peers.set peer.network.peers.len.int64
-
     debug "Peer connected. Initiating sync", peer,
           localHeadSlot = ourStatus.headSlot,
           remoteHeadSlot = theirStatus.headSlot,

tests/test_discovery_helpers.nim

@@ -0,0 +1,17 @@
+import
+  net, unittest, testutil,
+  eth/p2p/enode, libp2p/multiaddress,
+  ../beacon_chain/eth2_discovery
+
+suite "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
+