Skip to content

fix: ignore parent bids at epoch boundary #9756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nflaig merged 4 commits into from
Aug 4, 2026
Merged

fix: ignore parent bids at epoch boundary #9756

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
06c8620
fix: ignore parent bids at epoch boundary
nflaig Aug 3, 2026
7fe6fd5
test: remove execution payload bid unit tests
nflaig Aug 3, 2026
135272d
refactor: handle epoch boundary in bid compatibility
nflaig Aug 3, 2026
9acfe6c
document epoch boundary bid spec divergence
nflaig Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/beacon-node/src/chain/validation/executionPayloadBid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
getExecutionPayloadBidSigningRoot,
isActiveBuilder,
isGasLimitTargetCompatible,
isStartSlotOfEpoch,
isStatePostGloas,
} from "@lodestar/state-transition";
import {RootHex, Slot, ValidatorIndex, gloas} from "@lodestar/types";
Expand Down Expand Up @@ -46,8 +47,8 @@ function getMinBidValue(currentHighestBid: number): number {
/**
* Check whether a bid builds on one of the paths compatible with the local head branch.
*
* The direct parent path is always allowed for proposer-boost reorgs. Otherwise the bid
* must build on the local head's full or empty payload variant, as selected for its slot.
* Building directly on the parent is allowed for proposer-boost reorgs outside epoch boundaries.
* Otherwise the bid must build on the local head's full or empty payload variant, as selected for its slot.
*/
function isBidCompatibleWithHead(
forkChoice: IForkChoice,
Expand All @@ -60,7 +61,9 @@ function isBidCompatibleWithHead(
const buildsOnParentPayload = bidParentBlockHash === head.parentBlockHash;

if (buildsOnParentBlock && buildsOnParentPayload) {
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);
Comment thread
nflaig marked this conversation as resolved.

@lodekeeper lodekeeper Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

@lodekeeper lodekeeper Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@nflaig nflaig Aug 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@lodekeeper lodekeeper Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@nflaig nflaig Aug 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lodekeeper if anything, this should be a separate PR, I have merged this one now

@lodekeeper lodekeeper Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, agreed. I won’t try to carry this as part of the already-merged PR.

I’m 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’m parking that fork commit as optional follow-up material unless we decide we want the stricter local-policy behavior separately.

}

if (bidParentBlockRoot !== head.blockRoot) {
Expand Down
Loading