feat(avm-simulator): cap gas for external calls (AztecProtocol#6479)

barretenberg/cpp/src/barretenberg/vm/avm_trace/aztec_constants.hpp

@@ -63,7 +63,6 @@ const size_t ARGS_HASH_CHUNK_COUNT = 64;
 const size_t MAX_ARGS_LENGTH = ARGS_HASH_CHUNK_COUNT * ARGS_HASH_CHUNK_LENGTH;
 const size_t INITIAL_L2_BLOCK_NUM = 1;
 const size_t BLOB_SIZE_IN_BYTES = 31 * 4096;
-const size_t NESTED_CALL_L2_GAS_BUFFER = 20000;
 const size_t MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS = 20000;
 const size_t MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS = 3000;
 const size_t MAX_PACKED_BYTECODE_SIZE_PER_UNCONSTRAINED_FUNCTION_IN_FIELDS = 3000;

docs/docs/protocol-specs/public-vm/nested-calls.mdx

@@ -3,33 +3,41 @@
 A **nested contract call** occurs _during_ AVM execution and is triggered by a **contract call instruction**. The AVM [instruction set](./instruction-set) includes three contract call instructions: [`CALL`](./instruction-set#isa-section-call), [`STATICCALL`](./instruction-set#isa-section-staticcall), and [`DELEGATECALL`](./instruction-set#isa-section-delegatecall).
 
 A nested contract call performs the following operations:
+
 1. [Charge gas](#gas-cost-of-call-instruction) for the nested call
 1. [Trace the nested contract call](#tracing-nested-contract-calls)
 1. [Derive the **nested context**](#context-initialization-for-nested-calls) from the calling context and the call instruction
 1. Initiate [AVM execution](./execution) within the nested context until a halt is reached
 1. [Update the **calling context**](#updating-the-calling-context-after-nested-call-halts) after the nested call halts
 
 Or, in pseudocode:
+
 ```jsx
 // instr.args are { gasOffset, addrOffset, argsOffset, retOffset, retSize }
 
-isStaticCall = instr.opcode == STATICCALL
-isDelegateCall = instr.opcode == DELEGATECALL
-
-chargeGas(context,
-          l2GasCost=M[instr.args.gasOffset],
-          daGasCost=M[instr.args.gasOffset+1])
-traceNestedCall(context, instr.args.addrOffset)
-nestedContext = deriveContext(context, instr.args, isStaticCall, isDelegateCall)
-execute(nestedContext)
-updateContextAfterNestedCall(context, instr.args, nestedContext)
+isStaticCall = instr.opcode == STATICCALL;
+isDelegateCall = instr.opcode == DELEGATECALL;
+l2GasCost = min(M[instr.args.gasOffset], context.machineState.l2GasLeft);
+daGasCost = min(M[instr.args.gasOffset + 1], context.machineState.daGasLeft);
+
+chargeGas(context, l2GasCost, daGasCost);
+traceNestedCall(context, instr.args.addrOffset);
+nestedContext = deriveContext(
+  context,
+  instr.args,
+  isStaticCall,
+  isDelegateCall
+);
+execute(nestedContext);
+updateContextAfterNestedCall(context, instr.args, nestedContext);
 ```
 
 These call instructions share the same argument definitions: `gasOffset`, `addrOffset`, `argsOffset`, `argsSize`, `retOffset`, `retSize`, and `successOffset` (defined in the [instruction set](./instruction-set)). These arguments will be referred to via those keywords below, and will often be used in conjunction with the `M[offset]` syntax which is shorthand for `context.machineState.memory[offset]`.
 
 ## Tracing nested contract calls
 
 Before nested execution begins, the contract call is traced.
+
 ```jsx
 traceNestedCall(context, addrOffset)
 // which is shorthand for
@@ -50,21 +58,20 @@ import NestedContext from "./_nested-context.md";
 
 <NestedContext />
 
-
 ## Gas cost of call instruction
 
 A call instruction's gas cost is derived from its `gasOffset` argument. In other words, the caller "allocates" gas for a nested call via its `gasOffset` argument.
 
 As with all instructions, gas is checked and cost is deducted _prior_ to the instruction's execution.
+
 ```jsx
-chargeGas(context,
-          l2GasCost=M[gasOffset],
-          daGasCost=M[gasOffset+1])
+chargeGas(context, l2GasCost, daGasCost);
 ```
 
 > The shorthand `chargeGas` is defined in ["Gas checks and tracking"](./execution#gas-checks-and-tracking).
 
 As with all instructions, gas is checked and cost is deducted _prior_ to the instruction's execution.
+
 ```jsx
 assert context.machineState.l2GasLeft - l2GasCost >= 0
 assert context.machineState.daGasLeft - daGasCost >= 0
@@ -77,8 +84,9 @@ When the nested call halts, it may not have used up its entire gas allocation. A
 ## Nested execution
 
 Once the nested call's context is initialized, execution within that context begins.
+
 ```jsx
-execute(nestedContext)
+execute(nestedContext);
 ```
 
 Execution (and the `execution` shorthand above) is detailed in ["Execution, Gas, Halting"](./execution). Note that execution mutates the nested context.
@@ -88,27 +96,32 @@ Execution (and the `execution` shorthand above) is detailed in ["Execution, Gas,
 After the nested call halts, the calling context is updated. The call's success is extracted, unused gas is refunded, output data can be copied to the caller's memory, world state and accrued substate are conditionally accepted, and the world state trace is updated. The following shorthand is used to refer to this process in the ["Instruction Set"](./instruction-set):
 
 ```jsx
-updateContextAfterNestedCall(context, instr.args, nestedContext)
+updateContextAfterNestedCall(context, instr.args, nestedContext);
 ```
 
 The caller checks whether the nested call succeeded, and places the answer in memory.
+
 ```jsx
-context.machineState.memory[instr.args.successOffset] = !nestedContext.results.reverted
+context.machineState.memory[instr.args.successOffset] =
+  !nestedContext.results.reverted;
 ```
 
 Any unused gas is refunded to the caller.
+
 ```jsx
-context.l2GasLeft += nestedContext.machineState.l2GasLeft
-context.daGasLeft += nestedContext.machineState.daGasLeft
+context.l2GasLeft += nestedContext.machineState.l2GasLeft;
+context.daGasLeft += nestedContext.machineState.daGasLeft;
 ```
 
 If the call instruction specifies non-zero `retSize`, the caller copies any returned output data to its memory.
+
 ```jsx
 if retSize > 0:
     context.machineState.memory[retOffset:retOffset+retSize] = nestedContext.results.output
 ```
 
 If the nested call succeeded, the caller accepts its world state and accrued substate modifications.
+
 ```jsx
 if !nestedContext.results.reverted:
     context.worldState = nestedContext.worldState
@@ -118,6 +131,7 @@ if !nestedContext.results.reverted:
 ### Accepting nested call's World State access trace
 
 If the nested call reverted, the caller initializes the "end-lifetime" of all world state accesses made within the nested call.
+
 ```jsx
 if nestedContext.results.reverted:
     // process all traces (this is shorthand)
@@ -132,6 +146,7 @@ if nestedContext.results.reverted:
 > A world state access that was made in a deeper nested _reverted_ context will already have its end-lifetime initialized. The caller does _not_ overwrite this access' end-lifetime here as it already has a narrower lifetime.
 
 Regardless of whether the nested call reverted, the caller accepts its updated world state access trace (with updated end-lifetimes).
+
 ```jsx
-context.worldStateAccessTrace = nestedContext.worldStateAccessTrace
+context.worldStateAccessTrace = nestedContext.worldStateAccessTrace;
 ```

l1-contracts/src/core/libraries/ConstantsGen.sol

@@ -76,7 +76,6 @@ library Constants {
   uint256 internal constant INITIALIZATION_SLOT_SEPARATOR = 1000_000_000;
   uint256 internal constant INITIAL_L2_BLOCK_NUM = 1;
   uint256 internal constant BLOB_SIZE_IN_BYTES = 31 * 4096;
-  uint256 internal constant NESTED_CALL_L2_GAS_BUFFER = 20000;
   uint256 internal constant MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS = 20000;
   uint256 internal constant MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS = 3000;
   uint256 internal constant MAX_PACKED_BYTECODE_SIZE_PER_UNCONSTRAINED_FUNCTION_IN_FIELDS = 3000;

noir-projects/aztec-nr/aztec/src/context/avm_context.nr

@@ -1,8 +1,5 @@
 use crate::hash::{compute_secret_hash, compute_message_hash, compute_message_nullifier};
-use dep::protocol_types::{
-    address::{AztecAddress, EthAddress},
-    constants::{L1_TO_L2_MESSAGE_LENGTH, NESTED_CALL_L2_GAS_BUFFER}, header::Header
-};
+use dep::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH, header::Header};
 use dep::protocol_types::traits::{Deserialize, Serialize, Empty};
 use dep::protocol_types::abis::function_selector::FunctionSelector;
 use dep::protocol_types::abis::public_circuit_public_inputs::PublicCircuitPublicInputs;
@@ -200,9 +197,12 @@ impl Empty for AvmContext {
 
 // Helper functions
 fn gas_for_call(user_gas: GasOpts) -> [Field; 2] {
+    // It's ok to use the max possible gas here, because the gas will be
+    // capped by the gas left in the (STATIC)CALL instruction.
+    let MAX_POSSIBLE_FIELD: Field = 0 - 1;
     [
-        user_gas.l2_gas.unwrap_or_else(|| l2_gas_left() - NESTED_CALL_L2_GAS_BUFFER),
-        user_gas.da_gas.unwrap_or_else(|| da_gas_left())
+        user_gas.l2_gas.unwrap_or(MAX_POSSIBLE_FIELD),
+        user_gas.da_gas.unwrap_or(MAX_POSSIBLE_FIELD)
     ]
 }
 

noir-projects/noir-protocol-circuits/crates/types/src/constants.nr

@@ -102,8 +102,6 @@ global MAX_ARGS_LENGTH: u64 = ARGS_HASH_CHUNK_COUNT * ARGS_HASH_CHUNK_LENGTH;
 global INITIALIZATION_SLOT_SEPARATOR: Field = 1000_000_000;
 global INITIAL_L2_BLOCK_NUM: Field = 1;
 global BLOB_SIZE_IN_BYTES: Field = 31 * 4096;
-// How much gas is subtracted from L2GASLEFT when making a nested public call by default in the AVM
-global NESTED_CALL_L2_GAS_BUFFER = 20000;
 
 // CONTRACT CLASS CONSTANTS
 global MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS: u64 = 20000;

yarn-project/circuits.js/src/constants.gen.ts

@@ -62,7 +62,6 @@ export const MAX_ARGS_LENGTH = ARGS_HASH_CHUNK_COUNT * ARGS_HASH_CHUNK_LENGTH;
 export const INITIALIZATION_SLOT_SEPARATOR = 1000_000_000;
 export const INITIAL_L2_BLOCK_NUM = 1;
 export const BLOB_SIZE_IN_BYTES = 31 * 4096;
-export const NESTED_CALL_L2_GAS_BUFFER = 20000;
 export const MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS = 20000;
 export const MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS = 3000;
 export const MAX_PACKED_BYTECODE_SIZE_PER_UNCONSTRAINED_FUNCTION_IN_FIELDS = 3000;

yarn-project/simulator/src/avm/fixtures/index.ts

@@ -90,8 +90,8 @@ export function initGlobalVariables(overrides?: Partial<GlobalVariables>): Globa
  */
 export function initMachineState(overrides?: Partial<AvmMachineState>): AvmMachineState {
   return AvmMachineState.fromState({
-    l2GasLeft: overrides?.l2GasLeft ?? 100e6,
-    daGasLeft: overrides?.daGasLeft ?? 100e6,
+    l2GasLeft: overrides?.l2GasLeft ?? 1e8,
+    daGasLeft: overrides?.daGasLeft ?? 1e8,
   });
 }
 

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

@@ -11,6 +11,7 @@ import { adjustCalldataIndex, initContext } from '../fixtures/index.js';
 import { HostStorage } from '../journal/host_storage.js';
 import { AvmPersistableStateManager } from '../journal/journal.js';
 import { encodeToBytecode } from '../serialization/bytecode_serialization.js';
+import { L2GasLeft } from './context_getters.js';
 import { Call, Return, Revert, StaticCall } from './external_calls.js';
 import { type Instruction } from './instruction.js';
 import { CalldataCopy } from './memory.js';
@@ -40,7 +41,7 @@ describe('External Calls', () => {
         ...Buffer.from('d2345678', 'hex'), // retOffset
         ...Buffer.from('e2345678', 'hex'), // retSize
         ...Buffer.from('f2345678', 'hex'), // successOffset
-        ...Buffer.from('f3345678', 'hex'), // temporaryFunctionSelectorOffset
+        ...Buffer.from('f3345678', 'hex'), // functionSelectorOffset
       ]);
       const inst = new Call(
         /*indirect=*/ 0x01,
@@ -51,7 +52,7 @@ describe('External Calls', () => {
         /*retOffset=*/ 0xd2345678,
         /*retSize=*/ 0xe2345678,
         /*successOffset=*/ 0xf2345678,
-        /*temporaryFunctionSelectorOffset=*/ 0xf3345678,
+        /*functionSelectorOffset=*/ 0xf3345678,
       );
 
       expect(Call.deserialize(buf)).toEqual(inst);
@@ -86,8 +87,7 @@ describe('External Calls', () => {
         ]),
       );
 
-      // const {  l2GasLeft: initialL2Gas, daGasLeft: initialDaGas } = context.machineState;
-      const { daGasLeft: initialDaGas } = context.machineState;
+      const { l2GasLeft: initialL2Gas, daGasLeft: initialDaGas } = context.machineState;
 
       context.machineState.memory.set(0, new Field(l2Gas));
       context.machineState.memory.set(1, new Field(daGas));
@@ -107,7 +107,7 @@ describe('External Calls', () => {
         retOffset,
         retSize,
         successOffset,
-        /*temporaryFunctionSelectorOffset=*/ 0,
+        /*functionSelectorOffset=*/ 0,
       );
       await instruction.execute(context);
 
@@ -128,48 +128,60 @@ describe('External Calls', () => {
       const expectedStoredValue = new Fr(1n);
       expect(nestedContractWrites!.get(slotNumber)).toEqual(expectedStoredValue);
 
-      // TODO(https://github.com/AztecProtocol/aztec-packages/issues/5625): gas not plumbed through correctly in nested calls.
-      // expect(context.machineState.l2GasLeft).toEqual(initialL2Gas - otherContextInstructionsL2GasCost);
+      expect(context.machineState.l2GasLeft).toBeLessThan(initialL2Gas);
       expect(context.machineState.daGasLeft).toEqual(initialDaGas);
     });
 
-    it('Should refuse to execute a call if not enough gas', async () => {
+    it('Should cap to available gas if allocated is bigger', async () => {
       const gasOffset = 0;
       const l2Gas = 1e9;
-      const daGas = 3e6;
+      const daGas = 1e9;
       const addrOffset = 2;
       const addr = new Fr(123456n);
-      const argsOffset = 3;
-      const args = [new Field(1n), new Field(2n), new Field(3n)];
-      const argsSize = args.length;
+      const argsSize = 0;
       const argsSizeOffset = 20;
       const retOffset = 7;
-      const retSize = 2;
+      const retSize = 1;
       const successOffset = 6;
 
+      const otherContextInstructionsBytecode = markBytecodeAsAvm(
+        encodeToBytecode([
+          new L2GasLeft(/*indirect=*/ 0, /*dstOffset=*/ 0),
+          new Return(/*indirect=*/ 0, /*retOffset=*/ 0, /*size=*/ 1),
+        ]),
+      );
+
+      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);
-
       jest
         .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
-        .mockRejectedValue(new Error('No bytecode expected to be requested since not enough gas'));
+        .mockReturnValue(Promise.resolve(otherContextInstructionsBytecode));
 
       const instruction = new Call(
         /*indirect=*/ 0,
         gasOffset,
         addrOffset,
-        argsOffset,
+        /*argsOffset=*/ 0,
         argsSizeOffset,
         retOffset,
         retSize,
         successOffset,
-        /*temporaryFunctionSelectorOffset=*/ 0,
+        /*functionSelectorOffset=*/ 0,
       );
+      await instruction.execute(context);
 
-      await expect(() => instruction.execute(context)).rejects.toThrow(/Not enough.*gas left/i);
+      const successValue = context.machineState.memory.get(successOffset);
+      expect(successValue).toEqual(new Uint8(1n));
+
+      const retValue = context.machineState.memory.get(retOffset).toBigInt();
+      expect(retValue).toBeLessThan(initialL2Gas);
+
+      expect(context.machineState.l2GasLeft).toBeLessThan(initialL2Gas);
+      expect(context.machineState.daGasLeft).toEqual(initialDaGas);
     });
   });
 
@@ -185,7 +197,7 @@ describe('External Calls', () => {
         ...Buffer.from('d2345678', 'hex'), // retOffset
         ...Buffer.from('e2345678', 'hex'), // retSize
         ...Buffer.from('f2345678', 'hex'), // successOffset
-        ...Buffer.from('f3345678', 'hex'), // temporaryFunctionSelectorOffset
+        ...Buffer.from('f3345678', 'hex'), // functionSelectorOffset
       ]);
       const inst = new StaticCall(
         /*indirect=*/ 0x01,
@@ -196,7 +208,7 @@ describe('External Calls', () => {
         /*retOffset=*/ 0xd2345678,
         /*retSize=*/ 0xe2345678,
         /*successOffset=*/ 0xf2345678,
-        /*temporaryFunctionSelectorOffset=*/ 0xf3345678,
+        /*functionSelectorOffset=*/ 0xf3345678,
       );
 
       expect(StaticCall.deserialize(buf)).toEqual(inst);
@@ -241,7 +253,7 @@ describe('External Calls', () => {
         retOffset,
         retSize,
         successOffset,
-        /*temporaryFunctionSelectorOffset=*/ 0,
+        /*functionSelectorOffset=*/ 0,
       );
       await expect(() => instruction.execute(context)).rejects.toThrow(
         'Static call cannot update the state, emit L2->L1 messages or generate logs',