fix: ignore parent bids at epoch boundary - #9756
Merged
Merged
Conversation
nflaig
marked this pull request as ready for review
August 3, 2026 11:12
Contributor
Performance Report鉁旓笍 no performance regression detected Full benchmark results
|
twoeths
reviewed
Aug 3, 2026
4 tasks
spiral-ladder
approved these changes
Aug 3, 2026
spiral-ladder
left a comment
spiral-ladder
left a comment
Member
There was a problem hiding this comment.
lgtm, spec deviation comment looks good
lodekeeper
reviewed
Aug 3, 2026
lodekeeper
left a comment
lodekeeper
left a comment
Contributor
There was a problem hiding this comment.
The epoch-boundary drop itself looks safe for Lodestar's local production path and avoids the expensive unusable parent-state regen. I found one adjacent-policy gap in the broader direct-parent compatibility branch.
| return true; | ||
| // The spec allows this at epoch boundaries, but Lodestar does not propagate these bids because validating | ||
| // them requires an epoch transition for a parent state that cannot be used for proposer-boost reorgs. | ||
| return !isStartSlotOfEpoch(bidSlot); |
Contributor
There was a problem hiding this comment.
馃煛 This direct-parent branch should also require bidSlot === head.slot + 1. Lodestar can only build on head.parentRoot for the single-slot proposer-boost reorg case; ForkChoice.getProposerHead() returns the parent only when head.slot + 1 === slot and the slot is not an epoch boundary. As written, a stale-head/skipped-slot bid, or a same-slot bid after a same-slot head is already cached, can pass validation and be pooled/propagated even though produceBlockV4 will never query this (slot, head.parentRoot, head.parentBlockHash) tuple. Tightening this to bidSlot === head.slot + 1 && !isStartSlotOfEpoch(bidSlot) would keep the validation policy aligned with the only direct-parent case block production can actually use.
Contributor
There was a problem hiding this comment.
Correction: after the propagation-goal clarification, I think this adjacency requirement is too strict for gossip validation. I was mixing Lodestar local selection with network propagation policy.
For this PR, the useful filter is the narrower one already here: drop the direct-parent case at epoch boundaries because it can force the expensive parent-state epoch-transition path for a bid Lodestar cannot use for proposer-boost reorg. Non-adjacent direct-parent bids may still be worth propagating to peers even if this node will not select them locally, so I consider my slot-adjacency suggestion retracted.
Member
Author
There was a problem hiding this comment.
this is somewhat true, we will only return getProposerHead() for head.parentRoot if bid.slot === head.slot + 1, but as lodekeeper pointed out, this is about bid propagation, I do think though that we could limit the bids we let through for head.parentRoot based on a few more cheap checks
Contributor
There was a problem hiding this comment.
Implemented this as a narrow cheap check without going back to strict local-selection adjacency: direct-parent bids now pass only when bid.slot is head.slot or head.slot + 1, plus the existing epoch-boundary ignore. That keeps same-slot/next-slot propagation useful for peers while rejecting stale or far-ahead parent-of-head bids before regen.
I could not push directly to ChainSafe/lodestar:nflaig/drop-parent-bids-at-epoch-boundary as lodekeeper (403), so the signed commit is available on my fork: lodekeeper@b2c7f70
Member
Author
There was a problem hiding this comment.
@lodekeeper if anything, this should be a separate PR, I have merged this one now
Contributor
There was a problem hiding this comment.
Ack, agreed. I won鈥檛 try to carry this as part of the already-merged PR.
I鈥檓 also not opening a separate PR from b2c7f70a03 right now: after the later propagation-policy clarification, the slot-adjacency tightening is not obviously desirable for gossip validation. It would align acceptance with what this local node can select, but it could also under-propagate cheap direct-parent bids that may still be useful to peers. So I鈥檓 parking that fork commit as optional follow-up material unless we decide we want the stricter local-policy behavior separately.
twoeths
approved these changes
Aug 4, 2026
This was referenced Aug 5, 2026
Member
|
馃帀 This PR is included in v1.46.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
5 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.
Follow-up to #9739
Ignore bids building on the head parent at epoch boundaries, since proposer-boost reorgs are disabled and validating them would regenerate an unusable parent state through the epoch transition.