Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(forge): isolated execution #7186

Merged
merged 26 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,6 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
debug!(target: "cheatcodes", tx=?self.broadcastable_transactions.back().unwrap(), "broadcastable call");

let prev = account.info.nonce;
account.info.nonce += 1;
debug!(target: "cheatcodes", address=%broadcast.new_origin, nonce=prev+1, prev, "incremented nonce");
} else if broadcast.single_call {
let msg = "`staticcall`s are not allowed after `broadcast`; use `startBroadcast` instead";
Expand Down Expand Up @@ -939,6 +938,13 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
if data.journaled_state.depth() == broadcast.depth {
data.env.tx.caller = broadcast.original_origin;

if !call.is_static {
let account =
data.journaled_state.state().get_mut(&broadcast.new_origin).unwrap();

account.info.nonce += 1;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this change for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently we are increasing nonce before broadcasting CALL to simulate nonce increase during on-chain transaction

this is incorrect for--isolate because we need nonce to be up-to-date at the point when we are creating a transaction

so the change is to increase nonce after the CALL

however, thinking of it now, with new workaround when we explicitly decrease nonces in isolation, this is not really needed as long as we touch the account when pre-increase its nonce, updated in 33814e5

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah can we also doc that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call


// Clean single-call broadcast once we have returned to the original depth
if broadcast.single_call {
let _ = self.broadcast.take();
Expand Down Expand Up @@ -1487,7 +1493,6 @@ fn process_broadcast_create<DB: DatabaseExt>(
let prev = account.info.nonce;
account.info.nonce += 1;
debug!(target: "cheatcodes", address=%broadcast_sender, nonce=prev+1, prev, "incremented nonce in create2");

// Proxy deployer requires the data to be `salt ++ init_code`
let calldata = [&salt.to_be_bytes::<32>()[..], &bytecode[..]].concat();
(calldata.into(), Some(DEFAULT_CREATE2_DEPLOYER), prev)
Expand Down
9 changes: 9 additions & 0 deletions crates/common/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ pub struct EvmArgs {
#[clap(flatten)]
#[serde(flatten)]
pub env: EnvArgs,

/// Whether to enable isolation of calls.
mattsse marked this conversation as resolved.
Show resolved Hide resolved
#[clap(long)]
#[serde(skip)]
pub isolate: bool,
}

// Make this set of options a `figment::Provider` so that it can be merged into the `Config`
Expand All @@ -166,6 +171,10 @@ impl Provider for EvmArgs {
dict.insert("ffi".to_string(), self.ffi.into());
}

if self.isolate {
dict.insert("isolate".to_string(), self.isolate.into());
}

if self.always_use_create_2_factory {
dict.insert(
"always_use_create_2_factory".to_string(),
Expand Down
6 changes: 6 additions & 0 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,11 @@ pub struct Config {
/// Should be removed once EvmVersion Cancun is supported by solc
pub cancun: bool,

/// Whether to enable call isolation.
///
/// Useful for more correct gas accounting and EVM behavior in general.
pub isolate: bool,

/// Address labels
pub labels: HashMap<Address, String>,

Expand Down Expand Up @@ -1810,6 +1815,7 @@ impl Default for Config {
profile: Self::DEFAULT_PROFILE,
fs_permissions: FsPermissions::new([PathPermission::read("out")]),
cancun: false,
isolate: false,
__root: Default::default(),
src: "src".into(),
test: "test".into(),
Expand Down
10 changes: 8 additions & 2 deletions crates/evm/core/src/backend/fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use alloy_genesis::GenesisAccount;
use alloy_primitives::{Address, B256, U256};
use revm::{
db::DatabaseRef,
primitives::{AccountInfo, Bytecode, Env, ResultAndState},
Database, Inspector, JournaledState,
primitives::{Account, AccountInfo, Bytecode, Env, HashMap as Map, ResultAndState},
Database, DatabaseCommit, Inspector, JournaledState,
};
use std::{borrow::Cow, collections::HashMap};

Expand Down Expand Up @@ -279,3 +279,9 @@ impl<'a> Database for FuzzBackendWrapper<'a> {
DatabaseRef::block_hash_ref(self, number)
}
}

impl<'a> DatabaseCommit for FuzzBackendWrapper<'a> {
fn commit(&mut self, changes: Map<Address, Account>) {
self.backend.to_mut().commit(changes)
}
}
3 changes: 3 additions & 0 deletions crates/evm/core/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ pub struct EvmOpts {
/// The memory limit per EVM execution in bytes.
/// If this limit is exceeded, a `MemoryLimitOOG` result is thrown.
pub memory_limit: u64,

/// Whether to enable isolation of calls.
pub isolate: bool,
}

impl EvmOpts {
Expand Down
Loading
Loading