Fix direct peers (#5427)

* Fix direct peers

* Support ENRs in DP, use DP in local testnet

* fix docs

* bump libp2p

.gitmodules

@@ -24,7 +24,7 @@
 	path = vendor/nim-libp2p
 	url = https://github.com/status-im/nim-libp2p.git
 	ignore = untracked
-	branch = nimbus
+	branch = unstable
 [submodule "vendor/nimbus-build-system"]
 	path = vendor/nimbus-build-system
 	url = https://github.com/status-im/nimbus-build-system.git

beacon_chain/conf.nim

@@ -551,7 +551,7 @@ type
         name: "dump" .}: bool
 
       directPeers* {.
-        desc: "The list of privileged, secure and known peers to connect and maintain the connection to, this requires a not random netkey-file. In the complete multiaddress format like: /ip4/<address>/tcp/<port>/p2p/<peerId-public-key>. Peering agreements are established out of band and must be reciprocal."
+        desc: "The list of privileged, secure and known peers to connect and maintain the connection to. This requires a not random netkey-file. In the multiaddress format like: /ip4/<address>/tcp/<port>/p2p/<peerId-public-key>, or enr format (enr:-xx). Peering agreements are established out of band and must be reciprocal"
         name: "direct-peer" .}: seq[string]
 
       doppelgangerDetection* {.

beacon_chain/networking/eth2_network.nim

@@ -48,6 +48,8 @@ type
   ErrorMsg = List[byte, 256]
   SendResult* = Result[void, cstring]
 
+  DirectPeers = Table[PeerId, seq[MultiAddress]]
+
   # TODO: This is here only to eradicate a compiler
   # warning about unused import (rpc/messages).
   GossipMsg = messages.Message
@@ -77,6 +79,7 @@ type
     forkDigests*: ref ForkDigests
     rng*: ref HmacDrbgContext
     peers*: Table[PeerId, Peer]
+    directPeers*: DirectPeers
     validTopics: HashSet[string]
     peerPingerHeartbeatFut: Future[void]
     peerTrimmerHeartbeatFut: Future[void]
@@ -1509,6 +1512,7 @@ proc trimConnections(node: Eth2Node, count: int) =
   var toKick = count
 
   for peerId in scores.keys:
+    if peerId in node.directPeers: continue
     debug "kicking peer", peerId, score=scores[peerId]
     asyncSpawn node.getPeer(peerId).disconnect(PeerScoreLow)
     dec toKick
@@ -1793,6 +1797,7 @@ proc new(T: type Eth2Node,
          switch: Switch, pubsub: GossipSub,
          ip: Option[ValidIpAddress], tcpPort, udpPort: Option[Port],
          privKey: keys.PrivateKey, discovery: bool,
+         directPeers: DirectPeers,
          rng: ref HmacDrbgContext): T {.raises: [CatchableError].} =
   when not defined(local_testnet):
     let
@@ -1835,6 +1840,7 @@ proc new(T: type Eth2Node,
     rng: rng,
     connectTimeout: connectTimeout,
     seenThreshold: seenThreshold,
+    directPeers: directPeers,
     quota: TokenBucket.new(maxGlobalQuota, fullReplenishTime)
   )
 
@@ -2313,6 +2319,24 @@ proc createEth2Node*(rng: ref HmacDrbgContext,
     except CatchableError as exc: raise exc
     except Exception as exc: raiseAssert exc.msg
 
+    directPeers = block:
+      var res: DirectPeers
+      for s in config.directPeers:
+        let (peerId, address) =
+          if s.startsWith("enr:"):
+            let
+              typedEnr = parseBootstrapAddress(s).get().toTypedRecord().get()
+              peerAddress = toPeerAddr(typedEnr, tcpProtocol).get()
+            (peerAddress.peerId, peerAddress.addrs[0])
+          elif s.startsWith("/"):
+            parseFullAddress(s).tryGet()
+          else:
+            fatal "direct peers address should start with / (multiaddress) or enr:", conf=s
+            quit 1
+        res.mgetOrPut(peerId, @[]).add(address)
+        info "Adding privileged direct peer", peerId, address
+      res
+
     hostAddress = tcpEndPoint(config.listenAddress, config.tcpPort)
     announcedAddresses = if extIp.isNone() or extTcpPort.isNone(): @[]
                          else: @[tcpEndPoint(extIp.get(), extTcpPort.get())]
@@ -2371,19 +2395,7 @@ proc createEth2Node*(rng: ref HmacDrbgContext,
       behaviourPenaltyWeight: -15.9,
       behaviourPenaltyDecay: 0.986,
       disconnectBadPeers: true,
-      directPeers:
+      directPeers: directPeers,
-        block:
-          var res = initTable[PeerId, seq[MultiAddress]]()
-          if config.directPeers.len > 0:
-            for s in config.directPeers:
-              let
-                maddress = MultiAddress.init(s).tryGet()
-                mpeerId = maddress[multiCodec("p2p")].tryGet()
-                peerId = PeerId.init(mpeerId.protoAddress().tryGet()).tryGet()
-              res.mgetOrPut(peerId, @[]).add(maddress)
-              info "Adding priviledged direct peer", peerId, address = maddress
-          res
-        ,
       bandwidthEstimatebps: config.bandwidthEstimate.get(100_000_000)
     )
     pubsub = GossipSub.init(
@@ -2402,7 +2414,7 @@ proc createEth2Node*(rng: ref HmacDrbgContext,
   let node = Eth2Node.new(
     config, cfg, enrForkId, discoveryForkId, forkDigests, getBeaconTime, switch, pubsub, extIp,
     extTcpPort, extUdpPort, netKeys.seckey.asEthKey,
-    discovery = config.discv5Enabled, rng = rng)
+    discovery = config.discv5Enabled, directPeers, rng = rng)
 
   node.pubsub.subscriptionValidator =
     proc(topic: string): bool {.gcsafe, raises: [].} =

docs/the_nimbus_book/src/options.md

@@ -109,9 +109,10 @@ The following options are available:
      --discv5                  Enable Discovery v5 [=true].
      --dump                    Write SSZ dumps of blocks, attestations and states to data dir [=false].
      --direct-peer             The list of privileged, secure and known peers to connect and maintain the
-                               connection to, this requires a not random netkey-file. In the complete
+                               connection to. This requires a not random netkey-file. In the multiaddress
-                               multiaddress format like: /ip4/<address>/tcp/<port>/p2p/<peerId-public-key>.
+                               format like: /ip4/<address>/tcp/<port>/p2p/<peerId-public-key>, or enr format
-                               Peering agreements are established out of band and must be reciprocal..
+                               (enr:-xx). Peering agreements are established out of band and must be
+                               reciprocal.
      --doppelganger-detection  If enabled, the beacon node prudently listens for 2 epochs for attestations from
                                a validator with the same index (a doppelganger), before sending an attestation
                                itself. This protects against slashing (due to double-voting) but means you will

ncli/ncli_testnet.nim

@@ -32,6 +32,7 @@ type
   StartUpCommand {.pure.} = enum
     generateDeposits
     createTestnet
+    createTestnetEnr
     run
     sendDeposits
     analyzeLogs
@@ -165,6 +166,36 @@ type
         desc: "Output file with list of bootstrap nodes for the network"
         name: "output-bootstrap-file" .}: OutFile
 
+    of StartUpCommand.createTestnetEnr:
+      inputBootstrapEnr* {.
+        desc: "Path to the bootstrap ENR"
+        name: "bootstrap-enr" .}: InputFile
+
+      enrDataDir* {.
+        desc: "Nimbus data directory where the keys of the node will be placed"
+        name: "data-dir" .}: OutDir
+
+      enrNetKeyFile* {.
+        desc: "Source of network (secp256k1) private key file"
+        name: "enr-netkey-file" .}: OutFile
+
+      enrNetKeyInsecurePassword* {.
+        desc: "Use pre-generated INSECURE password for network private key file"
+        defaultValue: false,
+        name: "insecure-netkey-password" .}: bool
+
+      enrAddress* {.
+        desc: "The public IP address of that ENR"
+        defaultValue: init(ValidIpAddress, defaultAdminListenAddress)
+        defaultValueDesc: $defaultAdminListenAddressDesc
+        name: "enr-address" .}: ValidIpAddress
+
+      enrPort* {.
+        desc: "The TCP/UDP port of that ENR"
+        defaultValue: defaultEth2TcpPort
+        defaultValueDesc: $defaultEth2TcpPortDesc
+        name: "enr-port" .}: Port
+
     of StartUpCommand.sendDeposits:
       depositsFile* {.
         desc: "A LaunchPad deposits file"
@@ -320,6 +351,46 @@ proc createDepositTreeSnapshot(deposits: seq[DepositData],
     depositContractState: merkleizer.toDepositContractState,
     blockHeight: blockHeight)
 
+proc createEnr(rng: var HmacDrbgContext,
+               dataDir: string,
+               netKeyFile: string,
+               netKeyInsecurePassword: bool,
+               cfg: RuntimeConfig,
+               forkId: seq[byte],
+               address: ValidIpAddress,
+               port: Port): enr.Record
+               {.raises: [CatchableError].} =
+  type MetaData = altair.MetaData
+  let
+    networkKeys = rng.getPersistentNetKeys(
+      dataDir, netKeyFile, netKeyInsecurePassword, allowLoadExisting = false)
+
+    netMetadata = MetaData()
+    bootstrapEnr = enr.Record.init(
+      1, # sequence number
+      networkKeys.seckey.asEthKey,
+      some(address),
+      some(port),
+      some(port),
+      [
+        toFieldPair(enrForkIdField, forkId),
+        toFieldPair(enrAttestationSubnetsField, SSZ.encode(netMetadata.attnets))
+      ])
+  bootstrapEnr.tryGet()
+
+proc doCreateTestnetEnr*(config: CliConfig,
+                         rng: var HmacDrbgContext)
+                        {.raises: [CatchableError].} =
+  let
+    cfg = getRuntimeConfig(config.eth2Network)
+    bootstrapEnr = parseBootstrapAddress(toSeq(lines(string config.inputBootstrapEnr))[0]).get()
+    forkIdField = bootstrapEnr.tryGet(enrForkIdField, seq[byte]).get()
+    enr =
+      createEnr(rng, string config.enrDataDir, string config.enrNetKeyFile,
+        config.enrNetKeyInsecurePassword, cfg, forkIdField,
+        config.enrAddress, config.enrPort)
+  stderr.writeLine(enr.toURI)
+
 proc doCreateTestnet*(config: CliConfig,
                       rng: var HmacDrbgContext)
                      {.raises: [CatchableError].} =
@@ -417,29 +488,16 @@ proc doCreateTestnet*(config: CliConfig,
 
   let bootstrapFile = string config.outputBootstrapFile
   if bootstrapFile.len > 0:
-    type MetaData = altair.MetaData
     let
-      networkKeys = rng.getPersistentNetKeys(
-        string config.dataDir, string config.netKeyFile,
-        config.netKeyInsecurePassword, allowLoadExisting = false)
-
-      netMetadata = MetaData()
       forkId = getENRForkID(
         cfg,
         Epoch(0),
         genesisValidatorsRoot)
-      bootstrapEnr = enr.Record.init(
+      enr =
-        1, # sequence number
+        createEnr(rng, string config.dataDir, string config.netKeyFile,
-        networkKeys.seckey.asEthKey,
+          config.netKeyInsecurePassword, cfg, SSZ.encode(forkId),
-        some(config.bootstrapAddress),
+          config.bootstrapAddress, config.bootstrapPort)
-        some(config.bootstrapPort),
+    writeFile(bootstrapFile, enr.toURI)
-        some(config.bootstrapPort),
-        [
-          toFieldPair(enrForkIdField, SSZ.encode(forkId)),
-          toFieldPair(enrAttestationSubnetsField, SSZ.encode(netMetadata.attnets))
-        ])
-
-    writeFile(bootstrapFile, bootstrapEnr.tryGet().toURI)
     echo "Wrote ", bootstrapFile
 
 proc deployContract*(web3: Web3, code: string): Future[ReceiptObject] {.async.} =
@@ -593,6 +651,10 @@ proc main() {.async.} =
     let rng = HmacDrbgContext.new()
     doCreateTestnet(conf, rng[])
 
+  of StartUpCommand.createTestnetEnr:
+    let rng = HmacDrbgContext.new()
+    doCreateTestnetEnr(conf, rng[])
+
   of StartUpCommand.deployDepositContract:
     let web3 = await initWeb3(conf.web3Url, conf.privateKey)
     let receipt = await web3.deployContract(depositContractCode)

scripts/launch_local_testnet.sh

@@ -806,6 +806,13 @@ DEPOSITS_FILE="${DATA_DIR}/deposits.json"
 CONTAINER_DEPOSITS_FILE="${CONTAINER_DATA_DIR}/deposits.json"
 CONTAINER_DEPOSIT_TREE_SNAPSHOT_FILE="${CONTAINER_DATA_DIR}/deposit_tree_snapshot.ssz"
 
+CONTAINER_BOOTSTRAP_NETWORK_KEYFILE="bootstrap_network_key.json"
+DIRECTPEER_NETWORK_KEYFILE="directpeer_network_key.json"
+
+
+BOOTSTRAP_NODE=1
+DIRECTPEER_NODE=2
+
 if command -v ulimit; then
   echo "Raising limits"
   ulimit -n $((TOTAL_VALIDATORS * 10))
@@ -885,22 +892,38 @@ fi
 
 jq -r '.hash' "$EXECUTION_GENESIS_BLOCK_JSON" > "${DATA_DIR}/deposit_contract_block_hash.txt"
 
+for NUM_NODE in $(seq 1 $NUM_NODES); do
+  NODE_DATA_DIR="${DATA_DIR}/node${NUM_NODE}"
+  rm -rf "${NODE_DATA_DIR}"
+  scripts/makedir.sh "${NODE_DATA_DIR}" 2>&1
+done
+
 ./build/ncli_testnet createTestnet \
-  --data-dir="$CONTAINER_DATA_DIR" \
+  --data-dir="$CONTAINER_DATA_DIR/node$BOOTSTRAP_NODE" \
   --deposits-file="$CONTAINER_DEPOSITS_FILE" \
   --total-validators=$TOTAL_VALIDATORS \
   --output-genesis="$CONTAINER_DATA_DIR/genesis.ssz" \
   --output-bootstrap-file="$CONTAINER_DATA_DIR/bootstrap_nodes.txt" \
   --output-deposit-tree-snapshot="$CONTAINER_DEPOSIT_TREE_SNAPSHOT_FILE" \
   --bootstrap-address=127.0.0.1 \
-  --bootstrap-port=$BASE_PORT \
+  --bootstrap-port=$(( BASE_PORT + BOOTSTRAP_NODE - 1 )) \
-  --netkey-file=network_key.json \
+  --netkey-file=$CONTAINER_BOOTSTRAP_NETWORK_KEYFILE \
   --insecure-netkey-password=true \
   --genesis-time=$GENESIS_TIME \
   --capella-fork-epoch=$CAPELLA_FORK_EPOCH \
   --deneb-fork-epoch=$DENEB_FORK_EPOCH \
   --execution-genesis-block="$EXECUTION_GENESIS_BLOCK_JSON"
 
+DIRECTPEER_ENR=$(
+  ./build/ncli_testnet createTestnetEnr \
+    --data-dir="$CONTAINER_DATA_DIR/node$DIRECTPEER_NODE" \
+    --bootstrap-enr="$CONTAINER_DATA_DIR/bootstrap_nodes.txt" \
+    --enr-address=127.0.0.1 \
+    --enr-port=$(( BASE_PORT + DIRECTPEER_NODE - 1 )) \
+    --enr-netkey-file=$DIRECTPEER_NETWORK_KEYFILE \
+    --insecure-netkey-password=true 2>&1 > /dev/null
+)
+
 ./scripts/make_prometheus_config.sh \
     --nodes ${NUM_NODES} \
     --base-metrics-port ${BASE_METRICS_PORT} \
@@ -948,7 +971,6 @@ dump_logtrace() {
 }
 
 NODES_WITH_VALIDATORS=${NODES_WITH_VALIDATORS:-$NUM_NODES}
-BOOTSTRAP_NODE=1
 SYSTEM_VALIDATORS=$(( TOTAL_VALIDATORS - USER_VALIDATORS ))
 VALIDATORS_PER_NODE=$(( SYSTEM_VALIDATORS / NODES_WITH_VALIDATORS ))
 if [[ "${USE_VC}" == "1" ]]; then
@@ -981,8 +1003,6 @@ VALIDATOR_OFFSET=$(( SYSTEM_VALIDATORS / 2 ))
 BOOTSTRAP_ENR="${DATA_DIR}/node${BOOTSTRAP_NODE}/beacon_node.enr"
 CONTAINER_BOOTSTRAP_ENR="${CONTAINER_DATA_DIR}/node${BOOTSTRAP_NODE}/beacon_node.enr"
 
-CONTAINER_NETWORK_KEYFILE="network_key.json"
-
 # TODO The deposit generator tool needs to gain support for generating two sets
 #      of deposits (genesis + submitted ones). Then we can enable the sending of
 #      deposits here.
@@ -998,8 +1018,6 @@ for NUM_NODE in $(seq 1 $NUM_NODES); do
   # The first $NODES_WITH_VALIDATORS nodes split them equally between them,
   # after skipping the first $USER_VALIDATORS.
   NODE_DATA_DIR="${DATA_DIR}/node${NUM_NODE}"
-  rm -rf "${NODE_DATA_DIR}"
-  scripts/makedir.sh "${NODE_DATA_DIR}" 2>&1
   scripts/makedir.sh "${NODE_DATA_DIR}/validators" 2>&1
   scripts/makedir.sh "${NODE_DATA_DIR}/secrets" 2>&1
 
@@ -1081,28 +1099,21 @@ for NUM_NODE in $(seq 1 $NUM_NODES); do
     # Due to star topology, the bootstrap node must relay all attestations,
     # even if it itself is not interested. --subscribe-all-subnets could be
     # removed by switching to a fully-connected topology.
-    BOOTSTRAP_ARG="--netkey-file=${CONTAINER_NETWORK_KEYFILE} --insecure-netkey-password=true --subscribe-all-subnets"
+    BOOTSTRAP_ARG="--netkey-file=${CONTAINER_BOOTSTRAP_NETWORK_KEYFILE} --insecure-netkey-password=true --subscribe-all-subnets --direct-peer=$DIRECTPEER_ENR"
+  elif [[ ${NUM_NODE} == ${DIRECTPEER_NODE} ]]; then
+    # Start a node using the Direct Peer functionality instead of regular bootstraping
+    BOOTSTRAP_ARG="--netkey-file=${DIRECTPEER_NETWORK_KEYFILE} --direct-peer=$(cat $CONTAINER_BOOTSTRAP_ENR) --insecure-netkey-password=true"
   else
     BOOTSTRAP_ARG="--bootstrap-file=${CONTAINER_BOOTSTRAP_ENR}"
+  fi
 
+  if [[ ${NUM_NODE} != ${BOOTSTRAP_NODE} ]]; then
     if [[ "${CONST_PRESET}" == "minimal" ]]; then
       # The fast epoch and slot times in the minimal config might cause the
       # mesh to break down due to re-subscriptions happening within the prune
       # backoff time
       BOOTSTRAP_ARG="${BOOTSTRAP_ARG} --subscribe-all-subnets"
     fi
-
-    # Wait for the master node to write out its address file
-    START_TIMESTAMP=$(date +%s)
-    while [[ ! -f "${BOOTSTRAP_ENR}" ]]; do
-      sleep 0.1
-      NOW_TIMESTAMP=$(date +%s)
-      if [[ "$(( NOW_TIMESTAMP - START_TIMESTAMP - GENESIS_OFFSET ))" -ge "$BOOTSTRAP_TIMEOUT" ]]; then
-        echo "Bootstrap node failed to start in ${BOOTSTRAP_TIMEOUT} seconds. Aborting."
-        dump_logs
-        exit 1
-      fi
-    done
   fi
 
   WEB3_ARG=()
@@ -1190,6 +1201,18 @@ for NUM_NODE in $(seq 1 $NUM_NODES); do
       echo $PID > "$DATA_DIR/pids/nimbus_validator_client.${NUM_NODE}"
     fi
   fi
+
+  # Wait for the master node to write out its address file
+  START_TIMESTAMP=$(date +%s)
+  while [[ ! -f "${BOOTSTRAP_ENR}" ]]; do
+    sleep 0.1
+    NOW_TIMESTAMP=$(date +%s)
+    if [[ "$(( NOW_TIMESTAMP - START_TIMESTAMP - GENESIS_OFFSET ))" -ge "$BOOTSTRAP_TIMEOUT" ]]; then
+      echo "Bootstrap node failed to start in ${BOOTSTRAP_TIMEOUT} seconds. Aborting."
+      dump_logs
+      exit 1
+    fi
+  done
 done
 
 # light clients

vendor/nim-libp2p

@@ -1 +1 @@
-Subproject commit 1b8a7d271359418d6fe72bbe2976b66ab370df9b
+Subproject commit b2eac7ecbdb695b0b7033f2069b03a63d28aee2b