From 3927eebd56530ebf57b449f1e245426a9d7e0dad Mon Sep 17 00:00:00 2001 From: twoeths Date: Thu, 13 Aug 2026 13:56:30 +0700 Subject: [PATCH] fix: differentiate ALREADY_KNOWN vs REPEAT_PROPOSAL seen block proposal --- .../beacon-node/src/chain/blocks/index.ts | 2 +- .../src/chain/errors/blockError.ts | 2 +- .../src/chain/seenCache/seenBlockProposers.ts | 16 ++++++++-- .../beacon-node/src/chain/validation/block.ts | 28 ++++++++++++----- .../test/e2e/api/lodestar/lodestar.test.ts | 7 +++-- .../test/spec/utils/gossipValidation.ts | 6 +++- .../publishExecutionPayloadEnvelope.test.ts | 2 +- .../seenCache/seenBlockProposers.test.ts | 20 +++++++++++-- .../test/unit/chain/validation/block.test.ts | 30 ++++++++++++++++--- 9 files changed, 89 insertions(+), 24 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/index.ts b/packages/beacon-node/src/chain/blocks/index.ts index 374683a5471e..c8a643df835e 100644 --- a/packages/beacon-node/src/chain/blocks/index.ts +++ b/packages/beacon-node/src/chain/blocks/index.ts @@ -110,7 +110,7 @@ export async function processBlocks( for (const blockInput of relevantBlocks) { const block = blockInput.getBlock().message; - this.seenBlockProposers.add(block.slot, block.proposerIndex); + this.seenBlockProposers.add(block.slot, block.proposerIndex, blockInput.blockRootHex); } const {executionStatuses} = segmentExecStatus; diff --git a/packages/beacon-node/src/chain/errors/blockError.ts b/packages/beacon-node/src/chain/errors/blockError.ts index f9cd342c328c..6efa93a60504 100644 --- a/packages/beacon-node/src/chain/errors/blockError.ts +++ b/packages/beacon-node/src/chain/errors/blockError.ts @@ -118,7 +118,7 @@ export type BlockErrorType = | {code: BlockErrorCode.GENESIS_BLOCK} | {code: BlockErrorCode.WOULD_REVERT_FINALIZED_SLOT; blockSlot: Slot; finalizedSlot: Slot} | {code: BlockErrorCode.ALREADY_KNOWN; root: RootHex} - | {code: BlockErrorCode.REPEAT_PROPOSAL; proposerIndex: ValidatorIndex} + | {code: BlockErrorCode.REPEAT_PROPOSAL; proposerIndex: ValidatorIndex; root: RootHex} | {code: BlockErrorCode.BLOCK_SLOT_LIMIT_REACHED} | {code: BlockErrorCode.INCORRECT_PROPOSER; proposerIndex: ValidatorIndex} | {code: BlockErrorCode.PROPOSAL_SIGNATURE_INVALID; blockSlot: Slot} diff --git a/packages/beacon-node/src/chain/seenCache/seenBlockProposers.ts b/packages/beacon-node/src/chain/seenCache/seenBlockProposers.ts index d02454a8b8ef..a64207631352 100644 --- a/packages/beacon-node/src/chain/seenCache/seenBlockProposers.ts +++ b/packages/beacon-node/src/chain/seenCache/seenBlockProposers.ts @@ -19,7 +19,9 @@ const MIN_EQUIVOCATION_BLOCK_ROOTS_PER_PROPOSAL = 2; * The cache is pruned on finalization and bounds the number of roots stored per proposer and slot */ export class SeenBlockProposers { - private readonly proposerIndexesBySlot = new MapDef>(() => new Set()); + private readonly proposerIndexesBySlot = new MapDef>( + () => new Map() + ); private readonly signedBlockHeadersBySlot = new MapDef< Slot, MapDef> @@ -30,6 +32,14 @@ export class SeenBlockProposers { return this.proposerIndexesBySlot.get(blockSlot)?.has(proposerIndex) === true; } + /** + * The block proposer is known at slot with a different root. + */ + isRepeatProposal(blockSlot: Slot, proposerIndex: ValidatorIndex, blockRoot: RootHex): boolean { + const knownRoot = this.proposerIndexesBySlot.get(blockSlot)?.get(proposerIndex); + return knownRoot !== undefined && knownRoot !== blockRoot; + } + hasBlockRoot(blockSlot: Slot, proposerIndex: ValidatorIndex, blockRoot: RootHex): boolean { return this.signedBlockHeadersBySlot.get(blockSlot)?.get(proposerIndex)?.has(blockRoot) === true; } @@ -85,12 +95,12 @@ export class SeenBlockProposers { } /** Mark a block as known from gossip or another block import path */ - add(blockSlot: Slot, proposerIndex: ValidatorIndex): void { + add(blockSlot: Slot, proposerIndex: ValidatorIndex, blockRoot: RootHex): void { if (blockSlot < this.finalizedSlot) { throw Error(`blockSlot ${blockSlot} < finalizedSlot ${this.finalizedSlot}`); } - this.proposerIndexesBySlot.getOrDefault(blockSlot).add(proposerIndex); + this.proposerIndexesBySlot.getOrDefault(blockSlot).set(proposerIndex, blockRoot); } prune(finalizedSlot: Slot): void { diff --git a/packages/beacon-node/src/chain/validation/block.ts b/packages/beacon-node/src/chain/validation/block.ts index 11ec46526e55..e39a58758029 100644 --- a/packages/beacon-node/src/chain/validation/block.ts +++ b/packages/beacon-node/src/chain/validation/block.ts @@ -89,13 +89,20 @@ export async function validateGossipBlock( // [IGNORE] The block is the first block with valid signature received for the proposer for the slot, signed_beacon_block.message.slot. const proposerIndex = block.proposerIndex; - const hasBlockRoot = chain.seenBlockProposers.hasBlockRoot(blockSlot, proposerIndex, blockRoot); if (chain.seenBlockProposers.isKnown(blockSlot, proposerIndex)) { - if (!hasBlockRoot && !chain.seenBlockProposers.isEquivocating(blockSlot, proposerIndex)) { - await verifyBlockProposerSignature(chain, signedBlock, blockRoot, {verifyOnMainThread: false}); - chain.seenBlockProposers.observeBlockRoot(blockSlot, proposerIndex, blockRoot, signedBlockHeader); + if (chain.seenBlockProposers.isRepeatProposal(blockSlot, proposerIndex, blockRoot)) { + const hasBlockRoot = chain.seenBlockProposers.hasBlockRoot(blockSlot, proposerIndex, blockRoot); + if (!hasBlockRoot && !chain.seenBlockProposers.isEquivocating(blockSlot, proposerIndex)) { + await verifyBlockProposerSignature(chain, signedBlock, blockRoot, {verifyOnMainThread: false}); + chain.seenBlockProposers.observeBlockRoot(blockSlot, proposerIndex, blockRoot, signedBlockHeader); + } + throw new BlockGossipError(GossipAction.IGNORE, { + code: BlockErrorCode.REPEAT_PROPOSAL, + proposerIndex, + root: blockRoot, + }); } - throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.REPEAT_PROPOSAL, proposerIndex}); + throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.ALREADY_KNOWN, root: blockRoot}); } // [REJECT] The current finalized_checkpoint is an ancestor of block -- i.e. @@ -301,10 +308,17 @@ export async function validateGossipBlock( // Check again after all async validation and the early-block delay so concurrent proposals cannot both pass if (chain.seenBlockProposers.isKnown(blockSlot, proposerIndex)) { - throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.REPEAT_PROPOSAL, proposerIndex}); + if (chain.seenBlockProposers.isRepeatProposal(blockSlot, proposerIndex, blockRoot)) { + throw new BlockGossipError(GossipAction.IGNORE, { + code: BlockErrorCode.REPEAT_PROPOSAL, + proposerIndex, + root: blockRoot, + }); + } + throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.ALREADY_KNOWN, root: blockRoot}); } - chain.seenBlockProposers.add(blockSlot, proposerIndex); + chain.seenBlockProposers.add(blockSlot, proposerIndex, blockRoot); return {skippedSlots}; } diff --git a/packages/beacon-node/test/e2e/api/lodestar/lodestar.test.ts b/packages/beacon-node/test/e2e/api/lodestar/lodestar.test.ts index 774ba8e1c569..300a8bb2f178 100644 --- a/packages/beacon-node/test/e2e/api/lodestar/lodestar.test.ts +++ b/packages/beacon-node/test/e2e/api/lodestar/lodestar.test.ts @@ -5,6 +5,7 @@ import {chainConfig as chainConfigDef} from "@lodestar/config/default"; import {LogLevel, TestLoggerOpts, testLogger} from "@lodestar/logger/test-utils"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; import {phase0} from "@lodestar/types"; +import {toRootHex} from "@lodestar/utils"; import {BeaconNode} from "../../../../src/index.js"; import {ClockEvent} from "../../../../src/util/clock.js"; import {waitForEvent} from "../../../utils/events/resolver.js"; @@ -61,12 +62,12 @@ describe("api / impl / validator", () => { }); // live indices at epoch of consideration, epoch 0 - bn.chain.seenBlockProposers.add(0, 1); + bn.chain.seenBlockProposers.add(0, 1, toRootHex(Buffer.alloc(32))); bn.chain.seenBlockAttesters.add(0, 2); bn.chain.seenAttesters.add(0, 3); bn.chain.seenAggregators.add(0, 4); // live indices at other epochs, epoch 10 - bn.chain.seenBlockProposers.add(10, 1000); + bn.chain.seenBlockProposers.add(10, 1000, toRootHex(Buffer.alloc(32))); bn.chain.seenAttesters.add(10, 2000); bn.chain.seenAggregators.add(10, 3000); @@ -105,7 +106,7 @@ describe("api / impl / validator", () => { await waitForEvent(bn.chain.clock, ClockEvent.epoch, timeout); // wait for epoch 1 await waitForEvent(bn.chain.clock, ClockEvent.epoch, timeout); // wait for epoch 2 - bn.chain.seenBlockProposers.add(bn.chain.clock.currentEpoch, 1); + bn.chain.seenBlockProposers.add(bn.chain.clock.currentEpoch, 1, toRootHex(Buffer.alloc(32))); const client = getClient({baseUrl: `http://127.0.0.1:${restPort}`}, {config}); diff --git a/packages/beacon-node/test/spec/utils/gossipValidation.ts b/packages/beacon-node/test/spec/utils/gossipValidation.ts index 961400e7fb76..9f815abf2258 100644 --- a/packages/beacon-node/test/spec/utils/gossipValidation.ts +++ b/packages/beacon-node/test/spec/utils/gossipValidation.ts @@ -608,7 +608,11 @@ async function validateMessageForTopic( } await validateGossipBlock(chain.config, chain, signedBlock, fork); - chain.seenBlockProposers.add(signedBlock.message.slot, signedBlock.message.proposerIndex); + chain.seenBlockProposers.add( + signedBlock.message.slot, + signedBlock.message.proposerIndex, + toRootHex(sszTypesFor(fork).BeaconBlock.hashTreeRoot(signedBlock.message)) + ); break; } diff --git a/packages/beacon-node/test/unit/api/impl/beacon/blocks/publishExecutionPayloadEnvelope.test.ts b/packages/beacon-node/test/unit/api/impl/beacon/blocks/publishExecutionPayloadEnvelope.test.ts index a3207a9bceed..6240ddc311d1 100644 --- a/packages/beacon-node/test/unit/api/impl/beacon/blocks/publishExecutionPayloadEnvelope.test.ts +++ b/packages/beacon-node/test/unit/api/impl/beacon/blocks/publishExecutionPayloadEnvelope.test.ts @@ -63,7 +63,7 @@ describe("api - beacon - publishExecutionPayloadEnvelope", () => { modules.forkChoice.getBlockHex.mockReturnValue(generateProtoBlock({slot})); vi.mocked(modules.chain.seenPayloadEnvelopeInputCache.get).mockReturnValue(payloadInput); modules.chain.regen.getBlockSlotState.mockResolvedValue({forkName: ForkName.gloas} as IBeaconStateView); - modules.chain.seenBlockProposers.add(slot, proposerIndex); + modules.chain.seenBlockProposers.add(slot, proposerIndex, blockRoot); modules.chain.seenBlockProposers.observeBlockRoot( slot, proposerIndex, diff --git a/packages/beacon-node/test/unit/chain/seenCache/seenBlockProposers.test.ts b/packages/beacon-node/test/unit/chain/seenCache/seenBlockProposers.test.ts index 105a7dfbe640..bdf30c39fdcb 100644 --- a/packages/beacon-node/test/unit/chain/seenCache/seenBlockProposers.test.ts +++ b/packages/beacon-node/test/unit/chain/seenCache/seenBlockProposers.test.ts @@ -31,7 +31,7 @@ describe("SeenBlockProposers", () => { expect(cache.hasBlockRoot(slot, proposerIndex, blockRoot)).toBe(true); expect(cache.getEquivocationHeaders(slot, proposerIndex)).toBe(null); - cache.add(slot, proposerIndex); + cache.add(slot, proposerIndex, blockRoot); cache.observeBlockRoot(slot, proposerIndex, conflictingBlockRoot, header2); expect(cache.isKnown(slot, proposerIndex)).toBe(true); @@ -41,6 +41,20 @@ describe("SeenBlockProposers", () => { expect(cache.getEquivocationHeaders(slot, proposerIndex)).toEqual([header1, header2]); }); + it("flags a repeat proposal only for a different block root", () => { + const cache = new SeenBlockProposers(); + + // Not known yet: never a repeat, regardless of root + expect(cache.isRepeatProposal(slot, proposerIndex, blockRoot)).toBe(false); + + cache.add(slot, proposerIndex, blockRoot); + + // Known with the same root: a benign duplicate, not a repeat + expect(cache.isRepeatProposal(slot, proposerIndex, blockRoot)).toBe(false); + // Known with a different root: a genuine equivocation + expect(cache.isRepeatProposal(slot, proposerIndex, conflictingBlockRoot)).toBe(true); + }); + it("stores at most two roots per slot and proposer", () => { const cache = new SeenBlockProposers(); @@ -70,7 +84,7 @@ describe("SeenBlockProposers", () => { it("prunes known proposals and observed roots", () => { const cache = new SeenBlockProposers(); cache.observeBlockRoot(slot, proposerIndex, blockRoot, header1); - cache.add(slot, proposerIndex); + cache.add(slot, proposerIndex, blockRoot); cache.prune(slot + 1); @@ -84,7 +98,7 @@ describe("SeenBlockProposers", () => { const cache = new SeenBlockProposers(); cache.prune(slot + 1); - expect(() => cache.add(slot, proposerIndex)).toThrow(`blockSlot ${slot} < finalizedSlot ${slot + 1}`); + expect(() => cache.add(slot, proposerIndex, blockRoot)).toThrow(`blockSlot ${slot} < finalizedSlot ${slot + 1}`); expect(() => cache.observeBlockRoot(slot, proposerIndex, blockRoot, header1)).toThrow( `blockSlot ${slot} < finalizedSlot ${slot + 1}` ); diff --git a/packages/beacon-node/test/unit/chain/validation/block.test.ts b/packages/beacon-node/test/unit/chain/validation/block.test.ts index 60b499ce583d..86b93a89ae3b 100644 --- a/packages/beacon-node/test/unit/chain/validation/block.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/block.test.ts @@ -119,6 +119,27 @@ describe("gossip block validation", () => { setupChain(gloasConfig); }); + it("ignores a same-root duplicate as ALREADY_KNOWN, not REPEAT_PROPOSAL", async () => { + const forkTypes = gloasConfig.getForkTypes(clockSlot); + const signedBlock = forkTypes.SignedBeaconBlock.defaultValue(); + signedBlock.message.slot = clockSlot; + signedBlock.message.proposerIndex = proposerIndex; + const blockRoot = toRootHex(forkTypes.BeaconBlock.hashTreeRoot(signedBlock.message)); + chain.seenBlockProposers.observeBlockRoot( + clockSlot, + proposerIndex, + blockRoot, + signedBlockToSignedHeader(gloasConfig, signedBlock) + ); + chain.seenBlockProposers.add(clockSlot, proposerIndex, blockRoot); + + // Re-submitting the SAME block (same root) is a benign duplicate, not an equivocation + await expectRejectedWithLodestarError( + validateGossipBlock(gloasConfig, chain, signedBlock, ForkName.gloas), + BlockErrorCode.ALREADY_KNOWN + ); + }); + it("records a conflicting block root after verifying the proposer signature", async () => { const forkTypes = gloasConfig.getForkTypes(clockSlot); const signedBlock = forkTypes.SignedBeaconBlock.defaultValue(); @@ -131,7 +152,7 @@ describe("gossip block validation", () => { blockRoot, signedBlockToSignedHeader(gloasConfig, signedBlock) ); - chain.seenBlockProposers.add(clockSlot, proposerIndex); + chain.seenBlockProposers.add(clockSlot, proposerIndex, blockRoot); const conflictingBlock = forkTypes.SignedBeaconBlock.clone(signedBlock); conflictingBlock.message.stateRoot = Buffer.alloc(32, 1); @@ -165,7 +186,7 @@ describe("gossip block validation", () => { blockRoot, signedBlockToSignedHeader(gloasConfig, signedBlock) ); - chain.seenBlockProposers.add(clockSlot, proposerIndex); + chain.seenBlockProposers.add(clockSlot, proposerIndex, blockRoot); const conflictingBlock = forkTypes.SignedBeaconBlock.clone(signedBlock); conflictingBlock.message.stateRoot = Buffer.alloc(32, 1); @@ -197,7 +218,7 @@ describe("gossip block validation", () => { toRootHex(Buffer.alloc(32, 1)), ssz.phase0.SignedBeaconBlockHeader.defaultValue() ); - chain.seenBlockProposers.add(clockSlot, proposerIndex); + chain.seenBlockProposers.add(clockSlot, proposerIndex, blockRoot); const additionalBlock = forkTypes.SignedBeaconBlock.clone(signedBlock); additionalBlock.message.stateRoot = Buffer.alloc(32, 2); @@ -235,7 +256,8 @@ describe("gossip block validation", () => { await vi.advanceTimersByTimeAsync(0); expect(vi.getTimerCount()).toBe(1); - chain.seenBlockProposers.add(clockSlot, proposerIndex); + // A different proposal (different root) becomes known during the delay -> genuine repeat proposal + chain.seenBlockProposers.add(clockSlot, proposerIndex, toRootHex(Buffer.alloc(32, 0xff))); await vi.advanceTimersByTimeAsync(100); await validation; } finally {