Skip to content

Commit

Permalink
feat: calls to non-existent contracts in the AVM return failure
Browse files Browse the repository at this point in the history
  • Loading branch information
dbanks12 committed Nov 20, 2024
1 parent 85c8a3b commit 7b0f3a2
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 11 deletions.
36 changes: 32 additions & 4 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,16 @@ describe('AVM simulator: transpiled Noir contracts', () => {

expect(results.reverted).toBe(false);
});

it('execution of a non-existent contract immediately reverts', async () => {
const context = initContext();
const results = await new AvmSimulator(context).execute();

expect(results.reverted).toBe(true);
expect(results.output).toEqual([]);
expect(results.gasLeft).toEqual({ l2Gas: 0, daGas: 0 });
});

it('addition', async () => {
const calldata: Fr[] = [new Fr(1), new Fr(2)];
const context = initContext({ env: initExecutionEnvironment({ calldata }) });
Expand Down Expand Up @@ -891,6 +901,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
environment: AvmExecutionEnvironment,
nestedTrace: PublicSideEffectTraceInterface,
isStaticCall: boolean = false,
exists: boolean = true,
) => {
expect(trace.traceNestedCall).toHaveBeenCalledTimes(1);
expect(trace.traceNestedCall).toHaveBeenCalledWith(
Expand All @@ -900,17 +911,34 @@ describe('AVM simulator: transpiled Noir contracts', () => {
contractCallDepth: new Fr(1), // top call is depth 0, nested is depth 1
globals: environment.globals, // just confirming that nested env looks roughly right
isStaticCall: isStaticCall,
// TODO(7121): can't check calldata like this since it is modified on environment construction
// with AvmContextInputs. These should eventually go away.
//calldata: expect.arrayContaining(environment.calldata), // top-level call forwards args
// top-level calls forward args in these tests,
// but nested calls in these tests use public_dispatch, so selector is inserted as first arg
calldata: expect.arrayContaining([/*selector=*/ expect.anything(), ...environment.calldata]),
}),
/*startGasLeft=*/ expect.anything(),
/*bytecode=*/ expect.anything(),
/*bytecode=*/ exists ? expect.anything() : undefined,
/*avmCallResults=*/ expect.anything(), // we don't have the NESTED call's results to check
/*functionName=*/ expect.anything(),
);
};

it(`Nested call to non-existent contract`, async () => {
const calldata = [value0, value1];
const context = createContext(calldata);
const callBytecode = getAvmTestContractBytecode('nested_call_to_add');
// We don't mock getBytecode for the nested contract, so it will not exist
// which should cause the nested call to immediately revert

const nestedTrace = mock<PublicSideEffectTraceInterface>();
mockTraceFork(trace, nestedTrace);

const results = await new AvmSimulator(context).executeBytecode(callBytecode);
expect(results.reverted).toBe(true);
expect(results.output).toEqual([]);

expectTracedNestedCall(context.environment, nestedTrace, /*isStaticCall=*/ false, /*exists=*/ false);
});

it(`Nested call`, async () => {
const calldata = [value0, value1];
const context = createContext(calldata);
Expand Down
22 changes: 18 additions & 4 deletions yarn-project/simulator/src/avm/avm_simulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import { AvmMachineState } from './avm_machine_state.js';
import { isAvmBytecode } from './bytecode_utils.js';
import {
AvmExecutionError,
AvmRevertReason,
InvalidProgramCounterError,
NoBytecodeForContractError,
revertReasonFromExceptionalHalt,
revertReasonFromExplicitRevert,
} from './errors.js';
Expand Down Expand Up @@ -83,10 +83,24 @@ export class AvmSimulator {
public async execute(): Promise<AvmContractCallResult> {
const bytecode = await this.context.persistableState.getBytecode(this.context.environment.address);

// This assumes that we will not be able to send messages to accounts without code
// Pending classes and instances impl details
if (!bytecode) {
throw new NoBytecodeForContractError(this.context.environment.address);
// revert, consuming all gas
const message = `No bytecode found at: ${this.context.environment.address}. Reverting...`;
const revertReason = new AvmRevertReason(
message,
/*failingFunction=*/ {
contractAddress: this.context.environment.address,
functionSelector: this.context.environment.functionSelector,
},
/*noirCallStack=*/ [],
);
this.log.warn(message);
return new AvmContractCallResult(
/*reverted=*/ true,
/*output=*/ [],
/*gasLeft=*/ { l2Gas: 0, daGas: 0 },
revertReason,
);
}

return await this.executeBytecode(bytecode);
Expand Down
35 changes: 35 additions & 0 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,41 @@ describe('External Calls', () => {
expect(inst.serialize()).toEqual(buf);
});

it('Call to non-existent bytecode returns failure', async () => {
const gasOffset = 0;
const l2Gas = 2e6;
const daGas = 3e6;
const addrOffset = 2;
const addr = new Fr(123456n);
const argsOffset = 3;
const args = [new Field(1), new Field(2), new Field(3)];
const argsSize = args.length;
const argsSizeOffset = 20;
const successOffset = 6;

const { l2GasLeft: initialL2Gas, daGasLeft: initialDaGas } = context.machineState;

context.machineState.memory.set(0, new Field(l2Gas));
context.machineState.memory.set(1, new Field(daGas));
context.machineState.memory.set(2, new Field(addr));
context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
context.machineState.memory.setSlice(3, args);

const instruction = new Call(/*indirect=*/ 0, gasOffset, addrOffset, argsOffset, argsSizeOffset, successOffset);
await instruction.execute(context);

const successValue = context.machineState.memory.get(successOffset);
expect(successValue).toEqual(new Uint1(0n)); // failure, contract non-existent!

const retValue = context.machineState.nestedReturndata;
expect(retValue).toEqual([]);

// should charge for the CALL instruction itself, and all allocated gas should be consumed
expect(context.machineState.l2GasLeft).toBeLessThan(initialL2Gas - l2Gas);
expect(context.machineState.daGasLeft).toEqual(initialDaGas - daGas);
expect(context.machineState.collectedRevertInfo?.recursiveRevertReason?.message).toMatch(/No bytecode found/);
});

it('Should execute a call correctly', async () => {
const gasOffset = 0;
const l2Gas = 2e6;
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/simulator/src/avm/opcodes/external_calls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Fr, FunctionSelector, Gas, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circ

import type { AvmContext } from '../avm_context.js';
import { type AvmContractCallResult } from '../avm_contract_call_result.js';
import { gasLeftToGas } from '../avm_gas.js';
import { type Field, TypeTag, Uint1 } from '../avm_memory_types.js';
import { AvmSimulator } from '../avm_simulator.js';
import { Opcode, OperandType } from '../serialization/instruction_serialization.js';
Expand Down Expand Up @@ -95,7 +94,7 @@ abstract class ExternalCall extends Instruction {
memory.set(successOffset, new Uint1(success ? 1 : 0));

// Refund unused gas
context.machineState.refundGas(gasLeftToGas(nestedContext.machineState));
context.machineState.refundGas(nestedCallResults.gasLeft);

// Merge nested call's state and trace based on whether it succeeded.
if (success) {
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/avm/opcodes/instruction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export abstract class Instruction {
* Computes gas cost for the instruction based on its base cost and memory operations.
* @returns Gas cost.
*/
protected gasCost(dynMultiplier: number = 0): Gas {
public gasCost(dynMultiplier: number = 0): Gas {
const baseGasCost = getBaseGasCost(this.opcode);
const dynGasCost = mulGas(getDynamicGasCost(this.opcode), dynMultiplier);
return sumGas(baseGasCost, dynGasCost);
Expand Down

0 comments on commit 7b0f3a2

Please sign in to comment.