fix: derive proposer duties dependent root from state#9498
Conversation
![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: 936d2d15cc
ℹ️ 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".
| const dependentRoot = | ||
| // In v2 the dependent root is different after fulu due to deterministic proposer lookahead | ||
| proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state, epoch) || | ||
| proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state) || |
There was a problem hiding this comment.
When v2 post-Fulu duties for epoch N+1 are served from a state at epoch N, this call now derives the root from state.epoch and returns block_root(start(N-1)-1). Those duties come from state.nextProposers, which is keyed by state.nextDecisionRoot / the v2 spec root block_root(start(N)-1); the Gloas proposer-preference validator also checks headState.nextDecisionRoot for proposalEpoch - 1. As a result, validator clients that poll next-epoch v2 duties will sign preferences with a dependent root the beacon node rejects as unknown.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the proposer shuffling decision root calculation by removing the proposalEpoch parameter and relying on state.epoch instead, and adds corresponding unit tests. The reviewer points out that removing epoch introduces a bug when requesting proposer duties for the previous epoch (currentEpoch - 1), where state.epoch is currentEpoch, leading to an incorrect dependent root. The reviewer suggests keeping the proposalEpoch parameter and capping the computed decision slot if it is in the future to resolve the mid-epoch next-epoch future slot issue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const dependentRoot = | ||
| // In v2 the dependent root is different after fulu due to deterministic proposer lookahead | ||
| proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state, epoch) || | ||
| proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state) || |
There was a problem hiding this comment.
By removing epoch from proposerShufflingDecisionRoot and relying solely on state.epoch, we introduce a bug when requesting proposer duties for the previous epoch (currentEpoch - 1). In that case, state.epoch is currentEpoch, so the returned dependent root will incorrectly correspond to the current epoch instead of the previous epoch. We should keep passing epoch here and handle the mid-epoch next-epoch future slot issue inside proposerShufflingDecisionRoot by capping the decision slot.
| proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state) || | |
| proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state, epoch) || |
| * Block root that decided the proposer shuffling for the state used to serve proposer duties. | ||
| * | ||
| * Returns `null` when the genesis block decides its own shuffling (caller falls back to the | ||
| * genesis block root). | ||
| */ | ||
| export function proposerShufflingDecisionRoot( | ||
| fork: ForkName, | ||
| state: IBeaconStateView, | ||
| proposalEpoch: Epoch | ||
| ): Root | null { | ||
| const decisionSlot = proposerShufflingDecisionSlot(fork, proposalEpoch); | ||
| export function proposerShufflingDecisionRoot(fork: ForkName, state: IBeaconStateView): Root | null { | ||
| const decisionSlot = proposerShufflingDecisionSlot(fork, state); | ||
| if (state.slot === decisionSlot) { | ||
| return null; | ||
| } | ||
| return state.getBlockRootAtSlot(decisionSlot); | ||
| } | ||
|
|
||
| /** Slot whose block root keys the proposer shuffling for `proposalEpoch`. */ | ||
| function proposerShufflingDecisionSlot(fork: ForkName, proposalEpoch: Epoch): Slot { | ||
| /** Slot whose block root keys the proposer shuffling for `state`. */ | ||
| function proposerShufflingDecisionSlot(fork: ForkName, state: IBeaconStateView): Slot { | ||
| // Post-Fulu the shuffling is decided one epoch earlier (deterministic proposer lookahead, | ||
| // MIN_SEED_LOOKAHEAD = 1); pre-Fulu it is the last block before `proposalEpoch`. | ||
| const decisionEpoch = isForkPostFulu(fork) ? proposalEpoch - 1 : proposalEpoch; | ||
| // MIN_SEED_LOOKAHEAD = 1); pre-Fulu it is the last block before `state.epoch`. | ||
| const decisionEpoch = isForkPostFulu(fork) ? state.epoch - 1 : state.epoch; | ||
| return Math.max(computeStartSlotAtEpoch(decisionEpoch) - 1, 0); | ||
| } |
There was a problem hiding this comment.
To prevent returning the wrong dependent root for previous epoch requests (where state.epoch is currentEpoch but the requested epoch is currentEpoch - 1), we should keep proposalEpoch in the signature. To resolve the original issue where requesting the next epoch mid-epoch pre-Fulu attempts to read a future block root, we can cap the computed decisionSlot to the last slot of the state's current epoch if it is in the future.
* Block root that decided the proposer shuffling for `proposalEpoch` (keys that shuffling).
* Computed for the requested epoch, not `state.epoch`, so it is correct when `state` is one
* epoch off the requested epoch (e.g. serving next-epoch duties from the current state).
*
* Returns `null` when the genesis block decides its own shuffling (caller falls back to the
* genesis block root).
*/
export function proposerShufflingDecisionRoot(
fork: ForkName,
state: IBeaconStateView,
proposalEpoch: Epoch
): Root | null {
const decisionSlot = proposerShufflingDecisionSlot(fork, proposalEpoch, state.slot);
if (state.slot === decisionSlot) {
return null;
}
return state.getBlockRootAtSlot(decisionSlot);
}
/** Slot whose block root keys the proposer shuffling for `proposalEpoch`. */
function proposerShufflingDecisionSlot(fork: ForkName, proposalEpoch: Epoch, currentSlot: Slot): Slot {
// Post-Fulu the shuffling is decided one epoch earlier (deterministic proposer lookahead,
// MIN_SEED_LOOKAHEAD = 1); pre-Fulu it is the last block before `proposalEpoch`.
const decisionEpoch = isForkPostFulu(fork) ? proposalEpoch - 1 : proposalEpoch;
const decisionSlot = Math.max(computeStartSlotAtEpoch(decisionEpoch) - 1, 0);
if (decisionSlot >= currentSlot) {
const currentEpoch = Math.floor(currentSlot / SLOTS_PER_EPOCH);
return Math.max(computeStartSlotAtEpoch(currentEpoch) - 1, 0);
}
return decisionSlot;
}There was a problem hiding this comment.
Closing #9498 — #9499 takes the same direction your suggestion pointed at (keep proposalEpoch, handle the future-decision-slot case at the helper boundary) but adopts the Lighthouse-style head-root fallback rather than a cap, matching the beacon-APIs V1 event.block rule for that case. Genesis edge case is preserved via a Root | null return + getGenesisBlockRoot(state) fallback at the call site (per Codex's P2 on #9499).
|
Closing — superseded by #9499. Both Codex ( #9499 takes the Lighthouse-style approach: keep |
1 participant
Motivation
GET /eth/v1/validator/duties/proposer/{epoch}can be called for the next epoch while the node is still mid-epoch. After #9380,proposerShufflingDecisionRoot()computed the dependent root from the requested proposal epoch. For a v1 next-epoch request this points atstart(nextEpoch) - 1, which is still in the future for most of the current epoch and tripsgetBlockRootAtSlot()with:This showed up from Vero polling proposer duties against a Lodestar BN. Lodestar VC uses the v2 route post-Fulu, but the BN should serve the v1 next-epoch route without a 500 too.
Changes
state.epoch; near-boundarycurrentEpoch + 2v2 duties are still served from the checkpoint state advanced tonextEpoch.start(nextEpoch) - 1.Spec / client audit
proposer_lookaheadstarts from the beginning of the state current epoch and coversMIN_SEED_LOOKAHEAD + 1epochs.process_proposer_lookahead()computes the newly appended epoch at the epoch transition, so the state epoch is the right anchor for the dependent root of the proposer list being returned.Verification
pnpm buildpnpm vitest run --project unit packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.tspnpm check-typespnpm lintAuthored with AI assistance (OpenAI Codex / Lodekeeper).