fix: Include revertdata in Avm simulation errors for failures in nonrevertible

scripts/ci/get_e2e_jobs.sh

@@ -23,10 +23,13 @@ allow_list=(
   "e2e_avm_simulator"
   "e2e_block_building"
   "e2e_cross_chain_messaging"
+  "e2e_crowdfunding_and_claim"
   "e2e_deploy_contract"
   "e2e_fees"
+  "e2e_fees_failures"
   "e2e_fees_gas_estimation"
   "e2e_fees_private_payments"
+  "e2e_fees_private_refunds"
   "e2e_max_block_number"
   "e2e_nested_contract"
   "e2e_ordering"

yarn-project/circuit-types/src/tx/processed_tx.ts

@@ -1,5 +1,6 @@
 import {
   type AvmProvingRequest,
+  type AvmSimulationError,
   EncryptedNoteTxL2Logs,
   EncryptedTxL2Logs,
   PublicDataWrite,
@@ -52,7 +53,7 @@ export type ProcessedTx = Pick<Tx, 'clientIvcProof' | 'noteEncryptedLogs' | 'enc
   /**
    * Reason the tx was reverted.
    */
-  revertReason: SimulationError | undefined;
+  revertReason: AvmSimulationError | undefined;
   /**
    * The request for AVM proving.
    */
@@ -124,7 +125,7 @@ export function makeProcessedTx(
     avmProvingRequest,
     finalPublicDataUpdateRequests,
   }: {
-    revertReason?: SimulationError;
+    revertReason?: AvmSimulationError;
     gasUsed?: ProcessedTx['gasUsed'];
     avmProvingRequest?: AvmProvingRequest;
     finalPublicDataUpdateRequests?: PublicDataUpdateRequest[];

yarn-project/circuit-types/src/tx/public_simulation_output.ts

@@ -2,7 +2,7 @@ import { CombinedAccumulatedData, CombinedConstantData, Fr, Gas } from '@aztec/c
 import { mapValues } from '@aztec/foundation/collection';
 
 import { EncryptedTxL2Logs, UnencryptedTxL2Logs } from '../logs/tx_l2_logs.js';
-import { type SimulationError } from '../simulation_error.js';
+import { type FailingFunction, type NoirCallStack, SimulationError } from '../simulation_error.js';
 import { type PublicKernelPhase } from './processed_tx.js';
 
 /** Return values of simulating a circuit. */
@@ -37,14 +37,46 @@ export class NestedProcessReturnValues {
   }
 }
 
+/**
+ * A simulation error for the AVM.
+ * This error includes the revert data returned from the top level function.
+ */
+export class AvmSimulationError extends SimulationError {
+  constructor(
+    originalMessage: string,
+    functionErrorStack: FailingFunction[],
+    public revertData: Fr[],
+    noirErrorStack?: NoirCallStack,
+    options?: ErrorOptions,
+  ) {
+    super(originalMessage, functionErrorStack, noirErrorStack, options);
+  }
+
+  override toJSON() {
+    return {
+      ...super.toJSON(),
+      revertData: this.revertData.map(fr => fr.toString()),
+    };
+  }
+
+  static override fromJSON(obj: ReturnType<AvmSimulationError['toJSON']>) {
+    return new AvmSimulationError(
+      obj.originalMessage,
+      obj.functionErrorStack,
+      obj.revertData.map(serializedFr => Fr.fromString(serializedFr)),
+      obj.noirErrorStack,
+    );
+  }
+}
+
 /**
  * Outputs of processing the public component of a transaction.
  */
 export class PublicSimulationOutput {
   constructor(
     public encryptedLogs: EncryptedTxL2Logs,
     public unencryptedLogs: UnencryptedTxL2Logs,
-    public revertReason: SimulationError | undefined,
+    public revertReason: AvmSimulationError | undefined,
     public constants: CombinedConstantData,
     public end: CombinedAccumulatedData,
     public publicReturnValues: NestedProcessReturnValues[],

yarn-project/end-to-end/scripts/e2e_test_config.yml

@@ -30,8 +30,7 @@ tests:
   e2e_card_game: {}
   e2e_cheat_codes: {}
   e2e_cross_chain_messaging: {}
-  # TODO reenable in https://github.com/AztecProtocol/aztec-packages/pull/9727
-  # e2e_crowdfunding_and_claim: {}
+  e2e_crowdfunding_and_claim: {}
   e2e_deploy_contract: {}
   e2e_devnet_smoke: {}
   docs_examples:
@@ -42,18 +41,16 @@ tests:
   # TODO(https://github.com/AztecProtocol/aztec-packages/issues/9488): reenable
   # e2e_fees_dapp_subscription:
   #   test_path: "e2e_fees/dapp_subscription.test.ts"
-  # TODO reenable in https://github.com/AztecProtocol/aztec-packages/pull/9727
-  # e2e_fees_failures:
-  #   test_path: 'e2e_fees/failures.test.ts'
+  e2e_fees_failures:
+    test_path: 'e2e_fees/failures.test.ts'
   e2e_fees_fee_juice_payments:
     test_path: 'e2e_fees/fee_juice_payments.test.ts'
   e2e_fees_gas_estimation:
     test_path: 'e2e_fees/gas_estimation.test.ts'
   e2e_fees_private_payments:
     test_path: 'e2e_fees/private_payments.test.ts'
-  # TODO reenable in https://github.com/AztecProtocol/aztec-packages/pull/9727
-  # e2e_fees_private_refunds:
-  #   test_path: 'e2e_fees/private_refunds.test.ts'
+  e2e_fees_private_refunds:
+    test_path: 'e2e_fees/private_refunds.test.ts'
   e2e_keys: {}
   e2e_l1_with_wall_time: {}
   e2e_lending_contract: {}

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

@@ -346,7 +346,7 @@ describe('e2e_crowdfunding_and_claim', () => {
     donorWallets[1].setScopes([donorWallets[1].getAddress(), crowdfundingContract.address]);
 
     await expect(donorWallets[1].simulateTx(request, true, operatorWallet.getAddress())).rejects.toThrow(
-      'Assertion failed: Users cannot set msg_sender in first call',
+      'Circuit execution failed: Users cannot set msg_sender in first call',
     );
   });
 

yarn-project/pxe/src/pxe_service/error_enriching.ts

@@ -1,4 +1,4 @@
-import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types';
+import { type AvmSimulationError, type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types';
 import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js';
 import { type DebugLogger } from '@aztec/foundation/log';
 import { resolveAssertionMessageFromRevertData, resolveOpcodeLocations } from '@aztec/simulator';
@@ -53,8 +53,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas
 }
 
 export async function enrichPublicSimulationError(
-  err: SimulationError,
-  revertData: Fr[],
+  err: AvmSimulationError,
   contractDataOracle: ContractDataOracle,
   db: PxeDatabase,
   logger: DebugLogger,
@@ -70,7 +69,7 @@ export async function enrichPublicSimulationError(
     originalFailingFunction.contractAddress,
     FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)),
   );
-  const assertionMessage = resolveAssertionMessageFromRevertData(revertData, artifact);
+  const assertionMessage = resolveAssertionMessageFromRevertData(err.revertData, artifact);
   if (assertionMessage) {
     err.setOriginalMessage(err.getOriginalMessage() + `${assertionMessage}`);
   }

yarn-project/pxe/src/pxe_service/pxe_service.ts

@@ -1,5 +1,6 @@
 import {
   type AuthWitness,
+  AvmSimulationError,
   type AztecNode,
   EventMetadata,
   EventType,
@@ -781,18 +782,20 @@ export class PXEService implements PXE {
    * @param tx - The transaction to be simulated.
    */
   async #simulatePublicCalls(tx: Tx) {
-    const result = await this.node.simulatePublicCalls(tx);
-    if (result.revertReason) {
-      await enrichPublicSimulationError(
-        result.revertReason,
-        result.publicReturnValues[0].values || [],
-        this.contractDataOracle,
-        this.db,
-        this.log,
-      );
-      throw result.revertReason;
+    // Simulating public calls can throw if the TX fails in a phase that doesn't allow reverts (setup)
+    // Or return as reverted if it fails in a phase that allows reverts (app logic, teardown)
+    try {
+      const result = await this.node.simulatePublicCalls(tx);
+      if (result.revertReason) {
+        throw result.revertReason;
+      }
+      return result;
+    } catch (err) {
+      if (err instanceof AvmSimulationError) {
+        await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log);
+      }
+      throw err;
     }
-    return result;
   }
 
   async #profileKernelProver(

yarn-project/simulator/src/mocks/fixtures.ts

@@ -1,7 +1,7 @@
 import {
+  type AvmSimulationError,
   type FunctionCall,
   PublicExecutionRequest,
-  type SimulationError,
   UnencryptedFunctionL2Logs,
 } from '@aztec/circuit-types';
 import {
@@ -27,7 +27,7 @@ export class PublicExecutionResultBuilder {
   private _contractStorageReads: ContractStorageRead[] = [];
   private _returnValues: Fr[] = [];
   private _reverted = false;
-  private _revertReason: SimulationError | undefined = undefined;
+  private _revertReason: AvmSimulationError | undefined = undefined;
 
   constructor(executionRequest: PublicExecutionRequest) {
     this._executionRequest = executionRequest;
@@ -46,7 +46,7 @@ export class PublicExecutionResultBuilder {
     nestedExecutions?: PublicExecutionResult[];
     contractStorageUpdateRequests?: ContractStorageUpdateRequest[];
     contractStorageReads?: ContractStorageRead[];
-    revertReason?: SimulationError;
+    revertReason?: AvmSimulationError;
   }): PublicExecutionResultBuilder {
     const builder = new PublicExecutionResultBuilder(request);
 
@@ -76,7 +76,7 @@ export class PublicExecutionResultBuilder {
     nestedExecutions?: PublicExecutionResult[];
     contractStorageUpdateRequests?: ContractStorageUpdateRequest[];
     contractStorageReads?: ContractStorageRead[];
-    revertReason?: SimulationError;
+    revertReason?: AvmSimulationError;
   }) {
     const builder = new PublicExecutionResultBuilder(
       new PublicExecutionRequest(new CallContext(from, tx.to, tx.selector, false), tx.args),
@@ -113,7 +113,7 @@ export class PublicExecutionResultBuilder {
     return this;
   }
 
-  withReverted(reason: SimulationError): PublicExecutionResultBuilder {
+  withReverted(reason: AvmSimulationError): PublicExecutionResultBuilder {
     this._reverted = true;
     this._revertReason = reason;
     return this;

yarn-project/simulator/src/public/enqueued_call_simulator.ts

@@ -1,11 +1,11 @@
 import {
   type AvmProvingRequest,
+  type AvmSimulationError,
   MerkleTreeId,
   NestedProcessReturnValues,
   ProvingRequestType,
   type PublicExecutionRequest,
   PublicKernelPhase,
-  type SimulationError,
   type Tx,
   UnencryptedFunctionL2Logs,
 } from '@aztec/circuit-types';
@@ -84,7 +84,7 @@ export type EnqueuedCallResult = {
   /** Gas used during the execution this enqueued call */
   gasUsed: Gas;
   /** Revert reason, if any */
-  revertReason?: SimulationError;
+  revertReason?: AvmSimulationError;
 };
 
 export class EnqueuedCallSimulator {

yarn-project/simulator/src/public/enqueued_calls_processor.ts

@@ -1,11 +1,11 @@
 import {
   type AvmProvingRequest,
+  type AvmSimulationError,
   type MerkleTreeReadOperations,
   type NestedProcessReturnValues,
   type ProcessedTx,
   type PublicExecutionRequest,
   PublicKernelPhase,
-  type SimulationError,
   type Tx,
 } from '@aztec/circuit-types';
 import {
@@ -54,13 +54,13 @@ type PublicPhaseResult = {
   /** Time spent for the execution this phase */
   durationMs: number;
   /** Revert reason, if any */
-  revertReason?: SimulationError;
+  revertReason?: AvmSimulationError;
 };
 
 export type ProcessedPhase = {
   phase: PublicKernelPhase;
   durationMs: number;
-  revertReason?: SimulationError;
+  revertReason?: AvmSimulationError;
 };
 
 export type TxPublicCallsResult = {
@@ -72,7 +72,7 @@ export type TxPublicCallsResult = {
   /** Gas used during the execution this tx */
   gasUsed: ProcessedTx['gasUsed'];
   /** Revert reason, if any */
-  revertReason?: SimulationError;
+  revertReason?: AvmSimulationError;
   processedPhases: ProcessedPhase[];
 };
 
@@ -160,7 +160,7 @@ export class EnqueuedCallsProcessor {
     let publicKernelOutput = tx.data.toPublicKernelCircuitPublicInputs();
     let isFromPrivate = true;
     let returnValues: NestedProcessReturnValues[] = [];
-    let revertReason: SimulationError | undefined;
+    let revertReason: AvmSimulationError | undefined;
     for (let i = 0; i < phases.length; i++) {
       const phase = phases[i];
       const callRequests = EnqueuedCallsProcessor.getCallRequestsByPhase(tx, phase);
@@ -233,7 +233,7 @@ export class EnqueuedCallsProcessor {
     let avmProvingRequest: AvmProvingRequest;
     let publicKernelOutput = previousPublicKernelOutput;
     let gasUsed = Gas.empty();
-    let revertReason: SimulationError | undefined;
+    let revertReason: AvmSimulationError | undefined;
     for (let i = callRequests.length - 1; i >= 0 && !revertReason; i--) {
       const callRequest = callRequests[i];
       const executionRequest = executionRequests[i];

yarn-project/simulator/src/public/execution.ts

@@ -1,7 +1,7 @@
 import {
+  type AvmSimulationError,
   NestedProcessReturnValues,
   type PublicExecutionRequest,
-  type SimulationError,
   type UnencryptedFunctionL2Logs,
 } from '@aztec/circuit-types';
 import {
@@ -49,7 +49,7 @@ export interface PublicExecutionResult {
   /** Whether the execution reverted. */
   reverted: boolean;
   /** The revert reason if the execution reverted. */
-  revertReason?: SimulationError;
+  revertReason?: AvmSimulationError;
 
   /** The contract storage reads performed by the function. */
   contractStorageReads: ContractStorageRead[];

yarn-project/simulator/src/public/public_processor.test.ts

@@ -1,4 +1,5 @@
 import {
+  AvmSimulationError,
   type MerkleTreeWriteOperations,
   type ProcessedTx,
   type ProcessedTxHandler,
@@ -329,7 +330,7 @@ describe('public_processor', () => {
             PublicExecutionResultBuilder.fromFunctionCall({
               from: revertibleRequests[0].callContext.contractAddress,
               tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)),
-              revertReason: new SimulationError('Simulation Failed', []),
+              revertReason: new AvmSimulationError('Simulation Failed', [], []),
             }).build(),
           ],
         }).build(),
@@ -434,7 +435,7 @@ describe('public_processor', () => {
             PublicExecutionResultBuilder.fromFunctionCall({
               from: nonRevertibleRequests[0].callContext.contractAddress,
               tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)),
-              revertReason: new SimulationError('Simulation Failed', []),
+              revertReason: new AvmSimulationError('Simulation Failed', [], []),
             }).build(),
           ],
         }).build(),
@@ -546,7 +547,7 @@ describe('public_processor', () => {
               from: teardownRequest.callContext.contractAddress,
               tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)),
               contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x202), 17)],
-              revertReason: new SimulationError('Simulation Failed', []),
+              revertReason: new AvmSimulationError('Simulation Failed', [], []),
             }).build(teardownResultSettings),
           ],
         }).build(teardownResultSettings),
@@ -644,7 +645,7 @@ describe('public_processor', () => {
             new ContractStorageUpdateRequest(contractSlotB, fr(0x152), 14),
             new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 15),
           ],
-          revertReason: new SimulationError('Simulation Failed', []),
+          revertReason: new AvmSimulationError('Simulation Failed', [], []),
         }).build(),
 
         // Teardown
@@ -660,7 +661,7 @@ describe('public_processor', () => {
               from: teardownRequest.callContext.contractAddress,
               tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)),
               contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x202), 16)],
-              revertReason: new SimulationError('Simulation Failed', []),
+              revertReason: new AvmSimulationError('Simulation Failed', [], []),
             }).build(teardownResultSettings),
           ],
         }).build(teardownResultSettings),