cl: implement consensus-specs v1.7.0-alpha.8 for Gloas devnet-4#21308
cl: implement consensus-specs v1.7.0-alpha.8 for Gloas devnet-4#21308domiwei wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades Erigon’s consensus-layer implementation and spectest fixtures to consensus-specs v1.7.0-alpha.8 for Gloas/Fulu devnet-4 readiness, adding new req/resp protocols, fork-choice updates, and ePBS-related fields/validations (notably target_gas_limit).
Changes:
- Upgrade consensus-specs spectest fixtures to v1.7.0-alpha.8.
- Extend EL/Engine API + block builder plumbing to carry
targetGasLimit/target_gas_limitthrough JSON/SSZ and into block assembly. - Add/adjust CL networking + fork-choice features:
beacon_blocks_by_headreq/resp, envelope req/resp rate-limiting, PTC duplicate vote counting, parent payload gas-limit tracking, and checkpoint execution-hash selection updates.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test-fixtures.json | Bump consensus-specs fixtures to v1.7.0-alpha.8 mainnet tarball + checksum/size. |
| execution/engineapi/engine_types/ssz.go | SSZ encode/decode updates for PayloadAttributes across forks; adds targetGasLimit for Gloas+. |
| execution/engineapi/engine_types/jsonrpc.go | Adds targetGasLimit to PayloadAttributes JSON-RPC model. |
| execution/engineapi/engine_server.go | Plumbs TargetGasLimit into assemble block parameters for Gloas+. |
| execution/builder/parameters.go | Adds TargetGasLimit to PoS builder parameters struct. |
| execution/builder/create_block.go | Uses validator-provided TargetGasLimit when building headers. |
| cl/transition/impl/eth2/operations.go | Updates pending-validator detection to new IsPendingValidator signature. |
| cl/sentinel/handlers/rate_limiter.go | Adds per-protocol limits for new/added reqresp protocols. |
| cl/sentinel/handlers/handlers.go | Registers beacon_blocks_by_head handler. |
| cl/sentinel/handlers/execution_payload_envelopes.go | Adds per-item rate-limit consumption for envelope reqresp handlers. |
| cl/sentinel/handlers/blocks_by_head.go | Implements beacon_blocks_by_head req/resp handler. |
| cl/sentinel/communication/topics.go | Defines /beacon_blocks_by_head topic and protocol ID. |
| cl/phase1/stages/forkchoice.go | Switches FCU checkpoint hash selection to GetFinalizedExecutionHash. |
| cl/phase1/network/services/proposer_preferences_service.go | Updates epoch-range validation and logging; uses TargetGasLimit. |
| cl/phase1/network/services/proposer_preferences_service_test.go | Adjusts tests for MinSeedLookahead and TargetGasLimit fields/messages. |
| cl/phase1/network/services/gas_limit.go | Adds IsGasLimitTargetCompatible helper for bid validation. |
| cl/phase1/network/services/execution_payload_bid_service.go | Replaces exact gas limit match check with compatibility check using parent gas limit. |
| cl/phase1/network/services/execution_payload_bid_service_test.go | Updates bid validation tests for compatibility-based IGNORE behavior. |
| cl/phase1/forkchoice/payload_vote.go | Handles duplicate PTC indices; refactors payload vote majority checks; adds ShouldBuildOnFull. |
| cl/phase1/forkchoice/on_payload_attestation_message.go | Updates gossip PTC vote accounting for duplicate indices. |
| cl/phase1/forkchoice/on_execution_payload.go | Tracks execution payload gas limit by execution block hash. |
| cl/phase1/forkchoice/on_block.go | Tracks execution payload gas limit by execution block hash when processing blocks. |
| cl/phase1/forkchoice/mock_services/forkchoice_mock.go | Extends forkchoice mock for new interfaces + gas limit map. |
| cl/phase1/forkchoice/interface.go | Extends forkchoice reader interface (finalized exec hash, ShouldBuildOnFull, gas limit lookup). |
| cl/phase1/forkchoice/forkchoice.go | Implements gas limit cache + GetFinalizedExecutionHash + gas limit accessor. |
| cl/phase1/core/state/upgrade.go | Gloas upgrade: initializes bid gas limit from latest header; modifies builder onboarding logic. |
| cl/phase1/core/state/epbs.go | Refactors IsPendingValidator to accept config + explicit pending deposit list. |
| cl/cltypes/network.go | Adds SSZ type for BeaconBlocksByHeadRequest. |
| cl/cltypes/epbs_proposer.go | Renames proposer preference field to target_gas_limit and updates SSZ/json handling. |
| cl/clparams/config.go | Adds PAYLOAD_DUE_BPS; updates mainnet MIN_BUILDER_WITHDRAWABILITY_DELAY to 8192. |
| cl/clparams/config_test.go | Updates config YAML fixture + assertions for new/changed config constants. |
| cl/beacon/handler/block_production.go | Sets PayloadAttributes.TargetGasLimit from proposer preferences during block production (Gloas+). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if proposalEpoch < currentEpoch || proposalEpoch > currentEpoch+s.beaconCfg.MinSeedLookahead { | ||
| return fmt.Errorf("%w: proposal slot %d is in epoch %d, expected epoch in [%d, %d]", | ||
| ErrIgnore, proposalSlot, proposalEpoch, currentEpoch, currentEpoch+s.beaconCfg.MinSeedLookahead) |
There was a problem hiding this comment.
No off-by-one here. The spec comment uses Python range(current_epoch, current_epoch + MIN_SEED_LOOKAHEAD + 1) which is exclusive of the upper bound, producing values current_epoch through current_epoch + MIN_SEED_LOOKAHEAD inclusive. The Go check proposalEpoch > currentEpoch+MinSeedLookahead matches this exactly. The ProcessPtcWindow comparison cited is a different context (PTC window sizing), not a direct analog for this validation.
| if stateVersion.AfterOrEqual(clparams.GloasVersion) { | ||
| sn := hexutil.Uint64(targetSlot) | ||
| attrs.SlotNumber = &sn | ||
| if a.epbsPool != nil { | ||
| for _, pref := range a.epbsPool.GetPreferencesForSlot(targetSlot) { | ||
| if pref.Message != nil && pref.Message.ValidatorIndex == proposerIndex { | ||
| tgl := hexutil.Uint64(pref.Message.TargetGasLimit) | ||
| attrs.TargetGasLimit = &tgl | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
TargetGasLimit is a proposer-local preference (e.g. "I want 30M gas limit") that does not vary by fork view. Even if the cache holds entries from different dependent_root views, the same proposer's TargetGasLimit is identical across all of them. The structural observation is correct (the scan ignores DependentRoot), but it cannot cause an incorrect value to be passed to the EL.
| func (f *ForkChoiceStore) payloadDataAvailability(root common.Hash, available bool) bool { | ||
| voteRaw, ok := f.payloadDataAvailabilityVote.Load(root) | ||
| if !ok { | ||
| return false | ||
| return !available | ||
| } | ||
|
|
||
| // If the payload is not locally available, the blob data | ||
| // is not considered available regardless of the PTC vote | ||
| if !f.forkGraph.HasEnvelope(root) { | ||
| return false | ||
| return !available | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in 5b3b78d. Both payloadTimeliness and payloadDataAvailability now return false (no majority found) when votes or envelope are missing, matching the spec's count > PTC_SIZE // 2 semantics (0 > N/2 = false).
| // [New in Gloas:EIP7732] ShouldBuildOnFull returns whether the proposer should build on | ||
| // the full payload for the given head node. Used for proposer reorg of unavailable blocks. | ||
| ShouldBuildOnFull(head ForkChoiceNode) bool |
There was a problem hiding this comment.
Good catch — fixed in 5b3b78d. ShouldBuildOnFull is now called in block production (block_production.go), replacing HasEnvelope && ShouldExtendPayload with the spec-correct GetHeadPayloadStatus() == FULL && ShouldBuildOnFull(...) flow from prepare_execution_payload.
| isExistingBuilder := IsBuilderPubkey(b, deposit.PubKey) | ||
| hasBuilderCredentials := IsBuilderWithdrawalCredential(deposit.WithdrawalCredentials, cfg) | ||
|
|
||
| if isExistingBuilder || hasBuilderCredentials { | ||
| // Apply deposit for builder | ||
| ApplyDepositForBuilder(b, deposit.PubKey, deposit.WithdrawalCredentials, deposit.Amount, deposit.Signature, deposit.Slot) | ||
| if IsPendingValidator(cfg, newPendingDeposits, deposit.PubKey) { | ||
| newPendingDeposits.Append(deposit) | ||
| } else { | ||
| ApplyDepositForBuilder(b, deposit.PubKey, deposit.WithdrawalCredentials, deposit.Amount, deposit.Signature, deposit.Slot) | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in 5b3b78d. IsPendingValidator now receives the original pendingDeposits (full list) instead of newPendingDeposits (partial), ensuring a builder deposit at index N is not applied when a valid validator deposit for the same pubkey exists at index N+M.
There was a problem hiding this comment.
Update: reverted this change in b85e8f4. The spec tests (gloas/fork/fork/pyspec_tests/fork_*_builder_deposit*) confirm the code is correct as-is. newPendingDeposits is intentional — the spec only blocks a builder deposit when a validator deposit for the same pubkey was already processed earlier in the loop, not when one appears later in the queue. The sequential processing order is part of the spec semantics.
f81e0f2 to
a42c8ef
Compare
- Update MIN_BUILDER_WITHDRAWABILITY_DELAY 64→8192 (#5223) - Fix PTC votes for duplicated validators to count all indices (#5222) - Rename isPayloadTimely/isPayloadDataAvailable to bidirectional payloadTimeliness/payloadDataAvailability (#5212) - Add ShouldBuildOnFull for proposer reorg of unavailable blocks (#5186) - Modify notify_forkchoice_updated to use parent_block_hash for Gloas+ blocks (#5197) - Add PAYLOAD_DUE_BPS config constant (#5212) - Rename ProposerPreferences.GasLimit→TargetGasLimit (#5236) - Add target_gas_limit to PayloadAttributes and thread through engine API (#5235) - Replace exact gas_limit REJECT with is_gas_limit_target_compatible IGNORE check (#5236) - Use MIN_SEED_LOOKAHEAD in proposer preferences epoch validation (#5215) - Optimize builder onboarding at fork transition (#5254) - Add beacon_blocks_by_head ReqResp protocol for Fulu (#5181) - Upgrade spectest fixtures to v1.7.0-alpha.8 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…deposits IsPendingValidator was checking only newPendingDeposits (the already-processed subset) instead of the full pendingDeposits queue. When a builder deposit appeared before a validator deposit for the same pubkey, the conflict was invisible and the builder was incorrectly onboarded. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous change (newPendingDeposits → pendingDeposits) broke 9 gloas/fork spectest cases. The spec's onboard_builders_from_pending_deposits intentionally checks only the already-processed subset — deposit ordering determines whether a builder deposit can be applied before a later validator deposit for the same pubkey. This reverts the functional change from ff23a4d. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Lighthouse: glamsterdam-devnet-4 (official) Prysm: glamsterdam-devnet-4-tmp-minimal (interim, no official tag yet) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The old custom image (qu0b-gloas-bals-v2) caused the stability check to hang on attestation_stats/reorgs/forks sub-tasks, timing out CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ndingValidator list - payload_vote: payloadTimeliness and payloadDataAvailability return false (no majority) when votes or envelope are missing, matching the spec's count > PTC_SIZE/2 check (0 > N/2 = false). - block_production: replace HasEnvelope && ShouldExtendPayload with GetHeadPayloadStatus == FULL && ShouldBuildOnFull, aligning with the spec's prepare_execution_payload flow and giving ShouldBuildOnFull its first call site. - upgrade: onboardBuildersFromPendingDeposits calls IsPendingValidator with the original pendingDeposits list instead of the partial newPendingDeposits, ensuring later validator deposits are not missed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Spec tests confirm that onboardBuildersFromPendingDeposits intentionally uses newPendingDeposits (the partially-built list), not the original pendingDeposits. A builder deposit is only blocked when a validator deposit for the same pubkey was already processed earlier in the loop, not when one appears later in the queue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…alse bug The PTC vote arrays used [PtcSize]bool, where the zero-value (false) was indistinguishable from an explicit "false" vote. This caused unvoted slots to inflate the negative vote count, breaking payloadTimeliness, payloadDataAvailability, and ShouldBuildOnFull. Switch to [PtcSize]int8 (0=unvoted, 1=true, -1=false) so counting logic only considers slots that actually voted. Add unit tests covering the vote counting boundary conditions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ng fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
go-eth2-client lacks v1.7.0-alpha.8 support, so check_consensus_attestation_stats always reports 0%. Matches the other Gloas configs which already disable it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
75bc8f4 to
1adf4cf
Compare
…-Included header Use GetFinalizedExecutionHash instead of GetEth1Hash for finalized/safe hashes in block production FCU calls. For Gloas blocks GetEth1Hash returns the envelope payload hash which can be ahead of head, causing EL verifyForkchoiceHashes to reject with "Invalid forkchoice state". Add the Eth-Execution-Payload-Included response header and body field for Gloas block production per consensus-specs. Only include the execution payload envelope in JSON responses since SSZ serialization of BeaconResponse.Extra is not supported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if version >= clparams.GloasVersion { | ||
| assembleParams.TargetGasLimit = (*uint64)(payloadAttributes.TargetGasLimit) | ||
| } |
| if cost := min(int(req.Count), int(c.beaconConfig.MaxRequestBlocksDeneb)) - 1; !c.consumeRateLimit(s, cost) { | ||
| return nil | ||
| } |
| // WillEncodeSSZ reports whether the given Accept header will cause | ||
| // HandleEndpoint to use SSZ encoding. Mirrors the switch priority above. | ||
| func WillEncodeSSZ(accept string) bool { | ||
| switch { | ||
| case accept == "*/*", accept == "", strings.Contains(accept, "text/html"), strings.Contains(accept, "application/json"): | ||
| return false | ||
| case strings.Contains(accept, "application/octet-stream"): | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
| // [New in Gloas:EIP7732] GetFinalizedExecutionHash returns the EL block hash for | ||
| // finalized/justified checkpoints, using parent_block_hash from the bid for Gloas+ blocks. | ||
| GetFinalizedExecutionHash(eth2Root common.Hash) common.Hash |
yperbasis
left a comment
There was a problem hiding this comment.
A few nits / follow-ups — none blocking, but worth addressing before merge or in a follow-up:
Correctness/robustness
-
IsPendingValidatorincl/phase1/core/state/epbs.go:188now swallows theIsValidDepositSignatureerror (valid, _ := ...). The previous code logged it vialog.Debug. Small observability regression — if BLS verification starts erroring, the upgrade path will silently treat those deposits as non-pending and apply them as builder deposits. Suggest restoringif err == nil && valid(or at least keeping the debug log). -
epbsPool.GetPreferencesForSlotlookup incl/beacon/handler/block_production.go:831is non-deterministic when the pool holds multiple preferences for the same slot with differentDependentRoots.LRU.Keys()iteration order is unspecified, and the loopbreaks on first match. Normally only one fork view should produce preferences from a given proposer, but a deterministic tiebreak (e.g. highest dependent root or latest slot in cache order) — or a comment explaining why first-match is acceptable — would make this less surprising. -
IsGasLimitTargetCompatiblecheck incl/phase1/network/services/execution_payload_bid_service.go:243silently fails open whenGetExecutionPayloadGasLimit(bid.ParentBlockHash)misses the LRU. The other parent-cache check right above it (GetRecentExecutionPayloadStatus) IGNOREs on miss instead. Asymmetric — worth a brief comment on why this one fails open.
CI / workflows
-
assertoorpinned toethpandaops/assertoor:master-0ad56fbinglamsterdam.iois a master-branch SHA, not a tag. If the assertoor repo rebases or retags master, the pin breaks silently. Suggest a tracking issue to switch back to a release tag once assertoor cuts one that handles Gloas devnet-4 blocks. -
run_stability_check: falseinglamsterdam.ioshould also be tracked — re-enable once assertoor'sgo-eth2-clientcatches up to alpha.8 block bodies. A linked issue in the workflow comment would help future maintainers spot it. -
Test plan checkboxes in the PR description for CI unit / integration / kurtosis runs are still unchecked. Worth confirming those pass — particularly the kurtosis glamsterdam-devnet-4 run that's the actual integration target.
Style (CLAUDE.md says one-sentence comments, no paragraphs, no spec snippets in source)
cl/phase1/network/services/gas_limit.go:19-28— 7-line comment with an inlined Python spec snippet. The spec-ref link is enough; the Python duplicates the Go below it.- A few of the new
[New in Gloas:EIP7732]3-line comments (forkchoice.goGetFinalizedExecutionHash/GetExecutionPayloadGasLimit,interface.gonew methods) could be one sentence.
Not blocking — overall the load-bearing pieces (PTC three-state, FCU parent-block-hash semantics, IsGasLimitTargetCompatible formula, builder-onboarding ordering) look correct and the alpha.8 spectests passing is good evidence for the more sensitive corners.
Summary
notify_forkchoice_updatedusesparent_block_hash(feat(hive): add a few more exclusions #5197), proposer reorg unavailable blocks (Remove capitalization and trailing newlines from err strings #5186), PTC duplicate validator vote counting (Move miner inside TxPool #5222), separated payload timeliness / data availability deadlines (Fixed mining system contract txn lost #5212)target_gas_limitinPayloadAttributes(added VerkleProof field to header. #5235), gas limit gossip validation (#5236), builder onboarding at fork transition (Cleanup interfaces #5254),MIN_BUILDER_WITHDRAWABILITY_DELAY→ 8192 (Create new Stage for snapshots downloading and creation #5223),MIN_SEED_LOOKAHEADfor proposer preferences (tweak(sentry): downgrade inbound message handling err to debug #5215)beacon_blocks_by_headprotocol (erigon22: disable logIndex, historyIndex, callTracesIndex stages in --history.v2=true mode #5181)PAYLOAD_DUE_BPSaddedBug fixes after initial implementation
int8(ptcVoteAbsent/ptcVotePresent/ptcVoteWithheld) instead ofbool, wireShouldBuildOnFullcorrectlyKurtosis / CI
glamsterdam-devnet-4master-0ad56fb(latest available version that doesn't crash on Gloas blocks)run_stability_checkfor glamsterdam — assertoor'sgo-eth2-clientis still on alpha.7 and can't decode devnet-4 block bodies for attestation statsTest plan
make lint— 0 issuesgo test ./cl/spectest/— all gloas/fork, fork_choice, ssz_static tests pass (including 9 new builder deposit fork tests)payload_vote_test.go— new unit tests for three-state PTC vote logic🤖 Generated with Claude Code