feat: drop legacy eth1 bridge deposits from Fulu onwards (consensus-specs #4704)#9458
feat: drop legacy eth1 bridge deposits from Fulu onwards (consensus-specs #4704)#9458lodekeeper wants to merge 3 commits into
Conversation
…pecs ChainSafe#4704) Implements jtraglia's consensus-specs ChainSafe#4704: the Fulu state transition no longer processes eth1 bridge deposits via `block.body.deposits` or initializes `deposit_requests_start_index` from the EIP-6110 `process_deposit_request` path. At the Electra to Fulu boundary, if no Electra `deposit_request` already set `deposit_requests_start_index`, seed it to `eth1_data.deposit_count` so post-Fulu `process_pending_deposits` sees a consistent state (mirrors the `do_fork` helper from the Lighthouse port jtraglia/lighthouse#4704). - Gate `getEth1DepositCount` check and `body.deposits` processing on `fork < ForkSeq.fulu` in `processOperations` - Gate `depositRequestsStartIndex` initialization in `processDepositRequest` on `fork < ForkSeq.fulu` - Gate the "deposit_requests not before bridge deposits" condition in `processPendingDeposits` on `fork < ForkSeq.fulu` - Seed `depositRequestsStartIndex = eth1Data.depositCount` at the Fulu boundary in `stateTransition` when still unset - Drop the `^fulu/transition/.*` skip in `specTestIterator` Full transition spec tests (deneb, electra, fulu, gloas, minimal + mainnet): 4 test files / 920 tests / 0 fail. Co-authored-by: Justin Traglia <justintraglia@gmail.com> 🤖 Generated with AI assistance
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the Fulu deposit-mechanism removal (EIP-6110) by bypassing legacy eth1 bridge deposit checks and processing from the Fulu fork onwards, and re-enables the Fulu transition spec tests. The review feedback suggests moving the state upgrade mutation logic for depositRequestsStartIndex into the dedicated upgradeStateToFulu function to improve encapsulation and code cleanliness.
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.
| // [Modified in Fulu:EIP6110] Legacy eth1 bridge deposits are removed in Fulu. If no Electra | ||
| // deposit_request set `deposit_requests_start_index`, mark the bridge cutover as complete | ||
| // at the boundary so post-Fulu `process_pending_deposits` (which no longer gates on the | ||
| // bridge index) sees a consistent state. Mirrors consensus-specs #4704 do_fork helper. | ||
| const stateElectra = postState as CachedBeaconStateElectra; | ||
| if (stateElectra.depositRequestsStartIndex === UNSET_DEPOSIT_REQUESTS_START_INDEX) { | ||
| stateElectra.depositRequestsStartIndex = BigInt(stateElectra.eth1Data.depositCount); | ||
| } | ||
| postState = upgradeStateToFulu(stateElectra) as CachedBeaconStateAllForks; |
There was a problem hiding this comment.
To keep stateTransition.ts clean and ensure that the Electra-to-Fulu state upgrade logic is fully encapsulated, this mutation should be moved inside upgradeStateToFulu in packages/state-transition/src/slot/upgradeStateToFulu.ts.
This ensures that any other callers or tests upgrading the state directly will also correctly apply this transition logic.
In packages/state-transition/src/slot/upgradeStateToFulu.ts:
import {UNSET_DEPOSIT_REQUESTS_START_INDEX} from "@lodestar/params";
export function upgradeStateToFulu(stateElectra: CachedBeaconStateElectra): CachedBeaconStateFulu {
const {config} = stateElectra;
if (stateElectra.depositRequestsStartIndex === UNSET_DEPOSIT_REQUESTS_START_INDEX) {
stateElectra.depositRequestsStartIndex = BigInt(stateElectra.eth1Data.depositCount);
}
const stateElectraNode = ssz.electra.BeaconState.commitViewDU(stateElectra);
// ...| // [Modified in Fulu:EIP6110] Legacy eth1 bridge deposits are removed in Fulu. If no Electra | |
| // deposit_request set `deposit_requests_start_index`, mark the bridge cutover as complete | |
| // at the boundary so post-Fulu `process_pending_deposits` (which no longer gates on the | |
| // bridge index) sees a consistent state. Mirrors consensus-specs #4704 do_fork helper. | |
| const stateElectra = postState as CachedBeaconStateElectra; | |
| if (stateElectra.depositRequestsStartIndex === UNSET_DEPOSIT_REQUESTS_START_INDEX) { | |
| stateElectra.depositRequestsStartIndex = BigInt(stateElectra.eth1Data.depositCount); | |
| } | |
| postState = upgradeStateToFulu(stateElectra) as CachedBeaconStateAllForks; | |
| postState = upgradeStateToFulu(postState as CachedBeaconStateElectra) as CachedBeaconStateAllForks; |
There was a problem hiding this comment.
Agreed — moved into upgradeStateToFulu in 5bbff8a for encapsulation, matching the self-contained pattern of the other upgradeStateTo* helpers. Also dropped the now-unused UNSET_DEPOSIT_REQUESTS_START_INDEX import from stateTransition.ts.
![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: 835b3ac9f5
ℹ️ 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".
| `Block contains incorrect number of deposits: depositCount=${body.deposits.length} expected=${maxDeposits}` | ||
| ); | ||
| // [Modified in Fulu:EIP6110] Legacy eth1 bridge deposits are removed from Fulu onwards. | ||
| if (fork < ForkSeq.fulu) { |
There was a problem hiding this comment.
For Fulu/Gloas blocks that carry any legacy body.deposits, this branch skips both the old count check and the new zero-length assertion, and the later processDeposit loop is skipped as well, so the block is accepted while the deprecated deposits are silently ignored. The checked consensus reference in specrefs/functions.yml for process_operations#fulu/#gloas requires len(body.deposits) == 0, so a malicious or malformed post-Fulu block with non-empty legacy deposits would diverge from clients that correctly reject it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — that's a real correctness gap. Fixed in 5bbff8a:
if (fork < ForkSeq.fulu) {
// verify that outstanding deposits are processed up to the maximum number of deposits
const maxDeposits = getEth1DepositCount(state);
if (body.deposits.length !== maxDeposits) {
throw new Error(...);
}
} else if (body.deposits.length !== 0) {
// Spec: `assert len(body.deposits) == 0` (process_operations#fulu)
throw new Error(`Block must not contain legacy deposits post-Fulu: depositCount=${body.deposits.length}`);
}Mirrors the spec's assert len(body.deposits) == 0 in process_operations#fulu from consensus-specs#4704. Pre-Fulu count check unchanged; the existing if (fork < ForkSeq.fulu) guard around the processDeposit loop remains for clarity.
…tation Two review fixes: 1. Codex (P1): Add explicit `body.deposits.length === 0` assertion for Fulu+ blocks in processOperations. Without it, malformed/malicious Fulu+ blocks with non-empty legacy deposits would silently pass without processing or rejection, diverging from clients that enforce the spec assertion (`process_operations#fulu`: `assert len(body.deposits) == 0`). 2. Gemini: Move the boundary `depositRequestsStartIndex` mutation into `upgradeStateToFulu` for encapsulation, matching the self-contained pattern of the other upgrade helpers. Drops the `UNSET_DEPOSIT_REQUESTS_START_INDEX` import from stateTransition.ts. 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| export function upgradeStateToFulu(stateElectra: CachedBeaconStateElectra): CachedBeaconStateFulu { | ||
| const {config} = stateElectra; | ||
|
|
||
| // [Modified in Fulu:EIP6110] Legacy eth1 bridge deposits are removed in Fulu. If no Electra | ||
| // deposit_request set `deposit_requests_start_index`, mark the bridge cutover as complete at | ||
| // the boundary so post-Fulu `process_pending_deposits` (which no longer gates on the bridge | ||
| // index) sees a consistent state. Mirrors consensus-specs #4704 do_fork helper. | ||
| if (stateElectra.depositRequestsStartIndex === UNSET_DEPOSIT_REQUESTS_START_INDEX) { | ||
| stateElectra.depositRequestsStartIndex = BigInt(stateElectra.eth1Data.depositCount); | ||
| } | ||
|
|
There was a problem hiding this comment.
Hmm no, this is incorrect. We shouldn't have to do this.
There was a problem hiding this comment.
You're right, removed in f6e94b8. The spec's upgrade_to_fulu just copies deposit_requests_start_index as-is — no UNSET handling needed. process_pending_deposits is already gated on fork < ForkSeq.fulu so the sentinel value carries across the boundary harmlessly.
…StateToFulu Per jtraglia's review: the spec's upgrade_to_fulu simply copies deposit_requests_start_index as-is; no UNSET-sentinel handling is needed. Post-Fulu process_pending_deposits is already gated on fork < ForkSeq.fulu so the UNSET value can safely carry across the boundary. 🤖 Generated with AI assistance
|
Superseded by #9459 — @nflaig's implementation is strictly cleaner: |
2 participants
Summary
Implements jtraglia's consensus-specs #4704 on the Lodestar side. From Fulu onwards, the legacy eth1 bridge deposit flow is removed:
block.body.depositsis no longer processedgetEth1DepositCountconsistency check is skippedprocess_deposit_requestno longer initializesdeposit_requests_start_index(EIP-6110 already finalized that on Electra)process_pending_depositsno longer gates on "bridge deposits not yet applied"To keep post-Fulu
process_pending_depositsconsistent when no Electradeposit_requesthappened to setdeposit_requests_start_index, the Electra to Fulu state upgrade seeds the index toeth1_data.deposit_count(mirrors thedo_forktest helper in jtraglia's Lighthouse port).Follow-up to the review comment on #9422 (
drop legacy Fulu deposit path (consensus-specs #4704)).Changes
state-transition/src/block/processOperations.ts— gategetEth1DepositCountcheck andbody.depositsprocessing onfork < ForkSeq.fulustate-transition/src/block/processDepositRequest.ts— gatedepositRequestsStartIndexinitialization onfork < ForkSeq.fulustate-transition/src/epoch/processPendingDeposits.ts— gate the "deposit_requests not before bridge deposits" condition onfork < ForkSeq.fulustate-transition/src/stateTransition.ts— seeddepositRequestsStartIndex = eth1Data.depositCountat the Electra to Fulu boundary when still unsetbeacon-node/test/spec/utils/specTestIterator.ts— drop the^fulu/transition/.*skip (was waiting on this implementation)Test plan
pnpm lint(biome) — cleanpnpm check-types— cleanfulu/transition— 31 minimal + 25 mainnet, all pass (incl.transition_with_deposit_request_right_after_fork)gloas/transition— 25 minimal + 26 mainnet, all pass (fixes the 11 previously failingattester_slashing_*/proposer_slashing_*/transition_randomized_state/transition_with_one_fourth_slashed_active_validators_pre_fork, same root cause as the fulu hole)electra/transition(27) +deneb/transition— passNotes
v1.7.0-alpha.9(the version that regenerated these transition vectors with the eth1-bridge deposit flow removed).