feat(avm): take sizeOffset in CALL (#5763)

CC @Thunkar.

avm-transpiler/src/transpile.rs

@@ -2,7 +2,7 @@ use acvm::acir::brillig::Opcode as BrilligOpcode;
 use acvm::acir::circuit::brillig::Brillig;
 
 use acvm::brillig_vm::brillig::{
-    BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, MemoryAddress, ValueOrArray,
+    BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, HeapVector, MemoryAddress, ValueOrArray,
 };
 use acvm::FieldElement;
 
@@ -380,16 +380,16 @@ fn handle_external_call(
     inputs: &Vec<ValueOrArray>,
     opcode: AvmOpcode,
 ) {
-    if destinations.len() != 2 || inputs.len() != 4 {
+    if destinations.len() != 2 || inputs.len() != 5 {
         panic!(
-            "Transpiler expects ForeignCall (Static)Call to have 2 destinations and 4 inputs, got {} and {}.
+            "Transpiler expects ForeignCall (Static)Call to have 2 destinations and 5 inputs, got {} and {}.
             Make sure your call instructions's input/return arrays have static length (`[Field; <size>]`)!",
             destinations.len(),
             inputs.len()
         );
     }
-    let gas_offset_maybe = inputs[0];
-    let gas_offset = match gas_offset_maybe {
+    let gas = inputs[0];
+    let gas_offset = match gas {
         ValueOrArray::HeapArray(HeapArray { pointer, size }) => {
             assert!(size == 3, "Call instruction's gas input should be a HeapArray of size 3 (`[l1Gas, l2Gas, daGas]`)");
             pointer.0 as u32
@@ -401,13 +401,16 @@ fn handle_external_call(
         ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
         _ => panic!("Call instruction's target address input should be a basic MemoryAddress",),
     };
-    let args_offset_maybe = inputs[2];
-    let (args_offset, args_size) = match args_offset_maybe {
-        ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0 as u32, size as u32),
-        ValueOrArray::HeapVector(_) => panic!("Call instruction's args must be a HeapArray, not a HeapVector. Make sure you are explicitly defining its size (`[arg0, arg1, ... argN]`)!"),
-        _ => panic!("Call instruction's args input should be a HeapArray input"),
+    // The args are a slice, and this is represented as a (Field, HeapVector).
+    // The field is the length (memory address) and the HeapVector has the data and length again.
+    // This is an ACIR internal representation detail that leaks to the SSA.
+    // Observe that below, we use `inputs[3]` and therefore skip the length field.
+    let args = inputs[3];
+    let (args_offset, args_size_offset) = match args {
+        ValueOrArray::HeapVector(HeapVector { pointer, size }) => (pointer.0 as u32, size.0 as u32),
+        _ => panic!("Call instruction's args input should be a HeapVector input"),
     };
-    let temporary_function_selector_offset = match &inputs[3] {
+    let temporary_function_selector_offset = match &inputs[4] {
         ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
         _ => panic!(
             "Call instruction's temporary function selector input should be a basic MemoryAddress",
@@ -426,14 +429,23 @@ fn handle_external_call(
     };
     avm_instrs.push(AvmInstruction {
         opcode: opcode,
-        indirect: Some(0b01101), // (left to right) selector direct, ret offset INDIRECT, args offset INDIRECT, address offset direct, gas offset INDIRECT
+        // (left to right)
+        //   * selector direct
+        //   * ret offset INDIRECT
+        //   * arg size offset direct
+        //   * args offset INDIRECT
+        //   * address offset direct
+        //   * gas offset INDIRECT
+        indirect: Some(0b010101),
         operands: vec![
             AvmOperand::U32 { value: gas_offset },
             AvmOperand::U32 {
                 value: address_offset,
             },
             AvmOperand::U32 { value: args_offset },
-            AvmOperand::U32 { value: args_size },
+            AvmOperand::U32 {
+                value: args_size_offset,
+            },
             AvmOperand::U32 { value: ret_offset },
             AvmOperand::U32 { value: ret_size },
             AvmOperand::U32 {

docs/docs/protocol-specs/public-vm/gen/_instruction-set.mdx

@@ -1700,7 +1700,7 @@ Call into another contract
 	- **gasOffset**: offset to three words containing `{l1GasLeft, l2GasLeft, daGasLeft}`: amount of gas to provide to the callee
 	- **addrOffset**: address of the contract to call
 	- **argsOffset**: memory offset to args (will become the callee's calldata)
-	- **argsSize**: number of words to pass via callee's calldata
+	- **argsSizeOffset**: memory offset for the number of words to pass via callee's calldata
 	- **retOffset**: destination memory offset specifying where to store the data returned from the callee
 	- **retSize**: number of words to copy from data returned by callee
 	- **successOffset**: destination memory offset specifying where to store the call's success (0: failure, 1: success)
@@ -1748,7 +1748,7 @@ Call into another contract, disallowing World State and Accrued Substate modific
 	- **gasOffset**: offset to three words containing `{l1GasLeft, l2GasLeft, daGasLeft}`: amount of gas to provide to the callee
 	- **addrOffset**: address of the contract to call
 	- **argsOffset**: memory offset to args (will become the callee's calldata)
-	- **argsSize**: number of words to pass via callee's calldata
+	- **argsSizeOffset**: memory offset for the number of words to pass via callee's calldata
 	- **retOffset**: destination memory offset specifying where to store the data returned from the callee
 	- **retSize**: number of words to copy from data returned by callee
 	- **successOffset**: destination memory offset specifying where to store the call's success (0: failure, 1: success)
@@ -1793,7 +1793,7 @@ Call into another contract, but keep the caller's `sender` and `storageAddress`
 	- **gasOffset**: offset to three words containing `{l1GasLeft, l2GasLeft, daGasLeft}`: amount of gas to provide to the callee
 	- **addrOffset**: address of the contract to call
 	- **argsOffset**: memory offset to args (will become the callee's calldata)
-	- **argsSize**: number of words to pass via callee's calldata
+	- **argsSizeOffset**: memory offset for the number of words to pass via callee's calldata
 	- **retOffset**: destination memory offset specifying where to store the data returned from the callee
 	- **retSize**: number of words to copy from data returned by callee
 	- **successOffset**: destination memory offset specifying where to store the call's success (0: failure, 1: success)

docs/src/preprocess/InstructionSet/InstructionSet.js

@@ -38,10 +38,9 @@ const CALL_INSTRUCTION_ARGS = [
     description: "memory offset to args (will become the callee's calldata)",
   },
   {
-    name: "argsSize",
-    description: "number of words to pass via callee's calldata",
-    mode: "immediate",
-    type: "u32",
+    name: "argsSizeOffset",
+    description:
+      "memory offset for the number of words to pass via callee's calldata",
   },
   {
     name: "retOffset",

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

@@ -63,7 +63,7 @@ impl AvmContext {
         call(
             gas_for_call(gas),
             contract_address,
-            args,
+            args.as_slice(),
             temporary_function_selector
         )
     }
@@ -78,7 +78,7 @@ impl AvmContext {
         call_static(
             gas_for_call(gas),
             contract_address,
-            args,
+            args.as_slice(),
             temporary_function_selector
         )
     }
@@ -149,7 +149,7 @@ impl PublicContextInterface for AvmContext {
         let results = call(
             gas_for_call(gas_opts),
             contract_address,
-            args,
+            args.as_slice(),
             temporary_function_selector.to_field()
         );
         let data_to_return: [Field; RETURNS_COUNT] = results.0;
@@ -169,7 +169,7 @@ impl PublicContextInterface for AvmContext {
         let (data_to_return, success): ([Field; RETURNS_COUNT], u8) = call_static(
             gas_for_call(gas_opts),
             contract_address,
-            args,
+            args.as_slice(),
             temporary_function_selector.to_field()
         );
 
@@ -296,20 +296,20 @@ fn l1_to_l2_msg_exists(msg_hash: Field, msg_leaf_index: Field) -> u8 {}
 fn send_l2_to_l1_msg(recipient: EthAddress, content: Field) {}
 
 #[oracle(avmOpcodeCall)]
-fn call<ARGS_COUNT, RET_SIZE>(
+fn call<RET_SIZE>(
     gas: [Field; 3], // gas allocation: [l1_gas, l2_gas, da_gas]
     address: AztecAddress,
-    args: [Field; ARGS_COUNT],
+    args: [Field],
     // TODO(5110): consider passing in calldata directly
     temporary_function_selector: Field
 ) -> ([Field; RET_SIZE], u8) {}
 //    ^ return data      ^ success
 
 #[oracle(avmOpcodeStaticCall)]
-fn call_static<ARGS_COUNT, RET_SIZE>(
+fn call_static<RET_SIZE>(
     gas: [Field; 3], // gas allocation: [l1_gas, l2_gas, da_gas]
     address: AztecAddress,
-    args: [Field; ARGS_COUNT],
+    args: [Field],
     // TODO(5110): consider passing in calldata directly
     temporary_function_selector: Field
 ) -> ([Field; RET_SIZE], u8) {}

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

@@ -6,7 +6,7 @@ import { mock } from 'jest-mock-extended';
 import { type CommitmentsDB, type PublicContractsDB, type PublicStateDB } from '../../index.js';
 import { markBytecodeAsAvm } from '../../public/transitional_adaptors.js';
 import { type AvmContext } from '../avm_context.js';
-import { Field, Uint8 } from '../avm_memory_types.js';
+import { Field, Uint8, Uint32 } from '../avm_memory_types.js';
 import { adjustCalldataIndex, initContext } from '../fixtures/index.js';
 import { HostStorage } from '../journal/host_storage.js';
 import { AvmPersistableStateManager } from '../journal/journal.js';
@@ -36,7 +36,7 @@ describe('External Calls', () => {
         ...Buffer.from('12345678', 'hex'), // gasOffset
         ...Buffer.from('a2345678', 'hex'), // addrOffset
         ...Buffer.from('b2345678', 'hex'), // argsOffset
-        ...Buffer.from('c2345678', 'hex'), // argsSize
+        ...Buffer.from('c2345678', 'hex'), // argsSizeOffset
         ...Buffer.from('d2345678', 'hex'), // retOffset
         ...Buffer.from('e2345678', 'hex'), // retSize
         ...Buffer.from('f2345678', 'hex'), // successOffset
@@ -47,7 +47,7 @@ describe('External Calls', () => {
         /*gasOffset=*/ 0x12345678,
         /*addrOffset=*/ 0xa2345678,
         /*argsOffset=*/ 0xb2345678,
-        /*argsSize=*/ 0xc2345678,
+        /*argsSizeOffset=*/ 0xc2345678,
         /*retOffset=*/ 0xd2345678,
         /*retSize=*/ 0xe2345678,
         /*successOffset=*/ 0xf2345678,
@@ -68,6 +68,7 @@ describe('External Calls', () => {
       const argsOffset = 4;
       const args = [new Field(1n), new Field(2n), new Field(3n)];
       const argsSize = args.length;
+      const argsSizeOffset = 20;
       const retOffset = 8;
       const retSize = 2;
       const successOffset = 7;
@@ -93,6 +94,7 @@ describe('External Calls', () => {
       context.machineState.memory.set(1, new Field(l2Gas));
       context.machineState.memory.set(2, new Field(daGas));
       context.machineState.memory.set(3, new Field(addr));
+      context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
       context.machineState.memory.setSlice(4, args);
       jest
         .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
@@ -103,7 +105,7 @@ describe('External Calls', () => {
         gasOffset,
         addrOffset,
         argsOffset,
-        argsSize,
+        argsSizeOffset,
         retOffset,
         retSize,
         successOffset,
@@ -145,6 +147,7 @@ describe('External Calls', () => {
       const argsOffset = 4;
       const args = [new Field(1n), new Field(2n), new Field(3n)];
       const argsSize = args.length;
+      const argsSizeOffset = 20;
       const retOffset = 8;
       const retSize = 2;
       const successOffset = 7;
@@ -153,6 +156,7 @@ describe('External Calls', () => {
       context.machineState.memory.set(1, new Field(l2Gas));
       context.machineState.memory.set(2, new Field(daGas));
       context.machineState.memory.set(3, new Field(addr));
+      context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
       context.machineState.memory.setSlice(4, args);
 
       jest
@@ -164,7 +168,7 @@ describe('External Calls', () => {
         gasOffset,
         addrOffset,
         argsOffset,
-        argsSize,
+        argsSizeOffset,
         retOffset,
         retSize,
         successOffset,
@@ -183,7 +187,7 @@ describe('External Calls', () => {
         ...Buffer.from('12345678', 'hex'), // gasOffset
         ...Buffer.from('a2345678', 'hex'), // addrOffset
         ...Buffer.from('b2345678', 'hex'), // argsOffset
-        ...Buffer.from('c2345678', 'hex'), // argsSize
+        ...Buffer.from('c2345678', 'hex'), // argsSizeOffset
         ...Buffer.from('d2345678', 'hex'), // retOffset
         ...Buffer.from('e2345678', 'hex'), // retSize
         ...Buffer.from('f2345678', 'hex'), // successOffset
@@ -194,7 +198,7 @@ describe('External Calls', () => {
         /*gasOffset=*/ 0x12345678,
         /*addrOffset=*/ 0xa2345678,
         /*argsOffset=*/ 0xb2345678,
-        /*argsSize=*/ 0xc2345678,
+        /*argsSizeOffset=*/ 0xc2345678,
         /*retOffset=*/ 0xd2345678,
         /*retSize=*/ 0xe2345678,
         /*successOffset=*/ 0xf2345678,
@@ -214,12 +218,14 @@ describe('External Calls', () => {
       const args = [new Field(1n), new Field(2n), new Field(3n)];
 
       const argsSize = args.length;
+      const argsSizeOffset = 20;
       const retOffset = 8;
       const retSize = 2;
       const successOffset = 7;
 
       context.machineState.memory.set(0, gas);
       context.machineState.memory.set(1, addr);
+      context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
       context.machineState.memory.setSlice(2, args);
 
       const otherContextInstructions: Instruction[] = [
@@ -243,7 +249,7 @@ describe('External Calls', () => {
         gasOffset,
         addrOffset,
         argsOffset,
-        argsSize,
+        argsSizeOffset,
         retOffset,
         retSize,
         successOffset,

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

@@ -36,7 +36,7 @@ abstract class ExternalCall extends Instruction {
     private gasOffset: number /* Unused due to no formal gas implementation at this moment */,
     private addrOffset: number,
     private argsOffset: number,
-    private argsSize: number,
+    private argsSizeOffset: number,
     private retOffset: number,
     private retSize: number,
     private successOffset: number,
@@ -50,20 +50,23 @@ abstract class ExternalCall extends Instruction {
 
   public async execute(context: AvmContext) {
     const memory = context.machineState.memory.track(this.type);
-    const [gasOffset, addrOffset, argsOffset, retOffset, successOffset] = Addressing.fromWire(this.indirect).resolve(
-      [this.gasOffset, this.addrOffset, this.argsOffset, this.retOffset, this.successOffset],
+    const [gasOffset, addrOffset, argsOffset, argsSizeOffset, retOffset, successOffset] = Addressing.fromWire(
+      this.indirect,
+    ).resolve(
+      [this.gasOffset, this.addrOffset, this.argsOffset, this.argsSizeOffset, this.retOffset, this.successOffset],
       memory,
     );
 
     const callAddress = memory.getAs<Field>(addrOffset);
-    const calldata = memory.getSlice(argsOffset, this.argsSize).map(f => f.toFr());
+    const calldataSize = memory.get(argsSizeOffset).toNumber();
+    const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr());
     const l1Gas = memory.get(gasOffset).toNumber();
     const l2Gas = memory.getAs<Field>(gasOffset + 1).toNumber();
     const daGas = memory.getAs<Field>(gasOffset + 2).toNumber();
     const functionSelector = memory.getAs<Field>(this.temporaryFunctionSelectorOffset).toFr();
 
     const allocatedGas = { l1Gas, l2Gas, daGas };
-    const memoryOperations = { reads: this.argsSize + 5, writes: 1 + this.retSize, indirect: this.indirect };
+    const memoryOperations = { reads: calldataSize + 6, writes: 1 + this.retSize, indirect: this.indirect };
     const totalGas = sumGas(this.gasCost(memoryOperations), allocatedGas);
     context.machineState.consumeGas(totalGas);