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

Replace REVM calls with RPC calls on forge test with a cast-like approach #1

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 10 additions & 11 deletions cli/src/cmd/forge/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,14 @@ impl CoverageArgs {
}
}

impl Cmd for CoverageArgs {
type Output = ();

fn run(self) -> eyre::Result<Self::Output> {
impl CoverageArgs {
pub async fn run(self) -> eyre::Result<()> {
let (mut config, evm_opts) = self.load_config_and_evm_opts_emit_warnings()?;
let project = config.project()?;

// install missing dependencies
if install::install_missing_dependencies(&mut config, &project, self.build_args().silent) &&
config.auto_detect_remappings
if install::install_missing_dependencies(&mut config, &project, self.build_args().silent)
&& config.auto_detect_remappings
{
// need to re-configure here to also catch additional remappings
config = self.load_config();
Expand All @@ -85,7 +83,7 @@ impl Cmd for CoverageArgs {
let report = self.prepare(&config, output.clone())?;

p_println!(!self.opts.silent => "Running tests...");
self.collect(project, output, report, config, evm_opts)
self.collect(project, output, report, config, evm_opts).await
}
}

Expand Down Expand Up @@ -135,7 +133,7 @@ impl CoverageArgs {
for (path, mut source_file, version) in sources.into_sources_with_version() {
// Filter out dependencies
if project_paths.has_library_ancestor(std::path::Path::new(&path)) {
continue
continue;
}

if let Some(ast) = source_file.ast.take() {
Expand Down Expand Up @@ -255,7 +253,7 @@ impl CoverageArgs {
}

/// Runs tests, collects coverage data and generates the final report.
fn collect(
async fn collect(
self,
project: Project,
output: ProjectCompileOutput,
Expand All @@ -281,8 +279,9 @@ impl CoverageArgs {
// Run tests
let known_contracts = runner.known_contracts.clone();
let (tx, rx) = channel::<(String, SuiteResult)>();
let config_clone = config.clone();
let handle =
thread::spawn(move || runner.test(&self.filter, Some(tx), Default::default()).unwrap());
tokio::spawn(async move { runner.test(config_clone, &self.filter, Some(tx), Default::default()).await.unwrap() });

// Add hit data to the coverage report
for (artifact_id, hits) in rx
Expand Down Expand Up @@ -314,7 +313,7 @@ impl CoverageArgs {
}

// Reattach the thread
let _ = handle.join();
let _ = handle.await;

// Output final report
for report_kind in self.report {
Expand Down
12 changes: 5 additions & 7 deletions cli/src/cmd/forge/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,12 @@ impl SnapshotArgs {
}
}

impl Cmd for SnapshotArgs {
type Output = ();

fn run(mut self) -> eyre::Result<()> {
impl SnapshotArgs {
pub async fn run(mut self) -> eyre::Result<()> {
// Set fuzz seed so gas snapshots are deterministic
self.test.fuzz_seed = Some(U256::from_big_endian(&STATIC_FUZZ_SEED));

let outcome = self.test.execute_tests()?;
let outcome = self.test.execute_tests().await?;
outcome.ensure_ok()?;
let tests = self.config.apply(outcome);

Expand Down Expand Up @@ -180,12 +178,12 @@ impl SnapshotConfig {
fn is_in_gas_range(&self, gas_used: u64) -> bool {
if let Some(min) = self.min {
if gas_used < min {
return false
return false;
}
}
if let Some(max) = self.max {
if gas_used > max {
return false
return false;
}
}
true
Expand Down
29 changes: 13 additions & 16 deletions cli/src/cmd/forge/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl TestArgs {
/// configured filter will be executed
///
/// Returns the test results for all matching tests.
pub fn execute_tests(self) -> eyre::Result<TestOutcome> {
pub async fn execute_tests(self) -> eyre::Result<TestOutcome> {
// Merge all configs
let (mut config, mut evm_opts) = self.load_config_and_evm_opts_emit_warnings()?;

Expand Down Expand Up @@ -175,7 +175,7 @@ impl TestArgs {
match runner.count_filtered_tests(&filter) {
1 => {
// Run the test
let results = runner.test(&filter, None, test_options)?;
let results = runner.test(config, &filter, None, test_options).await?;

// Get the result of the single test
let (id, sig, test_kind, counterexample, breakpoints) = results.iter().map(|(id, SuiteResult{ test_results, .. })| {
Expand Down Expand Up @@ -231,7 +231,7 @@ impl TestArgs {
test_options,
self.gas_report,
self.fail_fast,
)
).await
}
}

Expand Down Expand Up @@ -277,13 +277,11 @@ impl Provider for TestArgs {
}
}

impl Cmd for TestArgs {
type Output = TestOutcome;

fn run(self) -> eyre::Result<Self::Output> {
impl TestArgs {
pub async fn run(self) -> eyre::Result<TestOutcome> {
trace!(target: "forge::test", "executing test command");
shell::set_shell(shell::Shell::from_args(self.opts.silent, self.json))?;
self.execute_tests()
self.execute_tests().await
}
}

Expand Down Expand Up @@ -357,7 +355,7 @@ impl TestOutcome {
pub fn ensure_ok(&self) -> eyre::Result<()> {
let failures = self.failures().count();
if self.allow_failure || failures == 0 {
return Ok(())
return Ok(());
}

if !shell::verbosity().is_normal() {
Expand All @@ -370,7 +368,7 @@ impl TestOutcome {
for (suite_name, suite) in self.results.iter() {
let failures = suite.failures().count();
if failures == 0 {
continue
continue;
}

let term = if failures > 1 { "tests" } else { "test" };
Expand Down Expand Up @@ -465,7 +463,7 @@ fn list(

/// Runs all the tests
#[allow(clippy::too_many_arguments)]
fn test(
async fn test(
config: Config,
mut runner: MultiContractRunner,
verbosity: u8,
Expand Down Expand Up @@ -498,7 +496,7 @@ fn test(
}

if json {
let results = runner.test(&filter, None, test_options)?;
let results = runner.test(config, &filter, None, test_options).await?;
println!("{}", serde_json::to_string(&results)?);
Ok(TestOutcome::new(results, allow_failure))
} else {
Expand All @@ -512,7 +510,9 @@ fn test(
let (tx, rx) = channel::<(String, SuiteResult)>();

// Run tests
let handle = thread::spawn(move || runner.test(&filter, Some(tx), test_options).unwrap());
let config_clone = config.clone();
let _ =
tokio::spawn(async move { runner.test(config_clone, &filter, Some(tx), test_options).await.unwrap() }).await;

let mut results: BTreeMap<String, SuiteResult> = BTreeMap::new();
let mut gas_report = GasReport::new(config.gas_reports, config.gas_reports_ignore);
Expand Down Expand Up @@ -617,9 +617,6 @@ fn test(
println!("{}", gas_report.finalize());
}

// reattach the thread
let _ = handle.join();

trace!(target: "forge::test", "received {} results", results.len());
Ok(TestOutcome::new(results, allow_failure))
}
Expand Down
9 changes: 5 additions & 4 deletions cli/src/forge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use foundry_cli::{
utils,
};

fn main() -> eyre::Result<()> {
#[tokio::main]
async fn main() -> eyre::Result<()> {
utils::load_dotenv();
handler::install()?;
utils::subscriber();
Expand All @@ -22,7 +23,7 @@ fn main() -> eyre::Result<()> {
if cmd.is_watch() {
utils::block_on(watch::watch_test(cmd))
} else {
let outcome = cmd.run()?;
let outcome = cmd.run().await?;
outcome.ensure_ok()
}
}
Expand All @@ -34,7 +35,7 @@ fn main() -> eyre::Result<()> {
))?;
utils::block_on(cmd.run_script(Default::default()))
}
Subcommands::Coverage(cmd) => cmd.run(),
Subcommands::Coverage(cmd) => cmd.run().await,
Subcommands::Bind(cmd) => cmd.run(),
Subcommands::Build(cmd) => {
if cmd.is_watch() {
Expand Down Expand Up @@ -78,7 +79,7 @@ fn main() -> eyre::Result<()> {
if cmd.is_watch() {
utils::block_on(watch::watch_snapshot(cmd))
} else {
cmd.run()
cmd.run().await
}
}
Subcommands::Fmt(cmd) => cmd.run(),
Expand Down
4 changes: 3 additions & 1 deletion cli/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ pub fn evm_spec(evm: &EvmVersion) -> SpecId {
EvmVersion::Istanbul => SpecId::ISTANBUL,
EvmVersion::Berlin => SpecId::BERLIN,
EvmVersion::London => SpecId::LONDON,
_ => panic!("Unsupported EVM version"),
// FIXME: This is a temporary patch.
_ => SpecId::LONDON,
// _ => panic!("Unsupported EVM version"),
}
}

Expand Down
2 changes: 2 additions & 0 deletions forge/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ foundry-common = { path = "./../common" }
foundry-config = { path = "./../config" }
foundry-evm = { path = "./../evm" }

cast = { path = "../cast" }

ethers = { workspace = true, features = ["solc-full"] }
eyre = "0.6.8"
semver = "1.0.17"
Expand Down
80 changes: 42 additions & 38 deletions forge/src/multi_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use foundry_evm::{
revm,
};
use foundry_utils::PostLinkInput;
use rayon::prelude::*;
use revm::primitives::SpecId;
use std::{collections::BTreeMap, path::Path, sync::mpsc::Sender};

Expand Down Expand Up @@ -115,8 +114,9 @@ impl MultiContractRunner {
/// before executing all contracts and their tests in _parallel_.
///
/// Each Executor gets its own instance of the `Backend`.
pub fn test(
pub async fn test(
&mut self,
config: foundry_config::Config,
filter: &impl TestFilter,
stream_result: Option<Sender<(String, SuiteResult)>>,
test_options: TestOptions,
Expand All @@ -126,47 +126,50 @@ impl MultiContractRunner {
// the db backend that serves all the data, each contract gets its own instance
let db = Backend::spawn(self.fork.take());

let results = self
.contracts
.par_iter()
.filter(|(id, _)| {
filter.matches_path(id.source.to_string_lossy()) &&
filter.matches_contract(&id.name)
})
.filter(|(_, (abi, _, _))| abi.functions().any(|func| filter.matches_test(&func.name)))
.map(|(id, (abi, deploy_code, libs))| {
let executor = ExecutorBuilder::default()
.with_cheatcodes(self.cheats_config.clone())
.with_config(self.env.clone())
.with_spec(self.evm_spec)
.with_gas_limit(self.evm_opts.gas_limit())
.set_tracing(self.evm_opts.verbosity >= 3)
.set_coverage(self.coverage)
.build(db.clone());
let identifier = id.identifier();
tracing::trace!(contract= ?identifier, "start executing all tests in contract");

let result = self.run_tests(
let mut results = BTreeMap::new();
for (id, (abi, deploy_code, libs)) in &self.contracts {
if !(filter.matches_path(id.source.to_string_lossy())
&& filter.matches_contract(&id.name)
&& abi.functions().any(|func| filter.matches_test(&func.name)))
{
continue;
}

let executor = ExecutorBuilder::default()
.with_cheatcodes(self.cheats_config.clone())
.with_config(self.env.clone())
.with_spec(self.evm_spec)
.with_gas_limit(self.evm_opts.gas_limit())
.set_tracing(self.evm_opts.verbosity >= 3)
.set_coverage(self.coverage)
.build(db.clone());
let identifier = id.identifier();
tracing::trace!(contract= ?identifier, "start executing all tests in contract");

let result = self
.run_tests(
&config,
&identifier,
abi,
executor,
deploy_code.clone(),
libs,
(filter, test_options),
)?;
)
.await?;

tracing::trace!(contract= ?identifier, "executed all tests in contract");
Ok((identifier, result))
})
.filter_map(Result::<_>::ok)
.filter(|(_, results)| !results.is_empty())
.map_with(stream_result, |stream_result, (name, result)| {
if let Some(stream_result) = stream_result.as_ref() {
let _ = stream_result.send((name.clone(), result.clone()));
}
(name, result)
})
.collect::<BTreeMap<_, _>>();
tracing::trace!(contract= ?identifier, "executed all tests in contract");

if result.is_empty() {
continue;
}

if let Some(stream_result) = stream_result.as_ref() {
let _ = stream_result.send((identifier.clone(), result.clone())).unwrap();
}

results.insert(identifier, result);
}

Ok(results)
}
Expand All @@ -178,8 +181,9 @@ impl MultiContractRunner {
err,
fields(name = %_name)
)]
fn run_tests(
async fn run_tests(
&self,
config: &foundry_config::Config,
_name: &str,
contract: &Abi,
executor: Executor,
Expand All @@ -196,7 +200,7 @@ impl MultiContractRunner {
self.errors.as_ref(),
libs,
);
runner.run_tests(filter, test_options, Some(&self.known_contracts))
runner.run_tests(config, filter, test_options, Some(&self.known_contracts)).await
}
}

Expand Down
Loading