Skip to content

fix: repoint the archiver tips cache before a write commits - #25383

Closed
spalladino wants to merge 2 commits into
merge-train/spartan-v5from
spl/archiver-tips-cache-refresh-after
Closed

fix: repoint the archiver tips cache before a write commits#25383
spalladino wants to merge 2 commits into
merge-train/spartan-v5from
spl/archiver-tips-cache-refresh-after

Conversation

@spalladino

@spalladino spalladino commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Split out from #25357.

The race

The archiver serves getL2Tips() from an in-memory cache (L2TipsCache) that is reloaded after each store write commits, while getProposedCheckpointData() 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 from getProposedCheckpointData() but still-stale tips, and misclassifies the chain state — e.g. NodePublicCallsSimulator deriving 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 at write.then(reload) before the write commits. Registration happens synchronously in the same tick as transactionAsync, 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).
  • A rejected tips load (a failed post-commit reload, or a failed first load) is dropped from the cache after the failure surfaces, so the next read retries from the store — one transient failure cannot leave every subsequent getL2Tips() rejecting until the next write happens to come along.
  • Every store mutation in ArchiverDataStoreUpdater goes through a single writeAndRefreshTips helper 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-commit refresh() is deleted (no callers left).
  • Correctness relies on store writes committing in registration order, which holds because the LMDB store serializes write transactions through a single writer queue; this is now documented on the class.

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.

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.
@spalladino

Copy link
Copy Markdown
Contributor Author

Closing: pointing the tips cache at the in-flight write makes every getL2Tips() reader block for the duration of any write, including large addCheckpoints batches during initial sync. That latency change is not acceptable for a hot read path. #25384 closes the same race with an atomic archiver snapshot instead, so this PR is superseded.

@spalladino spalladino closed this Sep 1, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant