Remove committee bit cache

ChainSafe · Nov 8, 2024 · 6792566 · 6792566
1 parent 768dbbd
commit 6792566
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 48 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
@@ -105,16 +105,15 @@ 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, committeeBits} =
+            const {indexedAttestation, subnet, attDataRootHex, committeeIndex, aggregationBits} =
               await validateGossipFnRetryUnknownRoot(validateFn, network, chain, slot, beaconBlockRoot);
 
             if (network.shouldAggregate(subnet, slot)) {
               const insertOutcome = chain.attestationPool.add(
                 committeeIndex,
                 attestation,
                 attDataRootHex,
-                aggregationBits,
-                committeeBits
+                aggregationBits
               );
               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
@@ -1,7 +1,7 @@
 import {Signature, aggregateSignatures} from "@chainsafe/blst";
 import {BitArray} from "@chainsafe/ssz";
 import {ChainForkConfig} from "@lodestar/config";
-import {isForkPostElectra} from "@lodestar/params";
+import {MAX_COMMITTEES_PER_SLOT, isForkPostElectra} from "@lodestar/params";
 import {Attestation, RootHex, SingleAttestation, Slot, isElectraSingleAttestation} from "@lodestar/types";
 import {assert, MapDef} from "@lodestar/utils";
 import {IClock} from "../../util/clock.js";
@@ -109,8 +109,7 @@ export class AttestationPool {
     committeeIndex: CommitteeIndex,
     attestation: SingleAttestation,
     attDataRootHex: RootHex,
-    aggregationBits: BitArray | null,
-    committeeBits: BitArray | null
+    aggregationBits: BitArray | null
   ): InsertOutcome {
     const slot = attestation.data.slot;
     const fork = this.config.getForkName(slot);
@@ -153,7 +152,7 @@ export class AttestationPool {
       return aggregateAttestationInto(aggregate, attestation, aggregationBits);
     }
     // Create new aggregate
-    aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation, aggregationBits, committeeBits));
+    aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation, aggregationBits));
     return InsertOutcome.NewData;
   }
 
@@ -251,18 +250,13 @@ function aggregateAttestationInto(
 /**
  * Format `contribution` into an efficient `aggregate` to add more contributions in with aggregateContributionInto()
  */
-function attestationToAggregate(
-  attestation: SingleAttestation,
-  aggregationBits: BitArray | null,
-  committeeBits: BitArray | null
-): AggregateFast {
+function attestationToAggregate(attestation: SingleAttestation, aggregationBits: BitArray | null): AggregateFast {
   if (isElectraSingleAttestation(attestation)) {
     assert.notNull(aggregationBits, "aggregationBits missing post-electra to generate aggregate");
-    assert.notNull(committeeBits, "committeeBits missing post-electra to generate aggregate");
     return {
       data: attestation.data,
       aggregationBits,
-      committeeBits,
+      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
@@ -26,8 +26,6 @@ export type AttestationDataCacheEntry = {
   subnet: number;
   // aggregationBits only populates post-electra. Pre-electra can use get it directly from attestationOrBytes
   aggregationBits: BitArray | null;
-  // committeeBits only populates post-electra. Pre-electra does not require it
-  committeeBits: 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
@@ -37,7 +37,6 @@ import {
 } from "@lodestar/types";
 import {assert, toRootHex} from "@lodestar/utils";
 import {MAXIMUM_GOSSIP_CLOCK_DISPARITY_SEC} from "../../constants/index.js";
-import {sszDeserializeAttestation} from "../../network/gossip/topic.js";
 import {sszDeserializeSingleAttestation} from "../../network/gossip/topic.js";
 import {getShufflingDependentRoot} from "../../util/dependentRoot.js";
 import {
@@ -71,7 +70,6 @@ export type AttestationValidationResult = {
   attDataRootHex: RootHex;
   committeeIndex: CommitteeIndex;
   aggregationBits: BitArray | null; // Field populated post-electra only
-  committeeBits: BitArray | null; // Field populated post-electra only
 };
 
 export type AttestationOrBytes = ApiAttestation | GossipAttestation;
@@ -327,7 +325,6 @@ async function validateAttestationNoSignatureCheck(
   }
 
   let aggregationBits: BitArray | null = null;
-  let committeeBits: BitArray | 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).
@@ -352,14 +349,9 @@ async function validateAttestationNoSignatureCheck(
     if (attestationOrCache.cache && attestationOrCache.cache.aggregationBits !== null) {
       aggregationBits = attestationOrCache.cache.aggregationBits;
     }
-    // Populate aggregationBits if cached post-electra, else we populate later
-    if (attestationOrCache.cache && attestationOrCache.cache.committeeBits !== null) {
-      committeeBits = attestationOrCache.cache.committeeBits;
-    }
   }
 
   let committeeValidatorIndices: Uint32Array;
-  let numCommittees: number | null = null; // Only populate when we compute shuffling
   let getSigningRoot: () => Uint8Array;
   let expectedSubnet: number;
   if (attestationOrCache.cache) {
@@ -408,7 +400,6 @@ async function validateAttestationNoSignatureCheck(
     committeeValidatorIndices = getCommitteeIndices(shuffling, attSlot, committeeIndex);
     getSigningRoot = () => getAttestationDataSigningRoot(chain.config, attData);
     expectedSubnet = computeSubnetForSlot(shuffling, attSlot, committeeIndex);
-    numCommittees = shuffling.committeesPerSlot;
   }
 
   let validatorIndex: number;
@@ -444,10 +435,6 @@ async function validateAttestationNoSignatureCheck(
 
       aggregationBits = BitArray.fromSingleBit(committeeValidatorIndices.length, committeeValidatorIndex);
     }
-    // If committeeBits is null, it means it is not cached and thus numCommittees must be computed
-    if (committeeBits === null && numCommittees !== null) {
-      committeeBits = BitArray.fromSingleBit(numCommittees, committeeIndex);
-    }
   }
 
   // LH > verify_middle_checks
@@ -518,7 +505,6 @@ async function validateAttestationNoSignatureCheck(
         attDataRootHex,
         attestationData: attData,
         aggregationBits: isForkPostElectra(fork) ? aggregationBits : null,
-        committeeBits: isForkPostElectra(fork) ? committeeBits : null,
       });
     }
   }
@@ -555,7 +541,6 @@ async function validateAttestationNoSignatureCheck(
     validatorIndex,
     committeeIndex,
     aggregationBits: isForkPostElectra(fork) ? aggregationBits : null,
-    committeeBits: isForkPostElectra(fork) ? committeeBits : null,
   };
 }
 

diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts
@@ -637,7 +637,7 @@ function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOp
         results.push(null);
 
         // Handler
-        const {indexedAttestation, attDataRootHex, attestation, committeeIndex, aggregationBits, committeeBits} =
+        const {indexedAttestation, attDataRootHex, attestation, committeeIndex, aggregationBits} =
           validationResult.result;
         metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation);
 
@@ -650,8 +650,7 @@ function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOp
               committeeIndex,
               attestation,
               attDataRootHex,
-              aggregationBits,
-              committeeBits
+              aggregationBits
             );
             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
@@ -1,6 +1,6 @@
-import {BitArray, fromHexString, toHexString} from "@chainsafe/ssz";
+import {fromHexString, toHexString} from "@chainsafe/ssz";
 import {createChainForkConfig, defaultChainConfig} from "@lodestar/config";
-import {GENESIS_SLOT, MAX_COMMITTEES_PER_SLOT, MAX_VALIDATORS_PER_COMMITTEE, SLOTS_PER_EPOCH} from "@lodestar/params";
+import {GENESIS_SLOT, SLOTS_PER_EPOCH} from "@lodestar/params";
 import {ssz} from "@lodestar/types";
 import {beforeEach, describe, expect, it, vi} from "vitest";
 import {AttestationPool} from "../../../../src/chain/opPools/attestationPool.js";
@@ -51,10 +51,9 @@ describe("AttestationPool", () => {
 
   it("add correct electra attestation", () => {
     const committeeIndex = 0;
-    const committeeBits = BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, committeeIndex);
     const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue();
     const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestation.data));
-    const outcome = pool.add(committeeIndex, electraAttestation, attDataRootHex, aggregationBits, committeeBits);
+    const outcome = pool.add(committeeIndex, electraAttestation, attDataRootHex, aggregationBits);
 
     expect(outcome).equal(InsertOutcome.NewData);
     expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toEqual(electraAttestation);
@@ -63,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, null, null);
+    const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex, null);
 
     expect(outcome).equal(InsertOutcome.NewData);
     expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation);
@@ -74,22 +73,18 @@ describe("AttestationPool", () => {
 
   it("add electra attestation without committee index", () => {
     const committeeIndex = null;
-    const committeeBits = ssz.electra.Attestation.fields.committeeBits.defaultValue();
     const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue();
     const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestation.data));
 
-    expect(() =>
-      pool.add(committeeIndex, electraAttestation, attDataRootHex, aggregationBits, committeeBits)
-    ).toThrow();
+    expect(() => pool.add(committeeIndex, electraAttestation, attDataRootHex, aggregationBits)).toThrow();
     expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toBeNull();
   });
 
   it("add phase0 attestation with committee index", () => {
     const committeeIndex = 0;
-    const committeeBits = BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, committeeIndex);
     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, committeeBits);
+    const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex, aggregationBits);
 
     expect(outcome).equal(InsertOutcome.NewData);
     expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation);
@@ -100,7 +95,6 @@ describe("AttestationPool", () => {
 
   it("add electra attestation with phase0 slot", () => {
     const electraAttestationDataWithPhase0Slot = {...ssz.phase0.AttestationData.defaultValue(), slot: GENESIS_SLOT};
-    const committeeBits = ssz.electra.Attestation.fields.committeeBits.defaultValue();
     const aggregationBits = ssz.electra.Attestation.fields.aggregationBits.defaultValue();
     const attestation = {
       ...ssz.electra.Attestation.defaultValue(),
@@ -109,7 +103,7 @@ describe("AttestationPool", () => {
     };
     const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestationDataWithPhase0Slot));
 
-    expect(() => pool.add(0, attestation, attDataRootHex, aggregationBits, committeeBits)).toThrow();
+    expect(() => pool.add(0, attestation, attDataRootHex, aggregationBits)).toThrow();
   });
 
   it("add phase0 attestation with electra slot", () => {
@@ -124,6 +118,6 @@ describe("AttestationPool", () => {
     };
     const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0AttestationDataWithElectraSlot));
 
-    expect(() => pool.add(0, attestation, attDataRootHex, null, null)).toThrow();
+    expect(() => pool.add(0, attestation, attDataRootHex, null)).toThrow();
   });
 });