based on david's

Fix failing test.

avm-transpiler/src/transpile.rs

@@ -180,7 +180,7 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
             BrilligOpcode::Stop { return_data_offset, return_data_size } => {
                 avm_instrs.push(AvmInstruction {
                     opcode: AvmOpcode::RETURN,
-                    indirect: Some(ALL_DIRECT),
+                    indirect: Some(ZEROTH_OPERAND_INDIRECT),
                     operands: vec![
                         AvmOperand::U32 { value: *return_data_offset as u32 },
                         AvmOperand::U32 { value: *return_data_size as u32 },
@@ -239,9 +239,10 @@ fn handle_foreign_call(
     inputs: &Vec<ValueOrArray>,
 ) {
     match function {
+        "avmOpcodeCall" => handle_external_call(avm_instrs, destinations, inputs),
         "amvOpcodeEmitUnencryptedLog" => {
             handle_emit_unencrypted_log(avm_instrs, destinations, inputs)
-        },
+        }
         "avmOpcodeNoteHashExists" => handle_note_hash_exists(avm_instrs, destinations, inputs),
         "avmOpcodeEmitNoteHash" | "avmOpcodeEmitNullifier" => handle_emit_note_hash_or_nullifier(
             function == "avmOpcodeEmitNullifier",
@@ -257,7 +258,7 @@ fn handle_foreign_call(
         }
         "avmOpcodePoseidon" => {
             handle_single_field_hash_instruction(avm_instrs, function, destinations, inputs)
-        },
+        }
         "storageRead" => handle_storage_read(avm_instrs, destinations, inputs),
         "storageWrite" => handle_storage_write(avm_instrs, destinations, inputs),
         // Getters.
@@ -272,6 +273,81 @@ fn handle_foreign_call(
     }
 }
 
+/// Handle an AVM CALL
+/// (an external 'call' brillig foreign call was encountered)
+/// Adds the new instruction to the avm instructions list.
+fn handle_external_call(
+    avm_instrs: &mut Vec<AvmInstruction>,
+    destinations: &Vec<ValueOrArray>,
+    inputs: &Vec<ValueOrArray>,
+) {
+    if destinations.len() != 2 || inputs.len() != 4 {
+        panic!(
+            "Transpiler expects ForeignCall::CALL to have 2 destinations and 4 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 {
+        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
+        }
+        ValueOrArray::HeapVector(_) => panic!("Call instruction's gas input must be a HeapArray, not a HeapVector. Make sure you are explicitly defining its size as 3 (`[l1Gas, l2Gas, daGas]`)!"),
+        _ => panic!("Call instruction's gas input should be a HeapArray"),
+    };
+    let address_offset = match &inputs[1] {
+        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"),
+    };
+    let temporary_function_selector_offset = match &inputs[3] {
+        ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
+        _ => panic!(
+            "Call instruction's temporary function selector input should be a basic MemoryAddress",
+        ),
+    };
+
+    let ret_offset_maybe = destinations[0];
+    let (ret_offset, ret_size) = match ret_offset_maybe {
+        ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0 as u32, size as u32),
+        ValueOrArray::HeapVector(_) => panic!("Call instruction's return data must be a HeapArray, not a HeapVector. Make sure you are explicitly defining its size (`let returnData: [Field; <size>] = ...`)!"),
+        _ => panic!("Call instruction's returnData destination should be a HeapArray input"),
+    };
+    let success_offset = match &destinations[1] {
+        ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
+        _ => panic!("Call instruction's success destination should be a basic MemoryAddress",),
+    };
+    avm_instrs.push(AvmInstruction {
+        opcode: AvmOpcode::CALL,
+        indirect: Some(0b01101), // (left to right) selector direct, ret offset INDIRECT, args offset INDIRECT, address offset direct, gas offset INDIRECT
+        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: ret_offset },
+            AvmOperand::U32 { value: ret_size },
+            AvmOperand::U32 {
+                value: success_offset,
+            },
+            AvmOperand::U32 {
+                value: temporary_function_selector_offset,
+            },
+        ],
+        ..Default::default()
+    });
+}
+
 /// Handle an AVM NOTEHASHEXISTS instruction
 /// Adds the new instruction to the avm instructions list.
 fn handle_note_hash_exists(
@@ -328,7 +404,10 @@ fn handle_emit_unencrypted_log(
         [ValueOrArray::MemoryAddress(offset), ValueOrArray::HeapArray(array)] => {
             (offset.to_usize() as u32, array)
         }
-        _ => panic!("Unexpected inputs for ForeignCall::EMITUNENCRYPTEDLOG: {:?}", inputs),
+        _ => panic!(
+            "Unexpected inputs for ForeignCall::EMITUNENCRYPTEDLOG: {:?}",
+            inputs
+        ),
     };
     avm_instrs.push(AvmInstruction {
         opcode: AvmOpcode::EMITUNENCRYPTEDLOG,

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

@@ -1,5 +1,6 @@
 use dep::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH};
 use dep::protocol_types::traits::{Serialize};
+use dep::protocol_types::abis::function_selector::FunctionSelector;
 
 // Getters that will be converted by the transpiler into their
 // own opcodes
@@ -80,9 +81,19 @@ impl AVMContext {
     #[oracle(avmOpcodeSendL2ToL1Msg)]
     pub fn send_l2_to_l1_msg(self, recipient: EthAddress, content: Field) {}
 
-    ///////////////////////////////////////////////////////////////////////////
-    // The functions below allow interface-equivalence with PrivateContext
-    ///////////////////////////////////////////////////////////////////////////
+    #[oracle(avmOpcodeCall)]
+    fn call<ARGS_COUNT, RET_SIZE>(
+        self,
+        gas: [Field; 3], // gas allocation: [l1Gas, l2Gas, daGas]
+        address: AztecAddress,
+        args: [Field; ARGS_COUNT],
+        temporary_function_selector: Field
+    ) -> ([Field; RET_SIZE], u8) {}
+    //    ^ return data      ^ success
+
+    ////////////////////////////////////////////////////////////////////////////////
+    // The functions below allow interface-equivalence with current public/private
+    ////////////////////////////////////////////////////////////////////////////////
     pub fn this_address(self) -> AztecAddress {
         self.address()
     }
@@ -107,4 +118,25 @@ impl AVMContext {
         // Cannot nullify pending commitments in AVM, so `nullified_commitment` is not used
         self.emit_nullifier(nullifier);
     }
+
+    pub fn call_public_function<ARGS_COUNT, RET_SIZE>(
+        self: Self,
+        contract_address: AztecAddress,
+        temporary_function_selector: FunctionSelector,
+        args: [Field; ARGS_COUNT]
+    ) -> [Field; RET_SIZE] {
+        let gas = [/*l1Gas*/42, /*l2Gas*/24, /*daGas*/420];
+
+        let results = self.call(
+            gas,
+            contract_address,
+            args,
+            temporary_function_selector.to_field()
+        );
+        let returnData: [Field; RET_SIZE] = results.0;
+        let success: u8 = results.1;
+        assert(success == 1, "Nested call failed!");
+
+        returnData
+    }
 }

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

@@ -2,6 +2,7 @@ contract AvmTest {
     // Libs
     use dep::aztec::state_vars::PublicMutable;
     use dep::aztec::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH};
+    use dep::aztec::protocol_types::abis::function_selector::FunctionSelector;
     use dep::compressed_string::CompressedString;
 
     // avm lib
@@ -214,4 +215,37 @@ contract AvmTest {
     fn send_l2_to_l1_msg(recipient: EthAddress, content: Field) {
         context.message_portal(recipient, content)
     }
+
+    // Directly call the external call opcode to initiate a nested call to the add function
+    #[aztec(public-vm)]
+    fn raw_nested_call_to_add(argA: Field, argB: Field) -> pub Field {
+        let selector = FunctionSelector::from_signature("avm_addArgsReturn(Field,Field)").to_field();
+        let gas = [/*l1Gas*/42, /*l2Gas*/24, /*daGas*/420];
+
+        // Nested call
+        let results = context.call(gas, context.address(), [argA, argB], selector);
+        let returnData: [Field; 1] = results.0;
+        // this explicit size   ^ is necessary to ensure that retSize is compile-time
+        // (ensure the returnData is in a HeapArray not a HeapVector)
+        let success: u8 = results.1;
+
+        assert(success == 1, "Call failed");
+
+        let addResult = returnData[0];
+        addResult
+    }
+
+    // Use the `call_public_function` wrapper to initiate a nested call to the add function
+    #[aztec(public-vm)]
+    fn nested_call_to_add(argA: Field, argB: Field) -> pub Field {
+        let selector = FunctionSelector::from_signature("avm_addArgsReturn(Field,Field)");
+
+        // Nested call using standard context interface function
+        let returnData: [Field; 1] = context.call_public_function(context.address(), selector, [argA, argB]);
+        // this explicit size   ^ is necessary to ensure that retSize is compile-time
+        // (ensure the returnData is in a HeapArray not a HeapVector)
+
+        let addResult = returnData[0];
+        addResult
+    }
 }

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

@@ -1,4 +1,4 @@
-import { AztecAddress } from '@aztec/circuits.js';
+import { AztecAddress, FunctionSelector } from '@aztec/circuits.js';
 import { Fr } from '@aztec/foundation/fields';
 
 import { AvmExecutionEnvironment } from './avm_execution_environment.js';
@@ -35,8 +35,16 @@ export class AvmContext {
    * @param calldata - Data/arguments for nested call
    * @returns new AvmContext instance
    */
-  public createNestedContractCallContext(address: AztecAddress, calldata: Fr[]): AvmContext {
-    const newExecutionEnvironment = this.environment.deriveEnvironmentForNestedCall(address, calldata);
+  public createNestedContractCallContext(
+    address: AztecAddress,
+    calldata: Fr[],
+    temporaryFunctionSelector: FunctionSelector = FunctionSelector.empty(),
+  ): AvmContext {
+    const newExecutionEnvironment = this.environment.deriveEnvironmentForNestedCall(
+      address,
+      calldata,
+      temporaryFunctionSelector,
+    );
     const forkedWorldState = this.persistableState.fork();
     const machineState = AvmMachineState.fromState(this.machineState);
     return new AvmContext(forkedWorldState, newExecutionEnvironment, machineState);
@@ -54,8 +62,16 @@ export class AvmContext {
    * @param calldata - Data/arguments for nested call
    * @returns new AvmContext instance
    */
-  public createNestedContractStaticCallContext(address: AztecAddress, calldata: Fr[]): AvmContext {
-    const newExecutionEnvironment = this.environment.deriveEnvironmentForNestedStaticCall(address, calldata);
+  public createNestedContractStaticCallContext(
+    address: AztecAddress,
+    calldata: Fr[],
+    temporaryFunctionSelector: FunctionSelector = FunctionSelector.empty(),
+  ): AvmContext {
+    const newExecutionEnvironment = this.environment.deriveEnvironmentForNestedStaticCall(
+      address,
+      calldata,
+      temporaryFunctionSelector,
+    );
     const forkedWorldState = this.persistableState.fork();
     const machineState = AvmMachineState.fromState(this.machineState);
     return new AvmContext(forkedWorldState, newExecutionEnvironment, machineState);

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

@@ -36,12 +36,19 @@ export class AvmExecutionEnvironment {
 
     public readonly calldata: Fr[],
 
+    // Function selector is temporary since eventually public contract bytecode will be one blob
+    // containing all functions, and function selector will become an application-level mechanism
+    // (e.g. first few bytes of calldata + compiler-generated jump table)
     public readonly temporaryFunctionSelector: FunctionSelector,
   ) {}
 
-  public deriveEnvironmentForNestedCall(address: AztecAddress, calldata: Fr[]): AvmExecutionEnvironment {
+  public deriveEnvironmentForNestedCall(
+    address: AztecAddress,
+    calldata: Fr[],
+    temporaryFunctionSelector: FunctionSelector = FunctionSelector.empty(),
+  ): AvmExecutionEnvironment {
     return new AvmExecutionEnvironment(
-      /*address=*/ address,
+      address,
       /*storageAddress=*/ address,
       this.origin,
       this.sender,
@@ -53,14 +60,18 @@ export class AvmExecutionEnvironment {
       this.globals,
       this.isStaticCall,
       this.isDelegateCall,
-      /*calldata=*/ calldata,
-      this.temporaryFunctionSelector,
+      calldata,
+      temporaryFunctionSelector,
     );
   }
 
-  public deriveEnvironmentForNestedStaticCall(address: AztecAddress, calldata: Fr[]): AvmExecutionEnvironment {
+  public deriveEnvironmentForNestedStaticCall(
+    address: AztecAddress,
+    calldata: Fr[],
+    temporaryFunctionSelector: FunctionSelector = FunctionSelector.empty(),
+  ): AvmExecutionEnvironment {
     return new AvmExecutionEnvironment(
-      /*address=*/ address,
+      address,
       /*storageAddress=*/ address,
       this.origin,
       this.sender,
@@ -72,14 +83,18 @@ export class AvmExecutionEnvironment {
       this.globals,
       /*isStaticCall=*/ true,
       this.isDelegateCall,
-      /*calldata=*/ calldata,
-      this.temporaryFunctionSelector,
+      calldata,
+      temporaryFunctionSelector,
     );
   }
 
-  public newDelegateCall(address: AztecAddress, calldata: Fr[]): AvmExecutionEnvironment {
+  public newDelegateCall(
+    address: AztecAddress,
+    calldata: Fr[],
+    temporaryFunctionSelector: FunctionSelector = FunctionSelector.empty(),
+  ): AvmExecutionEnvironment {
     return new AvmExecutionEnvironment(
-      /*address=*/ address,
+      address,
       this.storageAddress,
       this.origin,
       this.sender,
@@ -91,8 +106,8 @@ export class AvmExecutionEnvironment {
       this.globals,
       this.isStaticCall,
       /*isDelegateCall=*/ true,
-      /*calldata=*/ calldata,
-      this.temporaryFunctionSelector,
+      calldata,
+      temporaryFunctionSelector,
     );
   }
 }

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

@@ -394,6 +394,38 @@ describe('AVM simulator', () => {
       });
     });
 
+    describe('Test nested external calls from noir contract', () => {
+      it(`Should execute contract function that makes a nested call`, async () => {
+        const calldata: Fr[] = [new Fr(1), new Fr(2)];
+        const callBytecode = getAvmTestContractBytecode('avm_raw_nested_call_to_add');
+        const addBytecode = getAvmTestContractBytecode('avm_addArgsReturn');
+        const context = initContext({ env: initExecutionEnvironment({ calldata }) });
+        jest
+          .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
+          .mockReturnValueOnce(Promise.resolve(addBytecode));
+
+        const results = await new AvmSimulator(context).executeBytecode(callBytecode);
+
+        expect(results.reverted).toBe(false);
+        expect(results.output).toEqual([new Fr(3)]);
+      });
+
+      it(`Should execute contract function that makes a nested call through the old interface`, async () => {
+        const calldata: Fr[] = [new Fr(1), new Fr(2)];
+        const callBytecode = getAvmTestContractBytecode('avm_nested_call_to_add');
+        const addBytecode = getAvmTestContractBytecode('avm_addArgsReturn');
+        const context = initContext({ env: initExecutionEnvironment({ calldata }) });
+        jest
+          .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
+          .mockReturnValueOnce(Promise.resolve(addBytecode));
+
+        const results = await new AvmSimulator(context).executeBytecode(callBytecode);
+
+        expect(results.reverted).toBe(false);
+        expect(results.output).toEqual([new Fr(3)]);
+      });
+    });
+
     describe('Storage accesses', () => {
       it('Should set a single value in storage', async () => {
         // We are setting the owner