From 882af4bc3d09a9ce226bc187d9d669b91b87b635 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:31:03 -0300 Subject: [PATCH 1/3] fix(archiver): authenticate every Inbox message recovery anchor on L1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stored message row keeps the L1 height it was first observed at, and recovery accepted any candidate recorded at or below the persisted finality marker as an anchor with no event lookup. That recorded height is never refreshed while the log agrees with the Inbox at the captured head, so a message re-mined to a higher block keeps its old, lower height — which can sit below the marker while the message itself is above it, unfinalized and replaceable. When such a message is later replaced, recovery keeps a prefix L1 no longer has: it rolls back from N to N, refetches, fails to chain the replacement onto the retained prefix, and restarts on the same false anchor. The node then makes no further progress on messages until it is restarted. Every anchor is now a message a bounded event lookup positively found on L1 at the same index and rolling hash. The per-pass lookup budget, the lookup window bounded above by the captured head, the deployment-block fallback and the single-transaction rollback are unchanged, and no new cache or persisted state is introduced. The finality marker is still written and advanced monotonically on authenticated syncs: what is removed is reliance on stale placement, not finality itself. The cost is that a recovery which used to stop early keeps walking backwards, and may prune proposed blocks whose messages sit below the marker; those messages return through ordinary forward ingestion. --- .../archiver/src/archiver-sync.test.ts | 66 ++++++++++++++----- .../src/modules/inbox_message_synchronizer.ts | 24 ++----- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index 67b5d09aea1f..bdc16e6787a6 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1114,7 +1114,7 @@ describe('Archiver Sync', () => { expect(await getStoredLeaves()).toEqual(asHex([msgs1[0], msgs1[1], msgs3[0], msgs3[1], msg40, msg50, msg51])); }); - it('short-circuits rollback at the finalized L1 block', async () => { + it('looks up a message recorded at the finalized L1 block before anchoring on it', async () => { // Sync two checkpoints worth of messages so we have history to roll back over. const msgs1 = [Fr.random(), Fr.random()]; fake.addMessages(CheckpointNumber(1), 100n, msgs1); @@ -1122,15 +1122,16 @@ describe('Archiver Sync', () => { const msgs3 = [Fr.random(), Fr.random(), Fr.random(), Fr.random()]; fake.addMessages(CheckpointNumber(3), 101n, msgs3); - // Mark block 100 as finalized so messages there cannot be reorged. + // Finality sits at block 100, the height the first two messages are recorded at. fake.setFinalizedL1BlockNumber(100n); fake.setL1BlockNumber(110n); await archiver.syncImmediate(); expect(await getStoredLeaves()).toEqual(asHex([...msgs1, ...msgs3])); - // Simulate L1 reorg: remove the last 2 messages from checkpoint 3 and add new ones. - fake.removeMessagesAfter(4); + // Simulate L1 reorg: remove every checkpoint-3 message and add a new one, so the search walks back past the + // messages recorded at the finalized block. + fake.removeMessagesAfter(2); const msg40 = Fr.random(); fake.addMessages(CheckpointNumber(4), 102n, [msg40]); @@ -1141,17 +1142,17 @@ describe('Archiver Sync', () => { await archiver.syncImmediate(); - // The two checkpoint-1 messages sit at L1 block 100 (≤ finalized). The rollback loop - // should stop there without issuing a per-message log query for them. + // A recorded height at or below the finalized block is a search hint, not proof that the message is still on + // the chain there, so the anchor at L1 block 100 is the one a lookup found. const callsAtFinalizedOrBelow = eventByHashSpy.mock.calls.filter( ([, aroundL1BlockNumber]) => aroundL1BlockNumber <= 100n, ); - expect(callsAtFinalizedOrBelow).toHaveLength(0); + expect(callsAtFinalizedOrBelow).toHaveLength(1); - expect(await getStoredLeaves()).toEqual(asHex([msgs1[0], msgs1[1], msgs3[0], msgs3[1], msg40])); + expect(await getStoredLeaves()).toEqual(asHex([msgs1[0], msgs1[1], msg40])); }); - it('falls back to per-message log queries when finalized block is undefined', async () => { + it('finds the common point with per-message log queries when no block is finalized yet', async () => { const msgs1 = [Fr.random(), Fr.random()]; fake.addMessages(CheckpointNumber(1), 100n, msgs1); @@ -1173,9 +1174,8 @@ describe('Archiver Sync', () => { await archiver.syncImmediate(); - // Without a finalized pointer the synchronizer must use per-message log queries to find the common point. The - // search starts below the canonical count (5), so msgs3[3] is never looked up: msgs3[2] mismatches on remote and - // msgs3[1] matches, anchoring the replay. + // The search starts below the canonical count (5), so msgs3[3] is never looked up: msgs3[2] mismatches on + // remote and msgs3[1] matches, anchoring the replay. expect(eventByHashSpy).toHaveBeenCalledTimes(2); expect(await getStoredLeaves()).toEqual(asHex([msgs1[0], msgs1[1], msgs3[0], msgs3[1], msg40])); @@ -1635,9 +1635,6 @@ describe('Archiver Sync', () => { }); it('discards a block consuming unchanged messages that were re-mined beyond the lookup window', async () => { - // The finalized marker stays below the messages' L1 block, so the inherited-finality shortcut cannot anchor - // them without a lookup. - fake.setFinalizedL1BlockNumber(95n); const msgs = randomLeaves(3); fake.addMessages(CheckpointNumber(1), 100n, msgs); fake.setL1BlockNumber(110n); @@ -2347,7 +2344,7 @@ describe('Archiver Sync', () => { expect(archiver.getL1BlockNumber()).toEqual(111n); }); - it('anchors on the finality marker of the last agreed sync, not on a fresher finalized height', async () => { + it('advances the persisted finality marker only once the log agrees with L1 again', async () => { const msgs = randomLeaves(4); fake.addMessages(CheckpointNumber(1), 100n, msgs.slice(0, 2)); fake.addMessages(CheckpointNumber(1), 101n, msgs.slice(2)); @@ -2356,8 +2353,8 @@ describe('Archiver Sync', () => { await archiver.syncImmediate(); expect((await archiverStore.messages.getMessagesFinalizedL1Block())?.l1BlockNumber).toEqual(90n); - // Every message is replaced and L1 finality has since moved past them. Trusting the fresh height would anchor - // on a stale message without a lookup and try to graft the replacement onto a prefix L1 no longer has. + // Every message is replaced and L1 finality has since moved past the heights they were recorded at. Recovery + // keeps nothing, and the marker the store carries through it does not stop the replacement from landing. const replacement = randomLeaves(4); fake.removeMessagesAfter(0); fake.addMessages(CheckpointNumber(1), 100n, replacement.slice(0, 2)); @@ -2373,6 +2370,39 @@ describe('Archiver Sync', () => { expect((await archiverStore.messages.getMessagesFinalizedL1Block())?.l1BlockNumber).toEqual(150n); }); + it('escapes a recovery whose stored messages sit below the finality marker but were re-mined above it', async () => { + const msgs = randomLeaves(2); + fake.addMessages(CheckpointNumber(1), 100n, msgs); + fake.setFinalizedL1BlockNumber(95n); + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex(msgs)); + + // L1 re-mines both messages thirty blocks later with their content, index and rolling hash intact. The log + // still agrees with the Inbox, so nothing refetches them and their stored rows keep the height they were first + // observed at, while finality passes that height without reaching where they now sit. + fake.moveMessagesToL1Block(100n, 130n); + fake.reorgL1BlocksFrom(100n); + fake.setFinalizedL1BlockNumber(120n); + fake.setL1BlockNumber(135n); + await archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex(msgs)); + expect((await archiverStore.messages.getMessagesFinalizedL1Block())?.l1BlockNumber).toEqual(120n); + + // L1 now replaces block 130 on, which is above the marker, so both messages really are gone. Anchoring on + // their recorded height would keep a prefix L1 no longer has and the refetch would never chain onto it. + const replacement = randomLeaves(2); + fake.removeMessagesAfter(0); + fake.addMessages(CheckpointNumber(2), 130n, replacement); + fake.reorgL1BlocksFrom(130n); + fake.setL1BlockNumber(140n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex(replacement)); + expect(archiver.getL1BlockNumber()).toEqual(140n); + expect(synchronizer.isRecoveringMessages()).toBe(false); + }); + it('re-verifies the persisted syncpoint before ingesting forward, so an empty batch cannot certify a stale tail', async () => { // One slot of L1 blocks per batch: two blocks. A arrives in block 2 and B in block 4; the log is synced to head 4. await useArchiver({ batchSize: 1 }); diff --git a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts index 6969d80bfa6e..b3b433b142de 100644 --- a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts +++ b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts @@ -115,10 +115,11 @@ export type InboxMessageRecoveryProgress = { * provider's view, not the chain's. When neither reading can settle the ambiguity the pass reports pending rather * than inventing evidence either way. * - * The inherited finalized-height shortcut is kept: a stored message observed at or below the finality marker - * persisted by the last sync that reached agreement with L1 is accepted as an anchor without a lookup, and the marker - * only advances on such agreement. A message re-mined above the finalized height whose old height was below it can - * therefore be trusted wrongly; that exception is deliberately retained and not repaired here. + * A stored message's recorded L1 height is a search hint and never evidence in itself: it says where the message was + * observed, not where the canonical chain carries it now, so every anchor is a message an event lookup found. That + * holds for heights at or below the persisted finality marker too. A message re-mined above the marker keeps the + * height it was first seen at, since a log that still agrees with the Inbox is never refetched, so its recorded + * height would claim a finality the message does not have and anchor the log on a prefix L1 has replaced. */ export class InboxMessageSynchronizer { private recovery: RecoveryState | undefined; @@ -482,14 +483,10 @@ export class InboxMessageSynchronizer { /** * Walks the local log backwards from the current candidate looking for a message L1 still emits at the same index * and hash, spending at most the per-pass lookup budget. Returns the prefix to roll back to, or undefined when the - * budget ran out first. A message at or below the finalized L1 block is accepted without a lookup; a search that - * runs out of candidates keeps nothing and starts again from the deployment block. + * budget ran out first. A search that runs out of candidates keeps nothing and starts again from the deployment + * block. */ private async searchAnchor(recovery: RecoveryState): Promise { - // Only the finality marker persisted by the last sync that reached agreement with L1 is trusted here: a fresher - // finalized height covers messages this node never verified against it, and trusting them would widen the - // inherited shortcut to whatever the local log happens to hold. - const finalizedL1Block = await this.stores.messages.getMessagesFinalizedL1Block(); let lookups = 0; while (true) { const candidateIndex = recovery.nextCandidateIndex; @@ -506,13 +503,6 @@ export class InboxMessageSynchronizer { // with no expected value rather than the Inbox's tip hash, which belongs to a different count entirely. throw new InboxMessagePrefixChangedError(candidateIndex + 1n, undefined, undefined); } - if (finalizedL1Block !== undefined && candidate.l1BlockNumber <= finalizedL1Block.l1BlockNumber) { - this.log.info(`Anchoring L1 to L2 message recovery at finalized L1 block ${candidate.l1BlockNumber}`, { - candidateIndex, - l1BlockNumber: candidate.l1BlockNumber, - }); - return { keep: positionAfter([candidate]), anchorL1Block: candidate.l1BlockNumber }; - } if (lookups >= this.opts.maxAnchorLookupsPerPass) { this.log.verbose(`L1 to L2 message anchor search paused after ${lookups} lookups`, this.getRecoveryProgress()); return undefined; From 32ad4977c3402acf80fe3649688adfff54e49eed Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 02:41:11 -0300 Subject: [PATCH 2/3] test(archiver): drain the block-triggered sync before reorging the head backwards addBlock triggers a sync it does not await, so the deployment-refill fixture left a pass in flight that had captured the pre-reorg head. Dropping the finalized-height shortcut made that pass do two event lookups and a rollback instead of returning at once, so it could now commit after the pass for the new, lower head and leave the old height as the synced one. The fixture failed about three times in ten; draining the pending sync before moving the head fixes it, 30 runs clean. Co-Authored-By: Claude Opus 5 (1M context) --- yarn-project/archiver/src/archiver-sync.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index bdc16e6787a6..6ad9cc15a701 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1942,6 +1942,10 @@ describe('Archiver Sync', () => { fake.setL1BlockNumber(115n); await archiver.syncImmediate(); await addLocalBlocksConsuming([4]); + // addBlock triggers a sync it does not await, and that pass captures the head as it is now. Drain it before + // moving the head backwards: left in flight, it recovers against the pre-reorg head and can commit after the + // pass below, leaving 115 as the synced height. + await archiver.syncImmediate(); // A replacement chain shorter than every stored height by more than the lookup window, carrying none of the // stored messages. Each candidate's window (95..105 and 105..115) starts above the new head, so bounding it by From cc6d63ac9448c85398fe04f040fa8f35e7060d72 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 11:29:33 -0300 Subject: [PATCH 3/3] test(archiver): drop the drain now covered by the local-block helper The helper this fixture calls drains the block-triggered sync itself, so the inline drain added here is a no-op second call. --- yarn-project/archiver/src/archiver-sync.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index 6ad9cc15a701..c44d3944278a 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1942,11 +1942,6 @@ describe('Archiver Sync', () => { fake.setL1BlockNumber(115n); await archiver.syncImmediate(); await addLocalBlocksConsuming([4]); - // addBlock triggers a sync it does not await, and that pass captures the head as it is now. Drain it before - // moving the head backwards: left in flight, it recovers against the pre-reorg head and can commit after the - // pass below, leaving 115 as the synced height. - await archiver.syncImmediate(); - // A replacement chain shorter than every stored height by more than the lookup window, carrying none of the // stored messages. Each candidate's window (95..105 and 105..115) starts above the new head, so bounding it by // the head inverts the range: no anchor is found, and the log rolls back to the deployment block.