fix: unsupport bundling attestations for pre-electra blocks

.github/workflows/test-sim.yml

@@ -91,35 +91,6 @@ jobs:
           name: sim-test-endpoints-logs
           path: packages/cli/test-logs
 
-  sim-test-deneb:
-    name: Deneb sim tests
-    needs: build
-    runs-on: buildjet-4vcpu-ubuntu-2204
-    steps:
-      # <common-build> - Uses YAML anchors in the future
-      - uses: actions/checkout@v4
-      - uses: "./.github/actions/setup-and-build"
-        with:
-          node: 24
-      - name: Load env variables
-        uses: ./.github/actions/dotenv
-      - name: Download required docker images before running tests
-        run: |
-          docker pull ${{env.GETH_DOCKER_IMAGE}}
-          docker pull ${{env.LIGHTHOUSE_DOCKER_IMAGE}}
-          docker pull ${{env.NETHERMIND_DOCKER_IMAGE}}
-      - name: Sim tests deneb
-        run: DEBUG='${{github.event.inputs.debug}}' yarn test:sim:deneb
-        working-directory: packages/cli
-        env:
-          GENESIS_DELAY_SLOTS: ${{github.event.inputs.genesisDelaySlots}}
-      - name: Upload debug log test files for "packages/cli"
-        if: ${{ always() }}
-        uses: actions/upload-artifact@v4
-        with:
-          name: sim-test-deneb-logs
-          path: packages/cli/test-logs
-
   sim-test-eth-backup-provider:
     name: Eth backup provider sim tests
     needs: build

docs/pages/contribution/testing/simulation-tests.md

@@ -34,10 +34,6 @@ This tests that various endpoints of the beacon node and validator client are wo
 yarn workspace @chainsafe/lodestar test:sim:endpoints
 ```
 
-### `test:sim:deneb`
-
-This test is still included in our CI but is no longer as important as it once was. Lodestar is often the first client to implement new features and this test was created before geth was upgraded with the features required to support the Deneb fork. To test that Lodestar was ready this test uses mocked geth instances. It is left as a placeholder for when the next fork comes along that requires a similar approach.
-
 ### `test:sim:mixedcleint`
 
 Checks that Lodestar is compatible with other consensus validators and vice-versa. All tests use Geth as the EL.

packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts

@@ -5,7 +5,6 @@ import {IForkChoice} from "@lodestar/fork-choice";
 import {
   ForkName,
   ForkSeq,
-  MAX_ATTESTATIONS,
   MAX_ATTESTATIONS_ELECTRA,
   MAX_COMMITTEES_PER_SLOT,
   MIN_ATTESTATION_INCLUSION_DELAY,
@@ -23,7 +22,6 @@ import {
   CachedBeaconStateAllForks,
   CachedBeaconStateAltair,
   CachedBeaconStateGloas,
-  CachedBeaconStatePhase0,
   EffectiveBalanceIncrements,
   RootCache,
   computeEpochAtSlot,
@@ -32,17 +30,7 @@ import {
   getAttestationParticipationStatus,
   getBlockRootAtSlot,
 } from "@lodestar/state-transition";
-import {
-  Attestation,
-  Epoch,
-  RootHex,
-  Slot,
-  ValidatorIndex,
-  electra,
-  isElectraAttestation,
-  phase0,
-  ssz,
-} from "@lodestar/types";
+import {Attestation, Epoch, RootHex, Slot, electra, isElectraAttestation, phase0, ssz} from "@lodestar/types";
 import {MapDef, assert, toRootHex} from "@lodestar/utils";
 import {Metrics} from "../../metrics/metrics.js";
 import {IntersectResult, intersectUint8Arrays} from "../../util/bitArray.js";
@@ -54,8 +42,6 @@ type DataRootHex = string;
 
 type CommitteeIndex = number;
 
-// for pre-electra
-type AttestationWithScore = {attestation: Attestation; score: number};
 /**
  * for electra, this is to consolidate aggregated attestations of the same attestation data into a single attestation to be included in block
  * note that this is local definition in this file and it's NOT validator consolidation
@@ -110,15 +96,6 @@ const MAX_RETAINED_ATTESTATIONS_PER_GROUP = 4;
  */
 const MAX_RETAINED_ATTESTATIONS_PER_GROUP_ELECTRA = 8;
 
-/**
- * Pre-electra, each slot has 64 committees, and each block has 128 attestations max so in average
- * we get 2 attestation per groups.
- * Starting from Jan 2024, we have a performance issue getting attestations for a block. Based on the
- * fact that lot of groups will have only 1 full participation attestation, increase this number
- * a bit higher than average. This also help decrease number of slots to search for attestations.
- */
-const MAX_ATTESTATIONS_PER_GROUP = 3;
-
 /**
  * For electra, there is on chain aggregation of attestations across committees, so we can just pick up to 8
  * attestations per group, sort by scores to get first 8.
@@ -245,108 +222,7 @@ export class AggregatedAttestationPool {
     forkChoice: IForkChoice,
     state: CachedBeaconStateAllForks
   ): phase0.Attestation[] {
-    const stateSlot = state.slot;
-    const stateEpoch = state.epochCtx.epoch;
-    const statePrevEpoch = stateEpoch - 1;
-
-    const notSeenValidatorsFn = getNotSeenValidatorsFn(this.config, state);
-    const validateAttestationDataFn = getValidateAttestationDataFn(forkChoice, state);
-
-    const attestationsByScore: AttestationWithScore[] = [];
-
-    const slots = Array.from(this.attestationGroupByIndexByDataHexBySlot.keys()).sort((a, b) => b - a);
-    let minScore = Number.MAX_SAFE_INTEGER;
-    let slotCount = 0;
-    slot: for (const slot of slots) {
-      slotCount++;
-      const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHexBySlot.get(slot);
-      // should not happen
-      if (!attestationGroupByIndexByDataHash) {
-        throw Error(`No aggregated attestation pool for slot=${slot}`);
-      }
-
-      const epoch = computeEpochAtSlot(slot);
-      // validateAttestation condition: Attestation target epoch not in previous or current epoch
-      if (!(epoch === stateEpoch || epoch === statePrevEpoch)) {
-        continue; // Invalid attestations
-      }
-      // validateAttestation condition: Attestation slot not within inclusion window
-      if (
-        !(
-          slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot &&
-          // Post deneb, attestations are valid for current and previous epoch
-          (ForkSeq[fork] >= ForkSeq.deneb || stateSlot <= slot + SLOTS_PER_EPOCH)
-        )
-      ) {
-        continue; // Invalid attestations
-      }
-
-      const inclusionDistance = stateSlot - slot;
-      for (const attestationGroupByIndex of attestationGroupByIndexByDataHash.values()) {
-        for (const [committeeIndex, attestationGroup] of attestationGroupByIndex.entries()) {
-          const notSeenCommitteeMembers = notSeenValidatorsFn(epoch, slot, committeeIndex);
-          if (notSeenCommitteeMembers === null || notSeenCommitteeMembers.size === 0) {
-            continue;
-          }
-
-          if (
-            slotCount > 2 &&
-            attestationsByScore.length >= MAX_ATTESTATIONS &&
-            notSeenCommitteeMembers.size / inclusionDistance < minScore
-          ) {
-            // after 2 slots, there are a good chance that we have 2 * MAX_ATTESTATIONS attestations and break the for loop early
-            // if not, we may have to scan all slots in the pool
-            // if we have enough attestations and the max possible score is lower than scores of `attestationsByScore`, we should skip
-            // otherwise it takes time to check attestation, add it and remove it later after the sort by score
-            continue;
-          }
-
-          if (validateAttestationDataFn(attestationGroup.data) !== null) {
-            continue;
-          }
-
-          // TODO: Is it necessary to validateAttestation for:
-          // - Attestation committee index not within current committee count
-          // - Attestation aggregation bits length does not match committee length
-          //
-          // These properties should not change after being validate in gossip
-          // IF they have to be validated, do it only with one attestation per group since same data
-          // The committeeCountPerSlot can be precomputed once per slot
-          const getAttestationsResult = attestationGroup.getAttestationsForBlock(
-            fork,
-            state.epochCtx.effectiveBalanceIncrements,
-            notSeenCommitteeMembers,
-            MAX_ATTESTATIONS_PER_GROUP
-          );
-          for (const {attestation, newSeenEffectiveBalance} of getAttestationsResult.result) {
-            const score = newSeenEffectiveBalance / inclusionDistance;
-            if (score < minScore) {
-              minScore = score;
-            }
-            attestationsByScore.push({
-              attestation,
-              score,
-            });
-          }
-
-          // Stop accumulating attestations there are enough that may have good scoring
-          if (attestationsByScore.length >= MAX_ATTESTATIONS * 2) {
-            break slot;
-          }
-        }
-      }
-    }
-
-    const sortedAttestationsByScore = attestationsByScore.sort((a, b) => b.score - a.score);
-    const attestationsForBlock: phase0.Attestation[] = [];
-    for (const [i, attestationWithScore] of sortedAttestationsByScore.entries()) {
-      if (i >= MAX_ATTESTATIONS) {
-        break;
-      }
-      // attestations could be modified in this op pool, so we need to clone for block
-      attestationsForBlock.push(ssz.phase0.Attestation.clone(attestationWithScore.attestation));
-    }
-    return attestationsForBlock;
+    throw new Error("Does not support producing blocks for pre-electra forks anymore");
   }
 
   /**
@@ -867,38 +743,7 @@ export function aggregateConsolidation({byCommittee, attData}: AttestationsConso
 export function getNotSeenValidatorsFn(config: BeaconConfig, state: CachedBeaconStateAllForks): GetNotSeenValidatorsFn {
   const stateSlot = state.slot;
   if (config.getForkName(stateSlot) === ForkName.phase0) {
-    // Get attestations to be included in a phase0 block.
-    // As we are close to altair, this is not really important, it's mainly for e2e.
-    // The performance is not great due to the different BeaconState data structure to altair.
-    // check for phase0 block already
-    const phase0State = state as CachedBeaconStatePhase0;
-    const stateEpoch = computeEpochAtSlot(stateSlot);
-
-    const previousEpochParticipants = extractParticipationPhase0(
-      phase0State.previousEpochAttestations.getAllReadonly(),
-      state
-    );
-    const currentEpochParticipants = extractParticipationPhase0(
-      phase0State.currentEpochAttestations.getAllReadonly(),
-      state
-    );
-
-    return (epoch: Epoch, slot: Slot, committeeIndex: number) => {
-      const participants =
-        epoch === stateEpoch ? currentEpochParticipants : epoch === stateEpoch - 1 ? previousEpochParticipants : null;
-      if (participants === null) {
-        return null;
-      }
-      const committee = state.epochCtx.getBeaconCommittee(slot, committeeIndex);
-
-      const notSeenCommitteeMembers = new Set<number>();
-      for (const [i, validatorIndex] of committee.entries()) {
-        if (!participants.has(validatorIndex)) {
-          notSeenCommitteeMembers.add(i);
-        }
-      }
-      return notSeenCommitteeMembers.size === 0 ? null : notSeenCommitteeMembers;
-    };
+    throw new Error("getNotSeenValidatorsFn is not supported phase0 state");
   }
 
   // altair and future forks
@@ -942,26 +787,6 @@ export function getNotSeenValidatorsFn(config: BeaconConfig, state: CachedBeacon
   };
 }
 
-export function extractParticipationPhase0(
-  attestations: phase0.PendingAttestation[],
-  state: CachedBeaconStateAllForks
-): Set<ValidatorIndex> {
-  const {epochCtx} = state;
-  const allParticipants = new Set<ValidatorIndex>();
-  for (const att of attestations) {
-    const aggregationBits = att.aggregationBits;
-    const attData = att.data;
-    const attSlot = attData.slot;
-    const committeeIndex = attData.index;
-    const committee = epochCtx.getBeaconCommittee(attSlot, committeeIndex);
-    const participants = aggregationBits.intersectValues(committee);
-    for (const participant of participants) {
-      allParticipants.add(participant);
-    }
-  }
-  return allParticipants;
-}
-
 /**
  * This returns a function to validate if an attestation data is compatible to a state.
  *

packages/beacon-node/src/execution/engine/mock.ts

@@ -1,4 +1,5 @@
 import crypto from "node:crypto";
+import {ChainConfig} from "@lodestar/config";
 import {
   BLOB_TX_TYPE,
   BYTES_PER_FIELD_ELEMENT,
@@ -7,7 +8,9 @@ import {
   ForkPostBellatrix,
   ForkPostCapella,
   ForkSeq,
+  SLOTS_PER_EPOCH,
 } from "@lodestar/params";
+import {computeTimeAtSlot} from "@lodestar/state-transition";
 import {ExecutionPayload, RootHex, bellatrix, deneb, ssz} from "@lodestar/types";
 import {fromHex, toRootHex} from "@lodestar/utils";
 import {ZERO_HASH_HEX} from "../../constants/index.js";
@@ -34,14 +37,11 @@ const INTEROP_GAS_LIMIT = 30e6;
 const PRUNE_PAYLOAD_ID_AFTER_MS = 5000;
 
 export type ExecutionEngineMockOpts = {
-  genesisBlockHash: string;
+  genesisBlockHash?: string;
   eth1BlockHash?: string;
   onlyPredefinedResponses?: boolean;
-  capellaForkTimestamp?: number;
-  denebForkTimestamp?: number;
-  electraForkTimestamp?: number;
-  fuluForkTimestamp?: number;
-  gloasForkTimestamp?: number;
+  genesisTime?: number;
+  config?: ChainConfig;
 };
 
 type ExecutionBlock = {
@@ -74,17 +74,21 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
   /** Preparing payloads to be retrieved via engine_getPayloadV1 */
   private readonly preparingPayloads = new Map<number, PreparedPayload>();
   private readonly payloadsForDeletion = new Map<number, number>();
-
   private readonly predefinedPayloadStatuses = new Map<RootHex, PayloadStatus>();
 
   private payloadId = 0;
+  private capellaForkTimestamp: number;
+  private denebForkTimestamp: number;
+  private electraForkTimestamp: number;
+  private fuluForkTimestamp: number;
+  private gloasForkTimestamp: number;
 
   readonly handlers: {
     [K in keyof EngineApiRpcParamTypes]: (...args: EngineApiRpcParamTypes[K]) => EngineApiRpcReturnTypes[K];
   };
 
   constructor(private readonly opts: ExecutionEngineMockOpts) {
-    this.validBlocks.set(opts.genesisBlockHash, {
+    this.validBlocks.set(opts.genesisBlockHash ?? ZERO_HASH_HEX, {
       parentHash: ZERO_HASH_HEX,
       blockHash: ZERO_HASH_HEX,
       timestamp: 0,
@@ -100,6 +104,29 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
       blockNumber: 1,
     });
 
+    const {config} = opts;
+
+    this.capellaForkTimestamp =
+      opts.genesisTime && config
+        ? computeTimeAtSlot(config, config.CAPELLA_FORK_EPOCH * SLOTS_PER_EPOCH, opts.genesisTime)
+        : Infinity;
+    this.denebForkTimestamp =
+      opts.genesisTime && config
+        ? computeTimeAtSlot(config, config.DENEB_FORK_EPOCH * SLOTS_PER_EPOCH, opts.genesisTime)
+        : Infinity;
+    this.electraForkTimestamp =
+      opts.genesisTime && config
+        ? computeTimeAtSlot(config, config.ELECTRA_FORK_EPOCH * SLOTS_PER_EPOCH, opts.genesisTime)
+        : Infinity;
+    this.fuluForkTimestamp =
+      opts.genesisTime && config
+        ? computeTimeAtSlot(config, config.FULU_FORK_EPOCH * SLOTS_PER_EPOCH, opts.genesisTime)
+        : Infinity;
+    this.gloasForkTimestamp =
+      opts.genesisTime && config
+        ? computeTimeAtSlot(config, config.GLOAS_FORK_EPOCH * SLOTS_PER_EPOCH, opts.genesisTime)
+        : Infinity;
+
     this.handlers = {
       engine_newPayloadV1: this.notifyNewPayload.bind(this),
       engine_newPayloadV2: this.notifyNewPayload.bind(this),
@@ -448,11 +475,11 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
   }
 
   private timestampToFork(timestamp: number): ForkPostBellatrix {
-    if (timestamp >= (this.opts.gloasForkTimestamp ?? Infinity)) return ForkName.gloas;
-    if (timestamp >= (this.opts.fuluForkTimestamp ?? Infinity)) return ForkName.fulu;
-    if (timestamp >= (this.opts.electraForkTimestamp ?? Infinity)) return ForkName.electra;
-    if (timestamp >= (this.opts.denebForkTimestamp ?? Infinity)) return ForkName.deneb;
-    if (timestamp >= (this.opts.capellaForkTimestamp ?? Infinity)) return ForkName.capella;
+    if (timestamp >= this.gloasForkTimestamp) return ForkName.gloas;
+    if (timestamp >= this.fuluForkTimestamp) return ForkName.fulu;
+    if (timestamp >= this.electraForkTimestamp) return ForkName.electra;
+    if (timestamp >= this.denebForkTimestamp) return ForkName.deneb;
+    if (timestamp >= this.capellaForkTimestamp) return ForkName.capella;
     return ForkName.bellatrix;
   }
 }