feat: upgrade to alpha.8 spec - #9375
Conversation
| ): number { | ||
| const pubkey = state.getValidator(proposerIndex).pubkey; | ||
| const registeredGasLimit = chain.executionBuilder?.getValidatorRegistration(pubkey)?.gasLimit; | ||
| return registeredGasLimit ?? Number(state.latestExecutionPayloadBid.gasLimit); |
There was a problem hiding this comment.
this is obviously wrong, but we need proposer preferences to properly handle this and a unified cache
There was a problem hiding this comment.
Code Review
This pull request implements the Gloas fork, introducing updates to payload attributes, builder onboarding logic, and configuration parameters across the codebase. The reviewer identified a potential type mismatch in the getProposerTargetGasLimit function, noting that registeredGasLimit might be a bigint while the function expects a number, and suggested wrapping the return value in Number() to ensure type consistency.
| ): number { | ||
| const pubkey = state.getValidator(proposerIndex).pubkey; | ||
| const registeredGasLimit = chain.executionBuilder?.getValidatorRegistration(pubkey)?.gasLimit; | ||
| return registeredGasLimit ?? Number(state.latestExecutionPayloadBid.gasLimit); |
There was a problem hiding this comment.
The registeredGasLimit retrieved from the executionBuilder is likely a bigint (matching the UintNum64 type in SSZ), while the return type of this function is number. Directly using the nullish coalescing operator with a number fallback will result in a type mismatch error in TypeScript (bigint | number is not assignable to number). It is safer to wrap the entire expression in Number() to ensure a consistent return type.
| return registeredGasLimit ?? Number(state.latestExecutionPayloadBid.gasLimit); | |
| return Number(registeredGasLimit ?? state.latestExecutionPayloadBid.gasLimit); |
Performance Report✔️ no performance regression detected Full benchmark results
|
| function keepPending(deposit: CompositeViewDU<typeof ssz.electra.PendingDeposit>): void { | ||
| remainingPendingDeposits.push(deposit); | ||
| if ( | ||
| isValidDepositSignature( |
There was a problem hiding this comment.
this is inefficient, we should not eagerly validate pending deposit signature as specified in the spec https://github.com/ethereum/consensus-specs/pull/5254/changes#diff-e37d6d50b0282d164b6ec932ac0dcc33d02a13861b6c884a088eaced1436087bR87
addressed in #9374
|
We also need to update our code to actually use PAYLOAD_DUE_BPS instead of PAYLOAD_ATTESTATION_DUE_BPS |
**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>
a07e25c to
1d0e0b9
Compare
|
automatically closed now that glamsterdam-devnet-4 branch now == unstable branch |
No description provided.