From 582299cf88db167856dc1e51584f79c35558f02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 11 Apr 2024 19:08:46 +0000 Subject: [PATCH 1/3] Move oracle tests --- .../execution_success/mock_oracle/Prover.toml | 2 - .../execution_success/mock_oracle/src/main.nr | 27 ----- .../mock_oracle/Nargo.toml | 3 +- .../noir_test_success/mock_oracle/Prover.toml | 0 .../noir_test_success/mock_oracle/src/main.nr | 103 ++++++++++++++++++ 5 files changed, 105 insertions(+), 30 deletions(-) delete mode 100644 test_programs/execution_success/mock_oracle/Prover.toml delete mode 100644 test_programs/execution_success/mock_oracle/src/main.nr rename test_programs/{execution_success => noir_test_success}/mock_oracle/Nargo.toml (57%) create mode 100644 test_programs/noir_test_success/mock_oracle/Prover.toml create mode 100644 test_programs/noir_test_success/mock_oracle/src/main.nr diff --git a/test_programs/execution_success/mock_oracle/Prover.toml b/test_programs/execution_success/mock_oracle/Prover.toml deleted file mode 100644 index 2b26a4ce471..00000000000 --- a/test_programs/execution_success/mock_oracle/Prover.toml +++ /dev/null @@ -1,2 +0,0 @@ -x = "10" - diff --git a/test_programs/execution_success/mock_oracle/src/main.nr b/test_programs/execution_success/mock_oracle/src/main.nr deleted file mode 100644 index 90fca7993cc..00000000000 --- a/test_programs/execution_success/mock_oracle/src/main.nr +++ /dev/null @@ -1,27 +0,0 @@ -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)); -} - diff --git a/test_programs/execution_success/mock_oracle/Nargo.toml b/test_programs/noir_test_success/mock_oracle/Nargo.toml similarity index 57% rename from test_programs/execution_success/mock_oracle/Nargo.toml rename to test_programs/noir_test_success/mock_oracle/Nargo.toml index b2916487e8c..428e965899c 100644 --- a/test_programs/execution_success/mock_oracle/Nargo.toml +++ b/test_programs/noir_test_success/mock_oracle/Nargo.toml @@ -2,5 +2,6 @@ name = "mock_oracle" type = "bin" authors = [""] +compiler_version = ">=0.23.0" -[dependencies] +[dependencies] \ No newline at end of file diff --git a/test_programs/noir_test_success/mock_oracle/Prover.toml b/test_programs/noir_test_success/mock_oracle/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/noir_test_success/mock_oracle/src/main.nr b/test_programs/noir_test_success/mock_oracle/src/main.nr new file mode 100644 index 00000000000..75843be75af --- /dev/null +++ b/test_programs/noir_test_success/mock_oracle/src/main.nr @@ -0,0 +1,103 @@ +use dep::std::test::OracleMock; + +struct Point { + x: Field, + y: Field, +} + +#[oracle(void_field)] +unconstrained fn void_field_oracle() -> Field {} + +unconstrained fn void_field() -> Field { + void_field_oracle() +} + +#[oracle(field_field)] +unconstrained fn field_field_oracle(_x: Field) -> Field {} + +unconstrained fn field_field(x: Field) -> Field { + field_field_oracle(x) +} + +#[oracle(struct_field)] +unconstrained fn struct_field_oracle(_point: Point, _array: [Field; 4]) -> Field {} + +unconstrained fn struct_field(point: Point, array: [Field; 4]) -> Field { + struct_field_oracle(point, array) +} + +#[test(should_fail)] +fn test_mock_no_returns() { + OracleMock::mock("void_field"); + void_field(); // Some return value must be set +} + +#[test] +fn test_mock() { + OracleMock::mock("void_field").returns(10); + assert_eq(void_field(), 10); +} + +#[test] +fn test_multiple_mock() { + let first_mock = OracleMock::mock("void_field").returns(10); + OracleMock::mock("void_field").returns(42); + + // The mocks are searched for in creation order, so the first one prevents the second from being called. + assert_eq(void_field(), 10); + + first_mock.clear(); + assert_eq(void_field(), 42); +} + +#[test] +fn test_multiple_mock_times() { + OracleMock::mock("void_field").returns(10).times(2); + OracleMock::mock("void_field").returns(42); + + assert_eq(void_field(), 10); + assert_eq(void_field(), 10); + assert_eq(void_field(), 42); +} + +#[test] +fn test_mock_with_params() { + OracleMock::mock("field_field").with_params((5,)).returns(10); + assert_eq(field_field(5), 10); +} + +#[test] +fn test_multiple_mock_with_params() { + OracleMock::mock("field_field").with_params((5,)).returns(10); + OracleMock::mock("field_field").with_params((7,)).returns(14); + + assert_eq(field_field(5), 10); + assert_eq(field_field(7), 14); +} + +#[test] +fn test_mock_struct_field() { + // Combination of simpler test cases + + let array = [1, 2, 3, 4]; + let another_array = [4, 3, 2, 1]; + let point = Point { x: 14, y: 27 }; + + OracleMock::mock("struct_field").returns(42).times(2); + let timeless_mock = OracleMock::mock("struct_field").returns(0); + + assert_eq(42, struct_field(point, array)); + assert_eq(42, struct_field(point, array)); + // The times(2) mock is now cleared + + assert_eq(0, struct_field(point, array)); + + // We clear the mock with no times() to allow other mocks to be callable + timeless_mock.clear(); + + OracleMock::mock("struct_field").with_params((point, array)).returns(10); + OracleMock::mock("struct_field").with_params((point, another_array)).returns(20); + assert_eq(10, struct_field(point, array)); + assert_eq(20, struct_field(point, another_array)); +} + From 8093b1df4d68ed629e9c6531f91f631a740dbd1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 11 Apr 2024 19:27:54 +0000 Subject: [PATCH 2/3] Add get_last_params --- noir_stdlib/src/test.nr | 7 +++ .../noir_test_success/mock_oracle/src/main.nr | 27 ++++++++++ tooling/nargo/src/ops/foreign_calls.rs | 49 ++++++++++++++----- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/noir_stdlib/src/test.nr b/noir_stdlib/src/test.nr index e1c320215de..e6a7e03fefc 100644 --- a/noir_stdlib/src/test.nr +++ b/noir_stdlib/src/test.nr @@ -4,6 +4,9 @@ unconstrained fn create_mock_oracle(name: str) -> Field {} #[oracle(set_mock_params)] unconstrained fn set_mock_params_oracle

(id: Field, params: P) {} +#[oracle(get_mock_last_params)] +unconstrained fn get_mock_last_params_oracle

(id: Field) -> P {} + #[oracle(set_mock_returns)] unconstrained fn set_mock_returns_oracle(id: Field, returns: R) {} @@ -27,6 +30,10 @@ impl OracleMock { self } + unconstrained pub fn get_last_params

(self) -> P { + get_mock_last_params_oracle(self.id) + } + unconstrained pub fn returns(self, returns: R) -> Self { set_mock_returns_oracle(self.id, returns); self diff --git a/test_programs/noir_test_success/mock_oracle/src/main.nr b/test_programs/noir_test_success/mock_oracle/src/main.nr index 75843be75af..d840ffaef66 100644 --- a/test_programs/noir_test_success/mock_oracle/src/main.nr +++ b/test_programs/noir_test_success/mock_oracle/src/main.nr @@ -5,6 +5,12 @@ struct Point { y: Field, } +impl Eq for Point { + fn eq(self, other: Point) -> bool { + (self.x == other.x) & (self.y == other.y) + } +} + #[oracle(void_field)] unconstrained fn void_field_oracle() -> Field {} @@ -75,6 +81,23 @@ fn test_multiple_mock_with_params() { assert_eq(field_field(7), 14); } +#[test] +fn test_mock_last_params() { + let mock = OracleMock::mock("field_field").returns(10); + assert_eq(field_field(5), 10); + + assert_eq(mock.get_last_params(), 5); +} + +#[test] +fn test_mock_last_params_many_calls() { + let mock = OracleMock::mock("field_field").returns(10); + assert_eq(field_field(5), 10); + assert_eq(field_field(7), 10); + + assert_eq(mock.get_last_params(), 7); +} + #[test] fn test_mock_struct_field() { // Combination of simpler test cases @@ -92,6 +115,10 @@ fn test_mock_struct_field() { assert_eq(0, struct_field(point, array)); + let last_params: (Point, [Field; 4]) = timeless_mock.get_last_params(); + assert_eq(last_params.0, point); + assert_eq(last_params.1, array); + // We clear the mock with no times() to allow other mocks to be callable timeless_mock.clear(); diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index ea67f17af2a..e0ecc4bc846 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -75,6 +75,7 @@ pub enum ForeignCall { AssertMessage, CreateMock, SetMockParams, + GetMockLastParams, SetMockReturns, SetMockTimes, ClearMock, @@ -93,6 +94,7 @@ impl ForeignCall { ForeignCall::AssertMessage => "assert_message", ForeignCall::CreateMock => "create_mock", ForeignCall::SetMockParams => "set_mock_params", + ForeignCall::GetMockLastParams => "get_mock_last_params", ForeignCall::SetMockReturns => "set_mock_returns", ForeignCall::SetMockTimes => "set_mock_times", ForeignCall::ClearMock => "clear_mock", @@ -105,6 +107,7 @@ impl ForeignCall { "assert_message" => Some(ForeignCall::AssertMessage), "create_mock" => Some(ForeignCall::CreateMock), "set_mock_params" => Some(ForeignCall::SetMockParams), + "get_mock_last_params" => Some(ForeignCall::GetMockLastParams), "set_mock_returns" => Some(ForeignCall::SetMockReturns), "set_mock_times" => Some(ForeignCall::SetMockTimes), "clear_mock" => Some(ForeignCall::ClearMock), @@ -122,6 +125,8 @@ struct MockedCall { name: String, /// Optionally match the parameters params: Option>, + /// The parameters with which the mock was last called + last_called_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 @@ -134,6 +139,7 @@ impl MockedCall { id, name, params: None, + last_called_params: None, result: ForeignCallResult { values: vec![] }, times_left: None, } @@ -185,7 +191,11 @@ impl DefaultForeignCallExecutor { Ok((id, params)) } - fn find_mock_by_id(&mut self, id: usize) -> Option<&mut MockedCall> { + fn find_mock_by_id(&mut self, id: usize) -> Option<&MockedCall> { + self.mocked_responses.iter().find(|response| response.id == id) + } + + fn find_mock_by_id_mut(&mut self, id: usize) -> Option<&mut MockedCall> { self.mocked_responses.iter_mut().find(|response| response.id == id) } @@ -250,15 +260,24 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { } Some(ForeignCall::SetMockParams) => { let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; - self.find_mock_by_id(id) + self.find_mock_by_id_mut(id) .unwrap_or_else(|| panic!("Unknown mock id {}", id)) .params = Some(params.to_vec()); Ok(ForeignCallResult::default().into()) } + Some(ForeignCall::GetMockLastParams) => { + let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; + let mock = self.find_mock_by_id(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)); + + let last_called_params = mock.last_called_params.clone().unwrap_or_else(|| panic!("Mock {} was never called", mock.name)); + + Ok(last_called_params.into()) + } Some(ForeignCall::SetMockReturns) => { let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; - self.find_mock_by_id(id) + self.find_mock_by_id_mut(id) .unwrap_or_else(|| panic!("Unknown mock id {}", id)) .result = ForeignCallResult { values: params.to_vec() }; @@ -269,7 +288,7 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { let times = params[0].unwrap_field().try_to_u64().expect("Invalid bit size of times"); - self.find_mock_by_id(id) + self.find_mock_by_id_mut(id) .unwrap_or_else(|| panic!("Unknown mock id {}", id)) .times_left = Some(times); @@ -288,14 +307,18 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { match (mock_response_position, &self.external_resolver) { (Some(response_position), _) => { - 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; + let mock = self + .mocked_responses + .get_mut(response_position) + .expect("Invalid position of mocked response"); + + + mock.last_called_params = Some(foreign_call.inputs.clone()); + + 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); } @@ -316,7 +339,7 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { Ok(parsed_response.into()) } - (None, None) => panic!("Unknown foreign call {}", foreign_call_name), + (None, None) => panic!("No mock for foreign call {}({:?})", foreign_call_name, &foreign_call.inputs), } } } From d1a69aef0d841ea10bb404972fc2ee48b23e20df Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 12 Apr 2024 12:54:42 +0100 Subject: [PATCH 3/3] chore: remove unnecessary `mut` --- tooling/nargo/src/ops/foreign_calls.rs | 37 +++++++++++++++----------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index e0ecc4bc846..bc91929e5e7 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -191,7 +191,7 @@ impl DefaultForeignCallExecutor { Ok((id, params)) } - fn find_mock_by_id(&mut self, id: usize) -> Option<&MockedCall> { + fn find_mock_by_id(&self, id: usize) -> Option<&MockedCall> { self.mocked_responses.iter().find(|response| response.id == id) } @@ -267,12 +267,15 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { Ok(ForeignCallResult::default().into()) } Some(ForeignCall::GetMockLastParams) => { - let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; - let mock = self.find_mock_by_id(id) - .unwrap_or_else(|| panic!("Unknown mock id {}", id)); + let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; + let mock = + self.find_mock_by_id(id).unwrap_or_else(|| panic!("Unknown mock id {}", id)); + + let last_called_params = mock + .last_called_params + .clone() + .unwrap_or_else(|| panic!("Mock {} was never called", mock.name)); - let last_called_params = mock.last_called_params.clone().unwrap_or_else(|| panic!("Mock {} was never called", mock.name)); - Ok(last_called_params.into()) } Some(ForeignCall::SetMockReturns) => { @@ -307,18 +310,17 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { match (mock_response_position, &self.external_resolver) { (Some(response_position), _) => { - let mock = self - .mocked_responses - .get_mut(response_position) - .expect("Invalid position of mocked response"); - + let mock = self + .mocked_responses + .get_mut(response_position) + .expect("Invalid position of mocked response"); - mock.last_called_params = Some(foreign_call.inputs.clone()); + mock.last_called_params = Some(foreign_call.inputs.clone()); - let result = mock.result.values.clone(); + let result = mock.result.values.clone(); - if let Some(times_left) = &mut mock.times_left { - *times_left -= 1; + if let Some(times_left) = &mut mock.times_left { + *times_left -= 1; if *times_left == 0 { self.mocked_responses.remove(response_position); } @@ -339,7 +341,10 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { Ok(parsed_response.into()) } - (None, None) => panic!("No mock for foreign call {}({:?})", foreign_call_name, &foreign_call.inputs), + (None, None) => panic!( + "No mock for foreign call {}({:?})", + foreign_call_name, &foreign_call.inputs + ), } } }