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..14a8361aa242 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,43 @@ describe('ArchiverDataStoreUpdater', () => { expect(publicLogsAfter.logs.length).toBe(0); }); }); + + describe('l2 tips cache refresh', () => { + // 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); + + const tipsBefore = await tipsCache.getL2Tips(); + + const block = await L2Block.random(BlockNumber(1), { + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + }); + + // 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); + + transactionSpy.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(), - }; - } }