diff --git a/packages/cli/src/cmds/beacon/initPeerIdAndEnr.ts b/packages/cli/src/cmds/beacon/initPeerIdAndEnr.ts index 1bc6f8e5fd00..37c5d766d7a3 100644 --- a/packages/cli/src/cmds/beacon/initPeerIdAndEnr.ts +++ b/packages/cli/src/cmds/beacon/initPeerIdAndEnr.ts @@ -53,14 +53,35 @@ export function isLocalMultiAddr(multiaddr: Multiaddr | undefined): boolean { return false; } -export function overwriteEnrWithCliArgs(enr: SignableENR, args: BeaconArgs, logger: Logger): void { +/** + * Only update the enr if the value has changed + */ +function maybeUpdateEnr( + enr: SignableENR, + key: T, + value: SignableENR[T] | undefined +): void { + if (enr[key] !== value) { + enr[key] = value; + } +} + +export function overwriteEnrWithCliArgs( + enr: SignableENR, + args: BeaconArgs, + logger: Logger, + opts?: {newEnr?: boolean; bootnode?: boolean} +): void { + const preSeq = enr.seq; const {port, discoveryPort, port6, discoveryPort6} = parseListenArgs(args); - enr.ip = args["enr.ip"] ?? enr.ip; - enr.tcp = args["enr.tcp"] ?? port ?? enr.tcp; - enr.udp = args["enr.udp"] ?? discoveryPort ?? enr.udp; - enr.ip6 = args["enr.ip6"] ?? enr.ip6; - enr.tcp6 = args["enr.tcp6"] ?? port6 ?? enr.tcp6; - enr.udp6 = args["enr.udp6"] ?? discoveryPort6 ?? enr.udp6; + maybeUpdateEnr(enr, "ip", args["enr.ip"] ?? enr.ip); + maybeUpdateEnr(enr, "ip6", args["enr.ip6"] ?? enr.ip6); + maybeUpdateEnr(enr, "udp", args["enr.udp"] ?? discoveryPort ?? enr.udp); + maybeUpdateEnr(enr, "udp6", args["enr.udp6"] ?? discoveryPort6 ?? enr.udp6); + if (!opts?.bootnode) { + maybeUpdateEnr(enr, "tcp", args["enr.tcp"] ?? port ?? enr.tcp); + maybeUpdateEnr(enr, "tcp6", args["enr.tcp6"] ?? port6 ?? enr.tcp6); + } function testMultiaddrForLocal(mu: Multiaddr, ip4: boolean): void { const isLocal = isLocalMultiAddr(mu); @@ -93,6 +114,19 @@ export function overwriteEnrWithCliArgs(enr: SignableENR, args: BeaconArgs, logg if (udpMultiaddr6) { testMultiaddrForLocal(udpMultiaddr6, false); } + + if (enr.seq !== preSeq) { + // If the enr is newly created, its sequence number can be set to 1 + // It's especially clean for fully configured bootnodes whose enrs never change + // Otherwise, we can increment the sequence number as little as possible + if (opts?.newEnr) { + enr.seq = BigInt(1); + } else { + enr.seq = preSeq + BigInt(1); + } + // invalidate cached signature + delete enr["_signature"]; + } } /** @@ -101,7 +135,8 @@ export function overwriteEnrWithCliArgs(enr: SignableENR, args: BeaconArgs, logg export async function initPeerIdAndEnr( args: BeaconArgs, beaconDir: string, - logger: Logger + logger: Logger, + bootnode?: boolean ): Promise<{peerId: PeerId; enr: SignableENR}> { const {persistNetworkIdentity} = args; @@ -114,7 +149,7 @@ export async function initPeerIdAndEnr( const readPersistedPeerIdAndENR = async ( peerIdFile: string, enrFile: string - ): Promise<{peerId: PeerId; enr: SignableENR}> => { + ): Promise<{peerId: PeerId; enr: SignableENR; newEnr: boolean}> => { let peerId: PeerId; let enr: SignableENR; @@ -123,7 +158,7 @@ export async function initPeerIdAndEnr( peerId = await readPeerId(peerIdFile); } catch (e) { logger.warn("Unable to read peerIdFile, creating a new peer id"); - return newPeerIdAndENR(); + return {...(await newPeerIdAndENR()), newEnr: true}; } // attempt to read stored enr try { @@ -131,29 +166,29 @@ export async function initPeerIdAndEnr( } catch (e) { logger.warn("Unable to decode stored local ENR, creating a new ENR"); enr = SignableENR.createV4(createKeypairFromPeerId(peerId)); - return {peerId, enr}; + return {peerId, enr, newEnr: true}; } // check stored peer id against stored enr if (!peerId.equals(await enr.peerId())) { logger.warn("Stored local ENR doesn't match peerIdFile, creating a new ENR"); enr = SignableENR.createV4(createKeypairFromPeerId(peerId)); - return {peerId, enr}; + return {peerId, enr, newEnr: true}; } - return {peerId, enr}; + return {peerId, enr, newEnr: false}; }; if (persistNetworkIdentity) { const enrFile = path.join(beaconDir, "enr"); const peerIdFile = path.join(beaconDir, "peer-id.json"); - const {peerId, enr} = await readPersistedPeerIdAndENR(peerIdFile, enrFile); - overwriteEnrWithCliArgs(enr, args, logger); + const {peerId, enr, newEnr} = await readPersistedPeerIdAndENR(peerIdFile, enrFile); + overwriteEnrWithCliArgs(enr, args, logger, {newEnr, bootnode}); // 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, logger); + overwriteEnrWithCliArgs(enr, args, logger, {newEnr: true, bootnode}); return {peerId, enr}; } } diff --git a/packages/cli/src/cmds/bootnode/handler.ts b/packages/cli/src/cmds/bootnode/handler.ts index 0623d67bda59..be639eb1bf4b 100644 --- a/packages/cli/src/cmds/bootnode/handler.ts +++ b/packages/cli/src/cmds/bootnode/handler.ts @@ -180,7 +180,7 @@ export async function bootnodeHandlerInit(args: BootnodeArgs & GlobalArgs) { ); const logger = initLogger(args, beaconPaths.dataDir, config, "bootnode.log"); - const {peerId, enr} = await initPeerIdAndEnr(args as unknown as BeaconArgs, bootnodeDir, logger); + const {peerId, enr} = await initPeerIdAndEnr(args as unknown as BeaconArgs, bootnodeDir, logger, true); return {discv5Args, metricsArgs, bootnodeDir, network, version, commit, peerId, enr, logger}; } diff --git a/packages/cli/test/unit/cmds/initPeerIdAndEnr.test.ts b/packages/cli/test/unit/cmds/initPeerIdAndEnr.test.ts new file mode 100644 index 000000000000..4bdfedf64b95 --- /dev/null +++ b/packages/cli/test/unit/cmds/initPeerIdAndEnr.test.ts @@ -0,0 +1,52 @@ +import fs from "node:fs"; +import tmp from "tmp"; +import {expect} from "chai"; +import {initPeerIdAndEnr} from "../../../src/cmds/beacon/initPeerIdAndEnr.js"; +import {BeaconArgs} from "../../../src/cmds/beacon/options.js"; +import {testLogger} from "../../utils.js"; + +describe("initPeerIdAndEnr", () => { + let tmpDir: tmp.DirResult; + + beforeEach(() => { + tmpDir = tmp.dirSync(); + }); + + afterEach(() => { + fs.rmSync(tmpDir.name, {recursive: true}); + }); + + it("first time should create a new enr and peer id", async () => { + const {enr, peerId} = await initPeerIdAndEnr( + {persistNetworkIdentity: true} as unknown as BeaconArgs, + tmpDir.name, + testLogger(), + true + ); + expect((await enr.peerId()).toString(), "enr peer id doesn't equal the returned peer id").to.equal( + peerId.toString() + ); + expect(enr.seq).to.equal(BigInt(1)); + expect(enr.tcp).to.equal(undefined); + expect(enr.tcp6).to.equal(undefined); + }); + + it("second time should use ths existing enr and peer id", async () => { + const run1 = await initPeerIdAndEnr( + {persistNetworkIdentity: true} as unknown as BeaconArgs, + tmpDir.name, + testLogger(), + true + ); + + const run2 = await initPeerIdAndEnr( + {persistNetworkIdentity: true} as unknown as BeaconArgs, + tmpDir.name, + testLogger(), + true + ); + + expect(run1.peerId.toString()).to.equal(run2.peerId.toString()); + expect(run1.enr.encodeTxt()).to.equal(run2.enr.encodeTxt()); + }); +});