chore: allow passing custom executors to fuzzer (#5710)

# Description ## Problem\* Resolves  ## Summary\* This PR pays off some tech debt by removing the fuzzer's dependency on the `nargo` package. This allows us to pass in custom foreign call executors and blackbox solvers, etc. It also sorts some circular dependencies which were preventing us from integrating it cleanly into the test harness function. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
noir-lang · Aug 13, 2024 · 0ebf1fe · 0ebf1fe
1 parent 3c6b998
commit 0ebf1fe
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 126 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cspell.json b/cspell.json
@@ -88,6 +88,7 @@
         "foralls",
         "formatcp",
         "frontends",
+        "fuzzer",
         "fxhash",
         "getrandom",
         "gloo",

diff --git 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
@@ -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<E> {
     /// 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<FieldElement>,
+            WitnessMap<FieldElement>,
+        ) -> Result<WitnessStack<FieldElement>, String>,
+    > FuzzedExecutor<E>
+{
     /// 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<FuzzOutcome, TestCaseError> {
         let initial_witness = self.program.abi.encode(&input_map, None).unwrap();
-        let result = execute_program(
-            &self.program.bytecode,
-            initial_witness,
-            &StubbedBlackBoxSolver,
-            &mut DefaultForeignCallExecutor::<FieldElement>::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/Cargo.toml b/tooling/nargo/Cargo.toml
@@ -25,6 +25,10 @@ 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]
 # TODO: This dependency is used to generate unit tests for `get_all_paths_in_dir`
 # TODO: once that method is moved to nargo_cli, we can move this dependency to nargo_cli

diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs
@@ -31,23 +31,75 @@ pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
     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 {
+                #[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: "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<FieldElement>,
+                         initial_witness: WitnessMap<FieldElement>|
+                         -> Result<WitnessStack<FieldElement>, String> {
+                            execute_program(
+                                program,
+                                initial_witness,
+                                blackbox_solver,
+                                &mut DefaultForeignCallExecutor::<FieldElement>::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),
     }

diff --git 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
@@ -10,8 +10,7 @@ use nargo::{
 };
 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,
@@ -180,47 +179,14 @@ fn run_test<S: BlackBoxFunctionSolver<FieldElement> + 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 fuzzer = FuzzedExecutor::new(compiled_program.into(), 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
@@ -2,7 +2,7 @@ use std::io::Write;
 use std::{collections::BTreeMap, path::PathBuf};
 
 use fm::FileManager;
-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::{
@@ -47,52 +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 fuzzer = FuzzedExecutor::new(compiled_program.into(), 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()),
-                }
-            };
+            let status = run_test(
+                &bn254_blackbox_solver::Bn254BlackBoxSolver,
+                &mut context,
+                &test_function,
+                false,
+                None,
+                &CompileOptions::default(),
+            );
             (test_name, status)
         })
         .collect();