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

[do not merge] Experimenting with static strategy #789

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f1c871c
add strategy objects
nbaztec Dec 12, 2024
b38109d
remove unused parts
nbaztec Dec 12, 2024
57f32f2
fix test build
nbaztec Dec 12, 2024
06aedf4
remove use_zk completely, fix ExecutorStrategy trait
nbaztec Dec 13, 2024
5d77f8c
fix deadlock
nbaztec Dec 14, 2024
6c00749
clippy
nbaztec Dec 14, 2024
b5fd952
fix tests, startup bug
nbaztec Dec 15, 2024
9009698
switch to try_lock to prevent deadlocks
nbaztec Dec 15, 2024
bee659d
revert try_lock
nbaztec Dec 15, 2024
b5b7e5f
noop instead of panic on zksync methods for evm, clippy
nbaztec Dec 15, 2024
5ee99fc
fix warp and roll
nbaztec Dec 15, 2024
2151459
fix script
nbaztec Dec 15, 2024
e9fbf13
make call immutable
nbaztec Dec 15, 2024
7b23078
deep clone strategies on clone
nbaztec Dec 16, 2024
095903b
trigger ci
nbaztec Dec 16, 2024
da60354
revert unintended change
nbaztec Dec 16, 2024
f740454
Merge branch 'main' into nish-abstraction-strategy
nbaztec Dec 16, 2024
193f955
fix zk cheatcodes in evm context
nbaztec Dec 16, 2024
a506866
fix get_code, remove unintended sleep
nbaztec Dec 16, 2024
0e9c64f
fix script test, clippy
nbaztec Dec 16, 2024
589c8ba
Merge branch 'main' into nish-abstraction-strategy
nbaztec Dec 16, 2024
f0d0f9d
fix zk_env
nbaztec Dec 16, 2024
d80312f
guard zk provider
nbaztec Dec 16, 2024
bdba380
use blocking provider call
nbaztec Dec 16, 2024
53d5d9a
Merge branch 'main' into nish-abstraction-strategy
nbaztec Dec 16, 2024
1876c01
use Box instead of Arc<Mutex<T>>
nbaztec Dec 17, 2024
d92ad57
chore: snapshot
popzxc Dec 18, 2024
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
Prev Previous commit
Next Next commit
fix deadlock
  • Loading branch information
nbaztec committed Dec 14, 2024
commit 5d77f8ce13ee98051087b92697fecdc0c2420f97
2 changes: 1 addition & 1 deletion crates/cli/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn get_provider(config: &Config) -> Result<RetryProvider> {
}

pub fn get_executor_strategy(config: &Config) -> Arc<Mutex<dyn ExecutorStrategyExt>> {
if config.zksync.run_in_zk_mode() {
if config.zksync.should_compile() {
Arc::new(Mutex::new(ZksyncExecutorStrategy::default()))
} else {
Arc::new(Mutex::new(EvmExecutorStrategy::default()))
Expand Down
2 changes: 1 addition & 1 deletion crates/config/src/zksync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub struct ZkSyncConfig {
impl Default for ZkSyncConfig {
fn default() -> Self {
Self {
compile: Default::default(),
compile: true,
startup: true,
zksolc: Default::default(),
solc_path: Default::default(),
Expand Down
29 changes: 3 additions & 26 deletions crates/evm/core/src/backend/cow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ use alloy_rpc_types::TransactionRequest;
use foundry_fork_db::DatabaseError;
use revm::{
db::DatabaseRef,
primitives::{
Account, AccountInfo, Bytecode, Env, EnvWithHandlerCfg, HashMap as Map, ResultAndState,
SpecId,
},
primitives::{Account, AccountInfo, Bytecode, Env, EnvWithHandlerCfg, HashMap as Map, SpecId},
Database, DatabaseCommit, JournaledState,
};
use std::{
Expand Down Expand Up @@ -49,9 +46,9 @@ pub struct CowBackend<'a> {
/// No calls on the `CowBackend` will ever persistently modify the `backend`'s state.
pub backend: Cow<'a, Backend>,
/// Keeps track of whether the backed is already initialized
is_initialized: bool,
pub is_initialized: bool,
/// The [SpecId] of the current backend.
spec_id: SpecId,
pub spec_id: SpecId,
}

impl<'a> CowBackend<'a> {
Expand All @@ -60,26 +57,6 @@ impl<'a> CowBackend<'a> {
Self { backend: Cow::Borrowed(backend), is_initialized: false, spec_id: SpecId::LATEST }
}

/// Executes the configured transaction of the `env` without committing state changes
///
/// Note: in case there are any cheatcodes executed that modify the environment, this will
/// update the given `env` with the new values.
#[instrument(name = "inspect", level = "debug", skip_all)]
pub fn inspect<I: InspectorExt>(
&mut self,
env: &mut EnvWithHandlerCfg,
inspector: &mut I,
) -> eyre::Result<ResultAndState> {
// this is a new call to inspect with a new env, so even if we've cloned the backend
// already, we reset the initialized state
self.is_initialized = false;
self.spec_id = env.handler_cfg.spec_id;

let strategy = self.backend.strategy.clone();
let mut guard = strategy.lock().expect("failed acquiring strategy");
guard.call_inspect(self, env, inspector)
}

pub fn new_borrowed(backend: &'a Backend) -> Self {
Self { backend: Cow::Borrowed(backend), is_initialized: false, spec_id: SpecId::LATEST }
}
Expand Down
58 changes: 3 additions & 55 deletions crates/evm/core/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use revm::{
precompile::{PrecompileSpecId, Precompiles},
primitives::{
Account, AccountInfo, BlobExcessGasAndPrice, Bytecode, Env, EnvWithHandlerCfg, EvmState,
EvmStorageSlot, HashMap as Map, Log, ResultAndState, SpecId, KECCAK_EMPTY,
EvmStorageSlot, HashMap as Map, Log, SpecId, KECCAK_EMPTY,
},
Database, DatabaseCommit, JournaledState,
};
Expand Down Expand Up @@ -675,42 +675,6 @@ impl Backend {
self.inner.has_state_snapshot_failure = has_state_snapshot_failure
}

// /// When creating or switching forks, we update the AccountInfo of the contract
// pub(crate) fn update_fork_db(
// &self,
// active_journaled_state: &mut JournaledState,
// target_fork: &mut Fork,
// zk_state: Option<ZkMergeState>,
// ) {
// self.update_fork_db_contracts(
// self.inner.persistent_accounts.iter().copied(),
// active_journaled_state,
// target_fork,
// zk_state,
// )
// }

// /// Merges the state of all `accounts` from the currently active db into the given `fork`
// pub(crate) fn update_fork_db_contracts(
// &self,
// accounts: impl IntoIterator<Item = Address>,
// active_journaled_state: &mut JournaledState,
// target_fork: &mut Fork,
// zk_state: Option<ZkMergeState>,
// ) {
// if let Some(db) = self.active_fork_db() {
// merge_account_data(accounts, db, active_journaled_state, target_fork, zk_state)
// } else {
// merge_account_data(
// accounts,
// &self.mem_db,
// active_journaled_state,
// target_fork,
// zk_state,
// )
// }
// }

/// Returns the memory db used if not in forking mode
pub fn mem_db(&self) -> &FoundryEvmInMemoryDB {
&self.mem_db
Expand Down Expand Up @@ -801,7 +765,7 @@ impl Backend {
/// Initializes settings we need to keep track of.
///
/// We need to track these mainly to prevent issues when switching between different evms
pub(crate) fn initialize(&mut self, env: &EnvWithHandlerCfg) {
pub fn initialize(&mut self, env: &EnvWithHandlerCfg) {
self.set_caller(env.tx.caller);
self.set_spec_id(env.handler_cfg.spec_id);
}
Expand All @@ -811,23 +775,6 @@ impl Backend {
EnvWithHandlerCfg::new_with_spec_id(Box::new(env), self.inner.spec_id)
}

/// Executes the configured test call of the `env` without committing state changes.
///
/// Note: in case there are any cheatcodes executed that modify the environment, this will
/// update the given `env` with the new values.
#[instrument(name = "inspect", level = "debug", skip_all)]
pub fn inspect<I: InspectorExt>(
&mut self,
env: &mut EnvWithHandlerCfg,
inspector: &mut I,
) -> eyre::Result<ResultAndState> {
self.initialize(env);

let strategy = self.strategy.clone();
let mut guard = strategy.lock().expect("failed acquiring strategy");
guard.call_inspect(self, env, inspector)
}

/// Returns true if the address is a precompile
pub fn is_existing_precompile(&self, addr: &Address) -> bool {
self.inner.precompiles().contains(addr)
Expand Down Expand Up @@ -1978,6 +1925,7 @@ fn update_env_block(env: &mut Env, block: &AnyRpcBlock) {

/// Executes the given transaction and commits state changes to the database _and_ the journaled
/// state, with an inspector.
#[allow(clippy::too_many_arguments)]
fn commit_transaction(
tx: &Transaction<AnyTxEnvelope>,
mut env: EnvWithHandlerCfg,
Expand Down
84 changes: 2 additions & 82 deletions crates/evm/core/src/backend/strategy.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
use std::fmt::Debug;

use crate::InspectorExt;

use super::{BackendInner, DatabaseExt, Fork, ForkDB, ForkType, FoundryEvmInMemoryDB};
use super::{BackendInner, Fork, ForkDB, ForkType, FoundryEvmInMemoryDB};
use alloy_primitives::{Address, U256};
use alloy_rpc_types::serde_helpers::OtherFields;
use eyre::Context;
use revm::{
db::CacheDB,
primitives::{EnvWithHandlerCfg, HashSet, ResultAndState},
DatabaseRef, JournaledState,
};
use revm::{db::CacheDB, primitives::HashSet, DatabaseRef, JournaledState};
use serde::{Deserialize, Serialize};

pub struct BackendStrategyForkInfo<'a> {
Expand Down Expand Up @@ -41,49 +33,6 @@ pub trait BackendStrategy: Debug + Send + Sync {
);

fn merge_db_account_data(&self, addr: Address, active: &ForkDB, fork_db: &mut ForkDB);

fn set_inspect_context(&mut self, other_fields: OtherFields);

/// Executes the configured test call of the `env` without committing state changes.
///
/// Note: in case there are any cheatcodes executed that modify the environment, this will
/// update the given `env` with the new values.
#[instrument(name = "inspect", level = "debug", skip_all)]
fn call_inspect<'i, 'db>(
&mut self,
db: &'db mut dyn DatabaseExt,
env: &mut EnvWithHandlerCfg,
inspector: &'i mut dyn InspectorExt,
) -> eyre::Result<ResultAndState> {
let mut evm = crate::utils::new_evm_with_inspector(db, env.clone(), inspector);

let res = evm.transact().wrap_err("backend: failed while inspecting")?;

env.env = evm.context.evm.inner.env;

Ok(res)
}

/// Executes the configured test call of the `env` without committing state changes.
///
/// Note: in case there are any cheatcodes executed that modify the environment, this will
/// update the given `env` with the new values.
#[instrument(name = "inspect", level = "debug", skip_all)]
fn transact_inspect<'i, 'db>(
&mut self,
db: &'db mut dyn DatabaseExt,
env: &mut EnvWithHandlerCfg,
_executor_env: &EnvWithHandlerCfg,
inspector: &'i mut dyn InspectorExt,
) -> eyre::Result<ResultAndState> {
let mut evm = crate::utils::new_evm_with_inspector(db, env.clone(), inspector);

let res = evm.transact().wrap_err("backend: failed while inspecting")?;

env.env = evm.context.evm.inner.env;

Ok(res)
}
}

pub trait BackendStrategyExt: BackendStrategy {
Expand Down Expand Up @@ -139,8 +88,6 @@ impl BackendStrategy for EvmBackendStrategy {
fn merge_db_account_data(&self, addr: Address, active: &ForkDB, fork_db: &mut ForkDB) {
EvmBackendMergeStrategy::merge_db_account_data(addr, active, fork_db);
}

fn set_inspect_context(&mut self, _other_fields: OtherFields) {}
}

impl BackendStrategyExt for EvmBackendStrategy {}
Expand Down Expand Up @@ -246,30 +193,3 @@ impl EvmBackendMergeStrategy {
fork_db.accounts.insert(addr, acc);
}
}

// pub trait GlobalStrategy: Debug + Send + Sync + Default + Clone {
// type Backend: BackendStrategy;
// type Executor: ExecutorStrategy;
// type CheatcodeInspector: CheatcodeInspectorStrategy;

// fn backend_strategy() -> Arc<Mutex<Self::Backend>> {
// Self::Backend::new()
// }

// fn executor_strategy() -> Arc<Mutex<Self::Executor>> {
// Self::Executor::new()
// }

// fn cheatcode_strategy() -> Self::CheatcodeInspector {
// Self::CheatcodeInspector::new()
// }
// }

// #[derive(Debug, Default, Clone)]
// pub struct EvmStrategy;

// impl GlobalStrategy for EvmStrategy {
// type Backend = EvmBackendStrategy;
// type Executor = EvmExecutor;
// type CheatcodeInspector = EvmCheatcodeInspector;
// }
35 changes: 23 additions & 12 deletions crates/evm/evm/src/executors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,7 @@ impl Executor {

#[inline]
pub fn set_transaction_other_fields(&mut self, other_fields: OtherFields) {
self.backend
.strategy
.lock()
.expect("failed acquiring strategy")
.set_inspect_context(other_fields);
self.strategy.lock().expect("failed acquiring strategy").set_inspect_context(other_fields);
}

/// Deploys a contract and commits the new state to the underlying database.
Expand Down Expand Up @@ -436,23 +432,38 @@ impl Executor {
pub fn call_with_env(&self, mut env: EnvWithHandlerCfg) -> eyre::Result<RawCallResult> {
let mut inspector = self.inspector().clone();
let mut backend = CowBackend::new_borrowed(self.backend());
// this is a new call to inspect with a new env, so even if we've cloned the backend
// already, we reset the initialized state
backend.is_initialized = false;
backend.spec_id = env.spec_id();

let result = self.strategy.lock().expect("failed acquiring strategy").call_inspect(
&mut backend,
&mut env,
&mut inspector,
)?;

let strategy = backend.backend.strategy.clone(); // clone to take a mutable borrow
let mut guard = strategy.lock().unwrap();
let result = guard.call_inspect(&mut backend, &mut env, &mut inspector)?;
convert_executed_result(env, inspector, result, backend.has_state_snapshot_failure())
}

/// Execute the transaction configured in `env.tx`.
///
/// Executes the configured transaction of the `env` without committing state changes
///
/// Note: in case there are any cheatcodes executed that modify the environment, this will
/// update the given `env` with the new values.
#[instrument(name = "transact", level = "debug", skip_all)]
pub fn transact_with_env(&mut self, mut env: EnvWithHandlerCfg) -> eyre::Result<RawCallResult> {
let mut inspector = self.inspector.clone();
let backend = &mut self.backend;
backend.initialize(&env);

let result_and_state = self
.strategy
.lock()
.expect("failed acquiring strategy")
.transact_inspect(backend, &mut env, &self.env, &mut inspector)?;

let strategy = backend.strategy.clone(); // clone to take a mutable borrow
let mut guard = strategy.lock().unwrap();
let result_and_state =
guard.transact_inspect(backend, &mut env, &self.env, &mut inspector)?;
let mut result = convert_executed_result(
env,
inspector,
Expand Down
Loading
Loading