From 79d369857a026443596f26d68176f4d9527e3300 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 10 Mar 2023 15:27:18 +0700 Subject: [PATCH 1/6] Limit preaggregating attestations --- packages/beacon-node/src/chain/chain.ts | 8 ++++++-- .../src/chain/opPools/attestationPool.ts | 17 +++++++++++++---- .../chain/opPools/syncCommitteeMessagePool.ts | 10 +++++++++- packages/beacon-node/src/chain/opPools/types.ts | 5 +++++ .../unit/chain/opPools/syncCommittee.test.ts | 13 ++++++++++++- .../beacon-node/test/utils/mocks/chain/chain.ts | 6 ++++-- 6 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index 99ca83af5e08..9d138def9f9d 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -96,9 +96,9 @@ export class BeaconChain implements IBeaconChain { readonly reprocessController: ReprocessController; // Ops pool - readonly attestationPool = new AttestationPool(); + readonly attestationPool: AttestationPool; readonly aggregatedAttestationPool = new AggregatedAttestationPool(); - readonly syncCommitteeMessagePool = new SyncCommitteeMessagePool(); + readonly syncCommitteeMessagePool: SyncCommitteeMessagePool; readonly syncContributionAndProofPool = new SyncContributionAndProofPool(); readonly opPool = new OpPool(); @@ -185,6 +185,10 @@ export class BeaconChain implements IBeaconChain { if (!clock) clock = new LocalClock({config, emitter, genesisTime: this.genesisTime, signal}); + const preAggregateCutOffTime = (2 / 3) * this.config.SECONDS_PER_SLOT; + this.attestationPool = new AttestationPool(clock, preAggregateCutOffTime); + this.syncCommitteeMessagePool = new SyncCommitteeMessagePool(clock, preAggregateCutOffTime); + this.seenAggregatedAttestations = new SeenAggregatedAttestations(metrics); this.seenContributionAndProof = new SeenContributionAndProof(metrics); diff --git a/packages/beacon-node/src/chain/opPools/attestationPool.ts b/packages/beacon-node/src/chain/opPools/attestationPool.ts index 075a2774e769..475f6265af93 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -3,6 +3,7 @@ import {PointFormat, Signature} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; import {BitArray, toHexString} from "@chainsafe/ssz"; import {MapDef} from "@lodestar/utils"; +import {BeaconClock} from "../clock/interface.js"; import {InsertOutcome, OpPoolError, OpPoolErrorCode} from "./types.js"; import {pruneBySlot, signatureFromBytesNoCheck} from "./utils.js"; @@ -60,6 +61,8 @@ export class AttestationPool { ); private lowestPermissibleSlot = 0; + constructor(private readonly clock: BeaconClock, private readonly cutOffSecFromSlot: number) {} + /** Returns current count of pre-aggregated attestations with unique data */ getAttestationCount(): number { let attestationCount = 0; @@ -77,7 +80,8 @@ export class AttestationPool { * `SignedAggregateAndProof`. * * If the attestation is too old (low slot) to be included in the pool it is simply dropped - * and no error is returned. + * and no error is returned. Also if it's at clock slot but come to the pool later than 2/3 + * of slot time, it's dropped too since it's not helpful for the validator anymore * * Expects the attestation to be fully validated: * - Valid signature @@ -94,6 +98,11 @@ export class AttestationPool { return InsertOutcome.Old; } + // Reject attestations in the current slot but come to this pool very late + if (this.clock.secFromSlot(slot) > this.cutOffSecFromSlot) { + return InsertOutcome.Late; + } + // Limit object per slot const aggregateByRoot = this.attestationByRootBySlot.getOrDefault(slot); if (aggregateByRoot.size >= MAX_ATTESTATIONS_PER_SLOT) { @@ -130,12 +139,12 @@ export class AttestationPool { } /** - * Removes any attestations with a slot lower than `current_slot` and bars any future - * attestations with a slot lower than `current_slot - SLOTS_RETAINED`. + * Removes any attestations with a slot lower than `current_slot - SLOTS_RETAINED`. + * Not intested in attestations in old slots, we only preaggregate attestations for the current slot. */ prune(clockSlot: Slot): void { pruneBySlot(this.attestationByRootBySlot, clockSlot, SLOTS_RETAINED); - this.lowestPermissibleSlot = Math.max(clockSlot - SLOTS_RETAINED, 0); + this.lowestPermissibleSlot = clockSlot; } /** diff --git a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts index c804c267f012..1f18a0dea124 100644 --- a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts +++ b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts @@ -4,6 +4,7 @@ import {SYNC_COMMITTEE_SIZE, SYNC_COMMITTEE_SUBNET_COUNT} from "@lodestar/params import {altair, Root, Slot, SubcommitteeIndex} from "@lodestar/types"; import {BitArray, toHexString} from "@chainsafe/ssz"; import {MapDef} from "@lodestar/utils"; +import {BeaconClock} from "../clock/interface.js"; import {InsertOutcome, OpPoolError, OpPoolErrorCode} from "./types.js"; import {pruneBySlot, signatureFromBytesNoCheck} from "./utils.js"; @@ -44,6 +45,8 @@ export class SyncCommitteeMessagePool { >(() => new MapDef>(() => new Map())); private lowestPermissibleSlot = 0; + constructor(private readonly clock: BeaconClock, private readonly cutOffSecFromSlot: number) {} + /** Returns current count of unique ContributionFast by block root and subnet */ get size(): number { let count = 0; @@ -66,6 +69,11 @@ export class SyncCommitteeMessagePool { throw new OpPoolError({code: OpPoolErrorCode.SLOT_TOO_LOW, slot, lowestPermissibleSlot}); } + // validator gets SyncCommitteeContribution at 2/3 of slot, it's no use to preaggregate later than that time + if (this.clock.secFromSlot(slot) > this.cutOffSecFromSlot) { + throw new OpPoolError({code: OpPoolErrorCode.LATE_MESSAGE, slot}); + } + // Limit object per slot const contributionsByRoot = this.contributionsByRootBySubnetBySlot.getOrDefault(slot).getOrDefault(subnet); if (contributionsByRoot.size >= MAX_ITEMS_PER_SLOT) { @@ -106,7 +114,7 @@ export class SyncCommitteeMessagePool { */ prune(clockSlot: Slot): void { pruneBySlot(this.contributionsByRootBySubnetBySlot, clockSlot, SLOTS_RETAINED); - this.lowestPermissibleSlot = Math.max(clockSlot - SLOTS_RETAINED, 0); + this.lowestPermissibleSlot = Math.max(clockSlot, 0); } } diff --git a/packages/beacon-node/src/chain/opPools/types.ts b/packages/beacon-node/src/chain/opPools/types.ts index 393f821bfa11..54a3b60f9687 100644 --- a/packages/beacon-node/src/chain/opPools/types.ts +++ b/packages/beacon-node/src/chain/opPools/types.ts @@ -11,6 +11,8 @@ export enum InsertOutcome { AlreadyKnown = "AlreadyKnown", /** Not existing in the pool but it's too old to add. No changes were made. */ Old = "Old", + /** Attestation comes to the pool at > 2/3 of slot. No changes were made */ + Late = "Late", /** The data is know, and the new participants have been added to the aggregated signature */ Aggregated = "Aggregated", /** The data is not better than the existing data*/ @@ -20,12 +22,15 @@ export enum InsertOutcome { export enum OpPoolErrorCode { /** The given object slot was too low to be stored. No changes were made. */ SLOT_TOO_LOW = "OP_POOL_ERROR_SLOT_TOO_LOW", + /** Good slot but it comes to the pool at late time */ + LATE_MESSAGE = "OP_POOL_ERROR_LATE_MESSAGE", /** Reached max number of unique objects per slot. This is a DoS protection function. */ REACHED_MAX_PER_SLOT = "OP_POOL_ERROR_REACHED_MAX_PER_SLOT", } export type OpPoolErrorType = | {code: OpPoolErrorCode.SLOT_TOO_LOW; slot: Slot; lowestPermissibleSlot: Slot} + | {code: OpPoolErrorCode.LATE_MESSAGE; slot: Slot} | {code: OpPoolErrorCode.REACHED_MAX_PER_SLOT}; export class OpPoolError extends LodestarError {} diff --git a/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts b/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts index 1582208de897..771c85d15c3a 100644 --- a/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts @@ -1,16 +1,21 @@ import {expect} from "chai"; +import sinon, {SinonStubbedInstance} from "sinon"; import bls from "@chainsafe/bls"; import {altair} from "@lodestar/types"; import {toHexString} from "@chainsafe/ssz"; import {SyncCommitteeMessagePool} from "../../../../src/chain/opPools/index.js"; +import {LocalClock} from "../../../../src/chain/clock/LocalClock.js"; describe("chain / opPools / SyncCommitteeMessagePool", function () { + const sandbox = sinon.createSandbox(); let cache: SyncCommitteeMessagePool; const subcommitteeIndex = 2; const indexInSubcommittee = 3; const beaconBlockRoot = Buffer.alloc(32, 1); const slot = 10; let syncCommittee: altair.SyncCommitteeMessage; + let clockStub: SinonStubbedInstance; + const cutOffTime = 1; before("Init BLS", async () => { const sk = bls.SecretKey.fromBytes(Buffer.alloc(32, 1)); @@ -23,11 +28,17 @@ describe("chain / opPools / SyncCommitteeMessagePool", function () { }); beforeEach(() => { - cache = new SyncCommitteeMessagePool(); + clockStub = sandbox.createStubInstance(LocalClock); + cache = new SyncCommitteeMessagePool(clockStub, cutOffTime); cache.add(subcommitteeIndex, syncCommittee, indexInSubcommittee); }); + afterEach(function () { + sandbox.restore(); + }); + it("should preaggregate SyncCommitteeContribution", () => { + clockStub.secFromSlot.returns(0); let contribution = cache.getContribution(subcommitteeIndex, syncCommittee.slot, syncCommittee.beaconBlockRoot); expect(contribution).to.be.not.null; const newSecretKey = bls.SecretKey.fromBytes(Buffer.alloc(32, 2)); diff --git a/packages/beacon-node/test/utils/mocks/chain/chain.ts b/packages/beacon-node/test/utils/mocks/chain/chain.ts index eb5693871084..a73978029e0e 100644 --- a/packages/beacon-node/test/utils/mocks/chain/chain.ts +++ b/packages/beacon-node/test/utils/mocks/chain/chain.ts @@ -83,9 +83,9 @@ export class MockBeaconChain implements IBeaconChain { reprocessController: ReprocessController; // Ops pool - readonly attestationPool = new AttestationPool(); + readonly attestationPool: AttestationPool; readonly aggregatedAttestationPool = new AggregatedAttestationPool(); - readonly syncCommitteeMessagePool = new SyncCommitteeMessagePool(); + readonly syncCommitteeMessagePool: SyncCommitteeMessagePool; readonly syncContributionAndProofPool = new SyncContributionAndProofPool(); readonly opPool = new OpPool(); @@ -126,6 +126,8 @@ export class MockBeaconChain implements IBeaconChain { emitter: this.emitter, signal: this.abortController.signal, }); + this.attestationPool = new AttestationPool(this.clock, (2 / 3) * this.config.SECONDS_PER_SLOT); + this.syncCommitteeMessagePool = new SyncCommitteeMessagePool(this.clock, (2 / 3) * this.config.SECONDS_PER_SLOT); this.forkChoice = mockForkChoice(); this.stateCache = new StateContextCache({}); this.checkpointStateCache = new CheckpointStateCache({}); From 1d94f76880531a285f0f366af238411bece3e0b3 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 14 Mar 2023 06:06:39 +0700 Subject: [PATCH 2/6] preaggregateSlotDistance hidden cli param --- packages/beacon-node/src/chain/chain.ts | 8 ++++++-- .../beacon-node/src/chain/opPools/attestationPool.ts | 9 +++++++-- .../src/chain/opPools/syncCommitteeMessagePool.ts | 9 +++++++-- packages/beacon-node/src/chain/options.ts | 8 ++++++++ packages/cli/src/options/beaconNodeOptions/chain.ts | 9 +++++++++ packages/cli/test/unit/options/beaconNodeOptions.test.ts | 2 ++ 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index 9d138def9f9d..7c9c333e897b 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -186,8 +186,12 @@ export class BeaconChain implements IBeaconChain { if (!clock) clock = new LocalClock({config, emitter, genesisTime: this.genesisTime, signal}); const preAggregateCutOffTime = (2 / 3) * this.config.SECONDS_PER_SLOT; - this.attestationPool = new AttestationPool(clock, preAggregateCutOffTime); - this.syncCommitteeMessagePool = new SyncCommitteeMessagePool(clock, preAggregateCutOffTime); + this.attestationPool = new AttestationPool(clock, preAggregateCutOffTime, this.opts?.preaggregateSlotDistance); + this.syncCommitteeMessagePool = new SyncCommitteeMessagePool( + clock, + preAggregateCutOffTime, + this.opts?.preaggregateSlotDistance + ); this.seenAggregatedAttestations = new SeenAggregatedAttestations(metrics); this.seenContributionAndProof = new SeenContributionAndProof(metrics); diff --git a/packages/beacon-node/src/chain/opPools/attestationPool.ts b/packages/beacon-node/src/chain/opPools/attestationPool.ts index 475f6265af93..ff76109434d9 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -61,7 +61,11 @@ export class AttestationPool { ); private lowestPermissibleSlot = 0; - constructor(private readonly clock: BeaconClock, private readonly cutOffSecFromSlot: number) {} + constructor( + private readonly clock: BeaconClock, + private readonly cutOffSecFromSlot: number, + private readonly preaggregateSlotDistance = 0 + ) {} /** Returns current count of pre-aggregated attestations with unique data */ getAttestationCount(): number { @@ -144,7 +148,8 @@ export class AttestationPool { */ prune(clockSlot: Slot): void { pruneBySlot(this.attestationByRootBySlot, clockSlot, SLOTS_RETAINED); - this.lowestPermissibleSlot = clockSlot; + // by default preaggregateSlotDistance is 0, i.e only accept attestations in the same clock slot. + this.lowestPermissibleSlot = Math.max(clockSlot - this.preaggregateSlotDistance, 0); } /** diff --git a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts index 1f18a0dea124..a65dae031293 100644 --- a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts +++ b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts @@ -45,7 +45,11 @@ export class SyncCommitteeMessagePool { >(() => new MapDef>(() => new Map())); private lowestPermissibleSlot = 0; - constructor(private readonly clock: BeaconClock, private readonly cutOffSecFromSlot: number) {} + constructor( + private readonly clock: BeaconClock, + private readonly cutOffSecFromSlot: number, + private readonly preaggregateSlotDistance = 0 + ) {} /** Returns current count of unique ContributionFast by block root and subnet */ get size(): number { @@ -114,7 +118,8 @@ export class SyncCommitteeMessagePool { */ prune(clockSlot: Slot): void { pruneBySlot(this.contributionsByRootBySubnetBySlot, clockSlot, SLOTS_RETAINED); - this.lowestPermissibleSlot = Math.max(clockSlot, 0); + // by default preaggregateSlotDistance is 0, i.e only accept SyncCommitteeMessage in the same clock slot. + this.lowestPermissibleSlot = Math.max(clockSlot - this.preaggregateSlotDistance, 0); } } diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index 5f9254e4fa45..cd8b01113494 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -5,6 +5,7 @@ import {ForkChoiceOpts} from "./forkChoice/index.js"; import {LightClientServerOpts} from "./lightClient/index.js"; export type IChainOptions = BlockProcessOpts & + PoolOpts & ForkChoiceOpts & ArchiverOpts & LightClientServerOpts & { @@ -47,6 +48,13 @@ export type BlockProcessOpts = { emitPayloadAttributes?: boolean; }; +export type PoolOpts = { + /** + * Only preaggregate attestation/sync committee message since clockSlot - preaggregateSlotDistance + */ + preaggregateSlotDistance?: number; +}; + export const defaultChainOptions: IChainOptions = { blsVerifyAllMainThread: false, blsVerifyAllMultiThread: false, diff --git a/packages/cli/src/options/beaconNodeOptions/chain.ts b/packages/cli/src/options/beaconNodeOptions/chain.ts index f21a14b830d8..53f735dbaa50 100644 --- a/packages/cli/src/options/beaconNodeOptions/chain.ts +++ b/packages/cli/src/options/beaconNodeOptions/chain.ts @@ -12,6 +12,7 @@ export type ChainArgs = { // "chain.persistInvalidSszObjectsDir": string; "chain.proposerBoostEnabled": boolean; "chain.disableImportExecutionFcU": boolean; + "chain.preaggregateSlotDistance": number; "chain.computeUnrealized": boolean; "chain.assertCorrectProgressiveBalances": boolean; "chain.maxSkipSlots": number; @@ -31,6 +32,7 @@ export function parseArgs(args: ChainArgs): IBeaconNodeOptions["chain"] { persistInvalidSszObjectsDir: undefined as any, proposerBoostEnabled: args["chain.proposerBoostEnabled"], disableImportExecutionFcU: args["chain.disableImportExecutionFcU"], + preaggregateSlotDistance: args["chain.preaggregateSlotDistance"], computeUnrealized: args["chain.computeUnrealized"], assertCorrectProgressiveBalances: args["chain.assertCorrectProgressiveBalances"], maxSkipSlots: args["chain.maxSkipSlots"], @@ -104,6 +106,13 @@ Will double processing times. Use only for debugging purposes.", group: "chain", }, + "chain.preaggregateSlotDistance": { + hidden: true, + type: "number", + description: "Only preaggregate attestations or sync committee message since clockSlot - preaggregateSlotDistance", + group: "chain", + }, + "chain.computeUnrealized": { hidden: true, type: "boolean", diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index 3ce362288f5c..9dc9bb4536a4 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -23,6 +23,7 @@ describe("options / beaconNodeOptions", () => { "chain.persistInvalidSszObjects": true, "chain.proposerBoostEnabled": false, "chain.disableImportExecutionFcU": false, + "chain.preaggregateSlotDistance": 1, "chain.computeUnrealized": true, suggestedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "chain.assertCorrectProgressiveBalances": true, @@ -109,6 +110,7 @@ describe("options / beaconNodeOptions", () => { persistInvalidSszObjects: true, proposerBoostEnabled: false, disableImportExecutionFcU: false, + preaggregateSlotDistance: 1, computeUnrealized: true, safeSlotsToImportOptimistically: 256, suggestedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", From adc2087be039c056326d5ecf14615429fbe8c6f7 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 14 Mar 2023 10:33:12 +0700 Subject: [PATCH 3/6] Log debug if error adding SyncCommitteeMessage to pool --- packages/beacon-node/src/api/impl/beacon/pool/index.ts | 2 +- packages/beacon-node/src/network/gossip/handlers/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/api/impl/beacon/pool/index.ts b/packages/beacon-node/src/api/impl/beacon/pool/index.ts index 8edef0a8fe4f..fd91e0a4fe99 100644 --- a/packages/beacon-node/src/api/impl/beacon/pool/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/pool/index.ts @@ -202,7 +202,7 @@ export function getBeaconPoolApi({ } errors.push(e as Error); - logger.error( + logger.debug( `Error on submitPoolSyncCommitteeSignatures [${i}]`, {slot: signature.slot, validatorIndex: signature.validatorIndex}, e as Error diff --git a/packages/beacon-node/src/network/gossip/handlers/index.ts b/packages/beacon-node/src/network/gossip/handlers/index.ts index 585b778dbe89..1f3d7f054e75 100644 --- a/packages/beacon-node/src/network/gossip/handlers/index.ts +++ b/packages/beacon-node/src/network/gossip/handlers/index.ts @@ -349,7 +349,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH try { chain.syncCommitteeMessagePool.add(subnet, syncCommittee, indexInSubcommittee); } catch (e) { - logger.error("Error adding to syncCommittee pool", {subnet}, e as Error); + logger.debug("Error adding to syncCommittee pool", {subnet}, e as Error); } }, From 092ea680443eda883bd02197a0bb09e22512b6cd Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 20 Mar 2023 16:08:13 +0700 Subject: [PATCH 4/6] SyncCommitteeMessagePool: return instead of throw error --- .../beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts | 4 ++-- packages/beacon-node/src/chain/opPools/types.ts | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts index a65dae031293..47f0a77c1d57 100644 --- a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts +++ b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts @@ -70,12 +70,12 @@ export class SyncCommitteeMessagePool { // Reject if too old. if (slot < lowestPermissibleSlot) { - throw new OpPoolError({code: OpPoolErrorCode.SLOT_TOO_LOW, slot, lowestPermissibleSlot}); + return InsertOutcome.Old; } // validator gets SyncCommitteeContribution at 2/3 of slot, it's no use to preaggregate later than that time if (this.clock.secFromSlot(slot) > this.cutOffSecFromSlot) { - throw new OpPoolError({code: OpPoolErrorCode.LATE_MESSAGE, slot}); + return InsertOutcome.Late; } // Limit object per slot diff --git a/packages/beacon-node/src/chain/opPools/types.ts b/packages/beacon-node/src/chain/opPools/types.ts index 54a3b60f9687..e91ec377178d 100644 --- a/packages/beacon-node/src/chain/opPools/types.ts +++ b/packages/beacon-node/src/chain/opPools/types.ts @@ -22,15 +22,12 @@ export enum InsertOutcome { export enum OpPoolErrorCode { /** The given object slot was too low to be stored. No changes were made. */ SLOT_TOO_LOW = "OP_POOL_ERROR_SLOT_TOO_LOW", - /** Good slot but it comes to the pool at late time */ - LATE_MESSAGE = "OP_POOL_ERROR_LATE_MESSAGE", /** Reached max number of unique objects per slot. This is a DoS protection function. */ REACHED_MAX_PER_SLOT = "OP_POOL_ERROR_REACHED_MAX_PER_SLOT", } export type OpPoolErrorType = | {code: OpPoolErrorCode.SLOT_TOO_LOW; slot: Slot; lowestPermissibleSlot: Slot} - | {code: OpPoolErrorCode.LATE_MESSAGE; slot: Slot} | {code: OpPoolErrorCode.REACHED_MAX_PER_SLOT}; export class OpPoolError extends LodestarError {} From 0d057f0fa53dc43e16ec5f91b5c0049426d8665d Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 20 Mar 2023 16:55:25 +0700 Subject: [PATCH 5/6] Add SyncCommitteeMesssage insertOutcome metric --- packages/beacon-node/src/metrics/metrics/lodestar.ts | 5 +++++ packages/beacon-node/src/network/gossip/handlers/index.ts | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index 01db193822a2..16c7d90bdf95 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -788,6 +788,11 @@ export function createLodestarMetrics( name: "lodestar_oppool_sync_committee_message_pool_size", help: "Current size of the SyncCommitteeMessagePool unique by slot subnet and block root", }), + syncCommitteeMessagePoolInsertOutcome: register.counter<"insertOutcome">({ + name: "lodestar_oppool_sync_committee_message_insert_outcome_total", + help: "Total number of InsertOutcome as a result of adding a SyncCommitteeMessage to pool", + labelNames: ["insertOutcome"], + }), syncContributionAndProofPoolSize: register.gauge({ name: "lodestar_oppool_sync_contribution_and_proof_pool_pool_size", help: "Current size of the SyncContributionAndProofPool unique by slot subnet and block root", diff --git a/packages/beacon-node/src/network/gossip/handlers/index.ts b/packages/beacon-node/src/network/gossip/handlers/index.ts index 1f3d7f054e75..8f04057b5c9f 100644 --- a/packages/beacon-node/src/network/gossip/handlers/index.ts +++ b/packages/beacon-node/src/network/gossip/handlers/index.ts @@ -347,7 +347,8 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH // Handler try { - chain.syncCommitteeMessagePool.add(subnet, syncCommittee, indexInSubcommittee); + const insertOutcome = chain.syncCommitteeMessagePool.add(subnet, syncCommittee, indexInSubcommittee); + metrics?.opPool.syncCommitteeMessagePoolInsertOutcome.inc({insertOutcome}); } catch (e) { logger.debug("Error adding to syncCommittee pool", {subnet}, e as Error); } From 7eaa0d0e4b753aac793da014b29bacb0f04c2037 Mon Sep 17 00:00:00 2001 From: tuyennhv Date: Mon, 27 Mar 2023 17:03:01 +0700 Subject: [PATCH 6/6] Update prune() method header Co-authored-by: Cayman --- packages/beacon-node/src/chain/opPools/attestationPool.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/attestationPool.ts b/packages/beacon-node/src/chain/opPools/attestationPool.ts index ff76109434d9..59e86d8f4e7d 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -143,8 +143,8 @@ export class AttestationPool { } /** - * Removes any attestations with a slot lower than `current_slot - SLOTS_RETAINED`. - * Not intested in attestations in old slots, we only preaggregate attestations for the current slot. + * Removes any attestations with a slot lower than `current_slot - preaggregateSlotDistance`. + * By default, not interested in attestations in old slots, we only preaggregate attestations for the current slot. */ prune(clockSlot: Slot): void { pruneBySlot(this.attestationByRootBySlot, clockSlot, SLOTS_RETAINED);