chore: Properly compute finalized block#21156
Conversation
spalladino
left a comment
There was a problem hiding this comment.
Looks good! Just two concerns:
1- There are some lingering TODOs about finalization that need to be removed now that this is implemented (search TODO(#13569)), and one that needs to be implemented (see rollbackTo in the archiver).
2- I'm very surprised that the e2e reorg tests that trigger a reorg that removes a proof via an L1 reorg still work with the change in anvil so that the L1 epoch length is 1. That setting means that proven becomes finalized immediately, so nodes shouldn't be able to handle a reorg to before the finalized checkpoint. No actionable here, just something I'm surprised/worried about.
| const getProvenCheckpointNumber = (node: AztecNode) => node.getL2Tips().then(tips => tips.proven.checkpoint.number); | ||
|
|
||
| it('prunes L2 blocks if a proof is removed due to an L1 reorg', async () => { | ||
| it.only('prunes L2 blocks if a proof is removed due to an L1 reorg', async () => { |
There was a problem hiding this comment.
Did you intend to add only?
…ztec-packages into spy/finalized-checkpoint
BEGIN_COMMIT_OVERRIDE fix: (A-623) increase committee timeout in scenario smoke test (#21193) feat: orchestrator enqueues via serial queue (#21247) feat: rollup mana limit gas validation (#21219) fix: make e2e HA test more deterministic (#21199) chore: fix chonk_browser lint warning (#21265) chore: deploy SPONSORED_FPC in test networks (#21254) fix: (A-635) e2e bot flake on nonce mismatch (#21288) chore: deflake duplicate attestations and proposals slash tests (#21294) fix(sequencer): fix log when not enough txs (#21297) chore: send env var to pods (#21307) fix: Simulate gas in n tps test. Set min txs per block to 1 (#21312) fix: update dependabot dependencies (#21238) test: run nightly bench of block capacity (#20726) fix: update block_capacity test to use new send() result types (#21345) fix(node): fix index misalignment in findLeavesIndexes (#21327) fix(log): do not log validation error if unregistered handler (#21111) fix: limit parallel blocks in prover to max AVM parallel simulations (#21320) fix: use native sha256 to speed up proving job id generation (#21292) chore: remove v4-devnet-1 (#21044) fix(validator): wait for l1 sync before processing block proposals (#21336) fix(txpool): cap priority fee with max fees when computing priority (#21279) chore: Properly compute finalized block (#21156) fix: remove extra argument in KVArchiverDataStore constructor call (#21361) chore: revert l2 slot time 72 -> 36 on scenario network (#21291) fix(archiver): do not error if proposed block matches checkpointed (#21367) fix(claude): rule to not append echo exit (#21368) chore: reduce severity of errors due to HA node not acquiring signature (#21311) fix: make reqresp batch retry test deterministic (#21322) fix: (A-643) add buffer to maxFeePerBlobGas for gas estimation and fix bump loop truncation (#21323) fix(e2e): use L2 priority fee in deploy_method same-block test (#21373) fix: reqresp flake & add logging (#21334) END_COMMIT_OVERRIDE
…ization race (#21452) ## Summary - Sets `anvilSlotsInAnEpoch: 32` in `e2e_offchain_payment` test setup, matching what `epochs_l1_reorgs` already does. ## Problem PR #21156 added `--slots-in-an-epoch 1` as the default for anvil, making `finalized = latest - 2`. PR #20893 added `e2e_offchain_payment` which simulates L1 reorgs. When both landed on `merge-train/fairies`, the reorg test fails deterministically because finalization races past the rollback target block. ## Fix Use `anvilSlotsInAnEpoch: 32` (matching Ethereum mainnet) so the finalized block stays far enough behind latest to allow rollbacks in the test. ClaudeBox log: https://claudebox.work/s/c5ac5d52da86e23a?run=4
|
❌ Failed to cherry-pick to |
Cherry-pick of 078737f from next. Conflicts present in: - yarn-project/archiver/src/factory.ts - yarn-project/archiver/src/store/block_store.ts - yarn-project/ethereum/src/test/start_anvil.ts
…ints (#21597) ## Summary - Updates the finalized block heuristic from `epochDuration * 2` to `epochDuration * 2 * 4` to subtract checkpoints (assumed 4 blocks each) instead of blocks. - The proper fix is in #21156 which replaces this heuristic with L1 finality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… test (#21642) ## Summary - Removes the historic/finalized block verification checks from `epochs_multiple.test.ts` - The finalization logic on v4 is incorrect: it subtracts a fixed number of blocks (`epochDuration * 2`) instead of accounting for variable blocks per slot (up to 4 per slot), causing test timeouts - The correct finalization implementation exists on `next` in #21156 but is non-trivial to backport to v4 - Keeps the proven sync check intact — only historic/finalized assertions are removed ## Context See discussion in Slack: the current `getFinalizedL2BlockNumber` uses `provenBlockNumber - epochDuration * 2` which doesn't account for variable blocks per slot. This causes the tx mempool to evict transactions too aggressively and the test to time out waiting for finalization. ## Test plan - CI should pass — the test still verifies epoch proving and proven block sync, just without the finalized block assertions ClaudeBox log: https://claudebox.work/s/a5e9cea005ce4a5a?run=1
Fixes A-551
Description
Replaces the heuristic finalized block computation (
provenBlock - 2 * epochDuration) with L1 finality.On each archiver sync iteration, we now:
getBlock({ blockTag: 'finalized' })Failures in this step are caught and logged as warnings so they don't disrupt the rest of the sync loop (e.g. if the RPC node can't serve state at the finalized block).
Changes
RollupContract.getProvenCheckpointNumbernow accepts an optional{ blockNumber }to query historical contract stateBlockStorestores alastFinalizedCheckpointsingleton and derivesgetFinalizedL2BlockNumberfrom it instead of the old arithmetic heuristicArchiverL1SynchronizergainsupdateFinalizedCheckpoint(), called every sync iterationKVArchiverDataStoreconstructor no longer takesl1Constants(theepochDurationit was used for is no longer needed)FakeL1Stateupdated to supportblockTag: 'finalized'andgetProvenCheckpointNumberwith a block number, enabling new sync tests