From 9b9ccf851f0cf55a5bdc04064dc23ec1b7b01fd7 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 8 May 2024 17:51:52 +0300 Subject: [PATCH 1/5] feature(invariant) - persist and replay failure --- crates/config/src/invariant.rs | 44 +++++- crates/config/src/lib.rs | 21 ++- crates/evm/evm/src/executors/fuzz/mod.rs | 13 +- crates/evm/evm/src/executors/invariant/mod.rs | 1 + .../evm/evm/src/executors/invariant/replay.rs | 46 +++--- .../evm/evm/src/executors/invariant/shrink.rs | 39 ++--- crates/evm/fuzz/src/lib.rs | 44 ++++-- crates/forge/bin/cmd/coverage.rs | 2 +- crates/forge/bin/cmd/test/mod.rs | 2 +- crates/forge/src/runner.rs | 103 ++++++++++++- crates/forge/tests/cli/cmd.rs | 25 +++- crates/forge/tests/cli/config.rs | 6 +- crates/forge/tests/it/invariant.rs | 137 ++++++++++-------- crates/forge/tests/it/test_helpers.rs | 1 + 14 files changed, 336 insertions(+), 148 deletions(-) diff --git a/crates/config/src/invariant.rs b/crates/config/src/invariant.rs index fa9e5489f79d..3d5b9482f4b1 100644 --- a/crates/config/src/invariant.rs +++ b/crates/config/src/invariant.rs @@ -8,9 +8,10 @@ use crate::{ }, }; use serde::{Deserialize, Serialize}; +use std::path::PathBuf; /// Contains for invariant testing -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct InvariantConfig { /// The number of runs that must execute for each invariant test group. pub runs: u32, @@ -31,6 +32,8 @@ pub struct InvariantConfig { pub max_assume_rejects: u32, /// Number of runs to execute and include in the gas report. pub gas_report_samples: u32, + /// Path where invariant failures are recorded and replayed. + pub failure_persist_dir: Option, } impl Default for InvariantConfig { @@ -44,10 +47,43 @@ impl Default for InvariantConfig { shrink_run_limit: 2usize.pow(18_u32), max_assume_rejects: 65536, gas_report_samples: 256, + failure_persist_dir: None, } } } +impl InvariantConfig { + /// Creates invariant configuration to write failures in `{PROJECT_ROOT}/cache/fuzz` dir. + pub fn new(cache_dir: PathBuf) -> Self { + InvariantConfig { + runs: 256, + depth: 15, + fail_on_revert: false, + call_override: false, + dictionary: FuzzDictionaryConfig { dictionary_weight: 80, ..Default::default() }, + shrink_run_limit: 2usize.pow(18_u32), + max_assume_rejects: 65536, + gas_report_samples: 256, + failure_persist_dir: Some(cache_dir), + } + } + + /// Returns path to failure dir of given invariant test contract. + pub fn failure_dir(self, contract_name: &str) -> PathBuf { + self.failure_persist_dir + .unwrap() + .join("failures") + .join(contract_name.split(':').last().unwrap()) + } + + /// Returns path to persisted failure of current invariant test (if any). + /// The path to failure file is `{failure_dir}/{test_contract_name}/{invariant_name}`, + /// for example: `cache/invariant/failures/InvariantTest/invariant_check` + pub fn failure_file(self, contract_name: &str, test_name: String) -> PathBuf { + self.failure_dir(contract_name).join(test_name) + } +} + impl InlineConfigParser for InvariantConfig { fn config_key() -> String { INLINE_CONFIG_INVARIANT_KEY.into() @@ -60,8 +96,7 @@ impl InlineConfigParser for InvariantConfig { 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; @@ -71,6 +106,9 @@ impl InlineConfigParser for InvariantConfig { "depth" => conf_clone.depth = parse_config_u32(key, value)?, "fail-on-revert" => conf_clone.fail_on_revert = parse_config_bool(key, value)?, "call-override" => conf_clone.call_override = parse_config_bool(key, value)?, + "failure-persist-dir" => { + conf_clone.failure_persist_dir = Some(PathBuf::from(value)) + } _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 96cd23a63fdd..906f25f0ac53 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -787,13 +787,20 @@ impl Config { pub fn cleanup(&self, project: &Project) -> Result<(), SolcError> { project.cleanup()?; - // Remove fuzz cache directory. - if let Some(fuzz_cache) = &self.fuzz.failure_persist_dir { - let path = project.root().join(fuzz_cache); - if path.exists() { - std::fs::remove_dir_all(&path).map_err(|e| SolcIoError::new(e, path))?; - } + macro_rules! remove_test_cache { + ($cache_dir:expr) => { + if let Some(test_cache) = $cache_dir { + let path = project.root().join(test_cache); + if path.exists() { + std::fs::remove_dir_all(&path).map_err(|e| SolcIoError::new(e, path))?; + } + } + }; } + // Remove fuzz cache directory. + remove_test_cache!(&self.fuzz.failure_persist_dir); + // Remove invariant cache directory. + remove_test_cache!(&self.invariant.failure_persist_dir); Ok(()) } @@ -1952,7 +1959,7 @@ impl Default for Config { path_pattern: None, path_pattern_inverse: None, fuzz: FuzzConfig::new("cache/fuzz".into()), - invariant: Default::default(), + invariant: InvariantConfig::new("cache/invariant".into()), always_use_create_2_factory: false, ffi: false, prompt_timeout: 120, diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index d81ddbaca01b..520a56cf2fe7 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -175,15 +175,10 @@ impl FuzzedExecutor { } else { vec![] }; - result.counterexample = Some(CounterExample::Single(BaseCounterExample { - sender: None, - addr: None, - signature: None, - contract_name: None, - traces: call.traces, - calldata, - args, - })); + + result.counterexample = Some(CounterExample::Single( + BaseCounterExample::from_fuzz_call(calldata, args, call.traces), + )); } _ => {} } diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index a15f98f8bd60..de959acd33cb 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -40,6 +40,7 @@ mod result; pub use result::InvariantFuzzTestResult; mod shrink; +pub use shrink::check_sequence; sol! { interface IInvariantTest { diff --git a/crates/evm/evm/src/executors/invariant/replay.rs b/crates/evm/evm/src/executors/invariant/replay.rs index e555811745dc..b9d0b08fcb71 100644 --- a/crates/evm/evm/src/executors/invariant/replay.rs +++ b/crates/evm/evm/src/executors/invariant/replay.rs @@ -8,7 +8,7 @@ use foundry_evm_core::constants::CALLER; use foundry_evm_coverage::HitMaps; use foundry_evm_fuzz::{ invariant::{BasicTxDetails, InvariantContract}, - BaseCounterExample, CounterExample, + BaseCounterExample, }; use foundry_evm_traces::{load_contracts, TraceKind, Traces}; use parking_lot::RwLock; @@ -28,7 +28,7 @@ pub fn replay_run( traces: &mut Traces, coverage: &mut Option, inputs: Vec, -) -> Result> { +) -> Result> { // We want traces for a failed case. executor.set_tracing(true); @@ -56,31 +56,34 @@ pub fn replay_run( )); // Create counter example to be used in failed case. - counterexample_sequence.push(BaseCounterExample::create( + counterexample_sequence.push(BaseCounterExample::from_invariant_call( *sender, *addr, bytes, &ided_contracts, call_result.traces, )); - - // Replay invariant to collect logs and traces. - let error_call_result = executor.call_raw( - CALLER, - invariant_contract.address, - invariant_contract - .invariant_function - .abi_encode_input(&[]) - .expect("invariant should have no inputs") - .into(), - U256::ZERO, - )?; - traces.push((TraceKind::Execution, error_call_result.traces.clone().unwrap())); - logs.extend(error_call_result.logs); } - Ok((!counterexample_sequence.is_empty()) - .then_some(CounterExample::Sequence(counterexample_sequence))) + // Replay invariant to collect logs and traces. + // We do this only once at the end of the replayed sequence. + // Checking after each call doesn't add valuable info for passing scenario + // (invariant call result is always success) nor for failed scenarios + // (invariant call result is always success until the last call that breaks it). + let invariant_result = executor.call_raw( + CALLER, + invariant_contract.address, + invariant_contract + .invariant_function + .abi_encode_input(&[]) + .expect("invariant should have no inputs") + .into(), + U256::ZERO, + )?; + traces.push((TraceKind::Execution, invariant_result.traces.clone().unwrap())); + logs.extend(invariant_result.logs); + + Ok(counterexample_sequence) } /// Replays the error case, shrinks the failing sequence and collects all necessary traces. @@ -94,15 +97,16 @@ pub fn replay_error( logs: &mut Vec, traces: &mut Traces, coverage: &mut Option, -) -> Result> { +) -> Result> { match failed_case.test_error { // Don't use at the moment. - TestError::Abort(_) => Ok(None), + TestError::Abort(_) => Ok(vec![]), TestError::Fail(_, ref calls) => { // Shrink sequence of failed calls. let calls = shrink_sequence(failed_case, calls, &executor)?; set_up_inner_replay(&mut executor, &failed_case.inner_sequence); + // Replay calls to get the counterexample and to collect logs, traces and coverage. replay_run( invariant_contract, diff --git a/crates/evm/evm/src/executors/invariant/shrink.rs b/crates/evm/evm/src/executors/invariant/shrink.rs index 7b78ae028bf5..edfceb666bb0 100644 --- a/crates/evm/evm/src/executors/invariant/shrink.rs +++ b/crates/evm/evm/src/executors/invariant/shrink.rs @@ -1,5 +1,5 @@ use crate::executors::{invariant::error::FailedInvariantCaseData, Executor}; -use alloy_primitives::U256; +use alloy_primitives::{Address, Bytes, U256}; use foundry_evm_core::constants::CALLER; use foundry_evm_fuzz::invariant::BasicTxDetails; use proptest::bits::{BitSetLike, VarBitSet}; @@ -97,7 +97,14 @@ pub(crate) fn shrink_sequence( let mut shrinker = CallSequenceShrinker::new(calls.len()); for _ in 0..failed_case.shrink_run_limit { // Check candidate sequence result. - match check_sequence(failed_case, executor.clone(), calls, shrinker.current().collect()) { + match check_sequence( + executor.clone(), + calls, + shrinker.current().collect(), + failed_case.addr, + failed_case.func.clone(), + failed_case.fail_on_revert, + ) { // If candidate sequence still fails then shrink more if possible. Ok(false) if !shrinker.simplify() => break, // If candidate sequence pass then restore last removed call and shrink other @@ -110,41 +117,39 @@ pub(crate) fn shrink_sequence( Ok(shrinker.current().map(|idx| &calls[idx]).cloned().collect()) } -/// Checks if the shrinked sequence fails test, if it does then we can try simplifying more. -fn check_sequence( - failed_case: &FailedInvariantCaseData, +/// Checks if the given call sequence breaks the invariant. +/// Used in shrinking phase for checking candidate sequences and in replay failures phase to test +/// persisted failures. +pub fn check_sequence( mut executor: Executor, calls: &[BasicTxDetails], sequence: Vec, + test_address: Address, + test_function: Bytes, + fail_on_revert: bool, ) -> eyre::Result { let mut sequence_failed = false; - // Apply the shrinked candidate sequence. + // Apply the call sequence. for call_index in sequence { let (sender, (addr, bytes)) = &calls[call_index]; let call_result = executor.call_raw_committing(*sender, *addr, bytes.clone(), U256::ZERO)?; - if call_result.reverted && failed_case.fail_on_revert { + if call_result.reverted && fail_on_revert { // Candidate sequence fails test. // We don't have to apply remaining calls to check sequence. sequence_failed = true; break; } } - // Return without checking the invariant if we already have failing sequence. + // Return without checking the invariant if we already have a failing sequence. if sequence_failed { return Ok(false); }; - // Check the invariant for candidate sequence. - // If sequence fails then we can continue with shrinking - the removed call does not affect - // failure. - // - // If sequence doesn't fail then we have to restore last removed call and continue with next - // call - removed call is a required step for reproducing the failure. - let mut call_result = - executor.call_raw(CALLER, failed_case.addr, failed_case.func.clone(), U256::ZERO)?; + // Check the invariant for call sequence. + let mut call_result = executor.call_raw(CALLER, test_address, test_function, U256::ZERO)?; Ok(executor.is_raw_call_success( - failed_case.addr, + test_address, Cow::Owned(call_result.state_changeset.take().unwrap()), &call_result, false, diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index b2a058e5bb02..c6b5e00499e8 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -43,19 +43,20 @@ pub struct BaseCounterExample { pub addr: Option
, /// The data to provide pub calldata: Bytes, - /// Function signature if it exists - pub signature: Option, /// Contract name if it exists pub contract_name: Option, + /// Function signature if it exists + pub signature: Option, + /// Args used to call the function + pub args: Option, /// Traces #[serde(skip)] pub traces: Option, - #[serde(skip)] - pub args: Vec, } impl BaseCounterExample { - pub fn create( + /// Creates counter example representing a step from invariant call sequence. + pub fn from_invariant_call( sender: Address, addr: Address, bytes: &Bytes, @@ -70,10 +71,12 @@ impl BaseCounterExample { sender: Some(sender), addr: Some(addr), calldata: bytes.clone(), - signature: Some(func.signature()), contract_name: Some(name.clone()), + signature: Some(func.signature()), + args: Some( + foundry_common::fmt::format_tokens(&args).format(", ").to_string(), + ), traces, - args, }; } } @@ -83,10 +86,27 @@ impl BaseCounterExample { sender: Some(sender), addr: Some(addr), calldata: bytes.clone(), + contract_name: None, signature: None, + args: None, + traces, + } + } + + /// Creates counter example for a fuzz test failure. + pub fn from_fuzz_call( + bytes: Bytes, + args: Vec, + traces: Option, + ) -> Self { + BaseCounterExample { + sender: None, + addr: None, + calldata: bytes, contract_name: None, + signature: None, + args: Some(foundry_common::fmt::format_tokens(&args).format(", ").to_string()), traces, - args: vec![], } } } @@ -108,10 +128,14 @@ impl fmt::Display for BaseCounterExample { if let Some(sig) = &self.signature { write!(f, "calldata={sig}")? } else { - write!(f, "calldata={}", self.calldata)? + write!(f, "calldata={}", &self.calldata)? } - write!(f, " args=[{}]", foundry_common::fmt::format_tokens(&self.args).format(", ")) + if let Some(args) = &self.args { + write!(f, " args=[{}]", args) + } else { + write!(f, " args=[]") + } } } diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index d099035fb804..d55c4db9a84b 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -312,7 +312,7 @@ impl CoverageArgs { .with_fork(evm_opts.get_fork(&config, env.clone())) .with_test_options(TestOptions { fuzz: config.fuzz.clone(), - invariant: config.invariant, + invariant: config.invariant.clone(), ..Default::default() }) .set_coverage(true) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index cef234966147..720d13284760 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -257,7 +257,7 @@ impl TestArgs { let test_options: TestOptions = TestOptionsBuilder::default() .fuzz(config.fuzz.clone()) - .invariant(config.invariant) + .invariant(config.invariant.clone()) .profiles(profiles) .build(&output, project_root)?; diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 48fa10b6dcd0..98c36be7a0ab 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -1,6 +1,7 @@ //! The Forge test runner. use crate::{ + fuzz::{invariant::BasicTxDetails, BaseCounterExample}, multi_runner::{is_matching_test, TestContract}, result::{SuiteResult, TestKind, TestResult, TestSetup, TestStatus}, TestFilter, TestOptions, @@ -21,7 +22,7 @@ use foundry_evm::{ executors::{ fuzz::{CaseOutcome, CounterExampleOutcome, FuzzOutcome, FuzzedExecutor}, invariant::{ - replay_error, replay_run, InvariantExecutor, InvariantFuzzError, + check_sequence, replay_error, replay_run, InvariantExecutor, InvariantFuzzError, InvariantFuzzTestResult, }, CallResult, EvmError, ExecutionErr, Executor, RawCallResult, @@ -33,7 +34,9 @@ use proptest::test_runner::TestRunner; use rayon::prelude::*; use std::{ borrow::Cow, + cmp::min, collections::{BTreeMap, HashMap}, + fs, sync::Arc, time::Instant, }; @@ -359,7 +362,7 @@ impl<'a> ContractRunner<'a> { self.run_invariant_test( runner, setup, - *invariant_config, + invariant_config.clone(), func, &known_contracts, identified_contracts.as_ref().unwrap(), @@ -548,14 +551,76 @@ impl<'a> ContractRunner<'a> { let mut evm = InvariantExecutor::new( self.executor.clone(), runner, - invariant_config, + invariant_config.clone(), identified_contracts, known_contracts, ); - let invariant_contract = InvariantContract { address, invariant_function: func, abi: &self.contract.abi }; + let mut logs = logs.clone(); + let mut traces = traces.clone(); + let mut coverage = coverage.clone(); + + // Try to replay recorded failure if any. + let failure_file = invariant_config + .clone() + .failure_file(self.name, invariant_contract.invariant_function.clone().name); + if failure_file.exists() { + if let Ok(data) = fs::read_to_string(failure_file) { + if let Ok(call_sequence) = serde_json::from_str::>(&data) { + // Create calls from failed sequence and check if invariant still broken. + let txes = call_sequence + .clone() + .into_iter() + .map(|seq| { + ( + seq.sender.unwrap_or_default(), + (seq.addr.unwrap_or_default(), seq.calldata), + ) + }) + .collect::>(); + if let Ok(success) = check_sequence( + self.executor.clone(), + &txes, + (0..min(txes.len(), invariant_config.depth as usize)).collect(), + invariant_contract.address, + invariant_contract.invariant_function.selector().to_vec().into(), + invariant_config.fail_on_revert, + ) { + if !success { + // If sequence still fails then replay error to collect traces and + // exit without executing new runs. + let _ = replay_run( + &invariant_contract, + self.executor.clone(), + known_contracts, + identified_contracts.clone(), + &mut logs, + &mut traces, + &mut coverage, + txes, + ); + return TestResult { + status: TestStatus::Failure, + reason: Some(format!( + "{} replay failure", + invariant_contract.invariant_function.name + )), + decoded_logs: decode_console_logs(&logs), + traces, + coverage, + counterexample: Some(CounterExample::Sequence(call_sequence)), + kind: TestKind::Invariant { runs: 1, calls: 1, reverts: 1 }, + duration: start.elapsed(), + ..Default::default() + } + } + } + } + } + } + let InvariantFuzzTestResult { error, cases, reverts, last_run_inputs, gas_report_traces } = match evm.invariant_fuzz(invariant_contract.clone(), &fuzz_fixtures) { Ok(x) => x, @@ -576,11 +641,9 @@ impl<'a> ContractRunner<'a> { }; let mut counterexample = None; - let mut logs = logs.clone(); - let mut traces = traces.clone(); let success = error.is_none(); let reason = error.as_ref().and_then(|err| err.revert_reason()); - let mut coverage = coverage.clone(); + match error { // If invariants were broken, replay the error to collect logs and traces Some(error) => match error { @@ -598,7 +661,31 @@ impl<'a> ContractRunner<'a> { &mut traces, &mut coverage, ) { - Ok(c) => counterexample = c, + Ok(call_sequence) => { + if !call_sequence.is_empty() { + // Persist error in invariant failure dir. + match fs::create_dir_all( + invariant_config.clone().failure_dir(self.name), + ) { + Ok(_) => { + let _ = fs::write( + invariant_config.failure_file( + self.name, + invariant_contract.invariant_function.clone().name, + ), + serde_json::to_string_pretty(&call_sequence).unwrap(), + ) + .map_err(|err| { + error!(%err, "Failed to record call sequence"); + }); + } + Err(err) => { + error!(%err, "Failed to create invariant failure dir") + } + } + counterexample = Some(CounterExample::Sequence(call_sequence)) + } + } Err(err) => { error!(%err, "Failed to replay invariant error"); } diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index b4d8df8ccd3a..de2c242418a0 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -3,7 +3,7 @@ use crate::constants::*; use foundry_compilers::{artifacts::Metadata, remappings::Remapping, ConfigurableContractArtifact}; use foundry_config::{ - parse_with_profile, BasicConfig, Chain, Config, FuzzConfig, SolidityErrorCode, + parse_with_profile, BasicConfig, Chain, Config, FuzzConfig, InvariantConfig, SolidityErrorCode, }; use foundry_test_utils::{ foundry_compilers::PathStyle, @@ -546,18 +546,27 @@ forgetest_init!(can_clean_config, |prj, cmd| { assert!(!artifact.exists()); }); -// checks that `clean` removes fuzz cache dir -forgetest_init!(can_clean_fuzz_cache, |prj, cmd| { - let config = Config { fuzz: FuzzConfig::new("cache/fuzz".into()), ..Default::default() }; +// checks that `clean` removes fuzz and invariant cache dirs +forgetest_init!(can_clean_test_cache, |prj, cmd| { + let config = Config { + fuzz: FuzzConfig::new("cache/fuzz".into()), + invariant: InvariantConfig::new("cache/invariant".into()), + ..Default::default() + }; prj.write_config(config); // default test contract is written in custom out directory - let cache_dir = prj.root().join("cache/fuzz"); - let _ = fs::create_dir(cache_dir.clone()); - assert!(cache_dir.exists()); + let fuzz_cache_dir = prj.root().join("cache/fuzz"); + let _ = fs::create_dir(fuzz_cache_dir.clone()); + let invariant_cache_dir = prj.root().join("cache/invariant"); + let _ = fs::create_dir(invariant_cache_dir.clone()); + + assert!(fuzz_cache_dir.exists()); + assert!(invariant_cache_dir.exists()); cmd.forge_fuse().arg("clean"); cmd.output(); - assert!(!cache_dir.exists()); + assert!(!fuzz_cache_dir.exists()); + assert!(!invariant_cache_dir.exists()); }); // checks that extra output works diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index f60eebfdef7e..9daa37646f16 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -72,7 +72,11 @@ forgetest!(can_extract_config_values, |prj, cmd| { failure_persist_file: Some("failures".to_string()), ..Default::default() }, - invariant: InvariantConfig { runs: 256, ..Default::default() }, + invariant: InvariantConfig { + runs: 256, + failure_persist_dir: Some("test-cache/fuzz".into()), + ..Default::default() + }, ffi: true, always_use_create_2_factory: false, prompt_timeout: 0, diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 90bc4c9c590e..56713f4a907e 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -2,14 +2,33 @@ use crate::{config::*, test_helpers::TEST_DATA_DEFAULT}; use alloy_primitives::U256; -use forge::{fuzz::CounterExample, result::TestStatus, TestOptions}; +use forge::{fuzz::CounterExample, TestOptions}; use foundry_test_utils::Filter; use std::collections::BTreeMap; +macro_rules! get_counterexample { + ($runner:ident, $filter:expr) => { + $runner + .test_collect($filter) + .values() + .last() + .expect("Invariant contract should be testable.") + .test_results + .values() + .last() + .expect("Invariant contract should be testable.") + .counterexample + .as_ref() + .expect("Invariant contract should have failed with a counterexample.") + }; +} + #[tokio::test(flavor = "multi_thread")] async fn test_invariant() { let filter = Filter::new(".*", ".*", ".*fuzz/invariant/(target|targetAbi|common)"); let mut runner = TEST_DATA_DEFAULT.runner(); + runner.test_options.invariant.failure_persist_dir = + Some(tempfile::tempdir().unwrap().into_path()); let results = runner.test_collect(&filter); assert_multiple( @@ -255,20 +274,8 @@ async fn test_invariant_shrink() { let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantInnerContract.t.sol"); let mut runner = TEST_DATA_DEFAULT.runner(); runner.test_options.fuzz.seed = Some(U256::from(119u32)); - let results = runner.test_collect(&filter); - let results = - results.values().last().expect("`InvariantInnerContract.t.sol` should be testable."); - - let result = - results.test_results.values().last().expect("`InvariantInnerContract` should be testable."); - - let counter = result - .counterexample - .as_ref() - .expect("`InvariantInnerContract` should have failed with a counterexample."); - - match counter { + match get_counterexample!(runner, &filter) { CounterExample::Single(_) => panic!("CounterExample should be a sequence."), // `fuzz_seed` at 119 makes this sequence shrinkable from 4 to 2. CounterExample::Sequence(sequence) => { @@ -313,23 +320,8 @@ async fn test_shrink(opts: TestOptions, contract_pattern: &str) { ); let mut runner = TEST_DATA_DEFAULT.runner(); runner.test_options = opts.clone(); - let results = runner.test_collect(&filter); - let results = results.values().last().expect("`InvariantShrinkWithAssert` should be testable."); - - let result = results - .test_results - .values() - .last() - .expect("`InvariantShrinkWithAssert` should be testable."); - assert_eq!(result.status, TestStatus::Failure); - - let counter = result - .counterexample - .as_ref() - .expect("`InvariantShrinkWithAssert` should have failed with a counterexample."); - - match counter { + match get_counterexample!(runner, &filter) { CounterExample::Single(_) => panic!("CounterExample should be a sequence."), CounterExample::Sequence(sequence) => { assert!(sequence.len() <= 3); @@ -349,30 +341,67 @@ async fn test_shrink_big_sequence() { runner.test_options = opts.clone(); runner.test_options.invariant.runs = 1; runner.test_options.invariant.depth = 500; - let results = runner.test_collect(&filter); - let results = - results.values().last().expect("`InvariantShrinkBigSequence` should be testable."); - let result = results + let initial_counterexample = runner + .test_collect(&filter) + .values() + .last() + .expect("Invariant contract should be testable.") .test_results .values() .last() - .expect("`InvariantShrinkBigSequence` should be testable."); + .expect("Invariant contract should be testable.") + .counterexample + .clone() + .unwrap(); - assert_eq!(result.status, TestStatus::Failure); + let initial_sequence = match initial_counterexample { + CounterExample::Single(_) => panic!("CounterExample should be a sequence."), + CounterExample::Sequence(sequence) => sequence, + }; + // ensure shrinks to same sequence of 77 + assert_eq!(initial_sequence.len(), 77); - let counter = result + // test failure persistence + let results = runner.test_collect(&filter); + assert_multiple( + &results, + BTreeMap::from([( + "default/fuzz/invariant/common/InvariantShrinkBigSequence.t.sol:ShrinkBigSequenceTest", + vec![( + "invariant_shrink_big_sequence()", + false, + Some("invariant_shrink_big_sequence replay failure".into()), + None, + None, + )], + )]), + ); + let new_sequence = match results + .values() + .last() + .expect("Invariant contract should be testable.") + .test_results + .values() + .last() + .expect("Invariant contract should be testable.") .counterexample - .as_ref() - .expect("`InvariantShrinkBigSequence` should have failed with a counterexample."); - - match counter { + .clone() + .unwrap() + { CounterExample::Single(_) => panic!("CounterExample should be a sequence."), - CounterExample::Sequence(sequence) => { - // ensure shrinks to same sequence of 77 - assert_eq!(sequence.len(), 77); - } + CounterExample::Sequence(sequence) => sequence, }; + // ensure shrinks to same sequence of 77 + assert_eq!(new_sequence.len(), 77); + // ensure calls within failed sequence are the same as initial one + for index in 0..77 { + let new_call = new_sequence.get(index).unwrap(); + let initial_call = initial_sequence.get(index).unwrap(); + assert_eq!(new_call.sender, initial_call.sender); + assert_eq!(new_call.addr, initial_call.addr); + assert_eq!(new_call.calldata, initial_call.calldata); + } } #[tokio::test(flavor = "multi_thread")] @@ -388,24 +417,8 @@ async fn test_shrink_fail_on_revert() { runner.test_options.invariant.fail_on_revert = true; runner.test_options.invariant.runs = 1; runner.test_options.invariant.depth = 100; - let results = runner.test_collect(&filter); - let results = - results.values().last().expect("`InvariantShrinkFailOnRevert` should be testable."); - - let result = results - .test_results - .values() - .last() - .expect("`InvariantShrinkFailOnRevert` should be testable."); - - assert_eq!(result.status, TestStatus::Failure); - - let counter = result - .counterexample - .as_ref() - .expect("`InvariantShrinkFailOnRevert` should have failed with a counterexample."); - match counter { + match get_counterexample!(runner, &filter) { CounterExample::Single(_) => panic!("CounterExample should be a sequence."), CounterExample::Sequence(sequence) => { // ensure shrinks to sequence of 10 diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index be87b9ce9348..b2993151d327 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -107,6 +107,7 @@ impl ForgeTestProfile { shrink_run_limit: 2usize.pow(18u32), max_assume_rejects: 65536, gas_report_samples: 256, + failure_persist_dir: Some(tempfile::tempdir().unwrap().into_path()), }) .build(output, Path::new(self.project().root())) .expect("Config loaded") From f6e1eefd0a5ff8c9f8b5ed38c3cc36400d5704eb Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 9 May 2024 13:44:58 +0300 Subject: [PATCH 2/5] Fix unit test --- crates/config/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 906f25f0ac53..d233e62a8130 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -4461,7 +4461,12 @@ mod tests { let loaded = Config::load().sanitized(); assert_eq!( loaded.invariant, - InvariantConfig { runs: 512, depth: 10, ..Default::default() } + InvariantConfig { + runs: 512, + depth: 10, + failure_persist_dir: Some(PathBuf::from("cache/invariant")), + ..Default::default() + } ); Ok(()) From 9b220808c198d4831c5abed6c1c13b9e5e10b7c4 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Fri, 17 May 2024 14:01:40 +0300 Subject: [PATCH 3/5] Changes after review: - replace test cache rm macro with closure - use commons for load / persist failure sequence --- crates/config/src/invariant.rs | 7 -- crates/config/src/lib.rs | 25 +++---- crates/forge/src/runner.rs | 123 ++++++++++++++------------------- 3 files changed, 64 insertions(+), 91 deletions(-) diff --git a/crates/config/src/invariant.rs b/crates/config/src/invariant.rs index 3d5b9482f4b1..869125324cc5 100644 --- a/crates/config/src/invariant.rs +++ b/crates/config/src/invariant.rs @@ -75,13 +75,6 @@ impl InvariantConfig { .join("failures") .join(contract_name.split(':').last().unwrap()) } - - /// Returns path to persisted failure of current invariant test (if any). - /// The path to failure file is `{failure_dir}/{test_contract_name}/{invariant_name}`, - /// for example: `cache/invariant/failures/InvariantTest/invariant_check` - pub fn failure_file(self, contract_name: &str, test_name: String) -> PathBuf { - self.failure_dir(contract_name).join(test_name) - } } impl InlineConfigParser for InvariantConfig { diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index d233e62a8130..e4b7c60fef6b 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -21,7 +21,7 @@ use foundry_compilers::{ }, cache::SOLIDITY_FILES_CACHE_FILENAME, compilers::{solc::SolcVersionManager, CompilerVersionManager}, - error::{SolcError, SolcIoError}, + error::SolcError, remappings::{RelativeRemapping, Remapping}, CompilerConfig, ConfigurableArtifacts, EvmVersion, Project, ProjectPathsConfig, Solc, SolcConfig, @@ -787,20 +787,17 @@ impl Config { pub fn cleanup(&self, project: &Project) -> Result<(), SolcError> { project.cleanup()?; - macro_rules! remove_test_cache { - ($cache_dir:expr) => { - if let Some(test_cache) = $cache_dir { - let path = project.root().join(test_cache); - if path.exists() { - std::fs::remove_dir_all(&path).map_err(|e| SolcIoError::new(e, path))?; - } + // Remove fuzz and invariant cache directories. + let remove_test_dir = |test_dir: &Option| { + if let Some(test_dir) = test_dir { + let path = project.root().join(test_dir); + if path.exists() { + let _ = fs::remove_dir_all(&path); } - }; - } - // Remove fuzz cache directory. - remove_test_cache!(&self.fuzz.failure_persist_dir); - // Remove invariant cache directory. - remove_test_cache!(&self.invariant.failure_persist_dir); + } + }; + remove_test_dir(&self.fuzz.failure_persist_dir); + remove_test_dir(&self.invariant.failure_persist_dir); Ok(()) } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 98c36be7a0ab..a5b0d59d4eaa 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -36,7 +36,6 @@ use std::{ borrow::Cow, cmp::min, collections::{BTreeMap, HashMap}, - fs, sync::Arc, time::Instant, }; @@ -562,60 +561,55 @@ impl<'a> ContractRunner<'a> { let mut traces = traces.clone(); let mut coverage = coverage.clone(); + let failure_dir = invariant_config.clone().failure_dir(self.name); + let failure_file = failure_dir.join(invariant_contract.invariant_function.clone().name); + // Try to replay recorded failure if any. - let failure_file = invariant_config - .clone() - .failure_file(self.name, invariant_contract.invariant_function.clone().name); - if failure_file.exists() { - if let Ok(data) = fs::read_to_string(failure_file) { - if let Ok(call_sequence) = serde_json::from_str::>(&data) { - // Create calls from failed sequence and check if invariant still broken. - let txes = call_sequence - .clone() - .into_iter() - .map(|seq| { - ( - seq.sender.unwrap_or_default(), - (seq.addr.unwrap_or_default(), seq.calldata), - ) - }) - .collect::>(); - if let Ok(success) = check_sequence( + if let Ok(call_sequence) = + foundry_common::fs::read_json_file::>(failure_file.as_path()) + { + // Create calls from failed sequence and check if invariant still broken. + let txes = call_sequence + .clone() + .into_iter() + .map(|seq| { + (seq.sender.unwrap_or_default(), (seq.addr.unwrap_or_default(), seq.calldata)) + }) + .collect::>(); + if let Ok(success) = check_sequence( + self.executor.clone(), + &txes, + (0..min(txes.len(), invariant_config.depth as usize)).collect(), + invariant_contract.address, + invariant_contract.invariant_function.selector().to_vec().into(), + invariant_config.fail_on_revert, + ) { + if !success { + // If sequence still fails then replay error to collect traces and + // exit without executing new runs. + let _ = replay_run( + &invariant_contract, self.executor.clone(), - &txes, - (0..min(txes.len(), invariant_config.depth as usize)).collect(), - invariant_contract.address, - invariant_contract.invariant_function.selector().to_vec().into(), - invariant_config.fail_on_revert, - ) { - if !success { - // If sequence still fails then replay error to collect traces and - // exit without executing new runs. - let _ = replay_run( - &invariant_contract, - self.executor.clone(), - known_contracts, - identified_contracts.clone(), - &mut logs, - &mut traces, - &mut coverage, - txes, - ); - return TestResult { - status: TestStatus::Failure, - reason: Some(format!( - "{} replay failure", - invariant_contract.invariant_function.name - )), - decoded_logs: decode_console_logs(&logs), - traces, - coverage, - counterexample: Some(CounterExample::Sequence(call_sequence)), - kind: TestKind::Invariant { runs: 1, calls: 1, reverts: 1 }, - duration: start.elapsed(), - ..Default::default() - } - } + known_contracts, + identified_contracts.clone(), + &mut logs, + &mut traces, + &mut coverage, + txes, + ); + return TestResult { + status: TestStatus::Failure, + reason: Some(format!( + "{} replay failure", + invariant_contract.invariant_function.name + )), + decoded_logs: decode_console_logs(&logs), + traces, + coverage, + counterexample: Some(CounterExample::Sequence(call_sequence)), + kind: TestKind::Invariant { runs: 1, calls: 1, reverts: 1 }, + duration: start.elapsed(), + ..Default::default() } } } @@ -664,24 +658,13 @@ impl<'a> ContractRunner<'a> { Ok(call_sequence) => { if !call_sequence.is_empty() { // Persist error in invariant failure dir. - match fs::create_dir_all( - invariant_config.clone().failure_dir(self.name), + if let Err(err) = foundry_common::fs::create_dir_all(failure_dir) { + error!(%err, "Failed to create invariant failure dir"); + } else if let Err(err) = foundry_common::fs::write_json_file( + failure_file.as_path(), + &call_sequence, ) { - Ok(_) => { - let _ = fs::write( - invariant_config.failure_file( - self.name, - invariant_contract.invariant_function.clone().name, - ), - serde_json::to_string_pretty(&call_sequence).unwrap(), - ) - .map_err(|err| { - error!(%err, "Failed to record call sequence"); - }); - } - Err(err) => { - error!(%err, "Failed to create invariant failure dir") - } + error!(%err, "Failed to record call sequence"); } counterexample = Some(CounterExample::Sequence(call_sequence)) } From db1b619ef2464c32f3af87b160077894376903c8 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Fri, 17 May 2024 20:40:30 +0300 Subject: [PATCH 4/5] Changes after review: display proper message if replayed sequence reverts before checking invariant --- .../evm/evm/src/executors/invariant/shrink.rs | 30 +++++++++++++------ crates/forge/src/runner.rs | 17 +++++++---- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/crates/evm/evm/src/executors/invariant/shrink.rs b/crates/evm/evm/src/executors/invariant/shrink.rs index edfceb666bb0..d231184811cf 100644 --- a/crates/evm/evm/src/executors/invariant/shrink.rs +++ b/crates/evm/evm/src/executors/invariant/shrink.rs @@ -106,10 +106,10 @@ pub(crate) fn shrink_sequence( failed_case.fail_on_revert, ) { // If candidate sequence still fails then shrink more if possible. - Ok(false) if !shrinker.simplify() => break, + Ok((false, _)) if !shrinker.simplify() => break, // If candidate sequence pass then restore last removed call and shrink other // calls if possible. - Ok(true) if !shrinker.complicate() => break, + Ok((true, _)) if !shrinker.complicate() => break, _ => {} } } @@ -120,6 +120,7 @@ pub(crate) fn shrink_sequence( /// Checks if the given call sequence breaks the invariant. /// Used in shrinking phase for checking candidate sequences and in replay failures phase to test /// persisted failures. +/// Returns the result of invariant check and if sequence was entirely applied. pub fn check_sequence( mut executor: Executor, calls: &[BasicTxDetails], @@ -127,10 +128,15 @@ pub fn check_sequence( test_address: Address, test_function: Bytes, fail_on_revert: bool, -) -> eyre::Result { +) -> eyre::Result<(bool, bool)> { let mut sequence_failed = false; // Apply the call sequence. + // We keep track of the number of calls applied to check if persisted sequence is + // replayed entirely. + let mut calls_applied = 0; for call_index in sequence { + calls_applied += 1; + let (sender, (addr, bytes)) = &calls[call_index]; let call_result = executor.call_raw_committing(*sender, *addr, bytes.clone(), U256::ZERO)?; @@ -141,17 +147,23 @@ pub fn check_sequence( break; } } + + let replayed_entirely = calls_applied == calls.len(); + // Return without checking the invariant if we already have a failing sequence. if sequence_failed { - return Ok(false); + return Ok((false, replayed_entirely)); }; // Check the invariant for call sequence. let mut call_result = executor.call_raw(CALLER, test_address, test_function, U256::ZERO)?; - Ok(executor.is_raw_call_success( - test_address, - Cow::Owned(call_result.state_changeset.take().unwrap()), - &call_result, - false, + Ok(( + executor.is_raw_call_success( + test_address, + Cow::Owned(call_result.state_changeset.take().unwrap()), + &call_result, + false, + ), + replayed_entirely, )) } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index a5b0d59d4eaa..168d1df7c438 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -576,7 +576,7 @@ impl<'a> ContractRunner<'a> { (seq.sender.unwrap_or_default(), (seq.addr.unwrap_or_default(), seq.calldata)) }) .collect::>(); - if let Ok(success) = check_sequence( + if let Ok((success, replayed_entirely)) = check_sequence( self.executor.clone(), &txes, (0..min(txes.len(), invariant_config.depth as usize)).collect(), @@ -599,10 +599,17 @@ impl<'a> ContractRunner<'a> { ); return TestResult { status: TestStatus::Failure, - reason: Some(format!( - "{} replay failure", - invariant_contract.invariant_function.name - )), + reason: if replayed_entirely { + Some(format!( + "{} replay failure", + invariant_contract.invariant_function.name + )) + } else { + Some(format!( + "{} persisted failure revert", + invariant_contract.invariant_function.name + )) + }, decoded_logs: decode_console_logs(&logs), traces, coverage, From 7686c03398e99f36af8462db6798c579e229827b Mon Sep 17 00:00:00 2001 From: grandizzy Date: Fri, 17 May 2024 21:59:30 +0300 Subject: [PATCH 5/5] Changes after review: simplify check sequence logic --- .../evm/evm/src/executors/invariant/shrink.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/crates/evm/evm/src/executors/invariant/shrink.rs b/crates/evm/evm/src/executors/invariant/shrink.rs index d231184811cf..d15ce3ef118e 100644 --- a/crates/evm/evm/src/executors/invariant/shrink.rs +++ b/crates/evm/evm/src/executors/invariant/shrink.rs @@ -129,32 +129,18 @@ pub fn check_sequence( test_function: Bytes, fail_on_revert: bool, ) -> eyre::Result<(bool, bool)> { - let mut sequence_failed = false; // Apply the call sequence. - // We keep track of the number of calls applied to check if persisted sequence is - // replayed entirely. - let mut calls_applied = 0; for call_index in sequence { - calls_applied += 1; - let (sender, (addr, bytes)) = &calls[call_index]; let call_result = executor.call_raw_committing(*sender, *addr, bytes.clone(), U256::ZERO)?; if call_result.reverted && fail_on_revert { // Candidate sequence fails test. // We don't have to apply remaining calls to check sequence. - sequence_failed = true; - break; + return Ok((false, false)); } } - let replayed_entirely = calls_applied == calls.len(); - - // Return without checking the invariant if we already have a failing sequence. - if sequence_failed { - return Ok((false, replayed_entirely)); - }; - // Check the invariant for call sequence. let mut call_result = executor.call_raw(CALLER, test_address, test_function, U256::ZERO)?; Ok(( @@ -164,6 +150,6 @@ pub fn check_sequence( &call_result, false, ), - replayed_entirely, + true, )) }