From 487aef9445e4e83531d541054ea0b60ccf11d7e3 Mon Sep 17 00:00:00 2001 From: tuyennhv Date: Mon, 31 Jul 2023 14:17:59 +0700 Subject: [PATCH] fix: make SLOTS_TO_SUBSCRIBE_IN_ADVANCE as hidden cli param (#5819) --- packages/beacon-node/src/network/options.ts | 2 ++ .../src/network/subnets/attnetsService.ts | 19 ++++++++----------- .../src/network/subnets/dllAttnetsService.ts | 15 ++++++--------- .../src/network/subnets/interface.ts | 1 + .../network/subnets/attnetsService.test.ts | 1 + .../network/subnets/dllAttnetsService.test.ts | 4 +++- .../src/options/beaconNodeOptions/network.ts | 11 +++++++++++ .../unit/options/beaconNodeOptions.test.ts | 2 ++ 8 files changed, 34 insertions(+), 21 deletions(-) diff --git a/packages/beacon-node/src/network/options.ts b/packages/beacon-node/src/network/options.ts index 80ffa595622d..b25645e2b945 100644 --- a/packages/beacon-node/src/network/options.ts +++ b/packages/beacon-node/src/network/options.ts @@ -36,4 +36,6 @@ export const defaultNetworkOptions: NetworkOptions = { gossipsubDHigh: 9, // TODO set to false in order to release 1.9.0 in a timely manner useWorker: false, + // subscribe 2 slots before aggregator dutied slot to get stable mesh peers as monitored on goerli + slotsToSubscribeBeforeAggregatorDuty: 2, }; diff --git a/packages/beacon-node/src/network/subnets/attnetsService.ts b/packages/beacon-node/src/network/subnets/attnetsService.ts index f49fd2ed0e88..30e72c932a30 100644 --- a/packages/beacon-node/src/network/subnets/attnetsService.ts +++ b/packages/beacon-node/src/network/subnets/attnetsService.ts @@ -39,9 +39,6 @@ enum SubnetSource { random = "random", } -/** As monitored on goerli, we only need to subscribe 2 slots before aggregator dutied slot to get stable mesh peers */ -const SLOTS_TO_SUBSCRIBE_IN_ADVANCE = 2; - /** * Manage random (long lived) subnets and committee (short lived) subnets. * - PeerManager uses attnetsService to know which peers are requried for duties @@ -82,18 +79,18 @@ export class AttnetsService implements IAttnetsService { private readonly metadata: MetadataController, private readonly logger: Logger, private readonly metrics: NetworkCoreMetrics | null, - private readonly opts?: SubnetsServiceOpts & SubnetsServiceTestOpts + private readonly opts: SubnetsServiceOpts & SubnetsServiceTestOpts ) { // if subscribeAllSubnets, we act like we have >= ATTESTATION_SUBNET_COUNT validators connecting to this node // so that we have enough subnet topic peers, see https://github.com/ChainSafe/lodestar/issues/4921 - if (this.opts?.subscribeAllSubnets) { + if (this.opts.subscribeAllSubnets) { for (let subnet = 0; subnet < ATTESTATION_SUBNET_COUNT; subnet++) { this.committeeSubnets.request({subnet, toSlot: Infinity}); } } - this.randBetweenFn = this.opts?.randBetweenFn ?? randBetween; - this.shuffleFn = this.opts?.shuffleFn ?? shuffle; + this.randBetweenFn = this.opts.randBetweenFn ?? randBetween; + this.shuffleFn = this.opts.shuffleFn ?? shuffle; if (metrics) { metrics.attnetsService.subscriptionsRandom.addCollect(() => this.onScrapeLodestarMetrics(metrics)); } @@ -160,7 +157,7 @@ export class AttnetsService implements IAttnetsService { unsubscribeSubnetsFromPrevFork(prevFork: ForkName): void { this.logger.info("Unsuscribing to random attnets from prev fork", {prevFork}); for (let subnet = 0; subnet < ATTESTATION_SUBNET_COUNT; subnet++) { - if (!this.opts?.subscribeAllSubnets) { + if (!this.opts.subscribeAllSubnets) { this.gossip.unsubscribeTopic({type: gossipType, fork: prevFork, subnet}); } } @@ -168,13 +165,13 @@ export class AttnetsService implements IAttnetsService { /** * Run per slot. - * - Subscribe to gossip subnets `${SLOTS_TO_SUBSCRIBE_IN_ADVANCE}` slots in advance + * - Subscribe to gossip subnets 2 slots in advance * - Unsubscribe from expired subnets */ private onSlot = (clockSlot: Slot): void => { try { for (const [dutiedSlot, subnets] of this.aggregatorSlotSubnet.entries()) { - if (dutiedSlot === clockSlot + SLOTS_TO_SUBSCRIBE_IN_ADVANCE) { + if (dutiedSlot === clockSlot + this.opts.slotsToSubscribeBeforeAggregatorDuty) { // Trigger gossip subscription first, in batch if (subnets.size > 0) { this.subscribeToSubnets(Array.from(subnets), SubnetSource.committee); @@ -353,7 +350,7 @@ export class AttnetsService implements IAttnetsService { /** Trigger a gossip un-subscrition only if no-one is still subscribed */ private unsubscribeSubnets(subnets: number[], slot: Slot, src: SubnetSource): void { // No need to unsubscribeTopic(). Return early to prevent repetitive extra work - if (this.opts?.subscribeAllSubnets) return; + if (this.opts.subscribeAllSubnets) return; const forks = getActiveForks(this.config, this.clock.currentEpoch); for (const subnet of subnets) { diff --git a/packages/beacon-node/src/network/subnets/dllAttnetsService.ts b/packages/beacon-node/src/network/subnets/dllAttnetsService.ts index 08dd9e91da15..ec996d9b9394 100644 --- a/packages/beacon-node/src/network/subnets/dllAttnetsService.ts +++ b/packages/beacon-node/src/network/subnets/dllAttnetsService.ts @@ -24,9 +24,6 @@ enum SubnetSource { longLived = "long_lived", } -/** As monitored on goerli, we only need to subscribe 2 slots before aggregator dutied slot to get stable mesh peers */ -const SLOTS_TO_SUBSCRIBE_IN_ADVANCE = 2; - /** * Manage deleterministic long lived (DLL) subnets and short lived subnets. * - PeerManager uses attnetsService to know which peers are required for duties and long lived subscriptions @@ -58,11 +55,11 @@ export class DLLAttnetsService implements IAttnetsService { private readonly logger: Logger, private readonly metrics: NetworkCoreMetrics | null, private readonly nodeId: NodeId | null, - private readonly opts?: SubnetsServiceOpts + private readonly opts: SubnetsServiceOpts ) { // if subscribeAllSubnets, we act like we have >= ATTESTATION_SUBNET_COUNT validators connecting to this node // so that we have enough subnet topic peers, see https://github.com/ChainSafe/lodestar/issues/4921 - if (this.opts?.subscribeAllSubnets) { + if (this.opts.subscribeAllSubnets) { for (let subnet = 0; subnet < ATTESTATION_SUBNET_COUNT; subnet++) { this.committeeSubnets.request({subnet, toSlot: Infinity}); } @@ -147,7 +144,7 @@ export class DLLAttnetsService implements IAttnetsService { unsubscribeSubnetsFromPrevFork(prevFork: ForkName): void { this.logger.info("Unsuscribing to long lived attnets from prev fork", {prevFork}); for (let subnet = 0; subnet < ATTESTATION_SUBNET_COUNT; subnet++) { - if (!this.opts?.subscribeAllSubnets) { + if (!this.opts.subscribeAllSubnets) { this.gossip.unsubscribeTopic({type: gossipType, fork: prevFork, subnet}); } } @@ -155,13 +152,13 @@ export class DLLAttnetsService implements IAttnetsService { /** * Run per slot. - * - Subscribe to gossip subnets `${SLOTS_TO_SUBSCRIBE_IN_ADVANCE}` slots in advance + * - Subscribe to gossip subnets 2 slots in advance * - Unsubscribe from expired subnets */ private onSlot = (clockSlot: Slot): void => { try { for (const [dutiedSlot, subnets] of this.aggregatorSlotSubnet.entries()) { - if (dutiedSlot === clockSlot + SLOTS_TO_SUBSCRIBE_IN_ADVANCE) { + if (dutiedSlot === clockSlot + this.opts.slotsToSubscribeBeforeAggregatorDuty) { // Trigger gossip subscription first, in batch if (subnets.size > 0) { this.subscribeToSubnets(Array.from(subnets), SubnetSource.committee); @@ -290,7 +287,7 @@ export class DLLAttnetsService implements IAttnetsService { **/ private unsubscribeSubnets(subnets: number[], slot: Slot, src: SubnetSource): void { // No need to unsubscribeTopic(). Return early to prevent repetitive extra work - if (this.opts?.subscribeAllSubnets) return; + if (this.opts.subscribeAllSubnets) return; const forks = getActiveForks(this.config, this.clock.currentEpoch); for (const subnet of subnets) { diff --git a/packages/beacon-node/src/network/subnets/interface.ts b/packages/beacon-node/src/network/subnets/interface.ts index 9f449427377f..e21d9ebb8d8c 100644 --- a/packages/beacon-node/src/network/subnets/interface.ts +++ b/packages/beacon-node/src/network/subnets/interface.ts @@ -29,6 +29,7 @@ export type ShuffleFn = (arr: T[]) => T[]; export type SubnetsServiceOpts = { deterministicLongLivedAttnets?: boolean; subscribeAllSubnets?: boolean; + slotsToSubscribeBeforeAggregatorDuty: number; }; export type SubnetsServiceTestOpts = { diff --git a/packages/beacon-node/test/unit/network/subnets/attnetsService.test.ts b/packages/beacon-node/test/unit/network/subnets/attnetsService.test.ts index 5b7354187a28..16b80549f0c0 100644 --- a/packages/beacon-node/test/unit/network/subnets/attnetsService.test.ts +++ b/packages/beacon-node/test/unit/network/subnets/attnetsService.test.ts @@ -70,6 +70,7 @@ describe("AttnetsService", function () { getCurrentSlot(config, Math.floor(Date.now() / 1000)); metadata = new MetadataController({}, {config, onSetValue: () => null}); service = new AttnetsService(config, clock, gossipStub, metadata, logger, null, { + slotsToSubscribeBeforeAggregatorDuty: 2, randBetweenFn, shuffleFn: shuffleFn as ShuffleFn, }); diff --git a/packages/beacon-node/test/unit/network/subnets/dllAttnetsService.test.ts b/packages/beacon-node/test/unit/network/subnets/dllAttnetsService.test.ts index 9b402396843d..c98563c9d3e9 100644 --- a/packages/beacon-node/test/unit/network/subnets/dllAttnetsService.test.ts +++ b/packages/beacon-node/test/unit/network/subnets/dllAttnetsService.test.ts @@ -48,7 +48,9 @@ describe("DLLAttnetsService", () => { // load getCurrentSlot first, vscode not able to debug without this getCurrentSlot(config, Math.floor(Date.now() / 1000)); metadata = new MetadataController({}, {config, onSetValue: () => null}); - service = new DLLAttnetsService(config, clock, gossipStub, metadata, logger, null, nodeId); + service = new DLLAttnetsService(config, clock, gossipStub, metadata, logger, null, nodeId, { + slotsToSubscribeBeforeAggregatorDuty: 2, + }); }); afterEach(() => { diff --git a/packages/cli/src/options/beaconNodeOptions/network.ts b/packages/cli/src/options/beaconNodeOptions/network.ts index be71619f4c8f..b8875f30aca6 100644 --- a/packages/cli/src/options/beaconNodeOptions/network.ts +++ b/packages/cli/src/options/beaconNodeOptions/network.ts @@ -19,6 +19,7 @@ export type NetworkArgs = { targetPeers?: number; deterministicLongLivedAttnets?: boolean; subscribeAllSubnets?: boolean; + slotsToSubscribeBeforeAggregatorDuty?: number; disablePeerScoring?: boolean; mdns?: boolean; "network.maxPeers"?: number; @@ -135,6 +136,8 @@ export function parseArgs(args: NetworkArgs): IBeaconNodeOptions["network"] { localMultiaddrs: [localMu, localMu6].filter(Boolean) as string[], deterministicLongLivedAttnets: args["deterministicLongLivedAttnets"], subscribeAllSubnets: args["subscribeAllSubnets"], + slotsToSubscribeBeforeAggregatorDuty: + args["slotsToSubscribeBeforeAggregatorDuty"] ?? defaultOptions.network.slotsToSubscribeBeforeAggregatorDuty, disablePeerScoring: args["disablePeerScoring"], connectToDiscv5Bootnodes: args["network.connectToDiscv5Bootnodes"], discv5FirstQueryDelayMs: args["network.discv5FirstQueryDelayMs"], @@ -235,6 +238,14 @@ export const options: CliCommandOptions = { group: "network", }, + slotsToSubscribeBeforeAggregatorDuty: { + hidden: true, + type: "number", + description: "Number of slots before an aggregator duty to subscribe to subnets", + defaultDescription: String(defaultOptions.network.slotsToSubscribeBeforeAggregatorDuty), + group: "network", + }, + disablePeerScoring: { type: "boolean", description: "Disable peer scoring, used for testing on devnets", diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index 40f820bac57e..31f11cf4a79d 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -75,6 +75,7 @@ describe("options / beaconNodeOptions", () => { targetPeers: 25, deterministicLongLivedAttnets: true, subscribeAllSubnets: true, + slotsToSubscribeBeforeAggregatorDuty: 1, disablePeerScoring: true, mdns: false, "network.maxPeers": 30, @@ -180,6 +181,7 @@ describe("options / beaconNodeOptions", () => { localMultiaddrs: ["/ip4/127.0.0.1/tcp/9001"], deterministicLongLivedAttnets: true, subscribeAllSubnets: true, + slotsToSubscribeBeforeAggregatorDuty: 1, disablePeerScoring: true, connectToDiscv5Bootnodes: true, discv5FirstQueryDelayMs: 1000,