From 7d3d255297185c04ee7b1c18ee8ffe9f4e0a6df5 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Wed, 30 Oct 2024 13:28:46 +0700 Subject: [PATCH] fix: reduce getSingleTrueBit() calls --- .../src/api/impl/beacon/pool/index.ts | 16 ++++++++-------- .../src/chain/opPools/attestationPool.ts | 13 ++++++++----- .../src/chain/validation/attestation.ts | 2 ++ .../src/network/processor/gossipHandlers.ts | 10 ++++++++-- .../unit/chain/opPools/attestationPool.test.ts | 13 +++++++------ 5 files changed, 33 insertions(+), 21 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 9343dd67a39..36f22badc0a 100644 --- a/packages/beacon-node/src/api/impl/beacon/pool/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/pool/index.ts @@ -99,16 +99,16 @@ export function getBeaconPoolApi({ // when a validator is configured with multiple beacon node urls, this attestation data may come from another beacon node // and the block hasn't been in our forkchoice since we haven't seen / processing that block // see https://github.com/ChainSafe/lodestar/issues/5098 - const {indexedAttestation, subnet, attDataRootHex, committeeIndex} = await validateGossipFnRetryUnknownRoot( - validateFn, - network, - chain, - slot, - beaconBlockRoot - ); + const {indexedAttestation, subnet, attDataRootHex, committeeIndex, participationIndex} = + await validateGossipFnRetryUnknownRoot(validateFn, network, chain, slot, beaconBlockRoot); if (network.shouldAggregate(subnet, slot)) { - const insertOutcome = chain.attestationPool.add(committeeIndex, attestation, attDataRootHex); + const insertOutcome = chain.attestationPool.add( + committeeIndex, + participationIndex, + attestation, + attDataRootHex + ); metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); } diff --git a/packages/beacon-node/src/chain/opPools/attestationPool.ts b/packages/beacon-node/src/chain/opPools/attestationPool.ts index 8d8fbb92c0f..3bf3a78a21c 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -105,7 +105,12 @@ export class AttestationPool { * - Valid committeeIndex * - Valid data */ - add(committeeIndex: CommitteeIndex, attestation: Attestation, attDataRootHex: RootHex): InsertOutcome { + add( + committeeIndex: CommitteeIndex, + participationIndex: number, + attestation: Attestation, + attDataRootHex: RootHex + ): InsertOutcome { const slot = attestation.data.slot; const fork = this.config.getForkName(slot); const lowestPermissibleSlot = this.lowestPermissibleSlot; @@ -144,7 +149,7 @@ export class AttestationPool { const aggregate = aggregateByIndex.get(committeeIndex); if (aggregate) { // Aggregate mutating - return aggregateAttestationInto(aggregate, attestation); + return aggregateAttestationInto(aggregate, attestation, participationIndex); } // Create new aggregate aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation)); @@ -216,9 +221,7 @@ export class AttestationPool { /** * Aggregate a new attestation into `aggregate` mutating it */ -function aggregateAttestationInto(aggregate: AggregateFast, attestation: Attestation): InsertOutcome { - const bitIndex = attestation.aggregationBits.getSingleTrueBit(); - +function aggregateAttestationInto(aggregate: AggregateFast, attestation: Attestation, bitIndex: number): InsertOutcome { // Should never happen, attestations are verified against this exact condition before assert.notNull(bitIndex, "Invalid attestation in pool, not exactly one bit set"); diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 0b3f490e412..e1818b276af 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -61,6 +61,7 @@ export type AttestationValidationResult = { subnet: number; attDataRootHex: RootHex; committeeIndex: CommitteeIndex; + participationIndex: number; }; export type AttestationOrBytes = ApiAttestation | GossipAttestation; @@ -505,6 +506,7 @@ async function validateAttestationNoSignatureCheck( signatureSet, validatorIndex, committeeIndex, + participationIndex: bitIndex, }; } diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index dab3af0df8b..4063dcdcf13 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -633,14 +633,20 @@ function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOp results.push(null); // Handler - const {indexedAttestation, attDataRootHex, attestation, committeeIndex} = validationResult.result; + const {indexedAttestation, attDataRootHex, attestation, committeeIndex, participationIndex} = + validationResult.result; metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); try { // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages // but don't add to attestation pool, to save CPU and RAM if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { - const insertOutcome = chain.attestationPool.add(committeeIndex, attestation, attDataRootHex); + const insertOutcome = chain.attestationPool.add( + committeeIndex, + participationIndex, + attestation, + attDataRootHex + ); metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); } } catch (e) { diff --git a/packages/beacon-node/test/unit/chain/opPools/attestationPool.test.ts b/packages/beacon-node/test/unit/chain/opPools/attestationPool.test.ts index 98453efaa3b..8926f6c206d 100644 --- a/packages/beacon-node/test/unit/chain/opPools/attestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/attestationPool.test.ts @@ -44,6 +44,7 @@ describe("AttestationPool", () => { }; let pool: AttestationPool; + const participationIndex = 0; beforeEach(() => { pool = new AttestationPool(config, clockStub, cutOffSecFromSlot); @@ -52,7 +53,7 @@ describe("AttestationPool", () => { it("add correct electra attestation", () => { const committeeIndex = 0; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestation.data)); - const outcome = pool.add(committeeIndex, electraAttestation, attDataRootHex); + const outcome = pool.add(committeeIndex, participationIndex, electraAttestation, attDataRootHex); expect(outcome).equal(InsertOutcome.NewData); expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toEqual(electraAttestation); @@ -61,7 +62,7 @@ describe("AttestationPool", () => { it("add correct phase0 attestation", () => { const committeeIndex = null; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0Attestation.data)); - const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex); + const outcome = pool.add(committeeIndex, participationIndex, phase0Attestation, attDataRootHex); expect(outcome).equal(InsertOutcome.NewData); expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation); @@ -74,14 +75,14 @@ describe("AttestationPool", () => { const committeeIndex = null; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestation.data)); - expect(() => pool.add(committeeIndex, electraAttestation, attDataRootHex)).toThrow(); + expect(() => pool.add(committeeIndex, participationIndex, electraAttestation, attDataRootHex)).toThrow(); expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toBeNull(); }); it("add phase0 attestation with committee index", () => { const committeeIndex = 0; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0Attestation.data)); - const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex); + const outcome = pool.add(committeeIndex, participationIndex, phase0Attestation, attDataRootHex); expect(outcome).equal(InsertOutcome.NewData); expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation); @@ -99,7 +100,7 @@ describe("AttestationPool", () => { }; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestationDataWithPhase0Slot)); - expect(() => pool.add(0, attestation, attDataRootHex)).toThrow(); + expect(() => pool.add(0, participationIndex, attestation, attDataRootHex)).toThrow(); }); it("add phase0 attestation with electra slot", () => { @@ -114,6 +115,6 @@ describe("AttestationPool", () => { }; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0AttestationDataWithElectraSlot)); - expect(() => pool.add(0, attestation, attDataRootHex)).toThrow(); + expect(() => pool.add(0, participationIndex, attestation, attDataRootHex)).toThrow(); }); });