Address comments

ChainSafe · May 7, 2024 · 4760572 · 4760572
1 parent 0de6dfa
commit 4760572
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 14 deletions.
diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts
@@ -147,6 +147,7 @@ export async function importBlock(
 
     for (const attestation of attestations) {
       try {
+        // TODO Electra: figure out how to reuse the attesting indices computed from state transition
         const indexedAttestation = postState.epochCtx.getIndexedAttestation(fork, attestation);
         const {target, beaconBlockRoot} = attestation.data;
 

diff --git a/packages/beacon-node/test/spec/presets/fork_choice.test.ts b/packages/beacon-node/test/spec/presets/fork_choice.test.ts
@@ -135,7 +135,10 @@ const forkChoiceTest =
               if (!attestation) throw Error(`No attestation ${step.attestation}`);
               const headState = chain.getHeadState();
               const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(attestation.data));
-              chain.forkChoice.onAttestation(headState.epochCtx.getIndexedAttestation(ForkSeq[fork], attestation), attDataRootHex);
+              chain.forkChoice.onAttestation(
+                headState.epochCtx.getIndexedAttestation(ForkSeq[fork], attestation),
+                attDataRootHex
+              );
             }
 
             // attester slashing step

diff --git a/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts
@@ -10,7 +10,7 @@ import {
   ApiAttestation,
   GossipAttestation,
   validateApiAttestation,
-validateAttestation,
+  validateAttestation,
 } from "../../../../../src/chain/validation/index.js";
 import {getAttDataBase64FromAttestationSerialized} from "../../../../../src/util/sszBytes.js";
 import {memoOnce} from "../../../../utils/cache.js";

diff --git a/packages/state-transition/src/block/processAttestationPhase0.ts b/packages/state-transition/src/block/processAttestationPhase0.ts
@@ -2,6 +2,7 @@ import {toHexString} from "@chainsafe/ssz";
 import {Slot, allForks, electra, phase0, ssz} from "@lodestar/types";
 
 import {MIN_ATTESTATION_INCLUSION_DELAY, SLOTS_PER_EPOCH, ForkSeq} from "@lodestar/params";
+import {assert} from "@lodestar/utils";
 import {computeEpochAtSlot} from "../util/index.js";
 import {CachedBeaconStatePhase0, CachedBeaconStateAllForks} from "../types.js";
 import {isValidIndexedAttestation} from "./index.js";
@@ -89,9 +90,7 @@ export function validateAttestation(
   }
 
   if (fork >= ForkSeq.electra) {
-    if (data.index !== 0) {
-      throw new Error(`AttestationData.index must be zero: index=${data.index}`);
-    }
+    assert.equal(data.index, 0, `AttestationData.index must be zero: index=${data.index}`);
     const attestationElectra = attestation as electra.Attestation;
     const committeeBitsLength = attestationElectra.committeeBits.bitLen;
 
@@ -109,11 +108,11 @@ export function validateAttestation(
       .map((committeeIndex) => epochCtx.getBeaconCommittee(data.slot, committeeIndex).length)
       .reduce((acc, committeeSize) => acc + committeeSize, 0);
 
-    if (attestationElectra.aggregationBits.bitLen !== participantCount) {
-      throw new Error(
-        `Attestation aggregation bits length does not match total number of committee participant aggregationBitsLength=${attestation.aggregationBits.bitLen} participantCount=${participantCount}`
-      );
-    }
+    assert.equal(
+      attestationElectra.aggregationBits.bitLen,
+      participantCount,
+      `Attestation aggregation bits length does not match total number of committee participant aggregationBitsLength=${attestation.aggregationBits.bitLen} participantCount=${participantCount}`
+    );
   } else {
     if (!(data.index < committeeCount)) {
       throw new Error(

diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts
@@ -660,7 +660,6 @@ export class EpochCache {
    * Return a single Uint32Array representing concatted committees of indices
    */
   getBeaconCommittees(slot: Slot, indices: CommitteeIndex[]): Uint32Array {
-
     if (indices.length === 0) {
       throw new Error("Attempt to get committees without providing CommitteeIndex");
     }
@@ -809,13 +808,12 @@ export class EpochCache {
       // In Lodestar it usually means a list of validator indices of participants in a committee
       // In the spec it means a list of committee indices according to committeeBits
       // This `committeeIndices` refers to the latter
+      // TODO Electra: resolve the naming conflicts
       const committeeIndices = committeeBits.getTrueBitIndexes();
 
       const validatorIndices = this.getBeaconCommittees(data.slot, committeeIndices);
 
-      const attestingIndices = new Set(aggregationBits.intersectValues(validatorIndices));
-
-      return Array.from(attestingIndices);
+      return aggregationBits.intersectValues(validatorIndices);
     }
   }
 

diff --git a/packages/state-transition/src/signatureSets/indexedAttestation.ts b/packages/state-transition/src/signatureSets/indexedAttestation.ts
@@ -41,6 +41,7 @@ export function getAttestationsSignatureSets(
   state: CachedBeaconStateAllForks,
   signedBlock: allForks.SignedBeaconBlock
 ): ISignatureSet[] {
+  // TODO: figure how to get attesting indices of an attestation once per block processing
   return signedBlock.message.body.attestations.map((attestation) =>
     getIndexedAttestationSignatureSet(
       state,