From 58fab4fccf59d365d57c74292a887b5eee4fdfdd Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 23 Jun 2026 12:11:15 -0300 Subject: [PATCH 1/2] fix(archiver): harden v4 l2 tips cache refresh --- .../src/modules/data_store_updater.test.ts | 24 ++++ .../src/modules/data_store_updater.ts | 15 +- .../archiver/src/store/block_store.ts | 135 ++++++++++++++++++ .../archiver/src/store/l2_tips_cache.ts | 76 +--------- 4 files changed, 174 insertions(+), 76 deletions(-) diff --git a/yarn-project/archiver/src/modules/data_store_updater.test.ts b/yarn-project/archiver/src/modules/data_store_updater.test.ts index 360efd21eef3..86a8d9deb915 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.test.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.test.ts @@ -8,11 +8,13 @@ import { L2Block } from '@aztec/stdlib/block'; import { ContractClassLog, PrivateLog } from '@aztec/stdlib/logs'; import '@aztec/stdlib/testing/jest'; +import { jest } from '@jest/globals'; import { readFileSync } from 'fs'; import { dirname, resolve } from 'path'; import { fileURLToPath } from 'url'; import { KVArchiverDataStore } from '../store/kv_archiver_store.js'; +import { L2TipsCache } from '../store/l2_tips_cache.js'; import { makeCheckpoint, makePublishedCheckpoint } from '../test/mock_structs.js'; import { ArchiverDataStoreUpdater } from './data_store_updater.js'; @@ -215,4 +217,26 @@ describe('ArchiverDataStoreUpdater', () => { expect(publicLogsAfter.logs.length).toBe(0); }); }); + + describe('l2 tips cache refresh', () => { + it('does not refresh the cache when the writer transaction aborts', async () => { + const tipsCache = new L2TipsCache(store.blockStore); + const updaterWithCache = new ArchiverDataStoreUpdater(store, tipsCache); + + const tipsBefore = await tipsCache.getL2Tips(); + + const block = await L2Block.random(BlockNumber(1), { + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + }); + + const failure = new Error('forced failure inside writer transaction'); + const addProposedBlockSpy = jest.spyOn(store.blockStore, 'addProposedBlock').mockRejectedValueOnce(failure); + + await expect(updaterWithCache.addProposedBlock(block)).rejects.toBe(failure); + await expect(tipsCache.getL2Tips()).resolves.toEqual(tipsBefore); + + addProposedBlockSpy.mockRestore(); + }); + }); }); diff --git a/yarn-project/archiver/src/modules/data_store_updater.ts b/yarn-project/archiver/src/modules/data_store_updater.ts index b1b8e737e328..33cb8bd70de6 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.ts @@ -80,9 +80,9 @@ export class ArchiverDataStoreUpdater { this.addContractDataToDb(block), ]); - await this.l2TipsCache?.refresh(); return opResults.every(Boolean); }); + await this.l2TipsCache?.refresh(); return result; } @@ -125,9 +125,9 @@ export class ArchiverDataStoreUpdater { ...newBlocks.map(block => this.addContractDataToDb(block)), ]); - await this.l2TipsCache?.refresh(); return { prunedBlocks, lastAlreadyInsertedBlockNumber }; }); + await this.l2TipsCache?.refresh(); return result; } @@ -224,9 +224,9 @@ export class ArchiverDataStoreUpdater { } const result = await this.removeBlocksAfter(blockNumber); - await this.l2TipsCache?.refresh(); return result; }); + await this.l2TipsCache?.refresh(); return result; } @@ -257,7 +257,7 @@ export class ArchiverDataStoreUpdater { * @returns True if the operation is successful. */ public async removeCheckpointsAfter(checkpointNumber: CheckpointNumber): Promise { - return await this.store.transactionAsync(async () => { + const result = await this.store.transactionAsync(async () => { const { blocksRemoved = [] } = await this.store.removeCheckpointsAfter(checkpointNumber); const opResults = await Promise.all([ @@ -268,9 +268,10 @@ export class ArchiverDataStoreUpdater { this.store.deleteLogs(blocksRemoved), ]); - await this.l2TipsCache?.refresh(); return opResults.every(Boolean); }); + await this.l2TipsCache?.refresh(); + return result; } /** @@ -280,8 +281,8 @@ export class ArchiverDataStoreUpdater { public async setProvenCheckpointNumber(checkpointNumber: CheckpointNumber): Promise { await this.store.transactionAsync(async () => { await this.store.setProvenCheckpointNumber(checkpointNumber); - await this.l2TipsCache?.refresh(); }); + await this.l2TipsCache?.refresh(); } /** @@ -291,8 +292,8 @@ export class ArchiverDataStoreUpdater { public async setFinalizedCheckpointNumber(checkpointNumber: CheckpointNumber): Promise { await this.store.transactionAsync(async () => { await this.store.setFinalizedCheckpointNumber(checkpointNumber); - await this.l2TipsCache?.refresh(); }); + await this.l2TipsCache?.refresh(); } /** Extracts and stores contract data from a single block. */ diff --git a/yarn-project/archiver/src/store/block_store.ts b/yarn-project/archiver/src/store/block_store.ts index fa611f0d8470..2be44525b85e 100644 --- a/yarn-project/archiver/src/store/block_store.ts +++ b/yarn-project/archiver/src/store/block_store.ts @@ -14,7 +14,10 @@ import { Body, CheckpointedL2Block, CommitteeAttestation, + GENESIS_CHECKPOINT_HEADER_HASH, L2Block, + type L2TipId, + type L2Tips, type ValidateCheckpointResult, deserializeValidateCheckpointResult, serializeValidateCheckpointResult, @@ -625,6 +628,138 @@ export class BlockStore { : BlockNumber(INITIAL_L2_BLOCK_NUM - 1); } + /** + * Resolves all L2 chain tips in a single read transaction so the snapshot is internally consistent. + * The result is guaranteed to satisfy finalized <= proven <= checkpointed <= proposed by block number. + */ + async getL2TipsData(genesisBlockHash: Fr): Promise { + return await this.db.transactionAsync(async () => { + const genesisBlockNumber = BlockNumber(INITIAL_L2_BLOCK_NUM - 1); + const genesisCheckpointNumber = CheckpointNumber(INITIAL_CHECKPOINT_NUMBER - 1); + const genesisBlockId = { number: genesisBlockNumber, hash: genesisBlockHash.toString() }; + const genesisCheckpointId = { + number: genesisCheckpointNumber, + hash: GENESIS_CHECKPOINT_HEADER_HASH.toString(), + }; + const genesisTip: L2TipId = { block: genesisBlockId, checkpoint: genesisCheckpointId }; + + const [latestBlockEntry] = await toArray(this.#blocks.entriesAsync({ reverse: true, limit: 1 })); + const [latestCheckpointEntry] = await toArray(this.#checkpoints.entriesAsync({ reverse: true, limit: 1 })); + const latestCheckpointNumber = latestCheckpointEntry + ? CheckpointNumber(latestCheckpointEntry[0]) + : genesisCheckpointNumber; + + const [provenRaw, finalizedRaw] = await Promise.all([ + this.#lastProvenCheckpoint.getAsync(), + this.#lastFinalizedCheckpoint.getAsync(), + ]); + + const provenCheckpointNumber = CheckpointNumber(Math.min(provenRaw ?? 0, latestCheckpointNumber)); + const finalizedCheckpointNumber = CheckpointNumber(Math.min(finalizedRaw ?? 0, provenCheckpointNumber)); + + const checkpointStorageCache = new Map(); + if (latestCheckpointEntry) { + checkpointStorageCache.set(CheckpointNumber(latestCheckpointEntry[0]), latestCheckpointEntry[1]); + } + + const loadCheckpointStorage = async ( + checkpointNumber: CheckpointNumber, + ): Promise => { + if (checkpointNumber === genesisCheckpointNumber) { + return undefined; + } + if (!checkpointStorageCache.has(checkpointNumber)) { + const checkpointStorage = await this.#checkpoints.getAsync(checkpointNumber); + if (!checkpointStorage) { + throw new CheckpointNotFoundError(checkpointNumber); + } + checkpointStorageCache.set(checkpointNumber, checkpointStorage); + } + return checkpointStorageCache.get(checkpointNumber)!; + }; + + const [provenCheckpoint, finalizedCheckpoint] = await Promise.all([ + loadCheckpointStorage(provenCheckpointNumber), + loadCheckpointStorage(finalizedCheckpointNumber), + ]); + + const blockHashCache = new Map(); + blockHashCache.set(genesisBlockNumber, genesisBlockHash.toString()); + if (latestBlockEntry) { + blockHashCache.set( + BlockNumber(latestBlockEntry[0]), + BlockHash.fromBuffer(latestBlockEntry[1].blockHash).toString(), + ); + } + + const loadBlockHash = async (blockNumber: BlockNumber): Promise => { + if (!blockHashCache.has(blockNumber)) { + const blockStorage = await this.#blocks.getAsync(blockNumber); + if (!blockStorage) { + throw new BlockNotFoundError(blockNumber); + } + blockHashCache.set(blockNumber, BlockHash.fromBuffer(blockStorage.blockHash).toString()); + } + return blockHashCache.get(blockNumber)!; + }; + + const proposed = + latestBlockEntry === undefined + ? genesisBlockId + : { + number: BlockNumber(latestBlockEntry[0]), + hash: BlockHash.fromBuffer(latestBlockEntry[1].blockHash).toString(), + }; + + const buildTipFromCheckpoint = async (stored: CheckpointStorage | undefined): Promise => { + if (!stored) { + return genesisTip; + } + const blockNumber = BlockNumber(stored.startBlock + stored.blockCount - 1); + return { + block: { number: blockNumber, hash: await loadBlockHash(blockNumber) }, + checkpoint: { + number: CheckpointNumber(stored.checkpointNumber), + hash: CheckpointHeader.fromBuffer(stored.header).hash().toString(), + }, + }; + }; + + const [checkpointed, proven, finalized] = await Promise.all([ + buildTipFromCheckpoint(latestCheckpointEntry?.[1]), + buildTipFromCheckpoint(provenCheckpoint), + buildTipFromCheckpoint(finalizedCheckpoint), + ]); + + if (proposed.number < checkpointed.block.number) { + throw new Error( + `Inconsistent block store: latest block ${proposed.number} is behind checkpointed block ${checkpointed.block.number}`, + ); + } + + if ( + finalized.checkpoint.number > proven.checkpoint.number || + proven.checkpoint.number > checkpointed.checkpoint.number + ) { + throw new Error( + `Inconsistent checkpoint numbers in chain tips: finalized=${finalized.checkpoint.number} proven=${proven.checkpoint.number} checkpointed=${checkpointed.checkpoint.number}`, + ); + } + + if ( + finalized.block.number > proven.block.number || + proven.block.number > checkpointed.block.number || + checkpointed.block.number > proposed.number + ) { + throw new Error( + `Inconsistent block numbers in chain tips: finalized=${finalized.block.number} proven=${proven.block.number} checkpointed=${checkpointed.block.number} proposed=${proposed.number}`, + ); + } + + return { proposed, checkpointed, proven, finalized }; + }); + } + async getLatestCheckpointNumber(): Promise { const [latestCheckpointNumber] = await toArray(this.#checkpoints.keysAsync({ reverse: true, limit: 1 })); if (latestCheckpointNumber === undefined) { diff --git a/yarn-project/archiver/src/store/l2_tips_cache.ts b/yarn-project/archiver/src/store/l2_tips_cache.ts index 64a0192e7624..1cd530405088 100644 --- a/yarn-project/archiver/src/store/l2_tips_cache.ts +++ b/yarn-project/archiver/src/store/l2_tips_cache.ts @@ -1,13 +1,13 @@ -import { GENESIS_BLOCK_HEADER_HASH, INITIAL_L2_BLOCK_NUM } from '@aztec/constants'; -import { BlockNumber, CheckpointNumber } from '@aztec/foundation/branded-types'; -import { type BlockData, type CheckpointId, GENESIS_CHECKPOINT_HEADER_HASH, type L2Tips } from '@aztec/stdlib/block'; +import { GENESIS_BLOCK_HEADER_HASH } from '@aztec/constants'; +import type { L2Tips } from '@aztec/stdlib/block'; import type { BlockStore } from './block_store.js'; /** * In-memory cache for L2 chain tips (proposed, checkpointed, proven, finalized). * Populated from the BlockStore on first access, then kept up-to-date by the ArchiverDataStoreUpdater. - * Refresh calls should happen within the store transaction that mutates block data to ensure consistency. + * Refresh calls should happen after the store transaction that mutates block data has committed, + * so the cache loads from committed state and is never replaced if the writer aborts. */ export class L2TipsCache { #tipsPromise: Promise | undefined; @@ -16,74 +16,12 @@ export class L2TipsCache { /** Returns the cached L2 tips. Loads from the block store on first call. */ public getL2Tips(): Promise { - return (this.#tipsPromise ??= this.loadFromStore()); + return (this.#tipsPromise ??= this.blockStore.getL2TipsData(GENESIS_BLOCK_HEADER_HASH)); } - /** Reloads the L2 tips from the block store. Should be called within the store transaction that mutates data. */ + /** Reloads the L2 tips from the block store. Should be called after the writer transaction has committed. */ public async refresh(): Promise { - this.#tipsPromise = this.loadFromStore(); + this.#tipsPromise = this.blockStore.getL2TipsData(GENESIS_BLOCK_HEADER_HASH); await this.#tipsPromise; } - - private async loadFromStore(): Promise { - const [latestBlockNumber, provenBlockNumber, checkpointedBlockNumber, finalizedBlockNumber] = await Promise.all([ - this.blockStore.getLatestBlockNumber(), - this.blockStore.getProvenBlockNumber(), - this.blockStore.getCheckpointedL2BlockNumber(), - this.blockStore.getFinalizedL2BlockNumber(), - ]); - - const genesisBlockHeader = { - blockHash: GENESIS_BLOCK_HEADER_HASH, - checkpointNumber: CheckpointNumber.ZERO, - } as const; - const beforeInitialBlockNumber = BlockNumber(INITIAL_L2_BLOCK_NUM - 1); - - const getBlockData = (blockNumber: BlockNumber) => - blockNumber > beforeInitialBlockNumber ? this.blockStore.getBlockData(blockNumber) : genesisBlockHeader; - - const [latestBlockData, provenBlockData, checkpointedBlockData, finalizedBlockData] = await Promise.all( - [latestBlockNumber, provenBlockNumber, checkpointedBlockNumber, finalizedBlockNumber].map(getBlockData), - ); - - if (!latestBlockData || !provenBlockData || !finalizedBlockData || !checkpointedBlockData) { - throw new Error('Failed to load block data for L2 tips'); - } - - const [provenCheckpointId, finalizedCheckpointId, checkpointedCheckpointId] = await Promise.all([ - this.getCheckpointIdForBlock(provenBlockData), - this.getCheckpointIdForBlock(finalizedBlockData), - this.getCheckpointIdForBlock(checkpointedBlockData), - ]); - - return { - proposed: { number: latestBlockNumber, hash: latestBlockData.blockHash.toString() }, - proven: { - block: { number: provenBlockNumber, hash: provenBlockData.blockHash.toString() }, - checkpoint: provenCheckpointId, - }, - finalized: { - block: { number: finalizedBlockNumber, hash: finalizedBlockData.blockHash.toString() }, - checkpoint: finalizedCheckpointId, - }, - checkpointed: { - block: { number: checkpointedBlockNumber, hash: checkpointedBlockData.blockHash.toString() }, - checkpoint: checkpointedCheckpointId, - }, - }; - } - - private async getCheckpointIdForBlock(blockData: Pick): Promise { - const checkpointData = await this.blockStore.getCheckpointData(blockData.checkpointNumber); - if (!checkpointData) { - return { - number: CheckpointNumber.ZERO, - hash: GENESIS_CHECKPOINT_HEADER_HASH.toString(), - }; - } - return { - number: checkpointData.checkpointNumber, - hash: checkpointData.header.hash().toString(), - }; - } } From 068728d96947aede67268e5d6310811454017719 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 23 Jun 2026 14:43:47 -0300 Subject: [PATCH 2/2] test(archiver): make l2 tips cache abort test catch the in-transaction refresh regression The previous test mocked the first writer op to reject, so refresh() was never reached on abort in either layout and the test passed even without the fix. Inject the failure at commit time instead, after the transaction body (and any in-transaction refresh) has run, so a regressed implementation that refreshes inside the writer transaction pollutes the cache with the rolled-back block. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/modules/data_store_updater.test.ts | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/yarn-project/archiver/src/modules/data_store_updater.test.ts b/yarn-project/archiver/src/modules/data_store_updater.test.ts index 86a8d9deb915..14a8361aa242 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.test.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.test.ts @@ -219,7 +219,12 @@ describe('ArchiverDataStoreUpdater', () => { }); describe('l2 tips cache refresh', () => { - it('does not refresh the cache when the writer transaction aborts', async () => { + // Regression guard for the v4 block-stream/cache race (A-1235): the tips cache must never be replaced with + // tips derived from a writer transaction's uncommitted view. The failure is injected at commit time, i.e. + // *after* the transaction body has run, so a regressed implementation that refreshes the cache inside the + // writer transaction would have already read the (rolled-back) block and polluted the cache. Aborting on the + // first write instead cannot catch this, since the in-transaction refresh would never be reached. + it('does not replace the tips cache when the writer transaction aborts at commit', async () => { const tipsCache = new L2TipsCache(store.blockStore); const updaterWithCache = new ArchiverDataStoreUpdater(store, tipsCache); @@ -230,13 +235,25 @@ describe('ArchiverDataStoreUpdater', () => { indexWithinCheckpoint: IndexWithinCheckpoint(0), }); - const failure = new Error('forced failure inside writer transaction'); - const addProposedBlockSpy = jest.spyOn(store.blockStore, 'addProposedBlock').mockRejectedValueOnce(failure); + // Run the real writer transaction (so its body observes the written block via read-your-writes), then throw + // once the body has completed to simulate a commit failure that rolls the whole transaction back. + const failure = new Error('forced failure committing writer transaction'); + const realTransactionAsync = store.transactionAsync.bind(store); + const transactionSpy = jest.spyOn(store, 'transactionAsync').mockImplementation( + (callback: () => Promise) => + // The wrapped thunk always rejects, so the resolved type is irrelevant; cast to satisfy the generic signature. + realTransactionAsync(async () => { + await callback(); + throw failure; + }) as Promise, + ); await expect(updaterWithCache.addProposedBlock(block)).rejects.toBe(failure); + + // The aborted transaction rolled the block back, so the cache must still reflect committed (genesis) state. await expect(tipsCache.getL2Tips()).resolves.toEqual(tipsBefore); - addProposedBlockSpy.mockRestore(); + transactionSpy.mockRestore(); }); }); });