Merge branch 'backport-to-v4-next-staging' into jb/ci-revert-compat-e2e-to-keys

benesjan · web-flow · commit 7a4df18ba11e · 2026-05-12T19:18:47.000+02:00
diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts
@@ -66,7 +66,7 @@ import { Matcher, type MatcherCreator, type MockProxy, mock } from 'jest-mock-ex
 import { toFunctionSelector } from 'viem';
 
 import type { ContractSyncService } from '../../contract_sync/contract_sync_service.js';
-import { syncState } from '../../contract_sync/helpers.js';
+import { syncScope } from '../../contract_sync/helpers.js';
 import type { MessageContextService } from '../../messages/message_context_service.js';
 import type { AddressStore } from '../../storage/address_store/address_store.js';
 import type { CapsuleStore } from '../../storage/capsule_store/capsule_store.js';
@@ -326,19 +326,9 @@ describe('Private Execution test suite', () => {
     messageContextService.getMessageContextsByTxHash.mockResolvedValue([]);
     // Configure mock to actually perform sync_state calls (needed for nested call tests)
     contractSyncService.ensureContractSynced.mockImplementation(
-      async (contractAddress, functionToInvokeAfterSync, utilityExecutor, anchorBlockHeader, jobId, scopes) => {
+      async (contractAddress, functionToInvokeAfterSync, utilityExecutor, _anchorBlockHeader, _jobId, scopes) => {
         for (const scope of scopes) {
-          await syncState(
-            contractAddress,
-            contractStore,
-            functionToInvokeAfterSync,
-            utilityExecutor,
-            noteStore,
-            aztecNode,
-            anchorBlockHeader,
-            jobId,
-            scope,
-          );
+          await syncScope(contractAddress, contractStore, functionToInvokeAfterSync, utilityExecutor, scope);
         }
       },
     );
diff --git a/yarn-project/pxe/src/contract_sync/contract_sync_service.test.ts b/yarn-project/pxe/src/contract_sync/contract_sync_service.test.ts
@@ -272,6 +272,61 @@ describe('ContractSyncService', () => {
     });
   });
 
+  describe('multi-scope sync batching', () => {
+    it('batches nullifier sync across all unsynced scopes', async () => {
+      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [
+        scopeA,
+        scopeB,
+      ]);
+      expect(noteStore.getNotes).toHaveBeenCalledTimes(1);
+      expect(noteStore.getNotes).toHaveBeenCalledWith(
+        expect.objectContaining({ contractAddress, scopes: [scopeA, scopeB] }),
+        jobId,
+      );
+    });
+
+    it('only includes unsynced scopes in nullifier sync', async () => {
+      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
+      expect(noteStore.getNotes).toHaveBeenCalledTimes(1);
+      expect(noteStore.getNotes).toHaveBeenCalledWith(
+        expect.objectContaining({ contractAddress, scopes: [scopeA] }),
+        jobId,
+      );
+
+      noteStore.getNotes.mockClear();
+      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [
+        scopeA,
+        scopeB,
+      ]);
+      // scopeA is already cached, so nullifier sync only runs for scopeB
+      expect(noteStore.getNotes).toHaveBeenCalledTimes(1);
+      expect(noteStore.getNotes).toHaveBeenCalledWith(
+        expect.objectContaining({ contractAddress, scopes: [scopeB] }),
+        jobId,
+      );
+    });
+
+    it('re-runs nullifier sync after scope invalidation', async () => {
+      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [
+        scopeA,
+        scopeB,
+      ]);
+      noteStore.getNotes.mockClear();
+
+      service.invalidateContractForScopes(contractAddress, [scopeA]);
+      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [
+        scopeA,
+        scopeB,
+      ]);
+      // Only scopeA was invalidated, so nullifier sync runs for just scopeA
+      expect(noteStore.getNotes).toHaveBeenCalledTimes(1);
+      expect(noteStore.getNotes).toHaveBeenCalledWith(
+        expect.objectContaining({ contractAddress, scopes: [scopeA] }),
+        jobId,
+      );
+    });
+  });
+
   describe('invalidateContractForScopes', () => {
     const contract2 = AztecAddress.fromBigInt(300n);
 
diff --git a/yarn-project/pxe/src/contract_sync/contract_sync_service.ts b/yarn-project/pxe/src/contract_sync/contract_sync_service.ts
@@ -1,14 +1,16 @@
 import type { Logger } from '@aztec/foundation/log';
 import { Semaphore } from '@aztec/foundation/queue';
+import { isProtocolContract } from '@aztec/protocol-contracts';
 import type { FunctionCall, FunctionSelector } from '@aztec/stdlib/abi';
 import type { AztecAddress } from '@aztec/stdlib/aztec-address';
 import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import type { BlockHeader } from '@aztec/stdlib/tx';
 
 import type { StagedStore } from '../job_coordinator/job_coordinator.js';
+import { NoteService } from '../notes/note_service.js';
 import type { ContractStore } from '../storage/contract_store/contract_store.js';
 import type { NoteStore } from '../storage/note_store/note_store.js';
-import { syncState, verifyCurrentClassId } from './helpers.js';
+import { syncScope, verifyCurrentClassId } from './helpers.js';
 
 /** Maximum number of scope syncs running concurrently across the PXE. */
 const MAX_CONCURRENT_SCOPE_SYNCS = 5;
@@ -29,7 +31,7 @@ export class ContractSyncService implements StagedStore {
 
   // Tracks class ID verification per contract. Keyed by contract address only (no scope), since
   // class ID verification is scope-independent. Cleared on wipe/discard.
-  private verifiedClassIds: Map<string, Promise<void>> = new Map();
+  private classIdVerificationCache: Map<string, Promise<void>> = new Map();
 
   // Per-job excluded contract addresses - these contracts should not be synced.
   private excludedFromSync: Map<string, Set<string>> = new Map();
@@ -72,22 +74,8 @@ export class ContractSyncService implements StagedStore {
       return;
     }
 
-    this.#startSyncIfNeeded(
-      contractAddress,
-      scopes,
-      () => verifyCurrentClassId(contractAddress, this.aztecNode, this.contractStore, anchorBlockHeader),
-      scope =>
-        syncState(
-          contractAddress,
-          this.contractStore,
-          functionToInvokeAfterSync,
-          utilityExecutor,
-          this.noteStore,
-          this.aztecNode,
-          anchorBlockHeader,
-          jobId,
-          scope,
-        ),
+    this.#startSyncIfNeeded(contractAddress, scopes, anchorBlockHeader, jobId, scope =>
+      syncScope(contractAddress, this.contractStore, functionToInvokeAfterSync, utilityExecutor, scope),
     );
 
     await this.#awaitSync(contractAddress, scopes);
@@ -105,7 +93,7 @@ export class ContractSyncService implements StagedStore {
   wipe(): void {
     this.log.debug(`Wiping contract sync cache (${this.syncedContracts.size} entries)`);
     this.syncedContracts.clear();
-    this.verifiedClassIds.clear();
+    this.classIdVerificationCache.clear();
   }
 
   commit(jobId: string): Promise<void> {
@@ -118,7 +106,7 @@ export class ContractSyncService implements StagedStore {
     // We clear the synced contracts cache here because, when the job is discarded, any associated database writes from
     // the sync are also undone.
     this.syncedContracts.clear();
-    this.verifiedClassIds.clear();
+    this.classIdVerificationCache.clear();
     this.excludedFromSync.delete(jobId);
     return Promise.resolve();
   }
@@ -128,14 +116,16 @@ export class ContractSyncService implements StagedStore {
   }
 
   /**
-   * If there are unsynced scopes, starts one sync per scope (bounded by #syncSlot) and stores each promise in the
-   * cache with per-scope error cleanup. The verifyFn runs once for the whole fan-out and is awaited by every new
-   * scope's promise, matching the pre-parallelization invariant that a cache-miss batch re-verifies the class id.
+   * For each unsynced scope, creates a promise that waits on:
+   *  1. Class ID verification (cached per contract, scope-independent).
+   *  2. Note nullifier sync (shared, batched across all unsynced scopes).
+   *  3. Per-scope sync (individual, semaphore-bounded).
    */
   #startSyncIfNeeded(
     contractAddress: AztecAddress,
     scopes: AztecAddress[],
-    verifyFn: () => Promise<void>,
+    anchorBlockHeader: BlockHeader,
+    jobId: string,
     syncScopeFn: (scope: AztecAddress) => Promise<void>,
   ): void {
     const scopesToSync = scopes.filter(scope => !this.syncedContracts.has(toKey(contractAddress, scope)));
@@ -144,11 +134,13 @@ export class ContractSyncService implements StagedStore {
     }
 
     this.log.debug(`Syncing contract ${contractAddress} for ${scopesToSync.length} scope(s)`);
-    const verifyPromise = this.#getOrStartVerification(contractAddress, verifyFn);
+
+    const verifyPromise = this.#verifyClassId(contractAddress, anchorBlockHeader);
+    const syncNullifiersPromise = this.#syncNoteNullifiers(contractAddress, anchorBlockHeader, jobId, scopesToSync);
 
     for (const scope of scopesToSync) {
       const key = toKey(contractAddress, scope);
-      const promise = Promise.all([verifyPromise, this.#runBounded(() => syncScopeFn(scope))])
+      const promise = Promise.all([verifyPromise, syncNullifiersPromise, this.#runBounded(() => syncScopeFn(scope))])
         .then(() => {})
         .catch(err => {
           this.syncedContracts.delete(key);
@@ -158,21 +150,40 @@ export class ContractSyncService implements StagedStore {
     }
   }
 
-  /** Returns the cached verification promise for a contract, starting a new one if needed. Evicts from cache on failure so retries re-verify. */
-  #getOrStartVerification(contractAddress: AztecAddress, verifyFn: () => Promise<void>): Promise<void> {
+  /** Verifies the local class ID matches the on-chain value (cached, evicts on failure so retries re-verify). */
+  #verifyClassId(contractAddress: AztecAddress, anchorBlockHeader: BlockHeader): Promise<void> {
     const contractKey = contractAddress.toString();
-    const cached = this.verifiedClassIds.get(contractKey);
+    const cached = this.classIdVerificationCache.get(contractKey);
     if (cached) {
       return cached;
     }
-    const promise = verifyFn().catch(err => {
-      this.verifiedClassIds.delete(contractKey);
-      throw err;
-    });
-    this.verifiedClassIds.set(contractKey, promise);
+    const promise = verifyCurrentClassId(contractAddress, this.aztecNode, this.contractStore, anchorBlockHeader).catch(
+      err => {
+        this.classIdVerificationCache.delete(contractKey);
+        throw err;
+      },
+    );
+    this.classIdVerificationCache.set(contractKey, promise);
     return promise;
   }
 
+  /** Syncs note nullifiers across all unsynced scopes in a single batched call. */
+  async #syncNoteNullifiers(
+    contractAddress: AztecAddress,
+    anchorBlockHeader: BlockHeader,
+    jobId: string,
+    scopes: AztecAddress[],
+  ): Promise<void> {
+    // Protocol contracts don't have private state to sync
+    if (isProtocolContract(contractAddress)) {
+      return;
+    }
+    // This runs in parallel with per-scope sync (which also writes to the note store). That's safe because
+    // the note store handles concurrent operations.
+    const noteService = new NoteService(this.noteStore, this.aztecNode, anchorBlockHeader, jobId);
+    await noteService.syncNoteNullifiers(contractAddress, scopes);
+  }
+
   /** Runs fn while holding a slot in #syncSlot, bounding total concurrent scope syncs. */
   async #runBounded<T>(fn: () => Promise<T>): Promise<T> {
     await this.#syncSlot.acquire();
diff --git a/yarn-project/pxe/src/contract_sync/helpers.ts b/yarn-project/pxe/src/contract_sync/helpers.ts
@@ -6,9 +6,7 @@ import { DelayedPublicMutableValues, DelayedPublicMutableValuesWithHash } from '
 import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import type { BlockHeader } from '@aztec/stdlib/tx';
 
-import { NoteService } from '../notes/note_service.js';
 import type { ContractStore } from '../storage/contract_store/contract_store.js';
-import type { NoteStore } from '../storage/note_store/note_store.js';
 
 /**
  * Read the current class id of a contract from the execution data provider or AztecNode. If not found, class id
@@ -38,36 +36,26 @@ export async function readCurrentClassId(
   return currentClassId;
 }
 
-export async function syncState(
+export async function syncScope(
   contractAddress: AztecAddress,
   contractStore: ContractStore,
   functionToInvokeAfterSync: FunctionSelector | null,
   utilityExecutor: (privateSyncCall: FunctionCall, scopes: AztecAddress[]) => Promise<any>,
-  noteStore: NoteStore,
-  aztecNode: AztecNode,
-  anchorBlockHeader: BlockHeader,
-  jobId: string,
   scope: AztecAddress,
 ) {
   // Protocol contracts don't have private state to sync
-  if (!isProtocolContract(contractAddress)) {
-    const syncStateFunctionCall = await contractStore.getFunctionCall('sync_state', [scope], contractAddress);
-    if (functionToInvokeAfterSync && functionToInvokeAfterSync.equals(syncStateFunctionCall.selector)) {
-      throw new Error(
-        'Forbidden `sync_state` invocation. `sync_state` can only be invoked by PXE, manual execution can lead to inconsistencies.',
-      );
-    }
-
-    const noteService = new NoteService(noteStore, aztecNode, anchorBlockHeader, jobId);
-    const scopes: AztecAddress[] = [scope];
+  if (isProtocolContract(contractAddress)) {
+    return;
+  }
 
-    // Both sync_state and syncNoteNullifiers interact with the note store, but running them in parallel is safe
-    // because note store is designed to handle concurrent operations.
-    await Promise.all([
-      utilityExecutor(syncStateFunctionCall, scopes),
-      noteService.syncNoteNullifiers(contractAddress, scopes),
-    ]);
+  const syncStateFunctionCall = await contractStore.getFunctionCall('sync_state', [scope], contractAddress);
+  if (functionToInvokeAfterSync && functionToInvokeAfterSync.equals(syncStateFunctionCall.selector)) {
+    throw new Error(
+      'Forbidden `sync_state` invocation. `sync_state` can only be invoked by PXE, manual execution can lead to inconsistencies.',
+    );
   }
+
+  await utilityExecutor(syncStateFunctionCall, [scope]);
 }
 
 /**
