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): rethrow nested assertions #6275

Merged
merged 2 commits 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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ contract AvmNestedCallsTest {
arg_a + arg_b
}

#[aztec(public-vm)]
fn assert_same(arg_a: Field, arg_b: Field) -> pub Field {
assert(arg_a == arg_b, "Values are not equal");
1
}

// Use the standard context interface to emit a new nullifier
#[aztec(public-vm)]
fn new_nullifier(nullifier: Field) {
Expand Down
18 changes: 17 additions & 1 deletion yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,23 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const results = await new AvmSimulator(context).executeBytecode(callBytecode);

expect(results.reverted).toBe(true); // The outer call should revert.
expect(results.revertReason?.message).toMatch(/Nested static call failed/);
expect(results.revertReason?.message).toEqual('Static calls cannot alter storage');
});

it(`Nested calls rethrow exceptions`, async () => {
const calldata: Fr[] = [new Fr(1), new Fr(2)];
const callBytecode = getAvmNestedCallsTestContractBytecode('nested_call_to_add');
// We actually don't pass the function ADD, but it's ok because the signature is the same.
const nestedBytecode = getAvmNestedCallsTestContractBytecode('assert_same');
const context = initContext({ env: initExecutionEnvironment({ calldata }) });
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
.mockReturnValue(Promise.resolve(nestedBytecode));

const results = await new AvmSimulator(context).executeBytecode(callBytecode);

expect(results.reverted).toBe(true); // The outer call should revert.
expect(results.revertReason?.message).toEqual('Assertion failed: Values are not equal');
});
});
});
Expand Down
32 changes: 11 additions & 21 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,31 +205,25 @@ describe('External Calls', () => {

it('Should fail if a static call attempts to touch storage', async () => {
const gasOffset = 0;
const gas = new Field(0);
const addrOffset = 1;
const gas = [new Field(0n), new Field(0n), new Field(0n)];
const addrOffset = 10;
const addr = new Field(123456n);
const argsOffset = 2;
const argsOffset = 20;
const args = [new Field(1n), new Field(2n), new Field(3n)];

const argsSize = args.length;
const argsSizeOffset = 20;
const retOffset = 8;
const argsSizeOffset = 40;
const retOffset = 80;
const retSize = 2;
const successOffset = 7;
const successOffset = 70;

context.machineState.memory.set(0, gas);
context.machineState.memory.set(1, addr);
context.machineState.memory.setSlice(gasOffset, gas);
context.machineState.memory.set(addrOffset, addr);
context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
context.machineState.memory.setSlice(2, args);
context.machineState.memory.setSlice(argsOffset, args);

const otherContextInstructions: Instruction[] = [
new CalldataCopy(
/*indirect=*/ 0,
/*csOffset=*/ adjustCalldataIndex(0),
/*copySize=*/ argsSize,
/*dstOffset=*/ 0,
),
new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*size=*/ 1, /*slotOffset=*/ 0),
new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*size=*/ 0, /*slotOffset=*/ 0),
];

const otherContextInstructionsBytecode = markBytecodeAsAvm(encodeToBytecode(otherContextInstructions));
Expand All @@ -249,11 +243,7 @@ describe('External Calls', () => {
successOffset,
/*temporaryFunctionSelectorOffset=*/ 0,
);
await instruction.execute(context);

// No revert has occurred, but the nested execution has failed
const successValue = context.machineState.memory.get(successOffset);
expect(successValue).toEqual(new Uint8(0n));
await expect(() => instruction.execute(context)).rejects.toThrow(/Static calls cannot alter storage/);
});
});

Expand Down
13 changes: 13 additions & 0 deletions yarn-project/simulator/src/avm/opcodes/external_calls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { gasLeftToGas, sumGas } from '../avm_gas.js';
import { Field, Uint8 } from '../avm_memory_types.js';
import { type AvmContractCallResults } from '../avm_message_call_result.js';
import { AvmSimulator } from '../avm_simulator.js';
import { AvmExecutionError } from '../errors.js';
import { Opcode, OperandType } from '../serialization/instruction_serialization.js';
import { Addressing } from './addressing_mode.js';
import { Instruction } from './instruction.js';
Expand Down Expand Up @@ -99,6 +100,18 @@ abstract class ExternalCall extends Instruction {

const success = !nestedCallResults.reverted;

// TRANSITIONAL: We rethrow here so that the MESSAGE gets propagated.
if (!success) {
class RethrownError extends AvmExecutionError {
constructor(message: string) {
super(message);
this.name = 'RethrownError';
}
}

throw new RethrownError(nestedCallResults.revertReason?.message || 'Unknown nested call error');
}

// We only take as much data as was specified in the return size and pad with zeroes if the return data is smaller
// than the specified size in order to prevent that memory to be left with garbage
const returnData = nestedCallResults.output.slice(0, this.retSize);
Expand Down
Loading