Skip to content

Commit

Permalink
fix(avm-simulator): correctly create call stack in shallow assertions (
Browse files Browse the repository at this point in the history
…#6274)

This makes the PXE correctly interpret assertions from the AVM
simulator. Work is still needed for nested assertions.

I also change the revert message to conform to the ACVM one.
  • Loading branch information
fcarreiro authored May 8, 2024
1 parent 92c1478 commit f6045fd
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 9 deletions.
8 changes: 8 additions & 0 deletions yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ describe('e2e_avm_simulator', () => {
avmContract = await AvmTestContract.deploy(wallet).send().deployed();
});

describe('Assertions', () => {
it('Processes assertions in the PXE', async () => {
await expect(avmContract.methods.assert_nullifier_exists(123).simulate()).rejects.toThrow(
"Assertion failed: Nullifier doesn't exist!",
);
});
});

describe('Gas metering', () => {
it('Tracks L2 gas usage on simulation', async () => {
const request = await avmContract.methods.add_args_return(20n, 30n).create();
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/simulator/src/avm/avm_machine_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,12 @@ export class AvmMachineState {
let revertReason = undefined;
if (this.reverted && this.output.length > 0) {
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(
'Reverted with output: ' + String.fromCharCode(...this.output.slice(1).map(fr => fr.toNumber())),
);
revertReason = new Error('Assertion failed: ' + String.fromCharCode(...revertOutput.map(fr => fr.toNumber())));
} catch (e) {
revertReason = new Error('Reverted with non-string output');
revertReason = new Error('<no output>');
}
}
return new AvmContractCallResults(this.reverted, this.output, revertReason);
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const results = await new AvmSimulator(context).executeBytecode(bytecode);

expect(results.reverted).toBe(true);
expect(results.revertReason?.message).toEqual("Reverted with output: Nullifier doesn't exist!");
expect(results.revertReason?.message).toEqual("Assertion failed: Nullifier doesn't exist!");
expect(results.output).toEqual([
new Fr(0),
...[..."Nullifier doesn't exist!"].flatMap(c => new Fr(c.charCodeAt(0))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ describe('External Calls', () => {
expect(context.machineState.halted).toBe(true);
expect(context.machineState.getResults()).toEqual({
reverted: true,
revertReason: new Error('Reverted with output: assert message'),
revertReason: new Error('Assertion failed: assert message'),
output: returnData.map(f => f.toFr()),
});
});
Expand Down
37 changes: 34 additions & 3 deletions yarn-project/simulator/src/public/transitional_adaptors.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
// All code in this file needs to die once the public executor is phased out in favor of the AVM.
import { UnencryptedFunctionL2Logs } from '@aztec/circuit-types';
import { type SimulationError, UnencryptedFunctionL2Logs } from '@aztec/circuit-types';
import {
type AztecAddress,
CallContext,
FunctionData,
type FunctionSelector,
type Gas,
type GasSettings,
type GlobalVariables,
type Header,
} from '@aztec/circuits.js';
import { Fr } from '@aztec/foundation/fields';

import { extractCallStack } from '../acvm/index.js';
import { type AvmContext } from '../avm/avm_context.js';
import { AvmExecutionEnvironment } from '../avm/avm_execution_environment.js';
import { type AvmContractCallResults } from '../avm/avm_message_call_result.js';
import { Mov } from '../avm/opcodes/memory.js';
import { createSimulationError } from '../common/errors.js';
import { ExecutionError, createSimulationError } from '../common/errors.js';
import { type PublicExecution, type PublicExecutionResult } from './execution.js';

/**
Expand Down Expand Up @@ -72,6 +75,29 @@ export function createPublicExecution(
return execution;
}

export function processRevertReason(
revertReason: Error | undefined,
contractAddress: AztecAddress,
functionSelector: FunctionSelector,
): SimulationError | undefined {
if (!revertReason) {
return undefined;
}
if (revertReason instanceof Error) {
const ee = new ExecutionError(
revertReason.message,
{
contractAddress,
functionSelector,
},
extractCallStack(revertReason),
{ cause: revertReason },
);

return createSimulationError(ee);
}
}

export function convertAvmResultsToPxResult(
avmResult: AvmContractCallResults,
startSideEffectCounter: number,
Expand All @@ -81,6 +107,7 @@ export function convertAvmResultsToPxResult(
): PublicExecutionResult {
const endPersistableState = endAvmContext.persistableState;
const endMachineState = endAvmContext.machineState;

return {
...endPersistableState.transitionalExecutionResult, // includes nestedExecutions
execution: fromPx,
Expand All @@ -92,7 +119,11 @@ export function convertAvmResultsToPxResult(
endPersistableState.transitionalExecutionResult.allUnencryptedLogs,
),
reverted: avmResult.reverted,
revertReason: avmResult.revertReason ? createSimulationError(avmResult.revertReason) : undefined,
revertReason: processRevertReason(
avmResult.revertReason,
endAvmContext.environment.address,
fromPx.functionData.selector,
),
startGasLeft: startGas,
endGasLeft: endMachineState.gasLeft,
transactionFee: endAvmContext.environment.transactionFee,
Expand Down

0 comments on commit f6045fd

Please sign in to comment.