Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(avm-simulator): correctly create call stack in shallow assertions #6274

Merged
merged 1 commit into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading