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

#6616: createSelectFork does not fork latest block number if you sleep #7087

Merged
merged 5 commits into from
Feb 19, 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
103 changes: 53 additions & 50 deletions crates/evm/core/src/fork/multi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ impl MultiFork {

type Handler = BackendHandler<Arc<Provider<BoxTransport>>>;

type CreateFuture = Pin<Box<dyn Future<Output = eyre::Result<(CreatedFork, Handler)>> + Send>>;
type CreateFuture =
Pin<Box<dyn Future<Output = eyre::Result<(ForkId, CreatedFork, Handler)>> + Send>>;
type CreateSender = OneshotSender<eyre::Result<(ForkId, SharedBackend, Env)>>;
type GetEnvSender = OneshotSender<Option<Env>>;

Expand Down Expand Up @@ -258,26 +259,32 @@ impl MultiForkHandler {
let fork_id = ForkId::new(&fork.url, fork.evm_opts.fork_block_number);
trace!(?fork_id, "created new forkId");

if let Some(fork) = self.forks.get(&fork_id).cloned() {
// assign a new unique fork id but reuse the existing backend
let unique_fork_id: ForkId =
format!("{}-{}", fork_id.as_str(), fork.num_senders()).into();
trace!(?fork_id, ?unique_fork_id, "created new unique forkId");
fork.inc_senders();
let backend = fork.backend.clone();
let env = fork.opts.env.clone();
self.forks.insert(unique_fork_id.clone(), fork);
let _ = sender.send(Ok((unique_fork_id, backend, env)));
} else {
// there could already be a task for the requested fork in progress
if let Some(in_progress) = self.find_in_progress_task(&fork_id) {
in_progress.push(sender);
return;
}
// there could already be a task for the requested fork in progress
if let Some(in_progress) = self.find_in_progress_task(&fork_id) {
in_progress.push(sender);
return;
}

// need to create a new fork
let task = Box::pin(create_fork(fork));
self.pending_tasks.push(ForkTask::Create(task, fork_id, sender, Vec::new()));
// need to create a new fork
let task = Box::pin(create_fork(fork));
self.pending_tasks.push(ForkTask::Create(task, fork_id, sender, Vec::new()));
}

fn insert_new_fork(
&mut self,
fork_id: ForkId,
fork: CreatedFork,
sender: CreateSender,
additional_senders: Vec<CreateSender>,
) {
self.forks.insert(fork_id.clone(), fork.clone());
let _ = sender.send(Ok((fork_id.clone(), fork.backend.clone(), fork.opts.env.clone())));

// notify all additional senders and track unique forkIds
for sender in additional_senders {
let next_fork_id = fork.inc_senders(fork_id.clone());
self.forks.insert(next_fork_id.clone(), fork.clone());
let _ = sender.send(Ok((next_fork_id, fork.backend.clone(), fork.opts.env.clone())));
}
}

Expand Down Expand Up @@ -346,25 +353,17 @@ impl Future for MultiForkHandler {
ForkTask::Create(mut fut, id, sender, additional_senders) => {
if let Poll::Ready(resp) = fut.poll_unpin(cx) {
match resp {
Ok((fork, handler)) => {
pin.handlers.push((id.clone(), handler));
let backend = fork.backend.clone();
let env = fork.opts.env.clone();
pin.forks.insert(id.clone(), fork.clone());

let _ = sender.send(Ok((id.clone(), backend.clone(), env.clone())));

// also notify all additional senders and track unique forkIds
for sender in additional_senders {
fork.inc_senders();
let unique_fork_id: ForkId =
format!("{}-{}", id.as_str(), fork.num_senders()).into();
pin.forks.insert(unique_fork_id.clone(), fork.clone());
let _ = sender.send(Ok((
unique_fork_id,
backend.clone(),
env.clone(),
)));
Ok((fork_id, fork, handler)) => {
if let Some(fork) = pin.forks.get(&fork_id).cloned() {
pin.insert_new_fork(
fork.inc_senders(fork_id),
fork,
sender,
additional_senders,
);
} else {
pin.handlers.push((fork_id.clone(), handler));
pin.insert_new_fork(fork_id, fork, sender, additional_senders);
}
}
Err(err) => {
Expand Down Expand Up @@ -446,12 +445,14 @@ impl CreatedFork {
Self { opts, backend, num_senders: Arc::new(AtomicUsize::new(1)) }
}

fn inc_senders(&self) {
self.num_senders.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
}

fn num_senders(&self) -> usize {
self.num_senders.load(std::sync::atomic::Ordering::Relaxed)
/// Increment senders and return unique identifier of the fork
fn inc_senders(&self, fork_id: ForkId) -> ForkId {
format!(
"{}-{}",
fork_id.as_str(),
self.num_senders.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
)
.into()
}
}

Expand Down Expand Up @@ -483,7 +484,7 @@ impl Drop for ShutDownMultiFork {
/// Creates a new fork
///
/// This will establish a new `Provider` to the endpoint and return the Fork Backend
async fn create_fork(mut fork: CreateFork) -> eyre::Result<(CreatedFork, Handler)> {
async fn create_fork(mut fork: CreateFork) -> eyre::Result<(ForkId, CreatedFork, Handler)> {
let provider = Arc::new(
ProviderBuilder::new(fork.url.as_str())
.maybe_max_retry(fork.evm_opts.fork_retries)
Expand All @@ -499,17 +500,19 @@ async fn create_fork(mut fork: CreateFork) -> eyre::Result<(CreatedFork, Handler

// we need to use the block number from the block because the env's number can be different on
// some L2s (e.g. Arbitrum).
let number = block.header.number.unwrap_or(meta.block_env.number);
let number = block.header.number.unwrap_or(meta.block_env.number).to::<u64>();

// determine the cache path if caching is enabled
let cache_path = if fork.enable_caching {
Config::foundry_block_cache_dir(meta.cfg_env.chain_id, number.to::<u64>())
Config::foundry_block_cache_dir(meta.cfg_env.chain_id, number)
} else {
None
};

let db = BlockchainDb::new(meta, cache_path);
let (backend, handler) = SharedBackend::new(provider, db, Some(number.to::<u64>().into()));
let (backend, handler) = SharedBackend::new(provider, db, Some(number.into()));
let fork = CreatedFork::new(fork, backend);
Ok((fork, handler))
let fork_id = ForkId::new(&fork.opts.url, number.into());

Ok((fork_id, fork, handler))
}
3 changes: 3 additions & 0 deletions crates/forge/tests/it/repros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,6 @@ test_repro!(6759);

// https://github.com/foundry-rs/foundry/issues/6966
test_repro!(6966);

// https://github.com/foundry-rs/foundry/issues/6616
test_repro!(6616);
22 changes: 22 additions & 0 deletions testdata/repros/Issue6616.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity 0.8.18;

import "ds-test/test.sol";
import "../cheats/Vm.sol";

// https://github.com/foundry-rs/foundry/issues/6616
contract Issue6616Test is DSTest {
Vm constant vm = Vm(HEVM_ADDRESS);

function testCreateForkRollLatestBlock() public {
vm.createSelectFork("rpcAlias");
uint256 startBlock = block.number;
// this will create new forks and exit once a new latest block is found
for (uint256 i; i < 10; i++) {
vm.sleep(5000);
vm.createSelectFork("rpcAlias");
if (block.number > startBlock) break;
}
assertGt(block.number, startBlock);
}
}
Loading