From 47bf0754d01cccb7eb1099f743411e5cf8a1cf78 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Sat, 23 Nov 2024 22:38:45 -0500 Subject: [PATCH 1/8] feat: add timeouts to fuzz testing Adds --fuzz-timeout-secs to fuzz tests which will cause a property test to timeout after a certain number of seconds. Also adds --fuzz-allow-timeouts so that timeouts are optionally not considered to be failures. --- crates/config/src/fuzz.rs | 8 ++++++ crates/evm/evm/src/executors/fuzz/mod.rs | 34 ++++++++++++++++++------ crates/forge/bin/cmd/test/mod.rs | 14 ++++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/crates/config/src/fuzz.rs b/crates/config/src/fuzz.rs index fb04383228a9..3d20cc00476d 100644 --- a/crates/config/src/fuzz.rs +++ b/crates/config/src/fuzz.rs @@ -32,6 +32,10 @@ pub struct FuzzConfig { pub failure_persist_file: Option, /// show `console.log` in fuzz test, defaults to `false` pub show_logs: bool, + /// Optional timeout for each property test + pub timeout_secs: Option, + /// Optionally allow timeouts to not be reported as failures + pub allow_timeouts: bool, } impl Default for FuzzConfig { @@ -45,6 +49,8 @@ impl Default for FuzzConfig { failure_persist_dir: None, failure_persist_file: None, show_logs: false, + timeout_secs: None, + allow_timeouts: false, } } } @@ -61,6 +67,8 @@ impl FuzzConfig { failure_persist_dir: Some(cache_dir), failure_persist_file: Some("failures".to_string()), show_logs: false, + timeout_secs: None, + allow_timeouts: false, } } } diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 2bbe80a63ead..2a9a54159b0a 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -98,7 +98,19 @@ impl FuzzedExecutor { let max_traces_to_collect = std::cmp::max(1, self.config.gas_report_samples) as usize; let show_logs = self.config.show_logs; + // Start a timer if timeout is set + let start_time = self.config.timeout_secs.map(|timeout| { + (std::time::Instant::now(), std::time::Duration::from_secs(timeout)) + }); + let run_result = self.runner.clone().run(&strategy, |calldata| { + // Check if the timeout has been reached + if let Some((start_time, timeout)) = start_time { + if start_time.elapsed() > timeout { + return Err(TestCaseError::fail("Timeout reached")) + } + } + let fuzz_res = self.single_fuzz(address, should_fail, calldata)?; // If running with progress then increment current run. @@ -193,17 +205,23 @@ impl FuzzedExecutor { } Err(TestError::Fail(reason, _)) => { let reason = reason.to_string(); - result.reason = (!reason.is_empty()).then_some(reason); + result.reason = (!reason.is_empty()).then_some(reason.clone()); - let args = if let Some(data) = calldata.get(4..) { - func.abi_decode_input(data, false).unwrap_or_default() + if reason == "Timeout reached" { + if self.config.allow_timeouts { + result.success = true; + } } else { - vec![] - }; + let args = if let Some(data) = calldata.get(4..) { + func.abi_decode_input(data, false).unwrap_or_default() + } else { + vec![] + }; - result.counterexample = Some(CounterExample::Single( - BaseCounterExample::from_fuzz_call(calldata, args, call.traces), - )); + result.counterexample = Some(CounterExample::Single( + BaseCounterExample::from_fuzz_call(calldata, args, call.traces), + )); + } } } diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 1a409b33a822..9893edc3870e 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -145,6 +145,14 @@ pub struct TestArgs { #[arg(long, env = "FOUNDRY_FUZZ_RUNS", value_name = "RUNS")] pub fuzz_runs: Option, + /// Timeout for each fuzz run in seconds. + #[arg(long, env = "FOUNDRY_FUZZ_TIMEOUT_SECS", value_name = "TIMEOUT_SECS")] + pub fuzz_timeout_secs: Option, + + /// Allow timeouts to not be reported as failures. + #[arg(long, env = "FOUNDRY_FUZZ_ALLOW_TIMEOUTS", value_name = "ALLOW_TIMEOUTS")] + pub fuzz_allow_timeouts: bool, + /// File to rerun fuzz failures from. #[arg(long)] pub fuzz_input_file: Option, @@ -870,6 +878,12 @@ 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_timeout_secs) = self.fuzz_timeout_secs { + fuzz_dict.insert("timeout_secs".to_string(), fuzz_timeout_secs.into()); + } + if self.fuzz_allow_timeouts { + fuzz_dict.insert("allow_timeouts".to_string(), true.into()); + } if let Some(fuzz_input_file) = self.fuzz_input_file.clone() { fuzz_dict.insert("failure_persist_file".to_string(), fuzz_input_file.into()); } From b2a5f8dae43e7cd799402347cb7088d8635fe4d7 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Mon, 25 Nov 2024 11:49:25 -0500 Subject: [PATCH 2/8] simplify timeout implementation --- crates/config/src/fuzz.rs | 13 +++---- crates/config/src/invariant.rs | 5 +++ crates/evm/evm/src/executors/fuzz/mod.rs | 35 +++++++++---------- crates/evm/evm/src/executors/invariant/mod.rs | 17 +++++++++ crates/forge/bin/cmd/test/mod.rs | 15 +++----- 5 files changed, 48 insertions(+), 37 deletions(-) diff --git a/crates/config/src/fuzz.rs b/crates/config/src/fuzz.rs index 3d20cc00476d..db0803643a6d 100644 --- a/crates/config/src/fuzz.rs +++ b/crates/config/src/fuzz.rs @@ -32,10 +32,8 @@ pub struct FuzzConfig { pub failure_persist_file: Option, /// show `console.log` in fuzz test, defaults to `false` pub show_logs: bool, - /// Optional timeout for each property test - pub timeout_secs: Option, - /// Optionally allow timeouts to not be reported as failures - pub allow_timeouts: bool, + /// Optional timeout (in seconds) for each property test + pub timeout: Option, } impl Default for FuzzConfig { @@ -49,8 +47,7 @@ impl Default for FuzzConfig { failure_persist_dir: None, failure_persist_file: None, show_logs: false, - timeout_secs: None, - allow_timeouts: false, + timeout: None, } } } @@ -67,8 +64,7 @@ impl FuzzConfig { failure_persist_dir: Some(cache_dir), failure_persist_file: Some("failures".to_string()), show_logs: false, - timeout_secs: None, - allow_timeouts: false, + timeout: None, } } } @@ -98,6 +94,7 @@ impl InlineConfigParser for FuzzConfig { } "failure-persist-file" => conf_clone.failure_persist_file = Some(value), "show-logs" => conf_clone.show_logs = parse_config_bool(key, value)?, + "timeout" => conf_clone.timeout = Some(parse_config_u64(key, value)?), _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } diff --git a/crates/config/src/invariant.rs b/crates/config/src/invariant.rs index 70e9a2b85847..6ca854d21bec 100644 --- a/crates/config/src/invariant.rs +++ b/crates/config/src/invariant.rs @@ -36,6 +36,8 @@ pub struct InvariantConfig { pub failure_persist_dir: Option, /// Whether to collect and display fuzzed selectors metrics. pub show_metrics: bool, + /// Optional timeout (in seconds) for each invariant test. + pub timeout: Option, } impl Default for InvariantConfig { @@ -51,6 +53,7 @@ impl Default for InvariantConfig { gas_report_samples: 256, failure_persist_dir: None, show_metrics: false, + timeout: None, } } } @@ -69,6 +72,7 @@ impl InvariantConfig { gas_report_samples: 256, failure_persist_dir: Some(cache_dir), show_metrics: false, + timeout: None, } } @@ -108,6 +112,7 @@ impl InlineConfigParser for InvariantConfig { } "shrink-run-limit" => conf_clone.shrink_run_limit = parse_config_u32(key, value)?, "show-metrics" => conf_clone.show_metrics = parse_config_bool(key, value)?, + "timeout" => conf_clone.timeout = Some(parse_config_u64(key, value)?), _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 2a9a54159b0a..13c91d39a373 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -98,16 +98,21 @@ impl FuzzedExecutor { let max_traces_to_collect = std::cmp::max(1, self.config.gas_report_samples) as usize; let show_logs = self.config.show_logs; - // Start a timer if timeout is set - let start_time = self.config.timeout_secs.map(|timeout| { + // Start a timer if timeout is set. + let start_time = self.config.timeout.map(|timeout| { (std::time::Instant::now(), std::time::Duration::from_secs(timeout)) }); let run_result = self.runner.clone().run(&strategy, |calldata| { - // Check if the timeout has been reached + // Check if the timeout has been reached. if let Some((start_time, timeout)) = start_time { if start_time.elapsed() > timeout { - return Err(TestCaseError::fail("Timeout reached")) + // At some point we might want to have a timeout be considered a failure. + // Easiest way to do this is to return an error here if some flag is set. + // Will correctly NOT increment the number of runs that is presented to the + // user because that number is calculated as the length of gas_by_case which + // doesn't get pushed to if we hit this branch. + return Ok(()); } } @@ -205,23 +210,17 @@ impl FuzzedExecutor { } Err(TestError::Fail(reason, _)) => { let reason = reason.to_string(); - result.reason = (!reason.is_empty()).then_some(reason.clone()); + result.reason = (!reason.is_empty()).then_some(reason); - if reason == "Timeout reached" { - if self.config.allow_timeouts { - result.success = true; - } + let args = if let Some(data) = calldata.get(4..) { + func.abi_decode_input(data, false).unwrap_or_default() } else { - let args = if let Some(data) = calldata.get(4..) { - func.abi_decode_input(data, false).unwrap_or_default() - } else { - vec![] - }; + vec![] + }; - result.counterexample = Some(CounterExample::Single( - BaseCounterExample::from_fuzz_call(calldata, args, call.traces), - )); - } + 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 f98dd21114cb..3bc8b59124c4 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -332,7 +332,24 @@ impl<'a> InvariantExecutor<'a> { let (invariant_test, invariant_strategy) = self.prepare_test(&invariant_contract, fuzz_fixtures)?; + // Start a timer if timeout is set. + let start_time = self.config.timeout.map(|timeout| { + (std::time::Instant::now(), std::time::Duration::from_secs(timeout)) + }); + let _ = self.runner.run(&invariant_strategy, |first_input| { + // Check if the timeout has been reached. + if let Some((start_time, timeout)) = start_time { + if start_time.elapsed() > timeout { + // At some point we might want to have a timeout be considered a failure. + // Easiest way to do this is to return an error here if some flag is set. + // Will correctly NOT increment the number of runs that is presented to the + // user because that number is calculated as the length of fuzz_cases which + // doesn't get push to if we hit this branch. + return Ok(()); + } + } + // Create current invariant run data. let mut current_run = InvariantTestRun::new( first_input, diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 9893edc3870e..576c9bc9ee79 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -146,12 +146,8 @@ pub struct TestArgs { pub fuzz_runs: Option, /// Timeout for each fuzz run in seconds. - #[arg(long, env = "FOUNDRY_FUZZ_TIMEOUT_SECS", value_name = "TIMEOUT_SECS")] - pub fuzz_timeout_secs: Option, - - /// Allow timeouts to not be reported as failures. - #[arg(long, env = "FOUNDRY_FUZZ_ALLOW_TIMEOUTS", value_name = "ALLOW_TIMEOUTS")] - pub fuzz_allow_timeouts: bool, + #[arg(long, env = "FOUNDRY_FUZZ_TIMEOUT", value_name = "TIMEOUT")] + pub fuzz_timeout: Option, /// File to rerun fuzz failures from. #[arg(long)] @@ -878,11 +874,8 @@ 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_timeout_secs) = self.fuzz_timeout_secs { - fuzz_dict.insert("timeout_secs".to_string(), fuzz_timeout_secs.into()); - } - if self.fuzz_allow_timeouts { - fuzz_dict.insert("allow_timeouts".to_string(), true.into()); + if let Some(fuzz_timeout) = self.fuzz_timeout { + fuzz_dict.insert("timeout".to_string(), fuzz_timeout.into()); } if let Some(fuzz_input_file) = self.fuzz_input_file.clone() { fuzz_dict.insert("failure_persist_file".to_string(), fuzz_input_file.into()); From a9003b3a478fdce1fedff209d4169b25cd076ac3 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Mon, 25 Nov 2024 12:00:35 -0500 Subject: [PATCH 3/8] use u32 for timeout --- crates/config/src/fuzz.rs | 4 ++-- crates/config/src/invariant.rs | 4 ++-- crates/evm/evm/src/executors/fuzz/mod.rs | 2 +- crates/evm/evm/src/executors/invariant/mod.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/config/src/fuzz.rs b/crates/config/src/fuzz.rs index db0803643a6d..36b6d0823d87 100644 --- a/crates/config/src/fuzz.rs +++ b/crates/config/src/fuzz.rs @@ -33,7 +33,7 @@ pub struct FuzzConfig { /// show `console.log` in fuzz test, defaults to `false` pub show_logs: bool, /// Optional timeout (in seconds) for each property test - pub timeout: Option, + pub timeout: Option, } impl Default for FuzzConfig { @@ -94,7 +94,7 @@ impl InlineConfigParser for FuzzConfig { } "failure-persist-file" => conf_clone.failure_persist_file = Some(value), "show-logs" => conf_clone.show_logs = parse_config_bool(key, value)?, - "timeout" => conf_clone.timeout = Some(parse_config_u64(key, value)?), + "timeout" => conf_clone.timeout = Some(parse_config_u32(key, value)?), _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } diff --git a/crates/config/src/invariant.rs b/crates/config/src/invariant.rs index 6ca854d21bec..f7ddb84f04f7 100644 --- a/crates/config/src/invariant.rs +++ b/crates/config/src/invariant.rs @@ -37,7 +37,7 @@ pub struct InvariantConfig { /// Whether to collect and display fuzzed selectors metrics. pub show_metrics: bool, /// Optional timeout (in seconds) for each invariant test. - pub timeout: Option, + pub timeout: Option, } impl Default for InvariantConfig { @@ -112,7 +112,7 @@ impl InlineConfigParser for InvariantConfig { } "shrink-run-limit" => conf_clone.shrink_run_limit = parse_config_u32(key, value)?, "show-metrics" => conf_clone.show_metrics = parse_config_bool(key, value)?, - "timeout" => conf_clone.timeout = Some(parse_config_u64(key, value)?), + "timeout" => conf_clone.timeout = Some(parse_config_u32(key, value)?), _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 13c91d39a373..9d7147115390 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -100,7 +100,7 @@ impl FuzzedExecutor { // Start a timer if timeout is set. let start_time = self.config.timeout.map(|timeout| { - (std::time::Instant::now(), std::time::Duration::from_secs(timeout)) + (std::time::Instant::now(), std::time::Duration::from_secs(timeout.into())) }); let run_result = self.runner.clone().run(&strategy, |calldata| { diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index 3bc8b59124c4..3f693b1b9535 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -334,7 +334,7 @@ impl<'a> InvariantExecutor<'a> { // Start a timer if timeout is set. let start_time = self.config.timeout.map(|timeout| { - (std::time::Instant::now(), std::time::Duration::from_secs(timeout)) + (std::time::Instant::now(), std::time::Duration::from_secs(timeout.into())) }); let _ = self.runner.run(&invariant_strategy, |first_input| { From b704851691c32ad5eb68ce716ae97321ac7bdabe Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Tue, 26 Nov 2024 10:59:24 -0500 Subject: [PATCH 4/8] switch back to failing for timeouts --- crates/evm/evm/src/executors/fuzz/mod.rs | 28 +++++++++---------- crates/evm/evm/src/executors/invariant/mod.rs | 10 +++---- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 9d7147115390..3e572ea468eb 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -107,12 +107,7 @@ impl FuzzedExecutor { // Check if the timeout has been reached. if let Some((start_time, timeout)) = start_time { if start_time.elapsed() > timeout { - // At some point we might want to have a timeout be considered a failure. - // Easiest way to do this is to return an error here if some flag is set. - // Will correctly NOT increment the number of runs that is presented to the - // user because that number is calculated as the length of gas_by_case which - // doesn't get pushed to if we hit this branch. - return Ok(()); + return Err(TestCaseError::fail("Timeout reached")); } } @@ -210,17 +205,20 @@ impl FuzzedExecutor { } Err(TestError::Fail(reason, _)) => { let reason = reason.to_string(); - result.reason = (!reason.is_empty()).then_some(reason); - - let args = if let Some(data) = calldata.get(4..) { - func.abi_decode_input(data, false).unwrap_or_default() + if reason == "Timeout reached" { + // If the reason is a timeout, we consider the fuzz test successful. + result.success = true; } else { - vec![] - }; + result.reason = (!reason.is_empty()).then_some(reason); + let args = calldata + .get(..4) + .and_then(|data| func.abi_decode_input(data, false).ok()) + .unwrap_or_default(); - result.counterexample = Some(CounterExample::Single( - BaseCounterExample::from_fuzz_call(calldata, args, call.traces), - )); + 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 3f693b1b9535..55a156463a10 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -341,12 +341,10 @@ impl<'a> InvariantExecutor<'a> { // Check if the timeout has been reached. if let Some((start_time, timeout)) = start_time { if start_time.elapsed() > timeout { - // At some point we might want to have a timeout be considered a failure. - // Easiest way to do this is to return an error here if some flag is set. - // Will correctly NOT increment the number of runs that is presented to the - // user because that number is calculated as the length of fuzz_cases which - // doesn't get push to if we hit this branch. - return Ok(()); + // Since we never record a revert here the test is still considered successful + // even though it timed out. We *want* this behavior for now, so that's ok, but + // future developers should be aware of this. + return Err(TestCaseError::fail("Timeout reached")); } } From 73c1b7705bcf9b6201d2835d2579ea4df159a650 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 27 Nov 2024 03:31:56 +0100 Subject: [PATCH 5/8] clippy --- crates/evm/evm/src/executors/fuzz/mod.rs | 6 +++--- crates/forge/tests/it/test_helpers.rs | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 3e572ea468eb..c6f492d6a98a 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -211,9 +211,9 @@ impl FuzzedExecutor { } else { result.reason = (!reason.is_empty()).then_some(reason); let args = calldata - .get(..4) - .and_then(|data| func.abi_decode_input(data, false).ok()) - .unwrap_or_default(); + .get(..4) + .and_then(|data| func.abi_decode_input(data, false).ok()) + .unwrap_or_default(); result.counterexample = Some(CounterExample::Single( BaseCounterExample::from_fuzz_call(calldata, args, call.traces), diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index 5e540d8c67aa..b7f9367f5be8 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -95,6 +95,7 @@ impl ForgeTestProfile { failure_persist_dir: Some(tempfile::tempdir().unwrap().into_path()), failure_persist_file: Some("testfailure".to_string()), show_logs: false, + timeout: None, }) .invariant(InvariantConfig { runs: 256, @@ -113,6 +114,7 @@ impl ForgeTestProfile { gas_report_samples: 256, failure_persist_dir: Some(tempfile::tempdir().unwrap().into_path()), show_metrics: false, + timeout: None, }) .build(output, Path::new(self.project().root())) .expect("Config loaded") From af9324ab0a64a72eafb37683acfffa11940173eb Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 27 Nov 2024 10:43:34 +0200 Subject: [PATCH 6/8] Nits: - move logic to interrupt invariant test in depth loop - add and reuse start_timer fn and TEST_TIMEOUT constant - add fuzz and invariant tests - fix failing test --- crates/evm/core/src/constants.rs | 3 ++ crates/evm/evm/src/executors/fuzz/mod.rs | 28 +++++++---- crates/evm/evm/src/executors/invariant/mod.rs | 27 +++++----- crates/forge/tests/it/fuzz.rs | 35 +++++++++++++ crates/forge/tests/it/invariant.rs | 50 +++++++++++++++++++ 5 files changed, 120 insertions(+), 23 deletions(-) diff --git a/crates/evm/core/src/constants.rs b/crates/evm/core/src/constants.rs index 70c7441d20a7..cebbdcb87c79 100644 --- a/crates/evm/core/src/constants.rs +++ b/crates/evm/core/src/constants.rs @@ -37,6 +37,9 @@ pub const MAGIC_ASSUME: &[u8] = b"FOUNDRY::ASSUME"; /// Magic return value returned by the `skip` cheatcode. Optionally appended with a reason. pub const MAGIC_SKIP: &[u8] = b"FOUNDRY::SKIP"; +/// Test timeout return value. +pub const TEST_TIMEOUT: &str = "FOUNDRY::TEST_TIMEOUT"; + /// The address that deploys the default CREATE2 deployer contract. pub const DEFAULT_CREATE2_DEPLOYER_DEPLOYER: Address = address!("3fAB184622Dc19b6109349B94811493BF2a45362"); diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index c6f492d6a98a..a5aad53ead93 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -6,7 +6,7 @@ use eyre::Result; use foundry_common::evm::Breakpoints; use foundry_config::FuzzConfig; use foundry_evm_core::{ - constants::MAGIC_ASSUME, + constants::{MAGIC_ASSUME, TEST_TIMEOUT}, decode::{RevertDecoder, SkipReason}, }; use foundry_evm_coverage::HitMaps; @@ -18,6 +18,7 @@ use foundry_evm_traces::SparsedTraceArena; use indicatif::ProgressBar; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; use std::{cell::RefCell, collections::BTreeMap}; +use std::time::{Duration, Instant}; mod types; pub use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome}; @@ -99,15 +100,13 @@ impl FuzzedExecutor { let show_logs = self.config.show_logs; // Start a timer if timeout is set. - let start_time = self.config.timeout.map(|timeout| { - (std::time::Instant::now(), std::time::Duration::from_secs(timeout.into())) - }); + let start_time = start_timer(self.config.timeout); let run_result = self.runner.clone().run(&strategy, |calldata| { // Check if the timeout has been reached. if let Some((start_time, timeout)) = start_time { if start_time.elapsed() > timeout { - return Err(TestCaseError::fail("Timeout reached")); + return Err(TestCaseError::fail(TEST_TIMEOUT)); } } @@ -205,15 +204,16 @@ impl FuzzedExecutor { } Err(TestError::Fail(reason, _)) => { let reason = reason.to_string(); - if reason == "Timeout reached" { + if reason == TEST_TIMEOUT { // If the reason is a timeout, we consider the fuzz test successful. result.success = true; } else { result.reason = (!reason.is_empty()).then_some(reason); - let args = calldata - .get(..4) - .and_then(|data| func.abi_decode_input(data, false).ok()) - .unwrap_or_default(); + let args = if let Some(data) = calldata.get(4..) { + func.abi_decode_input(data, false).unwrap_or_default() + } else { + vec![] + }; result.counterexample = Some(CounterExample::Single( BaseCounterExample::from_fuzz_call(calldata, args, call.traces), @@ -285,3 +285,11 @@ impl FuzzedExecutor { } } } + + +/// Starts timer for fuzz test, if any timeout configured. +pub(crate) fn start_timer(timeout: Option) -> Option<(Instant, Duration)> { + timeout.map(|timeout| { + (Instant::now(), Duration::from_secs(timeout.into())) + }) +} diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index 55a156463a10..6370c1b87c50 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -10,6 +10,7 @@ use foundry_config::InvariantConfig; use foundry_evm_core::{ constants::{ CALLER, CHEATCODE_ADDRESS, DEFAULT_CREATE2_DEPLOYER, HARDHAT_CONSOLE_ADDRESS, MAGIC_ASSUME, + TEST_TIMEOUT, }, precompiles::PRECOMPILES, }; @@ -51,6 +52,7 @@ use serde::{Deserialize, Serialize}; mod shrink; use crate::executors::EvmError; pub use shrink::check_sequence; +use crate::executors::fuzz::start_timer; sol! { interface IInvariantTest { @@ -333,21 +335,9 @@ impl<'a> InvariantExecutor<'a> { self.prepare_test(&invariant_contract, fuzz_fixtures)?; // Start a timer if timeout is set. - let start_time = self.config.timeout.map(|timeout| { - (std::time::Instant::now(), std::time::Duration::from_secs(timeout.into())) - }); + let start_time = start_timer(self.config.timeout); let _ = self.runner.run(&invariant_strategy, |first_input| { - // Check if the timeout has been reached. - if let Some((start_time, timeout)) = start_time { - if start_time.elapsed() > timeout { - // Since we never record a revert here the test is still considered successful - // even though it timed out. We *want* this behavior for now, so that's ok, but - // future developers should be aware of this. - return Err(TestCaseError::fail("Timeout reached")); - } - } - // Create current invariant run data. let mut current_run = InvariantTestRun::new( first_input, @@ -362,6 +352,17 @@ impl<'a> InvariantExecutor<'a> { } while current_run.depth < self.config.depth { + // Check if the timeout has been reached. + if let Some((start_time, timeout)) = start_time { + if start_time.elapsed() > timeout { + // Since we never record a revert here the test is still considered + // successful even though it timed out. We *want* + // this behavior for now, so that's ok, but + // future developers should be aware of this. + return Err(TestCaseError::fail(TEST_TIMEOUT)); + } + } + let tx = current_run.inputs.last().ok_or_else(|| { TestCaseError::fail("No input generated to call fuzzed target.") })?; diff --git a/crates/forge/tests/it/fuzz.rs b/crates/forge/tests/it/fuzz.rs index 8972c9bd98f1..9c9b742ddfb0 100644 --- a/crates/forge/tests/it/fuzz.rs +++ b/crates/forge/tests/it/fuzz.rs @@ -235,3 +235,38 @@ contract InlineMaxRejectsTest is Test { ... "#]]); }); + +// Tests that test timeout config is properly applied. +// If test doesn't timeout after one second, then test will fail with `rejected too many inputs`. +forgetest_init!(test_fuzz_timeout, |prj, cmd| { + prj.wipe_contracts(); + + prj.add_test( + "Contract.t.sol", + r#" +import {Test} from "forge-std/Test.sol"; + +contract FuzzTimeoutTest is Test { + /// forge-config: default.fuzz.max-test-rejects = 10000 + /// forge-config: default.fuzz.timeout = 1 + function test_fuzz_bound(uint256 a) public pure { + vm.assume(a == 0); + } +} + "#, + ) + .unwrap(); + + cmd.args(["test"]).assert_success().stdout_eq(str![[r#" +[COMPILING_FILES] with [SOLC_VERSION] +[SOLC_VERSION] [ELAPSED] +Compiler run successful! + +Ran 1 test for test/Contract.t.sol:FuzzTimeoutTest +[PASS] test_fuzz_bound(uint256) (runs: [..], [AVG_GAS]) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] + +Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +"#]]); +}); diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 3e09cd465a33..679acd1e7931 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -928,3 +928,53 @@ Ran 2 tests for test/SelectorMetricsTest.t.sol:CounterTest ... "#]]); }); + +// Tests that invariant exists with success after configured timeout. +forgetest_init!(should_apply_configured_timeout, |prj, cmd| { + // Add initial test that breaks invariant. + prj.add_test( + "TimeoutTest.t.sol", + r#" +import {Test} from "forge-std/Test.sol"; + +contract TimeoutHandler is Test { + uint256 public count; + + function increment() public { + count++; + } +} + +contract TimeoutTest is Test { + TimeoutHandler handler; + + function setUp() public { + handler = new TimeoutHandler(); + } + + /// forge-config: default.invariant.runs = 10000 + /// forge-config: default.invariant.depth = 20000 + /// forge-config: default.invariant.timeout = 1 + function invariant_counter_timeout() public view { + // Invariant will fail if more than 10000 increments. + // Make sure test timeouts after one second and remaining runs are canceled. + require(handler.count() < 10000); + } +} + "#, + ) + .unwrap(); + + cmd.args(["test", "--mt", "invariant_counter_timeout"]).assert_success().stdout_eq(str![[r#" +[COMPILING_FILES] with [SOLC_VERSION] +[SOLC_VERSION] [ELAPSED] +Compiler run successful! + +Ran 1 test for test/TimeoutTest.t.sol:TimeoutTest +[PASS] invariant_counter_timeout() (runs: 0, calls: 0, reverts: 0) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] + +Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +"#]]); +}); From 364c33bf4009ebb7cb9564de77ca59345a56864b Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 27 Nov 2024 11:03:15 +0200 Subject: [PATCH 7/8] Fix fmt --- crates/evm/evm/src/executors/fuzz/mod.rs | 12 ++++++------ crates/evm/evm/src/executors/invariant/mod.rs | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index a5aad53ead93..ac2fac8d5133 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -17,8 +17,11 @@ use foundry_evm_fuzz::{ use foundry_evm_traces::SparsedTraceArena; use indicatif::ProgressBar; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; -use std::{cell::RefCell, collections::BTreeMap}; -use std::time::{Duration, Instant}; +use std::{ + cell::RefCell, + collections::BTreeMap, + time::{Duration, Instant}, +}; mod types; pub use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome}; @@ -286,10 +289,7 @@ impl FuzzedExecutor { } } - /// Starts timer for fuzz test, if any timeout configured. pub(crate) fn start_timer(timeout: Option) -> Option<(Instant, Duration)> { - timeout.map(|timeout| { - (Instant::now(), Duration::from_secs(timeout.into())) - }) + timeout.map(|timeout| (Instant::now(), Duration::from_secs(timeout.into()))) } diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index 6370c1b87c50..bb5874f79010 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -50,9 +50,8 @@ pub use result::InvariantFuzzTestResult; use serde::{Deserialize, Serialize}; mod shrink; -use crate::executors::EvmError; +use crate::executors::{fuzz::start_timer, EvmError}; pub use shrink::check_sequence; -use crate::executors::fuzz::start_timer; sol! { interface IInvariantTest { From da87095e1fe7f7cb7de4d8099c2307ccd408982b Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 28 Nov 2024 12:30:48 +0200 Subject: [PATCH 8/8] Changes after review: introduce FuzzTestTimer --- crates/evm/evm/src/executors/fuzz/mod.rs | 23 +++++-------------- crates/evm/evm/src/executors/invariant/mod.rs | 20 ++++++++-------- crates/evm/evm/src/executors/mod.rs | 22 +++++++++++++++++- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index ac2fac8d5133..0d79f8fa4961 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -1,4 +1,4 @@ -use crate::executors::{Executor, RawCallResult}; +use crate::executors::{Executor, FuzzTestTimer, RawCallResult}; use alloy_dyn_abi::JsonAbiExt; use alloy_json_abi::Function; use alloy_primitives::{map::HashMap, Address, Bytes, Log, U256}; @@ -17,11 +17,7 @@ use foundry_evm_fuzz::{ use foundry_evm_traces::SparsedTraceArena; use indicatif::ProgressBar; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; -use std::{ - cell::RefCell, - collections::BTreeMap, - time::{Duration, Instant}, -}; +use std::{cell::RefCell, collections::BTreeMap}; mod types; pub use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome}; @@ -102,15 +98,13 @@ impl FuzzedExecutor { let max_traces_to_collect = std::cmp::max(1, self.config.gas_report_samples) as usize; let show_logs = self.config.show_logs; - // Start a timer if timeout is set. - let start_time = start_timer(self.config.timeout); + // Start timer for this fuzz test. + let timer = FuzzTestTimer::new(self.config.timeout); let run_result = self.runner.clone().run(&strategy, |calldata| { // Check if the timeout has been reached. - if let Some((start_time, timeout)) = start_time { - if start_time.elapsed() > timeout { - return Err(TestCaseError::fail(TEST_TIMEOUT)); - } + if timer.is_timed_out() { + return Err(TestCaseError::fail(TEST_TIMEOUT)); } let fuzz_res = self.single_fuzz(address, should_fail, calldata)?; @@ -288,8 +282,3 @@ impl FuzzedExecutor { } } } - -/// Starts timer for fuzz test, if any timeout configured. -pub(crate) fn start_timer(timeout: Option) -> Option<(Instant, Duration)> { - timeout.map(|timeout| (Instant::now(), Duration::from_secs(timeout.into()))) -} diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index bb5874f79010..d5fdb5668f5e 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -50,7 +50,7 @@ pub use result::InvariantFuzzTestResult; use serde::{Deserialize, Serialize}; mod shrink; -use crate::executors::{fuzz::start_timer, EvmError}; +use crate::executors::{EvmError, FuzzTestTimer}; pub use shrink::check_sequence; sol! { @@ -333,8 +333,8 @@ impl<'a> InvariantExecutor<'a> { let (invariant_test, invariant_strategy) = self.prepare_test(&invariant_contract, fuzz_fixtures)?; - // Start a timer if timeout is set. - let start_time = start_timer(self.config.timeout); + // Start timer for this invariant test. + let timer = FuzzTestTimer::new(self.config.timeout); let _ = self.runner.run(&invariant_strategy, |first_input| { // Create current invariant run data. @@ -352,14 +352,12 @@ impl<'a> InvariantExecutor<'a> { while current_run.depth < self.config.depth { // Check if the timeout has been reached. - if let Some((start_time, timeout)) = start_time { - if start_time.elapsed() > timeout { - // Since we never record a revert here the test is still considered - // successful even though it timed out. We *want* - // this behavior for now, so that's ok, but - // future developers should be aware of this. - return Err(TestCaseError::fail(TEST_TIMEOUT)); - } + if timer.is_timed_out() { + // Since we never record a revert here the test is still considered + // successful even though it timed out. We *want* + // this behavior for now, so that's ok, but + // future developers should be aware of this. + return Err(TestCaseError::fail(TEST_TIMEOUT)); } let tx = current_run.inputs.last().ok_or_else(|| { diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index b5b31b812298..2ccfad9e2a58 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -35,7 +35,10 @@ use revm::{ ResultAndState, SignedAuthorization, SpecId, TxEnv, TxKind, }, }; -use std::borrow::Cow; +use std::{ + borrow::Cow, + time::{Duration, Instant}, +}; mod builder; pub use builder::ExecutorBuilder; @@ -952,3 +955,20 @@ fn convert_executed_result( chisel_state, }) } + +/// Timer for a fuzz test. +pub struct FuzzTestTimer { + /// Inner fuzz test timer - (test start time, test duration). + inner: Option<(Instant, Duration)>, +} + +impl FuzzTestTimer { + pub fn new(timeout: Option) -> Self { + Self { inner: timeout.map(|timeout| (Instant::now(), Duration::from_secs(timeout.into()))) } + } + + /// Whether the current fuzz test timed out and should be stopped. + pub fn is_timed_out(&self) -> bool { + self.inner.is_some_and(|(start, duration)| start.elapsed() > duration) + } +}