fix: prune cache entries of repeat proposals that are not imported

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

@@ -104,7 +104,7 @@ export async function processBlocks(
         payloadEnvelopesToImport.delete(orphaned.slot);
         // never validated, drop it from the shared cache so it is not served to peers, by-root sync reloads the
         // entry from db if the payload is ever needed
-        this.seenPayloadEnvelopeInputCache.prune(orphaned.payloadEnvelopeInput.blockRootHex);
+        this.seenPayloadEnvelopeInputCache.remove(orphaned.payloadEnvelopeInput.blockRootHex);
       }
     }
 
@@ -278,7 +278,7 @@ async function importPayloadEnvelopesOfKnownBlocks(
       this.forkChoice.isDescendant(blockRootHex, PayloadStatus.EMPTY, head.blockRoot, head.payloadStatus)
     ) {
       // never validated, see processBlocks()
-      this.seenPayloadEnvelopeInputCache.prune(blockRootHex);
+      this.seenPayloadEnvelopeInputCache.remove(blockRootHex);
       orphaned.push({slot, payloadEnvelopeInput: payloadInput});
       continue;
     }

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

@@ -23,7 +23,7 @@ export enum PayloadEnvelopeInputSource {
  * - prune: explicit prune by root
  * - cap: insertion-order backstop cap (MAX_PAYLOAD_ENVELOPE_INPUT_CACHE_SIZE)
  */
-export type PayloadEnvelopeInputPruneReason = "belowParent" | "finalized" | "prune" | "cap";
+export type PayloadEnvelopeInputPruneReason = "belowParent" | "finalized" | "remove" | "cap";
 
 export type SourceMeta = {
   source: PayloadEnvelopeInputSource;

packages/beacon-node/src/chain/seenCache/seenPayloadEnvelopeInput.ts

@@ -256,10 +256,11 @@ export class SeenPayloadEnvelopeInput {
     return this.payloadInputs.size;
   }
 
-  prune(blockRootHex: RootHex): void {
+  /** Removes the single PayloadEnvelopeInput from the cache */
+  remove(blockRootHex: RootHex): void {
     const input = this.payloadInputs.get(blockRootHex);
     if (input) {
-      this.evictPayloadInput(input, "prune");
+      this.evictPayloadInput(input, "remove");
     }
   }
 

packages/beacon-node/src/network/processor/gossipHandlers.ts

@@ -244,8 +244,20 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand
 
         // IGNORE means the block is acceptable (e.g. FUTURE_SLOT, ALREADY_KNOWN), just not propagated.
         // Keep the optimistically-added cache entries; they are pruned on finalization. Only REJECT
-        // (provably invalid) and unexpected errors prune below.
+        // (provably invalid), unexpected errors and repeat proposals that are not imported prune.
         if (e.action === GossipAction.IGNORE) {
+          // Only a signature-verified sibling is imported by the beacon_block handler, any other repeat proposal
+          // is dropped from the caches and re-downloaded by sync if it ever becomes relevant. Its signature was not
+          // verified, so only its own entry is removed, never its claimed ancestors
+          if (
+            e.type.code === BlockErrorCode.REPEAT_PROPOSAL &&
+            !chain.seenBlockProposers.hasBlockRoot(slot, signedBlock.message.proposerIndex, blockRootHex)
+          ) {
+            chain.seenBlockInputCache.remove(blockRootHex);
+            if (isForkPostGloas(fork)) {
+              chain.seenPayloadEnvelopeInputCache.remove(blockRootHex);
+            }
+          }
           throw e;
         }
 
@@ -258,10 +270,10 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand
       }
 
       // REJECT or unexpected (non-BlockGossipError) error: drop the optimistically-added entries from
-      // both caches, keeping them consistent.
-      chain.seenBlockInputCache.prune(blockRootHex);
+      // both caches, keeping them consistent. The block may carry any parent root, so only its own entry is removed
+      chain.seenBlockInputCache.remove(blockRootHex);
       if (isForkPostGloas(fork)) {
-        chain.seenPayloadEnvelopeInputCache.prune(blockRootHex);
+        chain.seenPayloadEnvelopeInputCache.remove(blockRootHex);
       }
       throw e;
     } finally {
@@ -738,7 +750,8 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand
         if (
           e instanceof BlockGossipError &&
           e.type.code === BlockErrorCode.REPEAT_PROPOSAL &&
-          // this is make sure the block's proposer signature was verified, it should be true anyway
+          // Only a signature-verified sibling recorded in the seen cache is imported. The cache holds at most two
+          // roots per proposer and slot, which bounds full imports over gossip to one alternate
           chain.seenBlockProposers.hasBlockRoot(signedBlock.message.slot, e.type.proposerIndex, e.type.root)
         ) {
           // blockInput was optimistically seeded in validateBeaconBlock and retained on IGNORE

packages/beacon-node/test/mocks/mockedBeaconChain.ts

@@ -181,7 +181,7 @@ vi.mock("../../src/chain/chain.js", async (importActual) => {
       seenPayloadEnvelopeInputCache: {
         get: vi.fn(),
         getOrReload: vi.fn(),
-        prune: vi.fn(),
+        remove: vi.fn(),
       },
       seenPayloadEnvelope: vi.fn(),
       shufflingCache: new ShufflingCache(),

packages/beacon-node/test/unit/chain/blocks/processBlocks.test.ts

@@ -295,12 +295,12 @@ describe("chain / blocks / processBlocks", () => {
         DataAvailabilityStatus.NotRequired,
         {validSignature: false}
       );
-      expect(chain.seenPayloadEnvelopeInputCache.prune).toHaveBeenCalledExactlyOnceWith(
+      expect(chain.seenPayloadEnvelopeInputCache.remove).toHaveBeenCalledExactlyOnceWith(
         parent.payloadInput.blockRootHex
       );
     } else {
       expect([...(envelopesForDa?.keys() ?? [])]).toEqual([slot - 1, slot]);
-      expect(chain.seenPayloadEnvelopeInputCache.prune).not.toHaveBeenCalled();
+      expect(chain.seenPayloadEnvelopeInputCache.remove).not.toHaveBeenCalled();
     }
     // the batch's own map is left untouched, range sync still owns it
     expect(payloadEnvelopes.size).toBe(2);
@@ -375,15 +375,15 @@ describe("chain / blocks / processBlocks", () => {
     expect(importBlock).not.toHaveBeenCalled();
     if (headOnEmpty && skipAttestations) {
       expect(importExecutionPayload).not.toHaveBeenCalled();
-      expect(chain.seenPayloadEnvelopeInputCache.prune).toHaveBeenCalledExactlyOnceWith(blockRootHex);
+      expect(chain.seenPayloadEnvelopeInputCache.remove).toHaveBeenCalledExactlyOnceWith(blockRootHex);
       expect(chain.recomputeForkChoiceHead).not.toHaveBeenCalled();
       // the skipped orphaned envelope is returned so range sync can log the serving peer/client
       expect(result).toEqual({orphaned: [{slot, payloadEnvelopeInput: payloadInput}], skipped: true});
     } else {
       expect(importExecutionPayload).toHaveBeenCalledExactlyOnceWith(payloadInput, DataAvailabilityStatus.NotRequired, {
         validSignature: false,
       });
-      expect(chain.seenPayloadEnvelopeInputCache.prune).not.toHaveBeenCalled();
+      expect(chain.seenPayloadEnvelopeInputCache.remove).not.toHaveBeenCalled();
       expect(chain.recomputeForkChoiceHead).toHaveBeenCalledOnce();
       expect(result).toEqual({orphaned: [], skipped: false});
     }

packages/beacon-node/test/unit/chain/seenCache/seenPayloadEnvelopeInput.test.ts

@@ -183,21 +183,21 @@ describe("SeenPayloadEnvelopeInput", () => {
     expect(cache.size()).toBe(1);
   });
 
-  it("prune removes a single entry by root and leaves others", () => {
+  it("remove removes a single entry by root and leaves others", () => {
     const rootHex1 = addPayloadInput(1);
     const rootHex2 = addPayloadInput(2);
 
-    cache.prune(rootHex1);
+    cache.remove(rootHex1);
 
     expect(cache.get(rootHex1)).toBeUndefined();
     expect(cache.get(rootHex2)).toBeDefined();
     expect(cache.size()).toBe(1);
   });
 
-  it("prune is a no-op for an unknown root", () => {
+  it("remove is a no-op for an unknown root", () => {
     const rootHex = addPayloadInput(1);
 
-    expect(() => cache.prune(`0x${"ab".repeat(32)}`)).not.toThrow();
+    expect(() => cache.remove(`0x${"ab".repeat(32)}`)).not.toThrow();
     expect(cache.get(rootHex)).toBeDefined();
     expect(cache.size()).toBe(1);
   });

packages/beacon-node/test/unit/network/processor/gossipHandlers.test.ts

@@ -65,16 +65,34 @@ describe("getGossipHandlers", () => {
   });
 
   it("imports a signature-verified REPEAT_PROPOSAL (equivocating) block into fork choice but keeps IGNORE", async () => {
-    const {processBlock, threw} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: true});
+    const {processBlock, threw, removeBlockInput} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: true});
+    expect(removeBlockInput).not.toHaveBeenCalled();
 
     // imported so LMD-GHOST can weigh it ...
     expect(processBlock).toHaveBeenCalledOnce();
     // ... but the gossip result stays IGNORE (handler re-throws), so the message is not forwarded
     expect(threw).toBe(true);
   });
 
+  it("removes only the rejected block's own cache entry on gossip REJECT", async () => {
+    const {processBlock, threw, removeBlockInput, pruneBlockInput} = await runBeaconBlockRepeatProposal(denebConfig, {
+      recorded: false,
+      reject: true,
+    });
+    expect(threw).toBe(true);
+    expect(processBlock).not.toHaveBeenCalled();
+    expect(removeBlockInput).toHaveBeenCalledOnce();
+    expect(pruneBlockInput).not.toHaveBeenCalled();
+  });
+
   it("does not import a REPEAT_PROPOSAL block whose root was not recorded (unverified 3rd+ proposal)", async () => {
-    const {processBlock, threw} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: false});
+    const {processBlock, threw, removeBlockInput, pruneBlockInput} = await runBeaconBlockRepeatProposal(denebConfig, {
+      recorded: false,
+    });
+    // the block is not kept around, sync re-downloads it if it ever becomes relevant. Only its own entry goes,
+    // an unverified block must not be able to evict the ancestors it claims
+    expect(removeBlockInput).toHaveBeenCalledOnce();
+    expect(pruneBlockInput).not.toHaveBeenCalled();
 
     expect(processBlock).not.toHaveBeenCalled();
     expect(threw).toBe(true);
@@ -120,7 +138,7 @@ async function runBeaconBlockProcessingError(
     seenPayloadEnvelopeInputCache: {
       add: vi.fn(),
       get: vi.fn().mockReturnValue(undefined),
-      prune: vi.fn(),
+      remove: vi.fn(),
     } as unknown as IBeaconChain["seenPayloadEnvelopeInputCache"],
     serializedCache: {set: vi.fn()},
   } as unknown as IBeaconChain;
@@ -161,8 +179,13 @@ async function runBeaconBlockProcessingError(
 
 async function runBeaconBlockRepeatProposal(
   config: BeaconConfig,
-  {recorded}: {recorded: boolean}
-): Promise<{processBlock: ReturnType<typeof vi.fn>; threw: boolean}> {
+  {recorded, reject = false}: {recorded: boolean; reject?: boolean}
+): Promise<{
+  processBlock: ReturnType<typeof vi.fn>;
+  threw: boolean;
+  removeBlockInput: ReturnType<typeof vi.fn>;
+  pruneBlockInput: ReturnType<typeof vi.fn>;
+}> {
   const logger = testLogger();
   const peerIdStr = "16Uiu2HAmTestGossipPeer" as PeerIdStr;
   const signedBlock = ssz.deneb.SignedBeaconBlock.defaultValue();
@@ -179,13 +202,21 @@ async function runBeaconBlockRepeatProposal(
     peerIdStr,
   });
 
-  // gossip validation rejects the 2nd distinct block for this (proposer, slot) with REPEAT_PROPOSAL
+  // gossip validation rejects the 2nd distinct block for this (proposer, slot) with REPEAT_PROPOSAL,
+  // or the block outright when `reject` is set
   vi.mocked(validateGossipBlock).mockRejectedValue(
-    new BlockGossipError(GossipAction.IGNORE, {
-      code: BlockErrorCode.REPEAT_PROPOSAL,
-      proposerIndex: signedBlock.message.proposerIndex,
-      root: blockRootHex,
-    })
+    reject
+      ? new BlockGossipError(GossipAction.REJECT, {
+          code: BlockErrorCode.INCORRECT_PROPOSER,
+          slot: signedBlock.message.slot,
+          root: blockRootHex,
+          proposerIndex: signedBlock.message.proposerIndex,
+        })
+      : new BlockGossipError(GossipAction.IGNORE, {
+          code: BlockErrorCode.REPEAT_PROPOSAL,
+          proposerIndex: signedBlock.message.proposerIndex,
+          root: blockRootHex,
+        })
   );
 
   const seenBlockProposers = new SeenBlockProposers();
@@ -201,24 +232,28 @@ async function runBeaconBlockRepeatProposal(
   }
 
   const processBlock = vi.fn().mockResolvedValue(undefined);
+  const removeBlockInput = vi.fn();
+  const pruneBlockInput = vi.fn();
   const chain = {
     clock: new ClockStopped(1),
     custodyConfig: {sampledColumns: [], custodyColumns: []} as unknown as CustodyConfig,
     emitter: new ChainEventEmitter(),
     getBlobsTracker: {triggerGetBlobs: vi.fn()},
     logger,
+    persistInvalidSszValue: vi.fn(),
     processBlock,
     processProposerEquivocation: vi.fn(),
     seenBlockProposers,
     seenBlockInputCache: {
       getByBlock: vi.fn().mockReturnValue(blockInput),
       get: vi.fn().mockReturnValue(blockInput),
-      prune: vi.fn(),
+      remove: removeBlockInput,
+      prune: pruneBlockInput,
     } as unknown as SeenBlockInput,
     seenPayloadEnvelopeInputCache: {
       add: vi.fn(),
       get: vi.fn().mockReturnValue(undefined),
-      prune: vi.fn(),
+      remove: vi.fn(),
     } as unknown as IBeaconChain["seenPayloadEnvelopeInputCache"],
     serializedCache: {set: vi.fn()},
   } as unknown as IBeaconChain;
@@ -257,7 +292,7 @@ async function runBeaconBlockRepeatProposal(
   await new Promise((resolve) => setTimeout(resolve, 0));
   await new Promise((resolve) => setTimeout(resolve, 0));
 
-  return {processBlock, threw};
+  return {processBlock, threw, removeBlockInput, pruneBlockInput};
 }
 
 function getExecutionBlockError(