From 758b6b62918907c1a39f3090a77419003551745e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Fri, 6 Oct 2023 13:19:54 +0200 Subject: [PATCH] fix: ACIR optimizer should update assertion messages (#3010) --- acvm-repo/acvm/src/compiler/optimizers/mod.rs | 6 +++-- tooling/nargo/src/ops/test.rs | 27 ++++++++++--------- .../should_fail_mismatch/src/main.nr | 10 +++++-- .../should_fail_with_matches/src/main.nr | 20 +++++++++++--- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/optimizers/mod.rs b/acvm-repo/acvm/src/compiler/optimizers/mod.rs index 9c074bf6269..d04d21039f9 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/mod.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/mod.rs @@ -9,7 +9,7 @@ pub(crate) use redundant_range::RangeOptimizer; use self::unused_memory::UnusedMemoryOptimizer; -use super::AcirTransformationMap; +use super::{transform_assert_messages, AcirTransformationMap}; /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`]. pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) { @@ -36,10 +36,12 @@ pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) { // Range optimization pass let range_optimizer = RangeOptimizer::new(acir); - let (acir, acir_opcode_positions) = + let (mut acir, acir_opcode_positions) = range_optimizer.replace_redundant_ranges(acir_opcode_positions); let transformation_map = AcirTransformationMap { acir_opcode_positions }; + acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map); + (acir, transformation_map) } diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 9219807b435..31e7d62dfc9 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -47,12 +47,13 @@ fn test_status_program_compile_fail(err: RuntimeError, test_function: TestFuncti } // The test has failed compilation, extract the assertion message if present and check if it's expected. - if let RuntimeError::FailedConstraint { assert_message: Some(assert_message), .. } = &err { - let assert_message = assert_message.clone(); - check_expected_failure_message(test_function, &assert_message, Some(err.into())) + let assert_message = if let RuntimeError::FailedConstraint { assert_message, .. } = &err { + assert_message.clone() } else { - TestStatus::CompileError(err.into()) - } + None + }; + + check_expected_failure_message(test_function, assert_message, Some(err.into())) } /// The test function compiled successfully. @@ -91,15 +92,16 @@ fn test_status_program_compile_pass( }; } - let assertion_message = - circuit_execution_err.user_defined_failure_message().unwrap_or_default().to_string(); - - check_expected_failure_message(test_function, &assertion_message, diagnostic) + check_expected_failure_message( + test_function, + circuit_execution_err.user_defined_failure_message().map(|s| s.to_string()), + diagnostic, + ) } fn check_expected_failure_message( test_function: TestFunction, - failed_assertion: &str, + failed_assertion: Option, error_diagnostic: Option, ) -> TestStatus { // Extract the expected failure message, if there was one @@ -112,7 +114,8 @@ fn check_expected_failure_message( None => return TestStatus::Pass, }; - let expected_failure_message_matches = failed_assertion == expected_failure_message; + let expected_failure_message_matches = + matches!(&failed_assertion, Some(message) if message == expected_failure_message); if expected_failure_message_matches { return TestStatus::Pass; } @@ -122,7 +125,7 @@ fn check_expected_failure_message( message: format!( "\nerror: Test failed with the wrong message. \nExpected: {} \nGot: {}", test_function.failure_reason().unwrap_or_default(), - failed_assertion.trim_matches('\'') + failed_assertion.unwrap_or_default().trim_matches('\'') ), error_diagnostic, } diff --git a/tooling/nargo_cli/tests/noir_test_failure/should_fail_mismatch/src/main.nr b/tooling/nargo_cli/tests/noir_test_failure/should_fail_mismatch/src/main.nr index 2e8276e155f..b2672221c34 100644 --- a/tooling/nargo_cli/tests/noir_test_failure/should_fail_mismatch/src/main.nr +++ b/tooling/nargo_cli/tests/noir_test_failure/should_fail_mismatch/src/main.nr @@ -1,11 +1,17 @@ #[test(should_fail_with = "Not equal")] -fn test_main_1() { +fn test_different_string() { assert_eq(0, 1, "Different string"); } // The assert message has a space #[test(should_fail_with = "Not equal")] -fn test_main_1() { +fn test_with_extra_space() { assert_eq(0, 1, "Not equal "); +} + +// The assert message has a space +#[test(should_fail_with = "Not equal")] +fn test_runtime_mismatch() { + assert_eq(dep::std::hash::pedersen([27])[0], 0, "Not equal "); } \ No newline at end of file diff --git a/tooling/nargo_cli/tests/noir_test_success/should_fail_with_matches/src/main.nr b/tooling/nargo_cli/tests/noir_test_success/should_fail_with_matches/src/main.nr index a04a9dbc0f9..8ae5e56a463 100644 --- a/tooling/nargo_cli/tests/noir_test_success/should_fail_with_matches/src/main.nr +++ b/tooling/nargo_cli/tests/noir_test_success/should_fail_with_matches/src/main.nr @@ -1,5 +1,19 @@ - #[test(should_fail_with = "Not equal")] -fn test_main_1() { +fn test_should_fail_with_match() { assert_eq(0, 1, "Not equal"); -} \ No newline at end of file +} + +#[test(should_fail)] +fn test_should_fail_without_match() { + assert_eq(0, 1); +} + +#[test(should_fail_with = "Not equal")] +fn test_should_fail_with_runtime_match() { + assert_eq(dep::std::hash::pedersen([27])[0], 0, "Not equal"); +} + +#[test(should_fail)] +fn test_should_fail_without_runtime_match() { + assert_eq(dep::std::hash::pedersen([27])[0], 0); +}