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: fix re-entrancy in strategies #801

Merged
merged 6 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 5 additions & 8 deletions crates/cheatcodes/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use super::Result;
use crate::{
strategy::{CheatcodeInspectorStrategy, EvmCheatcodeInspectorStrategy},
Vm::Rpc,
};
use crate::{strategy::CheatcodeInspectorStrategy, Vm::Rpc};
use alloy_primitives::{map::AddressHashMap, U256};
use foundry_common::{fs::normalize_path, ContractsByArtifact};
use foundry_compilers::{utils::canonicalize, ProjectPathsConfig};
Expand Down Expand Up @@ -57,7 +54,7 @@ pub struct CheatsConfig {
/// Version of the script/test contract which is currently running.
pub running_version: Option<Version>,
/// The behavior strategy.
pub strategy: Box<dyn CheatcodeInspectorStrategy>,
pub strategy: CheatcodeInspectorStrategy,
/// Whether to enable legacy (non-reverting) assertions.
pub assertions_revert: bool,
/// Optional seed for the RNG algorithm.
Expand All @@ -73,7 +70,7 @@ impl CheatsConfig {
available_artifacts: Option<ContractsByArtifact>,
running_contract: Option<String>,
running_version: Option<Version>,
strategy: Box<dyn CheatcodeInspectorStrategy>,
strategy: CheatcodeInspectorStrategy,
) -> Self {
let mut allowed_paths = vec![config.root.0.clone()];
allowed_paths.extend(config.libs.clone());
Expand Down Expand Up @@ -234,7 +231,7 @@ impl Default for CheatsConfig {
available_artifacts: Default::default(),
running_contract: Default::default(),
running_version: Default::default(),
strategy: Box::new(EvmCheatcodeInspectorStrategy::default()),
strategy: CheatcodeInspectorStrategy::new_evm(),
assertions_revert: true,
seed: None,
}
Expand All @@ -253,7 +250,7 @@ mod tests {
None,
None,
None,
Box::new(EvmCheatcodeInspectorStrategy::default()),
CheatcodeInspectorStrategy::new_evm(),
)
}

Expand Down
18 changes: 8 additions & 10 deletions crates/cheatcodes/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Cheatcode for getNonce_0Call {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account } = self;

ccx.with_strategy(|strategy, ccx| strategy.cheatcode_get_nonce(ccx, *account))
ccx.state.strategy.runner.clone().cheatcode_get_nonce(ccx, *account)
}
}

Expand Down Expand Up @@ -350,7 +350,7 @@ impl Cheatcode for getBlobhashesCall {
impl Cheatcode for rollCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { newHeight } = self;
ccx.with_strategy(|strategy, ccx| strategy.cheatcode_roll(ccx, *newHeight))
ccx.state.strategy.runner.clone().cheatcode_roll(ccx, *newHeight)
}
}

Expand All @@ -372,7 +372,7 @@ impl Cheatcode for txGasPriceCall {
impl Cheatcode for warpCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { newTimestamp } = self;
ccx.with_strategy(|strategy, ccx| strategy.cheatcode_warp(ccx, *newTimestamp))
ccx.state.strategy.runner.clone().cheatcode_warp(ccx, *newTimestamp)
}
}

Expand Down Expand Up @@ -407,40 +407,38 @@ impl Cheatcode for dealCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account: address, newBalance: new_balance } = *self;

ccx.with_strategy(|strategy, ccx| strategy.cheatcode_deal(ccx, address, new_balance))
ccx.state.strategy.runner.clone().cheatcode_deal(ccx, address, new_balance)
}
}

impl Cheatcode for etchCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { target, newRuntimeBytecode } = self;

ccx.with_strategy(|strategy, ccx| strategy.cheatcode_etch(ccx, *target, newRuntimeBytecode))
ccx.state.strategy.runner.clone().cheatcode_etch(ccx, *target, newRuntimeBytecode)
}
}

impl Cheatcode for resetNonceCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account } = self;
ccx.with_strategy(|strategy, ccx| strategy.cheatcode_reset_nonce(ccx, *account))
ccx.state.strategy.runner.clone().cheatcode_reset_nonce(ccx, *account)
}
}

impl Cheatcode for setNonceCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account, newNonce } = *self;

ccx.with_strategy(|strategy, ccx| strategy.cheatcode_set_nonce(ccx, account, newNonce))
ccx.state.strategy.runner.clone().cheatcode_set_nonce(ccx, account, newNonce)
}
}

impl Cheatcode for setNonceUnsafeCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account, newNonce } = *self;

ccx.with_strategy(|strategy, ccx| {
strategy.cheatcode_set_nonce_unsafe(ccx, account, newNonce)
})
ccx.state.strategy.runner.clone().cheatcode_set_nonce_unsafe(ccx, account, newNonce)
}
}

Expand Down
12 changes: 10 additions & 2 deletions crates/cheatcodes/src/evm/fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,11 @@ impl Cheatcode for selectForkCall {
persist_caller(ccx);
check_broadcast(ccx.state)?;

ccx.with_strategy(|strategy, ccx| strategy.zksync_select_fork_vm(ccx.ecx, *forkId));
ccx.state.strategy.runner.zksync_select_fork_vm(
ccx.state.strategy.context.as_mut(),
ccx.ecx,
*forkId,
);
ccx.ecx.db.select_fork(*forkId, &mut ccx.ecx.env, &mut ccx.ecx.journaled_state)?;

Ok(Default::default())
Expand Down Expand Up @@ -281,7 +285,11 @@ fn create_select_fork(ccx: &mut CheatsCtxt, url_or_alias: &str, block: Option<u6

let fork = create_fork_request(ccx, url_or_alias, block)?;
let id = ccx.ecx.db.create_fork(fork)?;
ccx.with_strategy(|strategy, ccx| strategy.zksync_select_fork_vm(ccx.ecx, id));
ccx.state.strategy.runner.zksync_select_fork_vm(
ccx.state.strategy.context.as_mut(),
ccx.ecx,
id,
);
ccx.ecx.db.select_fork(id, &mut ccx.ecx.env, &mut ccx.ecx.journaled_state)?;
Ok(id.abi_encode())
}
Expand Down
8 changes: 2 additions & 6 deletions crates/cheatcodes/src/evm/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ impl Cheatcode for clearMockedCallsCall {
impl Cheatcode for mockCall_0Call {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { callee, data, returnData } = self;
ccx.with_strategy(|strategy, ccx| {
strategy.cheatcode_mock_call(ccx, *callee, data, returnData)
})
ccx.state.strategy.runner.clone().cheatcode_mock_call(ccx, *callee, data, returnData)
}
}

Expand Down Expand Up @@ -85,9 +83,7 @@ impl Cheatcode for mockCalls_1Call {
impl Cheatcode for mockCallRevert_0Call {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { callee, data, revertData } = self;
ccx.with_strategy(|strategy, ccx| {
strategy.cheatcode_mock_call_revert(ccx, *callee, data, revertData)
})
ccx.state.strategy.runner.clone().cheatcode_mock_call_revert(ccx, *callee, data, revertData)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/cheatcodes/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ impl Cheatcode for getArtifactPathByDeployedCodeCall {
impl Cheatcode for getCodeCall {
fn apply(&self, state: &mut Cheatcodes) -> Result {
let Self { artifactPath: path } = self;
state.with_strategy(|strategy, state| strategy.get_artifact_code(state, path, false))
state.strategy.runner.get_artifact_code(state, path, false)
}
}

Expand Down
85 changes: 34 additions & 51 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ pub use utils::CommonCreateInput;

pub type Ecx<'a, 'b, 'c> = &'a mut EvmContext<&'b mut (dyn DatabaseExt + 'c)>;
pub type InnerEcx<'a, 'b, 'c> = &'a mut InnerEvmContext<&'b mut (dyn DatabaseExt + 'c)>;
pub type Strategy<'a> = &'a mut dyn CheatcodeInspectorStrategy;

/// Helper trait for obtaining complete [revm::Inspector] instance from mutable reference to
/// [Cheatcodes].
Expand Down Expand Up @@ -528,7 +527,7 @@ pub struct Cheatcodes {
pub wallets: Option<Wallets>,

/// The behavior strategy.
pub strategy: Option<Box<dyn CheatcodeInspectorStrategy>>,
pub strategy: CheatcodeInspectorStrategy,
}

impl Clone for Cheatcodes {
Expand Down Expand Up @@ -568,7 +567,7 @@ impl Clone for Cheatcodes {
arbitrary_storage: self.arbitrary_storage.clone(),
deprecated: self.deprecated.clone(),
wallets: self.wallets.clone(),
strategy: self.strategy.as_ref().map(|s| s.new_cloned()),
strategy: self.strategy.clone(),
}
}
}
Expand All @@ -588,7 +587,7 @@ impl Cheatcodes {
Self {
fs_commit: true,
labels: config.labels.clone(),
strategy: Some(config.strategy.clone()),
strategy: config.strategy.clone(),
config,
block: Default::default(),
active_delegation: Default::default(),
Expand Down Expand Up @@ -763,7 +762,8 @@ impl Cheatcodes {
if ecx_inner.journaled_state.depth() == broadcast.depth {
input.set_caller(broadcast.new_origin);

self.strategy.as_mut().unwrap().record_broadcastable_create_transactions(
self.strategy.runner.record_broadcastable_create_transactions(
self.strategy.context.as_mut(),
self.config.clone(),
&input,
ecx_inner,
Expand Down Expand Up @@ -801,9 +801,9 @@ impl Cheatcodes {
}]);
}

if let Some(result) = self.with_strategy(|strategy, cheatcodes| {
strategy.zksync_try_create(cheatcodes, ecx, &input, executor)
}) {
if let Some(result) =
self.strategy.runner.clone().zksync_try_create(self, ecx, &input, executor)
{
return Some(result);
}

Expand Down Expand Up @@ -917,10 +917,7 @@ where {
}
}

self.strategy
.as_mut()
.expect("failed acquiring strategy")
.zksync_record_create_address(&outcome);
self.strategy.runner.zksync_record_create_address(self.strategy.context.as_mut(), &outcome);

outcome
}
Expand Down Expand Up @@ -963,10 +960,12 @@ where {
let prev = account.info.nonce;
let nonce = prev.saturating_sub(1);
account.info.nonce = nonce;
self.strategy
.as_mut()
.expect("failed acquiring strategy")
.zksync_sync_nonce(sender, nonce, ecx);
self.strategy.runner.zksync_sync_nonce(
self.strategy.context.as_mut(),
sender,
nonce,
ecx,
);

trace!(target: "cheatcodes", %sender, nonce, prev, "corrected nonce");
}
Expand Down Expand Up @@ -1000,10 +999,7 @@ where {
return None;
}

self.strategy
.as_mut()
.expect("failed acquiring strategy")
.zksync_set_deployer_call_input(call);
self.strategy.runner.zksync_set_deployer_call_input(self.strategy.context.as_mut(), call);

// Handle expected calls

Expand Down Expand Up @@ -1137,17 +1133,15 @@ where {
})
}

self.strategy
.as_mut()
.expect("failed acquiring strategy")
.record_broadcastable_call_transactions(
self.config.clone(),
call,
ecx_inner,
broadcast,
&mut self.broadcastable_transactions,
&mut self.active_delegation,
);
self.strategy.runner.record_broadcastable_call_transactions(
self.strategy.context.as_mut(),
self.config.clone(),
call,
ecx_inner,
broadcast,
&mut self.broadcastable_transactions,
&mut self.active_delegation,
);

let account =
ecx_inner.journaled_state.state().get_mut(&broadcast.new_origin).unwrap();
Expand Down Expand Up @@ -1220,9 +1214,9 @@ where {
}]);
}

if let Some(result) = self.with_strategy(|strategy, cheatcodes| {
strategy.zksync_try_call(cheatcodes, ecx, call, executor)
}) {
if let Some(result) =
self.strategy.runner.clone().zksync_try_call(self, ecx, call, executor)
{
return Some(result);
}

Expand Down Expand Up @@ -1264,17 +1258,6 @@ where {
None => false,
}
}

pub fn with_strategy<F, R>(&mut self, mut f: F) -> R
where
F: FnMut(Strategy, &mut Self) -> R,
{
let mut strategy = self.strategy.take();
let result = f(strategy.as_mut().expect("failed acquiring strategy").as_mut(), self);
self.strategy = strategy;

result
}
}

impl Inspector<&mut dyn DatabaseExt> for Cheatcodes {
Expand All @@ -1294,10 +1277,11 @@ impl Inspector<&mut dyn DatabaseExt> for Cheatcodes {
self.gas_metering.paused_frames.push(interpreter.gas);
}

self.strategy
.as_mut()
.expect("failed acquiring strategy")
.post_initialize_interp(interpreter, ecx);
self.strategy.runner.post_initialize_interp(
self.strategy.context.as_mut(),
interpreter,
ecx,
);
}

#[inline]
Expand Down Expand Up @@ -1342,8 +1326,7 @@ impl Inspector<&mut dyn DatabaseExt> for Cheatcodes {

#[inline]
fn step_end(&mut self, interpreter: &mut Interpreter, ecx: Ecx) {
if self.strategy.as_mut().expect("failed acquiring strategy").pre_step_end(interpreter, ecx)
{
if self.strategy.runner.pre_step_end(self.strategy.context.as_mut(), interpreter, ecx) {
return;
}

Expand Down
12 changes: 0 additions & 12 deletions crates/cheatcodes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ extern crate tracing;

use alloy_primitives::Address;
use foundry_evm_core::backend::DatabaseExt;
use inspector::Strategy;
use revm::{ContextPrecompiles, InnerEvmContext};
use spec::Status;

Expand Down Expand Up @@ -175,15 +174,4 @@ impl CheatsCtxt<'_, '_, '_, '_> {
pub(crate) fn is_precompile(&self, address: &Address) -> bool {
self.precompiles.contains(address)
}

pub(crate) fn with_strategy<F, R>(&mut self, mut f: F) -> R
where
F: FnMut(Strategy, &mut Self) -> R,
{
let mut strategy = self.state.strategy.take();
let result = f(strategy.as_mut().expect("failed acquiring strategy").as_mut(), self);
self.state.strategy = strategy;

result
}
}
Loading
Loading