refactor: returning last used tagging index

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

@@ -239,11 +239,12 @@ export interface ExecutionDataProvider {
   syncTaggedLogsAsSender(secret: DirectionalAppTaggingSecret, contractAddress: AztecAddress): Promise<void>;
 
   /**
-   * Returns the next index to be used to compute a tag when sending a log.
+   * Returns the last used index when sending a log with a given secret.
    * @param secret - The directional app tagging secret.
-   * @returns The next index to be used to compute a tag for the given 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).
    */
-  getNextIndexAsSender(secret: DirectionalAppTaggingSecret): Promise<number>;
+  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

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

@@ -8,19 +8,22 @@ import { DirectionalAppTaggingSecret, type IndexedTaggingSecret } from '@aztec/s
 export class ExecutionTaggingIndexCache {
   private taggingIndexMap: Map<string, number> = new Map();
 
-  public getTaggingIndex(secret: DirectionalAppTaggingSecret): number | undefined {
+  public getLastUsedIndex(secret: DirectionalAppTaggingSecret): number | undefined {
     return this.taggingIndexMap.get(secret.toString());
   }
 
-  public setTaggingIndex(secret: DirectionalAppTaggingSecret, index: number) {
+  public setLastUsedIndex(secret: DirectionalAppTaggingSecret, index: number) {
     const currentValue = this.taggingIndexMap.get(secret.toString());
     if (currentValue !== undefined && currentValue !== index - 1) {
       throw new Error(`Invalid tagging index update. Current value: ${currentValue}, new value: ${index}`);
     }
     this.taggingIndexMap.set(secret.toString(), index);
   }
 
-  public getIndexedTaggingSecrets(): IndexedTaggingSecret[] {
+  /**
+   * Returns the indexed tagging secrets that were used in this execution.
+   */
+  public getUsedIndexedTaggingSecrets(): IndexedTaggingSecret[] {
     return Array.from(this.taggingIndexMap.entries()).map(([secret, index]) => ({
       secret: DirectionalAppTaggingSecret.fromString(secret),
       index,

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

@@ -302,8 +302,8 @@ describe('Private Execution test suite', () => {
       throw new Error(`Unknown address: ${address}. Recipient: ${recipient}, Owner: ${owner}`);
     });
 
-    executionDataProvider.getNextIndexAsSender.mockImplementation((_secret: DirectionalAppTaggingSecret) => {
-      return Promise.resolve(0);
+    executionDataProvider.getLastUsedIndexAsSender.mockImplementation((_secret: DirectionalAppTaggingSecret) => {
+      return Promise.resolve(undefined);
     });
     executionDataProvider.getFunctionArtifact.mockImplementation(async (address, selector) => {
       const contract = contracts[address.toString()];

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

@@ -88,7 +88,7 @@ export async function executePrivateFunction(
   const newNotes = privateExecutionOracle.getNewNotes();
   const noteHashNullifierCounterMap = privateExecutionOracle.getNoteHashNullifierCounterMap();
   const offchainEffects = privateExecutionOracle.getOffchainEffects();
-  const indexedTaggingSecrets = privateExecutionOracle.getIndexedTaggingSecrets();
+  const indexedTaggingSecrets = privateExecutionOracle.getUsedIndexedTaggingSecrets();
   const nestedExecutionResults = privateExecutionOracle.getNestedExecutionResults();
 
   let timerSubtractionList = nestedExecutionResults;

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

@@ -14,7 +14,7 @@ import type { AuthWitness } from '@aztec/stdlib/auth-witness';
 import { AztecAddress } from '@aztec/stdlib/aztec-address';
 import { computeUniqueNoteHash, siloNoteHash, siloNullifier } from '@aztec/stdlib/hash';
 import { PrivateContextInputs } from '@aztec/stdlib/kernel';
-import type { ContractClassLog, IndexedTaggingSecret } from '@aztec/stdlib/logs';
+import type { ContractClassLog, DirectionalAppTaggingSecret, IndexedTaggingSecret } from '@aztec/stdlib/logs';
 import { Note, type NoteStatus } from '@aztec/stdlib/note';
 import {
   type BlockHeader,
@@ -152,10 +152,10 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP
   }
 
   /**
-   * Return the tagging indexes incremented by this execution along with the directional app tagging secrets.
+   * Returns the indexed tagging secrets that were used in this execution.
    */
-  public getIndexedTaggingSecrets(): IndexedTaggingSecret[] {
-    return this.taggingIndexCache.getIndexedTaggingSecrets();
+  public getUsedIndexedTaggingSecrets(): IndexedTaggingSecret[] {
+    return this.taggingIndexCache.getUsedIndexedTaggingSecrets();
   }
 
   /**
@@ -208,34 +208,31 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP
       recipient,
     );
 
+    const index = await this.#getIndexToUseForSecret(secret);
+    this.log.debug(
+      `Incrementing tagging index for sender: ${sender}, recipient: ${recipient}, contract: ${this.contractAddress} to ${index}`,
+    );
+    this.taggingIndexCache.setLastUsedIndex(secret, index);
+
+    return Tag.compute({ secret, index });
+  }
+
+  async #getIndexToUseForSecret(secret: DirectionalAppTaggingSecret): Promise<number> {
     // If we have the tagging index in the cache, we use it. If not we obtain it from the execution data provider.
-    // TODO(benesjan): Make this be `getLastUsedIndex` and refactor this function to look as proposed in this comment:
-    // https://github.com/AztecProtocol/aztec-packages/pull/17445#discussion_r2400365845
-    const maybeTaggingIndex = this.taggingIndexCache.getTaggingIndex(secret);
-    let taggingIndex: number;
+    const lastUsedIndexInTx = this.taggingIndexCache.getLastUsedIndex(secret);
 
-    if (maybeTaggingIndex !== undefined) {
-      taggingIndex = maybeTaggingIndex;
+    if (lastUsedIndexInTx !== undefined) {
+      return lastUsedIndexInTx + 1;
     } else {
       // 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.
-      this.log.debug(`Syncing tagged logs as sender ${sender} for contract ${this.contractAddress}`, {
-        directionalAppTaggingSecret: secret,
-        recipient,
-      });
       await this.executionDataProvider.syncTaggedLogsAsSender(secret, this.contractAddress);
-      taggingIndex = await this.executionDataProvider.getNextIndexAsSender(secret);
+      const lastUsedIndex = await this.executionDataProvider.getLastUsedIndexAsSender(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;
     }
-
-    // Now we increment the index by 1 and store it in the cache.
-    const nextTaggingIndex = taggingIndex + 1;
-    this.log.debug(
-      `Incrementing tagging index for sender: ${sender}, recipient: ${recipient}, contract: ${this.contractAddress} to ${nextTaggingIndex}`,
-    );
-    this.taggingIndexCache.setTaggingIndex(secret, nextTaggingIndex);
-
-    return Tag.compute({ secret, index: taggingIndex });
   }
 
   /**

yarn-project/pxe/src/contract_function_simulator/pxe_oracle_interface.test.ts

@@ -206,13 +206,13 @@ describe('PXEOracleInterface', () => {
         ),
       );
 
-      // First sender should have 2 logs, but keep index 1 since they were built using the same tag
-      // Next 4 senders should also have index 1 = offset + 1
-      // Last 5 senders should have index 2 = offset + 2
-      const indexes = await taggingDataProvider.getNextIndexesAsRecipient(secrets);
+      // First sender should have 2 logs, but keep index 0 since they were built using the same tag
+      // Next 4 senders should also have index 0 = offset + 0
+      // Last 5 senders should have index 1 = offset + 1
+      const indexes = await taggingDataProvider.getLastUsedIndexesAsRecipient(secrets);
 
       expect(indexes).toHaveLength(NUM_SENDERS);
-      expect(indexes).toEqual([1, 1, 1, 1, 1, 2, 2, 2, 2, 2]);
+      expect(indexes).toEqual([0, 0, 0, 0, 0, 1, 1, 1, 1, 1]);
 
       // We should have called the node 2 times:
       // 2 times: first time during initial request, second time after pushing the edge of the window once
@@ -244,10 +244,21 @@ describe('PXEOracleInterface', () => {
       );
 
       const getTaggingSecretsIndexesAsSenderForSenders = () =>
-        Promise.all(secrets.map(secret => taggingDataProvider.getNextIndexAsSender(secret)));
+        Promise.all(secrets.map(secret => taggingDataProvider.getLastUsedIndexesAsSender(secret)));
 
       const indexesAsSender = await getTaggingSecretsIndexesAsSenderForSenders();
-      expect(indexesAsSender).toStrictEqual([0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
+      expect(indexesAsSender).toStrictEqual([
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+      ]);
 
       expect(aztecNode.getLogsByTags.mock.calls.length).toBe(0);
 
@@ -263,7 +274,7 @@ describe('PXEOracleInterface', () => {
       }
 
       let indexesAsSenderAfterSync = await getTaggingSecretsIndexesAsSenderForSenders();
-      expect(indexesAsSenderAfterSync).toStrictEqual([1, 1, 1, 1, 1, 2, 2, 2, 2, 2]);
+      expect(indexesAsSenderAfterSync).toStrictEqual([0, 0, 0, 0, 0, 1, 1, 1, 1, 1]);
 
       // Only 1 window is obtained for each sender
       expect(aztecNode.getLogsByTags.mock.calls.length).toBe(NUM_SENDERS);
@@ -285,7 +296,7 @@ describe('PXEOracleInterface', () => {
       }
 
       indexesAsSenderAfterSync = await getTaggingSecretsIndexesAsSenderForSenders();
-      expect(indexesAsSenderAfterSync).toStrictEqual([12, 12, 12, 12, 12, 13, 13, 13, 13, 13]);
+      expect(indexesAsSenderAfterSync).toStrictEqual([10, 10, 10, 10, 10, 11, 11, 11, 11, 11]);
 
       expect(aztecNode.getLogsByTags.mock.calls.length).toBe(NUM_SENDERS * 2);
     });
@@ -313,13 +324,13 @@ describe('PXEOracleInterface', () => {
         ),
       );
 
-      // First sender should have 2 logs, but keep index 6 since they were built using the same tag
-      // Next 4 senders should also have index 6 = offset + 1
-      // Last 5 senders should have index 7 = offset + 2
-      const indexes = await taggingDataProvider.getNextIndexesAsRecipient(secrets);
+      // First sender should have 2 logs, but keep index 5 since they were built using the same tag
+      // Next 4 senders should also have index 5 = offset
+      // Last 5 senders should have index 6 = offset + 1
+      const indexes = await taggingDataProvider.getLastUsedIndexesAsRecipient(secrets);
 
       expect(indexes).toHaveLength(NUM_SENDERS);
-      expect(indexes).toEqual([6, 6, 6, 6, 6, 7, 7, 7, 7, 7]);
+      expect(indexes).toEqual([5, 5, 5, 5, 5, 6, 6, 6, 6, 6]);
 
       // We should have called the node 2 times:
       // 2 times: first time during initial request, second time after pushing the edge of the window once
@@ -344,22 +355,22 @@ describe('PXEOracleInterface', () => {
         ),
       );
 
-      // Increase our indexes to 2
-      await taggingDataProvider.setNextIndexesAsRecipient(secrets.map(secret => ({ secret, index: 2 })));
+      // Set last used indexes to 1 (so next scan starts at 2)
+      await taggingDataProvider.setLastUsedIndexesAsRecipient(secrets.map(secret => ({ secret, index: 1 })));
 
       await pxeOracleInterface.syncTaggedLogs(contractAddress, PENDING_TAGGED_LOG_ARRAY_BASE_SLOT);
 
       // Even if our index as recipient is higher than what the sender sent, we should be able to find the logs
       // since the window starts at Math.max(0, 2 - window_size) = 0
       await expectPendingTaggedLogArrayLengthToBe(contractAddress, NUM_SENDERS + 1 + NUM_SENDERS / 2);
 
-      // First sender should have 2 logs, but keep index 2 since they were built using the same tag
-      // Next 4 senders should also have index 2 = tagIndex + 1
-      // Last 5 senders should have index 3 = tagIndex + 2
-      const indexes = await taggingDataProvider.getNextIndexesAsRecipient(secrets);
+      // First sender should have 2 logs, but keep index 1 since they were built using the same tag
+      // Next 4 senders should also have index 1 = tagIndex
+      // Last 5 senders should have index 2 = tagIndex + 1
+      const indexes = await taggingDataProvider.getLastUsedIndexesAsRecipient(secrets);
 
       expect(indexes).toHaveLength(NUM_SENDERS);
-      expect(indexes).toEqual([2, 2, 2, 2, 2, 3, 3, 3, 3, 3]);
+      expect(indexes).toEqual([1, 1, 1, 1, 1, 2, 2, 2, 2, 2]);
 
       // We should have called the node 2 times:
       // first time during initial request, second time after pushing the edge of the window once
@@ -384,19 +395,19 @@ describe('PXEOracleInterface', () => {
         ),
       );
 
-      // We set the indexes to WINDOW_HALF_SIZE + 1 so that it's outside the window and for this reason no updates
-      // should be triggered.
+      // We set the last used indexes to WINDOW_HALF_SIZE so that next scan starts at WINDOW_HALF_SIZE + 1,
+      // which is outside the window, and for this reason no updates should be triggered.
       const index = WINDOW_HALF_SIZE + 1;
-      await taggingDataProvider.setNextIndexesAsRecipient(secrets.map(secret => ({ secret, index })));
+      await taggingDataProvider.setLastUsedIndexesAsRecipient(secrets.map(secret => ({ secret, index })));
 
       await pxeOracleInterface.syncTaggedLogs(contractAddress, PENDING_TAGGED_LOG_ARRAY_BASE_SLOT);
 
       // Only half of the logs should be synced since we start from index 1 = (11 - window_size), the other half should
       // be skipped
       await expectPendingTaggedLogArrayLengthToBe(contractAddress, NUM_SENDERS / 2);
 
-      // Indexes should remain where we set them (window_size + 1)
-      const indexes = await taggingDataProvider.getNextIndexesAsRecipient(secrets);
+      // Indexes should remain where we set them (window_size)
+      const indexes = await taggingDataProvider.getLastUsedIndexesAsRecipient(secrets);
 
       expect(indexes).toHaveLength(NUM_SENDERS);
       expect(indexes).toEqual([index, index, index, index, index, index, index, index, index, index]);
@@ -423,7 +434,7 @@ describe('PXEOracleInterface', () => {
         ),
       );
 
-      await taggingDataProvider.setNextIndexesAsRecipient(
+      await taggingDataProvider.setLastUsedIndexesAsRecipient(
         secrets.map(secret => ({ secret, index: WINDOW_HALF_SIZE + 2 })),
       );
 
@@ -443,13 +454,13 @@ describe('PXEOracleInterface', () => {
 
       await pxeOracleInterface.syncTaggedLogs(contractAddress, PENDING_TAGGED_LOG_ARRAY_BASE_SLOT);
 
-      // First sender should have 2 logs, but keep index 1 since they were built using the same tag
-      // Next 4 senders should also have index 1 = offset + 1
-      // Last 5 senders should have index 2 = offset + 2
-      const indexes = await taggingDataProvider.getNextIndexesAsRecipient(secrets);
+      // First sender should have 2 logs, but keep index 0 since they were built using the same tag
+      // Next 4 senders should also have index 0 = offset
+      // Last 5 senders should have index 1 = offset + 1
+      const indexes = await taggingDataProvider.getLastUsedIndexesAsRecipient(secrets);
 
       expect(indexes).toHaveLength(NUM_SENDERS);
-      expect(indexes).toEqual([1, 1, 1, 1, 1, 2, 2, 2, 2, 2]);
+      expect(indexes).toEqual([0, 0, 0, 0, 0, 1, 1, 1, 1, 1]);
 
       // We should have called the node 2 times:
       // first time during initial request, second time after pushing the edge of the window once