Skip to content

Commit

Permalink
fix(avm-simulator): rethrow nested assertions (#6275)
Browse files Browse the repository at this point in the history
Transitional fix to conform to the ACVM behaviour.
  • Loading branch information
fcarreiro committed May 8, 2024
1 parent f6045fd commit cd05b91
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 22 deletions.
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

0 comments on commit cd05b91

Please sign in to comment.