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 7f53bf9921b0..763d7036cba5 100644 --- a/packages/beacon-node/src/api/impl/beacon/pool/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/pool/index.ts @@ -105,7 +105,7 @@ 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, aggregationBits} = + const {indexedAttestation, subnet, attDataRootHex, committeeIndex, committeeValidatorIndex, committeeSize} = await validateGossipFnRetryUnknownRoot(validateFn, network, chain, slot, beaconBlockRoot); if (network.shouldAggregate(subnet, slot)) { @@ -113,7 +113,8 @@ export function getBeaconPoolApi({ committeeIndex, attestation, attDataRootHex, - aggregationBits + committeeValidatorIndex, + committeeSize ); 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 30f3738bb273..e7f302e42a50 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -109,7 +109,8 @@ export class AttestationPool { committeeIndex: CommitteeIndex, attestation: SingleAttestation, attDataRootHex: RootHex, - aggregationBits: BitArray | null + committeeValidatorIndex: number, + committeeSize: number ): InsertOutcome { const slot = attestation.data.slot; const fork = this.config.getForkName(slot); @@ -149,10 +150,10 @@ export class AttestationPool { const aggregate = aggregateByIndex.get(committeeIndex); if (aggregate) { // Aggregate mutating - return aggregateAttestationInto(aggregate, attestation, aggregationBits); + return aggregateAttestationInto(aggregate, attestation, committeeValidatorIndex); } // Create new aggregate - aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation, aggregationBits)); + aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation, committeeValidatorIndex, committeeSize)); return InsertOutcome.NewData; } @@ -224,13 +225,12 @@ export class AttestationPool { function aggregateAttestationInto( aggregate: AggregateFast, attestation: SingleAttestation, - aggregationBits: BitArray | null + committeeValidatorIndex: number ): InsertOutcome { let bitIndex: number | null; if (isElectraSingleAttestation(attestation)) { - assert.notNull(aggregationBits, "aggregationBits missing post-electra"); - bitIndex = aggregationBits.getSingleTrueBit(); + bitIndex = committeeValidatorIndex; } else { bitIndex = attestation.aggregationBits.getSingleTrueBit(); } @@ -250,12 +250,15 @@ function aggregateAttestationInto( /** * Format `contribution` into an efficient `aggregate` to add more contributions in with aggregateContributionInto() */ -function attestationToAggregate(attestation: SingleAttestation, aggregationBits: BitArray | null): AggregateFast { +function attestationToAggregate( + attestation: SingleAttestation, + committeeValidatorIndex: number, + committeeSize: number +): AggregateFast { if (isElectraSingleAttestation(attestation)) { - assert.notNull(aggregationBits, "aggregationBits missing post-electra to generate aggregate"); return { data: attestation.data, - aggregationBits, + aggregationBits: BitArray.fromSingleBit(committeeSize, committeeValidatorIndex), committeeBits: BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, attestation.committeeIndex), signature: signatureFromBytesNoCheck(attestation.signature), }; diff --git a/packages/beacon-node/src/chain/seenCache/seenAttestationData.ts b/packages/beacon-node/src/chain/seenCache/seenAttestationData.ts index 498c8f36ef8e..667b4aa98e9c 100644 --- a/packages/beacon-node/src/chain/seenCache/seenAttestationData.ts +++ b/packages/beacon-node/src/chain/seenCache/seenAttestationData.ts @@ -11,7 +11,6 @@ type AttDataBase64 = string; export type AttestationDataCacheEntry = { // part of shuffling data, so this does not take memory committeeValidatorIndices: Uint32Array; - // TODO: remove this? this is available in SingleAttestation committeeIndex: CommitteeIndex; // IndexedAttestationData signing root, 32 bytes signingRoot: Uint8Array; @@ -21,8 +20,6 @@ export type AttestationDataCacheEntry = { // for example in a mainnet node subscribing to all subnets, attestations are processed up to 20k per slot attestationData: phase0.AttestationData; subnet: number; - // aggregationBits only populates post-electra. Pre-electra can use get it directly from attestationOrBytes - aggregationBits: BitArray | null; }; export enum RejectReason { diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index b54ae10e8f64..395348a3ac34 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -69,7 +69,8 @@ export type AttestationValidationResult = { subnet: number; attDataRootHex: RootHex; committeeIndex: CommitteeIndex; - aggregationBits: BitArray | null; // Field populated post-electra only + committeeValidatorIndex: number; + committeeSize: number; }; export type AttestationOrBytes = ApiAttestation | GossipAttestation; @@ -325,6 +326,7 @@ async function validateAttestationNoSignatureCheck( } let aggregationBits: BitArray | null = null; + let committeeValidatorIndex: number | null = null; if (!isForkPostElectra(fork)) { // [REJECT] The attestation is unaggregated -- that is, it has exactly one participating validator // (len([bit for bit in attestation.aggregation_bits if bit]) == 1, i.e. exactly 1 bit is set). @@ -344,11 +346,7 @@ async function validateAttestationNoSignatureCheck( code: AttestationErrorCode.NOT_EXACTLY_ONE_AGGREGATION_BIT_SET, }); } - } else { - // Populate aggregationBits if cached post-electra, else we populate later - if (attestationOrCache.cache && attestationOrCache.cache.aggregationBits !== null) { - aggregationBits = attestationOrCache.cache.aggregationBits; - } + committeeValidatorIndex = bitIndex; } let committeeValidatorIndices: Uint32Array; @@ -407,10 +405,9 @@ async function validateAttestationNoSignatureCheck( if (!isForkPostElectra(fork)) { // The validity of aggregation bits are already checked above assert.notNull(aggregationBits); - const bitIndex = aggregationBits.getSingleTrueBit(); - assert.notNull(bitIndex); + assert.notNull(committeeValidatorIndex); - validatorIndex = committeeValidatorIndices[bitIndex]; + validatorIndex = committeeValidatorIndices[committeeValidatorIndex]; // [REJECT] The number of aggregation bits matches the committee size // -- i.e. len(attestation.aggregation_bits) == len(get_beacon_committee(state, data.slot, data.index)). // > TODO: Is this necessary? Lighthouse does not do this check. @@ -434,17 +431,12 @@ async function validateAttestationNoSignatureCheck( // [REJECT] The attester is a member of the committee -- i.e. // `attestation.attester_index in get_beacon_committee(state, attestation.data.slot, index)`. - // If `aggregationBitsElectra` exists, that means we have already cached it. No need to check again - if (aggregationBits === null) { - // Position of the validator in its committee - const committeeValidatorIndex = committeeValidatorIndices.indexOf(validatorIndex); - if (committeeValidatorIndex === -1) { - throw new AttestationError(GossipAction.REJECT, { - code: AttestationErrorCode.ATTESTER_NOT_IN_COMMITTEE, - }); - } - - aggregationBits = BitArray.fromSingleBit(committeeValidatorIndices.length, committeeValidatorIndex); + // Position of the validator in its committee + committeeValidatorIndex = committeeValidatorIndices.indexOf(validatorIndex); + if (committeeValidatorIndex === -1) { + throw new AttestationError(GossipAction.REJECT, { + code: AttestationErrorCode.ATTESTER_NOT_IN_COMMITTEE, + }); } } @@ -517,7 +509,6 @@ async function validateAttestationNoSignatureCheck( // root of AttestationData was already cached during getIndexedAttestationSignatureSet attDataRootHex, attestationData: attData, - aggregationBits: isForkPostElectra(fork) ? aggregationBits : null, }); } } @@ -553,7 +544,8 @@ async function validateAttestationNoSignatureCheck( signatureSet, validatorIndex, committeeIndex, - aggregationBits: isForkPostElectra(fork) ? aggregationBits : null, + committeeValidatorIndex, + committeeSize: committeeValidatorIndices.length, }; } diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index cdecb73ae95c..3fdebb3238c9 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -637,8 +637,14 @@ function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOp results.push(null); // Handler - const {indexedAttestation, attDataRootHex, attestation, committeeIndex, aggregationBits} = - validationResult.result; + const { + indexedAttestation, + attDataRootHex, + attestation, + committeeIndex, + committeeValidatorIndex, + committeeSize, + } = validationResult.result; metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); try { @@ -650,7 +656,8 @@ function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOp committeeIndex, attestation, attDataRootHex, - aggregationBits + committeeValidatorIndex, + committeeSize ); metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); } 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 2d2411b7ca73..5e897bb6380a 100644 --- a/packages/beacon-node/test/unit/chain/opPools/attestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/attestationPool.test.ts @@ -24,6 +24,9 @@ describe("AttestationPool", () => { const clockStub = getMockedClock(); vi.spyOn(clockStub, "secFromSlot").mockReturnValue(0); + const committeeValidatorIndex = 0; + const committeeSize = 128; + const cutOffSecFromSlot = (2 / 3) * config.SECONDS_PER_SLOT; // Mock attestations @@ -37,10 +40,10 @@ describe("AttestationPool", () => { signature: validSignature, }; const electraAttestation: electra.Attestation = { - ...ssz.electra.Attestation.defaultValue(), - committeeBits: BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, electraSingleAttestation.committeeIndex), + aggregationBits: BitArray.fromSingleBit(committeeSize, committeeValidatorIndex), data: electraAttestationData, signature: validSignature, + committeeBits: BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, electraSingleAttestation.committeeIndex), }; const phase0AttestationData: phase0.AttestationData = { ...ssz.phase0.AttestationData.defaultValue(), @@ -60,9 +63,14 @@ describe("AttestationPool", () => { it("add correct electra attestation", () => { const committeeIndex = 0; - const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue(); const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraSingleAttestation.data)); - const outcome = pool.add(committeeIndex, electraSingleAttestation, attDataRootHex, aggregationBits); + const outcome = pool.add( + committeeIndex, + electraSingleAttestation, + attDataRootHex, + committeeValidatorIndex, + committeeSize + ); expect(outcome).equal(InsertOutcome.NewData); expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toEqual(electraAttestation); @@ -71,7 +79,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, null); + const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex, committeeValidatorIndex, committeeSize); expect(outcome).equal(InsertOutcome.NewData); expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation); @@ -82,18 +90,18 @@ describe("AttestationPool", () => { it("add electra attestation without committee index", () => { const committeeIndex = null; - const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue(); const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraSingleAttestation.data)); - expect(() => pool.add(committeeIndex, electraSingleAttestation, attDataRootHex, aggregationBits)).toThrow(); + expect(() => + pool.add(committeeIndex, electraSingleAttestation, attDataRootHex, committeeValidatorIndex, committeeSize) + ).toThrow(); expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toBeNull(); }); it("add phase0 attestation with committee index", () => { const committeeIndex = 0; - const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue(); const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0Attestation.data)); - const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex, aggregationBits); + const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex, committeeValidatorIndex, committeeSize); expect(outcome).equal(InsertOutcome.NewData); expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation); @@ -104,7 +112,6 @@ describe("AttestationPool", () => { it("add electra attestation with phase0 slot", () => { const electraAttestationDataWithPhase0Slot = {...ssz.phase0.AttestationData.defaultValue(), slot: GENESIS_SLOT}; - const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue(); const singleAttestation = { ...ssz.electra.SingleAttestation.defaultValue(), data: electraAttestationDataWithPhase0Slot, @@ -112,7 +119,7 @@ describe("AttestationPool", () => { }; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestationDataWithPhase0Slot)); - expect(() => pool.add(0, singleAttestation, attDataRootHex, aggregationBits)).toThrow(); + expect(() => pool.add(0, singleAttestation, attDataRootHex, committeeValidatorIndex, committeeSize)).toThrow(); }); it("add phase0 attestation with electra slot", () => { @@ -127,6 +134,6 @@ describe("AttestationPool", () => { }; const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0AttestationDataWithElectraSlot)); - expect(() => pool.add(0, attestation, attDataRootHex, null)).toThrow(); + expect(() => pool.add(0, attestation, attDataRootHex, committeeValidatorIndex, committeeSize)).toThrow(); }); });