diff --git a/evm/src/executor/inspector/fuzzer.rs b/evm/src/executor/inspector/fuzzer.rs index 5b254c246c99..93b112ee907e 100644 --- a/evm/src/executor/inspector/fuzzer.rs +++ b/evm/src/executor/inspector/fuzzer.rs @@ -82,12 +82,13 @@ impl Fuzzer { state.insert(utils::u256_to_h256_be(*slot).into()); } - for index in 0..interpreter.memory.len() / 32 { - let mut slot = [0u8; 32]; - slot.clone_from_slice(interpreter.memory.get_slice(index * 32, 32)); + // TODO: disabled for now since it's flooding the dictionary + // for index in 0..interpreter.memory.len() / 32 { + // let mut slot = [0u8; 32]; + // slot.clone_from_slice(interpreter.memory.get_slice(index * 32, 32)); - state.insert(slot); - } + // state.insert(slot); + // } } /// Overrides an external call and tries to call any method of msg.sender. diff --git a/evm/src/fuzz/invariant/error.rs b/evm/src/fuzz/invariant/error.rs index 5bca7c993490..b3df64e2fb4f 100644 --- a/evm/src/fuzz/invariant/error.rs +++ b/evm/src/fuzz/invariant/error.rs @@ -9,6 +9,7 @@ use crate::{ use ethers::{abi::Function, types::Address}; use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact}; use proptest::test_runner::TestError; +use tracing::trace; #[derive(Debug, Clone)] pub struct InvariantFuzzError { @@ -196,6 +197,7 @@ impl InvariantFuzzError { let mut anchor = 0; let mut removed_calls = vec![]; let mut shrinked = calls.iter().collect::<Vec<_>>(); + trace!(target: "forge::test", "Shrinking."); while anchor != calls.len() { // Get the latest removed element, so we know which one to remove next. diff --git a/evm/src/fuzz/invariant/executor.rs b/evm/src/fuzz/invariant/executor.rs index 845767eb0073..67f9f69b3ab3 100644 --- a/evm/src/fuzz/invariant/executor.rs +++ b/evm/src/fuzz/invariant/executor.rs @@ -23,6 +23,7 @@ use ethers::{ }; use eyre::ContextCompat; use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact}; +use hashbrown::HashMap; use parking_lot::{Mutex, RwLock}; use proptest::{ strategy::{BoxedStrategy, Strategy, ValueTree}, @@ -142,14 +143,10 @@ impl<'a> InvariantExecutor<'a> { .expect("could not make raw evm call"); // Collect data for fuzzing from the state changeset. - let state_changeset = + let mut state_changeset = call_result.state_changeset.to_owned().expect("to have a state changeset."); - collect_state_from_call( - &call_result.logs, - &state_changeset, - fuzz_state.clone(), - ); + collect_data(&mut state_changeset, sender, &call_result, fuzz_state.clone()); if let Err(error) = collect_created_contracts( &state_changeset, @@ -206,6 +203,8 @@ impl<'a> InvariantExecutor<'a> { }); } + tracing::trace!(target: "forge::test::invariant::dictionary", "{:?}", fuzz_state.read().iter().map(hex::encode).collect::<Vec<_>>()); + let (reverts, invariants) = failures.into_inner().into_inner(); Ok(Some(InvariantFuzzTestResult { invariants, cases: fuzz_cases.into_inner(), reverts })) @@ -479,6 +478,36 @@ impl<'a> InvariantExecutor<'a> { } } +/// Collects data from call for fuzzing. However, it first verifies that the sender is not an EOA +/// before inserting it into the dictionary. Otherwise, we flood the dictionary with +/// randomly generated addresses. +fn collect_data( + state_changeset: &mut HashMap<Address, revm::Account>, + sender: &Address, + call_result: &RawCallResult, + fuzz_state: EvmFuzzState, +) { + // Verify it has no code. + let mut has_code = false; + if let Some(Some(code)) = state_changeset.get(sender).map(|account| account.info.code.as_ref()) + { + has_code = !code.is_empty(); + } + + // We keep the nonce changes to apply later. + let mut sender_changeset = None; + if !has_code { + sender_changeset = state_changeset.remove(sender); + } + + collect_state_from_call(&call_result.logs, &*state_changeset, fuzz_state); + + // Re-add changes + if let Some(changed) = sender_changeset { + state_changeset.insert(*sender, changed); + } +} + /// Verifies that the invariant run execution can continue. fn can_continue( invariant_contract: &InvariantContract, diff --git a/evm/src/fuzz/mod.rs b/evm/src/fuzz/mod.rs index 6fcbda8e439e..b4c92304ed4e 100644 --- a/evm/src/fuzz/mod.rs +++ b/evm/src/fuzz/mod.rs @@ -124,6 +124,8 @@ impl<'a> FuzzedExecutor<'a> { } }); + tracing::trace!(target: "forge::test::fuzz::dictionary", "{:?}", state.read().iter().map(hex::encode).collect::<Vec<_>>()); + let (calldata, call) = counterexample.into_inner(); let mut result = FuzzTestResult { cases: FuzzedCases::new(cases.into_inner()), diff --git a/evm/src/fuzz/strategies/invariants.rs b/evm/src/fuzz/strategies/invariants.rs index a7216ee2e049..72d6d8f3f60d 100644 --- a/evm/src/fuzz/strategies/invariants.rs +++ b/evm/src/fuzz/strategies/invariants.rs @@ -13,6 +13,8 @@ use proptest::prelude::*; pub use proptest::test_runner::Config as FuzzConfig; use std::sync::Arc; +use super::fuzz_param_from_state; + /// Given a target address, we generate random calldata. pub fn override_call_strat( fuzz_state: EvmFuzzState, @@ -75,7 +77,7 @@ fn generate_call( let senders = senders.clone(); let fuzz_state = fuzz_state.clone(); func.prop_flat_map(move |func| { - let sender = select_random_sender(senders.clone()); + let sender = select_random_sender(fuzz_state.clone(), senders.clone()); (sender, fuzz_contract_with_calldata(fuzz_state.clone(), contract, func)) }) }) @@ -83,12 +85,28 @@ fn generate_call( } /// Strategy to select a sender address: -/// * If `senders` is empty, then it's a completely random address. +/// * If `senders` is empty, then it's either a random address (10%) or from the dictionary (90%). /// * If `senders` is not empty, then there's an 80% chance that one from the list is selected. The -/// remaining 20% will be random. -fn select_random_sender(senders: Vec<Address>) -> impl Strategy<Value = Address> { - let fuzz_strategy = - fuzz_param(&ParamType::Address).prop_map(move |addr| addr.into_address().unwrap()).boxed(); +/// remaining 20% will either be a random address (10%) or from the dictionary (90%). +fn select_random_sender( + fuzz_state: EvmFuzzState, + senders: Vec<Address>, +) -> impl Strategy<Value = Address> { + let fuzz_strategy = proptest::strategy::Union::new_weighted(vec![ + ( + 10, + fuzz_param(&ParamType::Address) + .prop_map(move |addr| addr.into_address().unwrap()) + .boxed(), + ), + ( + 90, + fuzz_param_from_state(&ParamType::Address, fuzz_state) + .prop_map(move |addr| addr.into_address().unwrap()) + .boxed(), + ), + ]) + .boxed(); if !senders.is_empty() { let selector = diff --git a/evm/src/fuzz/strategies/param.rs b/evm/src/fuzz/strategies/param.rs index e1e044156402..5125edf41b91 100644 --- a/evm/src/fuzz/strategies/param.rs +++ b/evm/src/fuzz/strategies/param.rs @@ -99,7 +99,11 @@ pub fn fuzz_param_from_state(param: &ParamType, arc_state: EvmFuzzState) -> Boxe }, ParamType::Bool => value.prop_map(move |value| Token::Bool(value[31] == 1)).boxed(), ParamType::String => value - .prop_map(move |value| Token::String(String::from_utf8_lossy(&value[..]).to_string())) + .prop_map(move |value| { + Token::String( + String::from_utf8_lossy(&value[..]).trim().trim_end_matches('\0').to_string(), + ) + }) .boxed(), ParamType::Array(param) => proptest::collection::vec( fuzz_param_from_state(param, arc_state.clone()), @@ -128,7 +132,10 @@ pub fn fuzz_param_from_state(param: &ParamType, arc_state: EvmFuzzState) -> Boxe #[cfg(test)] mod tests { - use crate::fuzz::strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state}; + use crate::{ + fuzz::strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state}, + CALLER, + }; use ethers::abi::HumanReadableParser; use revm::db::{CacheDB, EmptyDB}; diff --git a/evm/src/fuzz/strategies/state.rs b/evm/src/fuzz/strategies/state.rs index 6b3413a39990..fc22b1cd75fc 100644 --- a/evm/src/fuzz/strategies/state.rs +++ b/evm/src/fuzz/strategies/state.rs @@ -84,17 +84,13 @@ pub fn build_initial_state<DB: DatabaseRef>(db: &CacheDB<DB>) -> EvmFuzzState { let mut state = FuzzDictionary::default(); for (address, account) in db.accounts.iter() { - let info = db.basic(*address); - // Insert basic account information state.insert(H256::from(*address).into()); - state.insert(utils::u256_to_h256_le(info.balance).into()); - state.insert(utils::u256_to_h256_le(U256::from(info.nonce)).into()); // Insert storage for (slot, value) in &account.storage { - state.insert(utils::u256_to_h256_le(*slot).into()); - state.insert(utils::u256_to_h256_le(*value).into()); + state.insert(utils::u256_to_h256_be(*slot).into()); + state.insert(utils::u256_to_h256_be(*value).into()); } } @@ -119,13 +115,11 @@ pub fn collect_state_from_call( for (address, account) in state_changeset { // Insert basic account information state.insert(H256::from(*address).into()); - state.insert(utils::u256_to_h256_le(account.info.balance).into()); - state.insert(utils::u256_to_h256_le(U256::from(account.info.nonce)).into()); // Insert storage for (slot, value) in &account.storage { - state.insert(utils::u256_to_h256_le(*slot).into()); - state.insert(utils::u256_to_h256_le(*value).into()); + state.insert(utils::u256_to_h256_be(*slot).into()); + state.insert(utils::u256_to_h256_be(*value).into()); } // Insert push bytes @@ -183,11 +177,8 @@ fn collect_push_bytes(code: Bytes) -> Vec<[u8; 32]> { return bytes } - let mut buffer: [u8; 32] = [0; 32]; - let _ = (&mut buffer[..]) - .write(&code[push_start..push_end]) - .expect("push was larger than 32 bytes"); - bytes.push(buffer); + bytes.push(U256::from_big_endian(&code[push_start..push_end]).into()); + i += push_size; } i += 1; diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 2241e7cf4276..ec47b12051d9 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -51,11 +51,19 @@ pub struct TestOptions { } impl TestOptions { + pub fn invariant_fuzzer(&self) -> TestRunner { + self.fuzzer_with_cases(self.invariant_runs) + } + pub fn fuzzer(&self) -> TestRunner { + self.fuzzer_with_cases(self.fuzz_runs) + } + + pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner { // TODO: Add Options to modify the persistence let cfg = proptest::test_runner::Config { failure_persistence: None, - cases: self.fuzz_runs, + cases, max_local_rejects: self.fuzz_max_local_rejects, max_global_rejects: self.fuzz_max_global_rejects, ..Default::default() diff --git a/forge/src/runner.rs b/forge/src/runner.rs index c63ab9945b14..9478cd6ebaab 100644 --- a/forge/src/runner.rs +++ b/forge/src/runner.rs @@ -302,7 +302,7 @@ impl<'a> ContractRunner<'a> { .collect(); let results = self.run_invariant_test( - test_options.fuzzer(), + test_options.invariant_fuzzer(), setup, test_options, functions.clone(), diff --git a/forge/tests/it/fuzz.rs b/forge/tests/it/fuzz.rs index 89fe39307746..5a39d179f3e8 100644 --- a/forge/tests/it/fuzz.rs +++ b/forge/tests/it/fuzz.rs @@ -1,16 +1,24 @@ //! Tests for invariants use crate::{config::*, test_helpers::filter::Filter}; +use ethers::types::U256; use forge::result::SuiteResult; - use foundry_evm::decode::decode_console_logs; +use std::collections::BTreeMap; #[test] fn test_fuzz() { let mut runner = runner(); - let suite_result = - runner.test(&Filter::new(".*", ".*", ".*fuzz/[^invariant]"), None, TEST_OPTS).unwrap(); + let suite_result = runner + .test( + &Filter::new(".*", ".*", ".*fuzz/[^invariant]").exclude_tests( + r#"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)"#, + ), + None, + TEST_OPTS, + ) + .unwrap(); assert!(!suite_result.is_empty()); @@ -40,3 +48,39 @@ fn test_fuzz() { } } } + +/// Test that showcases PUSH collection on normal fuzzing. Ignored until we collect them in a +/// smarter way. +#[test] +#[ignore] +fn test_fuzz_collection() { + let mut runner = runner(); + + let mut opts = TEST_OPTS; + opts.invariant_depth = 100; + opts.invariant_runs = 1000; + opts.fuzz_runs = 1000; + opts.fuzz_seed = Some(U256::from(6u32)); + runner.test_options = opts; + + let results = + runner.test(&Filter::new(".*", ".*", ".*fuzz/FuzzCollection.t.sol"), None, opts).unwrap(); + + assert_multiple( + &results, + BTreeMap::from([( + "fuzz/FuzzCollection.t.sol:SampleContractTest", + vec![ + ("invariantCounter", false, Some("broken counter.".into()), None, None), + ( + "testIncrement(address)", + false, + Some("Call did not revert as expected".into()), + None, + None, + ), + ("testNeedle(uint256)", false, Some("needle found.".into()), None, None), + ], + )]), + ); +} diff --git a/forge/tests/it/invariant.rs b/forge/tests/it/invariant.rs index 01535ef6a6be..eac430d1cbef 100644 --- a/forge/tests/it/invariant.rs +++ b/forge/tests/it/invariant.rs @@ -9,22 +9,27 @@ use std::collections::BTreeMap; fn test_invariant() { let mut runner = runner(); - let results = - runner.test(&Filter::new(".*", ".*", ".*fuzz/invariant/"), None, TEST_OPTS).unwrap(); + let results = runner + .test( + &Filter::new(".*", ".*", ".*fuzz/invariant/(target|targetAbi|common)"), + None, + TEST_OPTS, + ) + .unwrap(); assert_multiple( &results, BTreeMap::from([ ( - "fuzz/invariant/InvariantInnerContract.t.sol:InvariantInnerContract", + "fuzz/invariant/common/InvariantInnerContract.t.sol:InvariantInnerContract", vec![("invariantHideJesus", false, Some("jesus betrayed.".into()), None, None)], ), ( - "fuzz/invariant/InvariantReentrancy.t.sol:InvariantReentrancy", + "fuzz/invariant/common/InvariantReentrancy.t.sol:InvariantReentrancy", vec![("invariantNotStolen", true, None, None, None)], ), ( - "fuzz/invariant/InvariantTest1.t.sol:InvariantTest", + "fuzz/invariant/common/InvariantTest1.t.sol:InvariantTest", vec![("invariant_neverFalse", false, Some("false.".into()), None, None)], ), ( @@ -75,28 +80,67 @@ fn test_invariant_override() { runner.test_options = opts; let results = runner - .test(&Filter::new(".*", ".*", ".*fuzz/invariant/InvariantReentrancy.t.sol"), None, opts) + .test( + &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantReentrancy.t.sol"), + None, + opts, + ) .unwrap(); assert_multiple( &results, BTreeMap::from([( - "fuzz/invariant/InvariantReentrancy.t.sol:InvariantReentrancy", + "fuzz/invariant/common/InvariantReentrancy.t.sol:InvariantReentrancy", vec![("invariantNotStolen", false, Some("stolen.".into()), None, None)], )]), ); } +#[test] +fn test_invariant_storage() { + let mut runner = runner(); + + let mut opts = TEST_OPTS; + opts.invariant_depth = 100; + opts.fuzz_seed = Some(U256::from(6u32)); + runner.test_options = opts; + + let results = runner + .test( + &Filter::new(".*", ".*", ".*fuzz/invariant/storage/InvariantStorageTest.t.sol"), + None, + opts, + ) + .unwrap(); + + assert_multiple( + &results, + BTreeMap::from([( + "fuzz/invariant/storage/InvariantStorageTest.t.sol:InvariantStorageTest", + vec![ + ("invariantChangeAddress", false, Some("changedAddr".into()), None, None), + ("invariantChangeString", false, Some("changedStr".into()), None, None), + ("invariantChangeUint", false, Some("changedUint".into()), None, None), + ("invariantPush", false, Some("pushUint".into()), None, None), + ], + )]), + ); +} + #[test] fn test_invariant_shrink() { let mut runner = runner(); let mut opts = TEST_OPTS; - opts.fuzz_seed = Some(U256::from(100u32)); + opts.fuzz_seed = Some(U256::from(102u32)); runner.test_options = opts; let results = runner - .test(&Filter::new(".*", ".*", ".*fuzz/invariant/InvariantInnerContract.t.sol"), None, opts) + .test( + &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantInnerContract.t.sol"), + None, + opts, + ) .unwrap(); let results = diff --git a/testdata/fuzz/FuzzCollection.t.sol b/testdata/fuzz/FuzzCollection.t.sol new file mode 100644 index 000000000000..8f76df232e62 --- /dev/null +++ b/testdata/fuzz/FuzzCollection.t.sol @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import "ds-test/test.sol"; + +contract SampleContract { + uint256 public counter; + uint256 public counterX2; + address public owner = address(0xBEEF); + bool public found_needle; + + event Incremented(uint256 counter); + + modifier onlyOwner() { + require(msg.sender == owner, "ONLY_OWNER"); + _; + } + + function compare(uint256 val) public { + if (val == 0x4446) { + found_needle = true; + } + } + + function incrementBy(uint256 numToIncrement) public onlyOwner { + counter += numToIncrement; + counterX2 += numToIncrement * 2; + + emit Incremented(counter); + } + + function breakTheInvariant(uint256 x) public { + if (x == 0x5556 ) counterX2 = 0; + } +} + +interface Cheats { + function startPrank(address) external; + function expectRevert(bytes calldata msg) external; +} + +contract SampleContractTest is DSTest { + Cheats hevm = Cheats(HEVM_ADDRESS); + + event Incremented(uint256 counter); + + SampleContract public sample; + + function setUp() public { + sample = new SampleContract(); + } + + function testIncrement(address caller) public { + hevm.startPrank(address(caller)); + + hevm.expectRevert("ONLY_OWNER"); + sample.incrementBy(1); + } + + function testNeedle(uint256 needle) public { + sample.compare(needle); + require(!sample.found_needle(), "needle found."); + } + + function invariantCounter() public { + require(sample.counter() * 2 == sample.counterX2(), "broken counter."); + } +} diff --git a/testdata/fuzz/invariant/InvariantInnerContract.t.sol b/testdata/fuzz/invariant/common/InvariantInnerContract.t.sol similarity index 100% rename from testdata/fuzz/invariant/InvariantInnerContract.t.sol rename to testdata/fuzz/invariant/common/InvariantInnerContract.t.sol diff --git a/testdata/fuzz/invariant/InvariantReentrancy.t.sol b/testdata/fuzz/invariant/common/InvariantReentrancy.t.sol similarity index 100% rename from testdata/fuzz/invariant/InvariantReentrancy.t.sol rename to testdata/fuzz/invariant/common/InvariantReentrancy.t.sol diff --git a/testdata/fuzz/invariant/InvariantTest1.t.sol b/testdata/fuzz/invariant/common/InvariantTest1.t.sol similarity index 100% rename from testdata/fuzz/invariant/InvariantTest1.t.sol rename to testdata/fuzz/invariant/common/InvariantTest1.t.sol diff --git a/testdata/fuzz/invariant/storage/InvariantStorageTest.t.sol b/testdata/fuzz/invariant/storage/InvariantStorageTest.t.sol new file mode 100644 index 000000000000..8566e5cc8b47 --- /dev/null +++ b/testdata/fuzz/invariant/storage/InvariantStorageTest.t.sol @@ -0,0 +1,61 @@ +pragma solidity >0.8.13; + +import "ds-test/test.sol"; + +contract Contract { + address public addr = address(0xbeef); + string public str = "hello"; + uint256 public num = 1337; + uint256 public pushNum; + + function changeAddress(address _addr) public { + if(_addr == addr) { + addr = address(0); + } + } + + function changeString(string memory _str) public { + if(keccak256(bytes(_str)) == keccak256(bytes(str))) { + str = ""; + } + } + + function changeUint(uint256 _num) public { + if(_num == num) { + num = 0; + } + } + + function push(uint256 _num) public { + if(_num == 68) { + pushNum = 69; + } + } + +} + + +contract InvariantStorageTest is DSTest { + Contract c; + + function setUp() public { + c = new Contract(); + } + + function invariantChangeAddress() public { + require(c.addr() == address(0xbeef), "changedAddr"); + } + + function invariantChangeString() public { + require(keccak256(bytes(c.str())) == keccak256(bytes("hello")), "changedStr"); + } + + function invariantChangeUint() public { + require(c.num() == 1337, "changedUint"); + } + + function invariantPush() public { + require(c.pushNum() == 0, "pushUint"); + } + +} \ No newline at end of file