Skip to content

Commit

Permalink
feat(avm-simulator): plumb noir assertion messages (#5774)
Browse files Browse the repository at this point in the history
Enable recovering Noir assertion messages in the avm simulator.

We had discussed propagating (re-throwing) nested call assertions to callers, to match the current Public behaviour, but I'm not doing it here so that I don't clash with David's work on nested calls. Moreover, the behaviour of reverting if a nested call reverts is currently happening (see `call_public` in AvmContext in noir). The only difference is really that the messsage propagated is different: in current public you get the assertion message from the nested call (I think but I should double check) and in the AVM simulator you'd get "Nested (static) call failed!". We can discuss if we want to change this.
  • Loading branch information
fcarreiro committed Apr 16, 2024
1 parent a3ac5ff commit 2cf11ac
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ contract AvmTest {
#[aztec(public-vm)]
fn test_get_contract_instance() {
let ci = get_contract_instance_avm(context.this_address());
assert(ci.is_some());
assert(ci.is_some(), "Contract instance not found!");
}

/************************************************************************
Expand Down Expand Up @@ -258,7 +258,9 @@ contract AvmTest {

#[aztec(public-vm)]
fn check_selector() {
assert(context.selector() == FunctionSelector::from_signature("check_selector()"));
assert(
context.selector() == FunctionSelector::from_signature("check_selector()"), "Unexpected selector!"
);
}

#[aztec(public-vm)]
Expand Down Expand Up @@ -299,7 +301,7 @@ contract AvmTest {

#[aztec(public-vm)]
fn assert_nullifier_exists(nullifier: Field) {
assert(context.nullifier_exists(nullifier, context.this_address()));
assert(context.nullifier_exists(nullifier, context.this_address()), "Nullifier doesn't exist!");
}

// Use the standard context interface to emit a new nullifier
Expand Down
13 changes: 12 additions & 1 deletion yarn-project/simulator/src/avm/avm_machine_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,17 @@ export class AvmMachineState {
if (!this.halted) {
throw new Error('Execution results are not ready! Execution is ongoing.');
}
return new AvmContractCallResults(this.reverted, this.output);
let revertReason = undefined;
if (this.reverted && this.output.length > 0) {
try {
// Try to interpret the output as a text string.
revertReason = new Error(
'Reverted with output: ' + String.fromCharCode(...this.output.map(fr => fr.toNumber())),
);
} catch (e) {
revertReason = new Error('Reverted with non-string output');
}
}
return new AvmContractCallResults(this.reverted, this.output, revertReason);
}
}
16 changes: 14 additions & 2 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,18 @@ describe('AVM simulator: transpiled Noir contracts', () => {
expect(results.output).toEqual([new Fr(4), new Fr(6)]);
});

it('Assertion message', async () => {
const calldata: Fr[] = [new Fr(20)];
const context = initContext({ env: initExecutionEnvironment({ calldata }) });

const bytecode = getAvmTestContractBytecode('assert_nullifier_exists');
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.output).toEqual([..."Nullifier doesn't exist!"].flatMap(c => new Fr(c.charCodeAt(0))));
});

describe.each([
['set_opcode_u8', 8n],
['set_opcode_u32', 1n << 30n],
Expand Down Expand Up @@ -454,6 +466,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const results = await new AvmSimulator(context).executeBytecode(bytecode);

expect(results.reverted).toBe(true);
expect(results.revertReason?.message).toMatch(/Attempted to emit duplicate nullifier/);
// Only the first nullifier should be in the trace, second one failed to add
expect(context.persistableState.flush().newNullifiers).toEqual([
expect.objectContaining({
Expand Down Expand Up @@ -899,8 +912,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const results = await new AvmSimulator(context).executeBytecode(callBytecode);

expect(results.reverted).toBe(true); // The outer call should revert.
// TODO(fcarreiro): revertReason lost in translation between results.
// expect(results.revertReason).toEqual(/StaticCallStorageAlterError/);
expect(results.revertReason?.message).toMatch(/Nested static call failed/);
});
});
});
Expand Down
9 changes: 4 additions & 5 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,19 +304,18 @@ describe('External Calls', () => {
});

it('Should return data and revert from the revert opcode', async () => {
const returnData = [new Fr(1n), new Fr(2n), new Fr(3n)];
const returnData = [...'assert message'].flatMap(c => new Field(c.charCodeAt(0)));

context.machineState.memory.set(0, new Field(1n));
context.machineState.memory.set(1, new Field(2n));
context.machineState.memory.set(2, new Field(3n));
context.machineState.memory.setSlice(0, returnData);

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,
output: returnData,
revertReason: new Error('Reverted with output: assert message'),
output: returnData.map(f => f.toFr()),
});
});
});
Expand Down

0 comments on commit 2cf11ac

Please sign in to comment.