refactor!: expanding return value of getLogsByTags

Fixes F-231
Fixes #9789

In this PR I modify the the return value of our "get logs by tags" endpoints such that we don't need to perform another request for tx hash in PXE's log service. Since I was already modifying it I also dropped unused values from the type and made it a bit less messy.

I added here migration notes for all the changes I did to the endpoint not only in this PR but also in PRs down the stack.

Author: benesjan

docs/docs-developers/docs/resources/migration_notes.md

@@ -9,26 +9,34 @@ Aztec is in full-speed development. Literally every version breaks compatibility
 
 ## TBD
 
+### [Aztec Node] changes to `getLogsByTags` endpoint
+
+`getLogsByTags` endpoint has been optimized for our new log sync algorithm and these are the changes:
+
+- The `logsPerTag` pagination argument has been removed. Pagination was unnecessary here, since multiple logs per tag typically only occur if several devices are sending logs from the same sender to a recipient, which is unlikely to generate enough logs to require pagination.
+- The structure of `TxScopedL2Log` has been revised to meet the requirements of our new log sync algorithm.
+- The endpoint has been separated into two versions: `getPrivateLogsByTags` and `getPublicLogsByTagsFromContract`. This change was made because it was never desirable in PXE to mix public and private logs. The public version requires both a `Tag` and a contract address as input. In contrast to the private version—which uses `SiloedTag` (a tag that hashes the raw tag with the emitting contract's address)—the public version uses the raw `Tag` type, since kernels do not hash the tag with the contract address for public logs.
+
 ### [AVM] Gas cost multipliers for public execution to reach simulation/proving parity
 
 Gas costs for several AVM opcodes have been adjusted with multipliers to better align public simulation costs with actual proving costs.
 
-| Opcode | Multiplier | Previous Cost | New Cost |
-|--------|------------|---------------|----------|
-| FDIV | 25x | 9 | 225 |
-| SLOAD | 10x | 129 | 1,290 |
-| SSTORE | 20x | 1,657 | 33,140 |
-| NOTEHASHEXISTS | 4x | 126 | 504 |
-| EMITNOTEHASH | 15x | 1,285 | 19,275 |
-| NULLIFIEREXISTS | 7x | 132 | 924 |
-| EMITNULLIFIER | 20x | 1,540 | 30,800 |
-| L1TOL2MSGEXISTS | 5x | 108 | 540 |
-| SENDL2TOL1MSG | 2x | 209 | 418 |
-| CALL | 3x | 3,312 | 9,936 |
-| STATICCALL | 3x | 3,312 | 9,936 |
-| GETCONTRACTINSTANCE | 4x | 1,527 | 6,108 |
-| POSEIDON2 | 15x | 24 | 360 |
-| ECADD | 10x | 27 | 270 |
+| Opcode              | Multiplier | Previous Cost | New Cost |
+| ------------------- | ---------- | ------------- | -------- |
+| FDIV                | 25x        | 9             | 225      |
+| SLOAD               | 10x        | 129           | 1,290    |
+| SSTORE              | 20x        | 1,657         | 33,140   |
+| NOTEHASHEXISTS      | 4x         | 126           | 504      |
+| EMITNOTEHASH        | 15x        | 1,285         | 19,275   |
+| NULLIFIEREXISTS     | 7x         | 132           | 924      |
+| EMITNULLIFIER       | 20x        | 1,540         | 30,800   |
+| L1TOL2MSGEXISTS     | 5x         | 108           | 540      |
+| SENDL2TOL1MSG       | 2x         | 209           | 418      |
+| CALL                | 3x         | 3,312         | 9,936    |
+| STATICCALL          | 3x         | 3,312         | 9,936    |
+| GETCONTRACTINSTANCE | 4x         | 1,527         | 6,108    |
+| POSEIDON2           | 15x        | 24            | 360      |
+| ECADD               | 10x        | 27            | 270      |
 
 **Impact**: Contracts with public bytecode performing any of these operations will see increased gas consumption.
 

yarn-project/archiver/src/archiver/archiver_store_test_suite.ts

@@ -2276,17 +2276,13 @@ export function describeArchiverDataStore(
           [
             expect.objectContaining({
               blockNumber: 2,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[2 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePrivateLog(tags[0]),
-              isFromPublic: false,
+              logData: makePrivateLog(tags[0]).getEmittedFields(),
             }),
           ],
           [
             expect.objectContaining({
               blockNumber: 1,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[1 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePrivateLog(tags[1]),
-              isFromPublic: false,
+              logData: makePrivateLog(tags[1]).getEmittedFields(),
             }),
           ],
         ]);
@@ -2312,15 +2308,11 @@ export function describeArchiverDataStore(
           [
             expect.objectContaining({
               blockNumber: 1,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[1 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePrivateLog(tags[0]),
-              isFromPublic: false,
+              logData: makePrivateLog(tags[0]).getEmittedFields(),
             }),
             expect.objectContaining({
               blockNumber: newBlockNumber,
-              blockHash: L2BlockHash.fromField(await newCheckpoint.checkpoint.blocks[0].header.hash()),
-              log: newLog,
-              isFromPublic: false,
+              logData: newLog.getEmittedFields(),
             }),
           ],
         ]);
@@ -2338,9 +2330,7 @@ export function describeArchiverDataStore(
           [
             expect.objectContaining({
               blockNumber: 1,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[1 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePrivateLog(tags[1]),
-              isFromPublic: false,
+              logData: makePrivateLog(tags[1]).getEmittedFields(),
             }),
           ],
         ]);
@@ -2425,17 +2415,13 @@ export function describeArchiverDataStore(
           [
             expect.objectContaining({
               blockNumber: 2,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[2 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePublicLog(tags[0]),
-              isFromPublic: true,
+              logData: makePublicLog(tags[0]).getEmittedFields(),
             }),
           ],
           [
             expect.objectContaining({
               blockNumber: 1,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[1 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePublicLog(tags[1]),
-              isFromPublic: true,
+              logData: makePublicLog(tags[1]).getEmittedFields(),
             }),
           ],
         ]);
@@ -2461,15 +2447,11 @@ export function describeArchiverDataStore(
           [
             expect.objectContaining({
               blockNumber: 1,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[1 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePublicLog(tags[0]),
-              isFromPublic: true,
+              logData: makePublicLog(tags[0]).getEmittedFields(),
             }),
             expect.objectContaining({
               blockNumber: newBlockNumber,
-              blockHash: L2BlockHash.fromField(await newCheckpoint.checkpoint.blocks[0].header.hash()),
-              log: newLog,
-              isFromPublic: true,
+              logData: newLog.getEmittedFields(),
             }),
           ],
         ]);
@@ -2487,9 +2469,7 @@ export function describeArchiverDataStore(
           [
             expect.objectContaining({
               blockNumber: 1,
-              blockHash: L2BlockHash.fromField(await logsCheckpoints[1 - 1].checkpoint.blocks[0].header.hash()),
-              log: makePublicLog(tags[1]),
-              isFromPublic: true,
+              logData: makePublicLog(tags[1]).getEmittedFields(),
             }),
           ],
         ]);

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

@@ -1,4 +1,4 @@
-import { INITIAL_L2_BLOCK_NUM, MAX_NOTE_HASHES_PER_TX } from '@aztec/constants';
+import { INITIAL_L2_BLOCK_NUM } from '@aztec/constants';
 import { BlockNumber } from '@aztec/foundation/branded-types';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import { createLogger } from '@aztec/foundation/log';
@@ -57,21 +57,16 @@ export class LogStore {
    * @param block - The L2 block to extract logs from.
    * @returns An object containing the private and public tagged logs for the block.
    */
-  async #extractTaggedLogsFromBlock(block: L2BlockNew) {
-    const blockHash = L2BlockHash.fromField(await block.hash());
+  #extractTaggedLogsFromBlock(block: L2BlockNew) {
     // SiloedTag (as string) -> array of log buffers.
     const privateTaggedLogs = new Map<string, Buffer[]>();
     // "{contractAddress}_{tag}" (as string) -> array of log buffers.
     const publicTaggedLogs = new Map<string, Buffer[]>();
-    const dataStartIndexForBlock =
-      block.header.state.partial.noteHashTree.nextAvailableLeafIndex -
-      block.body.txEffects.length * MAX_NOTE_HASHES_PER_TX;
 
-    block.body.txEffects.forEach((txEffect, txIndex) => {
+    block.body.txEffects.forEach(txEffect => {
       const txHash = txEffect.txHash;
-      const dataStartIndexForTx = dataStartIndexForBlock + txIndex * MAX_NOTE_HASHES_PER_TX;
 
-      txEffect.privateLogs.forEach((log, logIndex) => {
+      txEffect.privateLogs.forEach(log => {
         // Private logs use SiloedTag (already siloed by kernel)
         const tag = log.fields[0];
         this.#log.debug(`Found private log with tag ${tag.toString()} in block ${block.number}`);
@@ -80,18 +75,17 @@ export class LogStore {
         currentLogs.push(
           new TxScopedL2Log(
             txHash,
-            dataStartIndexForTx,
-            logIndex,
             block.number,
-            blockHash,
             block.timestamp,
-            log,
+            log.getEmittedFields(),
+            txEffect.noteHashes,
+            txEffect.nullifiers[0],
           ).toBuffer(),
         );
         privateTaggedLogs.set(tag.toString(), currentLogs);
       });
 
-      txEffect.publicLogs.forEach((log, logIndex) => {
+      txEffect.publicLogs.forEach(log => {
         // Public logs use Tag directly (not siloed) and are stored with contract address
         const tag = log.fields[0];
         const contractAddress = log.contractAddress;
@@ -104,12 +98,11 @@ export class LogStore {
         currentLogs.push(
           new TxScopedL2Log(
             txHash,
-            dataStartIndexForTx,
-            logIndex,
             block.number,
-            blockHash,
             block.timestamp,
-            log,
+            log.getEmittedFields(),
+            txEffect.noteHashes,
+            txEffect.nullifiers[0],
           ).toBuffer(),
         );
         publicTaggedLogs.set(key, currentLogs);
@@ -125,10 +118,11 @@ export class LogStore {
    * @returns A map from tag (as string) to an array of serialized private logs belonging to that tag, and a map from
    * "{contractAddress}_{tag}" (as string) to an array of serialized public logs belonging to that key.
    */
-  async #extractTaggedLogs(
-    blocks: L2BlockNew[],
-  ): Promise<{ privateTaggedLogs: Map<string, Buffer[]>; publicTaggedLogs: Map<string, Buffer[]> }> {
-    const taggedLogsInBlocks = await Promise.all(blocks.map(block => this.#extractTaggedLogsFromBlock(block)));
+  #extractTaggedLogs(blocks: L2BlockNew[]): {
+    privateTaggedLogs: Map<string, Buffer[]>;
+    publicTaggedLogs: Map<string, Buffer[]>;
+  } {
+    const taggedLogsInBlocks = blocks.map(block => this.#extractTaggedLogsFromBlock(block));
 
     // Now we merge the maps from each block into a single map.
     const privateTaggedLogs = taggedLogsInBlocks.reduce((acc, { privateTaggedLogs }) => {
@@ -155,8 +149,8 @@ export class LogStore {
    * @param blocks - The blocks for which to add the logs.
    * @returns True if the operation is successful.
    */
-  async addLogs(blocks: L2BlockNew[]): Promise<boolean> {
-    const { privateTaggedLogs, publicTaggedLogs } = await this.#extractTaggedLogs(blocks);
+  addLogs(blocks: L2BlockNew[]): Promise<boolean> {
+    const { privateTaggedLogs, publicTaggedLogs } = this.#extractTaggedLogs(blocks);
 
     const keysOfPrivateLogsToUpdate = Array.from(privateTaggedLogs.keys());
     const keysOfPublicLogsToUpdate = Array.from(publicTaggedLogs.keys());

yarn-project/end-to-end/src/composed/e2e_token_bridge_tutorial_test.test.ts

@@ -229,5 +229,5 @@ describe('e2e_cross_chain_messaging token_bridge_tutorial_test', () => {
     logger.info(`New L1 balance of ${ownerEthAddress} is ${newL1Balance}`);
     // docs:end:l1-withdraw
     expect(newL1Balance).toBe(withdrawAmount);
-  }, 90000);
+  }, 110_000);
 });

yarn-project/pxe/src/logs/log_service.test.ts

@@ -1,13 +1,10 @@
-import { BlockNumber } from '@aztec/foundation/branded-types';
-import { randomInt } from '@aztec/foundation/crypto/random';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import { KeyStore } from '@aztec/key-store';
 import { openTmpStore } from '@aztec/kv-store/lmdb-v2';
 import { AztecAddress } from '@aztec/stdlib/aztec-address';
-import { L2BlockHash } from '@aztec/stdlib/block';
 import type { AztecNode } from '@aztec/stdlib/interfaces/server';
-import { PublicLog, Tag, TxScopedL2Log } from '@aztec/stdlib/logs';
-import { TxHash, randomIndexedTxEffect } from '@aztec/stdlib/tx';
+import { Tag } from '@aztec/stdlib/logs';
+import { randomTxScopedPrivateL2Log } from '@aztec/stdlib/testing';
 
 import { type MockProxy, mock } from 'jest-mock-extended';
 
@@ -70,18 +67,12 @@ describe('LogService', () => {
     });
 
     it('returns a public log if one is found', async () => {
-      const scopedLog = await TxScopedL2Log.random(true);
-      (scopedLog.log as PublicLog).contractAddress = contractAddress;
+      const scopedLog = randomTxScopedPrivateL2Log();
 
       aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[scopedLog]]);
       aztecNode.getPrivateLogsByTags.mockResolvedValue([[]]);
-      const indexedTxEffect = await randomIndexedTxEffect();
 
-      aztecNode.getTxEffect.mockImplementation((txHash: TxHash) =>
-        txHash.equals(scopedLog.txHash) ? Promise.resolve(indexedTxEffect) : Promise.resolve(undefined),
-      );
-
-      const request = new LogRetrievalRequest(contractAddress, new Tag(scopedLog.log.fields[0]));
+      const request = new LogRetrievalRequest(contractAddress, new Tag(scopedLog.logData[0]));
 
       const responses = await logService.bulkRetrieveLogs([request]);
 
@@ -90,106 +81,17 @@ describe('LogService', () => {
     });
 
     it('returns a private log if one is found', async () => {
-      const scopedLog = await TxScopedL2Log.random(false);
+      const scopedLog = randomTxScopedPrivateL2Log();
 
       aztecNode.getPrivateLogsByTags.mockResolvedValue([[scopedLog]]);
       aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[]]);
-      const indexedTxEffect = await randomIndexedTxEffect();
-      aztecNode.getTxEffect.mockResolvedValue(indexedTxEffect);
 
-      aztecNode.getTxEffect.mockImplementation((txHash: TxHash) =>
-        txHash.equals(scopedLog.txHash) ? Promise.resolve(indexedTxEffect) : Promise.resolve(undefined),
-      );
-
-      const request = new LogRetrievalRequest(contractAddress, new Tag(scopedLog.log.fields[0]));
+      const request = new LogRetrievalRequest(contractAddress, new Tag(scopedLog.logData[0]));
 
       const responses = await logService.bulkRetrieveLogs([request]);
 
       expect(responses.length).toEqual(1);
       expect(responses[0]).not.toBeNull();
     });
   });
-
-  describe('getPublicLogByTag', () => {
-    const tag = new Tag(Fr.random());
-
-    beforeEach(() => {
-      aztecNode.getPublicLogsByTagsFromContract.mockReset();
-      aztecNode.getTxEffect.mockReset();
-    });
-
-    it('returns null if no logs found for tag', async () => {
-      aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[]]);
-
-      const result = await logService.getPublicLogByTag(tag, contractAddress);
-      expect(result).toBeNull();
-    });
-
-    it('returns log data when single log found', async () => {
-      const scopedLog = await TxScopedL2Log.random(true);
-      const logContractAddress = (scopedLog.log as PublicLog).contractAddress;
-
-      aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[scopedLog]]);
-      const indexedTxEffect = await randomIndexedTxEffect();
-      aztecNode.getTxEffect.mockImplementation((txHash: TxHash) =>
-        txHash.equals(scopedLog.txHash) ? Promise.resolve(indexedTxEffect) : Promise.resolve(undefined),
-      );
-
-      const result = (await logService.getPublicLogByTag(tag, logContractAddress))!;
-
-      expect(result.logPayload).toEqual(scopedLog.log.getEmittedFieldsWithoutTag());
-      expect(result.uniqueNoteHashesInTx).toEqual(indexedTxEffect.data.noteHashes);
-      expect(result.txHash).toEqual(scopedLog.txHash);
-      expect(result.firstNullifierInTx).toEqual(indexedTxEffect.data.nullifiers[0]);
-
-      expect(aztecNode.getPublicLogsByTagsFromContract).toHaveBeenCalledWith(logContractAddress, [tag]);
-      expect(aztecNode.getTxEffect).toHaveBeenCalledWith(scopedLog.txHash);
-    });
-
-    it('throws if multiple logs found for tag', async () => {
-      const scopedLog = await TxScopedL2Log.random(true);
-      aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[scopedLog, scopedLog]]);
-      const logContractAddress = (scopedLog.log as PublicLog).contractAddress;
-
-      await expect(logService.getPublicLogByTag(tag, logContractAddress)).rejects.toThrow(/Got 2 logs for tag/);
-    });
-
-    it('throws if tx effect not found', async () => {
-      const scopedLog = await TxScopedL2Log.random(true);
-      aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[scopedLog]]);
-      aztecNode.getTxEffect.mockResolvedValue(undefined);
-      const logContractAddress = (scopedLog.log as PublicLog).contractAddress;
-
-      await expect(logService.getPublicLogByTag(tag, logContractAddress)).rejects.toThrow(
-        /failed to retrieve tx effects/,
-      );
-    });
-
-    it('returns log fields that are actually emitted', async () => {
-      const logContractAddress = await AztecAddress.random();
-      const logPlaintext = [Fr.random()];
-      const logContent = [tag.value, ...logPlaintext];
-
-      const log = PublicLog.from({
-        contractAddress: logContractAddress,
-        fields: logContent,
-      });
-      const scopedLogWithPadding = new TxScopedL2Log(
-        TxHash.random(),
-        randomInt(100),
-        randomInt(100),
-        BlockNumber(randomInt(100)),
-        L2BlockHash.random(),
-        0n,
-        log,
-      );
-
-      aztecNode.getPublicLogsByTagsFromContract.mockResolvedValue([[scopedLogWithPadding]]);
-      aztecNode.getTxEffect.mockResolvedValue(await randomIndexedTxEffect());
-
-      const result = await logService.getPublicLogByTag(tag, logContractAddress);
-
-      expect(result?.logPayload).toEqual(logPlaintext);
-    });
-  });
 });