feat(avm-simulator): error stack tracking and enriching in AVM to mat…

…ch ACVM/ACIR-SIM (#6289)

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

@@ -140,6 +140,10 @@ contract AvmTest {
         a + b
     }
 
+    /************************************************************************
+     * Misc 
+     ************************************************************************/
+
     #[aztec(public-vm)]
     fn u128_addition_overflow() -> U128 {
         let max_u128: U128 = U128::from_hex("0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF");
@@ -153,10 +157,24 @@ contract AvmTest {
         U128::from_integer(should_overflow)
     }
 
-    #[aztec(private)]
-    fn enqueue_public_from_private() {
-        AvmTest::at(context.this_address()).set_opcode_u8().static_enqueue(&mut context);
-        AvmTest::at(context.this_address()).set_read_storage_single(5).enqueue(&mut context);
+    #[aztec(public-vm)]
+    fn to_radix_le(input: Field) -> [u8; 10] {
+        let result: [u8] = input.to_le_radix(/*base=*/ 2, /*limbs=*/ 10);
+        result.as_array()
+    }
+
+    // Helper functions to demonstrate an internal call stack in error messages
+    fn inner_helper_with_failed_assertion() {
+        let not_true = false;
+        assert(not_true == true, "This assertion should fail!");
+    }
+    fn helper_with_failed_assertion() {
+        inner_helper_with_failed_assertion();
+    }
+
+    #[aztec(public-vm)]
+    fn assertion_failure() {
+        helper_with_failed_assertion()
     }
 
     /************************************************************************
@@ -341,9 +359,12 @@ contract AvmTest {
         context.message_portal(recipient, content)
     }
 
-    #[aztec(public-vm)]
-    fn to_radix_le(input: Field) -> [u8; 10] {
-        let result: [u8] = input.to_le_radix(/*base=*/ 2, /*limbs=*/ 10);
-        result.as_array()
+    /**
+     * Enqueue a public call from private
+     */
+    #[aztec(private)]
+    fn enqueue_public_from_private() {
+        AvmTest::at(context.this_address()).set_opcode_u8().static_enqueue(&mut context);
+        AvmTest::at(context.this_address()).set_read_storage_single(5).enqueue(&mut context);
     }
 }

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

@@ -34,9 +34,14 @@ describe('e2e_avm_simulator', () => {
     });
 
     describe('Assertions', () => {
-      it('Processes assertions in the PXE', async () => {
+      it('PXE processes failed assertions and fills in the error message with the expression', async () => {
+        await expect(avmContract.methods.assertion_failure().simulate()).rejects.toThrow(
+          "Assertion failed: This assertion should fail! 'not_true == true'",
+        );
+      });
+      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!",
+          "Assertion failed: Nullifier doesn't exist! 'context.nullifier_exists(nullifier, context.this_address())'",
         );
       });
     });

yarn-project/simulator/src/avm/avm_machine_state.ts

@@ -2,7 +2,6 @@ import { type Fr } from '@aztec/circuits.js';
 
 import { type Gas, GasDimensions } from './avm_gas.js';
 import { TaggedMemory } from './avm_memory_types.js';
-import { AvmContractCallResults } from './avm_message_call_result.js';
 import { OutOfGasError } from './errors.js';
 
 /**
@@ -36,7 +35,7 @@ export class AvmMachineState {
    * Signals that execution should end.
    * AvmContext execution continues executing instructions until the machine state signals "halted"
    */
-  public halted: boolean = false;
+  private halted: boolean = false;
   /** Signals that execution has reverted normally (this does not cover exceptional halts) */
   private reverted: boolean = false;
   /** Output data must NOT be modified once it is set */
@@ -118,40 +117,24 @@ export class AvmMachineState {
     this.output = output;
   }
 
+  public getHalted(): boolean {
+    return this.halted;
+  }
+
+  public getReverted(): boolean {
+    return this.reverted;
+  }
+
+  public getOutput(): Fr[] {
+    return this.output;
+  }
+
   /**
    * Flag an exceptional halt. Clears gas left and sets the reverted flag. No output data.
    */
-  protected exceptionalHalt() {
+  private exceptionalHalt() {
     GasDimensions.forEach(dimension => (this[`${dimension}Left`] = 0));
     this.reverted = true;
     this.halted = true;
   }
-
-  /**
-   * Get a summary of execution results for a halted machine state
-   * @returns summary of execution results
-   */
-  public getResults(): AvmContractCallResults {
-    if (!this.halted) {
-      throw new Error('Execution results are not ready! Execution is ongoing.');
-    }
-    let revertReason = undefined;
-    if (this.reverted) {
-      if (this.output.length === 0) {
-        revertReason = new Error('Assertion failed.');
-      } else {
-        try {
-          // We remove the first element which is the 'error selector'.
-          const revertOutput = this.output.slice(1);
-          // Try to interpret the output as a text string.
-          revertReason = new Error(
-            'Assertion failed: ' + String.fromCharCode(...revertOutput.map(fr => fr.toNumber())),
-          );
-        } catch (e) {
-          revertReason = new Error('Assertion failed: <cannot interpret as string>');
-        }
-      }
-    }
-    return new AvmContractCallResults(this.reverted, this.output, revertReason);
-  }
 }

yarn-project/simulator/src/avm/avm_message_call_result.ts

@@ -1,24 +1,13 @@
 import { type Fr } from '@aztec/foundation/fields';
 
+import { type AvmRevertReason } from './errors.js';
+
 /**
  * Results of an contract call's execution in the AVM.
  */
 export class AvmContractCallResults {
-  public readonly reverted: boolean;
-  public readonly output: Fr[];
-
-  /** For exceptional halts */
-  public readonly revertReason: Error | undefined;
-
-  constructor(reverted: boolean, output: Fr[], revertReason?: Error) {
-    this.reverted = reverted;
-    this.output = output;
-    this.revertReason = revertReason;
-  }
+  constructor(public reverted: boolean, public output: Fr[], public revertReason?: AvmRevertReason) {}
 
-  /**
-   * Generate a string representation of call results.
-   */
   toString(): string {
     let resultsStr = `reverted: ${this.reverted}, output: ${this.output}`;
     if (this.revertReason) {

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

@@ -62,7 +62,7 @@ describe('AVM simulator: injected bytecode', () => {
     const results = await new AvmSimulator(context).executeBytecode(bytecode);
     expect(results.reverted).toBe(true);
     expect(results.output).toEqual([]);
-    expect(results.revertReason?.name).toEqual('OutOfGasError');
+    expect(results.revertReason?.message).toEqual('Not enough L2GAS gas left');
     expect(context.machineState.l2GasLeft).toEqual(0);
     expect(context.machineState.daGasLeft).toEqual(0);
   });

yarn-project/simulator/src/avm/avm_simulator.ts

@@ -5,7 +5,13 @@ import { strict as assert } from 'assert';
 import { isAvmBytecode } from '../public/transitional_adaptors.js';
 import type { AvmContext } from './avm_context.js';
 import { AvmContractCallResults } from './avm_message_call_result.js';
-import { AvmExecutionError, InvalidProgramCounterError, NoBytecodeForContractError } from './errors.js';
+import {
+  AvmExecutionError,
+  InvalidProgramCounterError,
+  NoBytecodeForContractError,
+  revertReasonFromExceptionalHalt,
+  revertReasonFromExplicitRevert,
+} from './errors.js';
 import type { Instruction } from './opcodes/index.js';
 import { decodeFromBytecode } from './serialization/bytecode_serialization.js';
 
@@ -56,7 +62,7 @@ export class AvmSimulator {
     try {
       // Execute instruction pointed to by the current program counter
       // continuing until the machine state signifies a halt
-      while (!machineState.halted) {
+      while (!machineState.getHalted()) {
         const instruction = instructions[machineState.pc];
         assert(
           !!instruction,
@@ -76,21 +82,25 @@ export class AvmSimulator {
         }
       }
 
-      // Return results for processing by calling context
-      const results = machineState.getResults();
+      const output = machineState.getOutput();
+      const reverted = machineState.getReverted();
+      const revertReason = reverted ? revertReasonFromExplicitRevert(output, this.context) : undefined;
+      const results = new AvmContractCallResults(reverted, output, revertReason);
       this.log.debug(`Context execution results: ${results.toString()}`);
+      // Return results for processing by calling context
       return results;
-    } catch (e) {
-      this.log.verbose('Exceptional halt');
-      if (!(e instanceof AvmExecutionError)) {
-        this.log.verbose(`Unknown error thrown by avm: ${e}`);
-        throw e;
+    } catch (err: any) {
+      this.log.verbose('Exceptional halt (revert by something other than REVERT opcode)');
+      if (!(err instanceof AvmExecutionError)) {
+        this.log.verbose(`Unknown error thrown by AVM: ${err}`);
+        throw err;
       }
 
-      // Return results for processing by calling context
-      // Note: "exceptional halts" cannot return data
-      const results = new AvmContractCallResults(/*reverted=*/ true, /*output=*/ [], /*revertReason=*/ e);
+      const revertReason = revertReasonFromExceptionalHalt(err, this.context);
+      // Note: "exceptional halts" cannot return data, hence []
+      const results = new AvmContractCallResults(/*reverted=*/ true, /*output=*/ [], revertReason);
       this.log.debug(`Context execution results: ${results.toString()}`);
+      // Return results for processing by calling context
       return results;
     }
   }

yarn-project/simulator/src/avm/errors.ts

@@ -1,12 +1,16 @@
-import { type AztecAddress } from '@aztec/circuits.js';
+import { type FailingFunction, type NoirCallStack } from '@aztec/circuit-types';
+import { type AztecAddress, type Fr } from '@aztec/circuits.js';
+
+import { ExecutionError } from '../common/errors.js';
+import { type AvmContext } from './avm_context.js';
 
 /**
  * Avm-specific errors should derive from this
  */
 export abstract class AvmExecutionError extends Error {
-  constructor(message: string, ...rest: any[]) {
-    super(message, ...rest);
-    this.name = 'AvmInterpreterError';
+  constructor(message: string) {
+    super(message);
+    this.name = 'AvmExecutionError';
   }
 }
 
@@ -63,3 +67,89 @@ export class OutOfGasError extends AvmExecutionError {
     this.name = 'OutOfGasError';
   }
 }
+
+/**
+ * Error thrown to propagate a nested call's revert.
+ * @param message - the error's message
+ * @param nestedError - the revert reason of the nested call
+ */
+export class RethrownError extends AvmExecutionError {
+  constructor(message: string, public nestedError: AvmRevertReason) {
+    super(message);
+    this.name = 'RethrownError';
+  }
+}
+
+/**
+ * Meaningfully named alias for ExecutionError when used in the context of the AVM.
+ * Maintains a recursive structure reflecting the AVM's external callstack/errorstack, where
+ * options.cause is the error that caused this error (if this is not the root-cause itself).
+ */
+export class AvmRevertReason extends ExecutionError {
+  constructor(message: string, failingFunction: FailingFunction, noirCallStack: NoirCallStack, options?: ErrorOptions) {
+    super(message, failingFunction, noirCallStack, options);
+  }
+}
+
+/**
+ * Helper to create a "revert reason" error optionally with a nested error cause.
+ *
+ * @param message - the error message
+ * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack
+ * @param nestedError - the error that caused this one (if this is not the root-cause itself)
+ */
+function createRevertReason(message: string, context: AvmContext, nestedError?: AvmRevertReason): AvmRevertReason {
+  return new AvmRevertReason(
+    message,
+    /*failingFunction=*/ {
+      contractAddress: context.environment.address,
+      functionSelector: context.environment.temporaryFunctionSelector,
+    },
+    /*noirCallStack=*/ [...context.machineState.internalCallStack, context.machineState.pc].map(pc => `0.${pc}`),
+    /*options=*/ { cause: nestedError },
+  );
+}
+
+/**
+ * Create a "revert reason" error for an exceptional halt,
+ * creating the recursive structure if the halt was a RethrownError.
+ *
+ * @param haltingError - the lower-level error causing the exceptional halt
+ * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack
+ */
+export function revertReasonFromExceptionalHalt(haltingError: AvmExecutionError, context: AvmContext): AvmRevertReason {
+  // A RethrownError has a nested/child AvmRevertReason
+  const nestedError = haltingError instanceof RethrownError ? haltingError.nestedError : undefined;
+  return createRevertReason(haltingError.message, context, nestedError);
+}
+
+/**
+ * Create a "revert reason" error for an explicit revert (a root cause).
+ *
+ * @param revertData - output data of the explicit REVERT instruction
+ * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack
+ */
+export function revertReasonFromExplicitRevert(revertData: Fr[], context: AvmContext): AvmRevertReason {
+  const revertMessage = decodeRevertDataAsMessage(revertData);
+  return createRevertReason(revertMessage, context);
+}
+
+/**
+ * Interpret revert data as a message string.
+ *
+ * @param revertData - output data of an explicit REVERT instruction
+ */
+export function decodeRevertDataAsMessage(revertData: Fr[]): string {
+  if (revertData.length === 0) {
+    return 'Assertion failed.';
+  } else {
+    try {
+      // We remove the first element which is the 'error selector'.
+      const revertOutput = revertData.slice(1);
+      // Try to interpret the output as a text string.
+      return 'Assertion failed: ' + String.fromCharCode(...revertOutput.map(fr => fr.toNumber()));
+    } catch (e) {
+      return 'Assertion failed: <cannot interpret as string>';
+    }
+  }
+}

yarn-project/simulator/src/avm/opcodes/external_calls.test.ts

@@ -271,11 +271,9 @@ describe('External Calls', () => {
       const instruction = new Return(/*indirect=*/ 0, /*returnOffset=*/ 0, returnData.length);
       await instruction.execute(context);
 
-      expect(context.machineState.halted).toBe(true);
-      expect(context.machineState.getResults()).toEqual({
-        reverted: false,
-        output: returnData,
-      });
+      expect(context.machineState.getHalted()).toBe(true);
+      expect(context.machineState.getReverted()).toBe(false);
+      expect(context.machineState.getOutput()).toEqual(returnData);
     });
   });
 
@@ -302,12 +300,9 @@ describe('External Calls', () => {
       const instruction = new Revert(/*indirect=*/ 0, /*returnOffset=*/ 0, returnData.length);
       await instruction.execute(context);
 
-      expect(context.machineState.halted).toBe(true);
-      expect(context.machineState.getResults()).toEqual({
-        reverted: true,
-        revertReason: new Error('Assertion failed: assert message'),
-        output: returnData.map(f => f.toFr()),
-      });
+      expect(context.machineState.getHalted()).toBe(true);
+      expect(context.machineState.getReverted()).toBe(true);
+      expect(context.machineState.getOutput()).toEqual(returnData.map(f => f.toFr()));
     });
   });
 });