fix: ignore repeat proposal during block publishing#9235
fix: ignore repeat proposal during block publishing#9235lodekeeper wants to merge 2 commits intoChainSafe:unstablefrom
Conversation
Extend the existing ALREADY_KNOWN handler in publishBlock to also swallow REPEAT_PROPOSAL. In multi-node setups (DVT clusters, fallback configs) the same block can reach a node via gossip before the API, causing the seen block proposer check to throw BLOCK_ERROR_REPEAT_PROPOSAL. Both errors use GossipAction.IGNORE semantics and should return a silent 2xx rather than a 500 response. Fixes ChainSafe#9228 🤖 Generated with AI assistance
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the beacon block API to ignore REPEAT_PROPOSAL errors in addition to ALREADY_KNOWN errors when publishing blocks. A review comment suggests refining the debug log message and including the specific error code in the metadata to more accurately reflect the reason for ignoring the block and improve observability.
| error instanceof BlockGossipError && | ||
| (error.type.code === BlockErrorCode.ALREADY_KNOWN || error.type.code === BlockErrorCode.REPEAT_PROPOSAL) | ||
| ) { | ||
| chain.logger.debug("Ignoring known block during publishing", valLogMeta); |
There was a problem hiding this comment.
The log message "Ignoring known block" is slightly inaccurate when the error is REPEAT_PROPOSAL (which indicates a block for the same slot/proposer was seen, but not necessarily the same block root). Including the actual error code in the log metadata would improve observability and help distinguish between these two cases during debugging.
| chain.logger.debug("Ignoring known block during publishing", valLogMeta); | |
| chain.logger.debug("Ignoring block during publishing", {...valLogMeta, reason: error.type.code}); |
There was a problem hiding this comment.
Good catch — applied in b68ea2b. The log is now "Ignoring block during publishing" with reason: error.type.code in metadata, so ALREADY_KNOWN vs REPEAT_PROPOSAL is distinguishable.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04573899a8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| (error.type.code === BlockErrorCode.ALREADY_KNOWN || error.type.code === BlockErrorCode.REPEAT_PROPOSAL) | ||
| ) { |
There was a problem hiding this comment.
REPEAT_PROPOSAL is raised for any second proposal by the same proposer/slot, not just benign duplicates of an already-known root. By returning success for this code path, publishBlock() now silently drops blocks that may be a different root (equivocation or failover mismatch) before the publish/import section runs, so the caller receives a false-positive success even though this block was neither broadcast nor processed. Consider only suppressing when the same block root is already known/in-flight, and keep a surfaced error for non-identical repeat proposals.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good point — worth discussing the tradeoff.
REPEAT_PROPOSAL does fire purely on (slot, proposerIndex) (seenBlockProposers.ts tracks no root), so the suppression is semantically wider than ALREADY_KNOWN. The cases it collapses:
-
Same root, race window. API publish arrives after gossip validation has run for the same block (adds to
seenBlockProposersatblock.ts:248) but before fork-choice import finishes.ALREADY_KNOWNatblock.ts:59returns null;REPEAT_PROPOSALatblock.ts:68fires. This is the case issuePOST eth/v2/beacon/blocksreturningBLOCK_ERROR_REPEAT_PROPOSALin multi-node setup #9228 describes. -
Distinct root (equivocation / failover mismatch / VC bug). Same (slot, proposer), different body.
In both cases the block is rejected by gossip validation before the publishBlock/import section — so nothing is broadcast or imported either way. The only observable difference for the caller is 500 vs 2xx.
Reliably distinguishing (1) from (2) would need SeenBlockProposers to track seen roots per (slot, proposer), which is a structural cache change beyond the scope of this targeted fix. The reason: error.type.code metadata now in the log (commit b68ea2b, per @gemini-code-assist suggestion) does surface the REPEAT_PROPOSAL case to operators so case (2) is still observable.
Happy to tighten if @nflaig prefers — options I see:
- (a) keep current behavior and rely on the
reasonlog field for observability (current PR), - (b) extend
SeenBlockProposerswith aseenRootsByProposer: Map<Slot, Map<ValidatorIndex, Set<RootHex>>>and only suppress whenseenRootsByProposer.get(slot)?.get(proposerIndex)?.has(blockRoot)is true, surfacing otherwise, - (c) keep current behavior but add a warn-level log for the REPEAT_PROPOSAL branch to make distinct-root re-submissions stand out in logs.
The previous "Ignoring known block" log was misleading for REPEAT_PROPOSAL, which covers a different-root block from the same slot/proposer. Include the error code in metadata so ALREADY_KNOWN vs REPEAT_PROPOSAL is distinguishable in logs. 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Motivation
Fixes #9228.
In multi-node setups (e.g. DVT clusters or fallback configurations), the same block is often submitted to multiple beacon nodes. When one node receives the block via gossip slightly before the
POST eth/v2/beacon/blocksREST call, the gossip validation raisesBLOCK_ERROR_REPEAT_PROPOSALfrom theseenBlockProposerscheck, which currently propagates as an unhandled exception and returns HTTP 500 to the caller. Nothing is actually wrong with the submitted block.Description
Extend the existing
ALREADY_KNOWNhandler inpublishBlockto also swallowREPEAT_PROPOSAL. Both are thrown asBlockGossipErrorwithGossipAction.IGNORE, so they share the same remediation — log at debug and return silently so the API caller sees a successful response.This mirrors the handler added in #6457 for
ALREADY_KNOWN.Only the
gossipbroadcast-validation path is affected;consensus/consensusAndEquivocationpaths useverifyBlocksInEpochand do not go throughvalidateGossipBlock, so they cannot hitREPEAT_PROPOSAL.No new tests added — the existing handler for
ALREADY_KNOWNwas introduced without tests (#6457), and the underlying validation error is already tested at the gossip layer intest/unit/chain/validation/block.test.ts.