refactor(beacon-node): simplify libp2p init (#5774)

* refactor(beacon-node): simplify libp2p init * fix: update discv5 bootnode parsing * feat: throw on invalid bootnodes * chore: use valid enrs in tests * fix: validate enrs coming from bootnodesFile * fix: beacon args handler test
ChainSafe · Jul 21, 2023 · 03c36e2 · 03c36e2
1 parent 50551d3
commit 03c36e2
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 176 deletions.
diff --git a/packages/beacon-node/src/network/core/networkCore.ts b/packages/beacon-node/src/network/core/networkCore.ts
@@ -22,7 +22,7 @@ import {FORK_EPOCH_LOOKAHEAD, getActiveForks} from "../forks.js";
 import {NetworkOptions} from "../options.js";
 import {CommitteeSubscription, IAttnetsService} from "../subnets/interface.js";
 import {MetadataController} from "../metadata.js";
-import {createNodeJsLibp2p} from "../nodejs/util.js";
+import {createNodeJsLibp2p} from "../libp2p/index.js";
 import {PeersData} from "../peers/peersData.js";
 import {PeerAction, PeerRpcScoreStore, PeerScoreStats} from "../peers/index.js";
 import {getConnectionsMap} from "../util.js";

diff --git a/packages/beacon-node/src/network/index.ts b/packages/beacon-node/src/network/index.ts
@@ -1,7 +1,7 @@
 export * from "./events.js";
 export * from "./interface.js";
 export * from "./network.js";
-export * from "./nodejs/index.js";
+export * from "./libp2p/index.js";
 export * from "./gossip/index.js";
 export * from "./reqresp/ReqRespBeaconNode.js";
 export * from "./util.js";

diff --git a/.../beacon-node/src/network/nodejs/bundle.ts → ...s/beacon-node/src/network/libp2p/index.ts b/.../beacon-node/src/network/nodejs/bundle.ts → ...s/beacon-node/src/network/libp2p/index.ts
@@ -1,72 +1,93 @@
-import {createLibp2p} from "libp2p";
+import {PeerId} from "@libp2p/interface-peer-id";
+import {Registry} from "prom-client";
+import {ENR} from "@chainsafe/discv5";
+import type {Components} from "libp2p/components";
 import {identifyService} from "libp2p/identify";
-import {tcp} from "@libp2p/tcp";
-import {mplex} from "@libp2p/mplex";
 import {bootstrap} from "@libp2p/bootstrap";
 import {mdns} from "@libp2p/mdns";
-import {PeerId} from "@libp2p/interface-peer-id";
-import {Datastore} from "interface-datastore";
-import type {PeerDiscovery} from "@libp2p/interface-peer-discovery";
-import type {Components} from "libp2p/components";
+import {createLibp2p} from "libp2p";
+import {mplex} from "@libp2p/mplex";
 import {prometheusMetrics} from "@libp2p/prometheus-metrics";
-import {Registry} from "prom-client";
+import {tcp} from "@libp2p/tcp";
+import {defaultNetworkOptions, NetworkOptions} from "../options.js";
+import {Eth2PeerDataStore} from "../peers/datastore.js";
 import {Libp2p} from "../interface.js";
 import {createNoise} from "./noise.js";
 
-export type Libp2pOptions = {
-  peerId: PeerId;
-  addresses: {
-    listen: string[];
-    announce?: string[];
-  };
-  datastore?: Datastore;
-  peerDiscovery?: ((components: Components) => PeerDiscovery)[];
-  bootMultiaddrs?: string[];
-  maxConnections?: number;
-  minConnections?: number;
+export type NodeJsLibp2pOpts = {
+  peerStoreDir?: string;
+  disablePeerDiscovery?: boolean;
   metrics?: boolean;
   metricsRegistry?: Registry;
-  lodestarVersion?: string;
-  hideAgentVersion?: boolean;
-  mdns?: boolean;
 };
 
-export async function createNodejsLibp2p(options: Libp2pOptions): Promise<Libp2p> {
+export async function getDiscv5Multiaddrs(bootEnrs: string[]): Promise<string[]> {
+  const bootMultiaddrs = [];
+  for (const enrStr of bootEnrs) {
+    const enr = ENR.decodeTxt(enrStr);
+    const multiaddrWithPeerId = (await enr.getFullMultiaddr("tcp"))?.toString();
+    if (multiaddrWithPeerId) {
+      bootMultiaddrs.push(multiaddrWithPeerId);
+    }
+  }
+  return bootMultiaddrs;
+}
+
+export async function createNodeJsLibp2p(
+  peerId: PeerId,
+  networkOpts: Partial<NetworkOptions> = {},
+  nodeJsLibp2pOpts: NodeJsLibp2pOpts = {}
+): Promise<Libp2p> {
+  const localMultiaddrs = networkOpts.localMultiaddrs || defaultNetworkOptions.localMultiaddrs;
+  const {peerStoreDir, disablePeerDiscovery} = nodeJsLibp2pOpts;
+
+  let datastore: undefined | Eth2PeerDataStore = undefined;
+  if (peerStoreDir) {
+    datastore = new Eth2PeerDataStore(peerStoreDir);
+    await datastore.open();
+  }
+
   const peerDiscovery = [];
-  if (options.peerDiscovery) {
-    peerDiscovery.push(...options.peerDiscovery);
-  } else {
-    if ((options.bootMultiaddrs?.length ?? 0) > 0) {
-      peerDiscovery.push(bootstrap({list: options.bootMultiaddrs ?? []}));
+  if (!disablePeerDiscovery) {
+    const bootMultiaddrs = [
+      ...(networkOpts.bootMultiaddrs ?? defaultNetworkOptions.bootMultiaddrs ?? []),
+      // Append discv5.bootEnrs to bootMultiaddrs if requested
+      ...(networkOpts.connectToDiscv5Bootnodes ? await getDiscv5Multiaddrs(networkOpts.discv5?.bootEnrs ?? []) : []),
+    ];
+
+    if ((bootMultiaddrs.length ?? 0) > 0) {
+      peerDiscovery.push(bootstrap({list: bootMultiaddrs}));
     }
-    if (options.mdns) {
+
+    if (networkOpts.mdns) {
       peerDiscovery.push(mdns());
     }
   }
+
   return createLibp2p({
-    peerId: options.peerId,
+    peerId,
     addresses: {
-      listen: options.addresses.listen,
-      announce: options.addresses.announce || [],
+      listen: localMultiaddrs,
+      announce: [],
     },
     connectionEncryption: [createNoise()],
     // Reject connections when the server's connection count gets high
     transports: [
       tcp({
-        maxConnections: options.maxConnections,
+        maxConnections: networkOpts.maxPeers,
         closeServerOnMaxConnections: {
-          closeAbove: options.maxConnections ?? Infinity,
-          listenBelow: options.maxConnections ?? Infinity,
+          closeAbove: networkOpts.maxPeers ?? Infinity,
+          listenBelow: networkOpts.maxPeers ?? Infinity,
         },
       }),
     ],
     streamMuxers: [mplex({maxInboundStreams: 256})],
     peerDiscovery,
-    metrics: options.metrics
+    metrics: nodeJsLibp2pOpts.metrics
       ? prometheusMetrics({
           collectDefaultMetrics: false,
           preserveExistingMetrics: true,
-          registry: options.metricsRegistry,
+          registry: nodeJsLibp2pOpts.metricsRegistry,
         })
       : undefined,
     connectionManager: {
@@ -81,14 +102,10 @@ export async function createNodejsLibp2p(options: Libp2pOptions): Promise<Libp2p
       // DOCS: There is no way to turn off autodial other than setting minConnections to 0
       minConnections: 0,
     },
-    datastore: options.datastore,
+    datastore,
     services: {
       identify: identifyService({
-        agentVersion: options.hideAgentVersion
-          ? ""
-          : options.lodestarVersion
-          ? `lodestar/${options.lodestarVersion}`
-          : "lodestar",
+        agentVersion: networkOpts.private ? "" : networkOpts.version ? `lodestar/${networkOpts.version}` : "lodestar",
       }),
       // individual components are specified because the components object is a Proxy
       // and passing it here directly causes problems downstream, not to mention is slowwww

diff --git a/...s/beacon-node/src/network/nodejs/noise.ts → ...s/beacon-node/src/network/libp2p/noise.ts b/...s/beacon-node/src/network/nodejs/noise.ts → ...s/beacon-node/src/network/libp2p/noise.ts
diff --git a/packages/beacon-node/src/network/nodejs/index.ts b/packages/beacon-node/src/network/nodejs/index.ts
diff --git a/packages/beacon-node/src/network/nodejs/util.ts b/packages/beacon-node/src/network/nodejs/util.ts
diff --git a/packages/beacon-node/test/unit/network/util.test.ts b/packages/beacon-node/test/unit/network/util.test.ts
@@ -1,10 +1,7 @@
 import {expect} from "chai";
-import {createSecp256k1PeerId} from "@libp2p/peer-id-factory";
-import {generateKeypair, KeypairType, SignableENR} from "@chainsafe/discv5";
 import {config} from "@lodestar/config/default";
 import {ForkName} from "@lodestar/params";
-import {defaultNetworkOptions} from "../../../src/network/options.js";
-import {createNodeJsLibp2p} from "../../../src/network/index.js";
+import {getDiscv5Multiaddrs} from "../../../src/network/libp2p/index.js";
 import {getCurrentAndNextFork} from "../../../src/network/forks.js";
 
 describe("getCurrentAndNextFork", function () {
@@ -36,33 +33,13 @@ describe("getCurrentAndNextFork", function () {
   });
 });
 
-describe("createNodeJsLibp2p", () => {
+describe("getDiscv5Multiaddrs", () => {
   it("should extract bootMultiaddrs from enr with tcp", async function () {
     this.timeout(0);
-    const peerId = await createSecp256k1PeerId();
     const enrWithTcp = [
       "enr:-LK4QDiPGwNomqUqNDaM3iHYvtdX7M5qngson6Qb2xGIg1LwC8-Nic0aQwO0rVbJt5xp32sRE3S1YqvVrWO7OgVNv0kBh2F0dG5ldHOIAAAAAAAAAACEZXRoMpA7CIeVAAAgCf__________gmlkgnY0gmlwhBKNA4qJc2VjcDI1NmsxoQKbBS4ROQ_sldJm5tMgi36qm5I5exKJFb4C8dDVS_otAoN0Y3CCIyiDdWRwgiMo",
     ];
-    const bootMultiaddrs: string[] = [];
-    const keypair = generateKeypair(KeypairType.Secp256k1);
-    await createNodeJsLibp2p(
-      peerId,
-      {
-        connectToDiscv5Bootnodes: true,
-        discv5: {
-          enr: SignableENR.createV4(keypair).encodeTxt(),
-          bindAddrs: {
-            ip4: "/ip4/127.0.0.1/udp/0",
-          },
-          bootEnrs: enrWithTcp,
-        },
-        bootMultiaddrs,
-        localMultiaddrs: ["/ip4/127.0.0.1/tcp/0"],
-        targetPeers: defaultNetworkOptions.targetPeers,
-        maxPeers: defaultNetworkOptions.maxPeers,
-      },
-      {disablePeerDiscovery: true}
-    );
+    const bootMultiaddrs = await getDiscv5Multiaddrs(enrWithTcp);
     expect(bootMultiaddrs.length).to.be.equal(1);
     expect(bootMultiaddrs[0]).to.be.equal(
       "/ip4/18.141.3.138/tcp/9000/p2p/16Uiu2HAm5rokhpCBU7yBJHhMKXZ1xSVWwUcPMrzGKvU5Y7iBkmuK"
@@ -71,30 +48,10 @@ describe("createNodeJsLibp2p", () => {
 
   it("should not extract bootMultiaddrs from enr without tcp", async function () {
     this.timeout(0);
-    const peerId = await createSecp256k1PeerId();
     const enrWithoutTcp = [
       "enr:-Ku4QCFQW96tEDYPjtaueW3WIh1CB0cJnvw_ibx5qIFZGqfLLj-QajMX6XwVs2d4offuspwgH3NkIMpWtCjCytVdlywGh2F0dG5ldHOIEAIAAgABAUyEZXRoMpCi7FS9AQAAAAAiAQAAAAAAgmlkgnY0gmlwhFA4VK6Jc2VjcDI1NmsxoQNGH1sJJS86-0x9T7qQewz9Wn9zlp6bYxqqrR38JQ49yIN1ZHCCIyg",
     ];
-    const bootMultiaddrs: string[] = [];
-    const keypair = generateKeypair(KeypairType.Secp256k1);
-    await createNodeJsLibp2p(
-      peerId,
-      {
-        connectToDiscv5Bootnodes: true,
-        discv5: {
-          enr: SignableENR.createV4(keypair).encodeTxt(),
-          bindAddrs: {
-            ip4: "/ip4/127.0.0.1/udp/0",
-          },
-          bootEnrs: enrWithoutTcp,
-        },
-        bootMultiaddrs,
-        localMultiaddrs: ["/ip4/127.0.0.1/tcp/0"],
-        targetPeers: defaultNetworkOptions.targetPeers,
-        maxPeers: defaultNetworkOptions.maxPeers,
-      },
-      {disablePeerDiscovery: true}
-    );
+    const bootMultiaddrs = await getDiscv5Multiaddrs(enrWithoutTcp);
     expect(bootMultiaddrs.length).to.be.equal(0);
   });
 });
diff --git a/packages/beacon-node/test/utils/network.ts b/packages/beacon-node/test/utils/network.ts
@@ -12,7 +12,7 @@ import {
   NetworkInitModules,
   getReqRespHandlers,
 } from "../../src/network/index.js";
-import {createNodejsLibp2p, Libp2pOptions} from "../../src/network/nodejs/index.js";
+import {createNodeJsLibp2p} from "../../src/network/libp2p/index.js";
 import {Libp2p} from "../../src/network/interface.js";
 import {GetReqRespHandlerFn} from "../../src/network/reqresp/types.js";
 import {Eth1ForBlockProductionDisabled} from "../../src/eth1/index.js";
@@ -26,13 +26,9 @@ import {getStubbedBeaconDb} from "./mocks/db.js";
 import {ClockStatic} from "./clock.js";
 import {createCachedBeaconStateTest} from "./cachedBeaconState.js";
 
-export async function createNode(multiaddr: string, inPeerId?: PeerId, opts?: Partial<Libp2pOptions>): Promise<Libp2p> {
+export async function createNode(multiaddr: string, inPeerId?: PeerId): Promise<Libp2p> {
   const peerId = inPeerId || (await createSecp256k1PeerId());
-  return createNodejsLibp2p({
-    peerId,
-    addresses: {listen: [multiaddr]},
-    ...opts,
-  });
+  return createNodeJsLibp2p(peerId, {localMultiaddrs: [multiaddr]});
 }
 
 export async function createNetworkModules(

diff --git a/packages/cli/src/networks/index.ts b/packages/cli/src/networks/index.ts
@@ -1,5 +1,6 @@
 import fs from "node:fs";
 import got from "got";
+import {ENR} from "@chainsafe/discv5";
 import {SLOTS_PER_EPOCH} from "@lodestar/params";
 import {ApiError, getClient} from "@lodestar/api";
 import {getStateTypeFromBytes} from "@lodestar/beacon-node";
@@ -118,6 +119,13 @@ export function readBootnodes(bootnodesFilePath: string): string[] {
   const bootnodesFile = fs.readFileSync(bootnodesFilePath, "utf8");
 
   const bootnodes = parseBootnodesFile(bootnodesFile);
+  for (const enrStr of bootnodes) {
+    try {
+      ENR.decodeTxt(enrStr);
+    } catch (e) {
+      throw new Error(`Invalid ENR found in ${bootnodesFilePath}:\n    ${enrStr}`);
+    }
+  }
 
   if (bootnodes.length === 0) {
     throw new Error(`No bootnodes found on file ${bootnodesFilePath}`);

diff --git a/packages/cli/src/options/beaconNodeOptions/network.ts b/packages/cli/src/options/beaconNodeOptions/network.ts
@@ -1,4 +1,5 @@
 import {multiaddr} from "@multiformats/multiaddr";
+import {ENR} from "@chainsafe/discv5";
 import {defaultOptions, IBeaconNodeOptions} from "@lodestar/beacon-node";
 import {CliCommandOptions, YargsError} from "../../util/index.js";
 
@@ -104,6 +105,18 @@ export function parseArgs(args: NetworkArgs): IBeaconNodeOptions["network"] {
   }
   // Set discv5 opts to null to disable only if explicitly disabled
   const enableDiscv5 = args["discv5"] ?? true;
+
+  // TODO: Okay to set to empty array?
+  const bootEnrs = args["bootnodes"] ?? [];
+  // throw if user-provided enrs are invalid
+  for (const enrStr of bootEnrs) {
+    try {
+      ENR.decodeTxt(enrStr);
+    } catch (e) {
+      throw new YargsError(`Provided ENR in bootnodes is invalid:\n    ${enrStr}`);
+    }
+  }
+
   return {
     discv5: enableDiscv5
       ? {
@@ -112,8 +125,7 @@ export function parseArgs(args: NetworkArgs): IBeaconNodeOptions["network"] {
             ip4: bindMu as string,
             ip6: bindMu6,
           },
-          // TODO: Okay to set to empty array?
-          bootEnrs: args["bootnodes"] ?? [],
+          bootEnrs,
           // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any
           enr: undefined as any,
         }