chore: refactor AVM simulator's side-effect tracing

noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr

@@ -79,6 +79,7 @@ pub fn parse_public_call_stack_item_from_oracle(fields: [Field; ENQUEUE_PUBLIC_F
 
     // Note: Not using PublicCirclePublicInputs::deserialize here, because everything below args_hash is 0 and
     // there is no more data in fields because there is only ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_SIZE fields!
+    // WARNING: if updating, see comment in public_call_stack_item.ts's PublicCallStackItem.hash()
     let item = PublicCallStackItem {
         contract_address: AztecAddress::from_field(reader.read()),
         function_data: FunctionData { selector: FunctionSelector::from_field(reader.read()), is_private: false },

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -363,19 +363,19 @@ contract AvmTest {
     // Use the standard context interface to check for a nullifier
     #[aztec(public)]
     fn nullifier_exists(nullifier: Field) -> bool {
-        context.nullifier_exists(nullifier, context.this_address())
+        context.nullifier_exists(nullifier, context.storage_address())
     }
 
     #[aztec(public)]
     fn assert_nullifier_exists(nullifier: Field) {
-        assert(context.nullifier_exists(nullifier, context.this_address()), "Nullifier doesn't exist!");
+        assert(context.nullifier_exists(nullifier, context.storage_address()), "Nullifier doesn't exist!");
     }
 
     // Use the standard context interface to emit a new nullifier
     #[aztec(public)]
     fn emit_nullifier_and_check(nullifier: Field) {
         context.push_new_nullifier(nullifier, 0);
-        let exists = context.nullifier_exists(nullifier, context.this_address());
+        let exists = context.nullifier_exists(nullifier, context.storage_address());
         assert(exists, "Nullifier was just created, but its existence wasn't detected!");
     }
 

noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr

@@ -30,6 +30,7 @@ impl Hash for PublicCallStackItem {
 
 impl PublicCallStackItem {
     fn as_execution_request(self) -> Self {
+        // WARNING: if updating, see comment in public_call_stack_item.ts's `PublicCallStackItem.hash()`
         let public_inputs = self.public_inputs;
         let mut request_public_inputs = PublicCircuitPublicInputs::empty();
         request_public_inputs.call_context = public_inputs.call_context;

yarn-project/bb-prover/src/avm_proving.test.ts

@@ -35,6 +35,7 @@ import {
   initContext,
   initExecutionEnvironment,
   initHostStorage,
+  initPersistableStateManager,
 } from '@aztec/simulator/avm/fixtures';
 
 import { jest } from '@jest/globals';
@@ -43,11 +44,7 @@ import fs from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import path from 'path';
 
-import { AvmPersistableStateManager } from '../../simulator/src/avm/journal/journal.js';
-import {
-  convertAvmResultsToPxResult,
-  createPublicExecution,
-} from '../../simulator/src/public/transitional_adaptors.js';
+import { PublicSideEffectTrace } from '../../simulator/src/public/side_effect_trace.js';
 import { SerializableContractInstance } from '../../types/src/contracts/contract_instance.js';
 import { type BBSuccess, BB_RESULT, generateAvmProof, verifyAvmProof } from './bb/execute.js';
 import { extractVkData } from './verification_key/verification_key_data.js';
@@ -224,15 +221,13 @@ const proveAndVerifyAvmTestContract = async (
   storageDb.storageRead.mockResolvedValue(Promise.resolve(storageValue));
 
   const hostStorage = initHostStorage({ contractsDb });
-  const persistableState = new AvmPersistableStateManager(hostStorage);
+  const trace = new PublicSideEffectTrace(startSideEffectCounter);
+  const persistableState = initPersistableStateManager({ hostStorage, trace });
   const context = initContext({ env: environment, persistableState });
   const nestedCallBytecode = getAvmTestContractBytecode('add_args_return');
-  jest
-    .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
-    .mockReturnValue(Promise.resolve(nestedCallBytecode));
+  jest.spyOn(hostStorage.contractsDb, 'getBytecode').mockResolvedValue(nestedCallBytecode);
 
   const startGas = new Gas(context.machineState.gasLeft.daGas, context.machineState.gasLeft.l2Gas);
-  const oldPublicExecution = createPublicExecution(startSideEffectCounter, environment, calldata);
 
   const internalLogger = createDebugLogger('aztec:avm-proving-test');
   const logger = (msg: string, _data?: any) => internalLogger.verbose(msg);
@@ -255,25 +250,21 @@ const proveAndVerifyAvmTestContract = async (
     expect(avmResult.revertReason?.message).toContain(assertionErrString);
   }
 
-  const pxResult = convertAvmResultsToPxResult(
-    avmResult,
-    startSideEffectCounter,
-    oldPublicExecution,
+  const pxResult = trace.toPublicExecutionResult(
+    environment,
     startGas,
-    context,
-    simulator.getBytecode(),
+    /*endGasLeft=*/ Gas.from(context.machineState.gasLeft),
+    /*bytecode=*/ simulator.getBytecode()!,
+    avmResult,
     functionName,
   );
-  // TODO(dbanks12): public inputs should not be empty.... Need to construct them from AvmContext?
-  const uncompressedBytecode = simulator.getBytecode()!;
-  const publicInputs = getPublicInputs(pxResult);
 
   const avmCircuitInputs = new AvmCircuitInputs(
     functionName,
-    uncompressedBytecode,
-    context.environment.calldata,
-    publicInputs,
-    pxResult.avmHints,
+    /*bytecode=*/ simulator.getBytecode()!, // uncompressed bytecode
+    /*calldata=*/ context.environment.calldata,
+    /*publicInputs=*/ getPublicInputs(pxResult),
+    /*avmHints=*/ pxResult.avmCircuitHints,
   );
 
   // Then we prove.

yarn-project/circuit-types/src/mocks.ts

@@ -203,8 +203,10 @@ export const randomContractArtifact = (): ContractArtifact => ({
   notes: {},
 });
 
-export const randomContractInstanceWithAddress = (opts: { contractClassId?: Fr } = {}): ContractInstanceWithAddress =>
-  SerializableContractInstance.random(opts).withAddress(AztecAddress.random());
+export const randomContractInstanceWithAddress = (
+  opts: { contractClassId?: Fr } = {},
+  address: AztecAddress = AztecAddress.random(),
+): ContractInstanceWithAddress => SerializableContractInstance.random(opts).withAddress(address);
 
 export const randomDeployedContract = () => {
   const artifact = randomContractArtifact();

yarn-project/circuits.js/src/structs/avm/avm.ts

@@ -243,6 +243,7 @@ export class AvmContractInstanceHint {
   }
 }
 
+// TODO(dbanks12): rename AvmCircuitHints
 export class AvmExecutionHints {
   public readonly storageValues: Vector<AvmKeyValueHint>;
   public readonly noteHashExists: Vector<AvmKeyValueHint>;
@@ -267,6 +268,14 @@ export class AvmExecutionHints {
     this.contractInstances = new Vector(contractInstances);
   }
 
+  /**
+   * Return an empty instance.
+   * @returns an empty instance.
+   */
+  empty() {
+    return new AvmExecutionHints([], [], [], [], [], []);
+  }
+
   /**
    * Serializes the inputs to a buffer.
    * @returns - The inputs serialized to a buffer.

yarn-project/circuits.js/src/structs/contract_storage_read.ts

@@ -23,30 +23,19 @@ export class ContractStorageRead {
     /**
      * Side effect counter tracking position of this event in tx execution.
      */
-    public readonly sideEffectCounter: number,
+    public readonly counter: number,
+    /**
+     * Contract address whose storage is being read.
+     */
     public contractAddress?: AztecAddress, // TODO: Should not be optional. This is a temporary hack to silo the storage slot with the correct address for nested executions.
   ) {}
 
-  static from(args: {
-    /**
-     * Storage slot we are reading from.
-     */
-    storageSlot: Fr;
-    /**
-     * Value read from the storage slot.
-     */
-    currentValue: Fr;
-    /**
-     * Side effect counter tracking position of this event in tx execution.
-     */
-    sideEffectCounter: number;
-    contractAddress?: AztecAddress;
-  }) {
-    return new ContractStorageRead(args.storageSlot, args.currentValue, args.sideEffectCounter, args.contractAddress);
+  static from(args: { storageSlot: Fr; currentValue: Fr; counter: number; contractAddress?: AztecAddress }) {
+    return new ContractStorageRead(args.storageSlot, args.currentValue, args.counter, args.contractAddress);
   }
 
   toBuffer() {
-    return serializeToBuffer(this.storageSlot, this.currentValue, new Fr(this.sideEffectCounter));
+    return serializeToBuffer(this.storageSlot, this.currentValue, new Fr(this.counter));
   }
 
   static fromBuffer(buffer: Buffer | BufferReader) {
@@ -59,15 +48,15 @@ export class ContractStorageRead {
   }
 
   isEmpty() {
-    return this.storageSlot.isZero() && this.currentValue.isZero() && this.sideEffectCounter == 0;
+    return this.storageSlot.isZero() && this.currentValue.isZero() && this.counter == 0;
   }
 
   toFriendlyJSON() {
     return `Slot=${this.storageSlot.toFriendlyJSON()}: ${this.currentValue.toFriendlyJSON()}`;
   }
 
   toFields(): Fr[] {
-    const fields = [this.storageSlot, this.currentValue, new Fr(this.sideEffectCounter)];
+    const fields = [this.storageSlot, this.currentValue, new Fr(this.counter)];
     if (fields.length !== CONTRACT_STORAGE_READ_LENGTH) {
       throw new Error(
         `Invalid number of fields for ContractStorageRead. Expected ${CONTRACT_STORAGE_READ_LENGTH}, got ${fields.length}`,
@@ -81,8 +70,8 @@ export class ContractStorageRead {
 
     const storageSlot = reader.readField();
     const currentValue = reader.readField();
-    const sideEffectCounter = reader.readField().toNumber();
+    const counter = reader.readField().toNumber();
 
-    return new ContractStorageRead(storageSlot, currentValue, sideEffectCounter);
+    return new ContractStorageRead(storageSlot, currentValue, counter);
   }
 }

yarn-project/circuits.js/src/structs/contract_storage_update_request.ts

@@ -22,14 +22,17 @@ export class ContractStorageUpdateRequest {
      */
     public readonly newValue: Fr,
     /**
-     * Optional side effect counter tracking position of this event in tx execution.
+     * Side effect counter tracking position of this event in tx execution.
+     */
+    public readonly counter: number,
+    /**
+     * Contract address whose storage is being read.
      */
-    public readonly sideEffectCounter: number,
     public contractAddress?: AztecAddress, // TODO: Should not be optional. This is a temporary hack to silo the storage slot with the correct address for nested executions.
   ) {}
 
   toBuffer() {
-    return serializeToBuffer(this.storageSlot, this.newValue, this.sideEffectCounter);
+    return serializeToBuffer(this.storageSlot, this.newValue, this.counter);
   }
 
   static fromBuffer(buffer: Buffer | BufferReader) {
@@ -52,7 +55,7 @@ export class ContractStorageUpdateRequest {
    * @returns The array.
    */
   static getFields(fields: FieldsOf<ContractStorageUpdateRequest>) {
-    return [fields.storageSlot, fields.newValue, fields.sideEffectCounter, fields.contractAddress] as const;
+    return [fields.storageSlot, fields.newValue, fields.counter, fields.contractAddress] as const;
   }
 
   static empty() {
@@ -65,12 +68,12 @@ export class ContractStorageUpdateRequest {
 
   toFriendlyJSON() {
     return `Slot=${this.storageSlot.toFriendlyJSON()}: ${this.newValue.toFriendlyJSON()}, sideEffectCounter=${
-      this.sideEffectCounter
+      this.counter
     }`;
   }
 
   toFields(): Fr[] {
-    const fields = [this.storageSlot, this.newValue, new Fr(this.sideEffectCounter)];
+    const fields = [this.storageSlot, this.newValue, new Fr(this.counter)];
     if (fields.length !== CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH) {
       throw new Error(
         `Invalid number of fields for ContractStorageUpdateRequest. Expected ${CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH}, got ${fields.length}`,

yarn-project/circuits.js/src/structs/public_call_stack_item.ts

@@ -90,15 +90,24 @@ export class PublicCallStackItem {
    * @returns Hash.
    */
   public hash() {
+    let publicInputsToHash = this.publicInputs;
     if (this.isExecutionRequest) {
+      // An execution request (such as an enqueued call from private) is hashed with
+      // only the publicInput members present in a PublicCallRequest.
+      // This allows us to check that the request (which is created/hashed before
+      // side-effects and output info are unknown for public calls) matches the call
+      // being processed by a kernel iteration.
+      // WARNING: This subset of publicInputs that is set here must align with
+      // `parse_public_call_stack_item_from_oracle` in enqueue_public_function_call.nr
+      // and `PublicCallStackItem::as_execution_request()` in public_call_stack_item.ts
       const { callContext, argsHash } = this.publicInputs;
-      this.publicInputs = PublicCircuitPublicInputs.empty();
-      this.publicInputs.callContext = callContext;
-      this.publicInputs.argsHash = argsHash;
+      publicInputsToHash = PublicCircuitPublicInputs.empty();
+      publicInputsToHash.callContext = callContext;
+      publicInputsToHash.argsHash = argsHash;
     }
 
     return pedersenHash(
-      [this.contractAddress, this.functionData.hash(), this.publicInputs.hash()],
+      [this.contractAddress, this.functionData.hash(), publicInputsToHash.hash()],
       GeneratorIndex.CALL_STACK_ITEM,
     );
   }

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

@@ -38,7 +38,7 @@ describe('e2e_avm_simulator', () => {
       });
       it('PXE processes failed assertions and fills in the error message with the expression (even complex ones)', async () => {
         await expect(avmContract.methods.assert_nullifier_exists(123).simulate()).rejects.toThrow(
-          "Assertion failed: Nullifier doesn't exist! 'context.nullifier_exists(nullifier, context.this_address())'",
+          "Assertion failed: Nullifier doesn't exist! 'context.nullifier_exists(nullifier, context.storage_address())'",
         );
       });
     });

yarn-project/noir-protocol-circuits-types/src/index.ts

@@ -572,7 +572,7 @@ export function convertSimulatedPublicSetupInputsToWitnessMap(inputs: PublicKern
 }
 
 /**
- * Converts the inputs of the public setup circuit into a witness map
+ * Converts the inputs of the public app logic circuit into a witness map
  * @param inputs - The public kernel inputs.
  * @returns The witness map
  */

yarn-project/noir-protocol-circuits-types/src/type_conversion.ts

@@ -1826,7 +1826,7 @@ export function mapStorageUpdateRequestToNoir(
   return {
     storage_slot: mapFieldToNoir(storageUpdateRequest.storageSlot),
     new_value: mapFieldToNoir(storageUpdateRequest.newValue),
-    counter: mapNumberToNoir(storageUpdateRequest.sideEffectCounter),
+    counter: mapNumberToNoir(storageUpdateRequest.counter),
   };
 }
 /**
@@ -1855,7 +1855,7 @@ export function mapStorageReadToNoir(storageRead: ContractStorageRead): StorageR
   return {
     storage_slot: mapFieldToNoir(storageRead.storageSlot),
     current_value: mapFieldToNoir(storageRead.currentValue),
-    counter: mapNumberToNoir(storageRead.sideEffectCounter),
+    counter: mapNumberToNoir(storageRead.counter),
   };
 }
 /**

yarn-project/simulator/src/avm/avm_context.test.ts

@@ -16,6 +16,7 @@ describe('Avm Context', () => {
       allSameExcept(context.environment, {
         address: newAddress,
         storageAddress: newAddress,
+        contractCallDepth: Fr.ONE,
         // Calldata also includes AvmContextInputs
         calldata: anyAvmContextInputs().concat(newCalldata),
         isStaticCall: false,
@@ -46,6 +47,7 @@ describe('Avm Context', () => {
       allSameExcept(context.environment, {
         address: newAddress,
         storageAddress: newAddress,
+        contractCallDepth: Fr.ONE,
         // Calldata also includes AvmContextInputs
         calldata: anyAvmContextInputs().concat(newCalldata),
         isStaticCall: true,

yarn-project/simulator/src/avm/avm_execution_environment.test.ts

@@ -16,6 +16,7 @@ describe('Execution Environment', () => {
       allSameExcept(executionEnvironment, {
         address: newAddress,
         storageAddress: newAddress,
+        contractCallDepth: Fr.ONE,
         // Calldata also includes AvmContextInputs
         calldata: anyAvmContextInputs().concat(calldata),
       }),
@@ -30,6 +31,7 @@ describe('Execution Environment', () => {
     expect(newExecutionEnvironment).toEqual(
       allSameExcept(executionEnvironment, {
         address: newAddress,
+        contractCallDepth: Fr.ONE,
         isDelegateCall: true,
         // Calldata also includes AvmContextInputs
         calldata: anyAvmContextInputs().concat(calldata),
@@ -49,6 +51,7 @@ describe('Execution Environment', () => {
       allSameExcept(executionEnvironment, {
         address: newAddress,
         storageAddress: newAddress,
+        contractCallDepth: Fr.ONE,
         isStaticCall: true,
         // Calldata also includes AvmContextInputs
         calldata: anyAvmContextInputs().concat(calldata),