fix: only delete logs from rolled-back blocks, not entire tag (A-686)

yarn-project/archiver/src/store/kv_archiver_store.test.ts

@@ -1787,19 +1787,146 @@ describe('KVArchiverDataStore', () => {
     });
   });
 
-  it('deleteLogs', async () => {
-    const block = publishedCheckpoints[0].checkpoint.blocks[0];
-    await store.addProposedBlock(block);
-    await expect(store.addLogs([block])).resolves.toEqual(true);
+  describe('deleteLogs', () => {
+    it('deletes public logs for a block', async () => {
+      const block = publishedCheckpoints[0].checkpoint.blocks[0];
+      await store.addProposedBlock(block);
+      await expect(store.addLogs([block])).resolves.toEqual(true);
+
+      expect((await store.getPublicLogs({ fromBlock: BlockNumber(1) })).logs.length).toEqual(
+        block.body.txEffects.map(txEffect => txEffect.publicLogs).flat().length,
+      );
+
+      await store.deleteLogs([block]);
 
-    expect((await store.getPublicLogs({ fromBlock: BlockNumber(1) })).logs.length).toEqual(
-      block.body.txEffects.map(txEffect => txEffect.publicLogs).flat().length,
-    );
+      expect((await store.getPublicLogs({ fromBlock: BlockNumber(1) })).logs.length).toEqual(0);
+    });
 
-    // This one is a pain for memory as we would never want to just delete memory in the middle.
-    await store.deleteLogs([block]);
+    it('deletes contract class logs for a block', async () => {
+      // Create a block that explicitly has contract class logs
+      const block = await L2Block.random(BlockNumber(1), {
+        txsPerBlock: 2,
+        txOptions: { numContractClassLogs: 1 },
+        state: makeStateForBlock(1, 2),
+      });
+      await store.addProposedBlock(block);
+      await store.addLogs([block]);
+
+      const logsBefore = await store.getContractClassLogs({ fromBlock: BlockNumber(1) });
+      expect(logsBefore.logs.length).toBeGreaterThan(0);
+
+      await store.deleteLogs([block]);
+
+      const logsAfter = await store.getContractClassLogs({ fromBlock: BlockNumber(1) });
+      expect(logsAfter.logs.length).toEqual(0);
+    });
+
+    it('retains private logs from non-reorged block when same tag appears in reorged block', async () => {
+      const sharedTag = makePrivateLogTag(1, 0, 0);
+
+      // Block 1 with a private log using sharedTag
+      const cp1 = await makeCheckpointWithLogs(1, {
+        numTxsPerBlock: 1,
+        privateLogs: { numLogsPerTx: 1 },
+      });
+      const block1 = cp1.checkpoint.blocks[0];
+
+      // Block 2 with a private log using the SAME tag
+      const cp2 = await makeCheckpointWithLogs(2, {
+        previousArchive: block1.archive,
+        numTxsPerBlock: 1,
+        privateLogs: { numLogsPerTx: 1 },
+      });
+      const block2 = cp2.checkpoint.blocks[0];
+      // Override block2's private log tag to match block1's
+      block2.body.txEffects[0].privateLogs[0] = makePrivateLog(sharedTag);
+
+      await addProposedBlocks(store, [block1, block2], { force: true });
+      await store.addLogs([block1, block2]);
+
+      // Both blocks' logs should be present
+      const logsBefore = await store.getPrivateLogsByTags([sharedTag]);
+      expect(logsBefore[0]).toHaveLength(2);
+
+      // Reorg: delete block 2
+      await store.deleteLogs([block2]);
+
+      // Block 1's log should still be present
+      const logsAfter = await store.getPrivateLogsByTags([sharedTag]);
+      expect(logsAfter[0]).toHaveLength(1);
+      expect(logsAfter[0][0].blockNumber).toEqual(1);
+    });
 
-    expect((await store.getPublicLogs({ fromBlock: BlockNumber(1) })).logs.length).toEqual(0);
+    it('retains public logs from non-reorged block when same tag appears in reorged block', async () => {
+      const contractAddress = AztecAddress.fromNumber(543254);
+      const sharedTag = makePublicLogTag(1, 0, 0);
+
+      // Block 1 with a public log using sharedTag
+      const cp1 = await makeCheckpointWithLogs(1, {
+        numTxsPerBlock: 1,
+        publicLogs: { numLogsPerTx: 1, contractAddress },
+      });
+      const block1 = cp1.checkpoint.blocks[0];
+
+      // Block 2 with a public log using the SAME tag from the same contract
+      const cp2 = await makeCheckpointWithLogs(2, {
+        previousArchive: block1.archive,
+        numTxsPerBlock: 1,
+        publicLogs: { numLogsPerTx: 1, contractAddress },
+      });
+      const block2 = cp2.checkpoint.blocks[0];
+      // Override block2's public log tag to match block1's
+      block2.body.txEffects[0].publicLogs[0] = makePublicLog(sharedTag, contractAddress);
+
+      await addProposedBlocks(store, [block1, block2], { force: true });
+      await store.addLogs([block1, block2]);
+
+      // Both blocks' logs should be present
+      const logsBefore = await store.getPublicLogsByTagsFromContract(contractAddress, [sharedTag]);
+      expect(logsBefore[0]).toHaveLength(2);
+
+      // Reorg: delete block 2
+      await store.deleteLogs([block2]);
+
+      // Block 1's log should still be present
+      const logsAfter = await store.getPublicLogsByTagsFromContract(contractAddress, [sharedTag]);
+      expect(logsAfter[0]).toHaveLength(1);
+      expect(logsAfter[0][0].blockNumber).toEqual(1);
+    });
+
+    it('deletes multiple blocks at once', async () => {
+      const cp1 = await makeCheckpointWithLogs(1, {
+        numTxsPerBlock: 2,
+        privateLogs: { numLogsPerTx: 1 },
+        publicLogs: { numLogsPerTx: 1 },
+      });
+      const block1 = cp1.checkpoint.blocks[0];
+
+      const cp2 = await makeCheckpointWithLogs(2, {
+        previousArchive: block1.archive,
+        numTxsPerBlock: 2,
+        privateLogs: { numLogsPerTx: 1 },
+        publicLogs: { numLogsPerTx: 1 },
+      });
+      const block2 = cp2.checkpoint.blocks[0];
+
+      await addProposedBlocks(store, [block1, block2], { force: true });
+      await store.addLogs([block1, block2]);
+
+      // Verify logs exist
+      expect((await store.getPublicLogs({ fromBlock: BlockNumber(1) })).logs.length).toBeGreaterThan(0);
+
+      // Delete both blocks at once
+      await store.deleteLogs([block1, block2]);
+
+      expect((await store.getPublicLogs({ fromBlock: BlockNumber(1) })).logs.length).toEqual(0);
+    });
+
+    it('is a no-op when deleting blocks with no logs', async () => {
+      const block = publishedCheckpoints[0].checkpoint.blocks[0];
+      // Don't add logs, just try to delete
+      await expect(store.deleteLogs([block])).resolves.toEqual(true);
+    });
   });
 
   describe('getTxEffect', () => {

yarn-project/archiver/src/store/log_store.ts

@@ -1,6 +1,6 @@
 import { INITIAL_L2_BLOCK_NUM } from '@aztec/constants';
 import { BlockNumber } from '@aztec/foundation/branded-types';
-import { filterAsync } from '@aztec/foundation/collection';
+import { compactArray, filterAsync } from '@aztec/foundation/collection';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import { createLogger } from '@aztec/foundation/log';
 import { BufferReader, numToUInt32BE } from '@aztec/foundation/serialize';
@@ -313,18 +313,49 @@ export class LogStore {
 
   deleteLogs(blocks: L2Block[]): Promise<boolean> {
     return this.db.transactionAsync(async () => {
-      await Promise.all(
-        blocks.map(async block => {
-          // Delete private logs
-          const privateKeys = (await this.#privateLogKeysByBlock.getAsync(block.number)) ?? [];
-          await Promise.all(privateKeys.map(tag => this.#privateLogsByTag.delete(tag)));
-
-          // Delete public logs
-          const publicKeys = (await this.#publicLogKeysByBlock.getAsync(block.number)) ?? [];
-          await Promise.all(publicKeys.map(key => this.#publicLogsByContractAndTag.delete(key)));
-        }),
+      const blockNumbers = new Set(blocks.map(block => block.number));
+      const firstBlockToDelete = Math.min(...blockNumbers);
+
+      // Collect all unique private tags across all blocks being deleted
+      const allPrivateTags = new Set(
+        compactArray(await Promise.all(blocks.map(block => this.#privateLogKeysByBlock.getAsync(block.number)))).flat(),
+      );
+
+      // Trim private logs: for each tag, delete all instances including and after the first block being deleted.
+      // This hinges on the invariant that logs for a given tag are always inserted in order of block number, which is enforced in #addPrivateLogs.
+      for (const tag of allPrivateTags) {
+        const existing = await this.#privateLogsByTag.getAsync(tag);
+        if (existing === undefined || existing.length === 0) {
+          continue;
+        }
+        const lastIndexToKeep = existing.findLastIndex(
+          buf => TxScopedL2Log.getBlockNumberFromBuffer(buf) < firstBlockToDelete,
+        );
+        const remaining = existing.slice(0, lastIndexToKeep + 1);
+        await (remaining.length > 0 ? this.#privateLogsByTag.set(tag, remaining) : this.#privateLogsByTag.delete(tag));
+      }
+
+      // Collect all unique public keys across all blocks being deleted
+      const allPublicKeys = new Set(
+        compactArray(await Promise.all(blocks.map(block => this.#publicLogKeysByBlock.getAsync(block.number)))).flat(),
       );
 
+      // And do the same as we did with private logs
+      for (const key of allPublicKeys) {
+        const existing = await this.#publicLogsByContractAndTag.getAsync(key);
+        if (existing === undefined || existing.length === 0) {
+          continue;
+        }
+        const lastIndexToKeep = existing.findLastIndex(
+          buf => TxScopedL2Log.getBlockNumberFromBuffer(buf) < firstBlockToDelete,
+        );
+        const remaining = existing.slice(0, lastIndexToKeep + 1);
+        await (remaining.length > 0
+          ? this.#publicLogsByContractAndTag.set(key, remaining)
+          : this.#publicLogsByContractAndTag.delete(key));
+      }
+
+      // After trimming the tagged logs, we can delete the block-level keys that track which tags are in which blocks.
       await Promise.all(
         blocks.map(block =>
           Promise.all([

yarn-project/stdlib/src/logs/tx_scoped_l2_log.test.ts

@@ -0,0 +1,17 @@
+import { TxScopedL2Log } from './tx_scoped_l2_log.js';
+
+describe('TxScopedL2Log', () => {
+  it('should serialize and deserialize correctly', () => {
+    const log = TxScopedL2Log.random();
+    const buffer = log.toBuffer();
+    const deserializedLog = TxScopedL2Log.fromBuffer(buffer);
+    expect(deserializedLog.equals(log)).toBe(true);
+  });
+
+  it('should extract block number from buffer correctly', () => {
+    const log = TxScopedL2Log.random();
+    const buffer = log.toBuffer();
+    const blockNumber = TxScopedL2Log.getBlockNumberFromBuffer(buffer);
+    expect(blockNumber).toBe(log.blockNumber);
+  });
+});

yarn-project/stdlib/src/logs/tx_scoped_l2_log.ts

@@ -1,4 +1,5 @@
 import { BlockNumber, BlockNumberSchema } from '@aztec/foundation/branded-types';
+import { times } from '@aztec/foundation/collection';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import { schemas as foundationSchemas } from '@aztec/foundation/schemas';
 import {
@@ -83,6 +84,21 @@ export class TxScopedL2Log {
     return new TxScopedL2Log(txHash, blockNumber, blockTimestamp, logData, noteHashes, firstNullifier);
   }
 
+  static getBlockNumberFromBuffer(buffer: Buffer) {
+    return BlockNumber(buffer.readUint32BE(Fr.SIZE_IN_BYTES));
+  }
+
+  static random() {
+    return new TxScopedL2Log(
+      TxHash.fromField(Fr.random()),
+      BlockNumber(Math.floor(Math.random() * 100000) + 1),
+      BigInt(Math.floor(Date.now() / 1000)),
+      times(3, Fr.random),
+      times(3, Fr.random),
+      Fr.random(),
+    );
+  }
+
   equals(other: TxScopedL2Log) {
     return (
       this.txHash.equals(other.txHash) &&