From 5fe9143385231ebf67af670a9f001e0f4fab4a33 Mon Sep 17 00:00:00 2001 From: grandizzy <38490174+grandizzy@users.noreply.github.com> Date: Tue, 12 Mar 2024 16:52:28 +0200 Subject: [PATCH] feat(test): add fuzz tests failure persistence (#7336) * feat(forge): add fuzz tests failure persistence * Enable inline file failure config * New config not needed to be Option * Persist failures in proj cache dir * Make persist dirs option, remove foundry_fuzz_cache_dir fn --------- Co-authored-by: Matthias Seitz --- crates/config/src/fuzz.rs | 30 ++++++++++++-- crates/config/src/lib.rs | 2 +- crates/forge/bin/cmd/test/mod.rs | 9 +++- crates/forge/src/lib.rs | 29 +++++++++---- crates/forge/src/runner.rs | 10 +++-- crates/forge/tests/cli/config.rs | 2 + crates/forge/tests/it/fuzz.rs | 57 ++++++++++++++++++++++++-- crates/forge/tests/it/test_helpers.rs | 4 +- testdata/fuzz/FuzzFailurePersist.t.sol | 29 +++++++++++++ 9 files changed, 153 insertions(+), 19 deletions(-) create mode 100644 testdata/fuzz/FuzzFailurePersist.t.sol diff --git a/crates/config/src/fuzz.rs b/crates/config/src/fuzz.rs index 13e8d34d3f2f..3b0e13bcd509 100644 --- a/crates/config/src/fuzz.rs +++ b/crates/config/src/fuzz.rs @@ -5,9 +5,10 @@ use crate::inline::{ }; use alloy_primitives::U256; use serde::{Deserialize, Serialize}; +use std::path::PathBuf; /// Contains for fuzz testing -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct FuzzConfig { /// The number of test cases that must execute for each property test pub runs: u32, @@ -24,6 +25,10 @@ pub struct FuzzConfig { pub dictionary: FuzzDictionaryConfig, /// Number of runs to execute and include in the gas report. pub gas_report_samples: u32, + /// Path where fuzz failures are recorded and replayed. + pub failure_persist_dir: Option, + /// Name of the file to record fuzz failures, defaults to `failures`. + pub failure_persist_file: Option, } impl Default for FuzzConfig { @@ -34,6 +39,23 @@ impl Default for FuzzConfig { seed: None, dictionary: FuzzDictionaryConfig::default(), gas_report_samples: 256, + failure_persist_dir: None, + failure_persist_file: None, + } + } +} + +impl FuzzConfig { + /// Creates fuzz configuration to write failures in `{PROJECT_ROOT}/cache/fuzz` dir. + pub fn new(cache_dir: PathBuf) -> Self { + FuzzConfig { + runs: 256, + max_test_rejects: 65536, + seed: None, + dictionary: FuzzDictionaryConfig::default(), + gas_report_samples: 256, + failure_persist_dir: Some(cache_dir), + failure_persist_file: Some("failures".to_string()), } } } @@ -50,8 +72,7 @@ impl InlineConfigParser for FuzzConfig { return Ok(None) } - // self is Copy. We clone it with dereference. - let mut conf_clone = *self; + let mut conf_clone = self.clone(); for pair in overrides { let key = pair.0; @@ -62,6 +83,7 @@ impl InlineConfigParser for FuzzConfig { "dictionary-weight" => { conf_clone.dictionary.dictionary_weight = parse_config_u32(key, value)? } + "failure-persist-file" => conf_clone.failure_persist_file = Some(value), _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } @@ -130,11 +152,13 @@ mod tests { let configs = &[ "forge-config: default.fuzz.runs = 42424242".to_string(), "forge-config: default.fuzz.dictionary-weight = 42".to_string(), + "forge-config: default.fuzz.failure-persist-file = fuzz-failure".to_string(), ]; let base_config = FuzzConfig::default(); let merged: FuzzConfig = base_config.try_merge(configs).expect("No errors").unwrap(); assert_eq!(merged.runs, 42424242); assert_eq!(merged.dictionary.dictionary_weight, 42); + assert_eq!(merged.failure_persist_file, Some("fuzz-failure".to_string())); } #[test] diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 00077845530a..4f27b58b8edd 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -1869,7 +1869,7 @@ impl Default for Config { contract_pattern_inverse: None, path_pattern: None, path_pattern_inverse: None, - fuzz: Default::default(), + fuzz: FuzzConfig::new("cache/fuzz".into()), invariant: Default::default(), always_use_create_2_factory: false, ffi: false, diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 11ae1aee77e3..dfb1d8cf5941 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -97,6 +97,10 @@ pub struct TestArgs { #[arg(long, env = "FOUNDRY_FUZZ_RUNS", value_name = "RUNS")] pub fuzz_runs: Option, + /// File to rerun fuzz failures from. + #[arg(long)] + pub fuzz_input_file: Option, + #[command(flatten)] filter: FilterArgs, @@ -176,7 +180,7 @@ impl TestArgs { let profiles = get_available_profiles(toml)?; let test_options: TestOptions = TestOptionsBuilder::default() - .fuzz(config.fuzz) + .fuzz(config.clone().fuzz) .invariant(config.invariant) .profiles(profiles) .build(&output, project_root)?; @@ -518,6 +522,9 @@ impl Provider for TestArgs { if let Some(fuzz_runs) = self.fuzz_runs { fuzz_dict.insert("runs".to_string(), fuzz_runs.into()); } + if let Some(fuzz_input_file) = self.fuzz_input_file.clone() { + fuzz_dict.insert("failure_persist_file".to_string(), fuzz_input_file.into()); + } dict.insert("fuzz".to_string(), fuzz_dict.into()); if let Some(etherscan_api_key) = diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index f6b9a54a8e68..a430204e158d 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -6,7 +6,9 @@ use foundry_config::{ validate_profiles, Config, FuzzConfig, InlineConfig, InlineConfigError, InlineConfigParser, InvariantConfig, NatSpec, }; -use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner}; +use proptest::test_runner::{ + FailurePersistence, FileFailurePersistence, RngAlgorithm, TestRng, TestRunner, +}; use std::path::Path; pub mod coverage; @@ -93,8 +95,18 @@ impl TestOptions { where S: Into, { - let fuzz = self.fuzz_config(contract_id, test_fn); - self.fuzzer_with_cases(fuzz.runs) + let fuzz_config = self.fuzz_config(contract_id, test_fn).clone(); + let failure_persist_path = fuzz_config + .failure_persist_dir + .unwrap() + .join(fuzz_config.failure_persist_file.unwrap()) + .into_os_string() + .into_string() + .unwrap(); + self.fuzzer_with_cases( + fuzz_config.runs, + Some(Box::new(FileFailurePersistence::Direct(failure_persist_path.leak()))), + ) } /// Returns an "invariant" test runner instance. Parameters are used to select tight scoped fuzz @@ -109,7 +121,7 @@ impl TestOptions { S: Into, { let invariant = self.invariant_config(contract_id, test_fn); - self.fuzzer_with_cases(invariant.runs) + self.fuzzer_with_cases(invariant.runs, None) } /// Returns a "fuzz" configuration setup. Parameters are used to select tight scoped fuzz @@ -140,10 +152,13 @@ impl TestOptions { self.inline_invariant.get(contract_id, test_fn).unwrap_or(&self.invariant) } - pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner { - // TODO: Add Options to modify the persistence + pub fn fuzzer_with_cases( + &self, + cases: u32, + file_failure_persistence: Option>, + ) -> TestRunner { let config = proptest::test_runner::Config { - failure_persistence: None, + failure_persistence: file_failure_persistence, cases, max_global_rejects: self.fuzz.max_test_rejects, ..Default::default() diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 0164cd7df0e6..3b1a1a207178 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -294,7 +294,7 @@ impl<'a> ContractRunner<'a> { debug_assert!(func.is_test()); let runner = test_options.fuzz_runner(self.name, &func.name); let fuzz_config = test_options.fuzz_config(self.name, &func.name); - self.run_fuzz_test(func, should_fail, runner, setup, *fuzz_config) + self.run_fuzz_test(func, should_fail, runner, setup, fuzz_config.clone()) } else { debug_assert!(func.is_test()); self.run_test(func, should_fail, setup) @@ -604,8 +604,12 @@ impl<'a> ContractRunner<'a> { // Run fuzz test let start = Instant::now(); - let fuzzed_executor = - FuzzedExecutor::new(self.executor.clone(), runner.clone(), self.sender, fuzz_config); + let fuzzed_executor = FuzzedExecutor::new( + self.executor.clone(), + runner.clone(), + self.sender, + fuzz_config.clone(), + ); let state = fuzzed_executor.build_fuzz_state(); let result = fuzzed_executor.fuzz(func, address, should_fail, self.revert_decoder); diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index fa57e4077900..6714df59dd83 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -65,6 +65,8 @@ forgetest!(can_extract_config_values, |prj, cmd| { runs: 1000, max_test_rejects: 100203, seed: Some(U256::from(1000)), + failure_persist_dir: Some("test-cache/fuzz".into()), + failure_persist_file: Some("failures".to_string()), ..Default::default() }, invariant: InvariantConfig { runs: 256, ..Default::default() }, diff --git a/crates/forge/tests/it/fuzz.rs b/crates/forge/tests/it/fuzz.rs index 14a27d6b442a..9f59a7f42fb9 100644 --- a/crates/forge/tests/it/fuzz.rs +++ b/crates/forge/tests/it/fuzz.rs @@ -1,10 +1,14 @@ //! Fuzz tests. -use crate::config::*; -use alloy_primitives::U256; +use std::collections::BTreeMap; + +use alloy_primitives::{Bytes, U256}; +use forge::fuzz::CounterExample; + use forge::result::{SuiteResult, TestStatus}; use foundry_test_utils::Filter; -use std::collections::BTreeMap; + +use crate::config::*; #[tokio::test(flavor = "multi_thread")] async fn test_fuzz() { @@ -103,3 +107,50 @@ async fn test_fuzz_collection() { )]), ); } + +#[tokio::test(flavor = "multi_thread")] +async fn test_persist_fuzz_failure() { + let filter = Filter::new(".*", ".*", ".*fuzz/FuzzFailurePersist.t.sol"); + let mut runner = runner(); + runner.test_options.fuzz.runs = 1000; + + macro_rules! get_failure_result { + () => { + runner + .test_collect(&filter) + .get("fuzz/FuzzFailurePersist.t.sol:FuzzFailurePersistTest") + .unwrap() + .test_results + .get("test_persist_fuzzed_failure(uint256,int256,address,bool,string,(address,uint256),address[])") + .unwrap() + .counterexample + .clone() + }; + } + + // record initial counterexample calldata + let intial_counterexample = get_failure_result!(); + let initial_calldata = match intial_counterexample { + Some(CounterExample::Single(counterexample)) => counterexample.calldata, + _ => Bytes::new(), + }; + + // run several times and compare counterexamples calldata + for _ in 0..10 { + let new_calldata = match get_failure_result!() { + Some(CounterExample::Single(counterexample)) => counterexample.calldata, + _ => Bytes::new(), + }; + // calldata should be the same with the initial one + assert_eq!(initial_calldata, new_calldata); + } + + // write new failure in different file + runner.test_options.fuzz.failure_persist_file = Some("failure1".to_string()); + let new_calldata = match get_failure_result!() { + Some(CounterExample::Single(counterexample)) => counterexample.calldata, + _ => Bytes::new(), + }; + // empty file is used to load failure so new calldata is generated + assert_ne!(initial_calldata, new_calldata); +} diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index e615e2785695..969e673579cf 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -95,6 +95,8 @@ pub static TEST_OPTS: Lazy = Lazy::new(|| { max_calldata_fuzz_dictionary_addresses: 0, }, gas_report_samples: 256, + failure_persist_dir: Some(tempfile::tempdir().unwrap().into_path()), + failure_persist_file: Some("testfailure".to_string()), }) .invariant(InvariantConfig { runs: 256, @@ -126,6 +128,6 @@ pub fn fuzz_executor(executor: Executor) -> FuzzedExecutor { executor, proptest::test_runner::TestRunner::new(cfg), CALLER, - TEST_OPTS.fuzz, + TEST_OPTS.fuzz.clone(), ) } diff --git a/testdata/fuzz/FuzzFailurePersist.t.sol b/testdata/fuzz/FuzzFailurePersist.t.sol new file mode 100644 index 000000000000..1f7c4829cdcf --- /dev/null +++ b/testdata/fuzz/FuzzFailurePersist.t.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity 0.8.18; + +import "ds-test/test.sol"; +import "../cheats/Vm.sol"; + +struct TestTuple { + address user; + uint256 amount; +} + +contract FuzzFailurePersistTest is DSTest { + Vm vm = Vm(HEVM_ADDRESS); + + function test_persist_fuzzed_failure( + uint256 x, + int256 y, + address addr, + bool cond, + string calldata test, + TestTuple calldata tuple, + address[] calldata addresses + ) public { + // dummy assume to trigger runs + vm.assume(x > 1 && x < 1111111111111111111111111111); + vm.assume(y > 1 && y < 1111111111111111111111111111); + require(false); + } +}