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

feat(avm-simulator): plumb noir assertion messages #5774

Merged
merged 1 commit into from
Apr 16, 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
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 @@ -128,6 +128,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
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
Loading