refactor: more robust tagging index sync as sender (#17611)

In this PR I implement the approach that Nico originally described in
[this
post](https://forum.aztec.network/t/on-note-discovery-and-index-coordination/7165)
for sender tagging index sync (for recipient it will be done in a
followup PR).

To summarize I do the following:
1. Associate the tagging index with a hash of a tx in which the
corresponding tax was used,
2. I track the status of the given tx in the tagging data provider,
3. tagging index status can be either PENDING, FINALIZED or
DROPPED_OR_REVERTED (this status is only symbolic as when we realize a
corresponding tx has been dropped or reverted we just delete the pending
index),
4. when choosing the next index to use when sending a private log I
choose `HIGHEST_PENDING_INDEX + 1` (or `HIGHEST_FINALIZED_INDEX + 1` if
there is no pending index),
5. if the chosen index is further than WINDOW_LENGTH away from the
HIGHEST_FINALIZED_INDEX an error is thrown. This should be a good
guarantee of us never losing a log because we are always looking for a
WINDOW_LENGTH of logs.

# Notes for reviewer
- I separated `TaggingDataProvider` into `SenderTaggingDataProvider` and
`RecipientTaggingDataProvider` because the algorithms are completely
disjoint and it makes the code clearer.
- Moved the `syncTaggedLogsAsSender` function to `pxe/src/tagging`
directory and renamed it as `syncSenderTaggingIndexes`. Now
`PrivateExecutionOracle` just calls this function. This is a step in the
direction of deprecating `PXEOracleInterface` and it allowed me to write
a reasonably clear unit tests.

# Issues created in this PR
- #17775
- #17776

Author: benesjan

yarn-project/end-to-end/src/e2e_pending_note_hashes_contract.test.ts

@@ -287,6 +287,8 @@ describe('e2e_pending_note_hashes_contract', () => {
   });
 
   it('Should handle overflowing the kernel data structures in nested calls', async () => {
+    // This test verifies that a transaction can emit more notes than MAX_NOTE_HASHES_PER_TX without failing, since
+    // the notes are nullified and will be squashed by the kernel reset circuit.
     const sender = owner;
     const notesPerIteration = Math.min(MAX_NOTE_HASHES_PER_CALL, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL);
     const minToNeedReset = Math.min(MAX_NOTE_HASHES_PER_TX, MAX_NOTE_HASH_READ_REQUESTS_PER_TX) + 1;

yarn-project/pxe/src/block_synchronizer/block_synchronizer.test.ts

@@ -11,15 +11,15 @@ import { type MockProxy, mock } from 'jest-mock-extended';
 
 import { AnchorBlockDataProvider } from '../storage/anchor_block_data_provider/anchor_block_data_provider.js';
 import { NoteDataProvider } from '../storage/note_data_provider/note_data_provider.js';
-import { TaggingDataProvider } from '../storage/tagging_data_provider/tagging_data_provider.js';
+import { RecipientTaggingDataProvider } from '../storage/tagging_data_provider/recipient_tagging_data_provider.js';
 import { BlockSynchronizer } from './block_synchronizer.js';
 
 describe('BlockSynchronizer', () => {
   let synchronizer: BlockSynchronizer;
   let tipsStore: L2TipsKVStore;
   let anchorBlockDataProvider: AnchorBlockDataProvider;
   let noteDataProvider: NoteDataProvider;
-  let taggingDataProvider: TaggingDataProvider;
+  let recipientTaggingDataProvider: RecipientTaggingDataProvider;
   let aztecNode: MockProxy<AztecNode>;
   let blockStream: MockProxy<L2BlockStream>;
 
@@ -36,12 +36,12 @@ describe('BlockSynchronizer', () => {
     tipsStore = new L2TipsKVStore(store, 'pxe');
     anchorBlockDataProvider = new AnchorBlockDataProvider(store);
     noteDataProvider = await NoteDataProvider.create(store);
-    taggingDataProvider = new TaggingDataProvider(store);
+    recipientTaggingDataProvider = new RecipientTaggingDataProvider(store);
     synchronizer = new TestSynchronizer(
       aztecNode,
       anchorBlockDataProvider,
       noteDataProvider,
-      taggingDataProvider,
+      recipientTaggingDataProvider,
       tipsStore,
     );
   });
@@ -59,7 +59,7 @@ describe('BlockSynchronizer', () => {
       .spyOn(noteDataProvider, 'rollbackNotesAndNullifiers')
       .mockImplementation(() => Promise.resolve());
     const resetNoteSyncData = jest
-      .spyOn(taggingDataProvider, 'resetNoteSyncData')
+      .spyOn(recipientTaggingDataProvider, 'resetNoteSyncData')
       .mockImplementation(() => Promise.resolve());
     aztecNode.getBlockHeader.mockImplementation(async blockNumber =>
       (await L2Block.random(BlockNumber(blockNumber as number))).getBlockHeader(),

yarn-project/pxe/src/block_synchronizer/block_synchronizer.ts

@@ -7,7 +7,7 @@ import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import type { PXEConfig } from '../config/index.js';
 import type { AnchorBlockDataProvider } from '../storage/anchor_block_data_provider/anchor_block_data_provider.js';
 import type { NoteDataProvider } from '../storage/note_data_provider/note_data_provider.js';
-import type { TaggingDataProvider } from '../storage/tagging_data_provider/tagging_data_provider.js';
+import type { RecipientTaggingDataProvider } from '../storage/tagging_data_provider/recipient_tagging_data_provider.js';
 
 /**
  * The BlockSynchronizer class orchestrates synchronization between PXE and Aztec node, maintaining an up-to-date
@@ -23,7 +23,7 @@ export class BlockSynchronizer implements L2BlockStreamEventHandler {
     private node: AztecNode,
     private anchorBlockDataProvider: AnchorBlockDataProvider,
     private noteDataProvider: NoteDataProvider,
-    private taggingDataProvider: TaggingDataProvider,
+    private recipientTaggingDataProvider: RecipientTaggingDataProvider,
     private l2TipsStore: L2TipsKVStore,
     config: Partial<Pick<PXEConfig, 'l2BlockBatchSize'>> = {},
     loggerOrSuffix?: string | Logger,
@@ -66,7 +66,10 @@ export class BlockSynchronizer implements L2BlockStreamEventHandler {
         await this.noteDataProvider.rollbackNotesAndNullifiers(event.block.number, lastSynchedBlockNumber);
         // Remove all note tagging indexes to force a full resync. This is suboptimal, but unless we track the
         // block number in which each index is used it's all we can do.
-        await this.taggingDataProvider.resetNoteSyncData();
+        // Note: This is now unnecessary for the sender tagging data provider because the new algorithm handles reorgs.
+        // TODO(#17775): Once this issue is implemented we will have the index-block number mapping, so we can
+        // implement this more intelligently.
+        await this.recipientTaggingDataProvider.resetNoteSyncData();
         // Update the header to the last block.
         const newHeader = await this.node.getBlockHeader(event.block.number);
         if (!newHeader) {

yarn-project/pxe/src/contract_function_simulator/execution_data_provider.ts

@@ -6,12 +6,14 @@ import type { FunctionArtifactWithContractName, FunctionSelector } from '@aztec/
 import type { AztecAddress } from '@aztec/stdlib/aztec-address';
 import type { L2Block } from '@aztec/stdlib/block';
 import type { CompleteAddress, ContractInstance } from '@aztec/stdlib/contract';
+import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import type { KeyValidationRequest } from '@aztec/stdlib/kernel';
 import type { DirectionalAppTaggingSecret } from '@aztec/stdlib/logs';
 import type { NoteStatus } from '@aztec/stdlib/note';
 import { type MerkleTreeId, type NullifierMembershipWitness, PublicDataWitness } from '@aztec/stdlib/trees';
 import type { NodeStats } from '@aztec/stdlib/tx';
 
+import type { SenderTaggingDataProvider } from '../storage/tagging_data_provider/sender_tagging_data_provider.js';
 import type { NoteData } from './oracle/interfaces.js';
 import type { MessageLoadOracleInputs } from './oracle/message_load_oracle_inputs.js';
 
@@ -219,25 +221,6 @@ export interface ExecutionDataProvider {
     recipient: AztecAddress,
   ): Promise<DirectionalAppTaggingSecret>;
 
-  /**
-   * Updates the local index of the shared tagging secret of a (sender, recipient, contract) tuple if a log with
-   * a larger index is found from the node.
-   * @param secret - The secret that's unique for (sender, recipient, contract) tuple while the direction
-   * of sender -> recipient matters.
-   * @param contractAddress - The address of the contract that the logs are tagged for. Needs to be provided to store
-   * because the function performs second round of siloing which is necessary because kernels do it as well (they silo
-   * first field of the private log which corresponds to the tag).
-   */
-  syncTaggedLogsAsSender(secret: DirectionalAppTaggingSecret, contractAddress: AztecAddress): Promise<void>;
-
-  /**
-   * Returns the last used index when sending a log with a given secret.
-   * @param secret - The directional app tagging secret.
-   * @returns The last used index for the given directional app tagging secret, or undefined if we never sent a log
-   * from this sender to a recipient in a given contract (implicitly included in the secret).
-   */
-  getLastUsedIndexAsSender(secret: DirectionalAppTaggingSecret): Promise<number | undefined>;
-
   /**
    * Synchronizes the private logs tagged with scoped addresses and all the senders in the address book. Stores the found
    * logs in CapsuleArray ready for a later retrieval in Aztec.nr.
@@ -332,4 +315,8 @@ export interface ExecutionDataProvider {
    * @returns The execution statistics.
    */
   getStats(): ExecutionStats;
+
+  // Exposed when moving in the direction of #17776
+  get aztecNode(): AztecNode;
+  get senderTaggingDataProvider(): SenderTaggingDataProvider;
 }

yarn-project/pxe/src/contract_function_simulator/execution_tagging_index_cache.ts

@@ -21,7 +21,7 @@ export class ExecutionTaggingIndexCache {
   }
 
   /**
-   * Returns the pre tags that were used in this execution (and that need to be stored in the db).
+   * Returns the pre-tags that were used in this execution (and that need to be stored in the db).
    */
   public getUsedPreTags(): PreTag[] {
     return Array.from(this.taggingIndexMap.entries()).map(([secret, index]) => ({

yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts

@@ -40,6 +40,7 @@ import {
 } from '@aztec/stdlib/contract';
 import { GasFees, GasSettings } from '@aztec/stdlib/gas';
 import { computeNoteHashNonce, computeSecretHash, computeUniqueNoteHash, siloNoteHash } from '@aztec/stdlib/hash';
+import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import { KeyValidationRequest } from '@aztec/stdlib/kernel';
 import { computeAppNullifierSecretKey, deriveKeys } from '@aztec/stdlib/keys';
 import { DirectionalAppTaggingSecret } from '@aztec/stdlib/logs';
@@ -60,6 +61,7 @@ import { jest } from '@jest/globals';
 import { Matcher, type MatcherCreator, type MockProxy, mock } from 'jest-mock-extended';
 import { toFunctionSelector } from 'viem';
 
+import type { SenderTaggingDataProvider } from '../../storage/tagging_data_provider/sender_tagging_data_provider.js';
 import { ContractFunctionSimulator } from '../contract_function_simulator.js';
 import type { ExecutionDataProvider } from '../execution_data_provider.js';
 import type { NoteData } from './interfaces.js';
@@ -102,6 +104,8 @@ describe('Private Execution test suite', () => {
   const simulator = new WASMSimulator();
 
   let executionDataProvider: MockProxy<ExecutionDataProvider>;
+  let senderTaggingDataProvider: MockProxy<SenderTaggingDataProvider>;
+  let aztecNode: MockProxy<AztecNode>;
   let acirSimulator: ContractFunctionSimulator;
 
   let anchorBlockHeader = BlockHeader.empty();
@@ -266,7 +270,30 @@ describe('Private Execution test suite', () => {
   beforeEach(async () => {
     trees = {};
     executionDataProvider = mock<ExecutionDataProvider>();
+    senderTaggingDataProvider = mock<SenderTaggingDataProvider>();
+    aztecNode = mock<AztecNode>();
     contracts = {};
+
+    // Mock the senderTaggingDataProvider getter
+    Object.defineProperty(executionDataProvider, 'senderTaggingDataProvider', {
+      get: () => senderTaggingDataProvider,
+    });
+
+    // Mock the aztecNode getter
+    Object.defineProperty(executionDataProvider, 'aztecNode', {
+      get: () => aztecNode,
+    });
+
+    // Mock sender tagging data provider methods
+    senderTaggingDataProvider.getLastFinalizedIndex.mockResolvedValue(undefined);
+    senderTaggingDataProvider.getLastUsedIndex.mockResolvedValue(undefined);
+    senderTaggingDataProvider.getTxHashesOfPendingIndexes.mockResolvedValue([]);
+    senderTaggingDataProvider.storePendingIndexes.mockResolvedValue();
+
+    // Mock aztec node methods - the return array needs to have the same length as the number of tags
+    // on the input.
+    aztecNode.getLogsByTags.mockImplementation((tags: Fr[]) => Promise.resolve(tags.map(() => [])));
+
     executionDataProvider.getKeyValidationRequest.mockImplementation(
       async (pkMHash: Fr, contractAddress: AztecAddress) => {
         if (pkMHash.equals(await ownerCompleteAddress.publicKeys.masterNullifierPublicKey.hash())) {
@@ -303,9 +330,6 @@ describe('Private Execution test suite', () => {
       throw new Error(`Unknown address: ${address}. Recipient: ${recipient}, Owner: ${owner}`);
     });
 
-    executionDataProvider.getLastUsedIndexAsSender.mockImplementation((_secret: DirectionalAppTaggingSecret) => {
-      return Promise.resolve(undefined);
-    });
     executionDataProvider.getFunctionArtifact.mockImplementation(async (address, selector) => {
       const contract = contracts[address.toString()];
       if (!contract) {
@@ -323,9 +347,6 @@ describe('Private Execution test suite', () => {
     executionDataProvider.calculateDirectionalAppTaggingSecret.mockImplementation((_contract, _sender, _recipient) => {
       return Promise.resolve(DirectionalAppTaggingSecret.fromString('0x1'));
     });
-    executionDataProvider.syncTaggedLogsAsSender.mockImplementation((_directionalAppTaggingSecret, _contractAddress) =>
-      Promise.resolve(),
-    );
     executionDataProvider.loadCapsule.mockImplementation((_, __) => Promise.resolve(null));
 
     executionDataProvider.getPublicStorageAt.mockImplementation(

yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts

@@ -26,6 +26,7 @@ import {
   type TxContext,
 } from '@aztec/stdlib/tx';
 
+import { syncSenderTaggingIndexes } from '../../tagging/sync/sync_sender_tagging_indexes.js';
 import { Tag } from '../../tagging/tag.js';
 import type { ExecutionDataProvider } from '../execution_data_provider.js';
 import type { ExecutionNoteCache } from '../execution_note_cache.js';
@@ -152,7 +153,7 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP
   }
 
   /**
-   * Returns the pre tags that were used in this execution (and that need to be stored in the db).
+   * Returns the pre-tags that were used in this execution (and that need to be stored in the db).
    */
   public getUsedPreTags(): PreTag[] {
     return this.taggingIndexCache.getUsedPreTags();
@@ -224,11 +225,16 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP
     if (lastUsedIndexInTx !== undefined) {
       return lastUsedIndexInTx + 1;
     } else {
+      // TODO(#17776): Don't access the Aztec node and senderTaggingDataProvider via the executionDataProvider.
+      const aztecNode = this.executionDataProvider.aztecNode;
+      const senderTaggingDataProvider = this.executionDataProvider.senderTaggingDataProvider;
+
       // This is a tagging secret we've not yet used in this tx, so first sync our store to make sure its indices
       // are up to date. We do this here because this store is not synced as part of the global sync because
       // that'd be wasteful as most tagging secrets are not used in each tx.
-      await this.executionDataProvider.syncTaggedLogsAsSender(secret, this.contractAddress);
-      const lastUsedIndex = await this.executionDataProvider.getLastUsedIndexAsSender(secret);
+      await syncSenderTaggingIndexes(secret, this.contractAddress, aztecNode, senderTaggingDataProvider);
+
+      const lastUsedIndex = await senderTaggingDataProvider.getLastUsedIndex(secret);
       // If lastUsedIndex is undefined, we've never used this secret, so start from 0
       // Otherwise, the next index to use is one past the last used index
       return lastUsedIndex === undefined ? 0 : lastUsedIndex + 1;