From e1c91f543c66421d209b75a6a5315d597c240e71 Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Mon, 9 Sep 2024 19:20:05 +0200 Subject: [PATCH 1/6] test: add isolate mode JS integration test --- .../contracts/LastCallGasIsolated.t.sol | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol diff --git a/js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol b/js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol new file mode 100644 index 000000000..f2eb7359b --- /dev/null +++ b/js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity ^0.8.24; + +import "./test.sol"; +import "./Vm.sol"; + +contract Target { + uint256 public slot0; + + function expandMemory(uint256 n) public pure returns (uint256) { + uint256[] memory arr = new uint256[](n); + + for (uint256 i = 0; i < n; i++) { + arr[i] = i; + } + + return arr.length; + } + + function setValue(uint256 value) public { + slot0 = value; + } + + function resetValue() public { + slot0 = 0; + } + + fallback() external {} +} + +abstract contract LastCallGasFixture is DSTest { + Vm constant vm = Vm(HEVM_ADDRESS); + Target public target; + + struct Gas { + uint64 gasTotalUsed; + uint64 gasMemoryUsed; + int64 gasRefunded; + } + + function testRevertNoCachedLastCallGas() public { + vm.expectRevert(); + vm.lastCallGas(); + } + + function _setup() internal { + // Cannot be set in `setUp` due to `testRevertNoCachedLastCallGas` + // relying on no calls being made before `lastCallGas` is called. + target = new Target(); + } + + function _performCall() internal returns (bool success) { + (success,) = address(target).call(""); + } + + function _performExpandMemory() internal view { + target.expandMemory(1000); + } + + function _performRefund() internal { + target.setValue(1); + target.resetValue(); + } + + function _assertGas(Vm.Gas memory lhs, Gas memory rhs) internal { + assertGt(lhs.gasLimit, 0); + assertGt(lhs.gasRemaining, 0); + assertEq(lhs.gasTotalUsed, rhs.gasTotalUsed); + assertEq(lhs.gasMemoryUsed, rhs.gasMemoryUsed); + assertEq(lhs.gasRefunded, rhs.gasRefunded); + } +} + +// Test that `lastCallGas` works correctly in isolate mode. +contract LastCallGasIsolatedTest is LastCallGasFixture { + function testRecordLastCallGas() public { + _setup(); + _performCall(); + _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21065, gasMemoryUsed: 0, gasRefunded: 0})); + + _performCall(); + _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21065, gasMemoryUsed: 0, gasRefunded: 0})); + + _performCall(); + _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21065, gasMemoryUsed: 0, gasRefunded: 0})); + } + + function testRecordGasMemory() public { + _setup(); + _performExpandMemory(); + _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 129820, gasMemoryUsed: 4994, gasRefunded: 0})); + } + + function testRecordGasRefund() public { + _setup(); + _performRefund(); + _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21400, gasMemoryUsed: 0, gasRefunded: 4800})); + } +} From bdfc5720790e09252c45a0490745b27c0fbf15d9 Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Tue, 10 Sep 2024 12:59:37 +0200 Subject: [PATCH 2/6] Simplify isolate mode test and add inverse test Co-authored-by: Franco Victorio --- .../contracts/LastCallGasIsolated.t.sol | 99 ------------------- 1 file changed, 99 deletions(-) delete mode 100644 js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol diff --git a/js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol b/js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol deleted file mode 100644 index f2eb7359b..000000000 --- a/js/integration-tests/solidity-tests/contracts/LastCallGasIsolated.t.sol +++ /dev/null @@ -1,99 +0,0 @@ -// SPDX-License-Identifier: MIT OR Apache-2.0 -pragma solidity ^0.8.24; - -import "./test.sol"; -import "./Vm.sol"; - -contract Target { - uint256 public slot0; - - function expandMemory(uint256 n) public pure returns (uint256) { - uint256[] memory arr = new uint256[](n); - - for (uint256 i = 0; i < n; i++) { - arr[i] = i; - } - - return arr.length; - } - - function setValue(uint256 value) public { - slot0 = value; - } - - function resetValue() public { - slot0 = 0; - } - - fallback() external {} -} - -abstract contract LastCallGasFixture is DSTest { - Vm constant vm = Vm(HEVM_ADDRESS); - Target public target; - - struct Gas { - uint64 gasTotalUsed; - uint64 gasMemoryUsed; - int64 gasRefunded; - } - - function testRevertNoCachedLastCallGas() public { - vm.expectRevert(); - vm.lastCallGas(); - } - - function _setup() internal { - // Cannot be set in `setUp` due to `testRevertNoCachedLastCallGas` - // relying on no calls being made before `lastCallGas` is called. - target = new Target(); - } - - function _performCall() internal returns (bool success) { - (success,) = address(target).call(""); - } - - function _performExpandMemory() internal view { - target.expandMemory(1000); - } - - function _performRefund() internal { - target.setValue(1); - target.resetValue(); - } - - function _assertGas(Vm.Gas memory lhs, Gas memory rhs) internal { - assertGt(lhs.gasLimit, 0); - assertGt(lhs.gasRemaining, 0); - assertEq(lhs.gasTotalUsed, rhs.gasTotalUsed); - assertEq(lhs.gasMemoryUsed, rhs.gasMemoryUsed); - assertEq(lhs.gasRefunded, rhs.gasRefunded); - } -} - -// Test that `lastCallGas` works correctly in isolate mode. -contract LastCallGasIsolatedTest is LastCallGasFixture { - function testRecordLastCallGas() public { - _setup(); - _performCall(); - _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21065, gasMemoryUsed: 0, gasRefunded: 0})); - - _performCall(); - _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21065, gasMemoryUsed: 0, gasRefunded: 0})); - - _performCall(); - _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21065, gasMemoryUsed: 0, gasRefunded: 0})); - } - - function testRecordGasMemory() public { - _setup(); - _performExpandMemory(); - _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 129820, gasMemoryUsed: 4994, gasRefunded: 0})); - } - - function testRecordGasRefund() public { - _setup(); - _performRefund(); - _assertGas(vm.lastCallGas(), Gas({gasTotalUsed: 21400, gasMemoryUsed: 0, gasRefunded: 4800})); - } -} From 8a9bd9a1cfb3403f4557d2da3255b1804e80ec2a Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Tue, 10 Sep 2024 21:10:30 +0200 Subject: [PATCH 3/6] test: add JS fuzz/invariant integration tests --- crates/edr_napi/index.d.ts | 6 +- crates/edr_napi/src/solidity_tests/config.rs | 19 +- crates/foundry/config/src/fuzz.rs | 11 +- crates/foundry/config/src/invariant.rs | 14 +- crates/foundry/forge/src/lib.rs | 61 ++++-- crates/foundry/forge/src/runner.rs | 203 +++++++++++------- crates/foundry/forge/tests/it/fuzz.rs | 2 +- crates/foundry/forge/tests/it/test_helpers.rs | 4 +- .../contracts/ForkCheatcode.t.sol | 1 - .../solidity-tests/contracts/Invariant.t.sol | 39 ++++ .../solidity-tests/contracts/Overflow.t.sol | 28 +++ .../solidity-tests/package.json | 4 +- .../solidity-tests/test/fuzz.ts | 107 +++++++++ .../solidity-tests/test/testContext.ts | 18 -- pnpm-lock.yaml | 6 + 15 files changed, 389 insertions(+), 134 deletions(-) create mode 100644 js/integration-tests/solidity-tests/contracts/Invariant.t.sol create mode 100644 js/integration-tests/solidity-tests/contracts/Overflow.t.sol create mode 100644 js/integration-tests/solidity-tests/test/fuzz.ts diff --git a/crates/edr_napi/index.d.ts b/crates/edr_napi/index.d.ts index ef4cc51b3..806525216 100644 --- a/crates/edr_napi/index.d.ts +++ b/crates/edr_napi/index.d.ts @@ -515,8 +515,8 @@ export interface SolidityTestRunnerConfigArgs { } /** Fuzz testing configuration */ export interface FuzzConfigArgs { - /** Path where fuzz failures are recorded and replayed. */ - failurePersistDir: string + /** Path where fuzz failures are recorded and replayed if set. */ + failurePersistDir?: string /** Name of the file to record fuzz failures, defaults to `failures`. */ failurePersistFile?: string /** @@ -560,7 +560,7 @@ export interface FuzzConfigArgs { } /** Invariant testing configuration. */ export interface InvariantConfigArgs { - /** Path where invariant failures are recorded and replayed. */ + /** Path where invariant failures are recorded and replayed if set. */ failurePersistDir?: string /** * The number of runs that must execute for each invariant test group. diff --git a/crates/edr_napi/src/solidity_tests/config.rs b/crates/edr_napi/src/solidity_tests/config.rs index fd720bc10..7f7ebfd36 100644 --- a/crates/edr_napi/src/solidity_tests/config.rs +++ b/crates/edr_napi/src/solidity_tests/config.rs @@ -182,7 +182,8 @@ impl TryFrom for SolidityTestRunnerConfig { let invariant: InvariantConfig = fuzz .as_ref() - .map(|f| invariant.unwrap_or_default().defaults_from_fuzz(f)) + .map(|f| invariant.clone().unwrap_or_default().defaults_from_fuzz(f)) + .or(invariant) .map(TryFrom::try_from) .transpose()? .unwrap_or_default(); @@ -301,8 +302,8 @@ impl TryFrom for SolidityTestRunnerConfig { #[napi(object)] #[derive(Clone, Default, Debug)] pub struct FuzzConfigArgs { - /// Path where fuzz failures are recorded and replayed. - pub failure_persist_dir: String, + /// Path where fuzz failures are recorded and replayed if set. + pub failure_persist_dir: Option, /// Name of the file to record fuzz failures, defaults to `failures`. pub failure_persist_file: Option, /// The amount of fuzz runs to perform for each fuzz test case. Higher @@ -348,8 +349,8 @@ impl TryFrom for FuzzConfig { include_push_bytes, } = value; - let failure_persist_dir = Some(failure_persist_dir.into()); - let failure_persist_file = Some(failure_persist_file.unwrap_or("failures".to_string())); + let failure_persist_dir = failure_persist_dir.map(PathBuf::from); + let failure_persist_file = failure_persist_file.unwrap_or("failures".to_string()); let seed = seed .map(|s| { s.parse().map_err(|_err| { @@ -362,6 +363,8 @@ impl TryFrom for FuzzConfig { seed, failure_persist_dir, failure_persist_file, + // TODO https://github.com/NomicFoundation/edr/issues/657 + gas_report_samples: 0, ..FuzzConfig::default() }; @@ -393,7 +396,7 @@ impl TryFrom for FuzzConfig { #[napi(object)] #[derive(Clone, Default, Debug)] pub struct InvariantConfigArgs { - /// Path where invariant failures are recorded and replayed. + /// Path where invariant failures are recorded and replayed if set. pub failure_persist_dir: Option, /// The number of runs that must execute for each invariant test group. /// Defaults to 256. @@ -442,7 +445,7 @@ impl InvariantConfigArgs { } = fuzz; if self.failure_persist_dir.is_none() { - self.failure_persist_dir = Some(failure_persist_dir.clone()); + self.failure_persist_dir.clone_from(failure_persist_dir); } if self.runs.is_none() { @@ -483,6 +486,8 @@ impl From for InvariantConfig { let mut invariant = InvariantConfig { failure_persist_dir, + // TODO https://github.com/NomicFoundation/edr/issues/657 + gas_report_samples: 0, ..InvariantConfig::default() }; diff --git a/crates/foundry/config/src/fuzz.rs b/crates/foundry/config/src/fuzz.rs index 366a0e835..1f17717cc 100644 --- a/crates/foundry/config/src/fuzz.rs +++ b/crates/foundry/config/src/fuzz.rs @@ -24,7 +24,7 @@ pub struct FuzzConfig { /// 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, + pub failure_persist_file: String, } impl Default for FuzzConfig { @@ -36,7 +36,7 @@ impl Default for FuzzConfig { dictionary: FuzzDictionaryConfig::default(), gas_report_samples: 0, failure_persist_dir: None, - failure_persist_file: None, + failure_persist_file: "failures".into(), } } } @@ -46,13 +46,8 @@ impl FuzzConfig { /// `{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: 0, failure_persist_dir: Some(cache_dir), - failure_persist_file: Some("failures".to_string()), + ..FuzzConfig::default() } } } diff --git a/crates/foundry/config/src/invariant.rs b/crates/foundry/config/src/invariant.rs index 4ec907731..13806f9c6 100644 --- a/crates/foundry/config/src/invariant.rs +++ b/crates/foundry/config/src/invariant.rs @@ -69,10 +69,16 @@ impl InvariantConfig { } /// Returns path to failure dir of given invariant test contract. - pub fn failure_dir(self, contract_name: &str) -> PathBuf { + pub fn failure_dir(&self, contract_name: &str) -> Option { self.failure_persist_dir - .unwrap() - .join("failures") - .join(contract_name.split(':').last().unwrap()) + .as_ref() + .map(|failure_persist_dir| { + failure_persist_dir.join("failures").join( + contract_name + .split(':') + .last() + .expect("contract name should have solc version suffix"), + ) + }) } } diff --git a/crates/foundry/forge/src/lib.rs b/crates/foundry/forge/src/lib.rs index 935c7230f..5bd52a668 100644 --- a/crates/foundry/forge/src/lib.rs +++ b/crates/foundry/forge/src/lib.rs @@ -1,6 +1,12 @@ #[macro_use] extern crate tracing; +use std::{ + collections::HashSet, + fmt::Debug, + sync::{OnceLock, RwLock}, +}; + use foundry_config::{FuzzConfig, InvariantConfig}; use proptest::test_runner::{ FailurePersistence, FileFailurePersistence, RngAlgorithm, TestRng, TestRunner, @@ -25,6 +31,8 @@ pub mod result; pub use foundry_common::traits::TestFilter; pub use foundry_evm::*; +static FAILURE_PATHS: OnceLock>> = OnceLock::new(); + /// Metadata on how to run fuzz/invariant tests #[derive(Clone, Debug, Default)] pub struct TestOptions { @@ -38,19 +46,46 @@ impl TestOptions { /// Returns a "fuzz" test runner instance. pub fn fuzz_runner(&self) -> TestRunner { let fuzz_config = self.fuzz_config().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(), - ))), - ) + + if let Some(failure_persist_dir) = fuzz_config.failure_persist_dir { + let failure_persist_path = failure_persist_dir + .join(fuzz_config.failure_persist_file) + .into_os_string() + .into_string() + .expect("path should be valid UTF-8"); + + // HACK: We need to leak the path as + // `proptest::test_runner::FileFailurePersistence` requires a + // `&'static str`. We mitigate this by making sure that one particular path + // is only leaked once. + let failure_paths = FAILURE_PATHS.get_or_init(RwLock::default); + // Need to be in a block to ensure that the read lock is dropped before we try + // to insert. + { + let failure_paths_guard = failure_paths.read().expect("lock is not poisoned"); + if let Some(static_path) = failure_paths_guard.get(&*failure_persist_path) { + return self.fuzzer_with_cases( + fuzz_config.runs, + Some(Box::new(FileFailurePersistence::Direct(static_path))), + ); + } + } + // Write block + { + let mut failure_paths_guard = failure_paths.write().expect("lock is not poisoned"); + failure_paths_guard.insert(failure_persist_path.clone().leak()); + let static_path = failure_paths_guard + .get(&*failure_persist_path) + .expect("must exist since we just inserted it"); + + self.fuzzer_with_cases( + fuzz_config.runs, + Some(Box::new(FileFailurePersistence::Direct(static_path))), + ) + } + } else { + self.fuzzer_with_cases(fuzz_config.runs, None) + } } /// Returns an "invariant" test runner instance. diff --git a/crates/foundry/forge/src/runner.rs b/crates/foundry/forge/src/runner.rs index 8e7fecb82..392f66714 100644 --- a/crates/foundry/forge/src/runner.rs +++ b/crates/foundry/forge/src/runner.rs @@ -3,13 +3,14 @@ use std::{ borrow::Cow, cmp::min, collections::{BTreeMap, HashMap}, + path::Path, sync::Arc, time::Instant, }; use alloy_dyn_abi::DynSolValue; use alloy_json_abi::Function; -use alloy_primitives::{Address, U256}; +use alloy_primitives::{Address, Log, U256}; use eyre::Result; use foundry_common::{ contracts::{ContractsByAddress, ContractsByArtifact}, @@ -42,6 +43,7 @@ use crate::{ fuzz::{invariant::BasicTxDetails, BaseCounterExample}, multi_runner::{is_matching_test, TestContract}, result::{SuiteResult, TestKind, TestResult, TestSetup, TestStatus}, + traces::Traces, TestFilter, TestOptions, }; @@ -669,75 +671,23 @@ impl<'a> ContractRunner<'a> { 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); + let failure_file = failure_dir + .as_ref() + .map(|failure_dir| failure_dir.join(&invariant_contract.invariant_function.name)); - // Try to replay recorded failure if any. - 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| BasicTxDetails { - sender: seq.sender.unwrap_or_default(), - call_details: CallDetails { - target: seq.addr.unwrap_or_default(), - calldata: seq.calldata, - }, - }) - .collect::>(); - if let Ok((success, replayed_entirely)) = 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: 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, - counterexample: Some(CounterExample::Sequence(call_sequence)), - kind: TestKind::Invariant { - runs: 1, - calls: 1, - reverts: 1, - }, - duration: start.elapsed(), - ..Default::default() - }; - } + if let Some(failure_file) = failure_file.as_ref() { + if let Some(result) = self.try_to_replay_recorded_failures(ReplayArgs { + failure_file, + invariant_config: &invariant_config, + known_contracts, + identified_contracts, + invariant_contract: &invariant_contract, + logs: &mut logs, + traces: &mut traces, + coverage: &mut coverage, + start: &start, + }) { + return result; } } @@ -795,13 +745,18 @@ impl<'a> ContractRunner<'a> { Ok(call_sequence) => { if !call_sequence.is_empty() { // Persist error in invariant failure dir. - 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, - ) { - error!(%err, "Failed to record call sequence"); + if let Some(failure_dir) = failure_dir { + let failure_file = failure_file.expect("failure file "); + 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, + ) { + error!(%err, "Failed to record call sequence"); + } } counterexample = Some(CounterExample::Sequence(call_sequence)); } @@ -1000,6 +955,102 @@ impl<'a> ContractRunner<'a> { .collect(), } } + + fn try_to_replay_recorded_failures(&'a self, args: ReplayArgs<'a>) -> Option { + let ReplayArgs { + failure_file, + invariant_config, + known_contracts, + identified_contracts, + invariant_contract, + logs, + traces, + coverage, + start, + } = args; + + if let Ok(call_sequence) = + foundry_common::fs::read_json_file::>(failure_file) + { + // Create calls from failed sequence and check if invariant still broken. + let txes = call_sequence + .clone() + .into_iter() + .map(|seq| BasicTxDetails { + sender: seq.sender.unwrap_or_default(), + call_details: CallDetails { + target: seq.addr.unwrap_or_default(), + calldata: seq.calldata, + }, + }) + .collect::>(); + if let Ok((success, replayed_entirely)) = 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(), + logs, + traces, + coverage, + txes, + ); + return Some(TestResult { + status: TestStatus::Failure, + 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: traces.clone(), + coverage: coverage.clone(), + counterexample: Some(CounterExample::Sequence(call_sequence)), + kind: TestKind::Invariant { + runs: 1, + calls: 1, + reverts: 1, + }, + duration: start.elapsed(), + ..Default::default() + }); + } + } + } + None + } +} + +struct ReplayArgs<'a> { + failure_file: &'a Path, + invariant_config: &'a InvariantConfig, + known_contracts: &'a ContractsByArtifact, + identified_contracts: &'a ContractsByAddress, + invariant_contract: &'a InvariantContract<'a>, + logs: &'a mut Vec, + traces: &'a mut Traces, + coverage: &'a mut Option, + start: &'a Instant, } /// Utility function to merge coverage options diff --git a/crates/foundry/forge/tests/it/fuzz.rs b/crates/foundry/forge/tests/it/fuzz.rs index cfb2cdfc8..e061c05b6 100644 --- a/crates/foundry/forge/tests/it/fuzz.rs +++ b/crates/foundry/forge/tests/it/fuzz.rs @@ -170,7 +170,7 @@ async fn test_persist_fuzz_failure() { } // write new failure in different file, but keep the same directory - fuzz_config.failure_persist_file = Some("failure1".to_string()); + fuzz_config.failure_persist_file = "failure1".to_string(); let runner = TEST_DATA_DEFAULT.runner_with_fuzz_config(fuzz_config).await; let new_calldata = match get_failure_result!(runner) { Some(CounterExample::Single(counterexample)) => counterexample.calldata, diff --git a/crates/foundry/forge/tests/it/test_helpers.rs b/crates/foundry/forge/tests/it/test_helpers.rs index 870b46319..3696a0c69 100644 --- a/crates/foundry/forge/tests/it/test_helpers.rs +++ b/crates/foundry/forge/tests/it/test_helpers.rs @@ -132,7 +132,7 @@ pub struct TestFuzzConfig { pub dictionary: TestFuzzDictionaryConfig, pub gas_report_samples: u32, pub failure_persist_dir: Option, - pub failure_persist_file: Option, + pub failure_persist_file: String, } impl Default for TestFuzzConfig { @@ -144,7 +144,7 @@ impl Default for TestFuzzConfig { dictionary: TestFuzzDictionaryConfig::default(), gas_report_samples: 256, failure_persist_dir: Some(tempfile::tempdir().unwrap().into_path()), - failure_persist_file: Some("testfailure".to_string()), + failure_persist_file: "testfailure".into(), } } } diff --git a/js/integration-tests/solidity-tests/contracts/ForkCheatcode.t.sol b/js/integration-tests/solidity-tests/contracts/ForkCheatcode.t.sol index b219bfa13..643e74a6a 100644 --- a/js/integration-tests/solidity-tests/contracts/ForkCheatcode.t.sol +++ b/js/integration-tests/solidity-tests/contracts/ForkCheatcode.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; - import "./test.sol"; import "./Vm.sol"; diff --git a/js/integration-tests/solidity-tests/contracts/Invariant.t.sol b/js/integration-tests/solidity-tests/contracts/Invariant.t.sol new file mode 100644 index 000000000..6102cc28f --- /dev/null +++ b/js/integration-tests/solidity-tests/contracts/Invariant.t.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "./test.sol"; +import "./Vm.sol"; + +contract StochasticWrongContract { + uint256 public val1; + uint256 public val2; + uint256 public val3; + + function addToA(uint256 amount) external { + // This is an intentional bug in the contract to trigger invariant failure. + // If the conditional is removed, the invariant will pass. + if (amount % 13 != 0) { + val1 += amount; + } + val3 += amount; + } + + function addToB(uint256 amount) external { + val2 += amount; + val3 += amount; + } +} + +// Test that the invariant testing works correctly by catching a bug in the contract. +contract FailingInvariantTest is DSTest { + StochasticWrongContract val; + + function setUp() external { + val = new StochasticWrongContract(); + } + + function invariant() external { + assertEq(val.val1() + val.val2(), val.val3()); + } +} + diff --git a/js/integration-tests/solidity-tests/contracts/Overflow.t.sol b/js/integration-tests/solidity-tests/contracts/Overflow.t.sol new file mode 100644 index 000000000..910682a91 --- /dev/null +++ b/js/integration-tests/solidity-tests/contracts/Overflow.t.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "./test.sol"; +import "./Vm.sol"; + +// Contract to be tested with overflow vulnerability +contract MyContract { + function addWithOverflow(uint256 a, uint256 b) public pure returns (uint256) { + return a + b; + } +} + +// Test that the fuzzing catches overflows +contract OverflowTest is DSTest { + Vm constant vm = Vm(HEVM_ADDRESS); + + MyContract public myContract; + + function setUp() public { + myContract = new MyContract(); + } + + function testFuzzAddWithOverflow(uint256 a, uint256 b) public { + uint256 result = myContract.addWithOverflow(a, b); + assertEq(result, a + b); + } +} diff --git a/js/integration-tests/solidity-tests/package.json b/js/integration-tests/solidity-tests/package.json index 18a439d79..250a7b58e 100644 --- a/js/integration-tests/solidity-tests/package.json +++ b/js/integration-tests/solidity-tests/package.json @@ -11,10 +11,12 @@ "@types/mocha": ">=9.1.0", "@types/node": "^20.0.0", "chai": "^4.3.6", + "chai-as-promised": "^7.1.1", "hardhat": "2.22.10", "prettier": "^3.2.5", "ts-node": "^10.8.0", - "typescript": "~5.5.3" + "typescript": "~5.5.3", + "@types/chai-as-promised": "^7.1.8" }, "keywords": [], "license": "MIT", diff --git a/js/integration-tests/solidity-tests/test/fuzz.ts b/js/integration-tests/solidity-tests/test/fuzz.ts new file mode 100644 index 000000000..33921b298 --- /dev/null +++ b/js/integration-tests/solidity-tests/test/fuzz.ts @@ -0,0 +1,107 @@ +import chai, { assert, expect } from "chai"; +import chaiAsPromised from "chai-as-promised"; +import { TestContext } from "./testContext"; +import fs from "node:fs/promises"; +import { FuzzConfigArgs } from "@nomicfoundation/edr"; + +chai.use(chaiAsPromised); + +describe("Fuzz and invariant testing", function () { + let testContext: TestContext; + + before(async function () { + testContext = await TestContext.setup(); + }); + + it("FailingFuzz", async function () { + const failureDir = testContext.fuzzFailuresPersistDir; + + // Remove invariant config failure directory to make sure it's created fresh. + await fs.rm(failureDir, { + recursive: true, + force: true, + }); + + const result1 = await testContext.runTestsWithStats("OverflowTest"); + assert.equal(result1.failedTests, 1); + assert.equal(result1.totalTests, 1); + + // The fuzz failure directory should not be created if we don't set the directory + await assert.isRejected(fs.stat(failureDir)); + + const result2 = await testContext.runTestsWithStats("OverflowTest", { + fuzz: { + failurePersistDir: failureDir, + }, + }); + assert.equal(result2.failedTests, 1); + assert.equal(result2.totalTests, 1); + + // The fuzz failure directory should now be created + await assert.isFulfilled(fs.stat(failureDir)); + }); + + // One test as steps should be sequential + it("FailingInvariant", async function () { + const failureDir = testContext.invariantFailuresPersistDir; + const defaultConfig = { + depth: 15, + // This is false by default, we just specify it here to make it obvious to the reader. + failOnRevert: false, + }; + + // Remove invariant config failure directory to make sure it's created fresh. + await fs.rm(failureDir, { + recursive: true, + force: true, + }); + + const result1 = await testContext.runTestsWithStats( + "FailingInvariantTest", + { + invariant: defaultConfig, + } + ); + assert.equal(result1.failedTests, 1); + assert.equal(result1.totalTests, 1); + + // The invariant failure directory should not be created if we don't set the directory + await assert.isRejected(fs.stat(failureDir)); + + const firstStart = performance.now(); + const result2 = await testContext.runTestsWithStats( + "FailingInvariantTest", + { + invariant: { + ...defaultConfig, + failurePersistDir: failureDir, + }, + } + ); + const firstDuration = performance.now() - firstStart; + assert.equal(result2.failedTests, 1); + assert.equal(result2.totalTests, 1); + + // The invariant failure directory should now be created + await assert.isFulfilled(fs.stat(failureDir)); + + const secondStart = performance.now(); + const result3 = await testContext.runTestsWithStats( + "FailingInvariantTest", + { + invariant: { + ...defaultConfig, + failurePersistDir: failureDir, + }, + } + ); + assert.equal(result2.failedTests, 1); + assert.equal(result2.totalTests, 1); + const secondDuration = performance.now() - secondStart; + + // The invariant failure directory should still be there + await assert.isFulfilled(fs.stat(failureDir)); + // The second run should be faster than the first because it should use the persisted failure. + assert.isBelow(secondDuration, firstDuration * 0.25); + }); +}); diff --git a/js/integration-tests/solidity-tests/test/testContext.ts b/js/integration-tests/solidity-tests/test/testContext.ts index 2773751e3..ab42fe2a6 100644 --- a/js/integration-tests/solidity-tests/test/testContext.ts +++ b/js/integration-tests/solidity-tests/test/testContext.ts @@ -36,24 +36,6 @@ export class TestContext { }; } - fuzzConfig( - config?: Omit - ): FuzzConfigArgs { - return { - failurePersistDir: this.fuzzFailuresPersistDir, - ...config, - }; - } - - invariantConfig( - config?: Omit - ): InvariantConfigArgs { - return { - failurePersistDir: this.invariantFailuresPersistDir, - ...config, - }; - } - async runTestsWithStats( contractName: string, config?: Omit diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 36f4c6bec..8e44c6f61 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -424,6 +424,9 @@ importers: '@types/chai': specifier: ^4.2.0 version: 4.3.12 + '@types/chai-as-promised': + specifier: ^7.1.8 + version: 7.1.8 '@types/mocha': specifier: '>=9.1.0' version: 10.0.6 @@ -433,6 +436,9 @@ importers: chai: specifier: ^4.3.6 version: 4.4.1 + chai-as-promised: + specifier: ^7.1.1 + version: 7.1.1(chai@4.4.1) hardhat: specifier: 2.22.10 version: 2.22.10(ts-node@10.9.2(@types/node@20.16.1)(typescript@5.5.4))(typescript@5.5.4) From db7cb92ff835b0299189ba59ebd7951623a27b12 Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Wed, 11 Sep 2024 11:36:36 +0200 Subject: [PATCH 4/6] Fix linter --- js/integration-tests/solidity-tests/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/integration-tests/solidity-tests/package.json b/js/integration-tests/solidity-tests/package.json index 250a7b58e..e25fce221 100644 --- a/js/integration-tests/solidity-tests/package.json +++ b/js/integration-tests/solidity-tests/package.json @@ -8,6 +8,7 @@ "@nomicfoundation/edr-helpers": "workspace:*", "@tsconfig/node20": "^20.1.4", "@types/chai": "^4.2.0", + "@types/chai-as-promised": "^7.1.8", "@types/mocha": ">=9.1.0", "@types/node": "^20.0.0", "chai": "^4.3.6", @@ -15,8 +16,7 @@ "hardhat": "2.22.10", "prettier": "^3.2.5", "ts-node": "^10.8.0", - "typescript": "~5.5.3", - "@types/chai-as-promised": "^7.1.8" + "typescript": "~5.5.3" }, "keywords": [], "license": "MIT", From 7fe83157fbf35a267350a94199c93b41a49085b8 Mon Sep 17 00:00:00 2001 From: Franco Victorio Date: Thu, 12 Sep 2024 09:54:04 +0200 Subject: [PATCH 5/6] Remove chai-as-promised We can use fs.existsSync to check if a directory exists --- js/integration-tests/solidity-tests/package.json | 2 -- js/integration-tests/solidity-tests/test/fuzz.ts | 14 ++++++-------- pnpm-lock.yaml | 6 ------ 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/js/integration-tests/solidity-tests/package.json b/js/integration-tests/solidity-tests/package.json index e25fce221..18a439d79 100644 --- a/js/integration-tests/solidity-tests/package.json +++ b/js/integration-tests/solidity-tests/package.json @@ -8,11 +8,9 @@ "@nomicfoundation/edr-helpers": "workspace:*", "@tsconfig/node20": "^20.1.4", "@types/chai": "^4.2.0", - "@types/chai-as-promised": "^7.1.8", "@types/mocha": ">=9.1.0", "@types/node": "^20.0.0", "chai": "^4.3.6", - "chai-as-promised": "^7.1.1", "hardhat": "2.22.10", "prettier": "^3.2.5", "ts-node": "^10.8.0", diff --git a/js/integration-tests/solidity-tests/test/fuzz.ts b/js/integration-tests/solidity-tests/test/fuzz.ts index 33921b298..dcdf68a1d 100644 --- a/js/integration-tests/solidity-tests/test/fuzz.ts +++ b/js/integration-tests/solidity-tests/test/fuzz.ts @@ -1,11 +1,9 @@ import chai, { assert, expect } from "chai"; -import chaiAsPromised from "chai-as-promised"; import { TestContext } from "./testContext"; import fs from "node:fs/promises"; +import { existsSync } from "node:fs"; import { FuzzConfigArgs } from "@nomicfoundation/edr"; -chai.use(chaiAsPromised); - describe("Fuzz and invariant testing", function () { let testContext: TestContext; @@ -27,7 +25,7 @@ describe("Fuzz and invariant testing", function () { assert.equal(result1.totalTests, 1); // The fuzz failure directory should not be created if we don't set the directory - await assert.isRejected(fs.stat(failureDir)); + assert.isFalse(existsSync(failureDir)); const result2 = await testContext.runTestsWithStats("OverflowTest", { fuzz: { @@ -38,7 +36,7 @@ describe("Fuzz and invariant testing", function () { assert.equal(result2.totalTests, 1); // The fuzz failure directory should now be created - await assert.isFulfilled(fs.stat(failureDir)); + assert.isTrue(existsSync(failureDir)); }); // One test as steps should be sequential @@ -66,7 +64,7 @@ describe("Fuzz and invariant testing", function () { assert.equal(result1.totalTests, 1); // The invariant failure directory should not be created if we don't set the directory - await assert.isRejected(fs.stat(failureDir)); + assert.isFalse(existsSync(failureDir)); const firstStart = performance.now(); const result2 = await testContext.runTestsWithStats( @@ -83,7 +81,7 @@ describe("Fuzz and invariant testing", function () { assert.equal(result2.totalTests, 1); // The invariant failure directory should now be created - await assert.isFulfilled(fs.stat(failureDir)); + assert.isTrue(existsSync(failureDir)); const secondStart = performance.now(); const result3 = await testContext.runTestsWithStats( @@ -100,7 +98,7 @@ describe("Fuzz and invariant testing", function () { const secondDuration = performance.now() - secondStart; // The invariant failure directory should still be there - await assert.isFulfilled(fs.stat(failureDir)); + assert.isTrue(existsSync(failureDir)); // The second run should be faster than the first because it should use the persisted failure. assert.isBelow(secondDuration, firstDuration * 0.25); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8e44c6f61..36f4c6bec 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -424,9 +424,6 @@ importers: '@types/chai': specifier: ^4.2.0 version: 4.3.12 - '@types/chai-as-promised': - specifier: ^7.1.8 - version: 7.1.8 '@types/mocha': specifier: '>=9.1.0' version: 10.0.6 @@ -436,9 +433,6 @@ importers: chai: specifier: ^4.3.6 version: 4.4.1 - chai-as-promised: - specifier: ^7.1.1 - version: 7.1.1(chai@4.4.1) hardhat: specifier: 2.22.10 version: 2.22.10(ts-node@10.9.2(@types/node@20.16.1)(typescript@5.5.4))(typescript@5.5.4) From ab94d0b77756dbc8b1caa6fdadb7db918b2fe2f6 Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Thu, 12 Sep 2024 18:28:41 +0200 Subject: [PATCH 6/6] Address review feedback --- crates/foundry/forge/src/runner.rs | 3 +- .../solidity-tests/contracts/Invariant.t.sol | 20 ++++---- .../solidity-tests/test/fuzz.ts | 46 +++++++++++-------- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/crates/foundry/forge/src/runner.rs b/crates/foundry/forge/src/runner.rs index 392f66714..3f4e51468 100644 --- a/crates/foundry/forge/src/runner.rs +++ b/crates/foundry/forge/src/runner.rs @@ -746,7 +746,8 @@ impl<'a> ContractRunner<'a> { if !call_sequence.is_empty() { // Persist error in invariant failure dir. if let Some(failure_dir) = failure_dir { - let failure_file = failure_file.expect("failure file "); + let failure_file = failure_file + .expect("failure file must be some if failure_dir is some"); if let Err(err) = foundry_common::fs::create_dir_all(failure_dir) { diff --git a/js/integration-tests/solidity-tests/contracts/Invariant.t.sol b/js/integration-tests/solidity-tests/contracts/Invariant.t.sol index 6102cc28f..e2babc6c0 100644 --- a/js/integration-tests/solidity-tests/contracts/Invariant.t.sol +++ b/js/integration-tests/solidity-tests/contracts/Invariant.t.sol @@ -5,35 +5,35 @@ import "./test.sol"; import "./Vm.sol"; contract StochasticWrongContract { - uint256 public val1; - uint256 public val2; - uint256 public val3; + uint256 public a; + uint256 public b; + uint256 public both; function addToA(uint256 amount) external { // This is an intentional bug in the contract to trigger invariant failure. // If the conditional is removed, the invariant will pass. if (amount % 13 != 0) { - val1 += amount; + a += amount; } - val3 += amount; + both += amount; } function addToB(uint256 amount) external { - val2 += amount; - val3 += amount; + b += amount; + both += amount; } } // Test that the invariant testing works correctly by catching a bug in the contract. contract FailingInvariantTest is DSTest { - StochasticWrongContract val; + StochasticWrongContract wrongContract; function setUp() external { - val = new StochasticWrongContract(); + wrongContract = new StochasticWrongContract(); } function invariant() external { - assertEq(val.val1() + val.val2(), val.val3()); + assertEq(wrongContract.a() + wrongContract.b(), wrongContract.both()); } } diff --git a/js/integration-tests/solidity-tests/test/fuzz.ts b/js/integration-tests/solidity-tests/test/fuzz.ts index dcdf68a1d..d12bc88f5 100644 --- a/js/integration-tests/solidity-tests/test/fuzz.ts +++ b/js/integration-tests/solidity-tests/test/fuzz.ts @@ -2,7 +2,8 @@ import chai, { assert, expect } from "chai"; import { TestContext } from "./testContext"; import fs from "node:fs/promises"; import { existsSync } from "node:fs"; -import { FuzzConfigArgs } from "@nomicfoundation/edr"; +import { FuzzTestKind } from "@nomicfoundation/edr"; +import { runAllSolidityTests } from "@nomicfoundation/edr-helpers"; describe("Fuzz and invariant testing", function () { let testContext: TestContext; @@ -42,7 +43,8 @@ describe("Fuzz and invariant testing", function () { // One test as steps should be sequential it("FailingInvariant", async function () { const failureDir = testContext.invariantFailuresPersistDir; - const defaultConfig = { + const invariantConfig = { + runs: 256, depth: 15, // This is false by default, we just specify it here to make it obvious to the reader. failOnRevert: false, @@ -57,7 +59,7 @@ describe("Fuzz and invariant testing", function () { const result1 = await testContext.runTestsWithStats( "FailingInvariantTest", { - invariant: defaultConfig, + invariant: invariantConfig, } ); assert.equal(result1.failedTests, 1); @@ -66,40 +68,46 @@ describe("Fuzz and invariant testing", function () { // The invariant failure directory should not be created if we don't set the directory assert.isFalse(existsSync(failureDir)); - const firstStart = performance.now(); - const result2 = await testContext.runTestsWithStats( - "FailingInvariantTest", + const results2 = await runAllSolidityTests( + testContext.artifacts, + testContext.matchingTest("FailingInvariantTest"), { + ...testContext.defaultConfig(), invariant: { - ...defaultConfig, + ...invariantConfig, failurePersistDir: failureDir, }, } ); - const firstDuration = performance.now() - firstStart; - assert.equal(result2.failedTests, 1); - assert.equal(result2.totalTests, 1); + assert.equal(results2.length, 1); + assert.equal(results2[0].testResults.length, 1); + assert.equal(results2[0].testResults[0].status, "Failure"); + const fuzzTestResult2 = results2[0].testResults[0].kind as FuzzTestKind; + // More than one run should be needed on a fresh invariant test. + assert.isTrue(fuzzTestResult2.runs > 1n); // The invariant failure directory should now be created assert.isTrue(existsSync(failureDir)); - const secondStart = performance.now(); - const result3 = await testContext.runTestsWithStats( - "FailingInvariantTest", + const results3 = await runAllSolidityTests( + testContext.artifacts, + testContext.matchingTest("FailingInvariantTest"), { + ...testContext.defaultConfig(), invariant: { - ...defaultConfig, + ...invariantConfig, failurePersistDir: failureDir, }, } ); - assert.equal(result2.failedTests, 1); - assert.equal(result2.totalTests, 1); - const secondDuration = performance.now() - secondStart; // The invariant failure directory should still be there assert.isTrue(existsSync(failureDir)); - // The second run should be faster than the first because it should use the persisted failure. - assert.isBelow(secondDuration, firstDuration * 0.25); + assert.equal(results3.length, 1); + assert.equal(results3[0].testResults.length, 1); + assert.equal(results3[0].testResults[0].status, "Failure"); + const fuzzTestResult3 = results3[0].testResults[0].kind as FuzzTestKind; + // The second time only one run should be needed, because the persisted failure is used. + assert.equal(fuzzTestResult3.runs, 1n); }); });