feat(pxe,nr): flesh out account stubs and don't exclude syncing for overrides (#23054)

flesh out account stubs so that syncing actually works for them, and
then remove their exclusion

Author: dbanks12

noir-projects/noir-contracts/contracts/account/simulated_ecdsa_account_contract/Nargo.toml

@@ -6,3 +6,4 @@ type = "contract"
 
 [dependencies]
 aztec = { path = "../../../../aztec-nr/aztec" }
+ecdsa_public_key_note = { path = "../../libs/ecdsa_public_key_note" }

noir-projects/noir-contracts/contracts/account/simulated_ecdsa_account_contract/src/main.nr

@@ -3,17 +3,26 @@ use aztec::macros::aztec;
 // Stub account contract for ECDSA accounts (both secp256k1 and secp256r1) used during simulation.
 // Matches the constructor signature of EcdsaKAccount / EcdsaRAccount so that deployment
 // simulations using this stub as an override do not fail on selector lookup.
-// See simulated_account_contract for the base stub without a constructor.
+// Mirrors the EcdsaKAccount/EcdsaRAccount storage layout so simulation sync can decode
+// the real account's signing_public_key note.
 #[aztec]
 pub contract SimulatedEcdsaAccount {
     use aztec::{
         authwit::{account::AccountActions, auth::IS_VALID_SELECTOR, entrypoint::app::AppPayload},
         context::PrivateContext,
-        macros::functions::{allow_phase_change, external, view},
+        macros::{functions::{allow_phase_change, external, view}, storage::storage},
         messages::encoding::MESSAGE_CIPHERTEXT_LEN,
         oracle::random::random,
+        state_vars::SinglePrivateImmutable,
     };
 
+    use ecdsa_public_key_note::EcdsaPublicKeyNote;
+
+    #[storage]
+    struct Storage<Context> {
+        signing_public_key: SinglePrivateImmutable<EcdsaPublicKeyNote, Context>,
+    }
+
     // Stub constructor matching the EcdsaKAccount / EcdsaRAccount constructor signature.
     // Does NOT use #[initializer] so that the macro does not inject
     // assert_initialization_matches_address_preimage_private, which would fail during kernelless
@@ -67,9 +76,4 @@ pub contract SimulatedEcdsaAccount {
     fn is_valid_impl(_context: &mut PrivateContext, _outer_hash: Field) -> bool {
         true
     }
-
-    #[external("utility")]
-    unconstrained fn sync_state() {
-        assert(false, "BUG ALERT: sync_state on a simulated account contract should never be triggered.");
-    }
 }

noir-projects/noir-contracts/contracts/account/simulated_schnorr_account_contract/src/main.nr

@@ -1,19 +1,30 @@
+mod public_key_note;
+
 use aztec::macros::aztec;
 
 // Stub account contract for Schnorr accounts used during simulation.
 // Matches the constructor signature of SchnorrAccount so that deployment
 // simulations using this stub as an override do not fail on selector lookup.
-// See simulated_account_contract for the base stub without a constructor.
+// Mirrors the SchnorrAccount storage layout so simulation sync can decode
+// the real account's signing_public_key note.
 #[aztec]
 pub contract SimulatedSchnorrAccount {
     use aztec::{
         authwit::{account::AccountActions, auth::IS_VALID_SELECTOR, entrypoint::app::AppPayload},
         context::PrivateContext,
-        macros::functions::{allow_phase_change, external, view},
+        macros::{functions::{allow_phase_change, external, view}, storage::storage},
         messages::encoding::MESSAGE_CIPHERTEXT_LEN,
         oracle::random::random,
+        state_vars::SinglePrivateImmutable,
     };
 
+    use crate::public_key_note::PublicKeyNote;
+
+    #[storage]
+    struct Storage<Context> {
+        signing_public_key: SinglePrivateImmutable<PublicKeyNote, Context>,
+    }
+
     // Stub constructor matching the SchnorrAccount constructor signature.
     // Does NOT use #[initializer] so that the macro does not inject
     // assert_initialization_matches_address_preimage_private, which would fail during kernelless
@@ -67,9 +78,4 @@ pub contract SimulatedSchnorrAccount {
     fn is_valid_impl(_context: &mut PrivateContext, _outer_hash: Field) -> bool {
         true
     }
-
-    #[external("utility")]
-    unconstrained fn sync_state() {
-        assert(false, "BUG ALERT: sync_state on a simulated account contract should never be triggered.");
-    }
 }

noir-projects/noir-contracts/contracts/account/simulated_schnorr_account_contract/src/public_key_note.nr

@@ -0,0 +1,10 @@
+use aztec::{macros::notes::note, protocol::traits::Packable};
+
+// Mirrors PublicKeyNote in schnorr_account_contract so the stub's auto-generated _compute_note_hash
+// can decode the real account's signing_public_key note during simulation sync.
+#[derive(Eq, Packable)]
+#[note]
+pub struct PublicKeyNote {
+    pub x: Field,
+    pub y: Field,
+}

yarn-project/pxe/src/contract_sync/contract_sync_service.test.ts

@@ -115,18 +115,6 @@ describe('ContractSyncService', () => {
       expectSyncedScopes([scopeA], [scopeB]);
     });
 
-    it('skips sync for excluded contract in the same job', async () => {
-      service.setExcludedFromSync(jobId, new Set([contractAddress.toString()]));
-      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
-      expectNoSync();
-    });
-
-    it('does not skip sync for excluded contract in a different job', async () => {
-      service.setExcludedFromSync('other-job', new Set([contractAddress.toString()]));
-      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
-      expectSyncedScopes([scopeA]);
-    });
-
     it('concurrent calls for same contract+scope share one sync promise', async () => {
       const p1 = service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [
         scopeA,
@@ -170,16 +158,6 @@ describe('ContractSyncService', () => {
   });
 
   describe('commit', () => {
-    it('clears exclusions for the given job', async () => {
-      service.setExcludedFromSync(jobId, new Set([contractAddress.toString()]));
-      await service.commit(jobId);
-
-      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
-      // When exclusions are set, contract sync is skipped. We verify the exclusions were cleared by confirming that sync
-      // was actually triggered.
-      expectSyncedScopes([scopeA]);
-    });
-
     it('does not clear sync cache', async () => {
       await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
       await service.commit(jobId);
@@ -197,32 +175,6 @@ describe('ContractSyncService', () => {
       // We check that the sync cache was cleared by checking that the sync was triggered twice.
       expectSyncedScopes([scopeA], [scopeA]);
     });
-
-    it('clears exclusions for the given job', async () => {
-      service.setExcludedFromSync(jobId, new Set([contractAddress.toString()]));
-      await service.discardStaged(jobId);
-
-      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
-      // When exclusions are set, contract sync is skipped. We verify the exclusions were cleared by confirming that sync
-      // was actually triggered.
-      expectSyncedScopes([scopeA]);
-    });
-
-    it('preserves exclusions for other jobs', async () => {
-      service.setExcludedFromSync(jobId, new Set([contractAddress.toString()]));
-      service.setExcludedFromSync('other-job', new Set([contractAddress.toString()]));
-      await service.discardStaged(jobId);
-
-      // jobId exclusion cleared, sync proceeds
-      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, jobId, [scopeA]);
-      expectSyncedScopes([scopeA]);
-
-      // other-job exclusion still active, sync skipped
-      await service.ensureContractSynced(contractAddress, null, utilityExecutor, anchorBlockHeader, 'other-job', [
-        scopeA,
-      ]);
-      expectSyncedScopes([scopeA]);
-    });
   });
 
   describe('class ID verification deduplication', () => {

yarn-project/pxe/src/contract_sync/contract_sync_service.ts

@@ -31,9 +31,6 @@ export class ContractSyncService implements StagedStore {
   // class ID verification is scope-independent. Cleared on wipe/discard.
   private verifiedClassIds: 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();
-
   // Bounds the number of scope syncs running concurrently. Scopes beyond this limit queue here. Sized to trade off
   // parallelism on non-ACIR work (node RPC, note store reads) against memory pressure from concurrent circuit
   // execution.
@@ -46,11 +43,6 @@ export class ContractSyncService implements StagedStore {
     private log: Logger,
   ) {}
 
-  /** Sets contracts that should be skipped during sync for a specific job. */
-  setExcludedFromSync(jobId: string, addresses: Set<string>): void {
-    this.excludedFromSync.set(jobId, addresses);
-  }
-
   /**
    * Ensures a contract's private state is synchronized and that the PXE holds the current class artifact.
    * Uses a cache to avoid redundant sync operations - the cache is wiped when the anchor block changes.
@@ -68,10 +60,6 @@ export class ContractSyncService implements StagedStore {
     jobId: string,
     scopes: AztecAddress[],
   ): Promise<void> {
-    if (this.#shouldSkipSync(jobId, contractAddress)) {
-      return;
-    }
-
     this.#startSyncIfNeeded(
       contractAddress,
       scopes,
@@ -108,24 +96,17 @@ export class ContractSyncService implements StagedStore {
     this.verifiedClassIds.clear();
   }
 
-  commit(jobId: string): Promise<void> {
-    // Clear excluded contracts for this job
-    this.excludedFromSync.delete(jobId);
+  commit(_jobId: string): Promise<void> {
     return Promise.resolve();
   }
 
-  discardStaged(jobId: string): Promise<void> {
+  discardStaged(_jobId: string): Promise<void> {
     // 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.excludedFromSync.delete(jobId);
     return Promise.resolve();
   }
-  /** Returns true if sync should be skipped for this contract */
-  #shouldSkipSync(jobId: string, contractAddress: AztecAddress): boolean {
-    return !!this.excludedFromSync.get(jobId)?.has(contractAddress.toString());
-  }
 
   /**
    * If there are unsynced scopes, starts one sync per scope (bounded by #syncSlot) and stores each promise in the

yarn-project/pxe/src/pxe.ts

@@ -1002,22 +1002,13 @@ export class PXE {
         const anchorBlockHeader = await this.anchorBlockStore.getBlockHeader();
         const syncTime = syncTimer.ms();
 
-        const overriddenContracts = overrides?.contracts ? new Set(Object.keys(overrides.contracts)) : undefined;
-        const hasOverriddenContracts = overriddenContracts !== undefined && overriddenContracts.size > 0;
-
-        if (hasOverriddenContracts && !skipKernels) {
+        if (overrides?.contracts && Object.keys(overrides.contracts).length > 0 && !skipKernels) {
           throw new Error(
             'Simulating with overridden contracts is not compatible with kernel execution. Please set skipKernels to true when simulating with overridden contracts.',
           );
         }
         const contractFunctionSimulator = this.#getSimulatorForTx(overrides);
 
-        if (hasOverriddenContracts) {
-          // Overridden contracts don't have a sync function, so calling sync on them would fail.
-          // We exclude them so the sync service skips them entirely.
-          this.contractSyncService.setExcludedFromSync(jobId, overriddenContracts);
-        }
-
         // Execution of private functions only; no proving, and no kernel logic.
         const privateExecutionResult = await this.#executePrivate({
           contractFunctionSimulator,