From a70795b8403e7e6b7fc0729907637bb7e35cd0bb Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 20 Nov 2024 17:41:59 -0300 Subject: [PATCH] fix: Allow unwinding multiple empty blocks The archiver kv store used the block **body** hash as identifier to store the block bodies. This means that two empty blocks would have the same identifier. So, when unwinding (ie deleting) two or more empty blocks, the first one would remove the body that corresponds to an empty block, and the second one would fail with "Body could not be retrieved". This fixes the issue by using the block hash, guaranteed to be unique, as a key to store the block bodies. --- .../src/archiver/archiver_store_test_suite.ts | 29 ++++++++++++++++--- .../archiver/kv_archiver_store/block_store.ts | 18 +++++++----- .../memory_archiver_store.ts | 2 +- .../src/interfaces/archiver.test.ts | 2 +- .../src/interfaces/aztec-node.test.ts | 2 +- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/yarn-project/archiver/src/archiver/archiver_store_test_suite.ts b/yarn-project/archiver/src/archiver/archiver_store_test_suite.ts index 9253a99fede0..c974942d422b 100644 --- a/yarn-project/archiver/src/archiver/archiver_store_test_suite.ts +++ b/yarn-project/archiver/src/archiver/archiver_store_test_suite.ts @@ -38,12 +38,18 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch [5, 2, () => blocks.slice(4, 6)], ]; + const makeL1Published = (block: L2Block, l1BlockNumber: number): L1Published => ({ + data: block, + l1: { + blockNumber: BigInt(l1BlockNumber), + blockHash: `0x${l1BlockNumber}`, + timestamp: BigInt(l1BlockNumber * 1000), + }, + }); + beforeEach(() => { store = getStore(); - blocks = times(10, i => ({ - data: L2Block.random(i + 1), - l1: { blockNumber: BigInt(i + 10), blockHash: `0x${i}`, timestamp: BigInt(i * 1000) }, - })); + blocks = times(10, i => makeL1Published(L2Block.random(i + 1), i + 10)); }); describe('addBlocks', () => { @@ -69,6 +75,21 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch expect(await store.getSynchedL2BlockNumber()).toBe(blockNumber - 1); expect(await store.getBlocks(blockNumber, 1)).toEqual([]); }); + + it('can unwind multiple empty blocks', async () => { + const emptyBlocks = times(10, i => makeL1Published(L2Block.random(i + 1, 0), i + 10)); + await store.addBlocks(emptyBlocks); + expect(await store.getSynchedL2BlockNumber()).toBe(10); + + await store.unwindBlocks(10, 3); + expect(await store.getSynchedL2BlockNumber()).toBe(7); + expect((await store.getBlocks(1, 10)).map(b => b.data.number)).toEqual([1, 2, 3, 4, 5, 6, 7]); + }); + + it('refuses to unwind blocks if the tip is not the last block', async () => { + await store.addBlocks(blocks); + await expect(store.unwindBlocks(5, 1)).rejects.toThrow(/can only unwind blocks from the tip/i); + }); }); describe('getBlocks', () => { diff --git a/yarn-project/archiver/src/archiver/kv_archiver_store/block_store.ts b/yarn-project/archiver/src/archiver/kv_archiver_store/block_store.ts index 85bbacc0369b..91ae9d578c23 100644 --- a/yarn-project/archiver/src/archiver/kv_archiver_store/block_store.ts +++ b/yarn-project/archiver/src/archiver/kv_archiver_store/block_store.ts @@ -20,7 +20,7 @@ export class BlockStore { /** Map block number to block data */ #blocks: AztecMap; - /** Map block body hash to block body */ + /** Map block hash to block body */ #blockBodies: AztecMap; /** Stores L1 block number in which the last processed L2 block was included */ @@ -72,7 +72,7 @@ export class BlockStore { void this.#txIndex.set(tx.txHash.toString(), [block.data.number, i]); }); - void this.#blockBodies.set(block.data.body.getTxsEffectsHash().toString('hex'), block.data.body.toBuffer()); + void this.#blockBodies.set(block.data.hash().toString(), block.data.body.toBuffer()); } void this.#lastSynchedL1Block.set(blocks[blocks.length - 1].l1.blockNumber); @@ -92,7 +92,7 @@ export class BlockStore { return this.db.transaction(() => { const last = this.getSynchedL2BlockNumber(); if (from != last) { - throw new Error(`Can only remove from the tip`); + throw new Error(`Can only unwind blocks from the tip (requested ${from} but current tip is ${last})`); } for (let i = 0; i < blocksToUnwind; i++) { @@ -106,7 +106,9 @@ export class BlockStore { block.data.body.txEffects.forEach(tx => { void this.#txIndex.delete(tx.txHash.toString()); }); - void this.#blockBodies.delete(block.data.body.getTxsEffectsHash().toString('hex')); + const blockHash = block.data.hash().toString(); + void this.#blockBodies.delete(blockHash); + this.#log.debug(`Unwound block ${blockNumber} ${blockHash}`); } return true; @@ -154,10 +156,12 @@ export class BlockStore { private getBlockFromBlockStorage(blockStorage: BlockStorage) { const header = Header.fromBuffer(blockStorage.header); const archive = AppendOnlyTreeSnapshot.fromBuffer(blockStorage.archive); - - const blockBodyBuffer = this.#blockBodies.get(header.contentCommitment.txsEffectsHash.toString('hex')); + const blockHash = header.hash().toString(); + const blockBodyBuffer = this.#blockBodies.get(blockHash); if (blockBodyBuffer === undefined) { - throw new Error('Body could not be retrieved'); + throw new Error( + `Could not retrieve body for block ${header.globalVariables.blockNumber.toNumber()} ${blockHash}`, + ); } const body = Body.fromBuffer(blockBodyBuffer); diff --git a/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts b/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts index 7f5418f79a6a..33d8a09128d9 100644 --- a/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts +++ b/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts @@ -201,7 +201,7 @@ export class MemoryArchiverStore implements ArchiverDataStore { public async unwindBlocks(from: number, blocksToUnwind: number): Promise { const last = await this.getSynchedL2BlockNumber(); if (from != last) { - throw new Error(`Can only remove the tip`); + throw new Error(`Can only unwind blocks from the tip (requested ${from} but current tip is ${last})`); } const stopAt = from - blocksToUnwind; diff --git a/yarn-project/circuit-types/src/interfaces/archiver.test.ts b/yarn-project/circuit-types/src/interfaces/archiver.test.ts index 6e9dacdd574d..36947324e24f 100644 --- a/yarn-project/circuit-types/src/interfaces/archiver.test.ts +++ b/yarn-project/circuit-types/src/interfaces/archiver.test.ts @@ -230,7 +230,7 @@ describe('ArchiverApiSchema', () => { it('addContractArtifact', async () => { await context.client.addContractArtifact(AztecAddress.random(), artifact); - }); + }, 20_000); it('getContract', async () => { const address = AztecAddress.random(); diff --git a/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts b/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts index 7e33bc9400d5..5b32cdd1d1e5 100644 --- a/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts +++ b/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts @@ -207,7 +207,7 @@ describe('AztecNodeApiSchema', () => { it('addContractArtifact', async () => { await context.client.addContractArtifact(AztecAddress.random(), artifact); - }); + }, 20_000); it('getLogs(Encrypted)', async () => { const response = await context.client.getLogs(1, 1, LogType.ENCRYPTED);