From 8d9a38071a65227cb76ffbfed37ce7866b78df0b Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 13 Jun 2024 23:48:20 +0100 Subject: [PATCH 01/17] feat: add fuzzer for Noir programs --- Cargo.lock | 2 + tooling/nargo/Cargo.toml | 1 + tooling/nargo/src/ops/fuzz/mod.rs | 109 +++++++++++++++++++++++ tooling/nargo/src/ops/fuzz/types.rs | 42 +++++++++ tooling/nargo/src/ops/mod.rs | 1 + tooling/nargo_cli/Cargo.toml | 1 + tooling/nargo_cli/src/cli/execute_cmd.rs | 9 ++ tooling/noirc_abi/Cargo.toml | 6 +- tooling/noirc_abi/src/arbitrary.rs | 18 +++- tooling/noirc_abi/src/lib.rs | 27 +++--- 10 files changed, 197 insertions(+), 19 deletions(-) create mode 100644 tooling/nargo/src/ops/fuzz/mod.rs create mode 100644 tooling/nargo/src/ops/fuzz/types.rs diff --git a/Cargo.lock b/Cargo.lock index 7b96737f241..94cb163c7b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2486,6 +2486,7 @@ dependencies = [ "noirc_errors", "noirc_frontend", "noirc_printable_type", + "proptest", "rand 0.8.5", "rayon", "serde", @@ -2529,6 +2530,7 @@ dependencies = [ "pprof 0.13.0", "predicates 2.1.5", "prettytable-rs", + "proptest", "rayon", "serde", "serde_json", diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index 8abec267d20..07333c6ef86 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -25,6 +25,7 @@ tracing.workspace = true rayon = "1.8.0" jsonrpc.workspace = true rand.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/fuzz/mod.rs b/tooling/nargo/src/ops/fuzz/mod.rs new file mode 100644 index 00000000000..58a72adcc91 --- /dev/null +++ b/tooling/nargo/src/ops/fuzz/mod.rs @@ -0,0 +1,109 @@ +//! This module has been adapted from Foundry's fuzzing implementation for the EVM. +//! https://github.com/foundry-rs/foundry/blob/6a85dbaa62f1c305f31cab37781232913055ae28/crates/evm/evm/src/executors/fuzz/mod.rs#L40 +//! +//! Code is used under the MIT license. + +use std::cell::RefCell; + +use acvm::{blackbox_solver::StubbedBlackBoxSolver, FieldElement}; +use noirc_abi::{arbitrary::arb_input_map, InputMap}; +use proptest::test_runner::{TestCaseError, TestError, TestRunner}; + +mod types; + +use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome, FuzzTestResult}; + +use crate::artifacts::program::ProgramArtifact; + +use super::{execute_program, DefaultForeignCallExecutor}; + +/// Wrapper around an [`Executor`] 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 { + program: ProgramArtifact, + + /// The fuzzer + runner: TestRunner, +} + +impl FuzzedExecutor { + /// Instantiates a fuzzed executor given a testrunner + pub fn new(program: ProgramArtifact, runner: TestRunner) -> Self { + Self { program, runner } + } + + /// Fuzzes the provided program. + pub fn fuzz(&self) -> FuzzTestResult { + // Stores the result and calldata of the last failed call, if any. + let counterexample: RefCell = RefCell::default(); + + let strategy = arb_input_map(&self.program.abi); + + let run_result: Result<_, TestError<_>> = self.runner.clone().run(&strategy, |input_map| { + println!("fuzzing with {input_map:?}"); + + let fuzz_res = self.single_fuzz(input_map)?; + + match fuzz_res { + FuzzOutcome::Case(_) => Ok(()), + FuzzOutcome::CounterExample(CounterExampleOutcome { + exit_reason: status, + counterexample: outcome, + .. + }) => { + println!("found counterexample, {outcome:?}"); + // We cannot use the calldata returned by the test runner in `TestError::Fail`, + // since that input represents the last run case, which may not correspond with + // our failure - when a fuzz case fails, proptest will try + // to run at least one more case to find a minimal failure + // case. + *counterexample.borrow_mut() = outcome; + Err(TestCaseError::fail(status)) + } + } + }); + + let mut result = + FuzzTestResult { success: run_result.is_ok(), reason: None, counterexample: None }; + + match run_result { + Err(TestError::Abort(reason)) => { + result.reason = Some(reason.to_string()); + } + Err(TestError::Fail(reason, _)) => { + let reason = reason.to_string(); + result.reason = if reason.is_empty() { None } else { Some(reason) }; + + result.counterexample = Some(counterexample.into_inner()); + } + _ => {} + } + + result + } + + /// Granular and single-step function that runs only one fuzz and returns either a `CaseOutcome` + /// 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), + ); + + // 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(), + counterexample: input_map, + })), + } + } +} diff --git a/tooling/nargo/src/ops/fuzz/types.rs b/tooling/nargo/src/ops/fuzz/types.rs new file mode 100644 index 00000000000..12daf5bb012 --- /dev/null +++ b/tooling/nargo/src/ops/fuzz/types.rs @@ -0,0 +1,42 @@ +use noirc_abi::InputMap; + +type CounterExample = InputMap; + +/// The outcome of a fuzz test +#[derive(Debug)] +pub struct FuzzTestResult { + /// Whether the test case was successful. This means that the transaction executed + /// properly, or that there was a revert and that the test was expected to fail + /// (prefixed with `testFail`) + pub success: bool, + + /// If there was a revert, this field will be populated. Note that the test can + /// still be successful (i.e self.success == true) when it's expected to fail. + pub reason: Option, + + /// Minimal reproduction test case for failing fuzz tests + pub counterexample: Option, +} + +/// Returned by a single fuzz in the case of a successful run +#[derive(Debug)] +pub struct CaseOutcome { + /// Data of a single fuzz test case + pub case: InputMap, +} + +/// Returned by a single fuzz when a counterexample has been discovered +#[derive(Debug)] +pub struct CounterExampleOutcome { + /// Minimal reproduction test case for failing test + pub counterexample: CounterExample, + /// The status of the call + pub exit_reason: String, +} + +/// Outcome of a single fuzz +#[derive(Debug)] +pub enum FuzzOutcome { + Case(CaseOutcome), + CounterExample(CounterExampleOutcome), +} diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index cada2f0e915..53d55b5d750 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -12,6 +12,7 @@ pub use self::test::{run_test, TestStatus}; mod compile; mod execute; mod foreign_calls; +pub mod fuzz; mod optimize; mod test; mod transform; diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index a9946c8700c..0d934322c75 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -49,6 +49,7 @@ color-eyre = "0.6.2" 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/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index b548336275b..de4f54bd605 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -6,6 +6,7 @@ use clap::Args; use nargo::artifacts::debug::DebugArtifact; use nargo::constants::PROVER_INPUT_FILE; use nargo::errors::try_to_diagnose_runtime_error; +use nargo::ops::fuzz::FuzzedExecutor; use nargo::ops::DefaultForeignCallExecutor; use nargo::package::Package; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; @@ -13,6 +14,7 @@ use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::InputMap; use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::graph::CrateName; +use proptest::test_runner::TestRunner; use super::compile_cmd::compile_workspace_full; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; @@ -112,6 +114,13 @@ pub(crate) fn execute_program( ) -> Result, CliError> { let initial_witness = compiled_program.abi.encode(inputs_map, None)?; + let runner = TestRunner::default(); + let fuzzer = FuzzedExecutor::new(compiled_program.clone().into(), runner); + + let result = fuzzer.fuzz(); + + println!("{result:?}"); + let solved_witness_stack_err = nargo::ops::execute_program( &compiled_program.program, initial_witness, diff --git a/tooling/noirc_abi/Cargo.toml b/tooling/noirc_abi/Cargo.toml index e8f57060367..573a6d8c29a 100644 --- a/tooling/noirc_abi/Cargo.toml +++ b/tooling/noirc_abi/Cargo.toml @@ -18,14 +18,14 @@ serde.workspace = true thiserror.workspace = true num-bigint = "0.4" num-traits = "0.2" +proptest.workspace = true +proptest-derive.workspace = true [dev-dependencies] strum = "0.24" strum_macros = "0.24" -proptest.workspace = true -proptest-derive.workspace = true [features] bn254 = ["acvm/bn254"] -bls12_381 = ["acvm/bls12_381"] \ No newline at end of file +bls12_381 = ["acvm/bls12_381"] diff --git a/tooling/noirc_abi/src/arbitrary.rs b/tooling/noirc_abi/src/arbitrary.rs index aecb620b79d..ef3630c2550 100644 --- a/tooling/noirc_abi/src/arbitrary.rs +++ b/tooling/noirc_abi/src/arbitrary.rs @@ -27,7 +27,8 @@ proptest::prop_compose! { pub(super) fn arb_field_from_integer(bit_size: u32)(value: u128)-> FieldElement { let width = (bit_size % 128).clamp(1, 127); let max_value = 2u128.pow(width) - 1; - FieldElement::from(value.clamp(0, max_value)) + let value = value % max_value; + FieldElement::from(value) } } @@ -139,6 +140,21 @@ fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { .sboxed() } +pub fn arb_input_map(abi: &Abi) -> BoxedStrategy { + let values: Vec<_> = abi + .parameters + .iter() + .map(|param| (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ))) + .collect(); + + values + .prop_map(|values| { + let input_map: InputMap = values.into_iter().collect(); + input_map + }) + .boxed() +} + prop_compose! { pub(super) fn arb_abi_and_input_map() (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type: Option) diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index b3d80099137..bc9f9d6d947 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -25,8 +25,7 @@ use std::{collections::BTreeMap, str}; // // This ABI has nothing to do with ACVM or ACIR. Although they implicitly have a relationship -#[cfg(test)] -mod arbitrary; +pub mod arbitrary; pub mod errors; pub mod input_parser; @@ -77,8 +76,7 @@ pub enum AbiType { }, } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] -#[cfg_attr(test, derive(arbitrary::Arbitrary))] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, arbitrary::Arbitrary)] #[serde(rename_all = "lowercase")] /// Represents whether the parameter is public or known only to the prover. pub enum AbiVisibility { @@ -89,8 +87,7 @@ pub enum AbiVisibility { DataBus, } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] -#[cfg_attr(test, derive(arbitrary::Arbitrary))] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, arbitrary::Arbitrary)] #[serde(rename_all = "lowercase")] pub enum Sign { Unsigned, @@ -146,13 +143,13 @@ impl From<&AbiType> for PrintableType { } } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -#[cfg_attr(test, derive(arbitrary::Arbitrary))] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, arbitrary::Arbitrary)] + /// An argument or return value of the circuit's `main` function. pub struct AbiParameter { pub name: String, #[serde(rename = "type")] - #[cfg_attr(test, proptest(strategy = "arbitrary::arb_abi_type()"))] + #[proptest(strategy = "arbitrary::arb_abi_type()")] pub typ: AbiType, pub visibility: AbiVisibility, } @@ -163,21 +160,21 @@ impl AbiParameter { } } -#[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(arbitrary::Arbitrary))] +#[derive(Clone, Debug, Serialize, Deserialize, arbitrary::Arbitrary)] + pub struct AbiReturnType { - #[cfg_attr(test, proptest(strategy = "arbitrary::arb_abi_type()"))] + #[proptest(strategy = "arbitrary::arb_abi_type()")] pub abi_type: AbiType, pub visibility: AbiVisibility, } -#[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(arbitrary::Arbitrary))] +#[derive(Clone, Debug, Serialize, Deserialize, arbitrary::Arbitrary)] + pub struct Abi { /// An ordered list of the arguments to the program's `main` function, specifying their types and visibility. pub parameters: Vec, pub return_type: Option, - #[cfg_attr(test, proptest(strategy = "proptest::prelude::Just(BTreeMap::from([]))"))] + #[proptest(strategy = "proptest::prelude::Just(BTreeMap::from([]))")] pub error_types: BTreeMap, } From b37f3242c169ea90e542d03c067430067485d757 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 14 Jun 2024 21:31:46 +0100 Subject: [PATCH 02/17] chore: restructure code and improve integer strategies --- Cargo.lock | 13 +- Cargo.toml | 2 + temp/Nargo.toml | 7 + temp/src/main.nr | 4 + tooling/fuzzer/Cargo.toml | 17 ++ .../src/ops/fuzz/mod.rs => fuzzer/src/lib.rs} | 56 +++--- tooling/fuzzer/src/strategies/int.rs | 185 ++++++++++++++++++ tooling/fuzzer/src/strategies/mod.rs | 97 +++++++++ tooling/fuzzer/src/strategies/uint.rs | 152 ++++++++++++++ .../src/ops/fuzz => fuzzer/src}/types.rs | 0 tooling/nargo/Cargo.toml | 1 - tooling/nargo/src/ops/mod.rs | 1 - tooling/nargo_cli/Cargo.toml | 1 + tooling/nargo_cli/src/cli/execute_cmd.rs | 9 - tooling/nargo_cli/src/cli/fuzz_cmd.rs | 71 +++++++ tooling/nargo_cli/src/cli/mod.rs | 3 + tooling/noirc_abi/src/arbitrary.rs | 40 +--- tooling/noirc_abi/src/lib.rs | 39 +++- 18 files changed, 621 insertions(+), 77 deletions(-) create mode 100644 temp/Nargo.toml create mode 100644 temp/src/main.nr create mode 100644 tooling/fuzzer/Cargo.toml rename tooling/{nargo/src/ops/fuzz/mod.rs => fuzzer/src/lib.rs} (67%) create mode 100644 tooling/fuzzer/src/strategies/int.rs create mode 100644 tooling/fuzzer/src/strategies/mod.rs create mode 100644 tooling/fuzzer/src/strategies/uint.rs rename tooling/{nargo/src/ops/fuzz => fuzzer/src}/types.rs (100%) create mode 100644 tooling/nargo_cli/src/cli/fuzz_cmd.rs diff --git a/Cargo.lock b/Cargo.lock index 94cb163c7b7..8136f289d31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2486,7 +2486,6 @@ dependencies = [ "noirc_errors", "noirc_frontend", "noirc_printable_type", - "proptest", "rand 0.8.5", "rayon", "serde", @@ -2519,6 +2518,7 @@ dependencies = [ "nargo_fmt", "nargo_toml", "noir_debugger", + "noir_fuzzer", "noir_lsp", "noirc_abi", "noirc_driver", @@ -2662,6 +2662,17 @@ dependencies = [ "thiserror", ] +[[package]] +name = "noir_fuzzer" +version = "0.30.0" +dependencies = [ + "acvm", + "nargo", + "noirc_abi", + "proptest", + "rand 0.8.5", +] + [[package]] name = "noir_grumpkin" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 639807819ab..11fec0641f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ members = [ # Crates related to tooling built on top of the Noir compiler "tooling/lsp", "tooling/debugger", + "tooling/fuzzer", "tooling/nargo", "tooling/nargo_fmt", "tooling/nargo_cli", @@ -68,6 +69,7 @@ noirc_frontend = { path = "compiler/noirc_frontend" } noirc_printable_type = { path = "compiler/noirc_printable_type" } # Noir tooling workspace dependencies +noir_fuzzer = { path = "tooling/fuzzer" } nargo = { path = "tooling/nargo" } nargo_fmt = { path = "tooling/nargo_fmt" } nargo_toml = { path = "tooling/nargo_toml" } diff --git a/temp/Nargo.toml b/temp/Nargo.toml new file mode 100644 index 00000000000..6b9591f1a13 --- /dev/null +++ b/temp/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "temp" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/temp/src/main.nr b/temp/src/main.nr new file mode 100644 index 00000000000..36b35d1f385 --- /dev/null +++ b/temp/src/main.nr @@ -0,0 +1,4 @@ +fn main(x: i64, y: pub i64) { + assert(x != y); +} + diff --git a/tooling/fuzzer/Cargo.toml b/tooling/fuzzer/Cargo.toml new file mode 100644 index 00000000000..5759f592634 --- /dev/null +++ b/tooling/fuzzer/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "noir_fuzzer" +description = "A fuzzer for Noir programs" +version.workspace = true +authors.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +acvm.workspace = true +nargo.workspace = true +noirc_abi.workspace = true +proptest.workspace = true +rand.workspace = true diff --git a/tooling/nargo/src/ops/fuzz/mod.rs b/tooling/fuzzer/src/lib.rs similarity index 67% rename from tooling/nargo/src/ops/fuzz/mod.rs rename to tooling/fuzzer/src/lib.rs index 58a72adcc91..d523fc371be 100644 --- a/tooling/nargo/src/ops/fuzz/mod.rs +++ b/tooling/fuzzer/src/lib.rs @@ -6,16 +6,17 @@ use std::cell::RefCell; use acvm::{blackbox_solver::StubbedBlackBoxSolver, FieldElement}; -use noirc_abi::{arbitrary::arb_input_map, InputMap}; +use noirc_abi::InputMap; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; +mod strategies; mod types; use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome, FuzzTestResult}; -use crate::artifacts::program::ProgramArtifact; +use nargo::artifacts::program::ProgramArtifact; -use super::{execute_program, DefaultForeignCallExecutor}; +use nargo::ops::{execute_program, DefaultForeignCallExecutor}; /// Wrapper around an [`Executor`] which provides fuzzing support using [`proptest`]. /// @@ -40,31 +41,32 @@ impl FuzzedExecutor { // Stores the result and calldata of the last failed call, if any. let counterexample: RefCell = RefCell::default(); - let strategy = arb_input_map(&self.program.abi); - - let run_result: Result<_, TestError<_>> = self.runner.clone().run(&strategy, |input_map| { - println!("fuzzing with {input_map:?}"); - - let fuzz_res = self.single_fuzz(input_map)?; - - match fuzz_res { - FuzzOutcome::Case(_) => Ok(()), - FuzzOutcome::CounterExample(CounterExampleOutcome { - exit_reason: status, - counterexample: outcome, - .. - }) => { - println!("found counterexample, {outcome:?}"); - // We cannot use the calldata returned by the test runner in `TestError::Fail`, - // since that input represents the last run case, which may not correspond with - // our failure - when a fuzz case fails, proptest will try - // to run at least one more case to find a minimal failure - // case. - *counterexample.borrow_mut() = outcome; - Err(TestCaseError::fail(status)) + let strategy = strategies::arb_input_map(&self.program.abi); + + let run_result: Result<(), TestError<_>> = + self.runner.clone().run(&strategy, |input_map| { + println!("fuzzing with {input_map:?}"); + + let fuzz_res = self.single_fuzz(input_map)?; + + match fuzz_res { + FuzzOutcome::Case(_) => Ok(()), + FuzzOutcome::CounterExample(CounterExampleOutcome { + exit_reason: status, + counterexample: outcome, + .. + }) => { + println!("found counterexample, {outcome:?}"); + // We cannot use the calldata returned by the test runner in `TestError::Fail`, + // since that input represents the last run case, which may not correspond with + // our failure - when a fuzz case fails, proptest will try + // to run at least one more case to find a minimal failure + // case. + *counterexample.borrow_mut() = outcome; + Err(TestCaseError::fail(status)) + } } - } - }); + }); let mut result = FuzzTestResult { success: run_result.is_ok(), reason: None, counterexample: None }; diff --git a/tooling/fuzzer/src/strategies/int.rs b/tooling/fuzzer/src/strategies/int.rs new file mode 100644 index 00000000000..c529d2f9501 --- /dev/null +++ b/tooling/fuzzer/src/strategies/int.rs @@ -0,0 +1,185 @@ +use proptest::{ + strategy::{NewTree, Strategy, ValueTree}, + test_runner::TestRunner, +}; +use rand::Rng; + +/// Value tree for signed ints (up to int256). +pub struct IntValueTree { + /// Lower base (by absolute value) + lo: i128, + /// Current value + curr: i128, + /// Higher base (by absolute value) + hi: i128, + /// If true cannot be simplified or complexified + fixed: bool, +} + +impl IntValueTree { + /// Create a new tree + /// # Arguments + /// * `start` - Starting value for the tree + /// * `fixed` - If `true` the tree would only contain one element and won't be simplified. + fn new(start: i128, fixed: bool) -> Self { + Self { lo: 0, curr: start, hi: start, fixed } + } + + fn reposition(&mut self) -> bool { + let interval = self.hi - self.lo; + let new_mid = self.lo + interval / 2i128; + + if new_mid == self.curr { + false + } else { + self.curr = new_mid; + true + } + } + + fn magnitude_greater(lhs: i128, rhs: i128) -> bool { + if lhs == 0 { + return false; + } + (lhs > rhs) ^ (lhs.is_negative()) + } +} + +impl ValueTree for IntValueTree { + type Value = i128; + + fn current(&self) -> Self::Value { + self.curr + } + + fn simplify(&mut self) -> bool { + if self.fixed || !Self::magnitude_greater(self.hi, self.lo) { + return false; + } + self.hi = self.curr; + self.reposition() + } + + fn complicate(&mut self) -> bool { + if self.fixed || !Self::magnitude_greater(self.hi, self.lo) { + return false; + } + + self.lo = if self.curr != i128::MIN && self.curr != i128::MAX { + self.curr + if self.hi.is_negative() { -1i128 } else { 1i128 } + } else { + self.curr + }; + + self.reposition() + } +} + +/// Value tree for signed ints (up to int256). +/// The strategy combines 3 different strategies, each assigned a specific weight: +/// 1. Generate purely random value in a range. This will first choose bit size uniformly (up `bits` +/// param). Then generate a value for this bit size. +/// 2. Generate a random value around the edges (+/- 3 around min, 0 and max possible value) +/// 3. Generate a value from a predefined fixtures set +/// +/// To define int fixtures: +/// - return an array of possible values for a parameter named `amount` declare a function `function +/// fixture_amount() public returns (int32[] memory)`. +/// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values +/// `function testFuzz_int32(int32 amount)`. +/// +/// If fixture is not a valid int type then error is raised and random value generated. +#[derive(Debug)] +pub struct IntStrategy { + /// Bit size of int (e.g. 256) + bits: usize, + /// The weight for edge cases (+/- 3 around 0 and max possible value) + edge_weight: usize, + /// The weight for purely random values + random_weight: usize, +} + +impl IntStrategy { + /// Create a new strategy. + /// #Arguments + /// * `bits` - Size of uint in bits + /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) + pub fn new(bits: usize) -> Self { + Self { bits, edge_weight: 10usize, random_weight: 50usize } + } + + fn generate_edge_tree(&self, runner: &mut TestRunner) -> NewTree { + let rng = runner.rng(); + + let offset = rng.gen_range(0..4); + // Choose if we want values around min, -0, +0, or max + let kind = rng.gen_range(0..4); + let start = match kind { + 0 => self.type_min() + offset, + 1 => -offset - 1i128, + 2 => offset, + 3 => self.type_max() - offset, + _ => unreachable!(), + }; + Ok(IntValueTree::new(start, false)) + } + + fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { + let rng = runner.rng(); + + let start: i128 = rng.gen_range(self.type_min()..=self.type_max()); + Ok(IntValueTree::new(start, false)) + } + + fn type_max(&self) -> i128 { + if self.bits < 128 { + (1i128 << (self.bits - 1)) - 1 + } else { + i128::MAX + } + } + + fn type_min(&self) -> i128 { + if self.bits < 128 { + -(1i128 << (self.bits - 1)) + } else { + i128::MIN + } + } +} + +impl Strategy for IntStrategy { + type Tree = IntValueTree; + type Value = i128; + + fn new_tree(&self, runner: &mut TestRunner) -> NewTree { + let total_weight = self.random_weight + self.edge_weight; + let bias = runner.rng().gen_range(0..total_weight); + // randomly select one of 2 strategies + match bias { + x if x < self.edge_weight => self.generate_edge_tree(runner), + _ => self.generate_random_tree(runner), + } + } +} + +#[cfg(test)] +mod tests { + use crate::strategies::int::IntValueTree; + use proptest::strategy::ValueTree; + + #[test] + fn test_int_tree_complicate_should_not_overflow() { + let mut int_tree = IntValueTree::new(i128::MAX, false); + assert_eq!(int_tree.hi, i128::MAX); + assert_eq!(int_tree.curr, i128::MAX); + int_tree.complicate(); + assert_eq!(int_tree.lo, i128::MAX); + + let mut int_tree = IntValueTree::new(i128::MIN, false); + assert_eq!(int_tree.hi, i128::MIN); + assert_eq!(int_tree.curr, i128::MIN); + int_tree.complicate(); + assert_eq!(int_tree.lo, i128::MIN); + } +} diff --git a/tooling/fuzzer/src/strategies/mod.rs b/tooling/fuzzer/src/strategies/mod.rs new file mode 100644 index 00000000000..f5b03953ba8 --- /dev/null +++ b/tooling/fuzzer/src/strategies/mod.rs @@ -0,0 +1,97 @@ +use int::IntStrategy; +use prop::collection::vec; +use proptest::prelude::*; + +use acvm::{AcirField, FieldElement}; + +use noirc_abi::{input_parser::InputValue, Abi, AbiType, InputMap, Sign}; +use std::collections::BTreeMap; +use uint::UintStrategy; + +mod int; +mod uint; + +proptest::prop_compose! { + pub(super) fn arb_field_from_integer(bit_size: u32)(value: u128)-> FieldElement { + let width = (bit_size % 128).clamp(1, 127); + let max_value = 2u128.pow(width) - 1; + let value = value % max_value; + FieldElement::from(value) + } +} + +pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { + match abi_type { + AbiType::Field => vec(any::(), 32) + .prop_map(|bytes| InputValue::Field(FieldElement::from_be_bytes_reduce(&bytes))) + .sboxed(), + AbiType::Integer { width, sign } if sign == &Sign::Unsigned => { + UintStrategy::new(*width as usize) + .prop_map(|uint| InputValue::Field(uint.into())) + .sboxed() + } + AbiType::Integer { width, .. } => { + let shift = 2i128.pow(*width); + IntStrategy::new(*width as usize) + .prop_map(move |mut int| { + if int < 0 { + int += shift + } + InputValue::Field(int.into()) + }) + .sboxed() + } + AbiType::Boolean => { + any::().prop_map(|val| InputValue::Field(FieldElement::from(val))).sboxed() + } + + AbiType::String { length } => { + // Strings only allow ASCII characters as each character must be able to be represented by a single byte. + let string_regex = format!("[[:ascii:]]{{{length}}}"); + proptest::string::string_regex(&string_regex) + .expect("parsing of regex should always succeed") + .prop_map(InputValue::String) + .sboxed() + } + AbiType::Array { length, typ } => { + let length = *length as usize; + let elements = vec(arb_value_from_abi_type(typ), length..=length); + + elements.prop_map(InputValue::Vec).sboxed() + } + + AbiType::Struct { fields, .. } => { + let fields: Vec> = fields + .iter() + .map(|(name, typ)| (Just(name.clone()), arb_value_from_abi_type(typ)).sboxed()) + .collect(); + + fields + .prop_map(|fields| { + let fields: BTreeMap<_, _> = fields.into_iter().collect(); + InputValue::Struct(fields) + }) + .sboxed() + } + + AbiType::Tuple { fields } => { + let fields: Vec<_> = fields.iter().map(arb_value_from_abi_type).collect(); + fields.prop_map(InputValue::Vec).sboxed() + } + } +} + +pub(super) fn arb_input_map(abi: &Abi) -> BoxedStrategy { + let values: Vec<_> = abi + .parameters + .iter() + .map(|param| (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ))) + .collect(); + + values + .prop_map(|values| { + let input_map: InputMap = values.into_iter().collect(); + input_map + }) + .boxed() +} diff --git a/tooling/fuzzer/src/strategies/uint.rs b/tooling/fuzzer/src/strategies/uint.rs new file mode 100644 index 00000000000..a7ecf89e5e1 --- /dev/null +++ b/tooling/fuzzer/src/strategies/uint.rs @@ -0,0 +1,152 @@ +use proptest::{ + strategy::{NewTree, Strategy, ValueTree}, + test_runner::TestRunner, +}; +use rand::Rng; + +/// Value tree for unsigned ints (up to uint256). +pub struct UintValueTree { + /// Lower base + lo: u128, + /// Current value + curr: u128, + /// Higher base + hi: u128, + /// If true cannot be simplified or complexified + fixed: bool, +} + +impl UintValueTree { + /// Create a new tree + /// # Arguments + /// * `start` - Starting value for the tree + /// * `fixed` - If `true` the tree would only contain one element and won't be simplified. + fn new(start: u128, fixed: bool) -> Self { + Self { lo: 0, curr: start, hi: start, fixed } + } + + fn reposition(&mut self) -> bool { + let interval = self.hi - self.lo; + let new_mid = self.lo + interval / 2; + + if new_mid == self.curr { + false + } else { + self.curr = new_mid; + true + } + } +} + +impl ValueTree for UintValueTree { + type Value = u128; + + fn current(&self) -> Self::Value { + self.curr + } + + fn simplify(&mut self) -> bool { + if self.fixed || (self.hi <= self.lo) { + return false; + } + self.hi = self.curr; + self.reposition() + } + + fn complicate(&mut self) -> bool { + if self.fixed || (self.hi <= self.lo) { + return false; + } + + self.lo = self.curr.wrapping_add(1); + self.reposition() + } +} + +/// Value tree for unsigned ints (up to uint256). +/// The strategy combines 3 different strategies, each assigned a specific weight: +/// 1. Generate purely random value in a range. This will first choose bit size uniformly (up `bits` +/// param). Then generate a value for this bit size. +/// 2. Generate a random value around the edges (+/- 3 around 0 and max possible value) +/// 3. Generate a value from a predefined fixtures set +/// +/// To define uint fixtures: +/// - return an array of possible values for a parameter named `amount` declare a function `function +/// fixture_amount() public returns (uint32[] memory)`. +/// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values +/// `function testFuzz_uint32(uint32 amount)`. +/// +/// If fixture is not a valid uint type then error is raised and random value generated. +#[derive(Debug)] +pub struct UintStrategy { + /// Bit size of uint (e.g. 256) + bits: usize, + + /// The weight for edge cases (+/- 3 around 0 and max possible value) + edge_weight: usize, + /// The weight for purely random values + random_weight: usize, +} + +impl UintStrategy { + /// Create a new strategy. + /// #Arguments + /// * `bits` - Size of uint in bits + /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) + pub fn new(bits: usize) -> Self { + Self { bits, edge_weight: 10usize, random_weight: 50usize } + } + + fn generate_edge_tree(&self, runner: &mut TestRunner) -> NewTree { + let rng = runner.rng(); + // Choose if we want values around 0 or max + let is_min = rng.gen_bool(0.5); + let offset = rng.gen_range(0..4); + let start = if is_min { offset } else { self.type_max().saturating_sub(offset) }; + Ok(UintValueTree::new(start, false)) + } + + fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { + let rng = runner.rng(); + let start: u128 = rng.gen_range(0..=self.type_max()); + + Ok(UintValueTree::new(start, false)) + } + + fn type_max(&self) -> u128 { + if self.bits < 128 { + (1 << self.bits) - 1 + } else { + u128::MAX + } + } +} + +impl Strategy for UintStrategy { + type Tree = UintValueTree; + type Value = u128; + fn new_tree(&self, runner: &mut TestRunner) -> NewTree { + let total_weight = self.random_weight + self.edge_weight; + let bias = runner.rng().gen_range(0..total_weight); + // randomly select one of 3 strategies + match bias { + x if x < self.edge_weight => self.generate_edge_tree(runner), + _ => self.generate_random_tree(runner), + } + } +} + +#[cfg(test)] +mod tests { + use crate::strategies::uint::UintValueTree; + use proptest::strategy::ValueTree; + + #[test] + fn test_uint_tree_complicate_max() { + let mut uint_tree = UintValueTree::new(u128::MAX, false); + assert_eq!(uint_tree.hi, u128::MAX); + assert_eq!(uint_tree.curr, u128::MAX); + uint_tree.complicate(); + assert_eq!(uint_tree.lo, u128::MIN); + } +} diff --git a/tooling/nargo/src/ops/fuzz/types.rs b/tooling/fuzzer/src/types.rs similarity index 100% rename from tooling/nargo/src/ops/fuzz/types.rs rename to tooling/fuzzer/src/types.rs diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index 07333c6ef86..8abec267d20 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -25,7 +25,6 @@ tracing.workspace = true rayon = "1.8.0" jsonrpc.workspace = true rand.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/mod.rs b/tooling/nargo/src/ops/mod.rs index 53d55b5d750..cada2f0e915 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -12,7 +12,6 @@ pub use self::test::{run_test, TestStatus}; mod compile; mod execute; mod foreign_calls; -pub mod fuzz; mod optimize; mod test; mod transform; diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index 0d934322c75..aad7f93ff75 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -32,6 +32,7 @@ noirc_driver.workspace = true noirc_frontend = { workspace = true, features = ["bn254"] } noirc_abi.workspace = true noirc_errors.workspace = true +noir_fuzzer.workspace = true acvm = { workspace = true, features = ["bn254"] } bn254_blackbox_solver.workspace = true toml.workspace = true diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index de4f54bd605..b548336275b 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -6,7 +6,6 @@ use clap::Args; use nargo::artifacts::debug::DebugArtifact; use nargo::constants::PROVER_INPUT_FILE; use nargo::errors::try_to_diagnose_runtime_error; -use nargo::ops::fuzz::FuzzedExecutor; use nargo::ops::DefaultForeignCallExecutor; use nargo::package::Package; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; @@ -14,7 +13,6 @@ use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::InputMap; use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::graph::CrateName; -use proptest::test_runner::TestRunner; use super::compile_cmd::compile_workspace_full; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; @@ -114,13 +112,6 @@ pub(crate) fn execute_program( ) -> Result, CliError> { let initial_witness = compiled_program.abi.encode(inputs_map, None)?; - let runner = TestRunner::default(); - let fuzzer = FuzzedExecutor::new(compiled_program.clone().into(), runner); - - let result = fuzzer.fuzz(); - - println!("{result:?}"); - let solved_witness_stack_err = nargo::ops::execute_program( &compiled_program.program, initial_witness, diff --git a/tooling/nargo_cli/src/cli/fuzz_cmd.rs b/tooling/nargo_cli/src/cli/fuzz_cmd.rs new file mode 100644 index 00000000000..430e7b2d79b --- /dev/null +++ b/tooling/nargo_cli/src/cli/fuzz_cmd.rs @@ -0,0 +1,71 @@ +use clap::Args; + +use nargo::artifacts::program::ProgramArtifact; +use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; +use noir_fuzzer::FuzzedExecutor; +use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; +use noirc_frontend::graph::CrateName; +use proptest::test_runner::TestRunner; + +use super::compile_cmd::compile_workspace_full; +use super::NargoConfig; +use crate::cli::fs::program::read_program_from_file; +use crate::errors::CliError; + +/// Executes a circuit to calculate its return value +#[derive(Debug, Clone, Args)] +#[clap(visible_alias = "e")] +pub(crate) struct FuzzCommand { + /// The name of the package to execute + #[clap(long, conflicts_with = "workspace")] + package: Option, + + /// Execute all packages in the workspace + #[clap(long, conflicts_with = "package")] + workspace: bool, + + #[clap(flatten)] + compile_options: CompileOptions, + + /// JSON RPC url to solve oracle calls + #[clap(long)] + oracle_resolver: Option, +} + +pub(crate) fn run(args: FuzzCommand, config: NargoConfig) -> Result<(), CliError> { + let toml_path = get_package_manifest(&config.program_dir)?; + let default_selection = + if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll }; + let selection = args.package.map_or(default_selection, PackageSelection::Selected); + let workspace = resolve_workspace_from_toml( + &toml_path, + selection, + Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), + )?; + + // Compile the full workspace in order to generate any build artifacts. + compile_workspace_full(&workspace, &args.compile_options)?; + + let binary_packages = workspace.into_iter().filter(|package| package.is_binary()); + for package in binary_packages { + let program_artifact_path = workspace.package_build_path(package); + let program: CompiledProgram = read_program_from_file(program_artifact_path)?.into(); + + fuzz_program(program.into(), args.oracle_resolver.as_deref())?; + } + Ok(()) +} + +fn fuzz_program( + compiled_program: ProgramArtifact, + _foreign_call_resolver_url: Option<&str>, +) -> Result<(), CliError> { + let runner = TestRunner::default(); + let fuzzer = FuzzedExecutor::new(compiled_program, runner); + + let result = fuzzer.fuzz(); + + println!("{result:?}"); + + Ok(()) +} diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 485ccc7abaf..53a09f61736 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -15,6 +15,7 @@ mod debug_cmd; mod execute_cmd; mod export_cmd; mod fmt_cmd; +mod fuzz_cmd; mod info_cmd; mod init_cmd; mod lsp_cmd; @@ -66,6 +67,7 @@ enum NargoCommand { #[command(hide = true)] // Hidden while the feature is being built out Debug(debug_cmd::DebugCommand), Test(test_cmd::TestCommand), + Fuzz(fuzz_cmd::FuzzCommand), Info(info_cmd::InfoCommand), Lsp(lsp_cmd::LspCommand), #[command(hide = true)] @@ -98,6 +100,7 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::Execute(args) => execute_cmd::run(args, config), NargoCommand::Export(args) => export_cmd::run(args, config), NargoCommand::Test(args) => test_cmd::run(args, config), + NargoCommand::Fuzz(args) => fuzz_cmd::run(args, config), NargoCommand::Info(args) => info_cmd::run(args, config), NargoCommand::Lsp(args) => lsp_cmd::run(args, config), NargoCommand::Dap(args) => dap_cmd::run(args, config), diff --git a/tooling/noirc_abi/src/arbitrary.rs b/tooling/noirc_abi/src/arbitrary.rs index ef3630c2550..c68d36d1576 100644 --- a/tooling/noirc_abi/src/arbitrary.rs +++ b/tooling/noirc_abi/src/arbitrary.rs @@ -1,19 +1,15 @@ -use iter_extended::{btree_map, vecmap}; use prop::collection::vec; use proptest::prelude::*; use acvm::{AcirField, FieldElement}; -use crate::{ - input_parser::InputValue, Abi, AbiParameter, AbiReturnType, AbiType, AbiVisibility, InputMap, - Sign, -}; +use crate::{input_parser::InputValue, Abi, AbiType, InputMap, Sign}; use std::collections::{BTreeMap, HashSet}; pub(super) use proptest_derive::Arbitrary; /// Mutates an iterator of mutable references to [`String`]s to ensure that all values are unique. -fn ensure_unique_strings<'a>(iter: impl Iterator) { +pub(super) fn ensure_unique_strings<'a>(iter: impl Iterator) { let mut seen_values: HashSet = HashSet::default(); for value in iter { while seen_values.contains(value.as_str()) { @@ -32,7 +28,7 @@ proptest::prop_compose! { } } -fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { +pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { match abi_type { AbiType::Field => vec(any::(), 32) .prop_map(|bytes| InputValue::Field(FieldElement::from_be_bytes_reduce(&bytes))) @@ -124,22 +120,6 @@ pub(super) fn arb_abi_type() -> BoxedStrategy { .boxed() } -fn arb_abi_param_and_value() -> BoxedStrategy<(AbiParameter, InputValue)> { - arb_abi_type() - .prop_flat_map(|typ| { - let value = arb_value_from_abi_type(&typ); - let param = arb_abi_param(typ); - (param, value) - }) - .boxed() -} - -fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { - (".+", any::()) - .prop_map(move |(name, visibility)| AbiParameter { name, typ: typ.clone(), visibility }) - .sboxed() -} - pub fn arb_input_map(abi: &Abi) -> BoxedStrategy { let values: Vec<_> = abi .parameters @@ -154,17 +134,3 @@ pub fn arb_input_map(abi: &Abi) -> BoxedStrategy { }) .boxed() } - -prop_compose! { - pub(super) fn arb_abi_and_input_map() - (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type: Option) - -> (Abi, InputMap) { - // Require that all parameter names are unique. - ensure_unique_strings(parameters_with_values.iter_mut().map(|(param_name,_)| &mut param_name.name)); - - let parameters = vecmap(¶meters_with_values, |(param, _)| param.clone()); - let input_map = btree_map(parameters_with_values, |(param, value)| (param.name, value)); - - (Abi { parameters, return_type, error_types: BTreeMap::default() }, input_map) - } -} diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index bc9f9d6d947..ee2329111ac 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -496,9 +496,46 @@ pub fn display_abi_error( #[cfg(test)] mod test { + use std::collections::BTreeMap; + + use iter_extended::{btree_map, vecmap}; use proptest::prelude::*; - use crate::arbitrary::arb_abi_and_input_map; + use crate::{ + arbitrary::{arb_abi_type, arb_value_from_abi_type, ensure_unique_strings}, + input_parser::InputValue, + Abi, AbiParameter, AbiType, AbiVisibility, InputMap, + }; + + fn arb_abi_param_and_value() -> BoxedStrategy<(AbiParameter, InputValue)> { + arb_abi_type() + .prop_flat_map(|typ| { + let value = arb_value_from_abi_type(&typ); + let param = arb_abi_param(typ); + (param, value) + }) + .boxed() + } + + fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { + (".+", any::()) + .prop_map(move |(name, visibility)| AbiParameter { name, typ: typ.clone(), visibility }) + .sboxed() + } + + prop_compose! { + pub(super) fn arb_abi_and_input_map() + (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type: Option) + -> (Abi, InputMap) { + // Require that all parameter names are unique. + ensure_unique_strings(parameters_with_values.iter_mut().map(|(param_name,_)| &mut param_name.name)); + + let parameters = vecmap(¶meters_with_values, |(param, _)| param.clone()); + let input_map = btree_map(parameters_with_values, |(param, value)| (param.name, value)); + + (Abi { parameters, return_type, error_types: BTreeMap::default() }, input_map) + } + } proptest! { #[test] From c398302d70363a4bfee6aa061e7465f9b8f8a3cc Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 15 Jun 2024 14:02:01 +0100 Subject: [PATCH 03/17] feat: hook fuzzing up to test functions --- .../src/hir/resolution/resolver.rs | 16 ++--- temp/Nargo.toml | 7 -- temp/src/main.nr | 4 -- tooling/nargo_cli/src/cli/fuzz_cmd.rs | 71 ------------------- tooling/nargo_cli/src/cli/mod.rs | 3 - tooling/nargo_cli/src/cli/test_cmd.rs | 52 +++++++++++--- 6 files changed, 51 insertions(+), 102 deletions(-) delete mode 100644 temp/Nargo.toml delete mode 100644 temp/src/main.nr delete mode 100644 tooling/nargo_cli/src/cli/fuzz_cmd.rs diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 133f971da19..bbe8dabb3fb 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -23,7 +23,7 @@ use crate::hir_def::expr::{ use crate::hir_def::function::FunctionBody; use crate::hir_def::traits::{Trait, TraitConstraint}; use crate::macros_api::SecondaryAttribute; -use crate::token::{Attributes, FunctionAttribute}; +use crate::token::Attributes; use regex::Regex; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::rc::Rc; @@ -1042,13 +1042,13 @@ impl<'a> Resolver<'a> { }); } - if matches!(attributes.function, Some(FunctionAttribute::Test { .. })) - && !parameters.is_empty() - { - self.push_err(ResolverError::TestFunctionHasParameters { - span: func.name_ident().span(), - }); - } + // if matches!(attributes.function, Some(FunctionAttribute::Test { .. })) + // && !parameters.is_empty() + // { + // self.push_err(ResolverError::TestFunctionHasParameters { + // span: func.name_ident().span(), + // }); + // } let mut typ = Type::Function(parameter_types, return_type, Box::new(Type::Unit)); diff --git a/temp/Nargo.toml b/temp/Nargo.toml deleted file mode 100644 index 6b9591f1a13..00000000000 --- a/temp/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "temp" -type = "bin" -authors = [""] -compiler_version = ">=0.30.0" - -[dependencies] \ No newline at end of file diff --git a/temp/src/main.nr b/temp/src/main.nr deleted file mode 100644 index 36b35d1f385..00000000000 --- a/temp/src/main.nr +++ /dev/null @@ -1,4 +0,0 @@ -fn main(x: i64, y: pub i64) { - assert(x != y); -} - diff --git a/tooling/nargo_cli/src/cli/fuzz_cmd.rs b/tooling/nargo_cli/src/cli/fuzz_cmd.rs deleted file mode 100644 index 430e7b2d79b..00000000000 --- a/tooling/nargo_cli/src/cli/fuzz_cmd.rs +++ /dev/null @@ -1,71 +0,0 @@ -use clap::Args; - -use nargo::artifacts::program::ProgramArtifact; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noir_fuzzer::FuzzedExecutor; -use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; -use noirc_frontend::graph::CrateName; -use proptest::test_runner::TestRunner; - -use super::compile_cmd::compile_workspace_full; -use super::NargoConfig; -use crate::cli::fs::program::read_program_from_file; -use crate::errors::CliError; - -/// Executes a circuit to calculate its return value -#[derive(Debug, Clone, Args)] -#[clap(visible_alias = "e")] -pub(crate) struct FuzzCommand { - /// The name of the package to execute - #[clap(long, conflicts_with = "workspace")] - package: Option, - - /// Execute all packages in the workspace - #[clap(long, conflicts_with = "package")] - workspace: bool, - - #[clap(flatten)] - compile_options: CompileOptions, - - /// JSON RPC url to solve oracle calls - #[clap(long)] - oracle_resolver: Option, -} - -pub(crate) fn run(args: FuzzCommand, config: NargoConfig) -> Result<(), CliError> { - let toml_path = get_package_manifest(&config.program_dir)?; - let default_selection = - if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll }; - let selection = args.package.map_or(default_selection, PackageSelection::Selected); - let workspace = resolve_workspace_from_toml( - &toml_path, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; - - // Compile the full workspace in order to generate any build artifacts. - compile_workspace_full(&workspace, &args.compile_options)?; - - let binary_packages = workspace.into_iter().filter(|package| package.is_binary()); - for package in binary_packages { - let program_artifact_path = workspace.package_build_path(package); - let program: CompiledProgram = read_program_from_file(program_artifact_path)?.into(); - - fuzz_program(program.into(), args.oracle_resolver.as_deref())?; - } - Ok(()) -} - -fn fuzz_program( - compiled_program: ProgramArtifact, - _foreign_call_resolver_url: Option<&str>, -) -> Result<(), CliError> { - let runner = TestRunner::default(); - let fuzzer = FuzzedExecutor::new(compiled_program, runner); - - let result = fuzzer.fuzz(); - - println!("{result:?}"); - - Ok(()) -} diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 53a09f61736..485ccc7abaf 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -15,7 +15,6 @@ mod debug_cmd; mod execute_cmd; mod export_cmd; mod fmt_cmd; -mod fuzz_cmd; mod info_cmd; mod init_cmd; mod lsp_cmd; @@ -67,7 +66,6 @@ enum NargoCommand { #[command(hide = true)] // Hidden while the feature is being built out Debug(debug_cmd::DebugCommand), Test(test_cmd::TestCommand), - Fuzz(fuzz_cmd::FuzzCommand), Info(info_cmd::InfoCommand), Lsp(lsp_cmd::LspCommand), #[command(hide = true)] @@ -100,7 +98,6 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::Execute(args) => execute_cmd::run(args, config), NargoCommand::Export(args) => export_cmd::run(args, config), NargoCommand::Test(args) => test_cmd::run(args, config), - NargoCommand::Fuzz(args) => fuzz_cmd::run(args, config), NargoCommand::Info(args) => info_cmd::run(args, config), NargoCommand::Lsp(args) => lsp_cmd::run(args, config), NargoCommand::Dap(args) => dap_cmd::run(args, config), diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 99c284e5019..0511f9cafdd 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -10,7 +10,8 @@ use nargo::{ }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ - check_crate, file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING, + check_crate, compile_no_check, file_manager_with_stdlib, CompileOptions, + NOIR_ARTIFACT_VERSION_STRING, }; use noirc_frontend::{ graph::CrateName, @@ -185,14 +186,47 @@ fn run_test + Default>( let blackbox_solver = S::default(); - nargo::ops::run_test( - &blackbox_solver, - &mut context, - test_function, - show_output, - foreign_call_resolver_url, - compile_options, - ) + 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()), + } + } } fn get_tests_in_package( From a26edb30ac38de159471fd33bab34d77fd4b9035 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 15 Jun 2024 14:18:17 +0100 Subject: [PATCH 04/17] chore: restrict usage of proptest in `noirc_abi` --- tooling/noirc_abi/Cargo.toml | 6 +- tooling/noirc_abi/src/arbitrary.rs | 137 +++++++++++++++++------------ tooling/noirc_abi/src/lib.rs | 55 ++---------- 3 files changed, 94 insertions(+), 104 deletions(-) diff --git a/tooling/noirc_abi/Cargo.toml b/tooling/noirc_abi/Cargo.toml index 573a6d8c29a..3920a4222f6 100644 --- a/tooling/noirc_abi/Cargo.toml +++ b/tooling/noirc_abi/Cargo.toml @@ -18,13 +18,13 @@ serde.workspace = true thiserror.workspace = true num-bigint = "0.4" num-traits = "0.2" -proptest.workspace = true -proptest-derive.workspace = true + [dev-dependencies] strum = "0.24" strum_macros = "0.24" - +proptest.workspace = true +proptest-derive.workspace = true [features] bn254 = ["acvm/bn254"] diff --git a/tooling/noirc_abi/src/arbitrary.rs b/tooling/noirc_abi/src/arbitrary.rs index c68d36d1576..b147d9ec3f4 100644 --- a/tooling/noirc_abi/src/arbitrary.rs +++ b/tooling/noirc_abi/src/arbitrary.rs @@ -2,8 +2,12 @@ use prop::collection::vec; use proptest::prelude::*; use acvm::{AcirField, FieldElement}; +use iter_extended::{btree_map, vecmap}; -use crate::{input_parser::InputValue, Abi, AbiType, InputMap, Sign}; +use crate::{ + input_parser::InputValue, Abi, AbiParameter, AbiReturnType, AbiType, AbiVisibility, InputMap, + Sign, +}; use std::collections::{BTreeMap, HashSet}; pub(super) use proptest_derive::Arbitrary; @@ -20,7 +24,7 @@ pub(super) fn ensure_unique_strings<'a>(iter: impl Iterator FieldElement { + fn arb_field_from_integer(bit_size: u32)(value: u128)-> FieldElement { let width = (bit_size % 128).clamp(1, 127); let max_value = 2u128.pow(width) - 1; let value = value % max_value; @@ -28,7 +32,56 @@ proptest::prop_compose! { } } -pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { +fn arb_primitive_abi_type() -> SBoxedStrategy { + const MAX_STRING_LEN: u32 = 1000; + proptest::prop_oneof![ + Just(AbiType::Field), + Just(AbiType::Boolean), + any::<(Sign, u32)>().prop_map(|(sign, width)| { + let width = (width % 128).clamp(1, 127); + AbiType::Integer { sign, width } + }), + // restrict length of strings to avoid running out of memory + (1..MAX_STRING_LEN).prop_map(|length| AbiType::String { length }), + ] + .sboxed() +} + +fn arb_abi_type() -> BoxedStrategy { + let leaf = arb_primitive_abi_type(); + + leaf.prop_recursive( + 8, // up to 8 levels deep + 256, // Shoot for maximum size of 256 nodes + 10, // We put up to 10 items per collection + |inner| { + prop_oneof![ + (1..10u32, inner.clone()) + .prop_map(|(length, typ)| { AbiType::Array { length, typ: Box::new(typ) } }) + .boxed(), + prop::collection::vec(inner.clone(), 1..10) + .prop_map(|fields| { AbiType::Tuple { fields } }) + .boxed(), + (".*", prop::collection::vec((".+", inner), 1..10)) + .prop_map(|(path, mut fields)| { + // Require that all field names are unique. + ensure_unique_strings(fields.iter_mut().map(|(field_name, _)| field_name)); + AbiType::Struct { path, fields } + }) + .boxed(), + ] + }, + ) + .boxed() +} + +fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { + (".+", any::()) + .prop_map(move |(name, visibility)| AbiParameter { name, typ: typ.clone(), visibility }) + .sboxed() +} + +fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { match abi_type { AbiType::Field => vec(any::(), 32) .prop_map(|bytes| InputValue::Field(FieldElement::from_be_bytes_reduce(&bytes))) @@ -77,60 +130,34 @@ pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy SBoxedStrategy { - const MAX_STRING_LEN: u32 = 1000; - proptest::prop_oneof![ - Just(AbiType::Field), - Just(AbiType::Boolean), - any::<(Sign, u32)>().prop_map(|(sign, width)| { - let width = (width % 128).clamp(1, 127); - AbiType::Integer { sign, width } - }), - // restrict length of strings to avoid running out of memory - (1..MAX_STRING_LEN).prop_map(|length| AbiType::String { length }), - ] - .sboxed() +fn arb_abi_param_and_value() -> BoxedStrategy<(AbiParameter, InputValue)> { + arb_abi_type() + .prop_flat_map(|typ| { + let value = arb_value_from_abi_type(&typ); + let param = arb_abi_param(typ); + (param, value) + }) + .boxed() } -pub(super) fn arb_abi_type() -> BoxedStrategy { - let leaf = arb_primitive_abi_type(); - - leaf.prop_recursive( - 8, // up to 8 levels deep - 256, // Shoot for maximum size of 256 nodes - 10, // We put up to 10 items per collection - |inner| { - prop_oneof![ - (1..10u32, inner.clone()) - .prop_map(|(length, typ)| { AbiType::Array { length, typ: Box::new(typ) } }) - .boxed(), - prop::collection::vec(inner.clone(), 1..10) - .prop_map(|fields| { AbiType::Tuple { fields } }) - .boxed(), - (".*", prop::collection::vec((".+", inner), 1..10)) - .prop_map(|(path, mut fields)| { - // Require that all field names are unique. - ensure_unique_strings(fields.iter_mut().map(|(field_name, _)| field_name)); - AbiType::Struct { path, fields } - }) - .boxed(), - ] - }, - ) - .boxed() +prop_compose! { + fn arb_abi_return_type() + (abi_type in arb_abi_type(), visibility: AbiVisibility) + -> AbiReturnType { + AbiReturnType { abi_type, visibility } + } } -pub fn arb_input_map(abi: &Abi) -> BoxedStrategy { - let values: Vec<_> = abi - .parameters - .iter() - .map(|param| (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ))) - .collect(); - - values - .prop_map(|values| { - let input_map: InputMap = values.into_iter().collect(); - input_map - }) - .boxed() +prop_compose! { + pub(super) fn arb_abi_and_input_map() + (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type in proptest::option::of(arb_abi_return_type())) + -> (Abi, InputMap) { + // Require that all parameter names are unique. + ensure_unique_strings(parameters_with_values.iter_mut().map(|(param_name,_)| &mut param_name.name)); + + let parameters = vecmap(¶meters_with_values, |(param, _)| param.clone()); + let input_map = btree_map(parameters_with_values, |(param, value)| (param.name, value)); + + (Abi { parameters, return_type, error_types: BTreeMap::default() }, input_map) + } } diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index ee2329111ac..14115336016 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -25,6 +25,7 @@ use std::{collections::BTreeMap, str}; // // This ABI has nothing to do with ACVM or ACIR. Although they implicitly have a relationship +#[cfg(test)] pub mod arbitrary; pub mod errors; @@ -76,7 +77,8 @@ pub enum AbiType { }, } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, arbitrary::Arbitrary)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[cfg_attr(test, derive(arbitrary::Arbitrary))] #[serde(rename_all = "lowercase")] /// Represents whether the parameter is public or known only to the prover. pub enum AbiVisibility { @@ -87,7 +89,8 @@ pub enum AbiVisibility { DataBus, } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, arbitrary::Arbitrary)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[cfg_attr(test, derive(arbitrary::Arbitrary))] #[serde(rename_all = "lowercase")] pub enum Sign { Unsigned, @@ -143,13 +146,12 @@ impl From<&AbiType> for PrintableType { } } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, arbitrary::Arbitrary)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] /// An argument or return value of the circuit's `main` function. pub struct AbiParameter { pub name: String, #[serde(rename = "type")] - #[proptest(strategy = "arbitrary::arb_abi_type()")] pub typ: AbiType, pub visibility: AbiVisibility, } @@ -160,21 +162,18 @@ impl AbiParameter { } } -#[derive(Clone, Debug, Serialize, Deserialize, arbitrary::Arbitrary)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct AbiReturnType { - #[proptest(strategy = "arbitrary::arb_abi_type()")] pub abi_type: AbiType, pub visibility: AbiVisibility, } -#[derive(Clone, Debug, Serialize, Deserialize, arbitrary::Arbitrary)] - +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct Abi { /// An ordered list of the arguments to the program's `main` function, specifying their types and visibility. pub parameters: Vec, pub return_type: Option, - #[proptest(strategy = "proptest::prelude::Just(BTreeMap::from([]))")] pub error_types: BTreeMap, } @@ -496,46 +495,10 @@ pub fn display_abi_error( #[cfg(test)] mod test { - use std::collections::BTreeMap; - use iter_extended::{btree_map, vecmap}; use proptest::prelude::*; - use crate::{ - arbitrary::{arb_abi_type, arb_value_from_abi_type, ensure_unique_strings}, - input_parser::InputValue, - Abi, AbiParameter, AbiType, AbiVisibility, InputMap, - }; - - fn arb_abi_param_and_value() -> BoxedStrategy<(AbiParameter, InputValue)> { - arb_abi_type() - .prop_flat_map(|typ| { - let value = arb_value_from_abi_type(&typ); - let param = arb_abi_param(typ); - (param, value) - }) - .boxed() - } - - fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { - (".+", any::()) - .prop_map(move |(name, visibility)| AbiParameter { name, typ: typ.clone(), visibility }) - .sboxed() - } - - prop_compose! { - pub(super) fn arb_abi_and_input_map() - (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type: Option) - -> (Abi, InputMap) { - // Require that all parameter names are unique. - ensure_unique_strings(parameters_with_values.iter_mut().map(|(param_name,_)| &mut param_name.name)); - - let parameters = vecmap(¶meters_with_values, |(param, _)| param.clone()); - let input_map = btree_map(parameters_with_values, |(param, value)| (param.name, value)); - - (Abi { parameters, return_type, error_types: BTreeMap::default() }, input_map) - } - } + use crate::arbitrary::arb_abi_and_input_map; proptest! { #[test] From 845447462cf319eb07e800c99126d7c53dcf510c Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 15 Jun 2024 14:25:12 +0100 Subject: [PATCH 05/17] chore: restrict visibility --- tooling/noirc_abi/src/arbitrary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/noirc_abi/src/arbitrary.rs b/tooling/noirc_abi/src/arbitrary.rs index b147d9ec3f4..44a323ecd4b 100644 --- a/tooling/noirc_abi/src/arbitrary.rs +++ b/tooling/noirc_abi/src/arbitrary.rs @@ -13,7 +13,7 @@ use std::collections::{BTreeMap, HashSet}; pub(super) use proptest_derive::Arbitrary; /// Mutates an iterator of mutable references to [`String`]s to ensure that all values are unique. -pub(super) fn ensure_unique_strings<'a>(iter: impl Iterator) { +fn ensure_unique_strings<'a>(iter: impl Iterator) { let mut seen_values: HashSet = HashSet::default(); for value in iter { while seen_values.contains(value.as_str()) { From fbb97aeb78f3352eed44087cf29434793355f515 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 15 Jun 2024 15:23:35 +0100 Subject: [PATCH 06/17] chore: fuzz stdlib --- noir_stdlib/src/uint128.nr | 18 +++++--- tooling/nargo_cli/tests/stdlib-tests.rs | 57 ++++++++++++++++++++----- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index 173fa54863a..829ab09ee1e 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -319,13 +319,12 @@ mod tests { use crate::uint128::{U128, pow64, pow63}; #[test] - fn test_not() { - let num = U128::from_u64s_le(0, 0); + fn test_not(lo: u64, hi: u64) { + let num = U128::from_u64s_le(lo, hi); let not_num = num.not(); - let max_u64: Field = pow64 - 1; - assert_eq(not_num.hi, max_u64); - assert_eq(not_num.lo, max_u64); + assert_eq(not_num.hi, (hi.not() as Field)); + assert_eq(not_num.lo, (lo.not() as Field)); let not_not_num = not_num.not(); assert_eq(num, not_not_num); @@ -493,6 +492,15 @@ mod tests { let end = a.to_integer(); assert_eq(start, end); } + + #[test] + fn integer_conversions_fuzz(lo: u64, hi: u64) { + let start: Field = (lo as Field) + pow64 * (hi as Field); + let a = U128::from_integer(start); + let end = a.to_integer(); + assert_eq(start, end); + } + #[test] fn test_wrapping_mul() { // 1*0==0 diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 70857b4b65e..5badd76605a 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -3,7 +3,7 @@ use std::{collections::BTreeMap, path::PathBuf}; use acvm::blackbox_solver::StubbedBlackBoxSolver; use fm::FileManager; -use noirc_driver::{check_crate, file_manager_with_stdlib, CompileOptions}; +use noirc_driver::{check_crate, compile_no_check, file_manager_with_stdlib, CompileOptions}; use noirc_frontend::hir::FunctionNameMatch; use nargo::{ @@ -47,15 +47,52 @@ fn run_stdlib_tests() { let test_report: Vec<(String, TestStatus)> = test_functions .into_iter() .map(|(test_name, test_function)| { - let status = run_test( - &StubbedBlackBoxSolver, - &mut context, - &test_function, - false, - None, - &CompileOptions::default(), - ); - + 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( + &StubbedBlackBoxSolver, + &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()), + } + }; (test_name, status) }) .collect(); From c36130e1a927b8715e84766e94e993b250a88cad Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 15 Jun 2024 18:38:39 +0100 Subject: [PATCH 07/17] chore: use `proptest`'s value tree implementations --- tooling/fuzzer/src/lib.rs | 4 +- tooling/fuzzer/src/strategies/int.rs | 120 ++------------------------ tooling/fuzzer/src/strategies/uint.rs | 104 +++------------------- tooling/fuzzer/src/types.rs | 8 +- 4 files changed, 25 insertions(+), 211 deletions(-) diff --git a/tooling/fuzzer/src/lib.rs b/tooling/fuzzer/src/lib.rs index d523fc371be..fc4d5b35e3e 100644 --- a/tooling/fuzzer/src/lib.rs +++ b/tooling/fuzzer/src/lib.rs @@ -18,12 +18,13 @@ use nargo::artifacts::program::ProgramArtifact; use nargo::ops::{execute_program, DefaultForeignCallExecutor}; -/// Wrapper around an [`Executor`] which provides fuzzing support using [`proptest`]. +/// 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 { + /// The program to be fuzzed program: ProgramArtifact, /// The fuzzer @@ -54,7 +55,6 @@ impl FuzzedExecutor { FuzzOutcome::CounterExample(CounterExampleOutcome { exit_reason: status, counterexample: outcome, - .. }) => { println!("found counterexample, {outcome:?}"); // We cannot use the calldata returned by the test runner in `TestError::Fail`, diff --git a/tooling/fuzzer/src/strategies/int.rs b/tooling/fuzzer/src/strategies/int.rs index c529d2f9501..d11cafcfae5 100644 --- a/tooling/fuzzer/src/strategies/int.rs +++ b/tooling/fuzzer/src/strategies/int.rs @@ -1,97 +1,17 @@ use proptest::{ - strategy::{NewTree, Strategy, ValueTree}, + strategy::{NewTree, Strategy}, test_runner::TestRunner, }; use rand::Rng; -/// Value tree for signed ints (up to int256). -pub struct IntValueTree { - /// Lower base (by absolute value) - lo: i128, - /// Current value - curr: i128, - /// Higher base (by absolute value) - hi: i128, - /// If true cannot be simplified or complexified - fixed: bool, -} - -impl IntValueTree { - /// Create a new tree - /// # Arguments - /// * `start` - Starting value for the tree - /// * `fixed` - If `true` the tree would only contain one element and won't be simplified. - fn new(start: i128, fixed: bool) -> Self { - Self { lo: 0, curr: start, hi: start, fixed } - } - - fn reposition(&mut self) -> bool { - let interval = self.hi - self.lo; - let new_mid = self.lo + interval / 2i128; - - if new_mid == self.curr { - false - } else { - self.curr = new_mid; - true - } - } - - fn magnitude_greater(lhs: i128, rhs: i128) -> bool { - if lhs == 0 { - return false; - } - (lhs > rhs) ^ (lhs.is_negative()) - } -} - -impl ValueTree for IntValueTree { - type Value = i128; - - fn current(&self) -> Self::Value { - self.curr - } - - fn simplify(&mut self) -> bool { - if self.fixed || !Self::magnitude_greater(self.hi, self.lo) { - return false; - } - self.hi = self.curr; - self.reposition() - } - - fn complicate(&mut self) -> bool { - if self.fixed || !Self::magnitude_greater(self.hi, self.lo) { - return false; - } - - self.lo = if self.curr != i128::MIN && self.curr != i128::MAX { - self.curr + if self.hi.is_negative() { -1i128 } else { 1i128 } - } else { - self.curr - }; - - self.reposition() - } -} - -/// Value tree for signed ints (up to int256). -/// The strategy combines 3 different strategies, each assigned a specific weight: +/// Strategy for signed ints (up to i128). +/// The strategy combines 2 different strategies, each assigned a specific weight: /// 1. Generate purely random value in a range. This will first choose bit size uniformly (up `bits` /// param). Then generate a value for this bit size. /// 2. Generate a random value around the edges (+/- 3 around min, 0 and max possible value) -/// 3. Generate a value from a predefined fixtures set -/// -/// To define int fixtures: -/// - return an array of possible values for a parameter named `amount` declare a function `function -/// fixture_amount() public returns (int32[] memory)`. -/// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values -/// `function testFuzz_int32(int32 amount)`. -/// -/// If fixture is not a valid int type then error is raised and random value generated. #[derive(Debug)] pub struct IntStrategy { - /// Bit size of int (e.g. 256) + /// Bit size of int (e.g. 128) bits: usize, /// The weight for edge cases (+/- 3 around 0 and max possible value) edge_weight: usize, @@ -101,9 +21,8 @@ pub struct IntStrategy { impl IntStrategy { /// Create a new strategy. - /// #Arguments - /// * `bits` - Size of uint in bits - /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) + /// # Arguments + /// * `bits` - Size of int in bits pub fn new(bits: usize) -> Self { Self { bits, edge_weight: 10usize, random_weight: 50usize } } @@ -121,14 +40,14 @@ impl IntStrategy { 3 => self.type_max() - offset, _ => unreachable!(), }; - Ok(IntValueTree::new(start, false)) + Ok(proptest::num::i128::BinarySearch::new(start)) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); let start: i128 = rng.gen_range(self.type_min()..=self.type_max()); - Ok(IntValueTree::new(start, false)) + Ok(proptest::num::i128::BinarySearch::new(start)) } fn type_max(&self) -> i128 { @@ -149,7 +68,7 @@ impl IntStrategy { } impl Strategy for IntStrategy { - type Tree = IntValueTree; + type Tree = proptest::num::i128::BinarySearch; type Value = i128; fn new_tree(&self, runner: &mut TestRunner) -> NewTree { @@ -162,24 +81,3 @@ impl Strategy for IntStrategy { } } } - -#[cfg(test)] -mod tests { - use crate::strategies::int::IntValueTree; - use proptest::strategy::ValueTree; - - #[test] - fn test_int_tree_complicate_should_not_overflow() { - let mut int_tree = IntValueTree::new(i128::MAX, false); - assert_eq!(int_tree.hi, i128::MAX); - assert_eq!(int_tree.curr, i128::MAX); - int_tree.complicate(); - assert_eq!(int_tree.lo, i128::MAX); - - let mut int_tree = IntValueTree::new(i128::MIN, false); - assert_eq!(int_tree.hi, i128::MIN); - assert_eq!(int_tree.curr, i128::MIN); - int_tree.complicate(); - assert_eq!(int_tree.lo, i128::MIN); - } -} diff --git a/tooling/fuzzer/src/strategies/uint.rs b/tooling/fuzzer/src/strategies/uint.rs index a7ecf89e5e1..5021e832b97 100644 --- a/tooling/fuzzer/src/strategies/uint.rs +++ b/tooling/fuzzer/src/strategies/uint.rs @@ -1,85 +1,17 @@ use proptest::{ - strategy::{NewTree, Strategy, ValueTree}, + strategy::{NewTree, Strategy}, test_runner::TestRunner, }; use rand::Rng; -/// Value tree for unsigned ints (up to uint256). -pub struct UintValueTree { - /// Lower base - lo: u128, - /// Current value - curr: u128, - /// Higher base - hi: u128, - /// If true cannot be simplified or complexified - fixed: bool, -} - -impl UintValueTree { - /// Create a new tree - /// # Arguments - /// * `start` - Starting value for the tree - /// * `fixed` - If `true` the tree would only contain one element and won't be simplified. - fn new(start: u128, fixed: bool) -> Self { - Self { lo: 0, curr: start, hi: start, fixed } - } - - fn reposition(&mut self) -> bool { - let interval = self.hi - self.lo; - let new_mid = self.lo + interval / 2; - - if new_mid == self.curr { - false - } else { - self.curr = new_mid; - true - } - } -} - -impl ValueTree for UintValueTree { - type Value = u128; - - fn current(&self) -> Self::Value { - self.curr - } - - fn simplify(&mut self) -> bool { - if self.fixed || (self.hi <= self.lo) { - return false; - } - self.hi = self.curr; - self.reposition() - } - - fn complicate(&mut self) -> bool { - if self.fixed || (self.hi <= self.lo) { - return false; - } - - self.lo = self.curr.wrapping_add(1); - self.reposition() - } -} - -/// Value tree for unsigned ints (up to uint256). -/// The strategy combines 3 different strategies, each assigned a specific weight: +/// Value tree for unsigned ints (up to u128). +/// The strategy combines 2 different strategies, each assigned a specific weight: /// 1. Generate purely random value in a range. This will first choose bit size uniformly (up `bits` /// param). Then generate a value for this bit size. /// 2. Generate a random value around the edges (+/- 3 around 0 and max possible value) -/// 3. Generate a value from a predefined fixtures set -/// -/// To define uint fixtures: -/// - return an array of possible values for a parameter named `amount` declare a function `function -/// fixture_amount() public returns (uint32[] memory)`. -/// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values -/// `function testFuzz_uint32(uint32 amount)`. -/// -/// If fixture is not a valid uint type then error is raised and random value generated. #[derive(Debug)] pub struct UintStrategy { - /// Bit size of uint (e.g. 256) + /// Bit size of uint (e.g. 128) bits: usize, /// The weight for edge cases (+/- 3 around 0 and max possible value) @@ -90,9 +22,8 @@ pub struct UintStrategy { impl UintStrategy { /// Create a new strategy. - /// #Arguments + /// # Arguments /// * `bits` - Size of uint in bits - /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) pub fn new(bits: usize) -> Self { Self { bits, edge_weight: 10usize, random_weight: 50usize } } @@ -103,14 +34,14 @@ impl UintStrategy { let is_min = rng.gen_bool(0.5); let offset = rng.gen_range(0..4); let start = if is_min { offset } else { self.type_max().saturating_sub(offset) }; - Ok(UintValueTree::new(start, false)) + Ok(proptest::num::u128::BinarySearch::new(start)) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); - let start: u128 = rng.gen_range(0..=self.type_max()); + let start = rng.gen_range(0..=self.type_max()); - Ok(UintValueTree::new(start, false)) + Ok(proptest::num::u128::BinarySearch::new(start)) } fn type_max(&self) -> u128 { @@ -123,30 +54,15 @@ impl UintStrategy { } impl Strategy for UintStrategy { - type Tree = UintValueTree; + type Tree = proptest::num::u128::BinarySearch; type Value = u128; fn new_tree(&self, runner: &mut TestRunner) -> NewTree { let total_weight = self.random_weight + self.edge_weight; let bias = runner.rng().gen_range(0..total_weight); - // randomly select one of 3 strategies + // randomly select one of 2 strategies match bias { x if x < self.edge_weight => self.generate_edge_tree(runner), _ => self.generate_random_tree(runner), } } } - -#[cfg(test)] -mod tests { - use crate::strategies::uint::UintValueTree; - use proptest::strategy::ValueTree; - - #[test] - fn test_uint_tree_complicate_max() { - let mut uint_tree = UintValueTree::new(u128::MAX, false); - assert_eq!(uint_tree.hi, u128::MAX); - assert_eq!(uint_tree.curr, u128::MAX); - uint_tree.complicate(); - assert_eq!(uint_tree.lo, u128::MIN); - } -} diff --git a/tooling/fuzzer/src/types.rs b/tooling/fuzzer/src/types.rs index 12daf5bb012..dbd4ba94ec1 100644 --- a/tooling/fuzzer/src/types.rs +++ b/tooling/fuzzer/src/types.rs @@ -5,12 +5,12 @@ type CounterExample = InputMap; /// The outcome of a fuzz test #[derive(Debug)] pub struct FuzzTestResult { - /// Whether the test case was successful. This means that the transaction executed - /// properly, or that there was a revert and that the test was expected to fail - /// (prefixed with `testFail`) + /// Whether the test case was successful. This means that the program executed + /// properly, or that there was a constraint failure and that the test was expected to fail + /// (has the `should_fail` attribute) pub success: bool, - /// If there was a revert, this field will be populated. Note that the test can + /// If there was a constraint failure, this field will be populated. Note that the test can /// still be successful (i.e self.success == true) when it's expected to fail. pub reason: Option, From cb2a8850e8f1869f60ed182b3d951737d606e709 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 16 Jun 2024 15:01:53 +0100 Subject: [PATCH 08/17] chore: allow test functions with args in elaborator --- compiler/noirc_frontend/src/elaborator/lints.rs | 9 --------- compiler/noirc_frontend/src/elaborator/mod.rs | 1 - 2 files changed, 10 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/lints.rs b/compiler/noirc_frontend/src/elaborator/lints.rs index 4859ac5f97c..b86912940eb 100644 --- a/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/compiler/noirc_frontend/src/elaborator/lints.rs @@ -130,15 +130,6 @@ pub(super) fn recursive_non_entrypoint_function( } } -/// Test functions cannot have arguments in order to be executable. -pub(super) fn test_function_with_args(func: &NoirFunction) -> Option { - if func.attributes().is_test_function() && !func.parameters().is_empty() { - Some(ResolverError::TestFunctionHasParameters { span: func.name_ident().span() }) - } else { - None - } -} - /// Check that we are not passing a mutable reference from a constrained runtime to an unconstrained runtime. pub(super) fn unconstrained_function_args( function_args: &[(Type, ExprId, Span)], diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 93e8b36f4e0..3ad587a770a 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -571,7 +571,6 @@ impl<'context> Elaborator<'context> { self.run_lint(|elaborator| { lints::low_level_function_outside_stdlib(func, elaborator.crate_id).map(Into::into) }); - self.run_lint(|_| lints::test_function_with_args(func).map(Into::into)); self.run_lint(|_| { lints::recursive_non_entrypoint_function(func, is_entry_point).map(Into::into) }); From c5ffa8f09ae711ec5cc1ef4d6c7a5508427f3f6a Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 16 Jun 2024 15:07:50 +0100 Subject: [PATCH 09/17] chore: revert changes to `noirc_abi` --- tooling/noirc_abi/src/arbitrary.rs | 111 +++++++++++++---------------- tooling/noirc_abi/src/lib.rs | 11 +-- 2 files changed, 58 insertions(+), 64 deletions(-) diff --git a/tooling/noirc_abi/src/arbitrary.rs b/tooling/noirc_abi/src/arbitrary.rs index 44a323ecd4b..aecb620b79d 100644 --- a/tooling/noirc_abi/src/arbitrary.rs +++ b/tooling/noirc_abi/src/arbitrary.rs @@ -1,8 +1,8 @@ +use iter_extended::{btree_map, vecmap}; use prop::collection::vec; use proptest::prelude::*; use acvm::{AcirField, FieldElement}; -use iter_extended::{btree_map, vecmap}; use crate::{ input_parser::InputValue, Abi, AbiParameter, AbiReturnType, AbiType, AbiVisibility, InputMap, @@ -24,63 +24,13 @@ fn ensure_unique_strings<'a>(iter: impl Iterator) { } proptest::prop_compose! { - fn arb_field_from_integer(bit_size: u32)(value: u128)-> FieldElement { + pub(super) fn arb_field_from_integer(bit_size: u32)(value: u128)-> FieldElement { let width = (bit_size % 128).clamp(1, 127); let max_value = 2u128.pow(width) - 1; - let value = value % max_value; - FieldElement::from(value) + FieldElement::from(value.clamp(0, max_value)) } } -fn arb_primitive_abi_type() -> SBoxedStrategy { - const MAX_STRING_LEN: u32 = 1000; - proptest::prop_oneof![ - Just(AbiType::Field), - Just(AbiType::Boolean), - any::<(Sign, u32)>().prop_map(|(sign, width)| { - let width = (width % 128).clamp(1, 127); - AbiType::Integer { sign, width } - }), - // restrict length of strings to avoid running out of memory - (1..MAX_STRING_LEN).prop_map(|length| AbiType::String { length }), - ] - .sboxed() -} - -fn arb_abi_type() -> BoxedStrategy { - let leaf = arb_primitive_abi_type(); - - leaf.prop_recursive( - 8, // up to 8 levels deep - 256, // Shoot for maximum size of 256 nodes - 10, // We put up to 10 items per collection - |inner| { - prop_oneof![ - (1..10u32, inner.clone()) - .prop_map(|(length, typ)| { AbiType::Array { length, typ: Box::new(typ) } }) - .boxed(), - prop::collection::vec(inner.clone(), 1..10) - .prop_map(|fields| { AbiType::Tuple { fields } }) - .boxed(), - (".*", prop::collection::vec((".+", inner), 1..10)) - .prop_map(|(path, mut fields)| { - // Require that all field names are unique. - ensure_unique_strings(fields.iter_mut().map(|(field_name, _)| field_name)); - AbiType::Struct { path, fields } - }) - .boxed(), - ] - }, - ) - .boxed() -} - -fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { - (".+", any::()) - .prop_map(move |(name, visibility)| AbiParameter { name, typ: typ.clone(), visibility }) - .sboxed() -} - fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { match abi_type { AbiType::Field => vec(any::(), 32) @@ -130,6 +80,49 @@ fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { } } +fn arb_primitive_abi_type() -> SBoxedStrategy { + const MAX_STRING_LEN: u32 = 1000; + proptest::prop_oneof![ + Just(AbiType::Field), + Just(AbiType::Boolean), + any::<(Sign, u32)>().prop_map(|(sign, width)| { + let width = (width % 128).clamp(1, 127); + AbiType::Integer { sign, width } + }), + // restrict length of strings to avoid running out of memory + (1..MAX_STRING_LEN).prop_map(|length| AbiType::String { length }), + ] + .sboxed() +} + +pub(super) fn arb_abi_type() -> BoxedStrategy { + let leaf = arb_primitive_abi_type(); + + leaf.prop_recursive( + 8, // up to 8 levels deep + 256, // Shoot for maximum size of 256 nodes + 10, // We put up to 10 items per collection + |inner| { + prop_oneof![ + (1..10u32, inner.clone()) + .prop_map(|(length, typ)| { AbiType::Array { length, typ: Box::new(typ) } }) + .boxed(), + prop::collection::vec(inner.clone(), 1..10) + .prop_map(|fields| { AbiType::Tuple { fields } }) + .boxed(), + (".*", prop::collection::vec((".+", inner), 1..10)) + .prop_map(|(path, mut fields)| { + // Require that all field names are unique. + ensure_unique_strings(fields.iter_mut().map(|(field_name, _)| field_name)); + AbiType::Struct { path, fields } + }) + .boxed(), + ] + }, + ) + .boxed() +} + fn arb_abi_param_and_value() -> BoxedStrategy<(AbiParameter, InputValue)> { arb_abi_type() .prop_flat_map(|typ| { @@ -140,17 +133,15 @@ fn arb_abi_param_and_value() -> BoxedStrategy<(AbiParameter, InputValue)> { .boxed() } -prop_compose! { - fn arb_abi_return_type() - (abi_type in arb_abi_type(), visibility: AbiVisibility) - -> AbiReturnType { - AbiReturnType { abi_type, visibility } - } +fn arb_abi_param(typ: AbiType) -> SBoxedStrategy { + (".+", any::()) + .prop_map(move |(name, visibility)| AbiParameter { name, typ: typ.clone(), visibility }) + .sboxed() } prop_compose! { pub(super) fn arb_abi_and_input_map() - (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type in proptest::option::of(arb_abi_return_type())) + (mut parameters_with_values in proptest::collection::vec(arb_abi_param_and_value(), 0..100), return_type: Option) -> (Abi, InputMap) { // Require that all parameter names are unique. ensure_unique_strings(parameters_with_values.iter_mut().map(|(param_name,_)| &mut param_name.name)); diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 14115336016..b3d80099137 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -26,7 +26,7 @@ use std::{collections::BTreeMap, str}; // This ABI has nothing to do with ACVM or ACIR. Although they implicitly have a relationship #[cfg(test)] -pub mod arbitrary; +mod arbitrary; pub mod errors; pub mod input_parser; @@ -147,11 +147,12 @@ impl From<&AbiType> for PrintableType { } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] - +#[cfg_attr(test, derive(arbitrary::Arbitrary))] /// An argument or return value of the circuit's `main` function. pub struct AbiParameter { pub name: String, #[serde(rename = "type")] + #[cfg_attr(test, proptest(strategy = "arbitrary::arb_abi_type()"))] pub typ: AbiType, pub visibility: AbiVisibility, } @@ -163,17 +164,20 @@ impl AbiParameter { } #[derive(Clone, Debug, Serialize, Deserialize)] - +#[cfg_attr(test, derive(arbitrary::Arbitrary))] pub struct AbiReturnType { + #[cfg_attr(test, proptest(strategy = "arbitrary::arb_abi_type()"))] pub abi_type: AbiType, pub visibility: AbiVisibility, } #[derive(Clone, Debug, Serialize, Deserialize)] +#[cfg_attr(test, derive(arbitrary::Arbitrary))] pub struct Abi { /// An ordered list of the arguments to the program's `main` function, specifying their types and visibility. pub parameters: Vec, pub return_type: Option, + #[cfg_attr(test, proptest(strategy = "proptest::prelude::Just(BTreeMap::from([]))"))] pub error_types: BTreeMap, } @@ -495,7 +499,6 @@ pub fn display_abi_error( #[cfg(test)] mod test { - use proptest::prelude::*; use crate::arbitrary::arb_abi_and_input_map; From 89132967a5a5e0987011b2974559b4bb30812c21 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 16 Jun 2024 15:08:42 +0100 Subject: [PATCH 10/17] chore: nit --- tooling/noirc_abi/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/noirc_abi/Cargo.toml b/tooling/noirc_abi/Cargo.toml index 3920a4222f6..4c0c1f75e42 100644 --- a/tooling/noirc_abi/Cargo.toml +++ b/tooling/noirc_abi/Cargo.toml @@ -19,7 +19,6 @@ thiserror.workspace = true num-bigint = "0.4" num-traits = "0.2" - [dev-dependencies] strum = "0.24" strum_macros = "0.24" From 7d4e1cc711434e09764b015ce96a238a0a44715c Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 16 Jun 2024 15:09:50 +0100 Subject: [PATCH 11/17] chore: delete commented code --- compiler/noirc_frontend/src/hir/resolution/resolver.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index fca5221942e..fe3b970ccfe 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1042,14 +1042,6 @@ impl<'a> Resolver<'a> { }); } - // if matches!(attributes.function, Some(FunctionAttribute::Test { .. })) - // && !parameters.is_empty() - // { - // self.push_err(ResolverError::TestFunctionHasParameters { - // span: func.name_ident().span(), - // }); - // } - let mut typ = Type::Function(parameter_types, return_type, Box::new(Type::Unit)); if !generics.is_empty() { From 0dea198afd012673d13c3868b018172133f5d3b4 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 17 Jun 2024 10:32:25 +0100 Subject: [PATCH 12/17] chore: nits --- tooling/fuzzer/src/lib.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tooling/fuzzer/src/lib.rs b/tooling/fuzzer/src/lib.rs index fc4d5b35e3e..1a1cfcec651 100644 --- a/tooling/fuzzer/src/lib.rs +++ b/tooling/fuzzer/src/lib.rs @@ -46,8 +46,6 @@ impl FuzzedExecutor { let run_result: Result<(), TestError<_>> = self.runner.clone().run(&strategy, |input_map| { - println!("fuzzing with {input_map:?}"); - let fuzz_res = self.single_fuzz(input_map)?; match fuzz_res { @@ -56,12 +54,10 @@ impl FuzzedExecutor { exit_reason: status, counterexample: outcome, }) => { - println!("found counterexample, {outcome:?}"); - // We cannot use the calldata returned by the test runner in `TestError::Fail`, + // We cannot use the input map returned by the test runner in `TestError::Fail`, // since that input represents the last run case, which may not correspond with - // our failure - when a fuzz case fails, proptest will try - // to run at least one more case to find a minimal failure - // case. + // our failure - when a fuzz case fails, proptest will try to run at least one + // more case to find a minimal failure case. *counterexample.borrow_mut() = outcome; Err(TestCaseError::fail(status)) } From 44884507f09b811feb45355705d9e778de3507f0 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 17 Jun 2024 11:55:55 +0100 Subject: [PATCH 13/17] chore: remove explicit counterexample tracking --- tooling/fuzzer/src/lib.rs | 40 +++++++++++++-------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/tooling/fuzzer/src/lib.rs b/tooling/fuzzer/src/lib.rs index 1a1cfcec651..ce65656f2d9 100644 --- a/tooling/fuzzer/src/lib.rs +++ b/tooling/fuzzer/src/lib.rs @@ -3,8 +3,6 @@ //! //! Code is used under the MIT license. -use std::cell::RefCell; - use acvm::{blackbox_solver::StubbedBlackBoxSolver, FieldElement}; use noirc_abi::InputMap; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; @@ -39,12 +37,9 @@ impl FuzzedExecutor { /// Fuzzes the provided program. pub fn fuzz(&self) -> FuzzTestResult { - // Stores the result and calldata of the last failed call, if any. - let counterexample: RefCell = RefCell::default(); - let strategy = strategies::arb_input_map(&self.program.abi); - let run_result: Result<(), TestError<_>> = + let run_result: Result<(), TestError> = self.runner.clone().run(&strategy, |input_map| { let fuzz_res = self.single_fuzz(input_map)?; @@ -52,35 +47,26 @@ impl FuzzedExecutor { FuzzOutcome::Case(_) => Ok(()), FuzzOutcome::CounterExample(CounterExampleOutcome { exit_reason: status, - counterexample: outcome, - }) => { - // We cannot use the input map returned by the test runner in `TestError::Fail`, - // since that input represents the last run case, which may not correspond with - // our failure - when a fuzz case fails, proptest will try to run at least one - // more case to find a minimal failure case. - *counterexample.borrow_mut() = outcome; - Err(TestCaseError::fail(status)) - } + .. + }) => Err(TestCaseError::fail(status)), } }); - let mut result = - FuzzTestResult { success: run_result.is_ok(), reason: None, counterexample: None }; - match run_result { - Err(TestError::Abort(reason)) => { - result.reason = Some(reason.to_string()); - } - Err(TestError::Fail(reason, _)) => { + Ok(()) => FuzzTestResult { success: true, reason: None, counterexample: None }, + + Err(TestError::Abort(reason)) => FuzzTestResult { + success: false, + reason: Some(reason.to_string()), + counterexample: None, + }, + Err(TestError::Fail(reason, counterexample)) => { let reason = reason.to_string(); - result.reason = if reason.is_empty() { None } else { Some(reason) }; + let reason = if reason.is_empty() { None } else { Some(reason) }; - result.counterexample = Some(counterexample.into_inner()); + FuzzTestResult { success: false, reason, counterexample: Some(counterexample) } } - _ => {} } - - result } /// Granular and single-step function that runs only one fuzz and returns either a `CaseOutcome` From b065e27fc61ea719f2b6ba3c323f0145d32e1d60 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 15 Jun 2024 20:55:31 +0100 Subject: [PATCH 14/17] feat: build simple dictionary from inspecting ACIR program --- .../fuzzer_checks/Nargo.toml | 5 + .../fuzzer_checks/src/main.nr | 7 + tooling/fuzzer/src/dictionary/mod.rs | 124 ++++++++++++++++++ tooling/fuzzer/src/lib.rs | 5 +- tooling/fuzzer/src/strategies/mod.rs | 36 ++--- tooling/fuzzer/src/strategies/uint.rs | 39 +++++- 6 files changed, 193 insertions(+), 23 deletions(-) create mode 100644 test_programs/noir_test_success/fuzzer_checks/Nargo.toml create mode 100644 test_programs/noir_test_success/fuzzer_checks/src/main.nr create mode 100644 tooling/fuzzer/src/dictionary/mod.rs diff --git a/test_programs/noir_test_success/fuzzer_checks/Nargo.toml b/test_programs/noir_test_success/fuzzer_checks/Nargo.toml new file mode 100644 index 00000000000..cd09d0d344d --- /dev/null +++ b/test_programs/noir_test_success/fuzzer_checks/Nargo.toml @@ -0,0 +1,5 @@ +[package] +name = "fuzzer_checks" +type = "bin" +authors = [""] +[dependencies] diff --git a/test_programs/noir_test_success/fuzzer_checks/src/main.nr b/test_programs/noir_test_success/fuzzer_checks/src/main.nr new file mode 100644 index 00000000000..2df18de0ebf --- /dev/null +++ b/test_programs/noir_test_success/fuzzer_checks/src/main.nr @@ -0,0 +1,7 @@ +use dep::std::field::bn254::{TWO_POW_128, assert_gt}; + +#[test(should_fail_with = "42 is not allowed")] +fn finds_magic_value(x: u32) { + let x = x as u64; + assert(2 * x != 42, "42 is not allowed"); +} diff --git a/tooling/fuzzer/src/dictionary/mod.rs b/tooling/fuzzer/src/dictionary/mod.rs new file mode 100644 index 00000000000..84fd9e173ef --- /dev/null +++ b/tooling/fuzzer/src/dictionary/mod.rs @@ -0,0 +1,124 @@ +//! This module defines how to build a dictionary of values which are likely to be correspond +//! to significant inputs during fuzzing by inspecting the [Program] being fuzzed. +//! +//! This dictionary can be fed into the fuzzer's [strategy][proptest::strategy::Strategy] in order to bias it towards +//! generating these values to ensure they get proper coverage. +use std::collections::HashSet; + +use acvm::{ + acir::{ + circuit::{ + brillig::{BrilligBytecode, BrilligInputs}, + directives::Directive, + opcodes::{BlackBoxFuncCall, FunctionInput}, + Circuit, Opcode, Program, + }, + native_types::Expression, + }, + brillig_vm::brillig::Opcode as BrilligOpcode, + AcirField, +}; + +/// Constructs a [HashSet] of values pulled from a [Program] which are likely to be correspond +/// to significant inputs during fuzzing. +pub(super) fn build_dictionary_from_program(program: &Program) -> HashSet { + let constrained_dictionaries = program.functions.iter().map(build_dictionary_from_circuit); + let unconstrained_dictionaries = + program.unconstrained_functions.iter().map(build_dictionary_from_unconstrained_function); + let dictionaries = constrained_dictionaries.chain(unconstrained_dictionaries); + + let mut constants: HashSet = HashSet::new(); + for dictionary in dictionaries { + constants.extend(dictionary); + } + constants +} + +fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet { + let mut constants: HashSet = HashSet::new(); + + fn insert_expr(dictionary: &mut HashSet, expr: &Expression) { + let quad_coefficients = expr.mul_terms.iter().map(|(k, _, _)| *k); + let linear_coefficients = expr.linear_combinations.iter().map(|(k, _)| *k); + let coefficients = linear_coefficients.chain(quad_coefficients); + + dictionary.extend(coefficients.clone()); + dictionary.insert(expr.q_c); + + // We divide the constant term by any coefficients in the expression to aid solving constraints such as `2 * x - 4 == 0`. + let scaled_constants = coefficients.map(|coefficient| expr.q_c / coefficient); + dictionary.extend(scaled_constants); + } + + fn insert_array_len(dictionary: &mut HashSet, array: &[T]) { + let array_length = array.len() as u128; + dictionary.insert(F::from(array_length)); + dictionary.insert(F::from(array_length - 1)); + } + + for opcode in &circuit.opcodes { + match opcode { + Opcode::AssertZero(expr) + | Opcode::Call { predicate: Some(expr), .. } + | Opcode::MemoryOp { predicate: Some(expr), .. } + | Opcode::Directive(Directive::ToLeRadix { a: expr, .. }) => { + insert_expr(&mut constants, expr) + } + + Opcode::MemoryInit { init, .. } => insert_array_len(&mut constants, init), + + Opcode::BrilligCall { inputs, predicate, .. } => { + for input in inputs { + match input { + BrilligInputs::Single(expr) => insert_expr(&mut constants, expr), + BrilligInputs::Array(exprs) => { + exprs.iter().for_each(|expr| insert_expr(&mut constants, expr)); + insert_array_len(&mut constants, exprs); + } + BrilligInputs::MemoryArray(_) => (), + } + } + if let Some(predicate) = predicate { + insert_expr(&mut constants, predicate) + } + } + + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { + input: FunctionInput { num_bits, .. }, + }) => { + let field = 1u128 << num_bits; + constants.insert(F::from(field)); + constants.insert(F::from(field - 1)); + } + _ => (), + } + } + + constants +} + +fn build_dictionary_from_unconstrained_function( + function: &BrilligBytecode, +) -> HashSet { + let mut constants: HashSet = HashSet::new(); + + for opcode in &function.bytecode { + match opcode { + BrilligOpcode::Cast { bit_size, .. } => { + let field = 1u128 << bit_size; + constants.insert(F::from(field)); + constants.insert(F::from(field - 1)); + } + BrilligOpcode::Const { bit_size, value, .. } => { + constants.insert(*value); + + let field = 1u128 << bit_size; + constants.insert(F::from(field)); + constants.insert(F::from(field - 1)); + } + _ => (), + } + } + + constants +} diff --git a/tooling/fuzzer/src/lib.rs b/tooling/fuzzer/src/lib.rs index ce65656f2d9..aa2ba486993 100644 --- a/tooling/fuzzer/src/lib.rs +++ b/tooling/fuzzer/src/lib.rs @@ -4,9 +4,11 @@ //! Code is used under the MIT license. use acvm::{blackbox_solver::StubbedBlackBoxSolver, FieldElement}; +use dictionary::build_dictionary_from_program; use noirc_abi::InputMap; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; +mod dictionary; mod strategies; mod types; @@ -37,7 +39,8 @@ impl FuzzedExecutor { /// Fuzzes the provided program. pub fn fuzz(&self) -> FuzzTestResult { - let strategy = strategies::arb_input_map(&self.program.abi); + let dictionary = build_dictionary_from_program(&self.program.bytecode); + let strategy = strategies::arb_input_map(&self.program.abi, dictionary); let run_result: Result<(), TestError> = self.runner.clone().run(&strategy, |input_map| { diff --git a/tooling/fuzzer/src/strategies/mod.rs b/tooling/fuzzer/src/strategies/mod.rs index f5b03953ba8..46187a28d5b 100644 --- a/tooling/fuzzer/src/strategies/mod.rs +++ b/tooling/fuzzer/src/strategies/mod.rs @@ -5,28 +5,22 @@ use proptest::prelude::*; use acvm::{AcirField, FieldElement}; use noirc_abi::{input_parser::InputValue, Abi, AbiType, InputMap, Sign}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use uint::UintStrategy; mod int; mod uint; -proptest::prop_compose! { - pub(super) fn arb_field_from_integer(bit_size: u32)(value: u128)-> FieldElement { - let width = (bit_size % 128).clamp(1, 127); - let max_value = 2u128.pow(width) - 1; - let value = value % max_value; - FieldElement::from(value) - } -} - -pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { +pub(super) fn arb_value_from_abi_type( + abi_type: &AbiType, + dictionary: HashSet, +) -> SBoxedStrategy { match abi_type { AbiType::Field => vec(any::(), 32) .prop_map(|bytes| InputValue::Field(FieldElement::from_be_bytes_reduce(&bytes))) .sboxed(), AbiType::Integer { width, sign } if sign == &Sign::Unsigned => { - UintStrategy::new(*width as usize) + UintStrategy::new(*width as usize, dictionary) .prop_map(|uint| InputValue::Field(uint.into())) .sboxed() } @@ -55,7 +49,7 @@ pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { let length = *length as usize; - let elements = vec(arb_value_from_abi_type(typ), length..=length); + let elements = vec(arb_value_from_abi_type(typ, dictionary), length..=length); elements.prop_map(InputValue::Vec).sboxed() } @@ -63,7 +57,9 @@ pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { let fields: Vec> = fields .iter() - .map(|(name, typ)| (Just(name.clone()), arb_value_from_abi_type(typ)).sboxed()) + .map(|(name, typ)| { + (Just(name.clone()), arb_value_from_abi_type(typ, dictionary.clone())).sboxed() + }) .collect(); fields @@ -75,17 +71,23 @@ pub(super) fn arb_value_from_abi_type(abi_type: &AbiType) -> SBoxedStrategy { - let fields: Vec<_> = fields.iter().map(arb_value_from_abi_type).collect(); + let fields: Vec<_> = + fields.iter().map(|typ| arb_value_from_abi_type(typ, dictionary.clone())).collect(); fields.prop_map(InputValue::Vec).sboxed() } } } -pub(super) fn arb_input_map(abi: &Abi) -> BoxedStrategy { +pub(super) fn arb_input_map( + abi: &Abi, + dictionary: HashSet, +) -> BoxedStrategy { let values: Vec<_> = abi .parameters .iter() - .map(|param| (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ))) + .map(|param| { + (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ, dictionary.clone())) + }) .collect(); values diff --git a/tooling/fuzzer/src/strategies/uint.rs b/tooling/fuzzer/src/strategies/uint.rs index 5021e832b97..eb0d54f2f5d 100644 --- a/tooling/fuzzer/src/strategies/uint.rs +++ b/tooling/fuzzer/src/strategies/uint.rs @@ -1,3 +1,6 @@ +use std::collections::HashSet; + +use acvm::{AcirField, FieldElement}; use proptest::{ strategy::{NewTree, Strategy}, test_runner::TestRunner, @@ -13,9 +16,12 @@ use rand::Rng; pub struct UintStrategy { /// Bit size of uint (e.g. 128) bits: usize, - + /// A set of fixtures to be generated + fixtures: Vec, /// The weight for edge cases (+/- 3 around 0 and max possible value) edge_weight: usize, + /// The weight for fixtures + fixtures_weight: usize, /// The weight for purely random values random_weight: usize, } @@ -24,8 +30,14 @@ impl UintStrategy { /// Create a new strategy. /// # Arguments /// * `bits` - Size of uint in bits - pub fn new(bits: usize) -> Self { - Self { bits, edge_weight: 10usize, random_weight: 50usize } + pub fn new(bits: usize, fixtures: HashSet) -> Self { + Self { + bits, + fixtures: fixtures.into_iter().collect(), + edge_weight: 10usize, + fixtures_weight: 40usize, + random_weight: 50usize, + } } fn generate_edge_tree(&self, runner: &mut TestRunner) -> NewTree { @@ -37,6 +49,22 @@ impl UintStrategy { Ok(proptest::num::u128::BinarySearch::new(start)) } + fn generate_fixtures_tree(&self, runner: &mut TestRunner) -> NewTree { + // generate random cases if there's no fixtures + if self.fixtures.is_empty() { + return self.generate_random_tree(runner); + } + + // Generate value tree from fixture. + let fixture = &self.fixtures[runner.rng().gen_range(0..self.fixtures.len())]; + if fixture.num_bits() <= self.bits as u32 { + return Ok(proptest::num::u128::BinarySearch::new(fixture.to_u128())); + } + + // If fixture is not a valid type, generate random value. + self.generate_random_tree(runner) + } + fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); let start = rng.gen_range(0..=self.type_max()); @@ -57,11 +85,12 @@ impl Strategy for UintStrategy { type Tree = proptest::num::u128::BinarySearch; type Value = u128; fn new_tree(&self, runner: &mut TestRunner) -> NewTree { - let total_weight = self.random_weight + self.edge_weight; + let total_weight = self.random_weight + self.fixtures_weight + self.edge_weight; let bias = runner.rng().gen_range(0..total_weight); - // randomly select one of 2 strategies + // randomly select one of 3 strategies match bias { x if x < self.edge_weight => self.generate_edge_tree(runner), + x if x < self.edge_weight + self.fixtures_weight => self.generate_fixtures_tree(runner), _ => self.generate_random_tree(runner), } } From 7bae217815110eb976c2f82c2aeb73f92e58b11d Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 19 Jun 2024 12:24:36 +0000 Subject: [PATCH 15/17] chore: avoid panics in bitshifts when building dictionary --- tooling/fuzzer/src/dictionary/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/fuzzer/src/dictionary/mod.rs b/tooling/fuzzer/src/dictionary/mod.rs index 84fd9e173ef..bf2ab87be29 100644 --- a/tooling/fuzzer/src/dictionary/mod.rs +++ b/tooling/fuzzer/src/dictionary/mod.rs @@ -86,7 +86,7 @@ fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet< Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input: FunctionInput { num_bits, .. }, }) => { - let field = 1u128 << num_bits; + let field = 1u128.wrapping_shl(*num_bits); constants.insert(F::from(field)); constants.insert(F::from(field - 1)); } @@ -105,14 +105,14 @@ fn build_dictionary_from_unconstrained_function( for opcode in &function.bytecode { match opcode { BrilligOpcode::Cast { bit_size, .. } => { - let field = 1u128 << bit_size; + let field = 1u128.wrapping_shl(*bit_size); constants.insert(F::from(field)); constants.insert(F::from(field - 1)); } BrilligOpcode::Const { bit_size, value, .. } => { constants.insert(*value); - let field = 1u128 << bit_size; + let field = 1u128.wrapping_shl(*bit_size); constants.insert(F::from(field)); constants.insert(F::from(field - 1)); } From 71bbcbb0184c7ff13a17e9594f837f018858b553 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 19 Jun 2024 13:37:25 +0100 Subject: [PATCH 16/17] Update tooling/fuzzer/src/strategies/uint.rs --- tooling/fuzzer/src/strategies/uint.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/fuzzer/src/strategies/uint.rs b/tooling/fuzzer/src/strategies/uint.rs index eb0d54f2f5d..94610dbc829 100644 --- a/tooling/fuzzer/src/strategies/uint.rs +++ b/tooling/fuzzer/src/strategies/uint.rs @@ -30,6 +30,7 @@ impl UintStrategy { /// Create a new strategy. /// # Arguments /// * `bits` - Size of uint in bits + /// * `fixtures` - Set of `FieldElements` representing values which the fuzzer weight towards testing. pub fn new(bits: usize, fixtures: HashSet) -> Self { Self { bits, From 030e1cb6cc4c05dc78fce89e645b7703d19abd1d Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 20 Jun 2024 10:13:46 +0100 Subject: [PATCH 17/17] Update test_programs/noir_test_success/fuzzer_checks/src/main.nr --- test_programs/noir_test_success/fuzzer_checks/src/main.nr | 1 - 1 file changed, 1 deletion(-) diff --git a/test_programs/noir_test_success/fuzzer_checks/src/main.nr b/test_programs/noir_test_success/fuzzer_checks/src/main.nr index 2df18de0ebf..2b928db092e 100644 --- a/test_programs/noir_test_success/fuzzer_checks/src/main.nr +++ b/test_programs/noir_test_success/fuzzer_checks/src/main.nr @@ -1,4 +1,3 @@ -use dep::std::field::bn254::{TWO_POW_128, assert_gt}; #[test(should_fail_with = "42 is not allowed")] fn finds_magic_value(x: u32) {