fix(fork-choice): thread proposerBoostRoot into getPayloadStatusTiebreaker - #8944
Conversation
…eaker `getPayloadStatusTiebreaker` calls `shouldExtendPayload` to decide whether to prefer FULL over EMPTY for a previous-slot block. However, `proposerBoostRoot` was hardcoded to `null` at every call site inside `maybeUpdateBestChildAndDescendant`, causing `shouldExtendPayload` to always short-circuit on the "no proposer boost" condition and return `true` — making FULL always win the tiebreaker regardless of PTC votes. This breaks the core ePBS invariant: when a payload is not timely (insufficient PTC votes and proposer boost applies), the chain should extend via the EMPTY variant, not FULL. Fix by threading `proposerBoostRoot` through the call chain: - `maybeUpdateBestChildAndDescendant` now accepts `proposerBoostRoot` - `applyScoreChanges` passes `proposerBoost?.root ?? null` - `onBlock` and `onExecutionPayload` in ProtoArray accept and forward it - `ForkChoice` passes `this.proposerBoostRoot` to both ProtoArray methods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello @twoeths, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a significant bug within the Gloas (ePBS) fork choice mechanism. Previously, a hardcoded Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request correctly addresses a bug in the ePBS fork choice by threading the proposerBoostRoot through the call chain to ensure it is available for the payload status tie-breaker logic. This fix is essential for maintaining the invariant that the chain should extend via the EMPTY variant when a payload is not timely and a child block with proposer boost extends that EMPTY variant.
However, the signature changes in ProtoArray.onBlock, ProtoArray.onExecutionPayload, and ProtoArray.maybeUpdateBestChildAndDescendant introduce breaking changes for existing call sites. Specifically, the unit tests in packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts and packages/fork-choice/test/unit/protoArray/protoArray.test.ts (as seen in the provided file context) still call these methods without the new required parameter, which will lead to compilation errors. Providing a default value of null for the new proposerBoostRoot parameter would maintain backward compatibility and resolve these issues.
Pass null for proposerBoostRoot at the three protoArray.onBlock() calls in the ProtoArray initialization path — no proposer boost applies during initialization from stored state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l sites in tests Pass null for proposerBoostRoot at all protoArray.onBlock() and protoArray.onExecutionPayload() call sites in test and perf files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## nc/epbs-fc #8944 +/- ##
===========================================
Coverage 52.37% 52.37%
===========================================
Files 848 848
Lines 63198 63190 -8
Branches 4676 4676
===========================================
- Hits 33098 33094 -4
+ Misses 30032 30028 -4
Partials 68 68 🚀 New features to boost your workflow:
|
Summary
Fixes a bug in the Gloas (ePBS) fork choice where
proposerBoostRootwas hardcoded tonullat every call site ofgetPayloadStatusTiebreakerinsidemaybeUpdateBestChildAndDescendant.Root cause:
shouldExtendPayloadchecks four conditions in order. Condition 2 isproposerBoostRoot === null → return true. Withnullalways passed, FULL always wins the EMPTY vs FULL tiebreaker for previous-slot blocks, regardless of PTC votes. This breaks the core ePBS invariant: when a payload is not timely, the chain should extend via EMPTY.Fix: Thread
proposerBoostRootexplicitly through the call chain:maybeUpdateBestChildAndDescendant(parentIndex, childIndex, currentSlot, proposerBoostRoot)applyScoreChanges→ passesproposerBoost?.root ?? nullProtoArray.onBlockandProtoArray.onExecutionPayload→ accept and forward the paramForkChoice.onBlockandForkChoice.onExecutionPayload→ passthis.proposerBoostRootThe
getPayloadStatusTiebreakerbody (return shouldExtend ? 2 : 0) is already spec-correct and is unchanged.Test plan
pnpm vitest run packages/fork-choice)gloas.test.tsthat sets aproposerBoostRootand verifies EMPTY is preferred whenshouldExtendPayloadreturns false (payload not timely, proposer boost applies to this block)🤖 Generated with Claude Code
cc @ensi321 @nflaig