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
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
2 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