feat: implement non-eip-7688 changes in alpha.12 - #9606
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the fast confirmation test suite to temporarily skip two failing Electra 'is_one_confirmed' test cases ('is_one_confirmed_fails_large_validator_slashed' and 'is_one_confirmed_fails_recently_activated_validator_voting_in_empty_slot') due to known issues that require a broader overhaul of the fast-confirmation rule (FCR). There are no review comments, so I have no feedback to provide.
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.
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
ensi321
added a commit
that referenced
this pull request
Jul 7, 2026
ensi321
changed the title
ensi321
added a commit
that referenced
this pull request
Jul 16, 2026
Enable the v1.7.0-alpha.12 consensus-spec-tests on the EIP-7688 branch. With progressive containers in place the gloas ssz_static / state-transition / fork-choice suites pass; the remaining failures are all NOT EIP-7688 related and handled elsewhere: - ALPHA12-MISC (unskip in #9606): builder-deposit / builder-exit processing and the alpha.12 constant bumps (PAYLOAD_DUE_BPS, MIN_BUILDER_WITHDRAWABILITY_DELAY, MAX_BUILDER_DEPOSIT_REQUESTS_PER_PAYLOAD) land on the follow-up alpha.12-misc branch. - FCR rework: gloas fast_confirmation suite + two is_one_confirmed cases. - cell-level DAS: fulu PartialDataColumnGroupID ssz_static. - ssz 1.6.2: consolidations-at-limit hits an @chainsafe/ssz@1.6.1 stack overflow at mainnet scale (issue #9656); clears when ssz 1.6.2 is synced via unstable. Also: - Bump spec-tests-version.json to v1.7.0-alpha.12. - Re-sync ethspecify specrefs to v1.7.0-alpha.12: drop entries for spec objects removed/renamed at alpha.12 (get_dependent_root, get_proposer_dependent_root, get_proposer_preferences_signature, is_epoch_boundary, BUILDER_REGISTRY_LIMIT, BUILDER_PENDING_WITHDRAWALS_LIMIT), source the new gloas/electra SSZ types, gloas p2p size-bound presets, and get_shuffling_dependent_root / get_signed_proposer_preferences / is_valid_indexed_attestation / is_not_epoch_boundary, and except the unimplemented heze + attestation-upgrade + slot-based gindex helpers. `ethspecify check` reports 1144 valid references. - Fix the ProgressiveTestStruct ssz_generic type: field A is ProgressiveList[byte] (a byte list, hex JSON) so it must be ProgressiveByteListType, not ProgressiveListBasicType. Fixes all phase0/ssz_generic/containers/ProgressiveTestStruct cases. Verified green: minimal 55235, mainnet 7185, general 5367, bls 90, config-sync, params-preset; ethspecify check 1144 valid. lint + check-types clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- PAYLOAD_DUE_BPS 7500 -> 5000 (consensus-specs #5414) - MIN_BUILDER_WITHDRAWABILITY_DELAY mainnet 8192 -> 64 (consensus-specs #5426) - MAX_BUILDER_DEPOSIT_REQUESTS_PER_PAYLOAD 256 -> 64 (consensus-specs #5420) - EnvelopesByRange cap uses MAX_REQUEST_PAYLOADS (consensus-specs #5383) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aligns the builder withdrawal prefix with consensus-specs #5416 (was 0x03) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ethereum/consensus-specs#5429 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
see ethereum/consensus-specs#5439 Builder deposit requests are ignored unless the withdrawal credentials start with `BUILDER_WITHDRAWAL_PREFIX`. Accepted new builders are registered with `PAYLOAD_BUILDER_VERSION`. Checks: - `pnpm exec biome check packages/state-transition/src/block/processBuilderDepositRequest.ts packages/state-transition/test/unit/block/processBuilderDepositRequest.test.ts packages/state-transition/test/unit/block/processBuilderExitRequest.test.ts` - `pnpm --filter @lodestar/state-transition check-types` > This PR was written primarily by Codex.
consensus-specs #5436 (in v1.7.0-alpha.12) removed MAX_SIGNED_BEACON_BLOCK_SIZE, and EIP-7688 progressive lists removed BUILDER_REGISTRY_LIMIT and BUILDER_PENDING_WITHDRAWALS_LIMIT from the gloas preset. Post-gloas beacon block gossip is now bounded by MAX_PAYLOAD_SIZE per https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.12/specs/phase0/p2p-interface.md#the-gossip-domain-gossipsub Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
consensus-specs #5436 removed the deposits bounds from the gloas block and envelope gossip REJECT conditions (execution requests deposits are unbounded progressive lists). The extra checks would reject spec-valid messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
total active balance exceeds Number.MAX_SAFE_INTEGER at mainnet scale (~16M increments * 1e9 = 3.4e16), making the quorum threshold diverge from spec integer math. Keep the intermediate computation in bigint; the final threshold (~total_balance/32 * 6/10) is safely below 2**53. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All fixes referenced by the TODO GLOAS #9606 markers have landed on this branch, spec tests and config/preset e2e sync tests pass unmodified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oval Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9606 +/- ##
============================================
- Coverage 52.52% 52.51% -0.01%
============================================
Files 848 848
Lines 60394 60388 -6
Branches 4458 4458
============================================
- Hits 31719 31713 -6
Misses 28614 28614
Partials 61 61 🚀 New features to boost your workflow:
|
![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: a241c0503c
ℹ️ 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".
| if ( | ||
| block !== null && | ||
| attData.index === 1 && | ||
| !chain.forkChoice.hasPayloadHexUnsafe(toRootHex(attData.beaconBlockRoot)) |
There was a problem hiding this comment.
This updates the unaggregated attestation path to require forkChoice.hasPayloadHexUnsafe, but the aggregate path still uses chain.seenPayloadEnvelope in packages/beacon-node/src/chain/validation/aggregateAndProof.ts:99. In Gloas, an index == 1 aggregate submitted through publishAggregateAndProofsV2 can therefore be accepted and published while the envelope is only in seenPayloadEnvelopeInputCache and not fully imported with data, even though the alpha.12 gossip rule requires is_payload_verified for beacon_aggregate_and_proof too. Please apply the same fork-choice payload check there.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Addressed as of c00a00bc8c ("fix: require imported payload for index==1 aggregate gossip"): the index == 1 aggregate path in aggregateAndProof.ts now gates on !chain.forkChoice.hasPayloadHexUnsafe(toRootHex(attData.beaconBlockRoot)) → IGNORE EXECUTION_PAYLOAD_NOT_SEEN, structurally identical to the unaggregated path in attestation.ts:330. No seenPayloadEnvelope reference remains in the file, so publishAggregateAndProofsV2 now enforces the same is_payload_verified gossip rule. Good catch by the bot.
twoeths
reviewed
Jul 22, 2026
| skippedTests: [ | ||
| // TODO-GLOAS: re-enable after gloas light client is implemented | ||
| /\/gloas_fork$/, | ||
| // TODO GLOAS: Unskip in #9606 |
Member
There was a problem hiding this comment.
it's great to see we removed a lot of skipped tests in this PR
Contributor
There was a problem hiding this comment.
Thanks 🙏 — most of the gloas handlers that were stubbed through the earlier alphas are implemented now, so the skips could go. A few remain with TODOs for follow-up (the on_payload_attestation_message fork-choice handler, the gloas light-client path, and the networking runner that's been skipped since alpha.5).
nflaig
reviewed
Jul 23, 2026
Comment on lines
+35
to
+36
| // total active balance exceeds Number.MAX_SAFE_INTEGER at mainnet scale, keep the intermediate math in bigint | ||
| const perSlotBalance = |
Member
There was a problem hiding this comment.
is this true? how was this caught, seems like a new spec test?
Member
Author
There was a problem hiding this comment.
I think so.
Total active EB is like 34M ETH on mainnet, multiply by EFFECTIVE_BALANCE_INCREMENT = 34M * 10^9 = 3.4 * 10^16 Gwei. Which is greater than MAX_SAFE_INTEGER.
No spec test for this. Just caught this when reviewing with claude.
Member
There was a problem hiding this comment.
@lodekeeper can you confirm this math, I assumed Number was safe to use for anything that is accounted for in gwei due to max eth supply cap
Contributor
There was a problem hiding this comment.
Confirmed — ensi321 is right, it does overflow, and the current bigint version (from c7663acefa) handles it correctly.
The overflow is on the intermediate totalActiveBalanceIncrements * EFFECTIVE_BALANCE_INCREMENT, which reconstructs total active balance in gwei:
totalActiveBalanceIncrementsis tracked in ETH units (epochCache.ts:370keeps it that way precisely to fit total network balance in a JS number), so ~34M at today's mainnet.× EFFECTIVE_BALANCE_INCREMENT(1e9) → total active balance in gwei ≈ 3.4e16, vsMAX_SAFE_INTEGER≈ 9.007e15 → ~3.8× over. At ensi321's 41M ETH it's 4.1e16 (~4.6×). Conclusion holds regardless of the exact staking figure.
On your assumption — that's the part that doesn't hold. The safe ceiling for a gwei value in a JS number is MAX_SAFE_INTEGER / 1e9 ≈ 9.01M ETH:
- A single validator balance is always fine — even
MAX_EFFECTIVE_BALANCE_ELECTRA(2048 ETH = 2.048e12 gwei) is ~4000× under the limit. - Any sum across the validator set is not: total active balance (~34M ETH) already passes ~9M ETH, and the supply cap makes it worse, not safer — ~120M ETH in gwei is 1.2e17, ~13×
MAX_SAFE_INTEGER. So "accounted for in gwei ⇒ safe in Number" is true per-validator, not for aggregates.
The function itself is fine: all intermediate math stays in bigint and only the final result is cast back — and that result is small (perSlot × 6/10 ≈ 6.4e14 at 34M ETH, ≤ ~2.25e15 even if the entire supply were staked), comfortably under MAX_SAFE_INTEGER. So no further change needed here; epochCache.ts:392 (throw if totalActiveBalanceIncrements itself ≥ MAX_SAFE_INTEGER) is the backstop on the input side.
And yeah — no spec test would catch this as-is: it's silent precision loss, not a throw, so a unit test would need a state built at realistic mainnet-scale total balance to surface it.
Member
There was a problem hiding this comment.
realized we also use bigint for the ssz type
so this change is needed, that's a good catch
Contributor
There was a problem hiding this comment.
Yeah — and it's worth pinning down why that's consistent, because the two Gwei representations are used deliberately here.
Gwei = UintBn64 (bigint) is the right type for full-total balances that can exceed 2^53 — e.g. depositBalanceToConsume/exitBalanceToConsume (sszTypes.ts:522-523). But the field this threshold actually feeds — BuilderPendingPayment.weight (sszTypes.ts:239) — is intentionally UintNum64 (number), and that's correct rather than an oversight: weight accumulates only the same-slot attesting balance for one payment (processAttestationsAltair.ts:153, existingWeight + paymentWeightToAdd * EFFECTIVE_BALANCE_INCREMENT), so it's bounded by ~one slot's committee ≈ total_active_balance / 32. That's ~1.06e15 gwei at 34M ETH staked, and still only ~3.75e15 even if the entire supply were staked — comfortably under MAX_SAFE_INTEGER (it wouldn't reach it until ~288M ETH active, well past the supply cap).
So the overflow was specifically the intermediate full-total reconstruction inside the threshold fn (totalActiveBalanceIncrements × EFFECTIVE_BALANCE_INCREMENT ≈ 3.4e16, before the ÷32) — which is why the bigint-intermediate / small-Number-result shape is the right fix, and why the number-typed weight on the other side of the >= compare is fine as-is. Same reason the accumulation at :153 can stay in plain number: it's per-slot-scoped and never materializes the full total in one value.
tl;dr the split is intentional — full-total balances → Gwei/UintBn64; per-slot / bounded sums → UintNum64. weight is correctly the latter, so c7663acefa (bigint intermediates) is the whole of it, no further change needed.
Mirror the beacon_attestation is_payload_verified gating (#9611) on the beacon_aggregate_and_proof topic per https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.12/specs/gloas/p2p-interface.md#modified-beacon_aggregate_and_proof An index==1 aggregate was accepted once the envelope was merely seen on gossip, before it was imported and verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
twoeths
approved these changes
Jul 24, 2026
Member
|
🎉 This PR is included in v1.46.0 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #9390