From 40ba443c097610f6c29b78cfbc82ec1c8039632c Mon Sep 17 00:00:00 2001 From: Tom Date: Sat, 10 Aug 2024 16:08:15 -0500 Subject: [PATCH 1/4] feat: remove `nargo` dependency from `fuzzer` package --- Cargo.lock | 1 - tooling/fuzzer/Cargo.toml | 1 - tooling/fuzzer/src/lib.rs | 36 +++++++++++++++---------- tooling/nargo_cli/src/cli/test_cmd.rs | 27 ++++++++++++++++--- tooling/nargo_cli/tests/stdlib-tests.rs | 15 ++++++++++- 5 files changed, 59 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47b63ff2f4f..0bee84419aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2697,7 +2697,6 @@ name = "noir_fuzzer" version = "0.33.0" dependencies = [ "acvm", - "nargo", "noirc_abi", "noirc_artifacts", "proptest", diff --git a/tooling/fuzzer/Cargo.toml b/tooling/fuzzer/Cargo.toml index ef49d707d6a..106d8abead1 100644 --- a/tooling/fuzzer/Cargo.toml +++ b/tooling/fuzzer/Cargo.toml @@ -11,7 +11,6 @@ license.workspace = true [dependencies] acvm.workspace = true -nargo.workspace = true noirc_artifacts.workspace = true noirc_abi.workspace = true proptest.workspace = true diff --git a/tooling/fuzzer/src/lib.rs b/tooling/fuzzer/src/lib.rs index 28d7353f35a..28a43279c95 100644 --- a/tooling/fuzzer/src/lib.rs +++ b/tooling/fuzzer/src/lib.rs @@ -3,7 +3,13 @@ //! //! Code is used under the MIT license. -use acvm::{blackbox_solver::StubbedBlackBoxSolver, FieldElement}; +use acvm::{ + acir::{ + circuit::Program, + native_types::{WitnessMap, WitnessStack}, + }, + FieldElement, +}; use dictionary::build_dictionary_from_program; use noirc_abi::InputMap; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; @@ -16,25 +22,32 @@ use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome, FuzzTestResult}; use noirc_artifacts::program::ProgramArtifact; -use nargo::ops::{execute_program, DefaultForeignCallExecutor}; - /// An executor for Noir programs which which provides fuzzing support using [`proptest`]. /// /// After instantiation, calling `fuzz` will proceed to hammer the program with /// inputs, until it finds a counterexample. The provided [`TestRunner`] contains all the /// configuration which can be overridden via [environment variables](proptest::test_runner::Config) -pub struct FuzzedExecutor { +pub struct FuzzedExecutor { /// The program to be fuzzed program: ProgramArtifact, + /// A function which executes the programs with a given set of inputs + executor: E, + /// The fuzzer runner: TestRunner, } -impl FuzzedExecutor { +impl< + E: Fn( + &Program, + WitnessMap, + ) -> Result, String>, + > FuzzedExecutor +{ /// Instantiates a fuzzed executor given a testrunner - pub fn new(program: ProgramArtifact, runner: TestRunner) -> Self { - Self { program, runner } + pub fn new(program: ProgramArtifact, executor: E, runner: TestRunner) -> Self { + Self { program, executor, runner } } /// Fuzzes the provided program. @@ -76,19 +89,14 @@ impl FuzzedExecutor { /// or a `CounterExampleOutcome` pub fn single_fuzz(&self, input_map: InputMap) -> Result { let initial_witness = self.program.abi.encode(&input_map, None).unwrap(); - let result = execute_program( - &self.program.bytecode, - initial_witness, - &StubbedBlackBoxSolver, - &mut DefaultForeignCallExecutor::::new(false, None), - ); + let result = (self.executor)(&self.program.bytecode, initial_witness); // TODO: Add handling for `vm.assume` equivalent match result { Ok(_) => Ok(FuzzOutcome::Case(CaseOutcome { case: input_map })), Err(err) => Ok(FuzzOutcome::CounterExample(CounterExampleOutcome { - exit_reason: err.to_string(), + exit_reason: err, counterexample: input_map, })), } diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 1cf5b32c381..073d42ec1d8 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -1,12 +1,20 @@ use std::io::Write; -use acvm::{BlackBoxFunctionSolver, FieldElement}; +use acvm::{ + acir::{ + circuit::Program, + native_types::{WitnessMap, WitnessStack}, + }, + BlackBoxFunctionSolver, FieldElement, +}; use bn254_blackbox_solver::Bn254BlackBoxSolver; use clap::Args; use fm::FileManager; use nargo::{ - insert_all_files_for_workspace_into_file_manager, ops::TestStatus, package::Package, parse_all, - prepare_package, + insert_all_files_for_workspace_into_file_manager, + ops::{execute_program, DefaultForeignCallExecutor, TestStatus}, + package::Package, + parse_all, prepare_package, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ @@ -206,7 +214,18 @@ fn run_test + Default>( Ok(compiled_program) => { let runner = TestRunner::default(); - let fuzzer = FuzzedExecutor::new(compiled_program.into(), runner); + let executor = |program: &Program, + initial_witness: WitnessMap| + -> Result, String> { + execute_program( + program, + initial_witness, + &blackbox_solver, + &mut DefaultForeignCallExecutor::::new(false, None), + ) + .map_err(|err| err.to_string()) + }; + let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); let result = fuzzer.fuzz(); if result.success { diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 0444f79d371..8e3ec3d7f3f 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -1,7 +1,11 @@ use std::io::Write; use std::{collections::BTreeMap, path::PathBuf}; +use acvm::acir::circuit::Program; +use acvm::acir::native_types::{WitnessMap, WitnessStack}; +use acvm::FieldElement; use fm::FileManager; +use nargo::ops::{execute_program, DefaultForeignCallExecutor}; use noirc_driver::{check_crate, compile_no_check, file_manager_with_stdlib, CompileOptions}; use noirc_frontend::hir::FunctionNameMatch; @@ -78,7 +82,16 @@ fn run_stdlib_tests() { Ok(compiled_program) => { let runner = TestRunner::default(); - let fuzzer = FuzzedExecutor::new(compiled_program.into(), runner); + let executor = |program: &Program, initial_witness: WitnessMap| -> Result, String> { + execute_program( + program, + initial_witness, + &bn254_blackbox_solver::Bn254BlackBoxSolver, + &mut DefaultForeignCallExecutor::::new(false, None), + ) + .map_err(|err| err.to_string()) + }; + let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); let result = fuzzer.fuzz(); if result.success { From 05eba09ab2df1728b28114a6770df6c61e075400 Mon Sep 17 00:00:00 2001 From: Tom Date: Sat, 10 Aug 2024 16:21:32 -0500 Subject: [PATCH 2/4] feat: push fuzzing test code into `nargo` package --- Cargo.lock | 4 +- cspell.json | 1 + tooling/nargo/Cargo.toml | 2 + tooling/nargo/src/ops/test.rs | 72 +++++++++++++++++------ tooling/nargo_cli/Cargo.toml | 2 - tooling/nargo_cli/src/cli/test_cmd.rs | 77 ++++--------------------- tooling/nargo_cli/tests/stdlib-tests.rs | 67 +++------------------ 7 files changed, 80 insertions(+), 145 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0bee84419aa..74e5bda2b66 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2508,11 +2508,13 @@ dependencies = [ "jsonrpc-core-client", "jsonrpc-derive", "jsonrpc-http-server", + "noir_fuzzer", "noirc_abi", "noirc_driver", "noirc_errors", "noirc_frontend", "noirc_printable_type", + "proptest", "rand 0.8.5", "rayon", "serde", @@ -2545,7 +2547,6 @@ dependencies = [ "nargo_fmt", "nargo_toml", "noir_debugger", - "noir_fuzzer", "noir_lsp", "noirc_abi", "noirc_artifacts", @@ -2558,7 +2559,6 @@ dependencies = [ "pprof 0.13.0", "predicates 2.1.5", "prettytable-rs", - "proptest", "rayon", "serde", "serde_json", diff --git a/cspell.json b/cspell.json index b9199bea4bd..e939cd583e8 100644 --- a/cspell.json +++ b/cspell.json @@ -88,6 +88,7 @@ "foralls", "formatcp", "frontends", + "fuzzer", "fxhash", "getrandom", "gloo", diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index b0cf1cfcbb1..e247e4372a4 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -13,6 +13,7 @@ license.workspace = true acvm.workspace = true fm.workspace = true noirc_abi.workspace = true +noir_fuzzer.workspace = true noirc_driver.workspace = true noirc_errors.workspace = true noirc_frontend.workspace = true @@ -24,6 +25,7 @@ rayon = "1.8.0" jsonrpc.workspace = true rand.workspace = true serde.workspace = true +proptest.workspace = true [dev-dependencies] # TODO: This dependency is used to generate unit tests for `get_all_paths_in_dir` diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 18c6f2530b9..6301a98d938 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -1,5 +1,8 @@ use acvm::{ - acir::native_types::{WitnessMap, WitnessStack}, + acir::{ + circuit::Program, + native_types::{WitnessMap, WitnessStack}, + }, BlackBoxFunctionSolver, FieldElement, }; use noirc_abi::Abi; @@ -31,23 +34,58 @@ pub fn run_test>( foreign_call_resolver_url: Option<&str>, config: &CompileOptions, ) -> TestStatus { - let compiled_program = compile_no_check(context, config, test_function.get_id(), None, false); - match compiled_program { + let test_function_has_no_arguments = context + .def_interner + .function_meta(&test_function.get_id()) + .function_signature() + .0 + .is_empty(); + + match compile_no_check(context, config, test_function.get_id(), None, false) { Ok(compiled_program) => { - // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, - // otherwise constraints involving these expressions will not error. - let circuit_execution = execute_program( - &compiled_program.program, - WitnessMap::new(), - blackbox_solver, - &mut DefaultForeignCallExecutor::new(show_output, foreign_call_resolver_url), - ); - test_status_program_compile_pass( - test_function, - compiled_program.abi, - compiled_program.debug, - circuit_execution, - ) + if test_function_has_no_arguments { + // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, + // otherwise constraints involving these expressions will not error. + let circuit_execution = execute_program( + &compiled_program.program, + WitnessMap::new(), + blackbox_solver, + &mut DefaultForeignCallExecutor::new(show_output, foreign_call_resolver_url), + ); + test_status_program_compile_pass( + test_function, + compiled_program.abi, + compiled_program.debug, + circuit_execution, + ) + } else { + use noir_fuzzer::FuzzedExecutor; + use proptest::test_runner::TestRunner; + let runner = TestRunner::default(); + + let executor = |program: &Program, + initial_witness: WitnessMap| + -> Result, String> { + execute_program( + program, + initial_witness, + blackbox_solver, + &mut DefaultForeignCallExecutor::::new(false, None), + ) + .map_err(|err| err.to_string()) + }; + let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); + + let result = fuzzer.fuzz(); + if result.success { + TestStatus::Pass + } else { + TestStatus::Fail { + message: result.reason.unwrap_or_default(), + error_diagnostic: None, + } + } + } } Err(err) => test_status_program_compile_fail(err, test_function), } diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index c6cf842a623..dabb779ae8d 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -32,7 +32,6 @@ noirc_driver.workspace = true noirc_frontend = { workspace = true, features = ["bn254"] } noirc_abi.workspace = true noirc_errors.workspace = true -noir_fuzzer.workspace = true noirc_artifacts.workspace = true acvm = { workspace = true, features = ["bn254"] } bn254_blackbox_solver.workspace = true @@ -51,7 +50,6 @@ color-eyre.workspace = true tokio = { version = "1.0", features = ["io-std", "rt"] } dap.workspace = true clap-markdown = { git = "https://github.com/noir-lang/clap-markdown", rev = "450d759532c88f0dba70891ceecdbc9ff8f25d2b", optional = true } -proptest.workspace = true notify = "6.1.1" notify-debouncer-full = "0.3.1" diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 073d42ec1d8..399b8eb5635 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -1,25 +1,16 @@ use std::io::Write; -use acvm::{ - acir::{ - circuit::Program, - native_types::{WitnessMap, WitnessStack}, - }, - BlackBoxFunctionSolver, FieldElement, -}; +use acvm::{BlackBoxFunctionSolver, FieldElement}; use bn254_blackbox_solver::Bn254BlackBoxSolver; use clap::Args; use fm::FileManager; use nargo::{ - insert_all_files_for_workspace_into_file_manager, - ops::{execute_program, DefaultForeignCallExecutor, TestStatus}, - package::Package, - parse_all, prepare_package, + insert_all_files_for_workspace_into_file_manager, ops::TestStatus, package::Package, parse_all, + prepare_package, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ - check_crate, compile_no_check, file_manager_with_stdlib, CompileOptions, - NOIR_ARTIFACT_VERSION_STRING, + check_crate, file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING, }; use noirc_frontend::{ graph::CrateName, @@ -188,58 +179,14 @@ fn run_test + Default>( let blackbox_solver = S::default(); - let test_function_has_no_arguments = context - .def_interner - .function_meta(&test_function.get_id()) - .function_signature() - .0 - .is_empty(); - - if test_function_has_no_arguments { - nargo::ops::run_test( - &blackbox_solver, - &mut context, - test_function, - show_output, - foreign_call_resolver_url, - compile_options, - ) - } else { - use noir_fuzzer::FuzzedExecutor; - use proptest::test_runner::TestRunner; - - let compiled_program = - compile_no_check(&mut context, compile_options, test_function.get_id(), None, false); - match compiled_program { - Ok(compiled_program) => { - let runner = TestRunner::default(); - - let executor = |program: &Program, - initial_witness: WitnessMap| - -> Result, String> { - execute_program( - program, - initial_witness, - &blackbox_solver, - &mut DefaultForeignCallExecutor::::new(false, None), - ) - .map_err(|err| err.to_string()) - }; - let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); - - let result = fuzzer.fuzz(); - if result.success { - TestStatus::Pass - } else { - TestStatus::Fail { - message: result.reason.unwrap_or_default(), - error_diagnostic: None, - } - } - } - Err(err) => TestStatus::CompileError(err.into()), - } - } + nargo::ops::run_test( + &blackbox_solver, + &mut context, + test_function, + show_output, + foreign_call_resolver_url, + compile_options, + ) } fn get_tests_in_package( diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 8e3ec3d7f3f..e29b4a68c15 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -1,12 +1,8 @@ use std::io::Write; use std::{collections::BTreeMap, path::PathBuf}; -use acvm::acir::circuit::Program; -use acvm::acir::native_types::{WitnessMap, WitnessStack}; -use acvm::FieldElement; use fm::FileManager; -use nargo::ops::{execute_program, DefaultForeignCallExecutor}; -use noirc_driver::{check_crate, compile_no_check, file_manager_with_stdlib, CompileOptions}; +use noirc_driver::{check_crate, file_manager_with_stdlib, CompileOptions}; use noirc_frontend::hir::FunctionNameMatch; use nargo::{ @@ -51,61 +47,14 @@ fn run_stdlib_tests() { let test_report: Vec<(String, TestStatus)> = test_functions .into_iter() .map(|(test_name, test_function)| { - let test_function_has_no_arguments = context - .def_interner - .function_meta(&test_function.get_id()) - .function_signature() - .0 - .is_empty(); - - let status = if test_function_has_no_arguments { - run_test( - &bn254_blackbox_solver::Bn254BlackBoxSolver, - &mut context, - &test_function, - false, - None, - &CompileOptions::default(), - ) - } else { - use noir_fuzzer::FuzzedExecutor; - use proptest::test_runner::TestRunner; - - let compiled_program = compile_no_check( - &mut context, - &CompileOptions::default(), - test_function.get_id(), - None, - false, - ); - match compiled_program { - Ok(compiled_program) => { - let runner = TestRunner::default(); - - let executor = |program: &Program, initial_witness: WitnessMap| -> Result, String> { - execute_program( - program, - initial_witness, + let status = run_test( &bn254_blackbox_solver::Bn254BlackBoxSolver, - &mut DefaultForeignCallExecutor::::new(false, None), - ) - .map_err(|err| err.to_string()) - }; - let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); - - let result = fuzzer.fuzz(); - if result.success { - TestStatus::Pass - } else { - TestStatus::Fail { - message: result.reason.unwrap_or_default(), - error_diagnostic: None, - } - } - } - Err(err) => TestStatus::CompileError(err.into()), - } - }; + &mut context, + &test_function, + false, + None, + &CompileOptions::default(), + ); (test_name, status) }) .collect(); From b22300a1a4ea26351f786977f0d9da15984431b5 Mon Sep 17 00:00:00 2001 From: Tom Date: Sat, 10 Aug 2024 23:05:30 -0500 Subject: [PATCH 3/4] fix: disable fuzz testing in wasm targets --- tooling/nargo/Cargo.toml | 4 ++- tooling/nargo/src/ops/test.rs | 62 ++++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index e247e4372a4..56e88dacf2d 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -13,7 +13,6 @@ license.workspace = true acvm.workspace = true fm.workspace = true noirc_abi.workspace = true -noir_fuzzer.workspace = true noirc_driver.workspace = true noirc_errors.workspace = true noirc_frontend.workspace = true @@ -25,6 +24,9 @@ rayon = "1.8.0" jsonrpc.workspace = true rand.workspace = true serde.workspace = true + +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +noir_fuzzer.workspace = true proptest.workspace = true [dev-dependencies] diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 6301a98d938..fa778310513 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -1,8 +1,5 @@ use acvm::{ - acir::{ - circuit::Program, - native_types::{WitnessMap, WitnessStack}, - }, + acir::native_types::{WitnessMap, WitnessStack}, BlackBoxFunctionSolver, FieldElement, }; use noirc_abi::Abi; @@ -59,32 +56,45 @@ pub fn run_test>( circuit_execution, ) } else { - use noir_fuzzer::FuzzedExecutor; - use proptest::test_runner::TestRunner; - let runner = TestRunner::default(); - - let executor = |program: &Program, - initial_witness: WitnessMap| - -> Result, String> { - execute_program( - program, - initial_witness, - blackbox_solver, - &mut DefaultForeignCallExecutor::::new(false, None), - ) - .map_err(|err| err.to_string()) - }; - let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); - - let result = fuzzer.fuzz(); - if result.success { - TestStatus::Pass - } else { + #[cfg(target_arch = "wasm32")] + { + // We currently don't support fuzz testing on wasm32 as the u128 strategies do not exist on this platform. TestStatus::Fail { - message: result.reason.unwrap_or_default(), + message: "Fuzz tests are not supported on wasm32".to_string(), error_diagnostic: None, } } + + #[cfg(not(target_arch = "wasm32"))] + { + use acvm::acir::circuit::Program; + use noir_fuzzer::FuzzedExecutor; + use proptest::test_runner::TestRunner; + let runner = TestRunner::default(); + + let executor = |program: &Program, + initial_witness: WitnessMap| + -> Result, String> { + execute_program( + program, + initial_witness, + blackbox_solver, + &mut DefaultForeignCallExecutor::::new(false, foreign_call_resolver_url), + ) + .map_err(|err| err.to_string()) + }; + let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); + + let result = fuzzer.fuzz(); + if result.success { + TestStatus::Pass + } else { + TestStatus::Fail { + message: result.reason.unwrap_or_default(), + error_diagnostic: None, + } + } + } } } Err(err) => test_status_program_compile_fail(err, test_function), From 10d53dad045a2c9357cd7f0334c2c2f808bc8af5 Mon Sep 17 00:00:00 2001 From: Tom Date: Sat, 10 Aug 2024 23:07:47 -0500 Subject: [PATCH 4/4] chore: fmt --- tooling/nargo/src/ops/test.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index fa778310513..0faae5ff506 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -71,20 +71,24 @@ pub fn run_test>( use noir_fuzzer::FuzzedExecutor; use proptest::test_runner::TestRunner; let runner = TestRunner::default(); - - let executor = |program: &Program, - initial_witness: WitnessMap| - -> Result, String> { - execute_program( - program, - initial_witness, - blackbox_solver, - &mut DefaultForeignCallExecutor::::new(false, foreign_call_resolver_url), - ) - .map_err(|err| err.to_string()) - }; + + let executor = + |program: &Program, + initial_witness: WitnessMap| + -> Result, String> { + execute_program( + program, + initial_witness, + blackbox_solver, + &mut DefaultForeignCallExecutor::::new( + false, + foreign_call_resolver_url, + ), + ) + .map_err(|err| err.to_string()) + }; let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); - + let result = fuzzer.fuzz(); if result.success { TestStatus::Pass