Skip to content

fix: recompute head before proposer boost dependent root check #9813

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

Closed
ensi321 wants to merge 2 commits into from
Closed

fix: recompute head before proposer boost dependent root check #9813

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74a44de
fix: recompute head before proposer boost dependent root check
ensi321 Aug 12, 2026
68dfd76
Merge branch 'unstable' into nc/proposer-boost-stale-head
ensi321 Aug 17, 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
17 changes: 13 additions & 4 deletions packages/fork-choice/src/forkChoice/forkChoice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,11 @@ export class ForkChoice implements IForkChoice {
* Run the fork choice rule to determine the head.
* Update the head cache.
*
* Very expensive function (400ms / run as of Aug 2021). Call when the head really needs to be re-calculated.
* Cost is dominated by the `computeDeltas()` scan over every validator, so it is flat no matter how
* many votes actually changed: ~4.6ms at 1M validators, ~9.5ms at 2M, plus ~30% if the proto-array
* has grown to a day of non-finality (measured Aug 2026 on a M3 Mac, see
* `test/perf/forkChoice/updateHead.test.ts`). Only call when the head really needs to be
* re-calculated.
*
* ## Specification
*
Expand Down Expand Up @@ -759,12 +763,17 @@ export class ForkChoice implements IForkChoice {
// The store field `this.proposerBoostRoot` and `updateCheckpoints()` are mutated only after
// `protoArray.onBlock()` succeeds
const isTimely = this.isBlockTimely(block, blockDelaySec);
const isProposerBoostBlock =
const isProposerBoostCandidate =
this.opts?.proposerBoost === true &&
isTimely &&
// only boost the first block we see
this.proposerBoostRoot === null &&
this.isProposerBoostSameDependentRoot(this.head.blockRoot, parentRootHex);
this.proposerBoostRoot === null;
const isProposerBoostBlock =
isProposerBoostCandidate &&
// Cached `this.head` may be stale especially after epoch transition.
// Need to updateHead() here to get the correct head
// This code only executes when block is timely. So minimal exposure to minor regression
this.isProposerBoostSameDependentRoot(this.updateHead().blockRoot, parentRootHex);

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add regression coverage for stale cached heads

Add a targeted test where an epoch transition changes fork-choice weights while this.head still points to a branch with a different shuffling-dependent root, then verify that the first timely block receives proposer boost based on the recomputed head. This consensus-critical bug fix changes only production code, so the stale-cache scenario can regress without a focused test despite the repository requiring a failing regression test for bug fixes.

AGENTS.md reference: AGENTS.md:L377-L382

Useful? React with 👍 / 👎.

@ensi321 ensi321 Aug 17, 2026 •
edited
Loading

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.

We have already conducted test and the performance impact is minimal as noted in the code comment

@nflaig nflaig Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

performance impact is minimal

@ensi321 I am surprised by this, I don't see this can be minimal, it should have a measurable impact I would expect this to affect our block processing time

image

did we run this on a live mainnet node, or how was the evaluated?

Comment on lines +775 to +776

@nflaig nflaig Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This code only executes when block is timely. So minimal exposure to minor regression

this comment is really misleading, when block is timely is 99.9% of blocks on mainnet, so this is very large exposure, with this change we run fork choice computation twice every slot?

the case where isProposerBoostSameDependentRoot() would produce a incorrect result seems very narrow to me, you would need two branches (A, B) have different dependent roots, meaning those have diverged more than an epoch ago, and our head needs to be on branch A before epoch tick, but branch B after epoch processing has a newer unrealized justified checkpoint so our head should be B, then first timely block extends branch B. But our head is still on branch A when we receive that block on branch B, meaning we incorrectly deny proposer boost for the timely block on branch B.

give the above, there are several pre-conditions for this to even be relevant, we might be able to narrow this down so we only need to re-compute our head if it's really necessary

@nflaig nflaig Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ensi321 ensi321 Aug 20, 2026 •
edited
Loading

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.

you are right comment is bad

// Candidate boost root used for protoArray.onBlock's best-child weighting. Committed to the
// store only after the insertion succeeds.
const proposerBoostRoot = isProposerBoostBlock ? blockRootHex : this.proposerBoostRoot;
Expand Down
Loading