From 836a3f2453afae83dae9cb57df11b7cf4ef90af8 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Oct 2023 12:10:33 +0000 Subject: [PATCH 1/4] feat: Maintain shape of foreign call arguments --- acvm-repo/acvm/src/pwg/brillig.rs | 4 +- acvm-repo/acvm/tests/solver.rs | 15 ++++--- acvm-repo/acvm_js/src/foreign_call/inputs.rs | 8 ++-- acvm-repo/acvm_js/src/foreign_call/outputs.rs | 12 +++--- acvm-repo/brillig/src/foreign_call.rs | 40 ++++++++++++++++--- acvm-repo/brillig/src/lib.rs | 2 +- acvm-repo/brillig_vm/src/lib.rs | 40 ++++++++++--------- .../noirc_evaluator/src/brillig/brillig_ir.rs | 4 +- compiler/noirc_printable_type/src/lib.rs | 28 ++++++------- tooling/nargo/src/ops/foreign_calls.rs | 19 +++++---- 10 files changed, 106 insertions(+), 66 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 4941e20d5b8..9b0ecd87492 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -1,5 +1,5 @@ use acir::{ - brillig::{RegisterIndex, Value}, + brillig::{ForeignCallParam, RegisterIndex, Value}, circuit::{ brillig::{Brillig, BrilligInputs, BrilligOutputs}, OpcodeLocation, @@ -159,5 +159,5 @@ pub struct ForeignCallWaitInfo { /// An identifier interpreted by the caller process pub function: String, /// Resolved inputs to a foreign call computed in the previous steps of a Brillig VM process - pub inputs: Vec>, + pub inputs: Vec, } diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index ca0ca99ba07..d2bd3347945 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -145,7 +145,8 @@ fn inversion_brillig_oracle_equivalence() { assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input"); // As caller of VM, need to resolve foreign calls - let foreign_call_result = Value::from(foreign_call_wait_info.inputs[0][0].to_field().inverse()); + let foreign_call_result = + Value::from(foreign_call_wait_info.inputs[0].unwrap_value().to_field().inverse()); // Alter Brillig oracle opcode with foreign call resolution acvm.resolve_pending_foreign_call(foreign_call_result.into()); @@ -274,7 +275,8 @@ fn double_inversion_brillig_oracle() { acvm.get_pending_foreign_call().expect("should have a brillig foreign call request"); assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input"); - let x_plus_y_inverse = Value::from(foreign_call_wait_info.inputs[0][0].to_field().inverse()); + let x_plus_y_inverse = + Value::from(foreign_call_wait_info.inputs[0].unwrap_value().to_field().inverse()); // Resolve Brillig foreign call acvm.resolve_pending_foreign_call(x_plus_y_inverse.into()); @@ -291,7 +293,8 @@ fn double_inversion_brillig_oracle() { acvm.get_pending_foreign_call().expect("should have a brillig foreign call request"); assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input"); - let i_plus_j_inverse = Value::from(foreign_call_wait_info.inputs[0][0].to_field().inverse()); + let i_plus_j_inverse = + Value::from(foreign_call_wait_info.inputs[0].unwrap_value().to_field().inverse()); assert_ne!(x_plus_y_inverse, i_plus_j_inverse); // Alter Brillig oracle opcode @@ -396,7 +399,8 @@ fn oracle_dependent_execution() { assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input"); // Resolve Brillig foreign call - let x_inverse = Value::from(foreign_call_wait_info.inputs[0][0].to_field().inverse()); + let x_inverse = + Value::from(foreign_call_wait_info.inputs[0].unwrap_value().to_field().inverse()); acvm.resolve_pending_foreign_call(x_inverse.into()); // After filling data request, continue solving @@ -412,7 +416,8 @@ fn oracle_dependent_execution() { assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input"); // Resolve Brillig foreign call - let y_inverse = Value::from(foreign_call_wait_info.inputs[0][0].to_field().inverse()); + let y_inverse = + Value::from(foreign_call_wait_info.inputs[0].unwrap_value().to_field().inverse()); acvm.resolve_pending_foreign_call(y_inverse.into()); // We've resolved all the brillig foreign calls so we should be able to complete execution now. diff --git a/acvm-repo/acvm_js/src/foreign_call/inputs.rs b/acvm-repo/acvm_js/src/foreign_call/inputs.rs index 1238f9b1cb2..728fc2d3d2f 100644 --- a/acvm-repo/acvm_js/src/foreign_call/inputs.rs +++ b/acvm-repo/acvm_js/src/foreign_call/inputs.rs @@ -1,12 +1,14 @@ -use acvm::brillig_vm::brillig::Value; +use acvm::brillig_vm::brillig::ForeignCallParam; use crate::js_witness_map::field_element_to_js_string; -pub(super) fn encode_foreign_call_inputs(foreign_call_inputs: &[Vec]) -> js_sys::Array { +pub(super) fn encode_foreign_call_inputs( + foreign_call_inputs: &[ForeignCallParam], +) -> js_sys::Array { let inputs = js_sys::Array::default(); for input in foreign_call_inputs { let input_array = js_sys::Array::default(); - for value in input { + for value in input.values() { let hex_js_string = field_element_to_js_string(&value.to_field()); input_array.push(&hex_js_string); } diff --git a/acvm-repo/acvm_js/src/foreign_call/outputs.rs b/acvm-repo/acvm_js/src/foreign_call/outputs.rs index eea686b9369..630b1afb6fd 100644 --- a/acvm-repo/acvm_js/src/foreign_call/outputs.rs +++ b/acvm-repo/acvm_js/src/foreign_call/outputs.rs @@ -1,20 +1,20 @@ -use acvm::brillig_vm::brillig::{ForeignCallOutput, ForeignCallResult, Value}; +use acvm::brillig_vm::brillig::{ForeignCallParam, ForeignCallResult, Value}; use wasm_bindgen::JsValue; use crate::js_witness_map::js_value_to_field_element; -fn decode_foreign_call_output(output: JsValue) -> Result { +fn decode_foreign_call_output(output: JsValue) -> Result { if output.is_string() { let value = Value::from(js_value_to_field_element(output)?); - Ok(ForeignCallOutput::Single(value)) + Ok(ForeignCallParam::Single(value)) } else if output.is_array() { let output = js_sys::Array::from(&output); let mut values: Vec = Vec::with_capacity(output.length() as usize); for elem in output.iter() { - values.push(Value::from(js_value_to_field_element(elem)?)) + values.push(Value::from(js_value_to_field_element(elem)?)); } - Ok(ForeignCallOutput::Array(values)) + Ok(ForeignCallParam::Array(values)) } else { return Err("Non-string-or-array element in foreign_call_handler return".into()); } @@ -23,7 +23,7 @@ fn decode_foreign_call_output(output: JsValue) -> Result Result { - let mut values: Vec = Vec::with_capacity(js_array.length() as usize); + let mut values: Vec = Vec::with_capacity(js_array.length() as usize); for elem in js_array.iter() { values.push(decode_foreign_call_output(elem)?); } diff --git a/acvm-repo/brillig/src/foreign_call.rs b/acvm-repo/brillig/src/foreign_call.rs index 3c041d00d02..1359d7d604d 100644 --- a/acvm-repo/brillig/src/foreign_call.rs +++ b/acvm-repo/brillig/src/foreign_call.rs @@ -3,32 +3,60 @@ use serde::{Deserialize, Serialize}; /// Single output of a [foreign call][crate::Opcode::ForeignCall]. #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)] -pub enum ForeignCallOutput { +pub enum ForeignCallParam { Single(Value), Array(Vec), } +impl From for ForeignCallParam { + fn from(value: Value) -> Self { + ForeignCallParam::Single(value) + } +} + +impl From> for ForeignCallParam { + fn from(values: Vec) -> Self { + ForeignCallParam::Array(values) + } +} + +impl ForeignCallParam { + pub fn values(&self) -> Vec { + match self { + ForeignCallParam::Single(value) => vec![*value], + ForeignCallParam::Array(values) => values.clone(), + } + } + + pub fn unwrap_value(&self) -> Value { + match self { + ForeignCallParam::Single(value) => *value, + ForeignCallParam::Array(_) => panic!("Expected single value, found array"), + } + } +} + /// Represents the full output of a [foreign call][crate::Opcode::ForeignCall]. #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)] pub struct ForeignCallResult { /// Resolved output values of the foreign call. - pub values: Vec, + pub values: Vec, } impl From for ForeignCallResult { fn from(value: Value) -> Self { - ForeignCallResult { values: vec![ForeignCallOutput::Single(value)] } + ForeignCallResult { values: vec![value.into()] } } } impl From> for ForeignCallResult { fn from(values: Vec) -> Self { - ForeignCallResult { values: vec![ForeignCallOutput::Array(values)] } + ForeignCallResult { values: vec![values.into()] } } } -impl From> for ForeignCallResult { - fn from(values: Vec) -> Self { +impl From> for ForeignCallResult { + fn from(values: Vec) -> Self { ForeignCallResult { values } } } diff --git a/acvm-repo/brillig/src/lib.rs b/acvm-repo/brillig/src/lib.rs index 042caab99c9..422590d1f49 100644 --- a/acvm-repo/brillig/src/lib.rs +++ b/acvm-repo/brillig/src/lib.rs @@ -16,7 +16,7 @@ mod opcodes; mod value; pub use black_box::BlackBoxOp; -pub use foreign_call::{ForeignCallOutput, ForeignCallResult}; +pub use foreign_call::{ForeignCallParam, ForeignCallResult}; pub use opcodes::{ BinaryFieldOp, BinaryIntOp, HeapArray, HeapVector, RegisterIndex, RegisterOrMemory, }; diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index 8628a53a27e..e2c8ae6521a 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -12,8 +12,8 @@ //! [acvm]: https://crates.io/crates/acvm use acir::brillig::{ - BinaryFieldOp, BinaryIntOp, ForeignCallOutput, ForeignCallResult, HeapArray, HeapVector, - Opcode, RegisterIndex, RegisterOrMemory, Value, + BinaryFieldOp, BinaryIntOp, ForeignCallParam, ForeignCallResult, HeapArray, HeapVector, Opcode, + RegisterIndex, RegisterOrMemory, Value, }; use acir::FieldElement; // Re-export `brillig`. @@ -54,7 +54,7 @@ pub enum VMStatus { function: String, /// Input values /// Each input is a list of values as an input can be either a single value or a memory pointer - inputs: Vec>, + inputs: Vec, }, } @@ -119,7 +119,11 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { /// Sets the status of the VM to `ForeignCallWait`. /// Indicating that the VM is now waiting for a foreign call to be resolved. - fn wait_for_foreign_call(&mut self, function: String, inputs: Vec>) -> VMStatus { + fn wait_for_foreign_call( + &mut self, + function: String, + inputs: Vec, + ) -> VMStatus { self.status(VMStatus::ForeignCallWait { function, inputs }) } @@ -210,7 +214,7 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { for (destination, output) in destinations.iter().zip(values) { match destination { RegisterOrMemory::RegisterIndex(value_index) => match output { - ForeignCallOutput::Single(value) => { + ForeignCallParam::Single(value) => { self.registers.set(*value_index, *value); } _ => unreachable!( @@ -219,7 +223,7 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { }, RegisterOrMemory::HeapArray(HeapArray { pointer: pointer_index, size }) => { match output { - ForeignCallOutput::Array(values) => { + ForeignCallParam::Array(values) => { if values.len() != *size { invalid_foreign_call_result = true; break; @@ -236,7 +240,7 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { } RegisterOrMemory::HeapVector(HeapVector { pointer: pointer_index, size: size_index }) => { match output { - ForeignCallOutput::Array(values) => { + ForeignCallParam::Array(values) => { // Set our size in the size register self.registers.set(*size_index, Value::from(values.len())); // Convert the destination pointer to a usize @@ -330,14 +334,12 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { self.status.clone() } - fn get_register_value_or_memory_values(&self, input: RegisterOrMemory) -> Vec { + fn get_register_value_or_memory_values(&self, input: RegisterOrMemory) -> ForeignCallParam { match input { - RegisterOrMemory::RegisterIndex(value_index) => { - vec![self.registers.get(value_index)] - } + RegisterOrMemory::RegisterIndex(value_index) => self.registers.get(value_index).into(), RegisterOrMemory::HeapArray(HeapArray { pointer: pointer_index, size }) => { let start = self.registers.get(pointer_index); - self.memory.read_slice(start.to_usize(), size).to_vec() + self.memory.read_slice(start.to_usize(), size).to_vec().into() } RegisterOrMemory::HeapVector(HeapVector { pointer: pointer_index, @@ -345,7 +347,7 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { }) => { let start = self.registers.get(pointer_index); let size = self.registers.get(size_index); - self.memory.read_slice(start.to_usize(), size.to_usize()).to_vec() + self.memory.read_slice(start.to_usize(), size.to_usize()).to_vec().into() } } } @@ -922,7 +924,7 @@ mod tests { vm.status, VMStatus::ForeignCallWait { function: "double".into(), - inputs: vec![vec![Value::from(5u128)]] + inputs: vec![Value::from(5u128).into()] } ); @@ -983,7 +985,7 @@ mod tests { vm.status, VMStatus::ForeignCallWait { function: "matrix_2x2_transpose".into(), - inputs: vec![initial_matrix] + inputs: vec![initial_matrix.into()] } ); @@ -1056,13 +1058,13 @@ mod tests { vm.status, VMStatus::ForeignCallWait { function: "string_double".into(), - inputs: vec![input_string.clone()] + inputs: vec![input_string.clone().into()] } ); // Push result we're waiting for vm.foreign_call_results.push(ForeignCallResult { - values: vec![ForeignCallOutput::Array(output_string.clone())], + values: vec![ForeignCallParam::Array(output_string.clone())], }); // Resume VM @@ -1118,7 +1120,7 @@ mod tests { vm.status, VMStatus::ForeignCallWait { function: "matrix_2x2_transpose".into(), - inputs: vec![initial_matrix.clone()] + inputs: vec![initial_matrix.clone().into()] } ); @@ -1203,7 +1205,7 @@ mod tests { vm.status, VMStatus::ForeignCallWait { function: "matrix_2x2_transpose".into(), - inputs: vec![matrix_a, matrix_b] + inputs: vec![matrix_a.into(), matrix_b.into()] } ); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index d1ce1b551b2..a5647c38b60 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -1009,7 +1009,7 @@ pub(crate) mod tests { use std::vec; use acvm::acir::brillig::{ - BinaryIntOp, ForeignCallOutput, ForeignCallResult, HeapVector, RegisterIndex, + BinaryIntOp, ForeignCallParam, ForeignCallResult, HeapVector, RegisterIndex, RegisterOrMemory, Value, }; use acvm::brillig_vm::{Registers, VMStatus, VM}; @@ -1133,7 +1133,7 @@ pub(crate) mod tests { Registers { inner: vec![] }, vec![], bytecode, - vec![ForeignCallResult { values: vec![ForeignCallOutput::Array(number_sequence)] }], + vec![ForeignCallResult { values: vec![ForeignCallParam::Array(number_sequence)] }], &DummyBlackBoxSolver, ); let status = vm.process_opcodes(); diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 46f59f665a0..45d8926fd83 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -1,6 +1,6 @@ use std::{collections::BTreeMap, str}; -use acvm::{brillig_vm::brillig::Value, FieldElement}; +use acvm::{brillig_vm::brillig::ForeignCallParam, FieldElement}; use iter_extended::vecmap; use regex::{Captures, Regex}; use serde::{Deserialize, Serialize}; @@ -75,14 +75,14 @@ pub enum ForeignCallError { ParsingError(#[from] serde_json::Error), } -impl TryFrom<&[Vec]> for PrintableValueDisplay { +impl TryFrom<&[ForeignCallParam]> for PrintableValueDisplay { type Error = ForeignCallError; - fn try_from(foreign_call_inputs: &[Vec]) -> Result { + fn try_from(foreign_call_inputs: &[ForeignCallParam]) -> Result { let (is_fmt_str, foreign_call_inputs) = foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; - if is_fmt_str[0].to_field().is_one() { + if is_fmt_str.unwrap_value().to_field().is_one() { convert_fmt_string_inputs(foreign_call_inputs) } else { convert_string_inputs(foreign_call_inputs) @@ -91,7 +91,7 @@ impl TryFrom<&[Vec]> for PrintableValueDisplay { } fn convert_string_inputs( - foreign_call_inputs: &[Vec], + foreign_call_inputs: &[ForeignCallParam], ) -> Result { // Fetch the PrintableType from the foreign call input // The remaining input values should hold what is to be printed @@ -101,7 +101,7 @@ fn convert_string_inputs( // We must use a flat map here as each value in a struct will be in a separate input value let mut input_values_as_fields = - input_values.iter().flat_map(|values| vecmap(values, |value| value.to_field())); + input_values.iter().flat_map(|param| vecmap(param.values(), |value| value.to_field())); let value = decode_value(&mut input_values_as_fields, &printable_type); @@ -109,12 +109,12 @@ fn convert_string_inputs( } fn convert_fmt_string_inputs( - foreign_call_inputs: &[Vec], + foreign_call_inputs: &[ForeignCallParam], ) -> Result { - let (message_as_values, input_and_printable_values) = + let (message, input_and_printable_values) = foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - let message_as_fields = vecmap(message_as_values, |value| value.to_field()); + let message_as_fields = vecmap(message.values(), |value| value.to_field()); let message_as_string = decode_string_value(&message_as_fields); let (num_values, input_and_printable_values) = input_and_printable_values @@ -122,7 +122,7 @@ fn convert_fmt_string_inputs( .ok_or(ForeignCallError::MissingForeignCallInputs)?; let mut output = Vec::new(); - let num_values = num_values[0].to_field().to_u128() as usize; + let num_values = num_values.unwrap_value().to_field().to_u128() as usize; for (i, printable_value) in input_and_printable_values .iter() @@ -134,7 +134,7 @@ fn convert_fmt_string_inputs( let mut input_values_as_fields = input_and_printable_values[i..(i + type_size)] .iter() - .flat_map(|values| vecmap(values, |value| value.to_field())); + .flat_map(|param| vecmap(param.values(), |value| value.to_field())); let value = decode_value(&mut input_values_as_fields, &printable_type); @@ -145,9 +145,9 @@ fn convert_fmt_string_inputs( } fn fetch_printable_type( - printable_type_as_values: &[Value], + printable_type: &ForeignCallParam, ) -> Result { - let printable_type_as_fields = vecmap(printable_type_as_values, |value| value.to_field()); + let printable_type_as_fields = vecmap(printable_type.values(), |value| value.to_field()); let printable_type_as_string = decode_string_value(&printable_type_as_fields); let printable_type: PrintableType = serde_json::from_str(&printable_type_as_string)?; @@ -307,7 +307,7 @@ fn decode_value( } } -fn decode_string_value(field_elements: &[FieldElement]) -> String { +pub fn decode_string_value(field_elements: &[FieldElement]) -> String { // TODO: Replace with `into` when Char is supported let string_as_slice = vecmap(field_elements, |e| { let mut field_as_bytes = e.to_be_bytes(); diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index db8cdceb20a..68962978fed 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -1,5 +1,6 @@ use acvm::{ - acir::brillig::{ForeignCallOutput, ForeignCallResult, Value}, + acir::brillig::{ForeignCallResult, Value}, + brillig_vm::brillig::ForeignCallParam, pwg::ForeignCallWaitInfo, }; use iter_extended::vecmap; @@ -52,24 +53,26 @@ impl ForeignCall { Ok(ForeignCallResult { values: vec![] }) } Some(ForeignCall::Sequence) => { - let sequence_length: u128 = foreign_call.inputs[0][0].to_field().to_u128(); + let sequence_length: u128 = + foreign_call.inputs[0].unwrap_value().to_field().to_u128(); let sequence = vecmap(0..sequence_length, Value::from); Ok(ForeignCallResult { values: vec![ - ForeignCallOutput::Single(sequence_length.into()), - ForeignCallOutput::Array(sequence), + ForeignCallParam::Single(sequence_length.into()), + ForeignCallParam::Array(sequence), ], }) } Some(ForeignCall::ReverseSequence) => { - let sequence_length: u128 = foreign_call.inputs[0][0].to_field().to_u128(); + let sequence_length: u128 = + foreign_call.inputs[0].unwrap_value().to_field().to_u128(); let sequence = vecmap((0..sequence_length).rev(), Value::from); Ok(ForeignCallResult { values: vec![ - ForeignCallOutput::Single(sequence_length.into()), - ForeignCallOutput::Array(sequence), + ForeignCallParam::Single(sequence_length.into()), + ForeignCallParam::Array(sequence), ], }) } @@ -77,7 +80,7 @@ impl ForeignCall { } } - fn execute_println(foreign_call_inputs: &[Vec]) -> Result<(), NargoError> { + fn execute_println(foreign_call_inputs: &[ForeignCallParam]) -> Result<(), NargoError> { let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; println!("{display_values}"); Ok(()) From 5a9ceffc139080d69050180af4f188a52bf1c60e Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Oct 2023 12:15:19 +0000 Subject: [PATCH 2/4] feat: Oracle mocker for nargo test --- .../noirc_frontend/src/hir/def_map/mod.rs | 6 +- compiler/noirc_frontend/src/lexer/token.rs | 4 + noir_stdlib/src/lib.nr | 1 + noir_stdlib/src/test.nr | 45 ++++++ tooling/nargo/src/ops/execute.rs | 7 +- tooling/nargo/src/ops/foreign_calls.rs | 142 +++++++++++++++++- .../execution_success/mock_oracle/Nargo.toml | 7 + .../execution_success/mock_oracle/Prover.toml | 2 + .../execution_success/mock_oracle/src/main.nr | 30 ++++ 9 files changed, 234 insertions(+), 10 deletions(-) create mode 100644 noir_stdlib/src/test.nr create mode 100644 tooling/nargo_cli/tests/execution_success/mock_oracle/Nargo.toml create mode 100644 tooling/nargo_cli/tests/execution_success/mock_oracle/Prover.toml create mode 100644 tooling/nargo_cli/tests/execution_success/mock_oracle/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 27f757074f6..ba2f4fdaae3 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -174,9 +174,9 @@ impl CrateDefMap { .value_definitions() .filter_map(|id| { id.as_function().map(|function_id| { - let is_entry_point = !interner - .function_attributes(&function_id) - .has_contract_library_method(); + let attributes = interner.function_attributes(&function_id); + let is_entry_point = !attributes.has_contract_library_method() + && !attributes.is_test_function(); ContractFunctionMeta { function_id, is_entry_point } }) }) diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 44ef83d44a7..ad81b163801 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -384,6 +384,10 @@ impl Attributes { .any(|attribute| attribute == &SecondaryAttribute::ContractLibraryMethod) } + pub fn is_test_function(&self) -> bool { + matches!(self.function, Some(FunctionAttribute::Test(_))) + } + /// Returns note if a deprecated secondary attribute is found pub fn get_deprecated_note(&self) -> Option> { self.secondary.iter().find_map(|attr| match attr { diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index 428eb77aa47..26cf7a225ee 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -18,6 +18,7 @@ mod collections; mod compat; mod option; mod string; +mod test; // Oracle calls are required to be wrapped in an unconstrained function // Thus, the only argument to the `println` oracle is expected to always be an ident diff --git a/noir_stdlib/src/test.nr b/noir_stdlib/src/test.nr new file mode 100644 index 00000000000..18446a4ef85 --- /dev/null +++ b/noir_stdlib/src/test.nr @@ -0,0 +1,45 @@ +#[oracle(create_mock)] +unconstrained fn create_mock_oracle(_name: str) -> Field {} + +#[oracle(set_mock_params)] +unconstrained fn set_mock_params_oracle

(_id: Field, _params: P) {} + +#[oracle(set_mock_returns)] +unconstrained fn set_mock_returns_oracle(_id: Field, _returns: R) {} + +#[oracle(set_mock_times)] +unconstrained fn set_mock_times_oracle(_id: Field, _times: u64) {} + +#[oracle(clear_mock)] +unconstrained fn clear_mock_oracle(_id: Field) {} + +struct OracleMock { + id: Field, +} + +impl OracleMock { + unconstrained pub fn mock(name: str) -> Self { + Self { + id: create_mock_oracle(name), + } + } + + unconstrained pub fn with_params

(self, params: P) -> Self { + set_mock_params_oracle(self.id, params); + self + } + + unconstrained pub fn returns(self, returns: R) -> Self { + set_mock_returns_oracle(self.id, returns); + self + } + + unconstrained pub fn times(self, times: u64) -> Self { + set_mock_times_oracle(self.id, times); + self + } + + unconstrained pub fn clear(self) { + clear_mock_oracle(self.id); + } +} diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index 33f41ebe819..f64c92f1b3d 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -5,7 +5,7 @@ use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; use crate::errors::ExecutionError; use crate::NargoError; -use super::foreign_calls::ForeignCall; +use super::foreign_calls::ForeignCallExecutor; pub fn execute_circuit( blackbox_solver: &B, @@ -24,6 +24,8 @@ pub fn execute_circuit( .map(|(_, message)| message.clone()) }; + let mut foreign_call_executor = ForeignCallExecutor::default(); + loop { let solver_status = acvm.solve(); @@ -57,7 +59,8 @@ pub fn execute_circuit( })); } ACVMStatus::RequiresForeignCall(foreign_call) => { - let foreign_call_result = ForeignCall::execute(&foreign_call, show_output)?; + let foreign_call_result = + foreign_call_executor.execute(&foreign_call, show_output)?; acvm.resolve_pending_foreign_call(foreign_call_result); } } diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 68962978fed..f80f01ac0d8 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -1,10 +1,9 @@ use acvm::{ - acir::brillig::{ForeignCallResult, Value}, - brillig_vm::brillig::ForeignCallParam, + acir::brillig::{ForeignCallParam, ForeignCallResult, Value}, pwg::ForeignCallWaitInfo, }; use iter_extended::vecmap; -use noirc_printable_type::PrintableValueDisplay; +use noirc_printable_type::{decode_string_value, ForeignCallError, PrintableValueDisplay}; use crate::NargoError; @@ -14,6 +13,11 @@ pub(crate) enum ForeignCall { Println, Sequence, ReverseSequence, + CreateMock, + SetMockParams, + SetMockReturns, + SetMockTimes, + ClearMock, } impl std::fmt::Display for ForeignCall { @@ -28,6 +32,11 @@ impl ForeignCall { ForeignCall::Println => "println", ForeignCall::Sequence => "get_number_sequence", ForeignCall::ReverseSequence => "get_reverse_number_sequence", + ForeignCall::CreateMock => "create_mock", + ForeignCall::SetMockParams => "set_mock_params", + ForeignCall::SetMockReturns => "set_mock_returns", + ForeignCall::SetMockTimes => "set_mock_times", + ForeignCall::ClearMock => "clear_mock", } } @@ -36,16 +45,57 @@ impl ForeignCall { "println" => Some(ForeignCall::Println), "get_number_sequence" => Some(ForeignCall::Sequence), "get_reverse_number_sequence" => Some(ForeignCall::ReverseSequence), + "create_mock" => Some(ForeignCall::CreateMock), + "set_mock_params" => Some(ForeignCall::SetMockParams), + "set_mock_returns" => Some(ForeignCall::SetMockReturns), + "set_mock_times" => Some(ForeignCall::SetMockTimes), + "clear_mock" => Some(ForeignCall::ClearMock), _ => None, } } +} + +#[derive(Debug, PartialEq, Eq, Clone)] +struct MockedCall { + id: usize, + name: String, + params: Option>, + result: ForeignCallResult, + times_left: Option, +} + +impl MockedCall { + fn new(id: usize, name: String) -> Self { + Self { + id, + name, + params: None, + result: ForeignCallResult { values: vec![] }, + times_left: None, + } + } +} + +impl MockedCall { + fn matches(&self, name: &str, params: &Vec) -> bool { + self.name == name && (self.params.is_none() || self.params.as_ref() == Some(params)) + } +} +#[derive(Debug, Default)] +pub(crate) struct ForeignCallExecutor { + last_mock_id: usize, + mocked_responses: Vec, +} + +impl ForeignCallExecutor { pub(crate) fn execute( + &mut self, foreign_call: &ForeignCallWaitInfo, show_output: bool, ) -> Result { let foreign_call_name = foreign_call.function.as_str(); - match Self::lookup(foreign_call_name) { + match ForeignCall::lookup(foreign_call_name) { Some(ForeignCall::Println) => { if show_output { Self::execute_println(&foreign_call.inputs)?; @@ -76,10 +126,92 @@ impl ForeignCall { ], }) } - None => panic!("unexpected foreign call {foreign_call_name:?}"), + Some(ForeignCall::CreateMock) => { + let mock_oracle_name = Self::parse_string(&foreign_call.inputs[0]); + assert!(ForeignCall::lookup(&mock_oracle_name).is_none()); + let id = self.last_mock_id; + self.mocked_responses.push(MockedCall::new(id, mock_oracle_name)); + self.last_mock_id += 1; + + Ok(ForeignCallResult { values: vec![Value::from(id).into()] }) + } + Some(ForeignCall::SetMockParams) => { + let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; + self.find_mock_by_id(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)) + .params = Some(params.to_vec()); + + Ok(ForeignCallResult { values: vec![] }) + } + Some(ForeignCall::SetMockReturns) => { + let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; + self.find_mock_by_id(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)) + .result = ForeignCallResult { values: params.to_vec() }; + + Ok(ForeignCallResult { values: vec![] }) + } + Some(ForeignCall::SetMockTimes) => { + let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; + let times = params[0] + .unwrap_value() + .to_field() + .try_to_u64() + .expect("Invalid bit size of times"); + + self.find_mock_by_id(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)) + .times_left = Some(times); + + Ok(ForeignCallResult { values: vec![] }) + } + Some(ForeignCall::ClearMock) => { + let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; + self.mocked_responses.retain(|response| response.id != id); + Ok(ForeignCallResult { values: vec![] }) + } + None => { + let response_position = self + .mocked_responses + .iter() + .position(|response| response.matches(foreign_call_name, &foreign_call.inputs)) + .unwrap_or_else(|| panic!("Unknown foreign call {}", foreign_call_name)); + + let mock = self + .mocked_responses + .get_mut(response_position) + .expect("Invalid position of mocked response"); + let result = mock.result.values.clone(); + + if let Some(times_left) = &mut mock.times_left { + *times_left -= 1; + if *times_left == 0 { + self.mocked_responses.remove(response_position); + } + } + + Ok(ForeignCallResult { values: result }) + } } } + fn extract_mock_id( + foreign_call_inputs: &[ForeignCallParam], + ) -> Result<(usize, &[ForeignCallParam]), ForeignCallError> { + let (id, params) = + foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; + Ok((id.unwrap_value().to_usize(), params)) + } + + fn find_mock_by_id(&mut self, id: usize) -> Option<&mut MockedCall> { + self.mocked_responses.iter_mut().find(|response| response.id == id) + } + + fn parse_string(param: &ForeignCallParam) -> String { + let fields: Vec<_> = param.values().into_iter().map(|value| value.to_field()).collect(); + decode_string_value(&fields) + } + fn execute_println(foreign_call_inputs: &[ForeignCallParam]) -> Result<(), NargoError> { let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; println!("{display_values}"); diff --git a/tooling/nargo_cli/tests/execution_success/mock_oracle/Nargo.toml b/tooling/nargo_cli/tests/execution_success/mock_oracle/Nargo.toml new file mode 100644 index 00000000000..f626c2967cc --- /dev/null +++ b/tooling/nargo_cli/tests/execution_success/mock_oracle/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "mock_oracle" +type = "bin" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/tooling/nargo_cli/tests/execution_success/mock_oracle/Prover.toml b/tooling/nargo_cli/tests/execution_success/mock_oracle/Prover.toml new file mode 100644 index 00000000000..2b26a4ce471 --- /dev/null +++ b/tooling/nargo_cli/tests/execution_success/mock_oracle/Prover.toml @@ -0,0 +1,2 @@ +x = "10" + diff --git a/tooling/nargo_cli/tests/execution_success/mock_oracle/src/main.nr b/tooling/nargo_cli/tests/execution_success/mock_oracle/src/main.nr new file mode 100644 index 00000000000..405f34a4a70 --- /dev/null +++ b/tooling/nargo_cli/tests/execution_success/mock_oracle/src/main.nr @@ -0,0 +1,30 @@ +use dep::std::test::OracleMock; + +struct Point { + x: Field, + y: Field, +} + +#[oracle(foo)] +unconstrained fn foo_oracle(_point: Point, _array: [Field; 4]) -> Field {} + +unconstrained fn main() { + let array = [1,2,3,4]; + let another_array = [4,3,2,1]; + let point = Point { + x: 14, + y: 27, + }; + + OracleMock::mock("foo").returns(42).times(1); + let mock = OracleMock::mock("foo").returns(0); + assert_eq(42, foo_oracle(point, array)); + assert_eq(0, foo_oracle(point, array)); + mock.clear(); + + OracleMock::mock("foo").with_params((point, array)).returns(10); + OracleMock::mock("foo").with_params((point, another_array)).returns(20); + assert_eq(10, foo_oracle(point, array)); + assert_eq(20, foo_oracle(point, another_array)); +} + From 4822ddf3782f9ce8a1a65827a9c7b316e959afea Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Oct 2023 14:37:14 +0000 Subject: [PATCH 3/4] docs: Improve doc comments for oracle mocker --- tooling/nargo/src/ops/foreign_calls.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index f80f01ac0d8..63aa6743394 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -55,12 +55,18 @@ impl ForeignCall { } } +/// This struct represents an oracle mock. It can be used for testing programs that use oracles. #[derive(Debug, PartialEq, Eq, Clone)] struct MockedCall { + /// The id of the mock, used to update or remove it id: usize, + /// The oracle it's mocking name: String, + /// Optionally match the parameters params: Option>, + /// The result to return when this mock is called result: ForeignCallResult, + /// How many times should this mock be called before it is removed times_left: Option, } @@ -84,7 +90,9 @@ impl MockedCall { #[derive(Debug, Default)] pub(crate) struct ForeignCallExecutor { + /// Mocks have unique ids used to identify them from noir, allowing to update or remove them. last_mock_id: usize, + /// The registered mocks mocked_responses: Vec, } From c812ba91929af4942bce2ddf89829fcc869612e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Mon, 2 Oct 2023 18:32:03 +0200 Subject: [PATCH 4/4] Update tooling/nargo/src/ops/foreign_calls.rs Co-authored-by: Maxim Vezenov --- tooling/nargo/src/ops/foreign_calls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 63aa6743394..e44ab1732c9 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -90,7 +90,7 @@ impl MockedCall { #[derive(Debug, Default)] pub(crate) struct ForeignCallExecutor { - /// Mocks have unique ids used to identify them from noir, allowing to update or remove them. + /// Mocks have unique ids used to identify them in Noir, allowing to update or remove them. last_mock_id: usize, /// The registered mocks mocked_responses: Vec,