feat: only give proposer boost to canonical proposer#9313
feat: only give proposer boost to canonical proposer#9313
Conversation
Per consensus-specs PR #4807, `update_proposer_boost_root` now only sets `proposer_boost_root` when the imported block's `proposer_index` matches the head state's expected proposer for the slot, so a divergent proposer shuffling on a non-canonical fork cannot nullify proposer boost. `onBlock` gains an `expectedProposerIndex` argument; `importBlock` resolves it via `headState.currentProposers` / `nextProposers` (O(1), backed by `state.proposerLookahead` post-fulu) — same fast path used by the proposer-preferences gossip validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the onBlock method in the fork choice implementation to include an expectedProposerIndex parameter, which is used to restrict proposer boost to blocks from the expected proposer. Changes include updating the IForkChoice interface, the importBlock logic, and relevant tests. A review comment suggests refactoring the logic for determining the expected proposer index to avoid duplication between the main code and test utilities.
| let expectedProposerIndex: number | null = null; | ||
| const headState = this.getHeadState(); | ||
| if (headState.epoch === blockEpoch) { | ||
| expectedProposerIndex = headState.currentProposers[blockSlot % SLOTS_PER_EPOCH]; | ||
| } else if (headState.epoch + 1 === blockEpoch) { | ||
| expectedProposerIndex = headState.nextProposers[blockSlot % SLOTS_PER_EPOCH]; | ||
| } |
There was a problem hiding this comment.
The logic for determining the expected proposer index from the head state is duplicated in packages/beacon-node/test/spec/utils/gossipValidation.ts. Consider encapsulating this logic into a shared helper function or using the existing state.getBeaconProposer(slot) method with appropriate error handling. This would improve maintainability and ensure consistency across the codebase when resolving proposers from a given state.
Performance Report✔️ no performance regression detected Full benchmark results
|
The retroactive proposer-boost spec change from v1.7.0-alpha.1 is now implemented, so `voting_source_beyond_two_epoch`, `justified_update_always_if_better`, and `justified_update_not_realized_finality` pass across all forks (altair → gloas, 21 fixtures total). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| executionStatus, | ||
| dataAvailabilityStatus | ||
| dataAvailabilityStatus, | ||
| orphanedBlock.message.proposerIndex |
There was a problem hiding this comment.
seems misleading because this requires an "expected proposer index" in terms of head view, may pass null instead?
same to others
|
|
||
| let expectedProposerIndex: number | null = null; | ||
| const headState = this.getHeadState(); | ||
| if (headState.epoch === blockEpoch) { |
There was a problem hiding this comment.
we also have BeaconStateView.getBeaconProposer() but it only query from current epoch
looks like a good chance to clean that up
ideally we should be able to have this logic once
closes #9231