diff --git a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs index a89041dbdcd..838886a03ce 100644 --- a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs +++ b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs @@ -252,7 +252,7 @@ fn complex_brillig_foreign_call() { value: FieldElement::from(0_usize), }, brillig::Opcode::CalldataCopy { - destination_address: MemoryAddress(0), + destination_address: MemoryAddress(32), size_address: MemoryAddress(0), offset_address: MemoryAddress(1), }, @@ -271,11 +271,6 @@ fn complex_brillig_foreign_call() { bit_size: BitSize::Integer(IntegerBitSize::U32), value: FieldElement::from(3_usize), }, - brillig::Opcode::CalldataCopy { - destination_address: MemoryAddress(0), - size_address: MemoryAddress(0), - offset_address: MemoryAddress(1), - }, brillig::Opcode::CalldataCopy { destination_address: MemoryAddress(1), size_address: MemoryAddress(3), @@ -343,17 +338,17 @@ fn complex_brillig_foreign_call() { let bytes = Program::serialize_program(&program); let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 85, 93, 14, 194, 32, 12, 110, 97, 34, 209, 55, 79, - 96, 162, 7, 96, 122, 1, 239, 98, 124, 211, 232, 163, 199, 223, 200, 74, 214, 117, 100, 123, - 88, 73, 182, 47, 33, 45, 208, 54, 253, 5, 132, 14, 190, 93, 72, 252, 142, 168, 33, 26, 207, - 45, 12, 145, 100, 31, 68, 195, 50, 212, 168, 103, 43, 148, 242, 209, 108, 192, 71, 91, 192, - 71, 96, 245, 95, 97, 189, 53, 235, 162, 154, 63, 155, 153, 159, 10, 186, 249, 114, 180, - 223, 19, 245, 50, 209, 81, 201, 192, 120, 240, 6, 72, 221, 115, 144, 86, 232, 174, 210, - 139, 230, 206, 95, 8, 101, 219, 193, 77, 196, 86, 46, 166, 91, 240, 34, 225, 138, 49, 213, - 158, 236, 149, 168, 197, 84, 190, 28, 227, 121, 215, 69, 28, 161, 239, 184, 231, 247, 243, - 123, 191, 254, 86, 168, 115, 181, 136, 11, 227, 175, 226, 30, 133, 158, 212, 229, 224, 159, - 73, 78, 22, 51, 114, 115, 182, 79, 237, 58, 19, 159, 62, 173, 6, 242, 203, 5, 245, 209, 6, - 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 85, 81, 14, 194, 48, 8, 133, 118, 206, 26, 255, 60, + 129, 137, 30, 160, 211, 11, 120, 23, 227, 159, 70, 63, 61, 190, 146, 209, 140, 177, 46, + 251, 24, 77, 182, 151, 44, 116, 45, 16, 120, 64, 139, 208, 34, 252, 63, 228, 245, 134, 165, + 99, 73, 251, 30, 250, 72, 186, 55, 150, 113, 30, 26, 180, 243, 21, 75, 197, 232, 86, 16, + 163, 47, 16, 35, 136, 250, 47, 176, 222, 150, 117, 49, 229, 207, 103, 230, 167, 130, 118, + 190, 106, 254, 223, 178, 12, 154, 104, 50, 114, 48, 28, 188, 30, 82, 247, 236, 180, 23, 62, + 171, 236, 178, 185, 202, 27, 194, 216, 119, 36, 54, 142, 35, 185, 149, 203, 233, 18, 131, + 34, 220, 48, 167, 38, 176, 191, 18, 181, 168, 5, 63, 178, 179, 8, 123, 232, 186, 234, 254, + 126, 125, 158, 143, 175, 87, 148, 74, 51, 194, 73, 172, 207, 234, 28, 149, 157, 182, 149, + 144, 15, 70, 78, 23, 51, 122, 83, 190, 15, 208, 181, 70, 122, 152, 126, 56, 83, 244, 10, + 181, 6, 0, 0, ]; assert_eq!(bytes, expected_serialization) diff --git a/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts b/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts index d1d4cc4b272..ba26e3d139a 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts +++ b/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts @@ -2,14 +2,14 @@ import { WitnessMap } from '@noir-lang/acvm_js'; // See `complex_brillig_foreign_call` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 85, 93, 14, 194, 32, 12, 110, 97, 34, 209, 55, 79, 96, 162, 7, 96, 122, 1, - 239, 98, 124, 211, 232, 163, 199, 223, 200, 74, 214, 117, 100, 123, 88, 73, 182, 47, 33, 45, 208, 54, 253, 5, 132, 14, - 190, 93, 72, 252, 142, 168, 33, 26, 207, 45, 12, 145, 100, 31, 68, 195, 50, 212, 168, 103, 43, 148, 242, 209, 108, - 192, 71, 91, 192, 71, 96, 245, 95, 97, 189, 53, 235, 162, 154, 63, 155, 153, 159, 10, 186, 249, 114, 180, 223, 19, - 245, 50, 209, 81, 201, 192, 120, 240, 6, 72, 221, 115, 144, 86, 232, 174, 210, 139, 230, 206, 95, 8, 101, 219, 193, - 77, 196, 86, 46, 166, 91, 240, 34, 225, 138, 49, 213, 158, 236, 149, 168, 197, 84, 190, 28, 227, 121, 215, 69, 28, - 161, 239, 184, 231, 247, 243, 123, 191, 254, 86, 168, 115, 181, 136, 11, 227, 175, 226, 30, 133, 158, 212, 229, 224, - 159, 73, 78, 22, 51, 114, 115, 182, 79, 237, 58, 19, 159, 62, 173, 6, 242, 203, 5, 245, 209, 6, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 85, 81, 14, 194, 48, 8, 133, 118, 206, 26, 255, 60, 129, 137, 30, 160, 211, + 11, 120, 23, 227, 159, 70, 63, 61, 190, 146, 209, 140, 177, 46, 251, 24, 77, 182, 151, 44, 116, 45, 16, 120, 64, 139, + 208, 34, 252, 63, 228, 245, 134, 165, 99, 73, 251, 30, 250, 72, 186, 55, 150, 113, 30, 26, 180, 243, 21, 75, 197, 232, + 86, 16, 163, 47, 16, 35, 136, 250, 47, 176, 222, 150, 117, 49, 229, 207, 103, 230, 167, 130, 118, 190, 106, 254, 223, + 178, 12, 154, 104, 50, 114, 48, 28, 188, 30, 82, 247, 236, 180, 23, 62, 171, 236, 178, 185, 202, 27, 194, 216, 119, + 36, 54, 142, 35, 185, 149, 203, 233, 18, 131, 34, 220, 48, 167, 38, 176, 191, 18, 181, 168, 5, 63, 178, 179, 8, 123, + 232, 186, 234, 254, 126, 125, 158, 143, 175, 87, 148, 74, 51, 194, 73, 172, 207, 234, 28, 149, 157, 182, 149, 144, 15, + 70, 78, 23, 51, 122, 83, 190, 15, 208, 181, 70, 122, 152, 126, 56, 83, 244, 10, 181, 6, 0, 0, ]); export const initialWitnessMap: WitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index 5ee00cf6c29..c85940cc1c7 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -46,6 +46,9 @@ impl BrilligContext { arguments: &[BrilligParameter], return_parameters: &[BrilligParameter], ) { + // We need to allocate the variable for every argument first so any register allocation doesn't mangle the expected order. + let mut argument_variables = self.allocate_function_arguments(arguments); + let calldata_size = Self::flattened_tuple_size(arguments); let return_data_size = Self::flattened_tuple_size(return_parameters); @@ -64,75 +67,45 @@ impl BrilligContext { // Copy calldata self.copy_and_cast_calldata(arguments); - // Allocate the variables for every argument: let mut current_calldata_pointer = Self::calldata_start_offset(); - let mut argument_variables: Vec<_> = arguments - .iter() - .map(|argument| match argument { - BrilligParameter::SingleAddr(bit_size) => { - let single_address = self.allocate_register(); - let var = BrilligVariable::SingleAddr(SingleAddrVariable { - address: single_address, - bit_size: *bit_size, - }); - self.mov_instruction(single_address, MemoryAddress(current_calldata_pointer)); - current_calldata_pointer += 1; - var - } - BrilligParameter::Array(_, _) => { - let pointer_to_the_array_in_calldata = - self.make_usize_constant_instruction(current_calldata_pointer.into()); - let rc_register = self.make_usize_constant_instruction(1_usize.into()); - let flattened_size = Self::flattened_size(argument); - let var = BrilligVariable::BrilligArray(BrilligArray { - pointer: pointer_to_the_array_in_calldata.address, - size: flattened_size, - rc: rc_register.address, - }); - - current_calldata_pointer += flattened_size; - var - } - BrilligParameter::Slice(_, _) => { - let pointer_to_the_array_in_calldata = - self.make_usize_constant_instruction(current_calldata_pointer.into()); - - let flattened_size = Self::flattened_size(argument); - let size_register = self.make_usize_constant_instruction(flattened_size.into()); - let rc_register = self.make_usize_constant_instruction(1_usize.into()); - - let var = BrilligVariable::BrilligVector(BrilligVector { - pointer: pointer_to_the_array_in_calldata.address, - size: size_register.address, - rc: rc_register.address, - }); - - current_calldata_pointer += flattened_size; - var - } - }) - .collect(); - - // Deflatten arrays + // Initialize the variables with the calldata for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) { match (argument_variable, argument) { + (BrilligVariable::SingleAddr(single_address), BrilligParameter::SingleAddr(_)) => { + self.mov_instruction( + single_address.address, + MemoryAddress(current_calldata_pointer), + ); + current_calldata_pointer += 1; + } ( BrilligVariable::BrilligArray(array), BrilligParameter::Array(item_type, item_count), ) => { + let flattened_size = array.size; + self.usize_const_instruction(array.pointer, current_calldata_pointer.into()); + self.usize_const_instruction(array.rc, 1_usize.into()); + + // Deflatten the array let deflattened_address = self.deflatten_array(item_type, array.size, array.pointer); self.mov_instruction(array.pointer, deflattened_address); array.size = item_type.len() * item_count; self.deallocate_register(deflattened_address); + + current_calldata_pointer += flattened_size; } ( BrilligVariable::BrilligVector(vector), BrilligParameter::Slice(item_type, item_count), ) => { let flattened_size = Self::flattened_size(argument); + self.usize_const_instruction(vector.pointer, current_calldata_pointer.into()); + self.usize_const_instruction(vector.rc, 1_usize.into()); + self.usize_const_instruction(vector.size, flattened_size.into()); + // Deflatten the vector let deflattened_address = self.deflatten_array(item_type, flattened_size, vector.pointer); self.mov_instruction(vector.pointer, deflattened_address); @@ -140,14 +113,45 @@ impl BrilligContext { vector.size, (item_type.len() * item_count).into(), ); - self.deallocate_register(deflattened_address); + + current_calldata_pointer += flattened_size; } - _ => {} + _ => unreachable!("ICE: cannot match variables against arguments"), } } } + fn allocate_function_arguments( + &mut self, + arguments: &[BrilligParameter], + ) -> Vec { + arguments + .iter() + .map(|argument| match argument { + BrilligParameter::SingleAddr(bit_size) => { + BrilligVariable::SingleAddr(SingleAddrVariable { + address: self.allocate_register(), + bit_size: *bit_size, + }) + } + BrilligParameter::Array(_, _) => { + let flattened_size = Self::flattened_size(argument); + BrilligVariable::BrilligArray(BrilligArray { + pointer: self.allocate_register(), + size: flattened_size, + rc: self.allocate_register(), + }) + } + BrilligParameter::Slice(_, _) => BrilligVariable::BrilligVector(BrilligVector { + pointer: self.allocate_register(), + size: self.allocate_register(), + rc: self.allocate_register(), + }), + }) + .collect() + } + fn copy_and_cast_calldata(&mut self, arguments: &[BrilligParameter]) { let calldata_size = Self::flattened_tuple_size(arguments); self.calldata_copy_instruction( diff --git a/noir/noir-repo/tooling/debugger/src/context.rs b/noir/noir-repo/tooling/debugger/src/context.rs index feae1dda42d..dde3fe84d88 100644 --- a/noir/noir-repo/tooling/debugger/src/context.rs +++ b/noir/noir-repo/tooling/debugger/src/context.rs @@ -1046,7 +1046,7 @@ mod tests { }) ); - // Execute the first Brillig opcode (calldata copy) + // Const let result = context.step_into_opcode(); assert!(matches!(result, DebugCommandResult::Ok)); assert_eq!( @@ -1058,7 +1058,7 @@ mod tests { }) ); - // execute the second Brillig opcode (const) + // Const let result = context.step_into_opcode(); assert!(matches!(result, DebugCommandResult::Ok)); assert_eq!( @@ -1070,26 +1070,50 @@ mod tests { }) ); - // try to execute the third Brillig opcode (and resolve the foreign call) + // Calldatacopy let result = context.step_into_opcode(); assert!(matches!(result, DebugCommandResult::Ok)); assert_eq!( context.get_current_debug_location(), Some(DebugLocation { circuit_id: 0, - opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }, + opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }, brillig_function_id: Some(BrilligFunctionId(0)), }) ); - // retry the third Brillig opcode (foreign call should be finished) + // Const let result = context.step_into_opcode(); assert!(matches!(result, DebugCommandResult::Ok)); assert_eq!( context.get_current_debug_location(), Some(DebugLocation { circuit_id: 0, - opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }, + opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 4 }, + brillig_function_id: Some(BrilligFunctionId(0)), + }) + ); + + // try to execute the Brillig opcode (and resolve the foreign call) + let result = context.step_into_opcode(); + assert!(matches!(result, DebugCommandResult::Ok)); + assert_eq!( + context.get_current_debug_location(), + Some(DebugLocation { + circuit_id: 0, + opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 4 }, + brillig_function_id: Some(BrilligFunctionId(0)), + }) + ); + + // retry the Brillig opcode (foreign call should be finished) + let result = context.step_into_opcode(); + assert!(matches!(result, DebugCommandResult::Ok)); + assert_eq!( + context.get_current_debug_location(), + Some(DebugLocation { + circuit_id: 0, + opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 5 }, brillig_function_id: Some(BrilligFunctionId(0)), }) ); diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts index 663b079a438..48f703c8185 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -4,7 +4,7 @@ import { mock } from 'jest-mock-extended'; import { type PublicSideEffectTraceInterface } from '../../public/side_effect_trace_interface.js'; import { type AvmContext } from '../avm_context.js'; -import { Field, Uint8, Uint32 } from '../avm_memory_types.js'; +import { Field, TypeTag, Uint8, Uint32 } from '../avm_memory_types.js'; import { markBytecodeAsAvm } from '../bytecode_utils.js'; import { adjustCalldataIndex, initContext, initHostStorage, initPersistableStateManager } from '../fixtures/index.js'; import { type HostStorage } from '../journal/host_storage.js'; @@ -14,7 +14,7 @@ import { mockGetBytecode, mockTraceFork } from '../test_utils.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'; +import { CalldataCopy, Set } from './memory.js'; import { SStore } from './storage.js'; describe('External Calls', () => { @@ -83,12 +83,9 @@ describe('External Calls', () => { // const otherContextInstructionsL2GasCost = 780; // Includes the cost of the call itself const otherContextInstructionsBytecode = markBytecodeAsAvm( encodeToBytecode([ - new CalldataCopy( - /*indirect=*/ 0, - /*csOffset=*/ adjustCalldataIndex(0), - /*copySize=*/ argsSize, - /*dstOffset=*/ 0, - ), + new Set(/*indirect=*/ 0, TypeTag.UINT32, adjustCalldataIndex(0), /*dstOffset=*/ 0), + new Set(/*indirect=*/ 0, TypeTag.UINT32, argsSize, /*dstOffset=*/ 1), + new CalldataCopy(/*indirect=*/ 0, /*csOffsetAddress=*/ 0, /*copySizeOffset=*/ 1, /*dstOffset=*/ 0), new SStore(/*indirect=*/ 0, /*srcOffset=*/ valueOffset, /*slotOffset=*/ slotOffset), new Return(/*indirect=*/ 0, /*retOffset=*/ 0, /*size=*/ 2), ]), diff --git a/yarn-project/simulator/src/avm/opcodes/memory.test.ts b/yarn-project/simulator/src/avm/opcodes/memory.test.ts index efbbdf5a362..798994b765c 100644 --- a/yarn-project/simulator/src/avm/opcodes/memory.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/memory.test.ts @@ -432,14 +432,14 @@ describe('Memory instructions', () => { const buf = Buffer.from([ CalldataCopy.opcode, // opcode 0x01, // indirect - ...Buffer.from('12345678', 'hex'), // cdOffset - ...Buffer.from('23456789', 'hex'), // copysize + ...Buffer.from('12345678', 'hex'), // cdOffsetAddress + ...Buffer.from('23456789', 'hex'), // copysizeOffset ...Buffer.from('3456789a', 'hex'), // dstOffset ]); const inst = new CalldataCopy( /*indirect=*/ 0x01, - /*cdOffset=*/ 0x12345678, - /*copysize=*/ 0x23456789, + /*cdOffsetAddress=*/ 0x12345678, + /*copysizeOffset=*/ 0x23456789, /*dstOffset=*/ 0x3456789a, );