fix: Allow unwinding multiple empty blocks (#10084)

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.

Author: spalladino

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<L2Block> => ({
+      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', () => {

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<number, BlockStorage>;
 
-  /** Map block body hash to block body */
+  /** Map block hash to block body */
   #blockBodies: AztecMap<string, Buffer>;
 
   /** 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);
 

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<boolean> {
     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;

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();

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);