Skip to content

Commit

Permalink
chore: bump default memory limit to 128MiB (#6338)
Browse files Browse the repository at this point in the history
* chore: bump default memory limit to 128MiB

* docs

* add traces to test fails

* fix: test memory limit
  • Loading branch information
DaniPopes authored Nov 17, 2023
1 parent 9b04724 commit 8c044be
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 80 deletions.
5 changes: 4 additions & 1 deletion crates/common/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,10 @@ pub struct EnvArgs {
#[serde(skip_serializing_if = "Option::is_none")]
pub block_gas_limit: Option<u64>,

/// 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<u64>,
Expand Down
2 changes: 1 addition & 1 deletion crates/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 35 additions & 28 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,58 +252,65 @@ pub struct Config {
/// The chain name or EIP-155 chain ID.
#[serde(rename = "chain_id", alias = "chain")]
pub chain: Option<Chain>,
/// 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<usize>,
/// `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<u64>,
/// 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<GasLimit>,
/// 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<ContractOutputSelection>,
/// 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
/// contract's metadata in the contract's json artifact, whereas the latter will emit the
/// output selection as separate files.
#[serde(default)]
pub extra_output_files: Vec<ContractOutputSelection>,
/// 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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
26 changes: 15 additions & 11 deletions crates/evm/core/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<RpcUrl>,

/// pins the block number for the state fork
/// Pins the block number for the state fork.
pub fork_block_number: Option<u64>,

/// The number of retries
/// The number of retries.
pub fork_retries: Option<u32>,

/// initial retry backoff
/// Initial retry backoff.
pub fork_retry_backoff: Option<u64>,

/// The available compute units per second
/// The available compute units per second.
///
/// See also <https://docs.alchemy.com/reference/compute-units#what-are-cups-compute-units-per-second>
pub compute_units_per_second: Option<u64>,

/// 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 of the EVM in bytes.
/// The memory limit per EVM execution in bytes.
/// If this limit is exceeded, a `MemoryLimitOOG` result is thrown.
pub memory_limit: u64,
}

Expand Down
20 changes: 9 additions & 11 deletions crates/evm/evm/src/executors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions crates/forge/bin/cmd/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions crates/forge/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()\"?",
Expand Down Expand Up @@ -387,8 +387,10 @@ impl<'a> ContractRunner<'a> {
// Record test execution time
debug!(
duration = ?start.elapsed(),
%success,
%gas
gas,
reverted,
should_fail,
success,
);

TestResult {
Expand Down
2 changes: 1 addition & 1 deletion crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions crates/forge/tests/cli/ext_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 8 additions & 9 deletions crates/forge/tests/it/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"),
)
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion crates/forge/tests/it/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ pub static EVM_OPTS: Lazy<EvmOpts> = Lazy::new(|| EvmOpts {
sender: Config::DEFAULT_SENDER,
initial_balance: U256::MAX,
ffi: true,
memory_limit: 1 << 24,
verbosity: 3,
memory_limit: 1 << 26,
..Default::default()
});

Expand Down
8 changes: 4 additions & 4 deletions testdata/foundry.toml
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 = "./" }]

Expand Down

0 comments on commit 8c044be

Please sign in to comment.