fix: avoid recomputing indexed attestations during block import

packages/beacon-node/src/chain/blocks/importBlock.ts

@@ -70,7 +70,8 @@ export async function importBlock(
   fullyVerifiedBlock: FullyVerifiedBlock,
   opts: ImportBlockOpts
 ): Promise<void> {
-  const {blockInput, postState, parentBlockSlot, executionStatus, dataAvailabilityStatus} = fullyVerifiedBlock;
+  const {blockInput, postState, parentBlockSlot, executionStatus, dataAvailabilityStatus, indexedAttestations} =
+    fullyVerifiedBlock;
   const block = blockInput.getBlock();
   const source = blockInput.getBlockSource();
   const {slot: blockSlot} = block.message;
@@ -138,10 +139,10 @@ export async function importBlock(
 
     const addAttestation = fork >= ForkSeq.electra ? addAttestationPostElectra : addAttestationPreElectra;
 
-    for (const attestation of attestations) {
+    for (let i = 0; i < attestations.length; i++) {
+      const attestation = attestations[i];
       try {
-        // TODO Electra: figure out how to reuse the attesting indices computed from state transition
-        const indexedAttestation = postState.epochCtx.getIndexedAttestation(fork, attestation);
+        const indexedAttestation = indexedAttestations[i];
         const {target, beaconBlockRoot} = attestation.data;
 
         const attDataRoot = toRootHex(ssz.phase0.AttestationData.hashTreeRoot(indexedAttestation.data));

packages/beacon-node/src/chain/blocks/index.ts

@@ -72,7 +72,7 @@ export async function processBlocks(
 
     // Fully verify a block to be imported immediately after. Does not produce any side-effects besides adding intermediate
     // states in the state cache through regen.
-    const {postStates, dataAvailabilityStatuses, proposerBalanceDeltas, segmentExecStatus} =
+    const {postStates, dataAvailabilityStatuses, proposerBalanceDeltas, segmentExecStatus, indexedAttestationsByBlock} =
       await verifyBlocksInEpoch.call(this, parentBlock, relevantBlocks, opts);
 
     // If segmentExecStatus has lvhForkchoice then, the entire segment should be invalid
@@ -94,6 +94,7 @@ export async function processBlocks(
         // start supporting optimistic syncing/processing
         dataAvailabilityStatus: dataAvailabilityStatuses[i],
         proposerBalanceDelta: proposerBalanceDeltas[i],
+        indexedAttestations: indexedAttestationsByBlock[i],
         // TODO: Make this param mandatory and capture in gossip
         seenTimestampSec: opts.seenTimestampSec ?? Math.floor(Date.now() / 1000),
       })

packages/beacon-node/src/chain/blocks/types.ts

@@ -2,7 +2,7 @@ import type {ChainForkConfig} from "@lodestar/config";
 import {MaybeValidExecutionStatus} from "@lodestar/fork-choice";
 import {ForkSeq} from "@lodestar/params";
 import {CachedBeaconStateAllForks, DataAvailabilityStatus, computeEpochAtSlot} from "@lodestar/state-transition";
-import type {Slot, fulu} from "@lodestar/types";
+import type {IndexedAttestation, Slot, fulu} from "@lodestar/types";
 import {IBlockInput} from "./blockInput/types.js";
 
 export enum GossipedInputType {
@@ -96,6 +96,10 @@ export type FullyVerifiedBlock = {
    */
   executionStatus: MaybeValidExecutionStatus;
   dataAvailabilityStatus: DataAvailabilityStatus;
+  /**
+   * Pre-computed indexed attestations from signature verification to avoid duplicate work
+   */
+  indexedAttestations: IndexedAttestation[];
   /** Seen timestamp seconds */
   seenTimestampSec: number;
 };

packages/beacon-node/src/chain/blocks/verifyBlock.ts

@@ -7,7 +7,7 @@ import {
   computeEpochAtSlot,
   isStateValidatorsNodesPopulated,
 } from "@lodestar/state-transition";
-import {bellatrix, deneb} from "@lodestar/types";
+import {IndexedAttestation, bellatrix, deneb} from "@lodestar/types";
 import {Logger, toRootHex} from "@lodestar/utils";
 import type {BeaconChain} from "../chain.js";
 import {BlockError, BlockErrorCode} from "../errors/index.js";
@@ -47,6 +47,7 @@ export async function verifyBlocksInEpoch(
   proposerBalanceDeltas: number[];
   segmentExecStatus: SegmentExecStatus;
   dataAvailabilityStatuses: DataAvailabilityStatus[];
+  indexedAttestationsByBlock: IndexedAttestation[][];
 }> {
   const blocks = blockInputs.map((blockInput) => blockInput.getBlock());
   const lastBlock = blocks.at(-1);
@@ -89,6 +90,16 @@ export async function verifyBlocksInEpoch(
     throw Error(`preState at slot ${preState0.slot} must be dialed to block epoch ${block0Epoch}`);
   }
 
+  // Store indexed attestations for each block to avoid recomputing them during import
+  const indexedAttestationsByBlock: IndexedAttestation[][] = [];
+
+  for (const [i, block] of blocks.entries()) {
+    const fork = this.config.getForkSeq(block.message.slot);
+    indexedAttestationsByBlock[i] = block.message.body.attestations.map((attestation) =>
+      preState0.epochCtx.getIndexedAttestation(fork, attestation)
+    );
+  }
+
   const abortController = new AbortController();
 
   try {
@@ -127,7 +138,15 @@ export async function verifyBlocksInEpoch(
 
       // All signatures at once
       opts.skipVerifyBlockSignatures !== true
-        ? verifyBlocksSignatures(this.bls, this.logger, this.metrics, preState0, blocks, opts)
+        ? verifyBlocksSignatures(
+            this.bls,
+            this.logger,
+            this.metrics,
+            preState0,
+            blocks,
+            indexedAttestationsByBlock,
+            opts
+          )
         : Promise.resolve({verifySignaturesTime: Date.now()}),
 
       // ideally we want to only persist blocks after verifying them however the reality is there are
@@ -222,7 +241,7 @@ export async function verifyBlocksInEpoch(
       );
     }
 
-    return {postStates, dataAvailabilityStatuses, proposerBalanceDeltas, segmentExecStatus};
+    return {postStates, dataAvailabilityStatuses, proposerBalanceDeltas, segmentExecStatus, indexedAttestationsByBlock};
   } finally {
     abortController.abort();
   }

packages/beacon-node/src/chain/blocks/verifyBlocksSignatures.ts

@@ -1,5 +1,5 @@
 import {CachedBeaconStateAllForks, getBlockSignatureSets} from "@lodestar/state-transition";
-import {SignedBeaconBlock} from "@lodestar/types";
+import {IndexedAttestation, SignedBeaconBlock} from "@lodestar/types";
 import {Logger} from "@lodestar/utils";
 import {Metrics} from "../../metrics/metrics.js";
 import {nextEventLoop} from "../../util/eventLoop.js";
@@ -20,6 +20,7 @@ export async function verifyBlocksSignatures(
   metrics: Metrics | null,
   preState0: CachedBeaconStateAllForks,
   blocks: SignedBeaconBlock[],
+  indexedAttestationsByBlock: IndexedAttestation[][],
   opts: ImportBlockOpts
 ): Promise<{verifySignaturesTime: number}> {
   const isValidPromises: Promise<boolean>[] = [];
@@ -37,7 +38,7 @@ export async function verifyBlocksSignatures(
       : //
         // Verify signatures per block to track which block is invalid
         bls.verifySignatureSets(
-          getBlockSignatureSets(preState0, block, {
+          getBlockSignatureSets(preState0, block, indexedAttestationsByBlock[i], {
             skipProposerSignature: opts.validProposerSignature,
           })
         );

packages/state-transition/src/signatureSets/index.ts

@@ -1,5 +1,5 @@
 import {ForkSeq} from "@lodestar/params";
-import {SignedBeaconBlock, altair, capella} from "@lodestar/types";
+import {IndexedAttestation, SignedBeaconBlock, altair, capella} from "@lodestar/types";
 import {getSyncCommitteeSignatureSet} from "../block/processSyncCommittee.js";
 import {CachedBeaconStateAllForks, CachedBeaconStateAltair} from "../types.js";
 import {ISignatureSet} from "../util/index.js";
@@ -26,6 +26,7 @@ export * from "./voluntaryExits.js";
 export function getBlockSignatureSets(
   state: CachedBeaconStateAllForks,
   signedBlock: SignedBeaconBlock,
+  indexedAttestations: IndexedAttestation[],
   opts?: {
     /** Useful since block proposer signature is verified beforehand on gossip validation */
     skipProposerSignature?: boolean;
@@ -38,7 +39,7 @@ export function getBlockSignatureSets(
     getRandaoRevealSignatureSet(state, signedBlock.message),
     ...getProposerSlashingsSignatureSets(state, signedBlock),
     ...getAttesterSlashingsSignatureSets(state, signedBlock),
-    ...getAttestationsSignatureSets(state, signedBlock),
+    ...getAttestationsSignatureSets(state, signedBlock, indexedAttestations),
     ...getVoluntaryExitsSignatureSets(state, signedBlock),
   ];
 

packages/state-transition/src/signatureSets/indexedAttestation.ts

@@ -39,13 +39,13 @@ export function getIndexedAttestationSignatureSet(
 
 export function getAttestationsSignatureSets(
   state: CachedBeaconStateAllForks,
-  signedBlock: SignedBeaconBlock
+  signedBlock: SignedBeaconBlock,
+  indexedAttestations: IndexedAttestation[]
 ): ISignatureSet[] {
-  // TODO: figure how to get attesting indices of an attestation once per block processing
-  return signedBlock.message.body.attestations.map((attestation) =>
-    getIndexedAttestationSignatureSet(
-      state,
-      state.epochCtx.getIndexedAttestation(state.config.getForkSeq(signedBlock.message.slot), attestation)
-    )
-  );
+  if (indexedAttestations.length !== signedBlock.message.body.attestations.length) {
+    throw Error(
+      `Indexed attestations length mismatch: got ${indexedAttestations.length}, expected ${signedBlock.message.body.attestations.length}`
+    );
+  }
+  return indexedAttestations.map((indexedAttestation) => getIndexedAttestationSignatureSet(state, indexedAttestation));
 }

packages/state-transition/test/unit/signatureSets/signatureSets.test.ts

@@ -65,8 +65,12 @@ describe("signatureSets", () => {
     }
 
     const state = generateCachedState(config, {validators});
+    const fork = state.config.getForkSeq(signedBlock.message.slot);
+    const indexedAttestations = signedBlock.message.body.attestations.map((attestation) =>
+      state.epochCtx.getIndexedAttestation(fork, attestation)
+    );
 
-    const signatureSets = getBlockSignatureSets(state, signedBlock);
+    const signatureSets = getBlockSignatureSets(state, signedBlock, indexedAttestations);
     expect(signatureSets.length).toBe(
       // block signature
       1 +