Add --nat flag (#5399)

* Add --nat flag

* Fix unit test

packages/beacon-node/src/network/nodejs/util.ts

@@ -4,7 +4,6 @@ import {ENR, SignableENR} from "@chainsafe/discv5";
 import {Libp2p} from "../interface.js";
 import {Eth2PeerDataStore} from "../peers/datastore.js";
 import {defaultDiscv5Options, defaultNetworkOptions, NetworkOptions} from "../options.js";
-import {isLocalMultiAddr, clearMultiaddrUDP} from "../util.js";
 import {createNodejsLibp2p as _createNodejsLibp2p} from "./bundle.js";
 
 export type NodeJsLibp2pOpts = {
@@ -31,14 +30,8 @@ export async function createNodeJsLibp2p(
   const enr = networkOpts.discv5?.enr;
   const {peerStoreDir, disablePeerDiscovery} = nodeJsLibp2pOpts;
 
-  if (enr !== undefined && typeof enr !== "string") {
-    if (enr instanceof SignableENR) {
-      if (enr.getLocationMultiaddr("udp") && !isLocalMultiAddr(enr.getLocationMultiaddr("udp"))) {
-        clearMultiaddrUDP(enr);
-      }
-    } else {
-      throw Error("network.discv5.enr must be an instance of ENR");
-    }
+  if (enr !== undefined && typeof enr !== "string" && !(enr instanceof SignableENR)) {
+    throw Error("network.discv5.enr must be an instance of SignableENR");
   }
 
   let datastore: undefined | Eth2PeerDataStore = undefined;

packages/beacon-node/src/network/util.ts

@@ -1,66 +1,11 @@
-import os from "node:os";
 import type {PeerId} from "@libp2p/interface-peer-id";
-import type {Multiaddr} from "@multiformats/multiaddr";
 import type {Connection} from "@libp2p/interface-connection";
 import type {ConnectionManager} from "@libp2p/interface-connection-manager";
 import type {Components} from "libp2p/components.js";
 import type {DefaultConnectionManager} from "libp2p/connection-manager/index.js";
 import type {DefaultDialer} from "libp2p/connection-manager/dialer/index.js";
-import type {SignableENR} from "@chainsafe/discv5";
 import type {Libp2p} from "./interface.js";
 
-// peers
-
-/**
- * Check if multiaddr belongs to the local network interfaces.
- */
-export function isLocalMultiAddr(multiaddr: Multiaddr | undefined): boolean {
-  if (!multiaddr) return false;
-
-  const protoNames = multiaddr.protoNames();
-  if (protoNames.length !== 2 && protoNames[1] !== "udp") {
-    throw new Error("Invalid udp multiaddr");
-  }
-
-  const interfaces = os.networkInterfaces();
-  const tuples = multiaddr.tuples();
-  const family = tuples[0][0];
-  const isIPv4: boolean = family === 4;
-  const ip = tuples[0][1];
-
-  if (!ip) {
-    return false;
-  }
-
-  const ipStr = isIPv4
-    ? Array.from(ip).join(".")
-    : Array.from(Uint16Array.from(ip))
-        .map((n) => n.toString(16))
-        .join(":");
-
-  for (const networkInterfaces of Object.values(interfaces)) {
-    for (const networkInterface of networkInterfaces || []) {
-      // since node version 18, the netowrkinterface family returns 4 | 6 instead of ipv4 | ipv6,
-      // even though the documentation says otherwise.
-      // This might be a bug that would be corrected in future version, in the meantime
-      // the check using endsWith ensures things work in node version 18 and earlier
-      if (String(networkInterface.family).endsWith(String(family)) && networkInterface.address === ipStr) {
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
-
-export function clearMultiaddrUDP(enr: SignableENR): void {
-  // enr.multiaddrUDP = undefined in new version
-  enr.delete("ip");
-  enr.delete("udp");
-  enr.delete("ip6");
-  enr.delete("udp6");
-}
-
 export function prettyPrintPeerId(peerId: PeerId): string {
   const id = peerId.toString();
   return `${id.substr(0, 2)}...${id.substr(id.length - 6, id.length)}`;

packages/beacon-node/test/unit/network/util.test.ts

@@ -1,25 +1,12 @@
-import {multiaddr} from "@multiformats/multiaddr";
 import {expect} from "chai";
 import {createSecp256k1PeerId} from "@libp2p/peer-id-factory";
 import {config} from "@lodestar/config/default";
 import {ForkName} from "@lodestar/params";
 import {generateKeypair, KeypairType, SignableENR} from "@chainsafe/discv5";
 import {defaultNetworkOptions} from "../../../src/network/options.js";
-import {createNodeJsLibp2p, isLocalMultiAddr} from "../../../src/network/index.js";
+import {createNodeJsLibp2p} from "../../../src/network/index.js";
 import {getCurrentAndNextFork} from "../../../src/network/forks.js";
 
-describe("Test isLocalMultiAddr", () => {
-  it("should return true for 127.0.0.1", () => {
-    const multi0 = multiaddr("/ip4/127.0.0.1/udp/30303");
-    expect(isLocalMultiAddr(multi0)).to.equal(true);
-  });
-
-  it("should return false for 0.0.0.0", () => {
-    const multi0 = multiaddr("/ip4/0.0.0.0/udp/30303");
-    expect(isLocalMultiAddr(multi0)).to.equal(false);
-  });
-});
-
 describe("getCurrentAndNextFork", function () {
   const altairEpoch = config.forks.altair.epoch;
   afterEach(() => {

packages/cli/src/cmds/beacon/initPeerIdAndEnr.ts

@@ -1,15 +1,67 @@
 import fs from "node:fs";
 import path from "node:path";
+import os from "node:os";
 import {PeerId} from "@libp2p/interface-peer-id";
 import {createSecp256k1PeerId} from "@libp2p/peer-id-factory";
+import {Multiaddr} from "@multiformats/multiaddr";
 import {createKeypairFromPeerId, SignableENR} from "@chainsafe/discv5";
 import {Logger} from "@lodestar/utils";
 import {exportToJSON, readPeerId} from "../../config/index.js";
 import {writeFile600Perm} from "../../util/file.js";
 import {defaultP2pPort} from "../../options/beaconNodeOptions/network.js";
 import {BeaconArgs} from "./options.js";
 
-export function overwriteEnrWithCliArgs(enr: SignableENR, args: BeaconArgs): void {
+/**
+ * Check if multiaddr belongs to the local network interfaces.
+ */
+export function isLocalMultiAddr(multiaddr: Multiaddr | undefined): boolean {
+  if (!multiaddr) return false;
+
+  const protoNames = multiaddr.protoNames();
+  if (protoNames.length !== 2 && protoNames[1] !== "udp") {
+    throw new Error("Invalid udp multiaddr");
+  }
+
+  const interfaces = os.networkInterfaces();
+  const tuples = multiaddr.tuples();
+  const family = tuples[0][0];
+  const isIPv4: boolean = family === 4;
+  const ip = tuples[0][1];
+
+  if (!ip) {
+    return false;
+  }
+
+  const ipStr = isIPv4
+    ? Array.from(ip).join(".")
+    : Array.from(Uint16Array.from(ip))
+        .map((n) => n.toString(16))
+        .join(":");
+
+  for (const networkInterfaces of Object.values(interfaces)) {
+    for (const networkInterface of networkInterfaces || []) {
+      // since node version 18, the netowrkinterface family returns 4 | 6 instead of ipv4 | ipv6,
+      // even though the documentation says otherwise.
+      // This might be a bug that would be corrected in future version, in the meantime
+      // the check using endsWith ensures things work in node version 18 and earlier
+      if (String(networkInterface.family).endsWith(String(family)) && networkInterface.address === ipStr) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
+export function clearMultiaddrUDP(enr: SignableENR): void {
+  // enr.multiaddrUDP = undefined in new version
+  enr.delete("ip");
+  enr.delete("udp");
+  enr.delete("ip6");
+  enr.delete("udp6");
+}
+
+export function overwriteEnrWithCliArgs(enr: SignableENR, args: BeaconArgs, logger: Logger): void {
   // TODO: Not sure if we should propagate port/defaultP2pPort options to the ENR
   enr.tcp = args["enr.tcp"] ?? args.port ?? defaultP2pPort;
   const udpPort = args["enr.udp"] ?? args.discoveryPort ?? args.port ?? defaultP2pPort;
@@ -18,6 +70,23 @@ export function overwriteEnrWithCliArgs(enr: SignableENR, args: BeaconArgs): voi
   if (args["enr.ip6"] != null) enr.ip6 = args["enr.ip6"];
   if (args["enr.tcp6"] != null) enr.tcp6 = args["enr.tcp6"];
   if (args["enr.udp6"] != null) enr.udp6 = args["enr.udp6"];
+
+  const udpMultiaddr = enr.getLocationMultiaddr("udp");
+  if (udpMultiaddr) {
+    const isLocal = isLocalMultiAddr(udpMultiaddr);
+    if (args.nat) {
+      if (isLocal) {
+        logger.warn("--nat flag is set with no purpose");
+      }
+    } else {
+      if (!isLocal) {
+        logger.warn(
+          "Configured ENR IP address is not local, clearing ENR IP and UDP. Set the --nat flag to prevent this"
+        );
+        clearMultiaddrUDP(enr);
+      }
+    }
+  }
 }
 
 /**
@@ -71,14 +140,14 @@ export async function initPeerIdAndEnr(
     const enrFile = path.join(beaconDir, "enr");
     const peerIdFile = path.join(beaconDir, "peer-id.json");
     const {peerId, enr} = await readPersistedPeerIdAndENR(peerIdFile, enrFile);
-    overwriteEnrWithCliArgs(enr, args);
+    overwriteEnrWithCliArgs(enr, args, logger);
     // Re-persist peer-id and enr
     writeFile600Perm(peerIdFile, exportToJSON(peerId));
     writeFile600Perm(enrFile, enr.encodeTxt());
     return {peerId, enr};
   } else {
     const {peerId, enr} = await newPeerIdAndENR();
-    overwriteEnrWithCliArgs(enr, args);
+    overwriteEnrWithCliArgs(enr, args, logger);
     return {peerId, enr};
   }
 }

packages/cli/src/cmds/beacon/options.ts

@@ -106,6 +106,7 @@ type ENRArgs = {
   "enr.udp"?: number;
   "enr.tcp6"?: number;
   "enr.udp6"?: number;
+  nat?: boolean;
 };
 
 const enrOptions: Record<string, Options> = {
@@ -139,6 +140,11 @@ const enrOptions: Record<string, Options> = {
     type: "number",
     group: "enr",
   },
+  nat: {
+    type: "boolean",
+    description: "Allow configuration of non-local addresses",
+    group: "enr",
+  },
 };
 
 export type DebugArgs = {attachToGlobalThis: boolean};

packages/cli/test/unit/cmds/beacon.test.ts

@@ -2,12 +2,13 @@ import path from "node:path";
 import fs from "node:fs";
 import {expect} from "chai";
 import {createFromJSON, createSecp256k1PeerId} from "@libp2p/peer-id-factory";
+import {multiaddr} from "@multiformats/multiaddr";
 import {chainConfig} from "@lodestar/config/default";
 import {chainConfigToJson} from "@lodestar/config";
 import {createKeypairFromPeerId, SignableENR} from "@chainsafe/discv5";
 import {exportToJSON} from "../../../src/config/peerId.js";
 import {beaconHandlerInit} from "../../../src/cmds/beacon/handler.js";
-import {initPeerIdAndEnr} from "../../../src/cmds/beacon/initPeerIdAndEnr.js";
+import {initPeerIdAndEnr, isLocalMultiAddr} from "../../../src/cmds/beacon/initPeerIdAndEnr.js";
 import {BeaconArgs} from "../../../src/cmds/beacon/options.js";
 import {GlobalArgs} from "../../../src/options/globalOptions.js";
 import {testFilesDir, testLogger} from "../../utils.js";
@@ -39,6 +40,7 @@ describe("cmds / beacon / args handler", () => {
       listenAddress: "0.0.0.0",
       "enr.ip": enrIp,
       "enr.tcp": enrTcp,
+      nat: true,
     });
 
     const enr = options.network.discv5?.enr as SignableENR;
@@ -96,6 +98,18 @@ describe("cmds / beacon / args handler", () => {
   });
 });
 
+describe("Test isLocalMultiAddr", () => {
+  it("should return true for 127.0.0.1", () => {
+    const multi0 = multiaddr("/ip4/127.0.0.1/udp/30303");
+    expect(isLocalMultiAddr(multi0)).to.equal(true);
+  });
+
+  it("should return false for 0.0.0.0", () => {
+    const multi0 = multiaddr("/ip4/0.0.0.0/udp/30303");
+    expect(isLocalMultiAddr(multi0)).to.equal(false);
+  });
+});
+
 describe("initPeerIdAndEnr", () => {
   it("should not reuse peer id, persistNetworkIdentity=false", async () => {
     const {peerId: peerId1} = await initPeerIdAndEnr(