fix: align voluntary exit pruning with inclusion rules

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

@@ -264,10 +264,7 @@ export class OpPool {
         // Signature validation is skipped in `isValidVoluntaryExit(,,false)` since it was already validated in gossip
         // However we must make sure that the signature fork is the same, or it will become invalid if included through
         // a future fork.
-        isVoluntaryExitSignatureIncludable(
-          stateFork,
-          this.config.getForkSeq(computeStartSlotAtEpoch(voluntaryExit.message.epoch))
-        )
+        isVoluntaryExitSignatureIncludable(stateFork, this.getVoluntaryExitFork(voluntaryExit))
       ) {
         voluntaryExits.push(voluntaryExit);
         if (voluntaryExits.length >= MAX_VOLUNTARY_EXITS) {
@@ -373,10 +370,10 @@ export class OpPool {
     const finalizedEpoch = headState.finalizedCheckpoint.epoch;
 
     for (const [key, voluntaryExit] of this.voluntaryExits.entries()) {
-      // VoluntaryExit messages signed in the previous fork become invalid and can never be included in any future
-      // block, so just drop as the head state advances into the next fork.
-      if (this.config.getForkSeq(computeStartSlotAtEpoch(voluntaryExit.message.epoch)) < headStateFork) {
+      const voluntaryExitFork = this.getVoluntaryExitFork(voluntaryExit);
+      if (!isVoluntaryExitSignatureIncludable(headStateFork, voluntaryExitFork)) {
         this.voluntaryExits.delete(key);
+        continue;
       }
 
       // TODO: Improve this simplistic condition
@@ -386,6 +383,10 @@ export class OpPool {
     }
   }
 
+  private getVoluntaryExitFork(voluntaryExit: phase0.SignedVoluntaryExit): ForkSeq {
+    return this.config.getForkSeq(computeStartSlotAtEpoch(voluntaryExit.message.epoch));
+  }
+
   /**
    * Prune BLS to execution changes that have been applied to the state more than 1 block ago.
    * In the worse case where head block is reorged, the same BlsToExecutionChange message can be re-added
@@ -418,8 +419,8 @@ export class OpPool {
  */
 function isVoluntaryExitSignatureIncludable(stateFork: ForkSeq, voluntaryExitFork: ForkSeq): boolean {
   if (stateFork >= ForkSeq.deneb) {
-    // Exists are perpetually valid https://eips.ethereum.org/EIPS/eip-7044
-    return true;
+    // Deneb onwards the signature domain fork is fixed to capella, so only capella+ exits remain perpetually valid.
+    return voluntaryExitFork >= ForkSeq.capella;
   }
   // Can only include exits from the current and previous fork
   return voluntaryExitFork === stateFork || voluntaryExitFork === stateFork - 1;

packages/beacon-node/test/unit/chain/opPools/opPool.test.ts

@@ -0,0 +1,81 @@
+import {describe, expect, it} from "vitest";
+import {createBeaconConfig, createChainForkConfig, defaultChainConfig} from "@lodestar/config";
+import {BeaconStateView, computeStartSlotAtEpoch} from "@lodestar/state-transition";
+import {phase0, ssz} from "@lodestar/types";
+import {OpPool} from "../../../../src/chain/opPools/opPool.js";
+import {createCachedBeaconStateTest} from "../../../utils/cachedBeaconState.js";
+import {generateState} from "../../../utils/state.js";
+
+const chainConfig = createChainForkConfig({
+  ...defaultChainConfig,
+  ALTAIR_FORK_EPOCH: 1,
+  BELLATRIX_FORK_EPOCH: 2,
+  CAPELLA_FORK_EPOCH: 3,
+  DENEB_FORK_EPOCH: 4,
+  ELECTRA_FORK_EPOCH: 5,
+});
+
+describe("OpPool voluntary exits", () => {
+  it("keeps previous-fork exits that are still includable before deneb", () => {
+    const headSlot = computeStartSlotAtEpoch(chainConfig.BELLATRIX_FORK_EPOCH);
+    const {pool, headBlock, headState} = createPoolContext(headSlot);
+
+    pool.insertVoluntaryExit(createVoluntaryExit(chainConfig.ALTAIR_FORK_EPOCH));
+    pool.pruneAll(headBlock, headState);
+
+    expect(pool.getAllVoluntaryExits()).toHaveLength(1);
+  });
+
+  it("prunes exits whose signatures are no longer includable before deneb", () => {
+    const headSlot = computeStartSlotAtEpoch(chainConfig.BELLATRIX_FORK_EPOCH);
+    const {pool, headBlock, headState} = createPoolContext(headSlot);
+
+    pool.insertVoluntaryExit(createVoluntaryExit(0));
+    pool.pruneAll(headBlock, headState);
+
+    expect(pool.getAllVoluntaryExits()).toHaveLength(0);
+  });
+
+  it("keeps capella exits after deneb because signatures remain perpetually valid", () => {
+    const headSlot = computeStartSlotAtEpoch(chainConfig.ELECTRA_FORK_EPOCH);
+    const {pool, headBlock, headState} = createPoolContext(headSlot);
+
+    pool.insertVoluntaryExit(createVoluntaryExit(chainConfig.CAPELLA_FORK_EPOCH));
+    pool.pruneAll(headBlock, headState);
+
+    expect(pool.getAllVoluntaryExits()).toHaveLength(1);
+  });
+
+  it("prunes pre-capella exits after deneb because their signature domain is no longer valid", () => {
+    const headSlot = computeStartSlotAtEpoch(chainConfig.ELECTRA_FORK_EPOCH);
+    const {pool, headBlock, headState} = createPoolContext(headSlot);
+
+    pool.insertVoluntaryExit(createVoluntaryExit(chainConfig.BELLATRIX_FORK_EPOCH));
+    pool.pruneAll(headBlock, headState);
+
+    expect(pool.getAllVoluntaryExits()).toHaveLength(0);
+  });
+});
+
+function createPoolContext(headSlot: number) {
+  const state = generateState({slot: headSlot}, chainConfig);
+  const config = createBeaconConfig(chainConfig, state.genesisValidatorsRoot);
+  const headBlock = config.getForkTypes(headSlot).SignedBeaconBlock.defaultValue();
+  headBlock.message.slot = headSlot;
+
+  return {
+    pool: new OpPool(config),
+    headBlock,
+    headState: new BeaconStateView(createCachedBeaconStateTest(state, config)),
+  };
+}
+
+function createVoluntaryExit(epoch: number): phase0.SignedVoluntaryExit {
+  return {
+    ...ssz.phase0.SignedVoluntaryExit.defaultValue(),
+    message: {
+      validatorIndex: 0,
+      epoch,
+    },
+  };
+}