From 9c91eb89fd27aca77a14b06614f67a847fd91801 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 17 Nov 2023 15:22:02 +0100 Subject: [PATCH 1/4] chore: bump default memory limit to 128MiB --- crates/common/src/evm.rs | 5 +- crates/config/README.md | 2 +- crates/config/src/lib.rs | 63 +++++++++++++---------- crates/evm/core/src/opts.rs | 4 +- crates/evm/evm/src/executors/mod.rs | 20 ++++--- crates/forge/bin/cmd/test/mod.rs | 10 ++-- crates/forge/src/runner.rs | 8 +-- crates/forge/tests/cli/config.rs | 2 +- crates/forge/tests/cli/ext_integration.rs | 6 +-- crates/forge/tests/it/test_helpers.rs | 1 - 10 files changed, 64 insertions(+), 57 deletions(-) diff --git a/crates/common/src/evm.rs b/crates/common/src/evm.rs index b721d6aafd91..830ae35da3d4 100644 --- a/crates/common/src/evm.rs +++ b/crates/common/src/evm.rs @@ -242,7 +242,10 @@ pub struct EnvArgs { #[serde(skip_serializing_if = "Option::is_none")] pub block_gas_limit: Option, - /// The memory limit of the EVM in bytes (32 MB by default) + /// The memory limit per EVM execution in bytes. + /// If this limit is exceeded, a `MemoryLimitOOG` result is thrown. + /// + /// The default is 128MiB. #[clap(long, value_name = "MEMORY_LIMIT")] #[serde(skip_serializing_if = "Option::is_none")] pub memory_limit: Option, diff --git a/crates/config/README.md b/crates/config/README.md index 1913cadeeb99..1fb1370006da 100644 --- a/crates/config/README.md +++ b/crates/config/README.md @@ -131,7 +131,7 @@ block_timestamp = 0 block_difficulty = 0 block_prevrandao = '0x0000000000000000000000000000000000000000' block_gas_limit = 30000000 -memory_limit = 33554432 +memory_limit = 134217728 extra_output = ["metadata"] extra_output_files = [] names = false diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 378a029118df..5175775315d5 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -252,48 +252,55 @@ pub struct Config { /// The chain name or EIP-155 chain ID. #[serde(rename = "chain_id", alias = "chain")] pub chain: Option, - /// Block gas limit + /// Block gas limit. pub gas_limit: GasLimit, /// EIP-170: Contract code size limit in bytes. Useful to increase this because of tests. pub code_size_limit: Option, - /// `tx.gasprice` value during EVM execution" + /// `tx.gasprice` value during EVM execution. /// /// This is an Option, so we can determine in fork mode whether to use the config's gas price - /// (if set by user) or the remote client's gas price + /// (if set by user) or the remote client's gas price. pub gas_price: Option, - /// the base fee in a block + /// The base fee in a block. pub block_base_fee_per_gas: u64, - /// the `block.coinbase` value during EVM execution + /// The `block.coinbase` value during EVM execution. pub block_coinbase: Address, - /// the `block.timestamp` value during EVM execution + /// The `block.timestamp` value during EVM execution. pub block_timestamp: u64, - /// the `block.difficulty` value during EVM execution + /// The `block.difficulty` value during EVM execution. pub block_difficulty: u64, - /// Before merge the `block.max_hash` after merge it is `block.prevrandao` + /// Before merge the `block.max_hash`, after merge it is `block.prevrandao`. pub block_prevrandao: B256, /// the `block.gaslimit` value during EVM execution pub block_gas_limit: Option, - /// The memory limit of the EVM (32 MB by default) + /// The memory limit per EVM execution in bytes. + /// If this limit is exceeded, a `MemoryLimitOOG` result is thrown. + /// + /// The default is 128MiB. pub memory_limit: u64, - /// Additional output selection for all contracts - /// such as "ir", "devdoc", "storageLayout", etc. - /// See [Solc Compiler Api](https://docs.soliditylang.org/en/latest/using-the-compiler.html#compiler-api) + /// Additional output selection for all contracts, such as "ir", "devdoc", "storageLayout", + /// etc. + /// + /// See the [Solc Compiler Api](https://docs.soliditylang.org/en/latest/using-the-compiler.html#compiler-api) for more information. /// - /// The following values are always set because they're required by `forge` - //{ - // "*": [ - // "abi", - // "evm.bytecode", - // "evm.deployedBytecode", - // "evm.methodIdentifiers" - // ] - // } - // "# + /// The following values are always set because they're required by `forge`: + /// ```json + /// { + /// "*": [ + /// "abi", + /// "evm.bytecode", + /// "evm.deployedBytecode", + /// "evm.methodIdentifiers" + /// ] + /// } + /// ``` #[serde(default)] pub extra_output: Vec, - /// If set , a separate `json` file will be emitted for every contract depending on the + /// If set, a separate JSON file will be emitted for every contract depending on the /// selection, eg. `extra_output_files = ["metadata"]` will create a `metadata.json` for - /// each contract in the project. See [Contract Metadata](https://docs.soliditylang.org/en/latest/metadata.html) + /// each contract in the project. + /// + /// See [Contract Metadata](https://docs.soliditylang.org/en/latest/metadata.html) for more information. /// /// The difference between `extra_output = ["metadata"]` and /// `extra_output_files = ["metadata"]` is that the former will include the @@ -301,9 +308,9 @@ pub struct Config { /// output selection as separate files. #[serde(default)] pub extra_output_files: Vec, - /// Print the names of the compiled contracts + /// Whether to print the names of the compiled contracts. pub names: bool, - /// Print the sizes of the compiled contracts + /// Whether to print the sizes of the compiled contracts. pub sizes: bool, /// If set to true, changes compilation pipeline to go through the Yul intermediate /// representation. @@ -1795,7 +1802,7 @@ impl Default for Config { block_difficulty: 0, block_prevrandao: Default::default(), block_gas_limit: None, - memory_limit: 1 << 25, // 32MiB = 33554432 bytes + memory_limit: 1 << 27, // 2**27 = 128MiB = 134_217_728 bytes eth_rpc_url: None, eth_rpc_jwt: None, etherscan_api_key: None, @@ -3379,7 +3386,7 @@ mod tests { initial_balance = '0xffffffffffffffffffffffff' libraries = [] libs = ['lib'] - memory_limit = 33554432 + memory_limit = 134217728 names = false no_storage_caching = false no_rpc_rate_limit = false diff --git a/crates/evm/core/src/opts.rs b/crates/evm/core/src/opts.rs index bc364bc51b45..b693d0eb3c46 100644 --- a/crates/evm/core/src/opts.rs +++ b/crates/evm/core/src/opts.rs @@ -49,7 +49,9 @@ pub struct EvmOpts { /// Verbosity mode of EVM output as number of occurrences pub verbosity: u8, - /// The memory limit of the EVM in bytes. + /// The memory limit per EVM execution in bytes. + /// + /// The default is 128MiB. pub memory_limit: u64, } diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index 4cbdcdb3d813..234757a907d8 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -489,26 +489,24 @@ impl Executor { // If this test failed any asserts, then this changeset will contain changes `false -> true` // for the contract's `failed` variable and the `globalFailure` flag in the state of the - // cheatcode address which are both read when call `"failed()(bool)"` in the next step + // cheatcode address which are both read when we call `"failed()(bool)"` in the next step backend.commit(state_changeset); - let executor = - Executor::new(backend, self.env.clone(), self.inspector.clone(), self.gas_limit); let mut success = !reverted; if success { // Check if a DSTest assertion failed - let call = - executor.call::<_, _>(CALLER, address, "failed()(bool)", vec![], U256::ZERO, None); - + let executor = + Executor::new(backend, self.env.clone(), self.inspector.clone(), self.gas_limit); + let call = executor.call(CALLER, address, "failed()(bool)", vec![], U256::ZERO, None); if let Ok(CallResult { result: failed, .. }) = call { - let failed = failed - .as_bool() - .expect("Failed to decode DSTest `failed` variable. This is a bug"); - success = !failed; + debug!(?failed, "DSTest"); + success = !failed.as_bool().unwrap(); } } - Ok(should_fail ^ success) + let result = should_fail ^ success; + debug!(should_fail, success, result); + Ok(result) } /// Creates the environment to use when executing a transaction in a test context diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 1ad5488701ac..42040a03744f 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -208,10 +208,10 @@ impl TestArgs { filter.args_mut().test_pattern = self.debug.clone(); let num_filtered = runner.matching_test_function_count(&filter); if num_filtered != 1 { - return Err( - eyre::eyre!("{num_filtered} tests matched your criteria, but exactly 1 test must match in order to run the debugger.\n - \n - Use --match-contract and --match-path to further limit the search.")); + eyre::bail!( + "{num_filtered} tests matched your criteria, but exactly 1 test must match in order to run the debugger.\n\n\ + Use --match-contract and --match-path to further limit the search." + ); } let test_funcs = runner.get_matching_test_functions(&filter); // if we debug a fuzz test, we should not collect data on the first run @@ -441,7 +441,7 @@ impl Test { } /// Represents the bundled results of all tests -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct TestOutcome { /// Whether failures are allowed pub allow_failure: bool, diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 7d781098771f..36c3001a95f0 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -183,7 +183,7 @@ impl<'a> ContractRunner<'a> { let needs_setup = setup_fns.len() == 1 && setup_fns[0].name == "setUp"; // There is a single miss-cased `setUp` function, so we add a warning - for setup_fn in setup_fns.iter() { + for &setup_fn in setup_fns.iter() { if setup_fn.name != "setUp" { warnings.push(format!( "Found invalid setup function \"{}\" did you mean \"setUp()\"?", @@ -387,8 +387,10 @@ impl<'a> ContractRunner<'a> { // Record test execution time debug!( duration = ?start.elapsed(), - %success, - %gas + gas, + reverted, + should_fail, + success, ); TestResult { diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index 74aa16e45fc6..27001b732172 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -79,7 +79,7 @@ forgetest!(can_extract_config_values, |prj, cmd| { block_difficulty: 10, block_prevrandao: B256::random(), block_gas_limit: Some(100u64.into()), - memory_limit: 2u64.pow(25), + memory_limit: 1 << 27, eth_rpc_url: Some("localhost".to_string()), eth_rpc_jwt: None, etherscan_api_key: None, diff --git a/crates/forge/tests/cli/ext_integration.rs b/crates/forge/tests/cli/ext_integration.rs index 8a6c0be8c93d..55c128b07f50 100644 --- a/crates/forge/tests/cli/ext_integration.rs +++ b/crates/forge/tests/cli/ext_integration.rs @@ -16,11 +16,7 @@ forgetest_external!( // `run: pnpm --version` is ok, `Command::new("pnpm")` isn't. Good job Windows. #[cfg_attr(windows, ignore = "Windows cannot find installed programs")] snekmate, - "pcaversaccio/snekmate", - // 64MiB memory limit: - // - https://github.com/foundry-rs/foundry/pull/6281 - // - https://github.com/bluealloy/revm/issues/865 - &["--memory-limit", &(1u64 << 26).to_string()] + "pcaversaccio/snekmate" ); // Forking tests diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index cfbbc015162f..3307514d7242 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -72,7 +72,6 @@ pub static EVM_OPTS: Lazy = Lazy::new(|| EvmOpts { sender: Config::DEFAULT_SENDER, initial_balance: U256::MAX, ffi: true, - memory_limit: 1 << 24, ..Default::default() }); From 41850a6d6a03ac9265c48395b8f678eea9530f70 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 17 Nov 2023 15:32:00 +0100 Subject: [PATCH 2/4] docs --- crates/evm/core/src/opts.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/evm/core/src/opts.rs b/crates/evm/core/src/opts.rs index b693d0eb3c46..5904d21c6479 100644 --- a/crates/evm/core/src/opts.rs +++ b/crates/evm/core/src/opts.rs @@ -12,44 +12,48 @@ use serde::{Deserialize, Deserializer, Serialize}; #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct EvmOpts { + /// The EVM environment configuration. #[serde(flatten)] pub env: Env, - /// Fetch state over a remote instead of starting from empty state + /// Fetch state over a remote instead of starting from empty state. #[serde(rename = "eth_rpc_url")] pub fork_url: Option, - /// pins the block number for the state fork + /// Pins the block number for the state fork. pub fork_block_number: Option, - /// The number of retries + /// The number of retries. pub fork_retries: Option, - /// initial retry backoff + /// Initial retry backoff. pub fork_retry_backoff: Option, - /// The available compute units per second + /// The available compute units per second. + /// + /// See also pub compute_units_per_second: Option, - /// Disables rate limiting entirely. + /// Disables RPC rate limiting entirely. pub no_rpc_rate_limit: bool, /// Disables storage caching entirely. pub no_storage_caching: bool, - /// the initial balance of each deployed test contract + /// The initial balance of each deployed test contract. pub initial_balance: U256, - /// the address which will be executing all tests + /// The address which will be executing all tests. pub sender: Address, - /// enables the FFI cheatcode + /// Enables the FFI cheatcode. pub ffi: bool, - /// Verbosity mode of EVM output as number of occurrences + /// Verbosity mode of EVM output as number of occurrences. pub verbosity: u8, /// The memory limit per EVM execution in bytes. + /// If this limit is exceeded, a `MemoryLimitOOG` result is thrown. /// /// The default is 128MiB. pub memory_limit: u64, From 8ef141ac80f38ef704710e972760db1782f79a6c Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 17 Nov 2023 15:55:27 +0100 Subject: [PATCH 3/4] add traces to test fails --- crates/forge/tests/it/config.rs | 17 ++++++++--------- crates/forge/tests/it/test_helpers.rs | 1 + testdata/foundry.toml | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/forge/tests/it/config.rs b/crates/forge/tests/it/config.rs index 95d03110fff9..b9473b1526f2 100644 --- a/crates/forge/tests/it/config.rs +++ b/crates/forge/tests/it/config.rs @@ -13,10 +13,8 @@ use foundry_evm::{ decode::decode_console_logs, inspectors::CheatsConfig, revm::primitives::SpecId, }; use foundry_test_utils::Filter; -use std::{ - collections::BTreeMap, - path::{Path, PathBuf}, -}; +use itertools::Itertools; +use std::{collections::BTreeMap, path::Path}; /// How to execute a a test run pub struct TestConfig { @@ -82,11 +80,12 @@ impl TestConfig { let outcome = if self.should_fail { "fail" } else { "pass" }; eyre::bail!( - "Test {} did not {} as expected.\nReason: {:?}\nLogs:\n{}", + "Test {} did not {} as expected.\nReason: {:?}\nLogs:\n{}\n\nTraces:\n{}", test_name, outcome, result.reason, - logs.join("\n") + logs.join("\n"), + result.traces.iter().map(|(_, a)| a).format("\n"), ) } } @@ -136,14 +135,14 @@ pub(crate) fn init_tracing() { .try_init(); } -pub fn manifest_root() -> PathBuf { +pub fn manifest_root() -> &'static Path { let mut root = Path::new(env!("CARGO_MANIFEST_DIR")); // need to check here where we're executing the test from, if in `forge` we need to also allow // `testdata` if root.ends_with("forge") { root = root.parent().unwrap(); } - root.to_path_buf() + root } /// Builds a base runner @@ -161,7 +160,7 @@ pub async fn runner() -> MultiContractRunner { /// Builds a non-tracing runner pub async fn runner_with_config(mut config: Config) -> MultiContractRunner { config.rpc_endpoints = rpc_endpoints(); - config.allow_paths.push(manifest_root()); + config.allow_paths.push(manifest_root().to_path_buf()); let root = &PROJECT.paths.root; let opts = &*EVM_OPTS; diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index 3307514d7242..a2d703b02081 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -72,6 +72,7 @@ pub static EVM_OPTS: Lazy = Lazy::new(|| EvmOpts { sender: Config::DEFAULT_SENDER, initial_balance: U256::MAX, ffi: true, + verbosity: 3, ..Default::default() }); diff --git a/testdata/foundry.toml b/testdata/foundry.toml index d696945d0640..94bdeea39493 100644 --- a/testdata/foundry.toml +++ b/testdata/foundry.toml @@ -1,5 +1,5 @@ [profile.default] -solc = "0.8.19" +solc = "0.8.18" block_base_fee_per_gas = 0 block_coinbase = "0x0000000000000000000000000000000000000000" block_difficulty = 0 @@ -36,10 +36,10 @@ remappings = ["ds-test/=lib/ds-test/src/"] sender = "0x00a329c0648769a73afac7f9381e08fb43dbea72" sizes = false sparse_mode = false -src = "src" -test = "test" +src = "./" +test = "./" tx_origin = "0x00a329c0648769a73afac7f9381e08fb43dbea72" -verbosity = 0 +verbosity = 3 via_ir = false fs_permissions = [{ access = "read-write", path = "./" }] From 102a00df1998c4c622abf85c1f5bceeb20795c4a Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 17 Nov 2023 15:57:52 +0100 Subject: [PATCH 4/4] fix: test memory limit --- crates/evm/core/src/opts.rs | 2 -- crates/forge/tests/it/test_helpers.rs | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/evm/core/src/opts.rs b/crates/evm/core/src/opts.rs index 5904d21c6479..1ccac43d4225 100644 --- a/crates/evm/core/src/opts.rs +++ b/crates/evm/core/src/opts.rs @@ -54,8 +54,6 @@ pub struct EvmOpts { /// The memory limit per EVM execution in bytes. /// If this limit is exceeded, a `MemoryLimitOOG` result is thrown. - /// - /// The default is 128MiB. pub memory_limit: u64, } diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index a2d703b02081..37393525f597 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -73,6 +73,7 @@ pub static EVM_OPTS: Lazy = Lazy::new(|| EvmOpts { initial_balance: U256::MAX, ffi: true, verbosity: 3, + memory_limit: 1 << 26, ..Default::default() });