fix(evm): fuzzing not properly collecting data

evm/src/executor/inspector/fuzzer.rs

@@ -82,12 +82,13 @@ impl Fuzzer {
             state.insert(utils::u256_to_h256_be(*slot).into());
         }
 
-        for index in 0..interpreter.memory.len() / 32 {
-            let mut slot = [0u8; 32];
-            slot.clone_from_slice(interpreter.memory.get_slice(index * 32, 32));
+        // TODO: disabled for now since it's flooding the dictionary
+        // for index in 0..interpreter.memory.len() / 32 {
+        //     let mut slot = [0u8; 32];
+        //     slot.clone_from_slice(interpreter.memory.get_slice(index * 32, 32));
 
-            state.insert(slot);
-        }
+        //     state.insert(slot);
+        // }
     }
 
     /// Overrides an external call and tries to call any method of msg.sender.

evm/src/fuzz/invariant/error.rs

@@ -9,6 +9,7 @@ use crate::{
 use ethers::{abi::Function, types::Address};
 use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact};
 use proptest::test_runner::TestError;
+use tracing::trace;
 
 #[derive(Debug, Clone)]
 pub struct InvariantFuzzError {
@@ -196,6 +197,7 @@ impl InvariantFuzzError {
         let mut anchor = 0;
         let mut removed_calls = vec![];
         let mut shrinked = calls.iter().collect::<Vec<_>>();
+        trace!(target: "forge::test", "Shrinking.");
 
         while anchor != calls.len() {
             // Get the latest removed element, so we know which one to remove next.

evm/src/fuzz/invariant/executor.rs

@@ -23,6 +23,7 @@ use ethers::{
 };
 use eyre::ContextCompat;
 use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact};
+use hashbrown::HashMap;
 use parking_lot::{Mutex, RwLock};
 use proptest::{
     strategy::{BoxedStrategy, Strategy, ValueTree},
@@ -142,14 +143,10 @@ impl<'a> InvariantExecutor<'a> {
                         .expect("could not make raw evm call");
 
                     // Collect data for fuzzing from the state changeset.
-                    let state_changeset =
+                    let mut state_changeset =
                         call_result.state_changeset.to_owned().expect("to have a state changeset.");
 
-                    collect_state_from_call(
-                        &call_result.logs,
-                        &state_changeset,
-                        fuzz_state.clone(),
-                    );
+                    collect_data(&mut state_changeset, sender, &call_result, fuzz_state.clone());
 
                     if let Err(error) = collect_created_contracts(
                         &state_changeset,
@@ -206,6 +203,8 @@ impl<'a> InvariantExecutor<'a> {
             });
         }
 
+        tracing::trace!(target: "forge::test::invariant::dictionary", "{:?}", fuzz_state.read().iter().map(hex::encode).collect::<Vec<_>>());
+
         let (reverts, invariants) = failures.into_inner().into_inner();
 
         Ok(Some(InvariantFuzzTestResult { invariants, cases: fuzz_cases.into_inner(), reverts }))
@@ -479,6 +478,36 @@ impl<'a> InvariantExecutor<'a> {
     }
 }
 
+/// Collects data from call for fuzzing. However, it first verifies that the sender is not an EOA
+/// before inserting it into the dictionary. Otherwise, we flood the dictionary with
+/// randomly generated addresses.
+fn collect_data(
+    state_changeset: &mut HashMap<Address, revm::Account>,
+    sender: &Address,
+    call_result: &RawCallResult,
+    fuzz_state: EvmFuzzState,
+) {
+    // Verify it has no code.
+    let mut has_code = false;
+    if let Some(Some(code)) = state_changeset.get(sender).map(|account| account.info.code.as_ref())
+    {
+        has_code = !code.is_empty();
+    }
+
+    // We keep the nonce changes to apply later.
+    let mut sender_changeset = None;
+    if !has_code {
+        sender_changeset = state_changeset.remove(sender);
+    }
+
+    collect_state_from_call(&call_result.logs, &*state_changeset, fuzz_state);
+
+    // Re-add changes
+    if let Some(changed) = sender_changeset {
+        state_changeset.insert(*sender, changed);
+    }
+}
+
 /// Verifies that the invariant run execution can continue.
 fn can_continue(
     invariant_contract: &InvariantContract,

evm/src/fuzz/mod.rs

@@ -124,6 +124,8 @@ impl<'a> FuzzedExecutor<'a> {
             }
         });
 
+        tracing::trace!(target: "forge::test::fuzz::dictionary", "{:?}", state.read().iter().map(hex::encode).collect::<Vec<_>>());
+
         let (calldata, call) = counterexample.into_inner();
         let mut result = FuzzTestResult {
             cases: FuzzedCases::new(cases.into_inner()),

evm/src/fuzz/strategies/invariants.rs

@@ -13,6 +13,8 @@ use proptest::prelude::*;
 pub use proptest::test_runner::Config as FuzzConfig;
 use std::sync::Arc;
 
+use super::fuzz_param_from_state;
+
 /// Given a target address, we generate random calldata.
 pub fn override_call_strat(
     fuzz_state: EvmFuzzState,
@@ -75,20 +77,36 @@ fn generate_call(
             let senders = senders.clone();
             let fuzz_state = fuzz_state.clone();
             func.prop_flat_map(move |func| {
-                let sender = select_random_sender(senders.clone());
+                let sender = select_random_sender(fuzz_state.clone(), senders.clone());
                 (sender, fuzz_contract_with_calldata(fuzz_state.clone(), contract, func))
             })
         })
         .boxed()
 }
 
 /// Strategy to select a sender address:
-/// * If `senders` is empty, then it's a completely random address.
+/// * If `senders` is empty, then it's either a random address (10%) or from the dictionary (90%).
 /// * If `senders` is not empty, then there's an 80% chance that one from the list is selected. The
-///   remaining 20% will be random.
-fn select_random_sender(senders: Vec<Address>) -> impl Strategy<Value = Address> {
-    let fuzz_strategy =
-        fuzz_param(&ParamType::Address).prop_map(move |addr| addr.into_address().unwrap()).boxed();
+///   remaining 20% will either be a random address (10%) or from the dictionary (90%).
+fn select_random_sender(
+    fuzz_state: EvmFuzzState,
+    senders: Vec<Address>,
+) -> impl Strategy<Value = Address> {
+    let fuzz_strategy = proptest::strategy::Union::new_weighted(vec![
+        (
+            10,
+            fuzz_param(&ParamType::Address)
+                .prop_map(move |addr| addr.into_address().unwrap())
+                .boxed(),
+        ),
+        (
+            90,
+            fuzz_param_from_state(&ParamType::Address, fuzz_state)
+                .prop_map(move |addr| addr.into_address().unwrap())
+                .boxed(),
+        ),
+    ])
+    .boxed();
 
     if !senders.is_empty() {
         let selector =

evm/src/fuzz/strategies/param.rs

@@ -99,7 +99,11 @@ pub fn fuzz_param_from_state(param: &ParamType, arc_state: EvmFuzzState) -> Boxe
         },
         ParamType::Bool => value.prop_map(move |value| Token::Bool(value[31] == 1)).boxed(),
         ParamType::String => value
-            .prop_map(move |value| Token::String(String::from_utf8_lossy(&value[..]).to_string()))
+            .prop_map(move |value| {
+                Token::String(
+                    String::from_utf8_lossy(&value[..]).trim().trim_end_matches('\0').to_string(),
+                )
+            })
             .boxed(),
         ParamType::Array(param) => proptest::collection::vec(
             fuzz_param_from_state(param, arc_state.clone()),
@@ -128,7 +132,10 @@ pub fn fuzz_param_from_state(param: &ParamType, arc_state: EvmFuzzState) -> Boxe
 
 #[cfg(test)]
 mod tests {
-    use crate::fuzz::strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state};
+    use crate::{
+        fuzz::strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state},
+        CALLER,
+    };
     use ethers::abi::HumanReadableParser;
     use revm::db::{CacheDB, EmptyDB};
 

evm/src/fuzz/strategies/state.rs

@@ -84,17 +84,13 @@ pub fn build_initial_state<DB: DatabaseRef>(db: &CacheDB<DB>) -> EvmFuzzState {
     let mut state = FuzzDictionary::default();
 
     for (address, account) in db.accounts.iter() {
-        let info = db.basic(*address);
-
         // Insert basic account information
         state.insert(H256::from(*address).into());
-        state.insert(utils::u256_to_h256_le(info.balance).into());
-        state.insert(utils::u256_to_h256_le(U256::from(info.nonce)).into());
 
         // Insert storage
         for (slot, value) in &account.storage {
-            state.insert(utils::u256_to_h256_le(*slot).into());
-            state.insert(utils::u256_to_h256_le(*value).into());
+            state.insert(utils::u256_to_h256_be(*slot).into());
+            state.insert(utils::u256_to_h256_be(*value).into());
         }
     }
 
@@ -119,13 +115,11 @@ pub fn collect_state_from_call(
     for (address, account) in state_changeset {
         // Insert basic account information
         state.insert(H256::from(*address).into());
-        state.insert(utils::u256_to_h256_le(account.info.balance).into());
-        state.insert(utils::u256_to_h256_le(U256::from(account.info.nonce)).into());
 
         // Insert storage
         for (slot, value) in &account.storage {
-            state.insert(utils::u256_to_h256_le(*slot).into());
-            state.insert(utils::u256_to_h256_le(*value).into());
+            state.insert(utils::u256_to_h256_be(*slot).into());
+            state.insert(utils::u256_to_h256_be(*value).into());
         }
 
         // Insert push bytes
@@ -183,11 +177,8 @@ fn collect_push_bytes(code: Bytes) -> Vec<[u8; 32]> {
                 return bytes
             }
 
-            let mut buffer: [u8; 32] = [0; 32];
-            let _ = (&mut buffer[..])
-                .write(&code[push_start..push_end])
-                .expect("push was larger than 32 bytes");
-            bytes.push(buffer);
+            bytes.push(U256::from_big_endian(&code[push_start..push_end]).into());
+
             i += push_size;
         }
         i += 1;

forge/src/lib.rs

@@ -51,11 +51,19 @@ pub struct TestOptions {
 }
 
 impl TestOptions {
+    pub fn invariant_fuzzer(&self) -> TestRunner {
+        self.fuzzer_with_cases(self.invariant_runs)
+    }
+
     pub fn fuzzer(&self) -> TestRunner {
+        self.fuzzer_with_cases(self.fuzz_runs)
+    }
+
+    pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner {
         // TODO: Add Options to modify the persistence
         let cfg = proptest::test_runner::Config {
             failure_persistence: None,
-            cases: self.fuzz_runs,
+            cases,
             max_local_rejects: self.fuzz_max_local_rejects,
             max_global_rejects: self.fuzz_max_global_rejects,
             ..Default::default()

forge/src/runner.rs

@@ -302,7 +302,7 @@ impl<'a> ContractRunner<'a> {
                 .collect();
 
             let results = self.run_invariant_test(
-                test_options.fuzzer(),
+                test_options.invariant_fuzzer(),
                 setup,
                 test_options,
                 functions.clone(),

forge/tests/it/fuzz.rs

@@ -1,16 +1,24 @@
 //! Tests for invariants
 
 use crate::{config::*, test_helpers::filter::Filter};
+use ethers::types::U256;
 use forge::result::SuiteResult;
-
 use foundry_evm::decode::decode_console_logs;
+use std::collections::BTreeMap;
 
 #[test]
 fn test_fuzz() {
     let mut runner = runner();
 
-    let suite_result =
-        runner.test(&Filter::new(".*", ".*", ".*fuzz/[^invariant]"), None, TEST_OPTS).unwrap();
+    let suite_result = runner
+        .test(
+            &Filter::new(".*", ".*", ".*fuzz/[^invariant]").exclude_tests(
+                r#"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)"#,
+            ),
+            None,
+            TEST_OPTS,
+        )
+        .unwrap();
 
     assert!(!suite_result.is_empty());
 
@@ -40,3 +48,39 @@ fn test_fuzz() {
         }
     }
 }
+
+/// Test that showcases PUSH collection on normal fuzzing. Ignored until we collect them in a
+/// smarter way.
+#[test]
+#[ignore]
+fn test_fuzz_collection() {
+    let mut runner = runner();
+
+    let mut opts = TEST_OPTS;
+    opts.invariant_depth = 100;
+    opts.invariant_runs = 1000;
+    opts.fuzz_runs = 1000;
+    opts.fuzz_seed = Some(U256::from(6u32));
+    runner.test_options = opts;
+
+    let results =
+        runner.test(&Filter::new(".*", ".*", ".*fuzz/FuzzCollection.t.sol"), None, opts).unwrap();
+
+    assert_multiple(
+        &results,
+        BTreeMap::from([(
+            "fuzz/FuzzCollection.t.sol:SampleContractTest",
+            vec![
+                ("invariantCounter", false, Some("broken counter.".into()), None, None),
+                (
+                    "testIncrement(address)",
+                    false,
+                    Some("Call did not revert as expected".into()),
+                    None,
+                    None,
+                ),
+                ("testNeedle(uint256)", false, Some("needle found.".into()), None, None),
+            ],
+        )]),
+    );
+}