feat: calls to non-existent contracts in the AVM simulator return failure

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -30,6 +30,7 @@ contract AvmTest {
 
     // Libs
     use dep::aztec::context::gas::GasOpts;
+    use dep::aztec::context::public_context::call;
     use dep::aztec::macros::{functions::{private, public}, storage::storage};
     use dep::aztec::oracle::get_contract_instance::{
         get_contract_instance_class_id_avm, get_contract_instance_deployer_avm,
@@ -515,6 +516,23 @@ contract AvmTest {
     /************************************************************************
      * Nested calls
      ************************************************************************/
+    #[public]
+    fn nested_call_to_nothing() {
+        let garbageAddress = AztecAddress::from_field(42);
+        AvmTest::at(garbageAddress).nested_call_to_nothing().call(&mut context)
+    }
+
+    #[public]
+    fn nested_call_to_nothing_recovers() {
+        let garbageAddress = AztecAddress::from_field(42);
+        let gas = [1, 1];
+        let success = call(gas, garbageAddress, &[]);
+        assert(
+            !success,
+            "Nested CALL instruction should return failure if target contract does not exist",
+        );
+    }
+
     #[public]
     fn nested_call_to_add_with_gas(
         arg_a: Field,
@@ -644,5 +662,6 @@ contract AvmTest {
         //let _ = nested_call_to_add(1, 2);
         //dep::aztec::oracle::debug_log::debug_log("nested_static_call_to_add");
         //let _ = nested_static_call_to_add(1, 2);
+        //let _ = nested_call_to_nothing_recovers();
     }
 }

yarn-project/end-to-end/src/e2e_avm_simulator.test.ts

@@ -155,6 +155,20 @@ describe('e2e_avm_simulator', () => {
     });
 
     describe('Nested calls', () => {
+      it('Top-level call to non-existent contract reverts', async () => {
+        // The nested call reverts (returns failure), but the caller doesn't HAVE to rethrow.
+        const tx = await avmContract.methods.nested_call_to_nothing_recovers().send().wait();
+        expect(tx.status).toEqual(TxStatus.SUCCESS);
+      });
+      it('Nested call to non-existent contract reverts & rethrows by default', async () => {
+        // The nested call reverts and by default caller rethrows
+        await expect(avmContract.methods.nested_call_to_nothing().send().wait()).rejects.toThrow(/No bytecode/);
+      });
+      it('Nested CALL instruction to non-existent contract returns failure, but caller can recover', async () => {
+        // The nested call reverts (returns failure), but the caller doesn't HAVE to rethrow.
+        const tx = await avmContract.methods.nested_call_to_nothing_recovers().send().wait();
+        expect(tx.status).toEqual(TxStatus.SUCCESS);
+      });
       it('Should NOT be able to emit the same unsiloed nullifier from the same contract', async () => {
         const nullifier = new Fr(1);
         await expect(

yarn-project/end-to-end/src/e2e_deploy_contract/contract_class_registration.test.ts

@@ -257,13 +257,21 @@ describe('e2e_deploy_contract contract class registration', () => {
   });
 
   describe('error scenarios in deployment', () => {
-    it('refuses to call a public function on an undeployed contract', async () => {
+    it('app logic call to an undeployed contract reverts, but can be included is not dropped', async () => {
       const whom = wallet.getAddress();
       const outgoingViewer = whom;
       const instance = await t.registerContract(wallet, StatefulTestContract, { initArgs: [whom, outgoingViewer, 42] });
-      await expect(
-        instance.methods.increment_public_value_no_init_check(whom, 10).send({ skipPublicSimulation: true }).wait(),
-      ).rejects.toThrow(/dropped/);
+      // Confirm that the tx reverts with the expected message
+      await expect(instance.methods.increment_public_value_no_init_check(whom, 10).send().wait()).rejects.toThrow(
+        /No bytecode/,
+      );
+      // This time, don't throw on revert and confirm that the tx is included
+      // despite reverting in app logic because of the call to a non-existent contract
+      const tx = await instance.methods
+        .increment_public_value_no_init_check(whom, 10)
+        .send({ skipPublicSimulation: true })
+        .wait({ dontThrowOnRevert: true });
+      expect(tx.status).toEqual(TxStatus.APP_LOGIC_REVERTED);
     });
 
     it('refuses to deploy an instance from a different deployer', () => {

yarn-project/pxe/src/pxe_service/pxe_service.ts

@@ -773,7 +773,11 @@ export class PXEService implements PXE {
       return result;
     } catch (err) {
       if (err instanceof SimulationError) {
-        await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log);
+        try {
+          await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log);
+        } catch (enrichErr) {
+          this.log.error(`Failed to enrich public simulation error: ${enrichErr}`);
+        }
       }
       throw err;
     }

yarn-project/simulator/src/avm/avm_simulator.test.ts

@@ -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 }) });
@@ -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(
@@ -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);

yarn-project/simulator/src/avm/avm_simulator.ts

@@ -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';
@@ -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);

yarn-project/simulator/src/avm/opcodes/external_calls.test.ts

@@ -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;

yarn-project/simulator/src/avm/opcodes/external_calls.ts

@@ -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';
@@ -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) {

yarn-project/simulator/src/avm/opcodes/instruction.ts

@@ -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);