fix: repoint the archiver tips cache before a write commits - #25383
Closed
spalladino wants to merge 2 commits into
Closed
fix: repoint the archiver tips cache before a write commits#25383spalladino wants to merge 2 commits into
spalladino wants to merge 2 commits into
Conversation
The archiver serves getL2Tips from an in-memory cache reloaded after each store write commits, while getProposedCheckpointData reads the store directly. Between a write committing and the cache reload finishing, a concurrent reader could pair pre-commit cached tips with post-commit store reads — e.g. misclassifying a just-promoted checkpoint as still in progress when deriving globals for the next block. L2TipsCache.refreshAfter now points the cache at the write's post-commit state before the write commits, so readers arriving in that window wait for committed state instead. The plain refresh() is replaced by a single writeAndRefreshTips helper in ArchiverDataStoreUpdater that wraps every store transaction and awaits the write and the reload together, so a failed write cannot leave the reload dangling as an unhandled rejection. If the write fails the cache keeps the tips it had. Split out from #25357, which adds the reader-side snapshot verification this complements.
…ure cannot poison reads A post-commit reload failure used to stay cached, so every getL2Tips() rejected until the next successful write - which on a quiet network may never come. The cache now drops a rejected promise (first load included) so the next read retries from the store, while the failure still surfaces to the writer. Also documents the serialized-writer ordering assumption and covers chained refreshes with tests.
Contributor
Author
|
Closing: pointing the tips cache at the in-flight write makes every |
spalladino
added a commit
that referenced
this pull request
Sep 7, 2026
#25384) Fixes A-1897. Part of A-1903. Design: https://claude.ai/code/artifact/e9b78bd2-501d-4c30-a81a-5a10cbeed392 Planning the next block needs several facts about the *same* instant: the L2 tips, the leading proposed (not-yet-L1-confirmed) checkpoint, the pending-chain validation status, and the headers those tips name. They came from separate reads with different backing — `getL2Tips()` served from the in-memory `L2TipsCache`, `getProposedCheckpointData()` and `getPendingChainValidationStatus()` read straight from the store, headers fetched by a further lookup — so an archiver write landing between them produced a torn set. The typical shape: a checkpoint that has just been promoted is gone from the proposed map while the cache still reports the pre-promotion tips, so the next block is classified as continuing an in-progress checkpoint and gets mis-priced. ### Design - `BlockStore.getL2Frontier(genesisBlockHash)` resolves the four tips, the leading proposed checkpoint, the proposed tip's `BlockHeader`, the latest checkpointed checkpoint (`CheckpointHeader` + its `L1PublishedData`), and the pending-chain validation status inside a single `db.transactionAsync`, which is a consistent LMDB read snapshot. It replaces `getL2TipsData`, reusing its body. Every added field comes from bytes the transaction already loaded: the latest block entry carries its header, the latest checkpoint entry carries the header the call already deserialized for the tip plus its L1 buffer, and the validation status is one singleton read. - `L2TipsCache` becomes `L2FrontierCache`: it holds the whole snapshot behind one promise, refreshed at exactly the points the tips cache refreshed before (every `ArchiverDataStoreUpdater` write, after the transaction commits). `getL2Tips()` stays and derives from the snapshot. Because readers get one immutable object, refresh ordering versus commit no longer affects consistency — only freshness — so a plain post-commit refresh suffices. - The snapshot carries an `l1SyncPoint: { blockNumber, blockHash }`, the L1 block whose state its data reflects. `L1Synchronizer.syncFromL1` sets it at the top of a pass, right after the no-new-block early return and **before any of that pass's writes**, via `L2FrontierCache.setL1SyncPoint` (which rewrites the cached object with no store read). The ordering is the invariant: the pass's writes reflect L1 state up to that block, so the anchor must move ahead of the data and never behind it. A reader that saw data from L1 block N under an anchor of N-1 would price a fee at N-1 while the data already includes a checkpoint that landed at N. Data behind the anchor is harmless, because the overrides plan derived from the snapshot fully describes the parent. - `L2BlockSource.getL2Frontier()` is the new interface method, with the `L2Frontier` type and zod schema in `l2_block_source.ts` and an entry in `ArchiverApiSchema` so the whole snapshot round-trips over JSON-RPC. `getCheckpointedTipSlot(frontier)` is a free helper over the checkpointed checkpoint's header, so the slot has one source rather than a duplicated field. ### Consumers migrated - `NodePublicCallsSimulator` makes one `getL2Frontier()` call and performs **no** block lookup at all. The mid-checkpoint globals copy reads `frontier.latestBlockHeader` instead of `getBlockData({ number })`; a missing header at a non-genesis proposed tip is now an invariant violation and throws, rather than falling through to the boundary path and double-inserting the ongoing checkpoint's L1-to-L2 messages. The slot floor reads the checkpointed checkpoint's header slot from the same snapshot, deleting the by-hash header read #25357 added. `buildSimulationOverridesPlan` reads `frontier.pendingChainValidationStatus` instead of calling `getPendingChainValidationStatus()`. - The sequencer's `checkSync` takes tips, proposed checkpoint and validation status from one `getL2Frontier()` instead of three reads in the same `Promise.all`; the orphan-block guard now compares the block against a pair from the same snapshot, and the comment claiming there was no split read to reconcile is corrected. `checkSync` also now looks the tip block up **by hash** instead of by number: the tips come from the cached snapshot, so after a prune-and-replace commits (but before the cache refreshes) a by-number read could return the replacement block while every hash check in the function still described the pruned one (pre-existing, surfaced by review). The archiver's `getBlockData` resolves a hash matching the initial header to the genesis sentinel, so the genesis case still works. - `automine_sequencer.ts` replaces its unguarded `Promise.all([getL2Tips(), getProposedCheckpointData()])` with one `getL2Frontier()`. - Standalone by-number/by-slot proposed-checkpoint queries stay store-backed and unchanged. `TXEArchiver` and `MockL2BlockSource` implement the new method directly (neither ever holds a proposed checkpoint, and both report a valid pending chain and no L1 sync point). ### Tests - `block_store.test.ts`: `getL2Frontier` on an empty store (no headers, valid pending chain), the tips / proposed checkpoint / block header / checkpointed header + L1 data together, the validation status read in the same snapshot, and a promotion moving the checkpointed tip and dropping the proposed entry within one snapshot. - `data_store_updater.test.ts`: a reader that lands in the window after a promotion commits but before the cache refreshes gets a self-consistent snapshot, header included. Verified red — with the pair split back apart the frontier and the proposed tip disagree (`Expected: 1, Received: 0`) — and green with the atomic read. - `l2_frontier_cache.test.ts`: warm reads do not hit the store; a refresh reloads once; a sync point set before or after the first load is attached without a store read and survives a refresh; each update yields a fresh object, so a snapshot a reader already holds cannot change under it. - `archiver-sync.test.ts`: the anchor-before-writes invariant, asserted over a real synchronizer and store. Verified red by moving `setL1SyncPoint` to the end of `syncFromL1` (`Expected: < 18, Received: 23`) and green with the call in place. - `node_public_calls_simulator.test.ts`: rewritten around a `makeFrontier` helper. The floor tests keep their coverage but now read the slot from the snapshot; the by-hash-lookup and torn-snapshot tests are replaced by "no block read happens at all" and "a snapshot missing the proposed tip header is rejected"; genesis still lets the clock win because no checkpoint has landed yet. - `archiver.test.ts` in stdlib covers the JSON-RPC round-trip for every new field (bigint and `Buffer32` included). ### Notes - Stacked on #25357, which introduces the slot floor this PR rewires onto the snapshot. GitHub retargets this PR to `merge-train/spartan-v5` automatically once #25357 merges. - The next PR in the series drives `FeeProviderImpl` from `l1SyncPoint`: it stops polling L1 for its own head, refreshes when the archiver's anchor advances, and pins every fee read to that block, so the quote and the simulation can no longer disagree about which checkpoints exist on L1. - `l1SyncPoint` is set when a sync pass starts, before any write, so the L2 data is never ahead of its anchor. The other direction is possible while a pass is in flight, and during a same-height L1 reorg the anchor points at the new fork until that pass reconciles the frontier; a pass that fails part-way leaves the anchor at the new block, and the next pass retries since the "no new L1 block" early return is only armed on success. That window was the same before this series, when fees were read at `latest`. - Supersedes #25383 (writer-side `L2TipsCache.refreshAfter`), now closed. - Deployments with a **remote archiver**: migrated clients call `getL2Frontier` unconditionally, so a new node against an older remote archiver gets method-not-found until the archiver is upgraded. Upgrade the archiver first (or together, as our deployments do); a client-side fallback to the old read pair would reintroduce the torn snapshot this PR removes, so none is provided.
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
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.
Split out from #25357.
The race
The archiver serves
getL2Tips()from an in-memory cache (L2TipsCache) that is reloaded after each store write commits, whilegetProposedCheckpointData()reads the store directly. Between a write committing and the cache reload finishing, a concurrent reader can pair pre-commit cached tips with post-commit store reads. Concretely: a checkpoint promotion commits, a reader then sees the promotion gone fromgetProposedCheckpointData()but still-stale tips, and misclassifies the chain state — e.g.NodePublicCallsSimulatorderiving next-block globals concludes the next block continues a checkpoint that is in fact already checkpointed, mis-pricing its fee.#25357 adds reader-side detection (read tips, read the proposed checkpoint, re-read tips, retry on movement), which catches any write that moves the tips between the two halves — but both tips reads can land inside the commit-to-reload window and hit the same stale cache, so the reader-side check alone cannot close the race. This PR closes it from the writer side.
The fix
L2TipsCache.refreshAfter(write)points the cache atwrite.then(reload)before the write commits. Registration happens synchronously in the same tick astransactionAsync, so the commit cannot beat it: any reader arriving after the write starts waits for post-commit state instead of being served pre-commit tips. If the write fails, the cache keeps the tips it had (nothing was committed).getL2Tips()rejecting until the next write happens to come along.ArchiverDataStoreUpdatergoes through a singlewriteAndRefreshTipshelper that wraps the transaction and awaits the write and the reload together (Promise.all), so a failed write cannot leave the reload dangling as an unhandled rejection. The plain post-commitrefresh()is deleted (no callers left).Tests
l2_tips_cache.test.ts: a reader arriving before the write resolves gets the post-commit tips; a failed write keeps the previous tips; a failed post-commit reload surfaces to the caller and the next read recovers; a failed first load recovers; chained refreshes end at the newest state, including when the first of two writes fails.data_store_updater.test.ts: parks a checkpoint promotion in the commit-to-return window and asserts a concurrent reader is served tips consistent with the store (the proposed-checkpoint frontier and the proposed tip describe the same chain).Note
A-1897 tracks the deeper fix: one atomic
getChainStatus()snapshot (tips + proposed checkpoint read in a single store transaction, cached in memory), which would make both the reader-side retry in #25357 and this writer-side mechanism unnecessary. If A-1897 lands soon, this PR can be dropped in its favor; until then this closes the live race.