feat: add proposer preferences#9377
Conversation
Implements beacon-APIs PR ethereum/beacon-APIs#593 for the gloas fork. Adds the operational ProposerPreferencesPool keyed (slot, dependent_root) and pruned per slot tick, plumbs it into BeaconChain, and exposes it via two new beacon API endpoints: GET /eth/v1/beacon/pool/proposer_preferences (slot-filtered) POST /eth/v1/beacon/pool/proposer_preferences (JSON + SSZ bodies) The POST handler validates each item via the existing gossip validator, adds to the pool, publishes through gossip (network.publishProposerPreferences), emits the new proposer_preferences SSE event, and reports per-item failures with persistInvalidSszValue on REJECT-class errors. The gossip handler mirrors the same persist-and-emit on a successfully validated incoming message. Also resolves the two // TODO GLOAS markers in validateExecutionPayloadBid: - IGNORE if no SignedProposerPreferences exists at (bid.slot, dependent_root), where dependent_root is derived from bid.parentBlockRoot via forkChoice.getAncestor(parentBlockRoot, computeStartSlotAtEpoch(bidEpoch) - 1). - REJECT if bid.fee_recipient != preferences.fee_recipient. - REJECT if bid.gas_limit != preferences.gas_limit. The parentBlockRoot in-fork-choice IGNORE was hoisted to the top of the validator so getAncestor can be called safely; its late copy is removed as unreachable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ProposerPreferencesService that runs every slot (gloas+) and submits
each local validator's SignedProposerPreferences within SLOTS_PER_EPOCH/4
slots of their proposal slot. Re-submits when the proposer dependent root
for an epoch shifts (reorg / dependent-root change), detected by comparing
the dependentRoot reported by BlockDutiesService against the one we last
submitted under. Submitted-slot tracking is updated only after the batch
POST succeeds so transient API failures retry naturally on the next tick.
Lifts BlockDutiesService ownership from BlockProposingService to validator.ts
so the new service can share the existing per-slot proposer-duty poll.
BlockDutiesService gains a setNotifyBlockProductionFn setter for late-binding
the block-production callback, and a public getProposersAtEpoch(epoch) getter
exposing the per-epoch {dependentRoot, data} cache.
Adds ValidatorStore.signProposerPreferences (mirrors signPayloadAttestation)
and SignableMessageType.PROPOSER_PREFERENCES (enum, type union,
requiresForkInfo, serialization switch) for remote-signer support.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements the Proposer Preferences feature for the Gloas fork, introducing a ProposerPreferencesPool, adding REST API endpoints for preference management, and integrating validation into the execution payload bid process. Feedback identifies a potential DoS vulnerability due to missing length limits on preference lists and a possible crash during ancestor lookups for epoch 0. Additionally, it is recommended to use bigint for gas limit comparisons and error types to prevent precision loss.
| const SyncCommitteeMessageListType = ArrayOf(ssz.altair.SyncCommitteeMessage); | ||
| const PayloadAttestationListType = ArrayOf(ssz.gloas.PayloadAttestation, MAX_PAYLOAD_ATTESTATIONS); | ||
| const PayloadAttestationMessageListType = ArrayOf(ssz.gloas.PayloadAttestationMessage, PTC_SIZE); | ||
| const SignedProposerPreferencesListType = ArrayOf(ssz.gloas.SignedProposerPreferences); |
There was a problem hiding this comment.
The SignedProposerPreferencesListType is defined as an array without a length limit. This could potentially lead to Denial of Service (DoS) attacks if a client submits an excessively large list in the submitSignedProposerPreferences endpoint. It is recommended to add a reasonable limit to the array definition to bound resource consumption during deserialization and processing.
There was a problem hiding this comment.
not super high priority but we can cap 2 * SLOTS_PER_EPOCH given proposers are known 2 epochs ahead
for the Pool.get() api, it could be higher during unfinality time, so make it 3 * SLOTS_PER_EPOCH would be enough
| const bidEpoch = computeEpochAtSlot(bid.slot); | ||
| const dependentRootHex = chain.forkChoice.getAncestor( | ||
| parentBlockRootHex, | ||
| computeStartSlotAtEpoch(bidEpoch) - 1 | ||
| ).blockRoot; | ||
| const proposerPreferences = chain.proposerPreferencesPool.get(bid.slot, dependentRootHex); |
There was a problem hiding this comment.
The call to chain.forkChoice.getAncestor uses computeStartSlotAtEpoch(bidEpoch) - 1 as the target slot. If bidEpoch is 0, this results in a slot of -1, which will cause getAncestor to throw an error and potentially crash the gossip validation task. Although Gloas is a post-genesis fork, handling the bidEpoch === 0 case (e.g., by using slot 0) ensures robustness in all configurations, including devnets.
const bidEpoch = computeEpochAtSlot(bid.slot);
const dependentRootSlot = computeStartSlotAtEpoch(bidEpoch);
const dependentRootHex = chain.forkChoice.getAncestor(
parentBlockRootHex,
dependentRootSlot > 0 ? dependentRootSlot - 1 : 0
).blockRoot;
const proposerPreferences = chain.proposerPreferencesPool.get(bid.slot, dependentRootHex);| // [REJECT] `bid.gas_limit == proposer_preferences.gas_limit`. | ||
| // Both compared against the matching `proposer_preferences` defined above (same branch | ||
| // via dependent_root, same proposal_slot). | ||
| // TODO GLOAS: Implement once a ProposerPreferencesPool exists. | ||
| const bidGasLimit = Number(bid.gasLimit); | ||
| if (bidGasLimit !== proposerPreferences.message.gasLimit) { | ||
| throw new ExecutionPayloadBidError(GossipAction.REJECT, { | ||
| code: ExecutionPayloadBidErrorCode.PROPOSER_PREFERENCES_GAS_LIMIT_MISMATCH, | ||
| builderIndex: bid.builderIndex, | ||
| bidGasLimit, | ||
| expectedGasLimit: proposerPreferences.message.gasLimit, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Converting bid.gasLimit (a bigint) to a number using Number() can lead to precision loss if the value exceeds Number.MAX_SAFE_INTEGER. It is safer to perform the comparison and error reporting using bigint values throughout.
// [REJECT] `bid.gas_limit == proposer_preferences.gas_limit`.
if (bid.gasLimit !== BigInt(proposerPreferences.message.gasLimit)) {
throw new ExecutionPayloadBidError(GossipAction.REJECT, {
code: ExecutionPayloadBidErrorCode.PROPOSER_PREFERENCES_GAS_LIMIT_MISMATCH,
builderIndex: bid.builderIndex,
bidGasLimit: bid.gasLimit,
expectedGasLimit: BigInt(proposerPreferences.message.gasLimit),
});
}| bidGasLimit: number; | ||
| expectedGasLimit: number; |
There was a problem hiding this comment.
The bidGasLimit and expectedGasLimit fields in the error type should use bigint instead of number to accurately represent the full range of potential gas limit values and avoid precision issues during error reporting.
| bidGasLimit: number; | |
| expectedGasLimit: number; | |
| bidGasLimit: bigint; | |
| expectedGasLimit: bigint; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a623c19d28
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.submitted.set(epoch, submission); | ||
| } | ||
|
|
||
| const dependentRootBytes = fromHex(dutiesAtEpoch.dependentRoot); |
There was a problem hiding this comment.
Use the Gloas proposer dependent root
In post-Fulu/Gloas, this value is sourced from the shared BlockDutiesService cache, which is populated by the v1 getProposerDuties call (packages/validator/src/services/blockDuties.ts:202). The beacon-node implementation explicitly returns a different dependent root only for v2 after Fulu (packages/beacon-node/src/api/impl/validator/index.ts:1219-1222), while proposer-preferences validation checks currentDecisionRoot/nextDecisionRoot. As a result, Lodestar validators sign preferences with the legacy dependent root and their own BN ignores them, leaving no matching preferences for execution payload bids.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
good catch. The api is already correct, we should change the p2p validation to use getDependentRoot(block: ProtoBlock, epochDifference: EpochDifference): RootHex { instead
| const dependentRootBytes = fromHex(dutiesAtEpoch.dependentRoot); | ||
|
|
||
| for (const duty of dutiesAtEpoch.data) { | ||
| if (duty.slot <= slot) continue; |
There was a problem hiding this comment.
Submit after late first-slot duties arrive
For validators assigned to the first slot of an epoch, this service only runs at slot boundaries. The next-epoch duties are populated by BlockDutiesService.pollBeaconProposersNextEpoch one second before the epoch transition (packages/validator/src/services/blockDuties.ts:149-153 and :190-193), after the last-slot tick of this loop has already skipped the missing duties; the next tick is the proposal slot itself, where this line drops the duty. No SignedProposerPreferences is ever submitted for that first-slot proposal, so execution payload bids for that slot are ignored for lack of matching preferences.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
good catch, we have a bug here
starting from fulu, we should be able to pool block duties 1 epoch ahead
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
| // root for the bid's branch and epoch. Derived via fork-choice ancestor lookup: the block at the | ||
| // last slot of `epoch - 1` reached from `bid.parent_block_root`. | ||
| const bidEpoch = computeEpochAtSlot(bid.slot); | ||
| const dependentRootHex = chain.forkChoice.getAncestor( |
There was a problem hiding this comment.
this is not correct according to https://github.com/ethereum/consensus-specs/blob/4a4937bea332d72a55a76aaebcb97fbcdc189f69/specs/gloas/p2p-interface.md?plain=1#L447
def get_proposer_dependent_root(state: BeaconState, epoch: Epoch) -> Root:
"""
Return the dependent root for the proposer lookahead at ``epoch``.
"""
return get_block_root_at_slot(
state, Slot(compute_start_slot_at_epoch(Epoch(epoch - MIN_SEED_LOOKAHEAD)) - 1)
)There was a problem hiding this comment.
need to handle differently at epoch boundary too
twoeths
left a comment
There was a problem hiding this comment.
looks like we can just use the ProposerPreferencesPool in this PR instead of the pre-existing SeenProposerPreferences
something to cleanup later
# Summary Fixes Lodestar's handling of proposer duties under the post-Fulu (EIP-7917) deterministic 1-epoch proposer lookahead. Surfaced while reviewing #9377: the validator was never querying `currentEpoch + 1` proposer duties post-Fulu, and the BN's dep_root computation was wrong when serving duties for an epoch other than `state.epoch`. Contains two related stories: 1. **BN-side bug fixes + lookahead support** so `getProposerDutiesV2` correctly serves `currentEpoch + 1` (and `currentEpoch + 2` near the boundary). 2. **Validator-side refactor** to consume that lookahead through an event-driven model that mirrors `AttestationDutiesService`, instead of per-slot polling. # BN side **`proposerShufflingDecisionRoot` bug fix** (`state-transition/src/util/shuffling.ts`) Previously derived the decision slot from `state.epoch`, which gave the wrong dep_root whenever the state was one epoch off the requested epoch (e.g. serving `state.epoch + 1` duties from the head state). Now takes `proposalEpoch` explicitly: - Pre-Fulu: `dep_root(E) = block@(startSlot(E) - 1)` — unchanged - Post-Fulu (MIN_SEED_LOOKAHEAD = 1): `dep_root(E) = block@(startSlot(E - 1) - 1)` — shifted back one epoch **`getProposerDuties` (`beacon-node/src/api/impl/validator/index.ts`)** Allows `epoch === currentEpoch + 2` near the next-epoch boundary post-Fulu. The duties are served from the upcoming-epoch (`currentEpoch + 1`) checkpoint state's `nextProposers`, which is populated by the `proposer_lookahead` field. The existing `nearNextEpoch` gate (`msToNextEpoch < prepareNextSlotLookAheadMs`) determines availability. # Validator side Original draft of this PR added a fork-aware `pollBeaconProposers` that, post-Fulu, polled `nextEpoch` every slot and `nextEpoch + nextEpoch+1` at the boundary. That was functional but raised a fair concern in review: *why fetch two epochs at the boundary, and why poll next-epoch every slot if its dep_root is stable post-Fulu?* The refactor (`refactor(validator): event-driven proposer duties via SSE head events`) replaces that with an attester-style model: | Trigger | Action | |---------|--------| | `clock.runEveryEpoch(epoch)` | Fetch `epoch` (+ `epoch + 1` post-Fulu, using the EIP-7917 lookahead) | | `chainHeaderTracker.runOnNewHead(headEvent)` | Compare incoming dep_roots against cache; refetch only the affected epoch on mismatch | | `clock.runEverySlot(slot)` | Notify block production from cache; pre-Fulu only — schedule the 1s-before-boundary fetch for `nextEpoch` (its dep_root only stabilizes at the boundary and isn't exposed via SSE) | The SSE head event already carries everything needed for both forks via a nice coincidence in the dep_root math: - **Pre-Fulu:** `currentDutyDependentRoot ≡ proposer_dep_root(currentEpoch)` - **Post-Fulu:** `previousDutyDependentRoot ≡ proposer_dep_root(currentEpoch)`, `currentDutyDependentRoot ≡ proposer_dep_root(nextEpoch)` No spec/event changes required — the same fields the validator already uses for attester duties cover the post-Fulu proposer lookahead window. A per-slot notification dedup (`notifiedSlot` / `notifiedProposers`) replaces the old "two-pass with `differenceHex`" pattern so any source of cache update (SSE refetch, cold-cache back-fill, epoch tick) only notifies *newly discovered* proposers and never duplicates `createAndPublishBlock` calls. # Results In steady state, the validator now makes **2 proposer-duty calls per epoch** (current + next epoch pre-fetch) plus refetches only on dep_root changes — matching the per-epoch cadence of `AttestationDutiesService` (which previously had been 32× more frequent). # Tests - 11 new `BlockDutiesService` unit tests covering: post-Fulu pre-fetch of next epoch, pre-Fulu vs post-Fulu fork detection, SSE-driven refetch on dep_root mismatch, no-op on dep_root match, cold-cache back-fill, pre-Fulu boundary scheduling + post-Fulu suppression, signer removal across epochs. - BN-side `getProposerDuties` tests updated to exercise the V2 path with a post-Fulu config. - E2E tests verified: `proposerBoostReorg`, `finalizedSync`, `checkpointSync` (Fulu fork crossings, reorgs, checkpoint sync) — all pass, all 30+ block proposals fire correctly, no new errors. # Known follow-ups (non-blocking) 1. **Genesis-state dep_root quirk (BN-side, cosmetic).** At very early genesis, the BN's `getProposerDuties` returns `genesisBlockRoot` via the `state.slot === decisionSlot` fallback, but later returns `state.getBlockRootAtSlot(0)` for the same epoch — they're cosmetically different roots for the same logical block. The old code didn't observe this because it didn't pre-fetch `nextEpoch` until ~1s before the boundary; the new code pre-fetches at the start of epoch 0 and sees one or two spurious `Proposer duties re-org` warnings per VC at startup. Duties are correct — pure metric noise. Worth a small BN-side normalization or a "skip pre-fetch on first epoch tick" guard. 2. **Concurrent `pollBeaconProposers` race.** If `onNewHead` and `runEveryEpochTask` race on the same epoch with asymmetric HTTP latencies, last-write-wins can briefly leave a stale dep_root cached. In practice the same BN serves both calls and returns identical payloads. Documented in a code comment; a per-epoch sequence number would harden it if it ever becomes a real problem. 3. **Gloas timing.** `BLOCK_DUTIES_LOOKAHEAD_BPS` may want to flip from "1s before the boundary" to "1s after" post-Gloas. Existing `TODO GLOAS: re-evaluate timing` is preserved. # AI disclosure Refactor designed and implemented with AI assistance. --------- Co-authored-by: Cayman <caymannava@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com>
**Motivation** Upgrade Lodestar to the `v1.7.0-alpha.8` following #9375 **What's changed since #9375** - consume ProposerPreferencesPool in #9377 - use `PAYLOAD_DUE_BPS` instead of `PAYLOAD_ATTESTATION_DUE_BPS` - the onboard builder is implemented in #9374, reenable spec tests **Detailed Description** - Bump `spec-tests-version.json` to `v1.7.0-alpha.8` and apply the matching `specrefs/*` updates. - Config: `MIN_BUILDER_WITHDRAWABILITY_DELAY` `64 → 8192`; add `PAYLOAD_DUE_BPS` (mainnet/minimal/types + validator critical params). - Add Gloas `targetGasLimit` to `PayloadAttributes` (SSZ, execution-engine `PayloadAttributes`/RPC + serialize/deserialize). - Rename `ProposerPreferences.gasLimit → targetGasLimit` (alpha.8) and update the unstable-only consumers not present on the #9375 branch: `validatorStore.signProposerPreferences`, gossip `validateExecutionPayloadBid`, and test/event fixtures. The gossip bid-validation rule keeps strict equality (rename only); `is_gas_limit_target_compatible` is a separate follow-up. - `upgradeStateToGloas`: set `latestExecutionPayloadBid.gasLimit` from the Fulu header and bump the spec-comment URL. The existing `onboardBuildersFromPendingDeposits` is already spec-equivalent and is left as-is; the previously-skipped `fork_invalid_validator_deposit_followed_by_builder_credentials` spec test is re-enabled and passes. - `produceBlockBody`: resolve the Gloas payload-attributes `targetGasLimit` from the `ProposerPreferencesPool` (same `(slot, dependent_root)` lookup as bid validation), falling back to the parent payload gas limit when no preferences are pooled. Addresses the #9375 review note that the builder-registration source was incorrect. - Add `getPayloadDueMs()` to `forkConfig` (spec `get_payload_due_ms`, `PAYLOAD_DUE_BPS`) and gate `producePayloadAttestationData`'s `payloadPresent` on the execution payload envelope being seen before that deadline (uses the envelope's own arrival time). Addresses the #9375 review note about using `PAYLOAD_DUE_BPS` instead of `PAYLOAD_ATTESTATION_DUE_BPS`. - Skip the new `gloas/fork_choice/on_payload_attestation_message` spec suite (PTC fork choice not yet implemented). **AI Assistance Disclosure** Used Claude Code to port and adapt the changes, address the PR review comments, and run verification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com> Co-authored-by: Cayman <caymannava@gmail.com>
Motivation
Description
SLOTS_PER_EPOCH / 4slots before proposal)AI Assistance Disclosure