fix: process REPEAT_PROPOSAL block - #9805
Merged
Merged
Conversation
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
twoeths
marked this pull request as ready for review
August 11, 2026 10:21
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9805 +/- ##
=========================================
Coverage 52.60% 52.60%
=========================================
Files 848 848
Lines 59996 59989 -7
Branches 4418 4418
=========================================
- Hits 31559 31558 -1
+ Misses 28378 28372 -6
Partials 59 59 🚀 New features to boost your workflow:
|
![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: aff7f38830
ℹ️ 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".
Comment on lines
+381
to
+382
| ...(skipPublish | ||
| ? [] |
There was a problem hiding this comment.
When skipPublish is true for a Fulu repeat proposal with blobs, this branch removes the beacon-block and data-column publish thunks, so sentPeersArr only contains the processBlock result. The Fulu metrics loop below still reads sentPeersArr[i + 1] for each data column and destructures it, which turns a successfully imported repeat-proposal publish into a TypeError instead of returning normally.
Useful? React with 👍 / 👎.
This was referenced Aug 12, 2026
nflaig
reviewed
Aug 12, 2026
| const blockInput = chain.seenBlockInputCache.get(e.type.root); | ||
| if (blockInput) { | ||
| chain.serializedCache.set(signedBlock, serializedData); | ||
| handleValidBeaconBlock(blockInput, peerIdStr, seenTimestampSec); |
Member
There was a problem hiding this comment.
I think we need to revisit the ordering in validateGossipBlock, so right now, if we throw a REPEAT_PROPOSAL that happens quite early, so the beacon block isn't fully validated here?
also it's before this check
if (blockState.getBeaconProposer(blockSlot) !== proposerIndex) {
throw new BlockGossipError(GossipAction.REJECT, {code: BlockErrorCode.INCORRECT_PROPOSER, proposerIndex});
}which seems questionable?
Member
Author
There was a problem hiding this comment.
I think the p2p rules are just quick checks to forward the gossip block without having to run state transition
if the block is really invalid, it should be caught inside the state transition itself
in this case it's
it's worth to mention this in the code
Member
There was a problem hiding this comment.
yeah that is probably fine was thinking the same way, but we should be more explicit about it, so adding a comment as suggested makes sense.
there was one worry that we could receive many invalid blocks and fill our cache but due to how gossip re-propagation works this should be unlikely unless a peer sends them directly to us, but I think we still wanna make sure that we at least validate the proposer signature in any case, although my worry here is that we run blockState.getBeaconProposer(blockSlot) !== proposerIndex later, so technically any validator can produce a valid signature (of course they would get slashed too if they do more >=2 so the incentives for doing so are low) and we downscore them, so it's likely fine
Member
Author
There was a problem hiding this comment.
but I think we still wanna make sure that we at least validate the proposer signature in any case
it's already checked at line 718 above
nflaig
approved these changes
Aug 13, 2026
nflaig
left a comment
nflaig
left a comment
Member
There was a problem hiding this comment.
LGTM, I can do the api cleanup to remove REPEAT_PROPOSAL handling separately
github-project-automation
Bot
moved this from In Progress
to Done
in Lodestar Team Coordination
nflaig
added a commit
that referenced
this pull request
Aug 17, 2026
Follow-up to #9805 and #9814 `REPEAT_PROPOSAL` now identifies a conflicting block root, but the publish block API still treated it like benign `ALREADY_KNOWN` and returned success. Remove that special case so repeat proposals use the existing gossip validation error path. Keep `ALREADY_KNOWN` benign and add focused API coverage for both cases.
nflaig
added a commit
that referenced
this pull request
Aug 31, 2026
Implement the proposer equivocation branch of `get_proposer_head` from ethereum/consensus-specs#4807, which reorgs a weak previous-slot head whose proposer is known to have equivocated: ```python elif all([head_weak, current_time_ok, proposer_equivocation]): return parent_node ``` This is the proposer side of the builder reveal safety argument that #9233 implemented the attester side of. A builder reveals once it sees a block with enough weight and no equivocation. If the proposer then publishes an equivocating block and the next proposer extends it, attesters withhold the boost via `should_apply_proposer_boost`, but for the equivocating block to actually be reorged the next proposer has to build on the parent, which is this branch. It lives in the phase0 spec so it applies pre-gloas as well. - `getProposerHead` returns the parent when the head is weak, from the previous slot and another block at the same slot from the same proposer is in fork choice, skipping the regular reorg conditions (`head_late`, `ffg_competitive`, `finalization_ok`, `proposing_on_time`, `parent_strong`). The boost worn off check stays a precondition for both branches, the spec asserts it before either - `shouldOverrideForkChoiceUpdate` mirrors the branch so the fcu skip at import and the payload attributes prediction in `prepareNextSlot` follow `getProposerHead`. It does not check `is_head_weak`, the head slot's attestations are still queued when it runs so the head is assumed weak, same as the regular branch - `hasEquivocatingBlock` takes a `ptcTimelyOnly` flag, `should_apply_proposer_boost` only counts PTC-timely siblings while `is_proposer_equivocation` counts any known block - the gloas parent payload status is preserved, `parentBlock` is already resolved via `getParentPayloadStatus` - link the `is_proposer_equivocation`, `should_apply_proposer_boost` and `record_block_timeliness` specrefs to their implementations and bump the `get_proposer_head` spec link, `v1.4.0-beta.4` predates this branch The existing check order and `notReorgedReason` values are unchanged, the new branch only runs ahead of them. Equivocating blocks reach fork choice since #9805, before that this could only trigger for blocks fetched via unknown block sync. There are no spec vectors for this branch upstream, coverage is via the `getProposerHead` and `shouldOverrideForkChoiceUpdate` unit tables. The four reorg cases fail on `unstable` and pass here, and the `get_proposer_head`, `reorg`, `should_override`, `ex_ante` and `should_apply_proposer_boost` fork choice spec tests still pass. Closes #9764
Member
|
🎉 This PR is included in v1.47.0 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
lodestar-geth-1ofglamsterdam-devnet-7, we saw 2 blocks of the same slot. The 2nd block got ignored, we ended up having to use UnknownBlockInput to download it again. And the 2nd block ended up being the canonical blockDescription
REPEAT_PROPOSALblock, but don't publish it to the network (to conform to the spec)part of #9799
AI Assistance Disclosure