From ef15bab4ca676a3ff45f6016c4581964f6bd4e4f Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 14 Mar 2024 15:38:07 +0200 Subject: [PATCH 01/22] fix(fuzz): deduplicate fuzz inputs --- crates/common/src/traits.rs | 15 ++++ crates/config/src/fuzz.rs | 5 -- crates/evm/evm/src/executors/fuzz/mod.rs | 5 +- crates/evm/evm/src/executors/invariant/mod.rs | 30 +++---- crates/evm/fuzz/src/lib.rs | 19 +++++ crates/evm/fuzz/src/strategies/address.rs | 55 +++++++++++++ crates/evm/fuzz/src/strategies/calldata.rs | 82 +++---------------- crates/evm/fuzz/src/strategies/int.rs | 41 ++++++++-- crates/evm/fuzz/src/strategies/invariants.rs | 28 +++---- crates/evm/fuzz/src/strategies/mod.rs | 7 +- crates/evm/fuzz/src/strategies/param.rs | 59 +++++++------ crates/evm/fuzz/src/strategies/uint.rs | 53 +++++++++--- crates/forge/bin/cmd/test/mod.rs | 6 +- crates/forge/src/runner.rs | 46 ++++++++++- crates/forge/tests/it/invariant.rs | 18 +--- crates/forge/tests/it/test_helpers.rs | 2 - .../common/InvariantCalldataDictionary.t.sol | 11 +++ 17 files changed, 292 insertions(+), 190 deletions(-) create mode 100644 crates/evm/fuzz/src/strategies/address.rs diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index 4232fb946dc3..0e7cf0dece31 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -33,6 +33,9 @@ pub trait TestFunctionExt { /// Returns whether this function is a `setUp` function. fn is_setup(&self) -> bool; + + /// Returns whether this function is a `fixtures` function. + fn is_fixtures(&self) -> bool; } impl TestFunctionExt for Function { @@ -56,6 +59,10 @@ impl TestFunctionExt for Function { fn is_setup(&self) -> bool { self.name.is_setup() } + + fn is_fixtures(&self) -> bool { + self.name.is_fixtures() + } } impl TestFunctionExt for String { @@ -78,6 +85,10 @@ impl TestFunctionExt for String { fn is_setup(&self) -> bool { self.as_str().is_setup() } + + fn is_fixtures(&self) -> bool { + self.as_str().is_fixtures() + } } impl TestFunctionExt for str { @@ -100,6 +111,10 @@ impl TestFunctionExt for str { fn is_setup(&self) -> bool { self.eq_ignore_ascii_case("setup") } + + fn is_fixtures(&self) -> bool { + self.starts_with("fixtures") + } } /// An extension trait for `std::error::Error` for ABI encoding. diff --git a/crates/config/src/fuzz.rs b/crates/config/src/fuzz.rs index 3b0e13bcd509..7049a401a5f3 100644 --- a/crates/config/src/fuzz.rs +++ b/crates/config/src/fuzz.rs @@ -111,10 +111,6 @@ pub struct FuzzDictionaryConfig { /// Once the fuzzer exceeds this limit, it will start evicting random entries #[serde(deserialize_with = "crate::deserialize_usize_or_max")] pub max_fuzz_dictionary_values: usize, - /// How many random addresses to use and to recycle when fuzzing calldata. - /// If not specified then `max_fuzz_dictionary_addresses` value applies. - #[serde(deserialize_with = "crate::deserialize_usize_or_max")] - pub max_calldata_fuzz_dictionary_addresses: usize, } impl Default for FuzzDictionaryConfig { @@ -127,7 +123,6 @@ impl Default for FuzzDictionaryConfig { max_fuzz_dictionary_addresses: (300 * 1024 * 1024) / 20, // limit this to 200MB max_fuzz_dictionary_values: (200 * 1024 * 1024) / 32, - max_calldata_fuzz_dictionary_addresses: 0, } } } diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index ec980c01ae3c..6bd16896bff5 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -14,7 +14,7 @@ use foundry_evm_fuzz::{ build_initial_state, collect_state_from_call, fuzz_calldata, fuzz_calldata_from_state, EvmFuzzState, }, - BaseCounterExample, CounterExample, FuzzCase, FuzzError, FuzzTestResult, + BaseCounterExample, CounterExample, FuzzCase, FuzzError, FuzzFixtures, FuzzTestResult, }; use foundry_evm_traces::CallTraceArena; use proptest::test_runner::{TestCaseError, TestError, TestRunner}; @@ -61,6 +61,7 @@ impl FuzzedExecutor { address: Address, should_fail: bool, rd: &RevertDecoder, + fuzz_fixtures: FuzzFixtures, ) -> FuzzTestResult { // Stores the first Fuzzcase let first_case: RefCell> = RefCell::default(); @@ -85,7 +86,7 @@ impl FuzzedExecutor { let mut weights = vec![]; let dictionary_weight = self.config.dictionary.dictionary_weight.min(100); if self.config.dictionary.dictionary_weight < 100 { - weights.push((100 - dictionary_weight, fuzz_calldata(func.clone()))); + weights.push((100 - dictionary_weight, fuzz_calldata(func.clone(), fuzz_fixtures))); } if dictionary_weight > 0 { weights.push(( diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index 591fa7cf444d..2cb174e79cae 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -21,12 +21,12 @@ use foundry_evm_fuzz::{ build_initial_state, collect_created_contracts, collect_state_from_call, invariant_strat, override_call_strat, EvmFuzzState, }, - FuzzCase, FuzzedCases, + FuzzCase, FuzzFixtures, FuzzedCases, }; use foundry_evm_traces::CallTraceArena; use parking_lot::{Mutex, RwLock}; use proptest::{ - strategy::{BoxedStrategy, Strategy, ValueTree}, + strategy::{BoxedStrategy, Strategy}, test_runner::{TestCaseError, TestRunner}, }; use revm::{primitives::HashMap, DatabaseCommit}; @@ -34,7 +34,6 @@ use std::{cell::RefCell, collections::BTreeMap, sync::Arc}; mod error; pub use error::{InvariantFailures, InvariantFuzzError, InvariantFuzzTestResult}; -use foundry_evm_fuzz::strategies::CalldataFuzzDictionary; mod funcs; pub use funcs::{assert_invariants, replay_run}; @@ -42,12 +41,8 @@ pub use funcs::{assert_invariants, replay_run}; use self::error::FailedInvariantCaseData; /// Alias for (Dictionary for fuzzing, initial contracts to fuzz and an InvariantStrategy). -type InvariantPreparation = ( - EvmFuzzState, - FuzzRunIdentifiedContracts, - BoxedStrategy, - CalldataFuzzDictionary, -); +type InvariantPreparation = + (EvmFuzzState, FuzzRunIdentifiedContracts, BoxedStrategy); /// Enriched results of an invariant run check. /// @@ -106,14 +101,15 @@ impl<'a> InvariantExecutor<'a> { pub fn invariant_fuzz( &mut self, invariant_contract: InvariantContract<'_>, + fixtures: FuzzFixtures, ) -> Result { // Throw an error to abort test run if the invariant function accepts input params if !invariant_contract.invariant_function.inputs.is_empty() { return Err(eyre!("Invariant test function should have no inputs")) } - let (fuzz_state, targeted_contracts, strat, calldata_fuzz_dictionary) = - self.prepare_fuzzing(&invariant_contract)?; + let (fuzz_state, targeted_contracts, strat) = + self.prepare_fuzzing(&invariant_contract, fixtures.clone())?; // Stores the consumed gas and calldata of every successful fuzz call. let fuzz_cases: RefCell> = RefCell::new(Default::default()); @@ -281,7 +277,7 @@ impl<'a> InvariantExecutor<'a> { Ok(()) }); - trace!(target: "forge::test::invariant::calldata_address_fuzz_dictionary", "{:?}", calldata_fuzz_dictionary.inner.addresses); + trace!(target: "forge::test::invariant::fixtures", "{:?}", fixtures); trace!(target: "forge::test::invariant::dictionary", "{:?}", fuzz_state.read().values().iter().map(hex::encode).collect::>()); let (reverts, error) = failures.into_inner().into_inner(); @@ -302,6 +298,7 @@ impl<'a> InvariantExecutor<'a> { fn prepare_fuzzing( &mut self, invariant_contract: &InvariantContract<'_>, + fuzz_fixtures: FuzzFixtures, ) -> eyre::Result { // Finds out the chosen deployed contracts and/or senders. self.select_contract_artifacts(invariant_contract.address, invariant_contract.abi)?; @@ -321,16 +318,13 @@ impl<'a> InvariantExecutor<'a> { let targeted_contracts: FuzzRunIdentifiedContracts = Arc::new(Mutex::new(targeted_contracts)); - let calldata_fuzz_config = - CalldataFuzzDictionary::new(&self.config.dictionary, fuzz_state.clone()); - // Creates the invariant strategy. let strat = invariant_strat( fuzz_state.clone(), targeted_senders, targeted_contracts.clone(), self.config.dictionary.dictionary_weight, - calldata_fuzz_config.clone(), + fuzz_fixtures.clone(), ) .no_shrink() .boxed(); @@ -348,7 +342,7 @@ impl<'a> InvariantExecutor<'a> { fuzz_state.clone(), targeted_contracts.clone(), target_contract_ref.clone(), - calldata_fuzz_config.clone(), + fuzz_fixtures.clone(), ), target_contract_ref, )); @@ -357,7 +351,7 @@ impl<'a> InvariantExecutor<'a> { self.executor.inspector.fuzzer = Some(Fuzzer { call_generator, fuzz_state: fuzz_state.clone(), collect: true }); - Ok((fuzz_state, targeted_contracts, strat, calldata_fuzz_config)) + Ok((fuzz_state, targeted_contracts, strat)) } /// Fills the `InvariantExecutor` with the artifact identifier filters (in `path:name` string diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index f8928ea3e13f..b68ff4d56b23 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -272,3 +272,22 @@ impl FuzzedCases { self.lowest().map(|c| c.gas).unwrap_or_default() } } + +#[derive(Clone, Default, Debug)] +pub struct FuzzFixtures { + inner: HashMap, +} + +impl FuzzFixtures { + pub fn new(fixtures: HashMap) -> FuzzFixtures { + Self { inner: fixtures } + } + + pub fn param_fixtures(&self, param_name: &String) -> Option<&[DynSolValue]> { + if let Some(param_fixtures) = self.inner.get(param_name) { + param_fixtures.as_array() + } else { + None + } + } +} diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs new file mode 100644 index 000000000000..dae56f6adf22 --- /dev/null +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -0,0 +1,55 @@ +use alloy_dyn_abi::DynSolValue; +use alloy_primitives::Address; +use proptest::{ + arbitrary::any, + prelude::{prop, BoxedStrategy}, + strategy::Strategy, +}; + +/// The address strategy combines 2 different strategies: +/// 1. A random addresses strategy if no fixture defined for current parameter. +/// 2. A fixture based strategy if configured values for current parameters. +/// If fixture is not a valid type then an error is raised and test suite will continue to execute +/// with random address. +/// +/// +/// For example: +/// To define fixtures for `owner` fuzzed parameter, return an array of possible values from +/// `function fixtures_owner() public returns (address[] memory)`. +/// Use `owner` named parameter in fuzzed test in order to create a custom strategy +/// `function testFuzz_ownerAddress(address owner, uint amount)`. +#[derive(Debug)] +pub struct AddressStrategy {} + +impl AddressStrategy { + /// Create a new address strategy. + pub fn address_strategy(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { + if let Some(fixtures) = fixtures { + let address_fixtures: Vec = + fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); + let address_fixtures_len = address_fixtures.len(); + any::() + .prop_map(move |index| index.index(address_fixtures_len)) + .prop_map(move |index| { + // Generate value tree from fixture. + // If fixture is not a valid address, raise error and generate random value. + if let Some(addr_fixture) = address_fixtures.get(index) { + if let Some(addr_fixture) = addr_fixture.as_address() { + return DynSolValue::Address(addr_fixture); + } + } + error!( + "{:?} is not a valid address fixture, generate random value", + address_fixtures.get(index) + ); + DynSolValue::Address(Address::random()) + }) + .boxed() + } else { + // If no addresses configured in dictionary then create unbounded addresses strategy. + any::<[u8; 32]>() + .prop_map(|x| DynSolValue::Address(Address::from_word(x.into()))) + .boxed() + } + } +} diff --git a/crates/evm/fuzz/src/strategies/calldata.rs b/crates/evm/fuzz/src/strategies/calldata.rs index cb56f3330b0d..f5c2c1a05b26 100644 --- a/crates/evm/fuzz/src/strategies/calldata.rs +++ b/crates/evm/fuzz/src/strategies/calldata.rs @@ -1,85 +1,23 @@ -use crate::strategies::{fuzz_param, EvmFuzzState}; +use crate::{strategies::fuzz_param, FuzzFixtures}; use alloy_dyn_abi::JsonAbiExt; use alloy_json_abi::Function; -use alloy_primitives::{Address, Bytes}; -use foundry_config::FuzzDictionaryConfig; -use hashbrown::HashSet; +use alloy_primitives::Bytes; use proptest::prelude::{BoxedStrategy, Strategy}; -use std::{fmt, sync::Arc}; - -/// Clonable wrapper around [CalldataFuzzDictionary]. -#[derive(Debug, Clone)] -pub struct CalldataFuzzDictionary { - pub inner: Arc, -} - -impl CalldataFuzzDictionary { - pub fn new(config: &FuzzDictionaryConfig, state: EvmFuzzState) -> Self { - Self { inner: Arc::new(CalldataFuzzDictionaryConfig::new(config, state)) } - } -} - -#[derive(Clone)] -pub struct CalldataFuzzDictionaryConfig { - /// Addresses that can be used for fuzzing calldata. - pub addresses: Vec
, -} - -impl fmt::Debug for CalldataFuzzDictionaryConfig { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CalldataFuzzDictionaryConfig").field("addresses", &self.addresses).finish() - } -} - -/// Represents custom configuration for invariant fuzzed calldata strategies. -/// -/// At the moment only the dictionary of addresses to be used for a fuzzed `function(address)` can -/// be configured, but support for other types can be added. -impl CalldataFuzzDictionaryConfig { - /// Creates config with the set of addresses that can be used for fuzzing invariant calldata (if - /// `max_calldata_fuzz_dictionary_addresses` configured). - /// The set of addresses contains a number of `max_calldata_fuzz_dictionary_addresses` random - /// addresses plus all addresses that already had their PUSH bytes collected (retrieved from - /// `EvmFuzzState`, if `include_push_bytes` config enabled). - pub fn new(config: &FuzzDictionaryConfig, state: EvmFuzzState) -> Self { - let mut addresses: HashSet
= HashSet::new(); - let dict_size = config.max_calldata_fuzz_dictionary_addresses; - - if dict_size > 0 { - loop { - if addresses.len() == dict_size { - break - } - addresses.insert(Address::random()); - } - - // Add all addresses that already had their PUSH bytes collected. - let mut state = state.write(); - addresses.extend(state.addresses()); - } - - Self { addresses: Vec::from_iter(addresses) } - } -} /// Given a function, it returns a strategy which generates valid calldata -/// for that function's input types. -pub fn fuzz_calldata(func: Function) -> BoxedStrategy { - fuzz_calldata_with_config(func, None) -} - -/// Given a function, it returns a strategy which generates valid calldata -/// for that function's input types, following custom configuration rules. -pub fn fuzz_calldata_with_config( - func: Function, - config: Option, -) -> BoxedStrategy { +/// for that function's input types, following declared test fixtures. +pub fn fuzz_calldata(func: Function, fuzz_fixtures: FuzzFixtures) -> BoxedStrategy { // We need to compose all the strategies generated for each parameter in all // possible combinations let strats = func .inputs .iter() - .map(|input| fuzz_param(&input.selector_type().parse().unwrap(), config.clone())) + .map(|input| { + fuzz_param( + &input.selector_type().parse().unwrap(), + fuzz_fixtures.param_fixtures(&input.name), + ) + }) .collect::>(); strats diff --git a/crates/evm/fuzz/src/strategies/int.rs b/crates/evm/fuzz/src/strategies/int.rs index e92c2d4642ca..d20caaa14a78 100644 --- a/crates/evm/fuzz/src/strategies/int.rs +++ b/crates/evm/fuzz/src/strategies/int.rs @@ -1,3 +1,4 @@ +use alloy_dyn_abi::{DynSolType, DynSolValue}; use alloy_primitives::{Sign, I256, U256}; use proptest::{ strategy::{NewTree, Strategy, ValueTree}, @@ -79,12 +80,21 @@ impl ValueTree for IntValueTree { /// param). Then generate a value for this bit size. /// 2. Generate a random value around the edges (+/- 3 around min, 0 and max possible value) /// 3. Generate a value from a predefined fixtures set +/// +/// To define int fixtures: +/// - return an array of possible values for a parameter named `amount` declare a function +/// `function fixtures_amount() public returns (int32[] memory)`. +/// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values +/// `function testFuzz_int32(int32 amount)`. +/// +/// If fixture is not a valid int type then an error is raised and test suite will continue to +/// execute with random int values. #[derive(Debug)] pub struct IntStrategy { /// Bit size of int (e.g. 256) bits: usize, /// A set of fixtures to be generated - fixtures: Vec, + fixtures: Vec, /// The weight for edge cases (+/- 3 around 0 and max possible value) edge_weight: usize, /// The weight for fixtures @@ -98,10 +108,14 @@ impl IntStrategy { /// #Arguments /// * `bits` - Size of uint in bits /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) - pub fn new(bits: usize, fixtures: Vec) -> Self { + pub fn new(bits: usize, fixtures: Option<&[DynSolValue]>) -> Self { + let int_fixtures = match fixtures { + Some(values) => Vec::from(values), + None => vec![], + }; Self { bits, - fixtures, + fixtures: int_fixtures, edge_weight: 10usize, fixtures_weight: 40usize, random_weight: 50usize, @@ -128,12 +142,25 @@ impl IntStrategy { } fn generate_fixtures_tree(&self, runner: &mut TestRunner) -> NewTree { - // generate edge cases if there's no fixtures + // generate random cases if there's no fixtures if self.fixtures.is_empty() { - return self.generate_edge_tree(runner) + return self.generate_random_tree(runner) + } + + // Generate value tree from fixture. + // If fixture is not a valid type, raise error and generate random value. + let fixture = &self.fixtures[runner.rng().gen_range(0..self.fixtures.len())]; + if let Some(int_fixture) = fixture.as_int() { + if int_fixture.1 == self.bits { + return Ok(IntValueTree::new(int_fixture.0, false)); + } } - let idx = runner.rng().gen_range(0..self.fixtures.len()); - Ok(IntValueTree::new(self.fixtures[idx], false)) + error!( + "{:?} is not a valid {} fixture, generate random tree", + fixture, + DynSolType::Int(self.bits) + ); + self.generate_random_tree(runner) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { diff --git a/crates/evm/fuzz/src/strategies/invariants.rs b/crates/evm/fuzz/src/strategies/invariants.rs index e6dedc9cd8dd..cf20f0c80d7e 100644 --- a/crates/evm/fuzz/src/strategies/invariants.rs +++ b/crates/evm/fuzz/src/strategies/invariants.rs @@ -1,7 +1,8 @@ -use super::{fuzz_calldata_with_config, fuzz_param_from_state, CalldataFuzzDictionary}; +use super::{fuzz_calldata, fuzz_param_from_state}; use crate::{ invariant::{BasicTxDetails, FuzzRunIdentifiedContracts, SenderFilters}, strategies::{fuzz_calldata_from_state, fuzz_param, EvmFuzzState}, + FuzzFixtures, }; use alloy_json_abi::{Function, JsonAbi}; use alloy_primitives::{Address, Bytes}; @@ -14,7 +15,7 @@ pub fn override_call_strat( fuzz_state: EvmFuzzState, contracts: FuzzRunIdentifiedContracts, target: Arc>, - calldata_fuzz_config: CalldataFuzzDictionary, + fuzz_fixtures: FuzzFixtures, ) -> SBoxedStrategy<(Address, Bytes)> { let contracts_ref = contracts.clone(); @@ -28,16 +29,11 @@ pub fn override_call_strat( ]) .prop_flat_map(move |target_address| { let fuzz_state = fuzz_state.clone(); - let calldata_fuzz_config = calldata_fuzz_config.clone(); + let fixtures = fuzz_fixtures.clone(); let (_, abi, functions) = contracts.lock().get(&target_address).unwrap().clone(); let func = select_random_function(abi, functions); func.prop_flat_map(move |func| { - fuzz_contract_with_calldata( - fuzz_state.clone(), - calldata_fuzz_config.clone(), - target_address, - func, - ) + fuzz_contract_with_calldata(fuzz_state.clone(), target_address, func, fixtures.clone()) }) }) .sboxed() @@ -58,11 +54,11 @@ pub fn invariant_strat( senders: SenderFilters, contracts: FuzzRunIdentifiedContracts, dictionary_weight: u32, - calldata_fuzz_config: CalldataFuzzDictionary, + fuzz_fixtures: FuzzFixtures, ) -> impl Strategy { // We only want to seed the first value, since we want to generate the rest as we mutate the // state - generate_call(fuzz_state, senders, contracts, dictionary_weight, calldata_fuzz_config) + generate_call(fuzz_state, senders, contracts, dictionary_weight, fuzz_fixtures) } /// Strategy to generate a transaction where the `sender`, `target` and `calldata` are all generated @@ -72,7 +68,7 @@ fn generate_call( senders: SenderFilters, contracts: FuzzRunIdentifiedContracts, dictionary_weight: u32, - calldata_fuzz_config: CalldataFuzzDictionary, + fuzz_fixtures: FuzzFixtures, ) -> BoxedStrategy { let random_contract = select_random_contract(contracts); let senders = Rc::new(senders); @@ -81,7 +77,7 @@ fn generate_call( let func = select_random_function(abi, functions); let senders = senders.clone(); let fuzz_state = fuzz_state.clone(); - let calldata_fuzz_config = calldata_fuzz_config.clone(); + let fixture = fuzz_fixtures.clone(); func.prop_flat_map(move |func| { let sender = select_random_sender(fuzz_state.clone(), senders.clone(), dictionary_weight); @@ -89,9 +85,9 @@ fn generate_call( sender, fuzz_contract_with_calldata( fuzz_state.clone(), - calldata_fuzz_config.clone(), contract, func, + fixture.clone(), ), ) }) @@ -183,16 +179,16 @@ fn select_random_function( /// for that function's input types. pub fn fuzz_contract_with_calldata( fuzz_state: EvmFuzzState, - calldata_fuzz_config: CalldataFuzzDictionary, contract: Address, func: Function, + fuzz_fixtures: FuzzFixtures, ) -> impl Strategy { // We need to compose all the strategies generated for each parameter in all // possible combinations // `prop_oneof!` / `TupleUnion` `Arc`s for cheap cloning #[allow(clippy::arc_with_non_send_sync)] let strats = prop_oneof![ - 60 => fuzz_calldata_with_config(func.clone(), Some(calldata_fuzz_config)), + 60 => fuzz_calldata(func.clone(), fuzz_fixtures), 40 => fuzz_calldata_from_state(func, fuzz_state), ]; strats.prop_map(move |calldata| { diff --git a/crates/evm/fuzz/src/strategies/mod.rs b/crates/evm/fuzz/src/strategies/mod.rs index 63e008ec0c96..f6d7a4da760a 100644 --- a/crates/evm/fuzz/src/strategies/mod.rs +++ b/crates/evm/fuzz/src/strategies/mod.rs @@ -1,3 +1,6 @@ +mod address; +pub use address::AddressStrategy; + mod int; pub use int::IntStrategy; @@ -8,9 +11,7 @@ mod param; pub use param::{fuzz_param, fuzz_param_from_state}; mod calldata; -pub use calldata::{ - fuzz_calldata, fuzz_calldata_with_config, CalldataFuzzDictionary, CalldataFuzzDictionaryConfig, -}; +pub use calldata::fuzz_calldata; mod state; pub use state::{ diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index 4c8dc03eca9e..b68af536e1ee 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -1,5 +1,4 @@ use super::state::EvmFuzzState; -use crate::strategies::calldata::CalldataFuzzDictionary; use alloy_dyn_abi::{DynSolType, DynSolValue}; use alloy_primitives::{Address, FixedBytes, I256, U256}; use arbitrary::Unstructured; @@ -8,42 +7,35 @@ use proptest::prelude::*; /// The max length of arrays we fuzz for is 256. const MAX_ARRAY_LEN: usize = 256; -/// Given a parameter type, returns a strategy for generating values for that type. +/// Given a parameter type and configured fixtures for param name, returns a strategy for generating +/// values for that type. Fixtures can be currently generated for uint, int and address types and +/// are defined for named parameter. +/// +/// For example, fixtures for parameter `owner` of type `address` can be defined in a function with +/// a `function fixtures_owner() public returns (address[] memory)` signature. +/// +/// Fixtures are matched on parameter name, hence fixtures defined in +/// `fixtures_owner` function can be used in a fuzzed test function with a signature like +/// `function testFuzz_ownerAddress(address owner, uint amount)`. +/// +/// If the type of fixtures is different than the parameter type then error is raised and a random +/// value is generated. /// /// Works with ABI Encoder v2 tuples. pub fn fuzz_param( param: &DynSolType, - config: Option, + fixtures: Option<&[DynSolValue]>, ) -> BoxedStrategy { let param = param.to_owned(); match param { - DynSolType::Address => { - if config.is_some() { - let fuzz_config = config.unwrap().inner; - let address_dict_len = fuzz_config.addresses.len(); - if address_dict_len > 0 { - // Create strategy to return random address from configured dictionary. - return any::() - .prop_map(move |index| index.index(address_dict_len)) - .prop_map(move |index| { - DynSolValue::Address(fuzz_config.addresses.get(index).cloned().unwrap()) - }) - .boxed() - } - } - - // If no config for addresses dictionary then create unbounded addresses strategy. - any::<[u8; 32]>() - .prop_map(|x| DynSolValue::Address(Address::from_word(x.into()))) - .boxed() - } + DynSolType::Address => super::AddressStrategy::address_strategy(fixtures), DynSolType::Int(n) => { - let strat = super::IntStrategy::new(n, vec![]); + let strat = super::IntStrategy::new(n, fixtures); let strat = strat.prop_map(move |x| DynSolValue::Int(x, n)); strat.boxed() } DynSolType::Uint(n) => { - let strat = super::UintStrategy::new(n, vec![]); + let strat = super::UintStrategy::new(n, fixtures); let strat = strat.prop_map(move |x| DynSolValue::Uint(x, n)); strat.boxed() } @@ -71,17 +63,17 @@ pub fn fuzz_param( .boxed(), DynSolType::Tuple(params) => params .iter() - .map(|p| fuzz_param(p, config.clone())) + .map(|p| fuzz_param(p, None)) .collect::>() .prop_map(DynSolValue::Tuple) .boxed(), DynSolType::FixedArray(param, size) => { - proptest::collection::vec(fuzz_param(¶m, config), size) + proptest::collection::vec(fuzz_param(¶m, None), size) .prop_map(DynSolValue::FixedArray) .boxed() } DynSolType::Array(param) => { - proptest::collection::vec(fuzz_param(¶m, config), 0..MAX_ARRAY_LEN) + proptest::collection::vec(fuzz_param(¶m, None), 0..MAX_ARRAY_LEN) .prop_map(DynSolValue::Array) .boxed() } @@ -193,7 +185,10 @@ pub fn fuzz_param_from_state( #[cfg(test)] mod tests { - use crate::strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state}; + use crate::{ + strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state}, + FuzzFixtures, + }; use foundry_common::abi::get_func; use foundry_config::FuzzDictionaryConfig; use revm::db::{CacheDB, EmptyDB}; @@ -203,9 +198,11 @@ mod tests { let f = "testArray(uint64[2] calldata values)"; let func = get_func(f).unwrap(); let db = CacheDB::new(EmptyDB::default()); - let state = build_initial_state(&db, &FuzzDictionaryConfig::default()); + let config = &FuzzDictionaryConfig::default(); + let state = build_initial_state(&db, config); + let fuzz_fixtures = FuzzFixtures::default(); let strat = proptest::strategy::Union::new_weighted(vec![ - (60, fuzz_calldata(func.clone())), + (60, fuzz_calldata(func.clone(), fuzz_fixtures)), (40, fuzz_calldata_from_state(func, state)), ]); let cfg = proptest::test_runner::Config { failure_persistence: None, ..Default::default() }; diff --git a/crates/evm/fuzz/src/strategies/uint.rs b/crates/evm/fuzz/src/strategies/uint.rs index e1d74552612e..48937a8ddd47 100644 --- a/crates/evm/fuzz/src/strategies/uint.rs +++ b/crates/evm/fuzz/src/strategies/uint.rs @@ -1,3 +1,4 @@ +use alloy_dyn_abi::{DynSolType, DynSolValue}; use alloy_primitives::U256; use proptest::{ strategy::{NewTree, Strategy, ValueTree}, @@ -71,12 +72,21 @@ impl ValueTree for UintValueTree { /// param). Then generate a value for this bit size. /// 2. Generate a random value around the edges (+/- 3 around 0 and max possible value) /// 3. Generate a value from a predefined fixtures set +/// +/// To define uint fixtures: +/// - return an array of possible values for a parameter named `amount` declare a function +/// `function fixtures_amount() public returns (uint32[] memory)`. +/// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values +/// `function testFuzz_uint32(uint32 amount)`. +/// +/// If fixture is not a valid uint type then an error is raised and test suite will continue to +/// execute with random int values. #[derive(Debug)] pub struct UintStrategy { /// Bit size of uint (e.g. 256) bits: usize, /// A set of fixtures to be generated - fixtures: Vec, + fixtures: Vec, /// The weight for edge cases (+/- 3 around 0 and max possible value) edge_weight: usize, /// The weight for fixtures @@ -90,10 +100,14 @@ impl UintStrategy { /// #Arguments /// * `bits` - Size of uint in bits /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) - pub fn new(bits: usize, fixtures: Vec) -> Self { + pub fn new(bits: usize, fixtures: Option<&[DynSolValue]>) -> Self { + let uint_fixtures = match fixtures { + Some(values) => Vec::from(values), + None => vec![], + }; Self { bits, - fixtures, + fixtures: uint_fixtures, edge_weight: 10usize, fixtures_weight: 40usize, random_weight: 50usize, @@ -105,19 +119,30 @@ impl UintStrategy { // Choose if we want values around 0 or max let is_min = rng.gen_bool(0.5); let offset = U256::from(rng.gen_range(0..4)); - let max = - if self.bits < 256 { (U256::from(1) << self.bits) - U256::from(1) } else { U256::MAX }; - let start = if is_min { offset } else { max.saturating_sub(offset) }; + let start = if is_min { offset } else { self.type_max().saturating_sub(offset) }; Ok(UintValueTree::new(start, false)) } fn generate_fixtures_tree(&self, runner: &mut TestRunner) -> NewTree { - // generate edge cases if there's no fixtures + // generate random cases if there's no fixtures if self.fixtures.is_empty() { - return self.generate_edge_tree(runner) + return self.generate_random_tree(runner) + } + + // Generate value tree from fixture. + // If fixture is not a valid type, raise error and generate random value. + let fixture = &self.fixtures[runner.rng().gen_range(0..self.fixtures.len())]; + if let Some(uint_fixture) = fixture.as_uint() { + if uint_fixture.1 == self.bits { + return Ok(UintValueTree::new(uint_fixture.0, false)); + } } - let idx = runner.rng().gen_range(0..self.fixtures.len()); - Ok(UintValueTree::new(self.fixtures[idx], false)) + error!( + "{:?} is not a valid {} fixture, generate random tree", + fixture, + DynSolType::Uint(self.bits) + ); + self.generate_random_tree(runner) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { @@ -151,6 +176,14 @@ impl UintStrategy { Ok(UintValueTree::new(start, false)) } + + fn type_max(&self) -> U256 { + if self.bits < 256 { + (U256::from(1) << self.bits) - U256::from(1) + } else { + U256::MAX + } + } } impl Strategy for UintStrategy { diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index dfb1d8cf5941..e0809be67dcd 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -181,7 +181,7 @@ impl TestArgs { let test_options: TestOptions = TestOptionsBuilder::default() .fuzz(config.clone().fuzz) - .invariant(config.invariant) + .invariant(config.clone().invariant) .profiles(profiles) .build(&output, project_root)?; @@ -202,7 +202,7 @@ impl TestArgs { let runner = MultiContractRunnerBuilder::default() .set_debug(should_debug) .initial_balance(evm_opts.initial_balance) - .evm_spec(config.evm_spec_id()) + .evm_spec(config.clone().evm_spec_id()) .sender(evm_opts.sender) .with_fork(evm_opts.get_fork(&config, env.clone())) .with_cheats_config(CheatsConfig::new(&config, evm_opts.clone(), None)) @@ -221,7 +221,7 @@ impl TestArgs { *test_pattern = Some(debug_test_pattern.clone()); } - let outcome = self.run_tests(runner, config, verbosity, &filter).await?; + let outcome = self.run_tests(runner, config.clone(), verbosity, &filter).await?; if should_debug { // There is only one test. diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 3b1a1a207178..093381a7da7b 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -22,7 +22,7 @@ use foundry_evm::{ invariant::{replay_run, InvariantExecutor, InvariantFuzzError, InvariantFuzzTestResult}, CallResult, EvmError, ExecutionErr, Executor, }, - fuzz::{invariant::InvariantContract, CounterExample}, + fuzz::{invariant::InvariantContract, CounterExample, FuzzFixtures}, traces::{load_contracts, TraceKind}, }; use proptest::test_runner::TestRunner; @@ -182,6 +182,26 @@ impl<'a> ContractRunner<'a> { Ok(setup) } + fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { + // collect test fixtures param:array of values + let fixtures_fns: Vec<_> = + self.contract.functions().filter(|func| func.name.is_fixtures()).collect(); + let mut fixtures = HashMap::new(); + fixtures_fns.iter().for_each(|func| { + if let Ok(CallResult { result, .. }) = self.executor.call( + CALLER, + address, + func.signature_with_outputs(), + vec![], + U256::ZERO, + None, + ) { + fixtures.insert(func.name.strip_prefix("fixtures_").unwrap().to_string(), result); + } + }); + FuzzFixtures::new(fixtures) + } + /// Runs all tests for a contract whose names match the provided regular expression pub fn run_tests( mut self, @@ -254,6 +274,12 @@ impl<'a> ContractRunner<'a> { ) } + // Collect fixtures from test contract. Fixtures are functions prefixed with `fixture_` and + // followed by the name of the parameter. For example: + // `fixture_test() returns (address[] memory)` function define an array of addresses to be + // used for fuzzed `test` named parameter, in scope of the current test. + let fuzz_fixtures = self.fuzz_fixtures(setup.address); + // Filter out functions sequentially since it's very fast and there is no need to do it // in parallel. let find_timer = Instant::now(); @@ -289,12 +315,20 @@ impl<'a> ContractRunner<'a> { func, known_contracts, identified_contracts.as_ref().unwrap(), + fuzz_fixtures.clone(), ) } else if func.is_fuzz_test() { debug_assert!(func.is_test()); let runner = test_options.fuzz_runner(self.name, &func.name); let fuzz_config = test_options.fuzz_config(self.name, &func.name); - self.run_fuzz_test(func, should_fail, runner, setup, fuzz_config.clone()) + self.run_fuzz_test( + func, + should_fail, + runner, + setup, + fuzz_config.clone(), + fuzz_fixtures.clone(), + ) } else { debug_assert!(func.is_test()); self.run_test(func, should_fail, setup) @@ -443,6 +477,7 @@ impl<'a> ContractRunner<'a> { } } + #[allow(clippy::too_many_arguments)] #[instrument(name = "invariant_test", skip_all)] pub fn run_invariant_test( &self, @@ -452,6 +487,7 @@ impl<'a> ContractRunner<'a> { func: &Function, known_contracts: Option<&ContractsByArtifact>, identified_contracts: &ContractsByAddress, + fuzz_fixtures: FuzzFixtures, ) -> TestResult { trace!(target: "forge::test::fuzz", "executing invariant test for {:?}", func.name); let empty = ContractsByArtifact::default(); @@ -493,7 +529,7 @@ impl<'a> ContractRunner<'a> { InvariantContract { address, invariant_function: func, abi: self.contract }; let InvariantFuzzTestResult { error, cases, reverts, last_run_inputs, gas_report_traces } = - match evm.invariant_fuzz(invariant_contract.clone()) { + match evm.invariant_fuzz(invariant_contract.clone(), fuzz_fixtures) { Ok(x) => x, Err(e) => { return TestResult { @@ -586,6 +622,7 @@ impl<'a> ContractRunner<'a> { runner: TestRunner, setup: TestSetup, fuzz_config: FuzzConfig, + fuzz_fixtures: FuzzFixtures, ) -> TestResult { let span = info_span!("fuzz_test", %should_fail); if !span.is_disabled() { @@ -611,7 +648,8 @@ impl<'a> ContractRunner<'a> { fuzz_config.clone(), ); let state = fuzzed_executor.build_fuzz_state(); - let result = fuzzed_executor.fuzz(func, address, should_fail, self.revert_decoder); + let result = + fuzzed_executor.fuzz(func, address, should_fail, self.revert_decoder, fuzz_fixtures); let mut debug = Default::default(); let mut breakpoints = Default::default(); diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 99b9ad962783..743bf31eebef 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -333,7 +333,7 @@ async fn test_invariant_preserve_state() { } #[tokio::test(flavor = "multi_thread")] -async fn test_invariant_calldata_fuzz_dictionary_addresses() { +async fn test_invariant_with_address_fixture() { // should not fail with default options (address dict not finite) let mut runner = runner(); let results = runner.test_collect(&Filter::new( @@ -341,22 +341,6 @@ async fn test_invariant_calldata_fuzz_dictionary_addresses() { ".*", ".*fuzz/invariant/common/InvariantCalldataDictionary.t.sol", )); - assert_multiple( - &results, - BTreeMap::from([( - "fuzz/invariant/common/InvariantCalldataDictionary.t.sol:InvariantCalldataDictionary", - vec![("invariant_owner_never_changes()", true, None, None, None)], - )]), - ); - - // same test should fail when calldata address dict is bounded - // set address dictionary to single entry to fail fast - runner.test_options.invariant.dictionary.max_calldata_fuzz_dictionary_addresses = 1; - let results = runner.test_collect(&Filter::new( - ".*", - ".*", - ".*fuzz/invariant/common/InvariantCalldataDictionary.t.sol", - )); assert_multiple( &results, BTreeMap::from([( diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index 969e673579cf..33d600d37df4 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -92,7 +92,6 @@ pub static TEST_OPTS: Lazy = Lazy::new(|| { dictionary_weight: 40, max_fuzz_dictionary_addresses: 10_000, max_fuzz_dictionary_values: 10_000, - max_calldata_fuzz_dictionary_addresses: 0, }, gas_report_samples: 256, failure_persist_dir: Some(tempfile::tempdir().unwrap().into_path()), @@ -109,7 +108,6 @@ pub static TEST_OPTS: Lazy = Lazy::new(|| { include_push_bytes: true, max_fuzz_dictionary_addresses: 10_000, max_fuzz_dictionary_values: 10_000, - max_calldata_fuzz_dictionary_addresses: 0, }, shrink_sequence: true, shrink_run_limit: 2usize.pow(18u32), diff --git a/testdata/fuzz/invariant/common/InvariantCalldataDictionary.t.sol b/testdata/fuzz/invariant/common/InvariantCalldataDictionary.t.sol index c8f87a600b6d..df700e7fd078 100644 --- a/testdata/fuzz/invariant/common/InvariantCalldataDictionary.t.sol +++ b/testdata/fuzz/invariant/common/InvariantCalldataDictionary.t.sol @@ -62,11 +62,14 @@ contract InvariantCalldataDictionary is DSTest { address owner; Owned owned; Handler handler; + address[] actors; function setUp() public { owner = address(this); owned = new Owned(); handler = new Handler(owned); + actors.push(owner); + actors.push(address(777)); } function targetSelectors() public returns (FuzzSelector[] memory) { @@ -78,6 +81,14 @@ contract InvariantCalldataDictionary is DSTest { return targets; } + function fixtures_sender() external returns (address[] memory) { + return actors; + } + + function fixtures_candidate() external returns (address[] memory) { + return actors; + } + function invariant_owner_never_changes() public { assertEq(owned.owner(), owner); } From 3c486668103de0189bf29591b4b30b358c85ccbc Mon Sep 17 00:00:00 2001 From: grandizzy Date: Tue, 19 Mar 2024 07:43:41 +0200 Subject: [PATCH 02/22] Fix tests, collect fixtures in test setup, arc fixtures --- crates/evm/fuzz/src/lib.rs | 6 ++-- crates/forge/src/result.rs | 8 +++-- crates/forge/src/runner.rs | 56 ++++++++++++++++++------------ crates/forge/tests/it/invariant.rs | 8 ++++- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index b68ff4d56b23..2ece9c3e0d56 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -14,7 +14,7 @@ use foundry_evm_coverage::HitMaps; use foundry_evm_traces::CallTraceArena; use itertools::Itertools; use serde::{Deserialize, Serialize}; -use std::{collections::HashMap, fmt}; +use std::{collections::HashMap, fmt, sync::Arc}; pub use proptest::test_runner::{Config as FuzzConfig, Reason}; @@ -275,12 +275,12 @@ impl FuzzedCases { #[derive(Clone, Default, Debug)] pub struct FuzzFixtures { - inner: HashMap, + inner: Arc>, } impl FuzzFixtures { pub fn new(fixtures: HashMap) -> FuzzFixtures { - Self { inner: fixtures } + Self { inner: Arc::new(fixtures) } } pub fn param_fixtures(&self, param_name: &String) -> Option<&[DynSolValue]> { diff --git a/crates/forge/src/result.rs b/crates/forge/src/result.rs index 94c6d58fcd12..68a73d70a8cf 100644 --- a/crates/forge/src/result.rs +++ b/crates/forge/src/result.rs @@ -6,7 +6,7 @@ use foundry_evm::{ coverage::HitMaps, debug::DebugArena, executors::EvmError, - fuzz::{CounterExample, FuzzCase}, + fuzz::{CounterExample, FuzzCase, FuzzFixtures}, traces::{CallTraceArena, CallTraceDecoder, TraceKind, Traces}, }; use serde::{Deserialize, Serialize}; @@ -522,6 +522,8 @@ pub struct TestSetup { pub reason: Option, /// Coverage info during setup pub coverage: Option, + /// Defined fuzz test fixtures + pub fuzz_fixtures: FuzzFixtures, } impl TestSetup { @@ -554,8 +556,9 @@ impl TestSetup { traces: Traces, labeled_addresses: HashMap, coverage: Option, + fuzz_fixtures: FuzzFixtures, ) -> Self { - Self { address, logs, traces, labeled_addresses, reason: None, coverage } + Self { address, logs, traces, labeled_addresses, reason: None, coverage, fuzz_fixtures } } pub fn failed_with( @@ -571,6 +574,7 @@ impl TestSetup { labeled_addresses, reason: Some(reason), coverage: None, + fuzz_fixtures: FuzzFixtures::default(), } } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 093381a7da7b..52d466deaea9 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -174,14 +174,36 @@ impl<'a> ContractRunner<'a> { traces.extend(setup_traces.map(|traces| (TraceKind::Setup, traces))); logs.extend(setup_logs); - TestSetup { address, logs, traces, labeled_addresses, reason, coverage } + TestSetup { + address, + logs, + traces, + labeled_addresses, + reason, + coverage, + fuzz_fixtures: self.fuzz_fixtures(address), + } } else { - TestSetup::success(address, logs, traces, Default::default(), None) + TestSetup::success( + address, + logs, + traces, + Default::default(), + None, + self.fuzz_fixtures(address), + ) }; Ok(setup) } + /// Collect fixtures from test contract. Fixtures are functions prefixed with `fixtures_` key + /// and followed by the name of the parameter. + /// + /// For example: + /// `fixtures_test() returns (address[] memory)` function + /// define an array of addresses to be used for fuzzed `test` named parameter in scope of the + /// current test. fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { // collect test fixtures param:array of values let fixtures_fns: Vec<_> = @@ -274,12 +296,6 @@ impl<'a> ContractRunner<'a> { ) } - // Collect fixtures from test contract. Fixtures are functions prefixed with `fixture_` and - // followed by the name of the parameter. For example: - // `fixture_test() returns (address[] memory)` function define an array of addresses to be - // used for fuzzed `test` named parameter, in scope of the current test. - let fuzz_fixtures = self.fuzz_fixtures(setup.address); - // Filter out functions sequentially since it's very fast and there is no need to do it // in parallel. let find_timer = Instant::now(); @@ -315,20 +331,12 @@ impl<'a> ContractRunner<'a> { func, known_contracts, identified_contracts.as_ref().unwrap(), - fuzz_fixtures.clone(), ) } else if func.is_fuzz_test() { debug_assert!(func.is_test()); let runner = test_options.fuzz_runner(self.name, &func.name); let fuzz_config = test_options.fuzz_config(self.name, &func.name); - self.run_fuzz_test( - func, - should_fail, - runner, - setup, - fuzz_config.clone(), - fuzz_fixtures.clone(), - ) + self.run_fuzz_test(func, should_fail, runner, setup, fuzz_config.clone()) } else { debug_assert!(func.is_test()); self.run_test(func, should_fail, setup) @@ -477,7 +485,6 @@ impl<'a> ContractRunner<'a> { } } - #[allow(clippy::too_many_arguments)] #[instrument(name = "invariant_test", skip_all)] pub fn run_invariant_test( &self, @@ -487,12 +494,12 @@ impl<'a> ContractRunner<'a> { func: &Function, known_contracts: Option<&ContractsByArtifact>, identified_contracts: &ContractsByAddress, - fuzz_fixtures: FuzzFixtures, ) -> TestResult { trace!(target: "forge::test::fuzz", "executing invariant test for {:?}", func.name); let empty = ContractsByArtifact::default(); let project_contracts = known_contracts.unwrap_or(&empty); - let TestSetup { address, logs, traces, labeled_addresses, coverage, .. } = setup; + let TestSetup { address, logs, traces, labeled_addresses, coverage, fuzz_fixtures, .. } = + setup; // First, run the test normally to see if it needs to be skipped. let start = Instant::now(); @@ -622,7 +629,6 @@ impl<'a> ContractRunner<'a> { runner: TestRunner, setup: TestSetup, fuzz_config: FuzzConfig, - fuzz_fixtures: FuzzFixtures, ) -> TestResult { let span = info_span!("fuzz_test", %should_fail); if !span.is_disabled() { @@ -636,7 +642,13 @@ impl<'a> ContractRunner<'a> { let _guard = span.enter(); let TestSetup { - address, mut logs, mut traces, mut labeled_addresses, mut coverage, .. + address, + mut logs, + mut traces, + mut labeled_addresses, + mut coverage, + fuzz_fixtures, + .. } = setup; // Run fuzz test diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 743bf31eebef..49bdf8fb47ae 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -139,7 +139,13 @@ async fn test_invariant() { ), ( "fuzz/invariant/common/InvariantCalldataDictionary.t.sol:InvariantCalldataDictionary", - vec![("invariant_owner_never_changes()", true, None, None, None)], + vec![( + "invariant_owner_never_changes()", + false, + Some("".into()), + None, + None, + )], ), ( "fuzz/invariant/common/InvariantAssume.t.sol:InvariantAssume", From 254017c7efd54b98e894fe04c088a8bc6ff9adf6 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Tue, 19 Mar 2024 13:14:07 +0200 Subject: [PATCH 03/22] Cleanup --- crates/common/src/traits.rs | 2 +- crates/evm/evm/src/executors/fuzz/mod.rs | 4 ++-- crates/evm/evm/src/executors/invariant/mod.rs | 6 ++--- crates/evm/fuzz/src/strategies/address.rs | 2 +- crates/evm/fuzz/src/strategies/int.rs | 6 +---- crates/evm/fuzz/src/strategies/uint.rs | 6 +---- crates/forge/src/runner.rs | 24 +++++++++---------- 7 files changed, 21 insertions(+), 29 deletions(-) diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index 0e7cf0dece31..d43b860f43b9 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -113,7 +113,7 @@ impl TestFunctionExt for str { } fn is_fixtures(&self) -> bool { - self.starts_with("fixtures") + self.starts_with("fixtures_") } } diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 5aaa45c144b1..5ac4e6f6e932 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -58,7 +58,7 @@ impl FuzzedExecutor { pub fn fuzz( &self, func: &Function, - fuzz_fixtures: FuzzFixtures, + fuzz_fixtures: &FuzzFixtures, address: Address, should_fail: bool, rd: &RevertDecoder, @@ -86,7 +86,7 @@ impl FuzzedExecutor { let dictionary_weight = self.config.dictionary.dictionary_weight.min(100); let strat = proptest::prop_oneof![ - 100 - dictionary_weight => fuzz_calldata(func.clone(), &fuzz_fixtures), + 100 - dictionary_weight => fuzz_calldata(func.clone(), fuzz_fixtures), dictionary_weight => fuzz_calldata_from_state(func.clone(), &state), ]; diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index cca0d94f0f23..7db9dd268f1f 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -148,7 +148,7 @@ impl<'a> InvariantExecutor<'a> { pub fn invariant_fuzz( &mut self, invariant_contract: InvariantContract<'_>, - fuzz_fixtures: FuzzFixtures, + fuzz_fixtures: &FuzzFixtures, ) -> Result { // Throw an error to abort test run if the invariant function accepts input params if !invariant_contract.invariant_function.inputs.is_empty() { @@ -156,7 +156,7 @@ impl<'a> InvariantExecutor<'a> { } let (fuzz_state, targeted_contracts, strat) = - self.prepare_fuzzing(&invariant_contract, fuzz_fixtures.clone())?; + self.prepare_fuzzing(&invariant_contract, fuzz_fixtures)?; // Stores the consumed gas and calldata of every successful fuzz call. let fuzz_cases: RefCell> = RefCell::new(Default::default()); @@ -346,7 +346,7 @@ impl<'a> InvariantExecutor<'a> { fn prepare_fuzzing( &mut self, invariant_contract: &InvariantContract<'_>, - fuzz_fixtures: FuzzFixtures, + fuzz_fixtures: &FuzzFixtures, ) -> eyre::Result { // Finds out the chosen deployed contracts and/or senders. self.select_contract_artifacts(invariant_contract.address)?; diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs index c3ce0f5f205b..1bebf7c74d4b 100644 --- a/crates/evm/fuzz/src/strategies/address.rs +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -29,10 +29,10 @@ impl AddressStrategy { fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); let address_fixtures_len = address_fixtures.len(); any::() - .prop_map(move |index| index.index(address_fixtures_len)) .prop_map(move |index| { // Generate value tree from fixture. // If fixture is not a valid address, raise error and generate random value. + let index = index.index(address_fixtures_len); if let Some(addr_fixture) = address_fixtures.get(index) { if let Some(addr_fixture) = addr_fixture.as_address() { return DynSolValue::Address(addr_fixture); diff --git a/crates/evm/fuzz/src/strategies/int.rs b/crates/evm/fuzz/src/strategies/int.rs index d20caaa14a78..fad4d56d8435 100644 --- a/crates/evm/fuzz/src/strategies/int.rs +++ b/crates/evm/fuzz/src/strategies/int.rs @@ -109,13 +109,9 @@ impl IntStrategy { /// * `bits` - Size of uint in bits /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) pub fn new(bits: usize, fixtures: Option<&[DynSolValue]>) -> Self { - let int_fixtures = match fixtures { - Some(values) => Vec::from(values), - None => vec![], - }; Self { bits, - fixtures: int_fixtures, + fixtures: Vec::from(fixtures.unwrap_or_default()), edge_weight: 10usize, fixtures_weight: 40usize, random_weight: 50usize, diff --git a/crates/evm/fuzz/src/strategies/uint.rs b/crates/evm/fuzz/src/strategies/uint.rs index 48937a8ddd47..c3164c23a5ca 100644 --- a/crates/evm/fuzz/src/strategies/uint.rs +++ b/crates/evm/fuzz/src/strategies/uint.rs @@ -101,13 +101,9 @@ impl UintStrategy { /// * `bits` - Size of uint in bits /// * `fixtures` - A set of fixed values to be generated (according to fixtures weight) pub fn new(bits: usize, fixtures: Option<&[DynSolValue]>) -> Self { - let uint_fixtures = match fixtures { - Some(values) => Vec::from(values), - None => vec![], - }; Self { bits, - fixtures: uint_fixtures, + fixtures: Vec::from(fixtures.unwrap_or_default()), edge_weight: 10usize, fixtures_weight: 40usize, random_weight: 50usize, diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index cde44236c976..adc019181fd9 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -206,17 +206,17 @@ impl<'a> ContractRunner<'a> { /// current test. fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { // collect test fixtures param:array of values - let fixtures_fns: Vec<_> = - self.contract.functions().filter(|func| func.name.is_fixtures()).collect(); let mut fixtures = HashMap::new(); - fixtures_fns.iter().for_each(|func| { - if let Ok(CallResult { raw: _, decoded_result }) = - self.executor.call(CALLER, address, func, &[], U256::ZERO, None) - { - fixtures.insert( - func.name.strip_prefix("fixtures_").unwrap().to_string(), - decoded_result, - ); + self.contract.functions().for_each(|func| { + if func.name.is_fixtures() { + if let Ok(CallResult { raw: _, decoded_result }) = + self.executor.call(CALLER, address, func, &[], U256::ZERO, None) + { + fixtures.insert( + func.name.strip_prefix("fixtures_").unwrap().to_string(), + decoded_result, + ); + } } }); FuzzFixtures::new(fixtures) @@ -538,7 +538,7 @@ impl<'a> ContractRunner<'a> { InvariantContract { address, invariant_function: func, abi: self.contract }; let InvariantFuzzTestResult { error, cases, reverts, last_run_inputs, gas_report_traces } = - match evm.invariant_fuzz(invariant_contract.clone(), fuzz_fixtures) { + match evm.invariant_fuzz(invariant_contract.clone(), &fuzz_fixtures) { Ok(x) => x, Err(e) => { return TestResult { @@ -665,7 +665,7 @@ impl<'a> ContractRunner<'a> { ); let state = fuzzed_executor.build_fuzz_state(); let result = - fuzzed_executor.fuzz(func, fuzz_fixtures, address, should_fail, self.revert_decoder); + fuzzed_executor.fuzz(func, &fuzz_fixtures, address, should_fail, self.revert_decoder); let mut debug = Default::default(); let mut breakpoints = Default::default(); From 14c869acb5131620da1de0cfbfb4ccdae00ee594 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 20 Mar 2024 08:37:53 +0200 Subject: [PATCH 04/22] Use fixture_ prefix --- crates/common/src/traits.rs | 16 ++++++++-------- crates/evm/fuzz/src/lib.rs | 5 +++++ crates/evm/fuzz/src/strategies/address.rs | 4 ++-- crates/evm/fuzz/src/strategies/int.rs | 2 +- crates/evm/fuzz/src/strategies/param.rs | 4 ++-- crates/evm/fuzz/src/strategies/uint.rs | 2 +- crates/forge/src/runner.rs | 8 ++++---- .../common/InvariantCalldataDictionary.t.sol | 4 ++-- 8 files changed, 25 insertions(+), 20 deletions(-) diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index d43b860f43b9..a3e78dafd099 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -34,8 +34,8 @@ pub trait TestFunctionExt { /// Returns whether this function is a `setUp` function. fn is_setup(&self) -> bool; - /// Returns whether this function is a `fixtures` function. - fn is_fixtures(&self) -> bool; + /// Returns whether this function is a `fixture` function. + fn is_fixture(&self) -> bool; } impl TestFunctionExt for Function { @@ -60,8 +60,8 @@ impl TestFunctionExt for Function { self.name.is_setup() } - fn is_fixtures(&self) -> bool { - self.name.is_fixtures() + fn is_fixture(&self) -> bool { + self.name.is_fixture() } } @@ -86,8 +86,8 @@ impl TestFunctionExt for String { self.as_str().is_setup() } - fn is_fixtures(&self) -> bool { - self.as_str().is_fixtures() + fn is_fixture(&self) -> bool { + self.as_str().is_fixture() } } @@ -112,8 +112,8 @@ impl TestFunctionExt for str { self.eq_ignore_ascii_case("setup") } - fn is_fixtures(&self) -> bool { - self.starts_with("fixtures_") + fn is_fixture(&self) -> bool { + self.starts_with("fixture_") } } diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index 2ece9c3e0d56..7d7db2f4fff1 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -273,6 +273,11 @@ impl FuzzedCases { } } +/// Fixtures to be used for fuzz tests. +/// The key represents name of the fuzzed parameter, value holds possible fuzzed values. +/// For example, for a fixture function declared as +/// `function fixture_sender() external returns (address[] memory senders)` +/// the fuzz fixtures will contain `sender` key with `senders` array as value #[derive(Clone, Default, Debug)] pub struct FuzzFixtures { inner: Arc>, diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs index 1bebf7c74d4b..344031e57cbe 100644 --- a/crates/evm/fuzz/src/strategies/address.rs +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -14,8 +14,8 @@ use proptest::{ /// /// /// For example: -/// To define fixtures for `owner` fuzzed parameter, return an array of possible values from -/// `function fixtures_owner() public returns (address[] memory)`. +/// To define fixture for `owner` fuzzed parameter, return an array of possible values from +/// `function fixture_owner() public returns (address[] memory)`. /// Use `owner` named parameter in fuzzed test in order to create a custom strategy /// `function testFuzz_ownerAddress(address owner, uint amount)`. #[derive(Debug)] diff --git a/crates/evm/fuzz/src/strategies/int.rs b/crates/evm/fuzz/src/strategies/int.rs index 36c0e3bfe7dc..07c14ef80ed2 100644 --- a/crates/evm/fuzz/src/strategies/int.rs +++ b/crates/evm/fuzz/src/strategies/int.rs @@ -87,7 +87,7 @@ impl ValueTree for IntValueTree { /// /// To define int fixtures: /// - return an array of possible values for a parameter named `amount` declare a function -/// `function fixtures_amount() public returns (int32[] memory)`. +/// `function fixture_amount() public returns (int32[] memory)`. /// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values /// `function testFuzz_int32(int32 amount)`. /// diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index de37d52c0e6f..75d69a27ecd2 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -11,10 +11,10 @@ const MAX_ARRAY_LEN: usize = 256; /// are defined for named parameter. /// /// For example, fixtures for parameter `owner` of type `address` can be defined in a function with -/// a `function fixtures_owner() public returns (address[] memory)` signature. +/// a `function fixture_owner() public returns (address[] memory)` signature. /// /// Fixtures are matched on parameter name, hence fixtures defined in -/// `fixtures_owner` function can be used in a fuzzed test function with a signature like +/// `fixture_owner` function can be used in a fuzzed test function with a signature like /// `function testFuzz_ownerAddress(address owner, uint amount)`. /// /// If the type of fixtures is different than the parameter type then error is raised and a random diff --git a/crates/evm/fuzz/src/strategies/uint.rs b/crates/evm/fuzz/src/strategies/uint.rs index d9264a6c3c4e..d5007123238a 100644 --- a/crates/evm/fuzz/src/strategies/uint.rs +++ b/crates/evm/fuzz/src/strategies/uint.rs @@ -75,7 +75,7 @@ impl ValueTree for UintValueTree { /// /// To define uint fixtures: /// - return an array of possible values for a parameter named `amount` declare a function -/// `function fixtures_amount() public returns (uint32[] memory)`. +/// `function fixture_amount() public returns (uint32[] memory)`. /// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values /// `function testFuzz_uint32(uint32 amount)`. /// diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index adc019181fd9..bbd521f5267c 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -197,23 +197,23 @@ impl<'a> ContractRunner<'a> { Ok(setup) } - /// Collect fixtures from test contract. Fixtures are functions prefixed with `fixtures_` key + /// Collect fixtures from test contract. Fixtures are functions prefixed with `fixture_` key /// and followed by the name of the parameter. /// /// For example: - /// `fixtures_test() returns (address[] memory)` function + /// `fixture_test() returns (address[] memory)` function /// define an array of addresses to be used for fuzzed `test` named parameter in scope of the /// current test. fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { // collect test fixtures param:array of values let mut fixtures = HashMap::new(); self.contract.functions().for_each(|func| { - if func.name.is_fixtures() { + if func.name.is_fixture() { if let Ok(CallResult { raw: _, decoded_result }) = self.executor.call(CALLER, address, func, &[], U256::ZERO, None) { fixtures.insert( - func.name.strip_prefix("fixtures_").unwrap().to_string(), + func.name.strip_prefix("fixture_").unwrap().to_string(), decoded_result, ); } diff --git a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol index 9ff21ff209c1..9fa63ff291d2 100644 --- a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol @@ -81,11 +81,11 @@ contract InvariantCalldataDictionary is DSTest { return targets; } - function fixtures_sender() external returns (address[] memory) { + function fixture_sender() external returns (address[] memory) { return actors; } - function fixtures_candidate() external returns (address[] memory) { + function fixture_candidate() external returns (address[] memory) { return actors; } From d0442de448876d84f95338f76d8ff0bd94b7f861 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 20 Mar 2024 09:17:10 +0200 Subject: [PATCH 05/22] Update tests to reflect that random values are used if no fixtures --- crates/evm/fuzz/src/strategies/calldata.rs | 44 ++++++++++++++++++++++ testdata/default/fuzz/FuzzInt.t.sol | 17 +++++---- testdata/default/fuzz/FuzzUint.t.sol | 11 +++--- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/crates/evm/fuzz/src/strategies/calldata.rs b/crates/evm/fuzz/src/strategies/calldata.rs index 43c7c5e42fcc..b6f88b5d5728 100644 --- a/crates/evm/fuzz/src/strategies/calldata.rs +++ b/crates/evm/fuzz/src/strategies/calldata.rs @@ -30,3 +30,47 @@ pub fn fuzz_calldata(func: Function, fuzz_fixtures: &FuzzFixtures) -> impl Strat .into() }) } + +#[cfg(test)] +mod tests { + use crate::{strategies::fuzz_calldata, FuzzFixtures}; + use alloy_dyn_abi::{DynSolValue, JsonAbiExt}; + use alloy_json_abi::Function; + use alloy_primitives::{Address, I256}; + use proptest::prelude::Strategy; + use std::collections::HashMap; + + #[test] + fn can_fuzz_with_fixtures() { + let function = Function::parse("test_fuzzed_address(address addressFixture)").unwrap(); + + let address_fixture = DynSolValue::Address(Address::random()); + let mut fixtures = HashMap::new(); + fixtures.insert( + "addressFixture".to_string(), + DynSolValue::Array(vec![address_fixture.clone()]), + ); + + let expected = function.abi_encode_input(&[address_fixture]).unwrap(); + let strategy = fuzz_calldata(function, &FuzzFixtures::new(fixtures)); + let _ = strategy.prop_map(move |fuzzed| { + assert_eq!(expected, fuzzed); + }); + } + + #[test] + fn can_fuzz_with_wrong_fixtures_type() { + let function = Function::parse("test_fuzzed_uint256(uint256 uintFixture)").unwrap(); + + let mut fixtures = HashMap::new(); + fixtures.insert( + "uintFixture".to_string(), + DynSolValue::Array(vec![DynSolValue::Int(I256::MAX, 8)]), + ); + + let strategy = fuzz_calldata(function, &FuzzFixtures::new(fixtures)); + let _ = strategy.prop_map(move |fuzzed| { + assert!(fuzzed.len() > 0); + }); + } +} diff --git a/testdata/default/fuzz/FuzzInt.t.sol b/testdata/default/fuzz/FuzzInt.t.sol index aac0825dbb5d..071727f6da92 100644 --- a/testdata/default/fuzz/FuzzInt.t.sol +++ b/testdata/default/fuzz/FuzzInt.t.sol @@ -3,7 +3,8 @@ pragma solidity 0.8.18; import "ds-test/test.sol"; -// See https://github.com/foundry-rs/foundry/pull/735 for context +// https://github.com/foundry-rs/foundry/pull/735 behavior changed with https://github.com/foundry-rs/foundry/issues/3521 +// random values (instead edge cases) are generated if no fixtures defined contract FuzzNumbersTest is DSTest { function testPositive(int256) public { assertTrue(true); @@ -14,31 +15,31 @@ contract FuzzNumbersTest is DSTest { } function testNegative0(int256 val) public { - assertTrue(val != 0); + assertTrue(val == 0); } function testNegative1(int256 val) public { - assertTrue(val != -1); + assertTrue(val == -1); } function testNegative2(int128 val) public { - assertTrue(val != 1); + assertTrue(val == 1); } function testNegativeMax0(int256 val) public { - assertTrue(val != type(int256).max); + assertTrue(val == type(int256).max); } function testNegativeMax1(int256 val) public { - assertTrue(val != type(int256).max - 2); + assertTrue(val == type(int256).max - 2); } function testNegativeMin0(int256 val) public { - assertTrue(val != type(int256).min); + assertTrue(val == type(int256).min); } function testNegativeMin1(int256 val) public { - assertTrue(val != type(int256).min + 2); + assertTrue(val == type(int256).min + 2); } function testEquality(int256 x, int256 y) public { diff --git a/testdata/default/fuzz/FuzzUint.t.sol b/testdata/default/fuzz/FuzzUint.t.sol index 5ae90a57bba0..923c2980f2bc 100644 --- a/testdata/default/fuzz/FuzzUint.t.sol +++ b/testdata/default/fuzz/FuzzUint.t.sol @@ -3,7 +3,8 @@ pragma solidity 0.8.18; import "ds-test/test.sol"; -// See https://github.com/foundry-rs/foundry/pull/735 for context +// https://github.com/foundry-rs/foundry/pull/735 behavior changed with https://github.com/foundry-rs/foundry/issues/3521 +// random values (instead edge cases) are generated if no fixtures defined contract FuzzNumbersTest is DSTest { function testPositive(uint256) public { assertTrue(true); @@ -14,19 +15,19 @@ contract FuzzNumbersTest is DSTest { } function testNegative0(uint256 val) public { - assertTrue(val != 0); + assertTrue(val == 0); } function testNegative2(uint256 val) public { - assertTrue(val != 2); + assertTrue(val == 2); } function testNegative2Max(uint256 val) public { - assertTrue(val != type(uint256).max - 2); + assertTrue(val == type(uint256).max - 2); } function testNegativeMax(uint256 val) public { - assertTrue(val != type(uint256).max); + assertTrue(val == type(uint256).max); } function testEquality(uint256 x, uint256 y) public { From 7e4b14c25fec0e3705a9cdfa14cbbdbae2f17a58 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 21 Mar 2024 07:29:51 +0200 Subject: [PATCH 06/22] Review changes --- crates/evm/fuzz/src/lib.rs | 1 + crates/evm/fuzz/src/strategies/address.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index 7d7db2f4fff1..129335a37b8c 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -288,6 +288,7 @@ impl FuzzFixtures { Self { inner: Arc::new(fixtures) } } + /// Returns configured fixtures for `param_name` fuzzed parameter. pub fn param_fixtures(&self, param_name: &String) -> Option<&[DynSolValue]> { if let Some(param_fixtures) = self.inner.get(param_name) { param_fixtures.as_array() diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs index 344031e57cbe..1ad1eba60287 100644 --- a/crates/evm/fuzz/src/strategies/address.rs +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -18,7 +18,7 @@ use proptest::{ /// `function fixture_owner() public returns (address[] memory)`. /// Use `owner` named parameter in fuzzed test in order to create a custom strategy /// `function testFuzz_ownerAddress(address owner, uint amount)`. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct AddressStrategy {} impl AddressStrategy { From 972d993b443501121d2cec45a89ec8bd44191ea9 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 21 Mar 2024 07:57:41 +0200 Subject: [PATCH 07/22] Group fuzz_calldata and fuzz_calldata_from_state in calldata mod --- crates/evm/fuzz/src/strategies/calldata.rs | 30 ++++++++++++++++++++-- crates/evm/fuzz/src/strategies/mod.rs | 5 ++-- crates/evm/fuzz/src/strategies/state.rs | 29 +-------------------- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/crates/evm/fuzz/src/strategies/calldata.rs b/crates/evm/fuzz/src/strategies/calldata.rs index b6f88b5d5728..404df3eeb805 100644 --- a/crates/evm/fuzz/src/strategies/calldata.rs +++ b/crates/evm/fuzz/src/strategies/calldata.rs @@ -1,8 +1,11 @@ -use crate::{strategies::fuzz_param, FuzzFixtures}; +use crate::{ + strategies::{fuzz_param, fuzz_param_from_state, EvmFuzzState}, + FuzzFixtures, +}; use alloy_dyn_abi::JsonAbiExt; use alloy_json_abi::Function; use alloy_primitives::Bytes; -use proptest::prelude::Strategy; +use proptest::prelude::{BoxedStrategy, Strategy}; /// Given a function, it returns a strategy which generates valid calldata /// for that function's input types, following declared test fixtures. @@ -31,6 +34,29 @@ pub fn fuzz_calldata(func: Function, fuzz_fixtures: &FuzzFixtures) -> impl Strat }) } +/// Given a function and some state, it returns a strategy which generated valid calldata for the +/// given function's input types, based on state taken from the EVM. +pub fn fuzz_calldata_from_state(func: Function, state: &EvmFuzzState) -> BoxedStrategy { + let strats = func + .inputs + .iter() + .map(|input| fuzz_param_from_state(&input.selector_type().parse().unwrap(), state)) + .collect::>(); + strats + .prop_map(move |values| { + func.abi_encode_input(&values) + .unwrap_or_else(|_| { + panic!( + "Fuzzer generated invalid arguments for function `{}` with inputs {:?}: {:?}", + func.name, func.inputs, values + ) + }) + .into() + }) + .no_shrink() + .boxed() +} + #[cfg(test)] mod tests { use crate::{strategies::fuzz_calldata, FuzzFixtures}; diff --git a/crates/evm/fuzz/src/strategies/mod.rs b/crates/evm/fuzz/src/strategies/mod.rs index f6d7a4da760a..54946031ed57 100644 --- a/crates/evm/fuzz/src/strategies/mod.rs +++ b/crates/evm/fuzz/src/strategies/mod.rs @@ -11,12 +11,11 @@ mod param; pub use param::{fuzz_param, fuzz_param_from_state}; mod calldata; -pub use calldata::fuzz_calldata; +pub use calldata::{fuzz_calldata, fuzz_calldata_from_state}; mod state; pub use state::{ - build_initial_state, collect_created_contracts, collect_state_from_call, - fuzz_calldata_from_state, EvmFuzzState, + build_initial_state, collect_created_contracts, collect_state_from_call, EvmFuzzState, }; mod invariants; diff --git a/crates/evm/fuzz/src/strategies/state.rs b/crates/evm/fuzz/src/strategies/state.rs index 1cf1d37882f1..d8aed9c49df4 100644 --- a/crates/evm/fuzz/src/strategies/state.rs +++ b/crates/evm/fuzz/src/strategies/state.rs @@ -1,13 +1,9 @@ -use super::fuzz_param_from_state; use crate::invariant::{ArtifactFilters, FuzzRunIdentifiedContracts}; -use alloy_dyn_abi::JsonAbiExt; -use alloy_json_abi::Function; -use alloy_primitives::{Address, Bytes, Log, B256, U256}; +use alloy_primitives::{Address, Log, B256, U256}; use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact}; use foundry_config::FuzzDictionaryConfig; use foundry_evm_core::utils::StateChangeset; use parking_lot::RwLock; -use proptest::prelude::{BoxedStrategy, Strategy}; use revm::{ db::{CacheDB, DatabaseRef}, interpreter::opcode::{self, spec_opcode_gas}, @@ -64,29 +60,6 @@ impl FuzzDictionary { } } -/// Given a function and some state, it returns a strategy which generated valid calldata for the -/// given function's input types, based on state taken from the EVM. -pub fn fuzz_calldata_from_state(func: Function, state: &EvmFuzzState) -> BoxedStrategy { - let strats = func - .inputs - .iter() - .map(|input| fuzz_param_from_state(&input.selector_type().parse().unwrap(), state)) - .collect::>(); - strats - .prop_map(move |values| { - func.abi_encode_input(&values) - .unwrap_or_else(|_| { - panic!( - "Fuzzer generated invalid arguments for function `{}` with inputs {:?}: {:?}", - func.name, func.inputs, values - ) - }) - .into() - }) - .no_shrink() - .boxed() -} - /// Builds the initial [EvmFuzzState] from a database. pub fn build_initial_state( db: &CacheDB, From 708e4db7b497dd49f9cf9d1bff6c87475b51f5b5 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 21 Mar 2024 08:44:25 +0200 Subject: [PATCH 08/22] Review changes: remove unnecessary clones, nicer code to collect fixtures --- crates/forge/bin/cmd/test/mod.rs | 6 +++--- crates/forge/src/runner.rs | 20 +++++++++----------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index e0809be67dcd..8b25452c3949 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -180,8 +180,8 @@ impl TestArgs { let profiles = get_available_profiles(toml)?; let test_options: TestOptions = TestOptionsBuilder::default() - .fuzz(config.clone().fuzz) - .invariant(config.clone().invariant) + .fuzz(config.fuzz.clone()) + .invariant(config.invariant) .profiles(profiles) .build(&output, project_root)?; @@ -221,7 +221,7 @@ impl TestArgs { *test_pattern = Some(debug_test_pattern.clone()); } - let outcome = self.run_tests(runner, config.clone(), verbosity, &filter).await?; + let outcome = self.run_tests(runner, config, verbosity, &filter).await?; if should_debug { // There is only one test. diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index bbd521f5267c..5daead036fb7 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -207,18 +207,16 @@ impl<'a> ContractRunner<'a> { fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { // collect test fixtures param:array of values let mut fixtures = HashMap::new(); - self.contract.functions().for_each(|func| { - if func.name.is_fixture() { - if let Ok(CallResult { raw: _, decoded_result }) = - self.executor.call(CALLER, address, func, &[], U256::ZERO, None) - { - fixtures.insert( - func.name.strip_prefix("fixture_").unwrap().to_string(), - decoded_result, - ); - } + for func in self.contract.functions().filter(|f| f.name.is_fixture()) { + if let Ok(CallResult { raw: _, decoded_result }) = + self.executor.call(CALLER, address, func, &[], U256::ZERO, None) + { + fixtures.insert( + func.name.strip_prefix("fixture_").unwrap().to_string(), + decoded_result, + ); } - }); + } FuzzFixtures::new(fixtures) } From 927689303e288ad9a5f92315d8e919b48b96f0fb Mon Sep 17 00:00:00 2001 From: grandizzy Date: Sun, 24 Mar 2024 14:09:46 +0200 Subject: [PATCH 09/22] Add support for bytes and string fixtures, fixture strategy macro. Solidity test --- crates/evm/fuzz/src/strategies/address.rs | 38 +++----- crates/evm/fuzz/src/strategies/bytes.rs | 86 +++++++++++++++++++ crates/evm/fuzz/src/strategies/mod.rs | 33 +++++++ crates/evm/fuzz/src/strategies/param.rs | 21 ++--- crates/evm/fuzz/src/strategies/string.rs | 53 ++++++++++++ crates/forge/tests/it/invariant.rs | 30 +++++++ .../invariant/common/InvariantFixtures.t.sol | 86 +++++++++++++++++++ 7 files changed, 307 insertions(+), 40 deletions(-) create mode 100644 crates/evm/fuzz/src/strategies/bytes.rs create mode 100644 crates/evm/fuzz/src/strategies/string.rs create mode 100644 testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs index 1ad1eba60287..136a0694422d 100644 --- a/crates/evm/fuzz/src/strategies/address.rs +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -1,3 +1,4 @@ +use crate::strategies::fixture_strategy; use alloy_dyn_abi::DynSolValue; use alloy_primitives::Address; use proptest::{ @@ -24,30 +25,19 @@ pub struct AddressStrategy {} impl AddressStrategy { /// Create a new address strategy. pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { - if let Some(fixtures) = fixtures { - let address_fixtures: Vec = - fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); - let address_fixtures_len = address_fixtures.len(); - any::() - .prop_map(move |index| { - // Generate value tree from fixture. - // If fixture is not a valid address, raise error and generate random value. - let index = index.index(address_fixtures_len); - if let Some(addr_fixture) = address_fixtures.get(index) { - if let Some(addr_fixture) = addr_fixture.as_address() { - return DynSolValue::Address(addr_fixture); - } - } - error!( - "{:?} is not a valid address fixture, generate random value", - address_fixtures.get(index) - ); - DynSolValue::Address(Address::random()) - }) - .boxed() - } else { - // If no config for addresses dictionary then create unbounded addresses strategy. + let value_from_fixture = |fixture: Option<&DynSolValue>| { + if let Some(fixture) = fixture { + if let Some(fixture) = fixture.as_address() { + return DynSolValue::Address(fixture); + } + } + error!("{:?} is not a valid address fixture, generate random value", fixture); + DynSolValue::Address(Address::random()) + }; + fixture_strategy!( + fixtures, + value_from_fixture, any::
().prop_map(DynSolValue::Address).boxed() - } + ) } } diff --git a/crates/evm/fuzz/src/strategies/bytes.rs b/crates/evm/fuzz/src/strategies/bytes.rs new file mode 100644 index 000000000000..394c90d252ce --- /dev/null +++ b/crates/evm/fuzz/src/strategies/bytes.rs @@ -0,0 +1,86 @@ +use crate::strategies::fixture_strategy; +use alloy_dyn_abi::{DynSolType, DynSolValue}; +use alloy_primitives::B256; +use proptest::{ + arbitrary::any, + prelude::{prop, BoxedStrategy}, + strategy::Strategy, +}; + +/// The bytes strategy combines 2 different strategies: +/// 1. A random bytes strategy if no fixture defined for current parameter. +/// 2. A fixture based strategy if configured values for current parameters. +/// If fixture is not a valid type then an error is raised and test suite will continue to execute +/// with random values. +/// +/// +/// For example: +/// To define fixture for `backup` fuzzed parameter, return an array of possible values from +/// `function fixture_backup() external pure returns (bytes[] memory)`. +/// Use `backup` named parameter in fuzzed test in order to create a custom strategy +/// `function testFuzz_backupValue(bytes memory backup)`. +#[derive(Debug, Default)] +pub struct BytesStrategy {} + +impl BytesStrategy { + /// Create a new bytes strategy. + pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { + let value_from_fixture = |fixture: Option<&DynSolValue>| { + if let Some(fixture) = fixture { + if let Some(fixture) = fixture.as_bytes() { + return DynSolValue::Bytes(fixture.to_vec()); + } + } + error!("{:?} is not a valid bytes fixture, generate random value", fixture); + let random: [u8; 32] = rand::random(); + DynSolValue::Bytes(random.to_vec()) + }; + fixture_strategy!( + fixtures, + value_from_fixture, + DynSolValue::type_strategy(&DynSolType::Bytes).boxed() + ) + } +} + +/// The fixed bytes strategy combines 2 different strategies: +/// 1. A random fixed bytes strategy if no fixture defined for current parameter. +/// 2. A fixture based strategy if configured values for current parameters. +/// If fixture is not a valid type then an error is raised and test suite will continue to execute +/// with random values. +/// +/// +/// For example: +/// To define fixture for `key` fuzzed parameter, return an array of possible values from +/// `function fixture_key() external pure returns (bytes32[] memory)`. +/// Use `key` named parameter in fuzzed test in order to create a custom strategy +/// `function testFuzz_keyValue(bytes32 key)`. +#[derive(Debug, Default)] +pub struct FixedBytesStrategy {} + +impl FixedBytesStrategy { + /// Create a new fixed bytes strategy. + pub fn init(size: usize, fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { + let value_from_fixture = move |fixture: Option<&DynSolValue>| { + if let Some(fixture) = fixture { + if let Some(fixture) = fixture.as_fixed_bytes() { + if fixture.1 == size { + return DynSolValue::FixedBytes(B256::from_slice(fixture.0), fixture.1); + } + } + } + error!("{:?} is not a valid fixed bytes fixture, generate random value", fixture); + DynSolValue::FixedBytes(B256::random(), size) + }; + fixture_strategy!( + fixtures, + value_from_fixture, + any::() + .prop_map(move |mut v| { + v[size..].fill(0); + DynSolValue::FixedBytes(v, size) + }) + .boxed() + ) + } +} diff --git a/crates/evm/fuzz/src/strategies/mod.rs b/crates/evm/fuzz/src/strategies/mod.rs index 54946031ed57..bf9bd52ef6d9 100644 --- a/crates/evm/fuzz/src/strategies/mod.rs +++ b/crates/evm/fuzz/src/strategies/mod.rs @@ -1,6 +1,9 @@ mod address; pub use address::AddressStrategy; +mod bytes; +pub use bytes::{BytesStrategy, FixedBytesStrategy}; + mod int; pub use int::IntStrategy; @@ -18,5 +21,35 @@ pub use state::{ build_initial_state, collect_created_contracts, collect_state_from_call, EvmFuzzState, }; +mod string; +pub use string::StringStrategy; + mod invariants; pub use invariants::{fuzz_contract_with_calldata, invariant_strat, override_call_strat}; + +/// Macro to create strategy with fixtures. +/// 1. A default strategy if no fixture defined for current parameter. +/// 2. A fixture based strategy if configured values for current parameter. +/// If fixture is not a valid type then an error is raised and test suite will continue to execute +/// with random values. +macro_rules! fixture_strategy { + ($fixtures:ident, $value_from_fixture:expr, $default_strategy:expr) => { + if let Some(fixtures) = $fixtures { + let custom_fixtures: Vec = + fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); + let custom_fixtures_len = custom_fixtures.len(); + any::() + .prop_map(move |index| { + // Generate value tree from fixture. + // If fixture is not a valid type, raise error and generate random value. + let index = index.index(custom_fixtures_len); + $value_from_fixture(custom_fixtures.get(index)) + }) + .boxed() + } else { + return $default_strategy + } + }; +} + +pub(crate) use fixture_strategy; diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index 75d69a27ecd2..94281b5a36f2 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -33,23 +33,12 @@ pub fn fuzz_param( DynSolType::Uint(n @ 8..=256) => super::UintStrategy::new(n, fuzz_fixtures) .prop_map(move |x| DynSolValue::Uint(x, n)) .boxed(), - DynSolType::Function | DynSolType::Bool | DynSolType::Bytes => { - DynSolValue::type_strategy(param).boxed() + DynSolType::Function | DynSolType::Bool => DynSolValue::type_strategy(param).boxed(), + DynSolType::Bytes => super::BytesStrategy::init(fuzz_fixtures), + DynSolType::FixedBytes(size @ 1..=32) => { + super::FixedBytesStrategy::init(size, fuzz_fixtures) } - DynSolType::FixedBytes(size @ 1..=32) => any::() - .prop_map(move |mut v| { - v[size..].fill(0); - DynSolValue::FixedBytes(v, size) - }) - .boxed(), - DynSolType::String => DynSolValue::type_strategy(param) - .prop_map(move |value| { - DynSolValue::String( - value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), - ) - }) - .boxed(), - + DynSolType::String => super::StringStrategy::init(fuzz_fixtures), DynSolType::Tuple(ref params) => params .iter() .map(|p| fuzz_param(p, None)) diff --git a/crates/evm/fuzz/src/strategies/string.rs b/crates/evm/fuzz/src/strategies/string.rs new file mode 100644 index 000000000000..ab4a226d06e8 --- /dev/null +++ b/crates/evm/fuzz/src/strategies/string.rs @@ -0,0 +1,53 @@ +use crate::strategies::fixture_strategy; +use alloy_dyn_abi::{DynSolType, DynSolValue}; +use proptest::{ + arbitrary::any, + prelude::{prop, BoxedStrategy}, + strategy::Strategy, +}; +use rand::{distributions::Alphanumeric, thread_rng, Rng}; + +/// The address strategy combines 2 different strategies: +/// 1. A random addresses strategy if no fixture defined for current parameter. +/// 2. A fixture based strategy if configured values for current parameters. +/// If fixture is not a valid type then an error is raised and test suite will continue to execute +// with random strings. +/// +/// +/// For example: +/// To define fixture for `person` fuzzed parameter, return an array of possible values from +/// `function fixture_person() public returns (string[] memory)`. +/// Use `person` named parameter in fuzzed test in order to create a custom strategy +/// `function testFuzz_personValue(string memory person)`. +#[derive(Debug, Default)] +pub struct StringStrategy {} + +impl StringStrategy { + /// Create a new string strategy. + pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { + let value_from_fixture = |fixture: Option<&DynSolValue>| { + if let Some(fixture) = fixture { + if let Some(fixture) = fixture.as_str() { + return DynSolValue::String(fixture.to_string()); + } + } + error!("{:?} is not a valid string fixture, generate random value", fixture); + let mut rng = thread_rng(); + let string_len = rng.gen_range(0..128); + let random: String = + (&mut rng).sample_iter(Alphanumeric).map(char::from).take(string_len).collect(); + DynSolValue::String(random) + }; + fixture_strategy!( + fixtures, + value_from_fixture, + DynSolValue::type_strategy(&DynSolType::String) + .prop_map(move |value| { + DynSolValue::String( + value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), + ) + }) + .boxed() + ) + } +} diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 4c69604bfbcf..f5b68d5f45b7 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -151,6 +151,16 @@ async fn test_invariant() { "default/fuzz/invariant/common/InvariantAssume.t.sol:InvariantAssume", vec![("invariant_dummy()", true, None, None, None)], ), + ( + "default/fuzz/invariant/common/InvariantFixtures.t.sol:InvariantFixtures", + vec![( + "invariant_target_not_compromised()", + false, + Some("".into()), + None, + None, + )], + ), ]), ); } @@ -400,3 +410,23 @@ async fn test_invariant_assume_respects_restrictions() { )]), ); } + +#[tokio::test(flavor = "multi_thread")] +async fn test_invariant_fixtures() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantFixtures.t.sol"); + let mut runner = TEST_DATA_DEFAULT.runner(); + let results = runner.test_collect(&filter); + assert_multiple( + &results, + BTreeMap::from([( + "default/fuzz/invariant/common/InvariantFixtures.t.sol:InvariantFixtures", + vec![( + "invariant_target_not_compromised()", + false, + Some("".into()), + None, + None, + )], + )]), + ); +} diff --git a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol new file mode 100644 index 000000000000..495baec57c7d --- /dev/null +++ b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity ^0.8.0; + +import "ds-test/test.sol"; + +contract Target { + bool ownerFound; + bool amountFound; + bool magicFound; + bool keyFound; + bool backupFound; + bool extraStringFound; + + function fuzzWithFixtures( + address owner, + uint256 amount, + int32 magic, + bytes32 key, + bytes memory backup, + string memory extra + ) external { + if (owner == 0x6B175474E89094C44Da98b954EedeAC495271d0F) { + ownerFound = true; + } + if (amount == 1122334455) amountFound = true; + if (magic == -777) magicFound = true; + if (key == "abcd1234") keyFound = true; + if (keccak256(backup) == keccak256("qwerty1234")) backupFound = true; + if (keccak256(abi.encodePacked(extra)) == keccak256(abi.encodePacked("112233aabbccdd"))) { + extraStringFound = true; + } + } + + function isCompromised() public view returns (bool) { + return ownerFound && amountFound && magicFound && keyFound && backupFound && extraStringFound; + } +} + +/// Try to compromise target contract by finding all accepted values using fixtures. +contract InvariantFixtures is DSTest { + Target target; + + function setUp() public { + target = new Target(); + } + + function fixture_owner() external pure returns (address[] memory) { + address[] memory addressFixture = new address[](1); + addressFixture[0] = 0x6B175474E89094C44Da98b954EedeAC495271d0F; + return addressFixture; + } + + function fixture_amount() external pure returns (uint256[] memory) { + uint256[] memory amountFixture = new uint256[](1); + amountFixture[0] = 1122334455; + return amountFixture; + } + + function fixture_magic() external pure returns (int32[] memory) { + int32[] memory magicFixture = new int32[](1); + magicFixture[0] = -777; + return magicFixture; + } + + function fixture_key() external pure returns (bytes32[] memory) { + bytes32[] memory keyFixture = new bytes32[](1); + keyFixture[0] = "abcd1234"; + return keyFixture; + } + + function fixture_backup() external pure returns (bytes[] memory) { + bytes[] memory backupFixture = new bytes[](1); + backupFixture[0] = "qwerty1234"; + return backupFixture; + } + + function fixture_extra() external pure returns (string[] memory) { + string[] memory extraFixture = new string[](1); + extraFixture[0] = "112233aabbccdd"; + return extraFixture; + } + + function invariant_target_not_compromised() public { + assertEq(target.isCompromised(), false); + } +} From a5e8da0935a112b380ab330ec878c7d44ca16dd0 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Tue, 26 Mar 2024 07:30:26 +0200 Subject: [PATCH 10/22] Remove unnecessary clone --- crates/forge/bin/cmd/test/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 8b25452c3949..8362e6b4b246 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -202,7 +202,7 @@ impl TestArgs { let runner = MultiContractRunnerBuilder::default() .set_debug(should_debug) .initial_balance(evm_opts.initial_balance) - .evm_spec(config.clone().evm_spec_id()) + .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) .with_fork(evm_opts.get_fork(&config, env.clone())) .with_cheats_config(CheatsConfig::new(&config, evm_opts.clone(), None)) From 09ee66ef249393ef86c7eb1146bf685d1877a36c Mon Sep 17 00:00:00 2001 From: grandizzy Date: Tue, 26 Mar 2024 13:34:26 +0200 Subject: [PATCH 11/22] Use inline config --- crates/common/src/traits.rs | 15 ----- crates/config/src/inline/conf_parser.rs | 56 ++++++++++++++--- crates/config/src/inline/mod.rs | 30 ++++++++- crates/config/src/inline/natspec.rs | 4 +- crates/config/src/lib.rs | 5 +- crates/forge/src/lib.rs | 62 ++++++++++++------- crates/forge/src/runner.rs | 39 ++++++------ .../common/InvariantCalldataDictionary.t.sol | 6 +- .../invariant/common/InvariantFixtures.t.sol | 26 +++++--- 9 files changed, 160 insertions(+), 83 deletions(-) diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index a3e78dafd099..4232fb946dc3 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -33,9 +33,6 @@ pub trait TestFunctionExt { /// Returns whether this function is a `setUp` function. fn is_setup(&self) -> bool; - - /// Returns whether this function is a `fixture` function. - fn is_fixture(&self) -> bool; } impl TestFunctionExt for Function { @@ -59,10 +56,6 @@ impl TestFunctionExt for Function { fn is_setup(&self) -> bool { self.name.is_setup() } - - fn is_fixture(&self) -> bool { - self.name.is_fixture() - } } impl TestFunctionExt for String { @@ -85,10 +78,6 @@ impl TestFunctionExt for String { fn is_setup(&self) -> bool { self.as_str().is_setup() } - - fn is_fixture(&self) -> bool { - self.as_str().is_fixture() - } } impl TestFunctionExt for str { @@ -111,10 +100,6 @@ impl TestFunctionExt for str { fn is_setup(&self) -> bool { self.eq_ignore_ascii_case("setup") } - - fn is_fixture(&self) -> bool { - self.starts_with("fixture_") - } } /// An extension trait for `std::error::Error` for ABI encoding. diff --git a/crates/config/src/inline/conf_parser.rs b/crates/config/src/inline/conf_parser.rs index acad057ae0d7..f3b81a987ac5 100644 --- a/crates/config/src/inline/conf_parser.rs +++ b/crates/config/src/inline/conf_parser.rs @@ -1,10 +1,13 @@ use super::{remove_whitespaces, InlineConfigParserError}; -use crate::{inline::INLINE_CONFIG_PREFIX, InlineConfigError, NatSpec}; +use crate::{ + inline::{INLINE_CONFIG_FIXTURE_KEY, INLINE_CONFIG_PREFIX}, + InlineConfigError, NatSpec, +}; use regex::Regex; /// This trait is intended to parse configurations from /// structured text. Foundry users can annotate Solidity test functions, -/// providing special configs just for the execution of a specific test. +/// providing special configs and fixtures just for the execution of a specific test. /// /// An example: /// @@ -18,6 +21,10 @@ use regex::Regex; /// /// forge-config: ci.fuzz.runs = 10000 /// function test_ImportantFuzzTest(uint256 x) public {...} /// } +/// +/// /// forge-config: fixture +/// function x() public returns (uint256[] memory) {...} +/// } /// ``` pub trait InlineConfigParser where @@ -103,7 +110,16 @@ where } } -/// Checks if all configuration lines specified in `natspec` use a valid profile. +/// Type of inline config. +pub enum InlineConfigType { + /// Profile inline config. + Profile, + /// Fixture inline config. + Fixture, +} + +/// Checks if all configuration lines specified in `natspec` use a valid profile +/// or are test fixture configurations. /// /// i.e. Given available profiles /// ```rust @@ -111,8 +127,15 @@ where /// ``` /// A configuration like `forge-config: ciii.invariant.depth = 1` would result /// in an error. -pub fn validate_profiles(natspec: &NatSpec, profiles: &[String]) -> Result<(), InlineConfigError> { +/// A fixture can be set by using `forge-config: fixture` configuration. +pub fn validate_inline_config_type( + natspec: &NatSpec, + profiles: &[String], +) -> Result { for config in natspec.config_lines() { + if config.eq(&format!("{INLINE_CONFIG_PREFIX}:{INLINE_CONFIG_FIXTURE_KEY}")) { + return Ok(InlineConfigType::Fixture); + } if !profiles.iter().any(|p| config.starts_with(&format!("{INLINE_CONFIG_PREFIX}:{p}."))) { let err_line: String = natspec.debug_context(); let profiles = format!("{profiles:?}"); @@ -122,7 +145,7 @@ pub fn validate_profiles(natspec: &NatSpec, profiles: &[String]) -> Result<(), I })? } } - Ok(()) + Ok(InlineConfigType::Profile) } /// Tries to parse a `u32` from `value`. The `key` argument is used to give details @@ -139,7 +162,7 @@ pub fn parse_config_bool(key: String, value: String) -> Result = Lazy::new(|| { @@ -48,6 +52,28 @@ impl InlineConfig { } } +/// Represents per-test fixtures, declared inline +/// as structured comments in Solidity test files. This allows +/// setting data sets for specific fuzzed parameters in a solidity test. +#[derive(Clone, Debug, Default)] +pub struct InlineFixturesConfig { + /// Maps a test-contract to a set of test-fixtures. + configs: HashMap>, +} + +impl InlineFixturesConfig { + /// Records a function to be used as fixture for given contract. + /// The name of function should be the same as the name of fuzzed parameter. + pub fn add_fixture(&mut self, contract: String, fixture: String) { + self.configs.entry(contract).or_default().insert(fixture); + } + + /// Returns functions to be used as fixtures for given contract. + pub fn get_fixtures(&mut self, contract: String) -> Option<&HashSet> { + self.configs.get(&contract) + } +} + pub(crate) fn remove_whitespaces(s: &str) -> String { s.chars().filter(|c| !c.is_whitespace()).collect() } diff --git a/crates/config/src/inline/natspec.rs b/crates/config/src/inline/natspec.rs index 3b62d40cc1c2..8cec0d980953 100644 --- a/crates/config/src/inline/natspec.rs +++ b/crates/config/src/inline/natspec.rs @@ -189,13 +189,14 @@ impl SolangParser { } let Ok((pt, comments)) = solang_parser::parse(src, 0) else { return }; - let mut prev_end = 0; + for item in &pt.0 { let pt::SourceUnitPart::ContractDefinition(c) = item else { continue }; let Some(id) = c.name.as_ref() else { continue }; if id.name != contract_name { continue }; + let mut prev_end = c.loc.start(); for part in &c.parts { let pt::ContractPart::FunctionDefinition(f) = part else { continue }; let start = f.loc.start(); @@ -215,7 +216,6 @@ impl SolangParser { } prev_end = f.loc.end(); } - prev_end = c.loc.end(); } } } diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 0c53a6c14e3f..46aa5bbf1e12 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -95,7 +95,10 @@ use providers::remappings::RemappingsProvider; mod inline; use crate::etherscan::EtherscanEnvProvider; -pub use inline::{validate_profiles, InlineConfig, InlineConfigError, InlineConfigParser, NatSpec}; +pub use inline::{ + validate_inline_config_type, InlineConfig, InlineConfigError, InlineConfigParser, + InlineConfigType, InlineFixturesConfig, NatSpec, +}; /// Foundry configuration /// diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index d163f819cc62..d1f3138908b8 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -3,8 +3,8 @@ extern crate tracing; use foundry_compilers::ProjectCompileOutput; use foundry_config::{ - validate_profiles, Config, FuzzConfig, InlineConfig, InlineConfigError, InlineConfigParser, - InvariantConfig, NatSpec, + validate_inline_config_type, Config, FuzzConfig, InlineConfig, InlineConfigError, + InlineConfigParser, InlineConfigType, InlineFixturesConfig, InvariantConfig, NatSpec, }; use proptest::test_runner::{ FailurePersistence, FileFailurePersistence, RngAlgorithm, TestRng, TestRunner, @@ -40,6 +40,8 @@ pub struct TestOptions { pub inline_fuzz: InlineConfig, /// Contains per-test specific "invariant" configurations. pub inline_invariant: InlineConfig, + /// Contains per-test specific "fixture" configurations. + pub inline_fixtures: InlineFixturesConfig, } impl TestOptions { @@ -55,33 +57,45 @@ impl TestOptions { let natspecs: Vec = NatSpec::parse(output, root); let mut inline_invariant = InlineConfig::::default(); let mut inline_fuzz = InlineConfig::::default(); + let mut inline_fixtures = InlineFixturesConfig::default(); for natspec in natspecs { - // Perform general validation - validate_profiles(&natspec, &profiles)?; - FuzzConfig::validate_configs(&natspec)?; - InvariantConfig::validate_configs(&natspec)?; - - // Apply in-line configurations for the current profile - let configs: Vec = natspec.current_profile_configs().collect(); - let c: &str = &natspec.contract; - let f: &str = &natspec.function; - let line: String = natspec.debug_context(); - - match base_fuzz.try_merge(&configs) { - Ok(Some(conf)) => inline_fuzz.insert(c, f, conf), - Ok(None) => { /* No inline config found, do nothing */ } - Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, - } - - match base_invariant.try_merge(&configs) { - Ok(Some(conf)) => inline_invariant.insert(c, f, conf), - Ok(None) => { /* No inline config found, do nothing */ } - Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, + match validate_inline_config_type(&natspec, &profiles)? { + InlineConfigType::Fixture => { + inline_fixtures.add_fixture(natspec.contract, natspec.function); + } + InlineConfigType::Profile => { + FuzzConfig::validate_configs(&natspec)?; + InvariantConfig::validate_configs(&natspec)?; + + // Apply in-line configurations for the current profile + let configs: Vec = natspec.current_profile_configs().collect(); + let c: &str = &natspec.contract; + let f: &str = &natspec.function; + let line: String = natspec.debug_context(); + + match base_fuzz.try_merge(&configs) { + Ok(Some(conf)) => inline_fuzz.insert(c, f, conf), + Ok(None) => { /* No inline config found, do nothing */ } + Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, + } + + match base_invariant.try_merge(&configs) { + Ok(Some(conf)) => inline_invariant.insert(c, f, conf), + Ok(None) => { /* No inline config found, do nothing */ } + Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, + } + } } } - Ok(Self { fuzz: base_fuzz, invariant: base_invariant, inline_fuzz, inline_invariant }) + Ok(Self { + fuzz: base_fuzz, + invariant: base_invariant, + inline_fuzz, + inline_invariant, + inline_fixtures, + }) } /// Returns a "fuzz" test runner instance. Parameters are used to select tight scoped fuzz diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 5daead036fb7..a3f18754f886 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -12,7 +12,7 @@ use foundry_common::{ contracts::{ContractsByAddress, ContractsByArtifact}, TestFunctionExt, }; -use foundry_config::{FuzzConfig, InvariantConfig}; +use foundry_config::{FuzzConfig, InlineFixturesConfig, InvariantConfig}; use foundry_evm::{ constants::CALLER, coverage::HitMaps, @@ -85,14 +85,14 @@ impl<'a> ContractRunner<'a> { impl<'a> ContractRunner<'a> { /// Deploys the test contract inside the runner from the sending account, and optionally runs /// the `setUp` function on the test contract. - pub fn setup(&mut self, setup: bool) -> TestSetup { - match self._setup(setup) { + pub fn setup(&mut self, setup: bool, fixtures: &InlineFixturesConfig) -> TestSetup { + match self._setup(setup, fixtures) { Ok(setup) => setup, Err(err) => TestSetup::failed(err.to_string()), } } - fn _setup(&mut self, setup: bool) -> Result { + fn _setup(&mut self, setup: bool, fixtures: &InlineFixturesConfig) -> Result { trace!(?setup, "Setting test contract"); // We max out their balance so that they can deploy and make calls. @@ -181,7 +181,7 @@ impl<'a> ContractRunner<'a> { labeled_addresses, reason, coverage, - fuzz_fixtures: self.fuzz_fixtures(address), + fuzz_fixtures: self.fuzz_fixtures(address, fixtures), } } else { TestSetup::success( @@ -190,7 +190,7 @@ impl<'a> ContractRunner<'a> { traces, Default::default(), None, - self.fuzz_fixtures(address), + self.fuzz_fixtures(address, fixtures), ) }; @@ -204,20 +204,21 @@ impl<'a> ContractRunner<'a> { /// `fixture_test() returns (address[] memory)` function /// define an array of addresses to be used for fuzzed `test` named parameter in scope of the /// current test. - fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { - // collect test fixtures param:array of values - let mut fixtures = HashMap::new(); - for func in self.contract.functions().filter(|f| f.name.is_fixture()) { - if let Ok(CallResult { raw: _, decoded_result }) = - self.executor.call(CALLER, address, func, &[], U256::ZERO, None) - { - fixtures.insert( - func.name.strip_prefix("fixture_").unwrap().to_string(), - decoded_result, - ); + fn fuzz_fixtures(&mut self, address: Address, fixtures: &InlineFixturesConfig) -> FuzzFixtures { + match fixtures.to_owned().get_fixtures(self.name.to_string()) { + Some(functions) => { + let mut fixtures = HashMap::with_capacity(functions.len()); + for func in self.contract.functions().filter(|f| functions.contains(&f.name)) { + if let Ok(CallResult { raw: _, decoded_result }) = + self.executor.call(CALLER, address, func, &[], U256::ZERO, None) + { + fixtures.insert(func.name.clone(), decoded_result); + } + } + FuzzFixtures::new(fixtures) } + None => FuzzFixtures::default(), } - FuzzFixtures::new(fixtures) } /// Runs all tests for a contract whose names match the provided regular expression @@ -263,7 +264,7 @@ impl<'a> ContractRunner<'a> { if tmp_tracing { self.executor.set_tracing(true); } - let setup = self.setup(needs_setup); + let setup = self.setup(needs_setup, &test_options.inline_fixtures); if tmp_tracing { self.executor.set_tracing(false); } diff --git a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol index 9fa63ff291d2..b05baf49e0cd 100644 --- a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol @@ -81,11 +81,13 @@ contract InvariantCalldataDictionary is DSTest { return targets; } - function fixture_sender() external returns (address[] memory) { + /// forge-config: fixture + function sender() external returns (address[] memory) { return actors; } - function fixture_candidate() external returns (address[] memory) { + /// forge-config: fixture + function candidate() external returns (address[] memory) { return actors; } diff --git a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol index 495baec57c7d..4a0f7b62a3ea 100644 --- a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol @@ -12,17 +12,17 @@ contract Target { bool extraStringFound; function fuzzWithFixtures( - address owner, - uint256 amount, + address owner_, + uint256 _amount, int32 magic, bytes32 key, bytes memory backup, string memory extra ) external { - if (owner == 0x6B175474E89094C44Da98b954EedeAC495271d0F) { + if (owner_ == 0x6B175474E89094C44Da98b954EedeAC495271d0F) { ownerFound = true; } - if (amount == 1122334455) amountFound = true; + if (_amount == 1122334455) amountFound = true; if (magic == -777) magicFound = true; if (key == "abcd1234") keyFound = true; if (keccak256(backup) == keccak256("qwerty1234")) backupFound = true; @@ -44,37 +44,43 @@ contract InvariantFixtures is DSTest { target = new Target(); } - function fixture_owner() external pure returns (address[] memory) { + /// forge-config: fixture + function owner_() external pure returns (address[] memory) { address[] memory addressFixture = new address[](1); addressFixture[0] = 0x6B175474E89094C44Da98b954EedeAC495271d0F; return addressFixture; } - function fixture_amount() external pure returns (uint256[] memory) { + /// forge-config: fixture + function _amount() external pure returns (uint256[] memory) { uint256[] memory amountFixture = new uint256[](1); amountFixture[0] = 1122334455; return amountFixture; } - function fixture_magic() external pure returns (int32[] memory) { + /// forge-config: fixture + function magic() external pure returns (int32[] memory) { int32[] memory magicFixture = new int32[](1); magicFixture[0] = -777; return magicFixture; } - function fixture_key() external pure returns (bytes32[] memory) { + /// forge-config: fixture + function key() external pure returns (bytes32[] memory) { bytes32[] memory keyFixture = new bytes32[](1); keyFixture[0] = "abcd1234"; return keyFixture; } - function fixture_backup() external pure returns (bytes[] memory) { + /// forge-config: fixture + function backup() external pure returns (bytes[] memory) { bytes[] memory backupFixture = new bytes[](1); backupFixture[0] = "qwerty1234"; return backupFixture; } - function fixture_extra() external pure returns (string[] memory) { + /// forge-config: fixture + function extra() external pure returns (string[] memory) { string[] memory extraFixture = new string[](1); extraFixture[0] = "112233aabbccdd"; return extraFixture; From 4785f935da479df59595ca814d2126a8b1774103 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Tue, 26 Mar 2024 17:37:49 +0200 Subject: [PATCH 12/22] More robust invariant assume test - previously rejecting when param was 0 (vm.assume(param != 0)) that is param should have been fuzzed twice with 0 in a run - with fuzz input deduplication is now harder to occur, changed rejected if param is not 0 (vm.assume(param != 0)) and narrow down to one run and just 10 depth --- crates/forge/tests/it/invariant.rs | 2 ++ testdata/default/fuzz/invariant/common/InvariantAssume.t.sol | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index f5b68d5f45b7..c6bf3e940ffc 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -394,6 +394,8 @@ async fn test_invariant_assume_does_not_revert() { async fn test_invariant_assume_respects_restrictions() { let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantAssume.t.sol"); let mut runner = TEST_DATA_DEFAULT.runner(); + runner.test_options.invariant.runs = 1; + runner.test_options.invariant.depth = 10; runner.test_options.invariant.max_assume_rejects = 1; let results = runner.test_collect(&filter); assert_multiple( diff --git a/testdata/default/fuzz/invariant/common/InvariantAssume.t.sol b/testdata/default/fuzz/invariant/common/InvariantAssume.t.sol index 4ac0d085c180..9808a870f722 100644 --- a/testdata/default/fuzz/invariant/common/InvariantAssume.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantAssume.t.sol @@ -8,7 +8,7 @@ contract Handler is DSTest { Vm constant vm = Vm(HEVM_ADDRESS); function doSomething(uint256 param) public { - vm.assume(param != 0); + vm.assume(param == 0); } } From fb86084fe7e283b62deb0beeed36ac0a05d329fc Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 10 Apr 2024 16:49:53 +0300 Subject: [PATCH 13/22] Fixtures as storage arrays, remove inline config --- crates/common/src/traits.rs | 23 +++++ crates/config/src/inline/conf_parser.rs | 60 +++---------- crates/config/src/inline/mod.rs | 32 +------ crates/config/src/lib.rs | 5 +- crates/forge/src/lib.rs | 62 ++++++-------- crates/forge/src/runner.rs | 85 ++++++++++++++----- crates/forge/tests/it/invariant.rs | 2 + .../common/InvariantCalldataDictionary.t.sol | 2 - .../invariant/common/InvariantFixtures.t.sol | 29 +------ 9 files changed, 130 insertions(+), 170 deletions(-) diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index 4232fb946dc3..fa8449f72798 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -33,6 +33,9 @@ pub trait TestFunctionExt { /// Returns whether this function is a `setUp` function. fn is_setup(&self) -> bool; + + /// Returns whether this function is a invariant setup function. + fn is_invariant_target_setup(&self) -> bool; } impl TestFunctionExt for Function { @@ -56,6 +59,10 @@ impl TestFunctionExt for Function { fn is_setup(&self) -> bool { self.name.is_setup() } + + fn is_invariant_target_setup(&self) -> bool { + self.name.is_invariant_target_setup() + } } impl TestFunctionExt for String { @@ -78,6 +85,10 @@ impl TestFunctionExt for String { fn is_setup(&self) -> bool { self.as_str().is_setup() } + + fn is_invariant_target_setup(&self) -> bool { + self.as_str().is_invariant_target_setup() + } } impl TestFunctionExt for str { @@ -100,6 +111,18 @@ impl TestFunctionExt for str { fn is_setup(&self) -> bool { self.eq_ignore_ascii_case("setup") } + + fn is_invariant_target_setup(&self) -> bool { + self.eq_ignore_ascii_case("excludeArtifacts") || + self.eq_ignore_ascii_case("excludeContracts") || + self.eq_ignore_ascii_case("excludeSenders") || + self.eq_ignore_ascii_case("targetArtifacts") || + self.eq_ignore_ascii_case("targetArtifactSelectors") || + self.eq_ignore_ascii_case("targetContracts") || + self.eq_ignore_ascii_case("targetSelectors") || + self.eq_ignore_ascii_case("targetSenders") || + self.eq_ignore_ascii_case("targetInterfaces") + } } /// An extension trait for `std::error::Error` for ABI encoding. diff --git a/crates/config/src/inline/conf_parser.rs b/crates/config/src/inline/conf_parser.rs index f3b81a987ac5..1f6fca6c7ac5 100644 --- a/crates/config/src/inline/conf_parser.rs +++ b/crates/config/src/inline/conf_parser.rs @@ -1,13 +1,10 @@ use super::{remove_whitespaces, InlineConfigParserError}; -use crate::{ - inline::{INLINE_CONFIG_FIXTURE_KEY, INLINE_CONFIG_PREFIX}, - InlineConfigError, NatSpec, -}; +use crate::{inline::INLINE_CONFIG_PREFIX, InlineConfigError, NatSpec}; use regex::Regex; /// This trait is intended to parse configurations from /// structured text. Foundry users can annotate Solidity test functions, -/// providing special configs and fixtures just for the execution of a specific test. +/// providing special configs just for the execution of a specific test. /// /// An example: /// @@ -21,10 +18,6 @@ use regex::Regex; /// /// forge-config: ci.fuzz.runs = 10000 /// function test_ImportantFuzzTest(uint256 x) public {...} /// } -/// -/// /// forge-config: fixture -/// function x() public returns (uint256[] memory) {...} -/// } /// ``` pub trait InlineConfigParser where @@ -110,16 +103,7 @@ where } } -/// Type of inline config. -pub enum InlineConfigType { - /// Profile inline config. - Profile, - /// Fixture inline config. - Fixture, -} - -/// Checks if all configuration lines specified in `natspec` use a valid profile -/// or are test fixture configurations. +/// Checks if all configuration lines specified in `natspec` use a valid profile. /// /// i.e. Given available profiles /// ```rust @@ -127,15 +111,8 @@ pub enum InlineConfigType { /// ``` /// A configuration like `forge-config: ciii.invariant.depth = 1` would result /// in an error. -/// A fixture can be set by using `forge-config: fixture` configuration. -pub fn validate_inline_config_type( - natspec: &NatSpec, - profiles: &[String], -) -> Result { +pub fn validate_profiles(natspec: &NatSpec, profiles: &[String]) -> Result<(), InlineConfigError> { for config in natspec.config_lines() { - if config.eq(&format!("{INLINE_CONFIG_PREFIX}:{INLINE_CONFIG_FIXTURE_KEY}")) { - return Ok(InlineConfigType::Fixture); - } if !profiles.iter().any(|p| config.starts_with(&format!("{INLINE_CONFIG_PREFIX}:{p}."))) { let err_line: String = natspec.debug_context(); let profiles = format!("{profiles:?}"); @@ -145,7 +122,7 @@ pub fn validate_inline_config_type( })? } } - Ok(InlineConfigType::Profile) + Ok(()) } /// Tries to parse a `u32` from `value`. The `key` argument is used to give details @@ -162,7 +139,7 @@ pub fn parse_config_bool(key: String, value: String) -> Result = Lazy::new(|| { @@ -41,7 +37,7 @@ impl InlineConfig { } /// Inserts an inline configuration, for a test function. - /// Configuration is identified by the pair "contract", "function". + /// Configuration is identified by the pair "contract", "function". pub fn insert(&mut self, contract_id: C, fn_name: F, config: T) where C: Into, @@ -52,28 +48,6 @@ impl InlineConfig { } } -/// Represents per-test fixtures, declared inline -/// as structured comments in Solidity test files. This allows -/// setting data sets for specific fuzzed parameters in a solidity test. -#[derive(Clone, Debug, Default)] -pub struct InlineFixturesConfig { - /// Maps a test-contract to a set of test-fixtures. - configs: HashMap>, -} - -impl InlineFixturesConfig { - /// Records a function to be used as fixture for given contract. - /// The name of function should be the same as the name of fuzzed parameter. - pub fn add_fixture(&mut self, contract: String, fixture: String) { - self.configs.entry(contract).or_default().insert(fixture); - } - - /// Returns functions to be used as fixtures for given contract. - pub fn get_fixtures(&mut self, contract: String) -> Option<&HashSet> { - self.configs.get(&contract) - } -} - pub(crate) fn remove_whitespaces(s: &str) -> String { s.chars().filter(|c| !c.is_whitespace()).collect() } diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 9a0a22448631..51769ab314a0 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -95,10 +95,7 @@ use providers::remappings::RemappingsProvider; mod inline; use crate::etherscan::EtherscanEnvProvider; -pub use inline::{ - validate_inline_config_type, InlineConfig, InlineConfigError, InlineConfigParser, - InlineConfigType, InlineFixturesConfig, NatSpec, -}; +pub use inline::{validate_profiles, InlineConfig, InlineConfigError, InlineConfigParser, NatSpec}; /// Foundry configuration /// diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index 849420e4af53..98dc0aa4783e 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -3,8 +3,8 @@ extern crate tracing; use foundry_compilers::ProjectCompileOutput; use foundry_config::{ - validate_inline_config_type, Config, FuzzConfig, InlineConfig, InlineConfigError, - InlineConfigParser, InlineConfigType, InlineFixturesConfig, InvariantConfig, NatSpec, + validate_profiles, Config, FuzzConfig, InlineConfig, InlineConfigError, InlineConfigParser, + InvariantConfig, NatSpec, }; use proptest::test_runner::{ FailurePersistence, FileFailurePersistence, RngAlgorithm, TestRng, TestRunner, @@ -40,8 +40,6 @@ pub struct TestOptions { pub inline_fuzz: InlineConfig, /// Contains per-test specific "invariant" configurations. pub inline_invariant: InlineConfig, - /// Contains per-test specific "fixture" configurations. - pub inline_fixtures: InlineFixturesConfig, } impl TestOptions { @@ -57,45 +55,33 @@ impl TestOptions { let natspecs: Vec = NatSpec::parse(output, root); let mut inline_invariant = InlineConfig::::default(); let mut inline_fuzz = InlineConfig::::default(); - let mut inline_fixtures = InlineFixturesConfig::default(); for natspec in natspecs { - match validate_inline_config_type(&natspec, &profiles)? { - InlineConfigType::Fixture => { - inline_fixtures.add_fixture(natspec.contract, natspec.function); - } - InlineConfigType::Profile => { - FuzzConfig::validate_configs(&natspec)?; - InvariantConfig::validate_configs(&natspec)?; - - // Apply in-line configurations for the current profile - let configs: Vec = natspec.current_profile_configs().collect(); - let c: &str = &natspec.contract; - let f: &str = &natspec.function; - let line: String = natspec.debug_context(); - - match base_fuzz.try_merge(&configs) { - Ok(Some(conf)) => inline_fuzz.insert(c, f, conf), - Ok(None) => { /* No inline config found, do nothing */ } - Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, - } - - match base_invariant.try_merge(&configs) { - Ok(Some(conf)) => inline_invariant.insert(c, f, conf), - Ok(None) => { /* No inline config found, do nothing */ } - Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, - } - } + // Perform general validation + validate_profiles(&natspec, &profiles)?; + FuzzConfig::validate_configs(&natspec)?; + InvariantConfig::validate_configs(&natspec)?; + + // Apply in-line configurations for the current profile + let configs: Vec = natspec.current_profile_configs().collect(); + let c: &str = &natspec.contract; + let f: &str = &natspec.function; + let line: String = natspec.debug_context(); + + match base_fuzz.try_merge(&configs) { + Ok(Some(conf)) => inline_fuzz.insert(c, f, conf), + Ok(None) => { /* No inline config found, do nothing */ } + Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, + } + + match base_invariant.try_merge(&configs) { + Ok(Some(conf)) => inline_invariant.insert(c, f, conf), + Ok(None) => { /* No inline config found, do nothing */ } + Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, } } - Ok(Self { - fuzz: base_fuzz, - invariant: base_invariant, - inline_fuzz, - inline_invariant, - inline_fixtures, - }) + Ok(Self { fuzz: base_fuzz, invariant: base_invariant, inline_fuzz, inline_invariant }) } /// Returns a "fuzz" test runner instance. Parameters are used to select tight scoped fuzz diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 1240a99fbf4c..17268c2b6f74 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -5,6 +5,7 @@ use crate::{ result::{SuiteResult, TestKind, TestResult, TestSetup, TestStatus}, TestFilter, TestOptions, }; +use alloy_dyn_abi::DynSolValue; use alloy_json_abi::Function; use alloy_primitives::{Address, U256}; use eyre::Result; @@ -12,7 +13,7 @@ use foundry_common::{ contracts::{ContractsByAddress, ContractsByArtifact}, TestFunctionExt, }; -use foundry_config::{FuzzConfig, InlineFixturesConfig, InvariantConfig}; +use foundry_config::{FuzzConfig, InvariantConfig}; use foundry_evm::{ constants::CALLER, coverage::HitMaps, @@ -76,14 +77,14 @@ impl<'a> ContractRunner<'a> { impl<'a> ContractRunner<'a> { /// Deploys the test contract inside the runner from the sending account, and optionally runs /// the `setUp` function on the test contract. - pub fn setup(&mut self, setup: bool, fixtures: &InlineFixturesConfig) -> TestSetup { - match self._setup(setup, fixtures) { + pub fn setup(&mut self, setup: bool) -> TestSetup { + match self._setup(setup) { Ok(setup) => setup, Err(err) => TestSetup::failed(err.to_string()), } } - fn _setup(&mut self, setup: bool, fixtures: &InlineFixturesConfig) -> Result { + fn _setup(&mut self, setup: bool) -> Result { trace!(?setup, "Setting test contract"); // We max out their balance so that they can deploy and make calls. @@ -172,7 +173,7 @@ impl<'a> ContractRunner<'a> { labeled_addresses, reason, coverage, - fuzz_fixtures: self.fuzz_fixtures(address, fixtures), + fuzz_fixtures: self.fuzz_fixtures(address), } } else { TestSetup::success( @@ -181,35 +182,75 @@ impl<'a> ContractRunner<'a> { traces, Default::default(), None, - self.fuzz_fixtures(address, fixtures), + self.fuzz_fixtures(address), ) }; Ok(setup) } - /// Collect fixtures from test contract. Fixtures are functions prefixed with `fixture_` key - /// and followed by the name of the parameter. + /// Collect fixtures from test contract. /// - /// For example: - /// `fixture_test() returns (address[] memory)` function - /// define an array of addresses to be used for fuzzed `test` named parameter in scope of the + /// - as storage arrays defined in test contract + /// Fixtures can be defined: + /// - as functions by having inline fixture configuration and same name as the parameter to be + /// fuzzed + /// + /// For example, a storage variable declared as + /// `uint256[] public amount = [1, 2, 3];` + /// define an array of uint256 values to be used for fuzzing `amount` named parameter in scope + /// of the current test. + /// + /// For example, a function declared as + /// `function owner() public returns (address[] memory)` + /// define an array of addresses to be used for fuzzing `owner` named parameter in scope of the /// current test. - fn fuzz_fixtures(&mut self, address: Address, fixtures: &InlineFixturesConfig) -> FuzzFixtures { - match fixtures.to_owned().get_fixtures(self.name.to_string()) { - Some(functions) => { - let mut fixtures = HashMap::with_capacity(functions.len()); - for func in self.contract.abi.functions().filter(|f| functions.contains(&f.name)) { + fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { + let mut fixtures = HashMap::new(); + self.contract + .abi + .functions() + .filter(|func| { + !func.is_setup() && + !func.is_invariant_test() && + !func.is_invariant_target_setup() && + !func.is_test() + }) + .for_each(|func| { + if func.inputs.is_empty() { + // Read fixtures declared as functions. if let Ok(CallResult { raw: _, decoded_result }) = self.executor.call(CALLER, address, func, &[], U256::ZERO, None) { fixtures.insert(func.name.clone(), decoded_result); } - } - FuzzFixtures::new(fixtures) - } - None => FuzzFixtures::default(), - } + } else { + // For reading fixtures from storage arrays we collect values by calling the + // function with incremented indexes until there's an error. + let mut vals = Vec::new(); + let mut index = 0; + loop { + if let Ok(CallResult { raw: _, decoded_result }) = self.executor.call( + CALLER, + address, + func, + &[DynSolValue::Uint(U256::from(index), 256)], + U256::ZERO, + None, + ) { + vals.push(decoded_result); + } else { + // No result returned for this index, we reached the end of storage + // array or the function is not a valid fixture. + break; + } + index += 1; + } + fixtures.insert(func.name.clone(), DynSolValue::Array(vals)); + }; + }); + + FuzzFixtures::new(fixtures) } /// Runs all tests for a contract whose names match the provided regular expression @@ -256,7 +297,7 @@ impl<'a> ContractRunner<'a> { if tmp_tracing { self.executor.set_tracing(true); } - let setup = self.setup(needs_setup, &test_options.inline_fixtures); + let setup = self.setup(needs_setup); if tmp_tracing { self.executor.set_tracing(false); } diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 4423e231211d..e03272ea80ca 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -477,6 +477,8 @@ async fn test_invariant_fuzzed_selected_targets() { async fn test_invariant_fixtures() { let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantFixtures.t.sol"); let mut runner = TEST_DATA_DEFAULT.runner(); + runner.test_options.invariant.runs = 1; + runner.test_options.invariant.depth = 100; let results = runner.test_collect(&filter); assert_multiple( &results, diff --git a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol index b05baf49e0cd..fb484f2e829e 100644 --- a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol @@ -81,12 +81,10 @@ contract InvariantCalldataDictionary is DSTest { return targets; } - /// forge-config: fixture function sender() external returns (address[] memory) { return actors; } - /// forge-config: fixture function candidate() external returns (address[] memory) { return actors; } diff --git a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol index 4a0f7b62a3ea..58fce7e5ed8b 100644 --- a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol @@ -19,7 +19,7 @@ contract Target { bytes memory backup, string memory extra ) external { - if (owner_ == 0x6B175474E89094C44Da98b954EedeAC495271d0F) { + if (owner_ == address(0x6B175474E89094C44Da98b954EedeAC495271d0F)) { ownerFound = true; } if (_amount == 1122334455) amountFound = true; @@ -39,47 +39,26 @@ contract Target { /// Try to compromise target contract by finding all accepted values using fixtures. contract InvariantFixtures is DSTest { Target target; + address[] public owner_ = [address(0x6B175474E89094C44Da98b954EedeAC495271d0F)]; + uint256[] public _amount = [1, 2, 1122334455]; + int32[] public magic = [-777, 777]; function setUp() public { target = new Target(); } - /// forge-config: fixture - function owner_() external pure returns (address[] memory) { - address[] memory addressFixture = new address[](1); - addressFixture[0] = 0x6B175474E89094C44Da98b954EedeAC495271d0F; - return addressFixture; - } - - /// forge-config: fixture - function _amount() external pure returns (uint256[] memory) { - uint256[] memory amountFixture = new uint256[](1); - amountFixture[0] = 1122334455; - return amountFixture; - } - - /// forge-config: fixture - function magic() external pure returns (int32[] memory) { - int32[] memory magicFixture = new int32[](1); - magicFixture[0] = -777; - return magicFixture; - } - - /// forge-config: fixture function key() external pure returns (bytes32[] memory) { bytes32[] memory keyFixture = new bytes32[](1); keyFixture[0] = "abcd1234"; return keyFixture; } - /// forge-config: fixture function backup() external pure returns (bytes[] memory) { bytes[] memory backupFixture = new bytes[](1); backupFixture[0] = "qwerty1234"; return backupFixture; } - /// forge-config: fixture function extra() external pure returns (string[] memory) { string[] memory extraFixture = new string[](1); extraFixture[0] = "112233aabbccdd"; From 6874ead09a3175ea03fa83a76ca8901ce019d2f2 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 10 Apr 2024 20:28:13 +0300 Subject: [PATCH 14/22] Simplify code --- crates/evm/fuzz/src/strategies/address.rs | 6 ++---- crates/evm/fuzz/src/strategies/bytes.rs | 6 ++---- crates/evm/fuzz/src/strategies/string.rs | 6 ++---- crates/forge/src/runner.rs | 2 +- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs index 136a0694422d..ba05257af86e 100644 --- a/crates/evm/fuzz/src/strategies/address.rs +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -26,10 +26,8 @@ impl AddressStrategy { /// Create a new address strategy. pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { let value_from_fixture = |fixture: Option<&DynSolValue>| { - if let Some(fixture) = fixture { - if let Some(fixture) = fixture.as_address() { - return DynSolValue::Address(fixture); - } + if let Some(val @ DynSolValue::Address(_)) = fixture { + return val.clone() } error!("{:?} is not a valid address fixture, generate random value", fixture); DynSolValue::Address(Address::random()) diff --git a/crates/evm/fuzz/src/strategies/bytes.rs b/crates/evm/fuzz/src/strategies/bytes.rs index 394c90d252ce..583fbe543509 100644 --- a/crates/evm/fuzz/src/strategies/bytes.rs +++ b/crates/evm/fuzz/src/strategies/bytes.rs @@ -26,10 +26,8 @@ impl BytesStrategy { /// Create a new bytes strategy. pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { let value_from_fixture = |fixture: Option<&DynSolValue>| { - if let Some(fixture) = fixture { - if let Some(fixture) = fixture.as_bytes() { - return DynSolValue::Bytes(fixture.to_vec()); - } + if let Some(val @ DynSolValue::Bytes(_)) = fixture { + return val.clone() } error!("{:?} is not a valid bytes fixture, generate random value", fixture); let random: [u8; 32] = rand::random(); diff --git a/crates/evm/fuzz/src/strategies/string.rs b/crates/evm/fuzz/src/strategies/string.rs index ab4a226d06e8..53668adb56ec 100644 --- a/crates/evm/fuzz/src/strategies/string.rs +++ b/crates/evm/fuzz/src/strategies/string.rs @@ -26,10 +26,8 @@ impl StringStrategy { /// Create a new string strategy. pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { let value_from_fixture = |fixture: Option<&DynSolValue>| { - if let Some(fixture) = fixture { - if let Some(fixture) = fixture.as_str() { - return DynSolValue::String(fixture.to_string()); - } + if let Some(val @ DynSolValue::String(_)) = fixture { + return val.clone() } error!("{:?} is not a valid string fixture, generate random value", fixture); let mut rng = thread_rng(); diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 17268c2b6f74..e4d9bc84b1ec 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -203,7 +203,7 @@ impl<'a> ContractRunner<'a> { /// /// For example, a function declared as /// `function owner() public returns (address[] memory)` - /// define an array of addresses to be used for fuzzing `owner` named parameter in scope of the + /// returns an array of addresses to be used for fuzzing `owner` named parameter in scope of the /// current test. fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { let mut fixtures = HashMap::new(); From ca4d44b67d0a950670f09e94766a34750e1f537a Mon Sep 17 00:00:00 2001 From: grandizzy Date: Wed, 10 Apr 2024 21:07:44 +0300 Subject: [PATCH 15/22] Support fixed size arrays fixtures --- crates/evm/fuzz/src/lib.rs | 5 ++++- .../default/fuzz/invariant/common/InvariantFixtures.t.sol | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index 129335a37b8c..9ab25a3d7579 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -291,7 +291,10 @@ impl FuzzFixtures { /// Returns configured fixtures for `param_name` fuzzed parameter. pub fn param_fixtures(&self, param_name: &String) -> Option<&[DynSolValue]> { if let Some(param_fixtures) = self.inner.get(param_name) { - param_fixtures.as_array() + match param_fixtures { + DynSolValue::FixedArray(_) => param_fixtures.as_fixed_array(), + _ => param_fixtures.as_array(), + } } else { None } diff --git a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol index 58fce7e5ed8b..6cc83101de1e 100644 --- a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol @@ -41,12 +41,18 @@ contract InvariantFixtures is DSTest { Target target; address[] public owner_ = [address(0x6B175474E89094C44Da98b954EedeAC495271d0F)]; uint256[] public _amount = [1, 2, 1122334455]; - int32[] public magic = [-777, 777]; function setUp() public { target = new Target(); } + function magic() external returns (int32[2] memory) { + int32[2] memory magic; + magic[0] = -777; + magic[1] = 777; + return magic; + } + function key() external pure returns (bytes32[] memory) { bytes32[] memory keyFixture = new bytes32[](1); keyFixture[0] = "abcd1234"; From acdf92d08ff583723879938c90327dc7504a27f5 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 11 Apr 2024 09:10:50 +0300 Subject: [PATCH 16/22] Update comment --- crates/forge/src/runner.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index e4d9bc84b1ec..5b7c202999e0 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -191,18 +191,18 @@ impl<'a> ContractRunner<'a> { /// Collect fixtures from test contract. /// - /// - as storage arrays defined in test contract /// Fixtures can be defined: + /// - as storage arrays in test contract /// - as functions by having inline fixture configuration and same name as the parameter to be /// fuzzed /// - /// For example, a storage variable declared as + /// Storage array fixtures: /// `uint256[] public amount = [1, 2, 3];` /// define an array of uint256 values to be used for fuzzing `amount` named parameter in scope /// of the current test. /// - /// For example, a function declared as - /// `function owner() public returns (address[] memory)` + /// Function fixtures: + /// `function owner() public returns (address[] memory){}` /// returns an array of addresses to be used for fuzzing `owner` named parameter in scope of the /// current test. fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { From 12115fa8b98c3b622febca300dc85e6644d2b03f Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 11 Apr 2024 11:29:46 +0300 Subject: [PATCH 17/22] Use DynSolValue::type_strategy for address and fixed bytes fuzzed params --- crates/evm/fuzz/src/strategies/address.rs | 4 ++-- crates/evm/fuzz/src/strategies/bytes.rs | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs index ba05257af86e..5885d48effd0 100644 --- a/crates/evm/fuzz/src/strategies/address.rs +++ b/crates/evm/fuzz/src/strategies/address.rs @@ -1,5 +1,5 @@ use crate::strategies::fixture_strategy; -use alloy_dyn_abi::DynSolValue; +use alloy_dyn_abi::{DynSolType, DynSolValue}; use alloy_primitives::Address; use proptest::{ arbitrary::any, @@ -35,7 +35,7 @@ impl AddressStrategy { fixture_strategy!( fixtures, value_from_fixture, - any::
().prop_map(DynSolValue::Address).boxed() + DynSolValue::type_strategy(&DynSolType::Address).boxed() ) } } diff --git a/crates/evm/fuzz/src/strategies/bytes.rs b/crates/evm/fuzz/src/strategies/bytes.rs index 583fbe543509..2559d372447d 100644 --- a/crates/evm/fuzz/src/strategies/bytes.rs +++ b/crates/evm/fuzz/src/strategies/bytes.rs @@ -73,12 +73,7 @@ impl FixedBytesStrategy { fixture_strategy!( fixtures, value_from_fixture, - any::() - .prop_map(move |mut v| { - v[size..].fill(0); - DynSolValue::FixedBytes(v, size) - }) - .boxed() + DynSolValue::type_strategy(&DynSolType::FixedBytes(size)).boxed() ) } } From e76fe8e5ac539ab5631e0842fd0bee7fffc45f09 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 11 Apr 2024 17:14:52 +0300 Subject: [PATCH 18/22] Add prefix to mark a storage array or a function as fixture --- crates/common/src/traits.rs | 24 ++---- crates/evm/fuzz/src/lib.rs | 15 +++- crates/forge/src/runner.rs | 83 +++++++++---------- .../invariant/common/InvariantFixtures.t.sol | 12 +-- 4 files changed, 64 insertions(+), 70 deletions(-) diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index fa8449f72798..8ed1edbec304 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -34,8 +34,8 @@ pub trait TestFunctionExt { /// Returns whether this function is a `setUp` function. fn is_setup(&self) -> bool; - /// Returns whether this function is a invariant setup function. - fn is_invariant_target_setup(&self) -> bool; + /// Returns whether this function is a fixture function. + fn is_fixture(&self) -> bool; } impl TestFunctionExt for Function { @@ -60,8 +60,8 @@ impl TestFunctionExt for Function { self.name.is_setup() } - fn is_invariant_target_setup(&self) -> bool { - self.name.is_invariant_target_setup() + fn is_fixture(&self) -> bool { + self.name.is_fixture() } } @@ -86,8 +86,8 @@ impl TestFunctionExt for String { self.as_str().is_setup() } - fn is_invariant_target_setup(&self) -> bool { - self.as_str().is_invariant_target_setup() + fn is_fixture(&self) -> bool { + self.as_str().is_fixture() } } @@ -112,16 +112,8 @@ impl TestFunctionExt for str { self.eq_ignore_ascii_case("setup") } - fn is_invariant_target_setup(&self) -> bool { - self.eq_ignore_ascii_case("excludeArtifacts") || - self.eq_ignore_ascii_case("excludeContracts") || - self.eq_ignore_ascii_case("excludeSenders") || - self.eq_ignore_ascii_case("targetArtifacts") || - self.eq_ignore_ascii_case("targetArtifactSelectors") || - self.eq_ignore_ascii_case("targetContracts") || - self.eq_ignore_ascii_case("targetSelectors") || - self.eq_ignore_ascii_case("targetSenders") || - self.eq_ignore_ascii_case("targetInterfaces") + fn is_fixture(&self) -> bool { + self.starts_with("fixture") } } diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index 9ab25a3d7579..b2a058e5bb02 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -289,8 +289,8 @@ impl FuzzFixtures { } /// Returns configured fixtures for `param_name` fuzzed parameter. - pub fn param_fixtures(&self, param_name: &String) -> Option<&[DynSolValue]> { - if let Some(param_fixtures) = self.inner.get(param_name) { + pub fn param_fixtures(&self, param_name: &str) -> Option<&[DynSolValue]> { + if let Some(param_fixtures) = self.inner.get(&normalize_fixture(param_name)) { match param_fixtures { DynSolValue::FixedArray(_) => param_fixtures.as_fixed_array(), _ => param_fixtures.as_array(), @@ -300,3 +300,14 @@ impl FuzzFixtures { } } } + +/// Extracts fixture name from a function name. +/// For example: fixtures defined in `fixture_Owner` function will be applied for `owner` parameter. +pub fn fixture_name(function_name: String) -> String { + normalize_fixture(function_name.strip_prefix("fixture").unwrap()) +} + +/// Normalize fixture parameter name, for example `_Owner` to `owner`. +fn normalize_fixture(param_name: &str) -> String { + param_name.trim_matches(&['_']).to_ascii_lowercase() +} diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 5b7c202999e0..940f6afa43d9 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -23,7 +23,7 @@ use foundry_evm::{ invariant::{replay_run, InvariantExecutor, InvariantFuzzError, InvariantFuzzTestResult}, CallResult, EvmError, ExecutionErr, Executor, RawCallResult, }, - fuzz::{invariant::InvariantContract, CounterExample, FuzzFixtures}, + fuzz::{fixture_name, invariant::InvariantContract, CounterExample, FuzzFixtures}, traces::{load_contracts, TraceKind}, }; use proptest::test_runner::TestRunner; @@ -192,63 +192,54 @@ impl<'a> ContractRunner<'a> { /// Collect fixtures from test contract. /// /// Fixtures can be defined: - /// - as storage arrays in test contract - /// - as functions by having inline fixture configuration and same name as the parameter to be + /// - as storage arrays in test contract, prefixed with `fixture` + /// - as functions prefixed with `fixture` and followed by parameter name to be /// fuzzed /// /// Storage array fixtures: - /// `uint256[] public amount = [1, 2, 3];` + /// `uint256[] public fixture_amount = [1, 2, 3];` /// define an array of uint256 values to be used for fuzzing `amount` named parameter in scope /// of the current test. /// /// Function fixtures: - /// `function owner() public returns (address[] memory){}` + /// `function fixture_owner() public returns (address[] memory){}` /// returns an array of addresses to be used for fuzzing `owner` named parameter in scope of the /// current test. fn fuzz_fixtures(&mut self, address: Address) -> FuzzFixtures { let mut fixtures = HashMap::new(); - self.contract - .abi - .functions() - .filter(|func| { - !func.is_setup() && - !func.is_invariant_test() && - !func.is_invariant_target_setup() && - !func.is_test() - }) - .for_each(|func| { - if func.inputs.is_empty() { - // Read fixtures declared as functions. - if let Ok(CallResult { raw: _, decoded_result }) = - self.executor.call(CALLER, address, func, &[], U256::ZERO, None) - { - fixtures.insert(func.name.clone(), decoded_result); - } - } else { - // For reading fixtures from storage arrays we collect values by calling the - // function with incremented indexes until there's an error. - let mut vals = Vec::new(); - let mut index = 0; - loop { - if let Ok(CallResult { raw: _, decoded_result }) = self.executor.call( - CALLER, - address, - func, - &[DynSolValue::Uint(U256::from(index), 256)], - U256::ZERO, - None, - ) { - vals.push(decoded_result); - } else { - // No result returned for this index, we reached the end of storage - // array or the function is not a valid fixture. - break; - } - index += 1; + self.contract.abi.functions().filter(|func| func.is_fixture()).for_each(|func| { + if func.inputs.is_empty() { + // Read fixtures declared as functions. + if let Ok(CallResult { raw: _, decoded_result }) = + self.executor.call(CALLER, address, func, &[], U256::ZERO, None) + { + fixtures.insert(fixture_name(func.name.clone()), decoded_result); + } + } else { + // For reading fixtures from storage arrays we collect values by calling the + // function with incremented indexes until there's an error. + let mut vals = Vec::new(); + let mut index = 0; + loop { + if let Ok(CallResult { raw: _, decoded_result }) = self.executor.call( + CALLER, + address, + func, + &[DynSolValue::Uint(U256::from(index), 256)], + U256::ZERO, + None, + ) { + vals.push(decoded_result); + } else { + // No result returned for this index, we reached the end of storage + // array or the function is not a valid fixture. + break; } - fixtures.insert(func.name.clone(), DynSolValue::Array(vals)); - }; - }); + index += 1; + } + fixtures.insert(fixture_name(func.name.clone()), DynSolValue::Array(vals)); + }; + }); FuzzFixtures::new(fixtures) } diff --git a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol index 6cc83101de1e..b3f1e17cb249 100644 --- a/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantFixtures.t.sol @@ -39,33 +39,33 @@ contract Target { /// Try to compromise target contract by finding all accepted values using fixtures. contract InvariantFixtures is DSTest { Target target; - address[] public owner_ = [address(0x6B175474E89094C44Da98b954EedeAC495271d0F)]; - uint256[] public _amount = [1, 2, 1122334455]; + address[] public fixture_owner_ = [address(0x6B175474E89094C44Da98b954EedeAC495271d0F)]; + uint256[] public fixture_amount = [1, 2, 1122334455]; function setUp() public { target = new Target(); } - function magic() external returns (int32[2] memory) { + function fixtureMagic() external returns (int32[2] memory) { int32[2] memory magic; magic[0] = -777; magic[1] = 777; return magic; } - function key() external pure returns (bytes32[] memory) { + function fixtureKey() external pure returns (bytes32[] memory) { bytes32[] memory keyFixture = new bytes32[](1); keyFixture[0] = "abcd1234"; return keyFixture; } - function backup() external pure returns (bytes[] memory) { + function fixtureBackup() external pure returns (bytes[] memory) { bytes[] memory backupFixture = new bytes[](1); backupFixture[0] = "qwerty1234"; return backupFixture; } - function extra() external pure returns (string[] memory) { + function fixtureExtra() external pure returns (string[] memory) { string[] memory extraFixture = new string[](1); extraFixture[0] = "112233aabbccdd"; return extraFixture; From 3ac9e332299afa10d31bb30f107d67b0123da900 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 11 Apr 2024 19:33:11 +0300 Subject: [PATCH 19/22] Fix test --- .../fuzz/invariant/common/InvariantCalldataDictionary.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol index fb484f2e829e..5387b020d37d 100644 --- a/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol +++ b/testdata/default/fuzz/invariant/common/InvariantCalldataDictionary.t.sol @@ -81,11 +81,11 @@ contract InvariantCalldataDictionary is DSTest { return targets; } - function sender() external returns (address[] memory) { + function fixtureSender() external returns (address[] memory) { return actors; } - function candidate() external returns (address[] memory) { + function fixtureCandidate() external returns (address[] memory) { return actors; } From 7e95208a40d5728af30948bc40755599f45f3a40 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 11 Apr 2024 19:45:15 +0300 Subject: [PATCH 20/22] Simplify code / fixture strategy macro, panic if configured fixture not of param type --- crates/evm/fuzz/src/strategies/address.rs | 41 ------------ crates/evm/fuzz/src/strategies/bytes.rs | 79 ----------------------- crates/evm/fuzz/src/strategies/mod.rs | 18 +----- crates/evm/fuzz/src/strategies/param.rs | 29 +++++++-- crates/evm/fuzz/src/strategies/string.rs | 51 --------------- 5 files changed, 25 insertions(+), 193 deletions(-) delete mode 100644 crates/evm/fuzz/src/strategies/address.rs delete mode 100644 crates/evm/fuzz/src/strategies/bytes.rs delete mode 100644 crates/evm/fuzz/src/strategies/string.rs diff --git a/crates/evm/fuzz/src/strategies/address.rs b/crates/evm/fuzz/src/strategies/address.rs deleted file mode 100644 index 5885d48effd0..000000000000 --- a/crates/evm/fuzz/src/strategies/address.rs +++ /dev/null @@ -1,41 +0,0 @@ -use crate::strategies::fixture_strategy; -use alloy_dyn_abi::{DynSolType, DynSolValue}; -use alloy_primitives::Address; -use proptest::{ - arbitrary::any, - prelude::{prop, BoxedStrategy}, - strategy::Strategy, -}; - -/// The address strategy combines 2 different strategies: -/// 1. A random addresses strategy if no fixture defined for current parameter. -/// 2. A fixture based strategy if configured values for current parameters. -/// If fixture is not a valid type then an error is raised and test suite will continue to execute -/// with random address. -/// -/// -/// For example: -/// To define fixture for `owner` fuzzed parameter, return an array of possible values from -/// `function fixture_owner() public returns (address[] memory)`. -/// Use `owner` named parameter in fuzzed test in order to create a custom strategy -/// `function testFuzz_ownerAddress(address owner, uint amount)`. -#[derive(Debug, Default)] -pub struct AddressStrategy {} - -impl AddressStrategy { - /// Create a new address strategy. - pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { - let value_from_fixture = |fixture: Option<&DynSolValue>| { - if let Some(val @ DynSolValue::Address(_)) = fixture { - return val.clone() - } - error!("{:?} is not a valid address fixture, generate random value", fixture); - DynSolValue::Address(Address::random()) - }; - fixture_strategy!( - fixtures, - value_from_fixture, - DynSolValue::type_strategy(&DynSolType::Address).boxed() - ) - } -} diff --git a/crates/evm/fuzz/src/strategies/bytes.rs b/crates/evm/fuzz/src/strategies/bytes.rs deleted file mode 100644 index 2559d372447d..000000000000 --- a/crates/evm/fuzz/src/strategies/bytes.rs +++ /dev/null @@ -1,79 +0,0 @@ -use crate::strategies::fixture_strategy; -use alloy_dyn_abi::{DynSolType, DynSolValue}; -use alloy_primitives::B256; -use proptest::{ - arbitrary::any, - prelude::{prop, BoxedStrategy}, - strategy::Strategy, -}; - -/// The bytes strategy combines 2 different strategies: -/// 1. A random bytes strategy if no fixture defined for current parameter. -/// 2. A fixture based strategy if configured values for current parameters. -/// If fixture is not a valid type then an error is raised and test suite will continue to execute -/// with random values. -/// -/// -/// For example: -/// To define fixture for `backup` fuzzed parameter, return an array of possible values from -/// `function fixture_backup() external pure returns (bytes[] memory)`. -/// Use `backup` named parameter in fuzzed test in order to create a custom strategy -/// `function testFuzz_backupValue(bytes memory backup)`. -#[derive(Debug, Default)] -pub struct BytesStrategy {} - -impl BytesStrategy { - /// Create a new bytes strategy. - pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { - let value_from_fixture = |fixture: Option<&DynSolValue>| { - if let Some(val @ DynSolValue::Bytes(_)) = fixture { - return val.clone() - } - error!("{:?} is not a valid bytes fixture, generate random value", fixture); - let random: [u8; 32] = rand::random(); - DynSolValue::Bytes(random.to_vec()) - }; - fixture_strategy!( - fixtures, - value_from_fixture, - DynSolValue::type_strategy(&DynSolType::Bytes).boxed() - ) - } -} - -/// The fixed bytes strategy combines 2 different strategies: -/// 1. A random fixed bytes strategy if no fixture defined for current parameter. -/// 2. A fixture based strategy if configured values for current parameters. -/// If fixture is not a valid type then an error is raised and test suite will continue to execute -/// with random values. -/// -/// -/// For example: -/// To define fixture for `key` fuzzed parameter, return an array of possible values from -/// `function fixture_key() external pure returns (bytes32[] memory)`. -/// Use `key` named parameter in fuzzed test in order to create a custom strategy -/// `function testFuzz_keyValue(bytes32 key)`. -#[derive(Debug, Default)] -pub struct FixedBytesStrategy {} - -impl FixedBytesStrategy { - /// Create a new fixed bytes strategy. - pub fn init(size: usize, fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { - let value_from_fixture = move |fixture: Option<&DynSolValue>| { - if let Some(fixture) = fixture { - if let Some(fixture) = fixture.as_fixed_bytes() { - if fixture.1 == size { - return DynSolValue::FixedBytes(B256::from_slice(fixture.0), fixture.1); - } - } - } - error!("{:?} is not a valid fixed bytes fixture, generate random value", fixture); - DynSolValue::FixedBytes(B256::random(), size) - }; - fixture_strategy!( - fixtures, - value_from_fixture, - DynSolValue::type_strategy(&DynSolType::FixedBytes(size)).boxed() - ) - } -} diff --git a/crates/evm/fuzz/src/strategies/mod.rs b/crates/evm/fuzz/src/strategies/mod.rs index 8648e6ad6c11..56a022b2a370 100644 --- a/crates/evm/fuzz/src/strategies/mod.rs +++ b/crates/evm/fuzz/src/strategies/mod.rs @@ -1,9 +1,3 @@ -mod address; -pub use address::AddressStrategy; - -mod bytes; -pub use bytes::{BytesStrategy, FixedBytesStrategy}; - mod int; pub use int::IntStrategy; @@ -19,29 +13,23 @@ pub use calldata::{fuzz_calldata, fuzz_calldata_from_state}; mod state; pub use state::{build_initial_state, collect_created_contracts, EvmFuzzState}; -mod string; -pub use string::StringStrategy; - mod invariants; pub use invariants::{fuzz_contract_with_calldata, invariant_strat, override_call_strat}; /// Macro to create strategy with fixtures. /// 1. A default strategy if no fixture defined for current parameter. /// 2. A fixture based strategy if configured values for current parameter. -/// If fixture is not a valid type then an error is raised and test suite will continue to execute -/// with random values. +/// If fixture is not a valid type then fuzzer will panic. macro_rules! fixture_strategy { - ($fixtures:ident, $value_from_fixture:expr, $default_strategy:expr) => { + ($fixtures:ident, $default_strategy:expr) => { if let Some(fixtures) = $fixtures { let custom_fixtures: Vec = fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); let custom_fixtures_len = custom_fixtures.len(); any::() .prop_map(move |index| { - // Generate value tree from fixture. - // If fixture is not a valid type, raise error and generate random value. let index = index.index(custom_fixtures_len); - $value_from_fixture(custom_fixtures.get(index)) + custom_fixtures.get(index).unwrap().clone() }) .boxed() } else { diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index 42eefc525707..f0f7339ac99b 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -1,4 +1,5 @@ use super::state::EvmFuzzState; +use crate::strategies::fixture_strategy; use alloy_dyn_abi::{DynSolType, DynSolValue}; use alloy_primitives::{Address, B256, I256, U256}; use proptest::prelude::*; @@ -17,8 +18,7 @@ const MAX_ARRAY_LEN: usize = 256; /// `fixture_owner` function can be used in a fuzzed test function with a signature like /// `function testFuzz_ownerAddress(address owner, uint amount)`. /// -/// If the type of fixtures is different than the parameter type then error is raised and a random -/// value is generated. +/// Fuzzer will panic if the type of fixtures is different than the parameter type. /// /// Works with ABI Encoder v2 tuples. pub fn fuzz_param( @@ -26,7 +26,10 @@ pub fn fuzz_param( fuzz_fixtures: Option<&[DynSolValue]>, ) -> BoxedStrategy { match *param { - DynSolType::Address => super::AddressStrategy::init(fuzz_fixtures), + DynSolType::Address => fixture_strategy!( + fuzz_fixtures, + DynSolValue::type_strategy(&DynSolType::Address).boxed() + ), DynSolType::Int(n @ 8..=256) => super::IntStrategy::new(n, fuzz_fixtures) .prop_map(move |x| DynSolValue::Int(x, n)) .boxed(), @@ -34,11 +37,23 @@ pub fn fuzz_param( .prop_map(move |x| DynSolValue::Uint(x, n)) .boxed(), DynSolType::Function | DynSolType::Bool => DynSolValue::type_strategy(param).boxed(), - DynSolType::Bytes => super::BytesStrategy::init(fuzz_fixtures), - DynSolType::FixedBytes(size @ 1..=32) => { - super::FixedBytesStrategy::init(size, fuzz_fixtures) + DynSolType::Bytes => { + fixture_strategy!(fuzz_fixtures, DynSolValue::type_strategy(&DynSolType::Bytes).boxed()) } - DynSolType::String => super::StringStrategy::init(fuzz_fixtures), + DynSolType::FixedBytes(size @ 1..=32) => fixture_strategy!( + fuzz_fixtures, + DynSolValue::type_strategy(&DynSolType::FixedBytes(size)).boxed() + ), + DynSolType::String => fixture_strategy!( + fuzz_fixtures, + DynSolValue::type_strategy(&DynSolType::String) + .prop_map(move |value| { + DynSolValue::String( + value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), + ) + }) + .boxed() + ), DynSolType::Tuple(ref params) => params .iter() .map(|p| fuzz_param(p, None)) diff --git a/crates/evm/fuzz/src/strategies/string.rs b/crates/evm/fuzz/src/strategies/string.rs deleted file mode 100644 index 53668adb56ec..000000000000 --- a/crates/evm/fuzz/src/strategies/string.rs +++ /dev/null @@ -1,51 +0,0 @@ -use crate::strategies::fixture_strategy; -use alloy_dyn_abi::{DynSolType, DynSolValue}; -use proptest::{ - arbitrary::any, - prelude::{prop, BoxedStrategy}, - strategy::Strategy, -}; -use rand::{distributions::Alphanumeric, thread_rng, Rng}; - -/// The address strategy combines 2 different strategies: -/// 1. A random addresses strategy if no fixture defined for current parameter. -/// 2. A fixture based strategy if configured values for current parameters. -/// If fixture is not a valid type then an error is raised and test suite will continue to execute -// with random strings. -/// -/// -/// For example: -/// To define fixture for `person` fuzzed parameter, return an array of possible values from -/// `function fixture_person() public returns (string[] memory)`. -/// Use `person` named parameter in fuzzed test in order to create a custom strategy -/// `function testFuzz_personValue(string memory person)`. -#[derive(Debug, Default)] -pub struct StringStrategy {} - -impl StringStrategy { - /// Create a new string strategy. - pub fn init(fixtures: Option<&[DynSolValue]>) -> BoxedStrategy { - let value_from_fixture = |fixture: Option<&DynSolValue>| { - if let Some(val @ DynSolValue::String(_)) = fixture { - return val.clone() - } - error!("{:?} is not a valid string fixture, generate random value", fixture); - let mut rng = thread_rng(); - let string_len = rng.gen_range(0..128); - let random: String = - (&mut rng).sample_iter(Alphanumeric).map(char::from).take(string_len).collect(); - DynSolValue::String(random) - }; - fixture_strategy!( - fixtures, - value_from_fixture, - DynSolValue::type_strategy(&DynSolType::String) - .prop_map(move |value| { - DynSolValue::String( - value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), - ) - }) - .boxed() - ) - } -} From 4cf19cb37f46a6e68ea4b5949998d5ec1cc47201 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Sat, 13 Apr 2024 15:27:27 +0300 Subject: [PATCH 21/22] Consistent panic with fixture strategy if uint / int fixture of different type. Keep level of randomness in fixture strategy, at par with uint / int strategies. --- crates/evm/fuzz/src/strategies/calldata.rs | 18 +------------- crates/evm/fuzz/src/strategies/int.rs | 10 ++------ crates/evm/fuzz/src/strategies/mod.rs | 28 ++++++++++++---------- crates/evm/fuzz/src/strategies/param.rs | 27 ++++++++++----------- crates/evm/fuzz/src/strategies/uint.rs | 10 ++------ 5 files changed, 33 insertions(+), 60 deletions(-) diff --git a/crates/evm/fuzz/src/strategies/calldata.rs b/crates/evm/fuzz/src/strategies/calldata.rs index 404df3eeb805..760d66175481 100644 --- a/crates/evm/fuzz/src/strategies/calldata.rs +++ b/crates/evm/fuzz/src/strategies/calldata.rs @@ -62,7 +62,7 @@ mod tests { use crate::{strategies::fuzz_calldata, FuzzFixtures}; use alloy_dyn_abi::{DynSolValue, JsonAbiExt}; use alloy_json_abi::Function; - use alloy_primitives::{Address, I256}; + use alloy_primitives::Address; use proptest::prelude::Strategy; use std::collections::HashMap; @@ -83,20 +83,4 @@ mod tests { assert_eq!(expected, fuzzed); }); } - - #[test] - fn can_fuzz_with_wrong_fixtures_type() { - let function = Function::parse("test_fuzzed_uint256(uint256 uintFixture)").unwrap(); - - let mut fixtures = HashMap::new(); - fixtures.insert( - "uintFixture".to_string(), - DynSolValue::Array(vec![DynSolValue::Int(I256::MAX, 8)]), - ); - - let strategy = fuzz_calldata(function, &FuzzFixtures::new(fixtures)); - let _ = strategy.prop_map(move |fuzzed| { - assert!(fuzzed.len() > 0); - }); - } } diff --git a/crates/evm/fuzz/src/strategies/int.rs b/crates/evm/fuzz/src/strategies/int.rs index 07c14ef80ed2..34bfc221a2bd 100644 --- a/crates/evm/fuzz/src/strategies/int.rs +++ b/crates/evm/fuzz/src/strategies/int.rs @@ -91,8 +91,7 @@ impl ValueTree for IntValueTree { /// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values /// `function testFuzz_int32(int32 amount)`. /// -/// If fixture is not a valid int type then an error is raised and test suite will continue to -/// execute with random int values. +/// If fixture is not a valid int type then fuzzer will panic. #[derive(Debug)] pub struct IntStrategy { /// Bit size of int (e.g. 256) @@ -155,12 +154,7 @@ impl IntStrategy { return Ok(IntValueTree::new(int_fixture.0, false)); } } - error!( - "{:?} is not a valid {} fixture, generate random tree", - fixture, - DynSolType::Int(self.bits) - ); - self.generate_random_tree(runner) + panic!("{:?} is not a valid {} fixture", fixture, DynSolType::Int(self.bits)); } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { diff --git a/crates/evm/fuzz/src/strategies/mod.rs b/crates/evm/fuzz/src/strategies/mod.rs index 56a022b2a370..0574afdec7c1 100644 --- a/crates/evm/fuzz/src/strategies/mod.rs +++ b/crates/evm/fuzz/src/strategies/mod.rs @@ -18,22 +18,26 @@ pub use invariants::{fuzz_contract_with_calldata, invariant_strat, override_call /// Macro to create strategy with fixtures. /// 1. A default strategy if no fixture defined for current parameter. -/// 2. A fixture based strategy if configured values for current parameter. -/// If fixture is not a valid type then fuzzer will panic. +/// 2. A weighted strategy that use fixtures and default strategy values for current parameter. +/// If fixture is not of the same type as fuzzed parameter then fuzzer will panic. macro_rules! fixture_strategy { ($fixtures:ident, $default_strategy:expr) => { if let Some(fixtures) = $fixtures { - let custom_fixtures: Vec = - fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); - let custom_fixtures_len = custom_fixtures.len(); - any::() - .prop_map(move |index| { - let index = index.index(custom_fixtures_len); - custom_fixtures.get(index).unwrap().clone() - }) - .boxed() + proptest::prop_oneof![ + 50 => { + let custom_fixtures: Vec = + fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); + let custom_fixtures_len = custom_fixtures.len(); + any::() + .prop_map(move |index| { + let index = index.index(custom_fixtures_len); + custom_fixtures.get(index).unwrap().clone() + }) + }, + 50 => $default_strategy + ].boxed() } else { - return $default_strategy + $default_strategy.boxed() } }; } diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index f0f7339ac99b..da09f11ff843 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -8,8 +8,8 @@ use proptest::prelude::*; const MAX_ARRAY_LEN: usize = 256; /// Given a parameter type and configured fixtures for param name, returns a strategy for generating -/// values for that type. Fixtures can be currently generated for uint, int and address types and -/// are defined for named parameter. +/// values for that type. Fixtures can be currently generated for uint, int, address, bytes and +/// string types and are defined for parameter name. /// /// For example, fixtures for parameter `owner` of type `address` can be defined in a function with /// a `function fixture_owner() public returns (address[] memory)` signature. @@ -26,10 +26,9 @@ pub fn fuzz_param( fuzz_fixtures: Option<&[DynSolValue]>, ) -> BoxedStrategy { match *param { - DynSolType::Address => fixture_strategy!( - fuzz_fixtures, - DynSolValue::type_strategy(&DynSolType::Address).boxed() - ), + DynSolType::Address => { + fixture_strategy!(fuzz_fixtures, DynSolValue::type_strategy(&DynSolType::Address)) + } DynSolType::Int(n @ 8..=256) => super::IntStrategy::new(n, fuzz_fixtures) .prop_map(move |x| DynSolValue::Int(x, n)) .boxed(), @@ -38,21 +37,19 @@ pub fn fuzz_param( .boxed(), DynSolType::Function | DynSolType::Bool => DynSolValue::type_strategy(param).boxed(), DynSolType::Bytes => { - fixture_strategy!(fuzz_fixtures, DynSolValue::type_strategy(&DynSolType::Bytes).boxed()) + fixture_strategy!(fuzz_fixtures, DynSolValue::type_strategy(&DynSolType::Bytes)) } DynSolType::FixedBytes(size @ 1..=32) => fixture_strategy!( fuzz_fixtures, - DynSolValue::type_strategy(&DynSolType::FixedBytes(size)).boxed() + DynSolValue::type_strategy(&DynSolType::FixedBytes(size)) ), DynSolType::String => fixture_strategy!( fuzz_fixtures, - DynSolValue::type_strategy(&DynSolType::String) - .prop_map(move |value| { - DynSolValue::String( - value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), - ) - }) - .boxed() + DynSolValue::type_strategy(&DynSolType::String).prop_map(move |value| { + DynSolValue::String( + value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), + ) + }) ), DynSolType::Tuple(ref params) => params .iter() diff --git a/crates/evm/fuzz/src/strategies/uint.rs b/crates/evm/fuzz/src/strategies/uint.rs index d5007123238a..7852d4a357e3 100644 --- a/crates/evm/fuzz/src/strategies/uint.rs +++ b/crates/evm/fuzz/src/strategies/uint.rs @@ -79,8 +79,7 @@ impl ValueTree for UintValueTree { /// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values /// `function testFuzz_uint32(uint32 amount)`. /// -/// If fixture is not a valid uint type then an error is raised and test suite will continue to -/// execute with random int values. +/// If fixture is not a valid uint type then fuzzer will panic. #[derive(Debug)] pub struct UintStrategy { /// Bit size of uint (e.g. 256) @@ -133,12 +132,7 @@ impl UintStrategy { return Ok(UintValueTree::new(uint_fixture.0, false)); } } - error!( - "{:?} is not a valid {} fixture, generate random tree", - fixture, - DynSolType::Uint(self.bits) - ); - self.generate_random_tree(runner) + panic!("{:?} is not a valid {} fixture", fixture, DynSolType::Uint(self.bits)); } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { From 997c5d4ef994aa11ddaa4e0be90632e36ea2d3dc Mon Sep 17 00:00:00 2001 From: grandizzy Date: Thu, 18 Apr 2024 08:04:27 +0300 Subject: [PATCH 22/22] Review changes: don't panic when invalid fixture, use prop_filter_map for fixture strategy and raise error --- crates/evm/fuzz/src/strategies/int.rs | 8 +++-- crates/evm/fuzz/src/strategies/mod.rs | 10 +++--- crates/evm/fuzz/src/strategies/param.rs | 48 +++++++++++++++++++++++-- crates/evm/fuzz/src/strategies/uint.rs | 8 +++-- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/crates/evm/fuzz/src/strategies/int.rs b/crates/evm/fuzz/src/strategies/int.rs index 34bfc221a2bd..b27f62f2854d 100644 --- a/crates/evm/fuzz/src/strategies/int.rs +++ b/crates/evm/fuzz/src/strategies/int.rs @@ -91,7 +91,7 @@ impl ValueTree for IntValueTree { /// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values /// `function testFuzz_int32(int32 amount)`. /// -/// If fixture is not a valid int type then fuzzer will panic. +/// If fixture is not a valid int type then error is raised and random value generated. #[derive(Debug)] pub struct IntStrategy { /// Bit size of int (e.g. 256) @@ -147,14 +147,16 @@ impl IntStrategy { } // Generate value tree from fixture. - // If fixture is not a valid type, raise error and generate random value. let fixture = &self.fixtures[runner.rng().gen_range(0..self.fixtures.len())]; if let Some(int_fixture) = fixture.as_int() { if int_fixture.1 == self.bits { return Ok(IntValueTree::new(int_fixture.0, false)); } } - panic!("{:?} is not a valid {} fixture", fixture, DynSolType::Int(self.bits)); + + // If fixture is not a valid type, raise error and generate random value. + error!("{:?} is not a valid {} fixture", fixture, DynSolType::Int(self.bits)); + self.generate_random_tree(runner) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { diff --git a/crates/evm/fuzz/src/strategies/mod.rs b/crates/evm/fuzz/src/strategies/mod.rs index 0574afdec7c1..74cefca2ab37 100644 --- a/crates/evm/fuzz/src/strategies/mod.rs +++ b/crates/evm/fuzz/src/strategies/mod.rs @@ -19,9 +19,9 @@ pub use invariants::{fuzz_contract_with_calldata, invariant_strat, override_call /// Macro to create strategy with fixtures. /// 1. A default strategy if no fixture defined for current parameter. /// 2. A weighted strategy that use fixtures and default strategy values for current parameter. -/// If fixture is not of the same type as fuzzed parameter then fuzzer will panic. +/// If fixture is not of the same type as fuzzed parameter then value is rejected and error raised. macro_rules! fixture_strategy { - ($fixtures:ident, $default_strategy:expr) => { + ($fixtures:ident, $strategy_value:expr, $default_strategy:expr) => { if let Some(fixtures) = $fixtures { proptest::prop_oneof![ 50 => { @@ -29,10 +29,10 @@ macro_rules! fixture_strategy { fixtures.iter().enumerate().map(|(_, value)| value.to_owned()).collect(); let custom_fixtures_len = custom_fixtures.len(); any::() - .prop_map(move |index| { + .prop_filter_map("invalid fixture", move |index| { let index = index.index(custom_fixtures_len); - custom_fixtures.get(index).unwrap().clone() - }) + $strategy_value(custom_fixtures.get(index)) + }) }, 50 => $default_strategy ].boxed() diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index da09f11ff843..12904cb00569 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -18,7 +18,8 @@ const MAX_ARRAY_LEN: usize = 256; /// `fixture_owner` function can be used in a fuzzed test function with a signature like /// `function testFuzz_ownerAddress(address owner, uint amount)`. /// -/// Fuzzer will panic if the type of fixtures is different than the parameter type. +/// Fuzzer will reject value and raise error if the fixture type is not of the same type as +/// parameter to fuzz. /// /// Works with ABI Encoder v2 tuples. pub fn fuzz_param( @@ -27,7 +28,18 @@ pub fn fuzz_param( ) -> BoxedStrategy { match *param { DynSolType::Address => { - fixture_strategy!(fuzz_fixtures, DynSolValue::type_strategy(&DynSolType::Address)) + fixture_strategy!( + fuzz_fixtures, + |fixture: Option<&DynSolValue>| { + if let Some(val @ DynSolValue::Address(_)) = fixture { + Some(val.clone()) + } else { + error!("{:?} is not a valid address fixture", fixture.unwrap()); + None + } + }, + DynSolValue::type_strategy(&DynSolType::Address) + ) } DynSolType::Int(n @ 8..=256) => super::IntStrategy::new(n, fuzz_fixtures) .prop_map(move |x| DynSolValue::Int(x, n)) @@ -37,14 +49,44 @@ pub fn fuzz_param( .boxed(), DynSolType::Function | DynSolType::Bool => DynSolValue::type_strategy(param).boxed(), DynSolType::Bytes => { - fixture_strategy!(fuzz_fixtures, DynSolValue::type_strategy(&DynSolType::Bytes)) + fixture_strategy!( + fuzz_fixtures, + |fixture: Option<&DynSolValue>| { + if let Some(val @ DynSolValue::Bytes(_)) = fixture { + Some(val.clone()) + } else { + error!("{:?} is not a valid bytes fixture", fixture.unwrap()); + None + } + }, + DynSolValue::type_strategy(&DynSolType::Bytes) + ) } DynSolType::FixedBytes(size @ 1..=32) => fixture_strategy!( fuzz_fixtures, + |fixture: Option<&DynSolValue>| { + if let Some(val @ DynSolValue::FixedBytes(_, _)) = fixture { + if let Some(val) = val.as_fixed_bytes() { + if val.1 == size { + return Some(DynSolValue::FixedBytes(B256::from_slice(val.0), val.1)) + } + } + } + error!("{:?} is not a valid fixed bytes fixture", fixture.unwrap()); + None + }, DynSolValue::type_strategy(&DynSolType::FixedBytes(size)) ), DynSolType::String => fixture_strategy!( fuzz_fixtures, + |fixture: Option<&DynSolValue>| { + if let Some(val @ DynSolValue::String(_)) = fixture { + Some(val.clone()) + } else { + error!("{:?} is not a valid string fixture", fixture.unwrap()); + None + } + }, DynSolValue::type_strategy(&DynSolType::String).prop_map(move |value| { DynSolValue::String( value.as_str().unwrap().trim().trim_end_matches('\0').to_string(), diff --git a/crates/evm/fuzz/src/strategies/uint.rs b/crates/evm/fuzz/src/strategies/uint.rs index 7852d4a357e3..1b1eb2540499 100644 --- a/crates/evm/fuzz/src/strategies/uint.rs +++ b/crates/evm/fuzz/src/strategies/uint.rs @@ -79,7 +79,7 @@ impl ValueTree for UintValueTree { /// - use `amount` named parameter in fuzzed test in order to include fixtures in fuzzed values /// `function testFuzz_uint32(uint32 amount)`. /// -/// If fixture is not a valid uint type then fuzzer will panic. +/// If fixture is not a valid uint type then error is raised and random value generated. #[derive(Debug)] pub struct UintStrategy { /// Bit size of uint (e.g. 256) @@ -125,14 +125,16 @@ impl UintStrategy { } // Generate value tree from fixture. - // If fixture is not a valid type, raise error and generate random value. let fixture = &self.fixtures[runner.rng().gen_range(0..self.fixtures.len())]; if let Some(uint_fixture) = fixture.as_uint() { if uint_fixture.1 == self.bits { return Ok(UintValueTree::new(uint_fixture.0, false)); } } - panic!("{:?} is not a valid {} fixture", fixture, DynSolType::Uint(self.bits)); + + // If fixture is not a valid type, raise error and generate random value. + error!("{:?} is not a valid {} fixture", fixture, DynSolType::Uint(self.bits)); + self.generate_random_tree(runner) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree {