Skip to content

Commit

Permalink
fix: debugger doesn't work with external libraries (#7504)
Browse files Browse the repository at this point in the history
* add TestContract

* use TestContract

* wip

* fix

* clippy + fmt

* smaller diff
  • Loading branch information
klkvr authored Apr 2, 2024
1 parent f625d0f commit 85cb9fb
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 98 deletions.
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.

1 change: 1 addition & 0 deletions crates/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ repository.workspace = true
foundry-block-explorers = { workspace = true, features = ["foundry-compilers"] }
foundry-compilers.workspace = true
foundry-config.workspace = true
foundry-linking.workspace = true

ethers-core.workspace = true
ethers-middleware.workspace = true
Expand Down
14 changes: 7 additions & 7 deletions crates/common/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ use comfy_table::{presets::ASCII_MARKDOWN, Attribute, Cell, Color, Table};
use eyre::{Context, Result};
use foundry_block_explorers::contract::Metadata;
use foundry_compilers::{
artifacts::{BytecodeObject, CompactContractBytecode, ContractBytecodeSome},
artifacts::{BytecodeObject, ContractBytecodeSome, Libraries},
remappings::Remapping,
report::{BasicStdoutReporter, NoReporter, Report},
Artifact, ArtifactId, FileFilter, Graph, Project, ProjectCompileOutput, ProjectPathsConfig,
Solc, SolcConfig,
};
use foundry_linking::Linker;
use rustc_hash::FxHashMap;
use std::{
collections::{BTreeMap, HashMap},
Expand Down Expand Up @@ -296,20 +297,19 @@ impl ContractSources {
pub fn from_project_output(
output: &ProjectCompileOutput,
root: &Path,
libraries: &Libraries,
) -> Result<ContractSources> {
let linker = Linker::new(root, output.artifact_ids().collect());

let mut sources = ContractSources::default();
for (id, artifact) in output.artifact_ids() {
if let Some(file_id) = artifact.id {
let abs_path = root.join(&id.source);
let source_code = std::fs::read_to_string(abs_path).wrap_err_with(|| {
format!("failed to read artifact source file for `{}`", id.identifier())
})?;
let compact = CompactContractBytecode {
abi: artifact.abi.clone(),
bytecode: artifact.bytecode.clone(),
deployed_bytecode: artifact.deployed_bytecode.clone(),
};
let contract = compact_to_contract(compact)?;
let linked = linker.link(&id, libraries)?;
let contract = compact_to_contract(linked)?;
sources.insert(&id, file_id, source_code, contract);
} else {
warn!(id = id.identifier(), "source not found");
Expand Down
10 changes: 8 additions & 2 deletions crates/forge/bin/cmd/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,20 @@ impl TestArgs {
let outcome = self.run_tests(runner, config, verbosity, &filter).await?;

if should_debug {
// There is only one test.
let Some((_, test_result)) = outcome.tests().next() else {
// Get first non-empty suite result. We will have only one such entry
let Some((suite_result, test_result)) = outcome
.results
.iter()
.find(|(_, r)| !r.test_results.is_empty())
.map(|(_, r)| (r, r.test_results.values().next().unwrap()))
else {
return Err(eyre::eyre!("no tests were executed"));
};

let sources = ContractSources::from_project_output(
output_clone.as_ref().unwrap(),
project.root(),
&suite_result.libraries,
)?;

// Run the debugger.
Expand Down
49 changes: 31 additions & 18 deletions crates/forge/src/multi_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use alloy_json_abi::{Function, JsonAbi};
use alloy_primitives::{Address, Bytes, U256};
use eyre::Result;
use foundry_common::{get_contract_name, ContractsByArtifact, TestFunctionExt};
use foundry_compilers::{contracts::ArtifactContracts, Artifact, ArtifactId, ProjectCompileOutput};
use foundry_compilers::{
artifacts::Libraries, contracts::ArtifactContracts, Artifact, ArtifactId, ProjectCompileOutput,
};
use foundry_evm::{
backend::Backend,
decode::RevertDecoder,
Expand All @@ -26,7 +28,15 @@ use std::{
time::Instant,
};

pub type DeployableContracts = BTreeMap<ArtifactId, (JsonAbi, Bytes, Vec<Bytes>)>;
#[derive(Debug, Clone)]
pub struct TestContract {
pub abi: JsonAbi,
pub bytecode: Bytes,
pub libs_to_deploy: Vec<Bytes>,
pub libraries: Libraries,
}

pub type DeployableContracts = BTreeMap<ArtifactId, TestContract>;

/// A multi contract runner receives a set of contracts deployed in an EVM instance and proceeds
/// to run all test functions in these contracts.
Expand Down Expand Up @@ -67,8 +77,10 @@ impl MultiContractRunner {
pub fn matching_contracts<'a>(
&'a self,
filter: &'a dyn TestFilter,
) -> impl Iterator<Item = (&ArtifactId, &(JsonAbi, Bytes, Vec<Bytes>))> {
self.contracts.iter().filter(|&(id, (abi, _, _))| matches_contract(id, abi, filter))
) -> impl Iterator<Item = (&ArtifactId, &TestContract)> {
self.contracts
.iter()
.filter(|&(id, TestContract { abi, .. })| matches_contract(id, abi, filter))
}

/// Returns an iterator over all test functions that match the filter.
Expand All @@ -77,7 +89,7 @@ impl MultiContractRunner {
filter: &'a dyn TestFilter,
) -> impl Iterator<Item = &Function> {
self.matching_contracts(filter)
.flat_map(|(_, (abi, _, _))| abi.functions())
.flat_map(|(_, TestContract { abi, .. })| abi.functions())
.filter(|func| is_matching_test(func, filter))
}

Expand All @@ -89,14 +101,14 @@ impl MultiContractRunner {
self.contracts
.iter()
.filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name))
.flat_map(|(_, (abi, _, _))| abi.functions())
.flat_map(|(_, TestContract { abi, .. })| abi.functions())
.filter(|func| func.is_test() || func.is_invariant_test())
}

/// Returns all matching tests grouped by contract grouped by file (file -> (contract -> tests))
pub fn list(&self, filter: &dyn TestFilter) -> BTreeMap<String, BTreeMap<String, Vec<String>>> {
self.matching_contracts(filter)
.map(|(id, (abi, _, _))| {
.map(|(id, TestContract { abi, .. })| {
let source = id.source.as_path().display().to_string();
let name = id.name.clone();
let tests = abi
Expand Down Expand Up @@ -169,22 +181,19 @@ impl MultiContractRunner {
find_time,
);

contracts.par_iter().for_each_with(tx, |tx, &(id, (abi, deploy_code, libs))| {
contracts.par_iter().for_each_with(tx, |tx, &(id, contract)| {
let identifier = id.identifier();
let executor = executor.clone();
let result = self.run_tests(&identifier, abi, executor, deploy_code, libs, filter);
let result = self.run_tests(&identifier, contract, executor, filter);
let _ = tx.send((identifier, result));
})
}

#[allow(clippy::too_many_arguments)]
fn run_tests(
&self,
name: &str,
contract: &JsonAbi,
contract: &TestContract,
executor: Executor,
deploy_code: &Bytes,
libs: &[Bytes],
filter: &dyn TestFilter,
) -> SuiteResult {
let mut span_name = name;
Expand All @@ -199,11 +208,9 @@ impl MultiContractRunner {
name,
executor,
contract,
deploy_code,
self.evm_opts.initial_balance,
self.sender,
&self.revert_decoder,
libs,
self.debug,
);
let r = runner.run_tests(filter, &self.test_options, Some(&self.known_contracts));
Expand Down Expand Up @@ -307,14 +314,17 @@ impl MultiContractRunnerBuilder {
.map(|(i, _)| (i.identifier(), root.join(&i.source).to_string_lossy().into()))
.collect::<BTreeMap<String, String>>();

let linker = Linker::new(root, contracts);
let linker = Linker::new(
root,
contracts.iter().map(|(id, artifact)| (id.clone(), artifact)).collect(),
);

// Create a mapping of name => (abi, deployment code, Vec<library deployment code>)
let mut deployable_contracts = DeployableContracts::default();

let mut known_contracts = ContractsByArtifact::default();

for (id, contract) in &linker.contracts.0 {
for (id, contract) in contracts.iter() {
let Some(abi) = contract.abi.as_ref() else {
continue;
};
Expand All @@ -341,7 +351,10 @@ impl MultiContractRunnerBuilder {
if abi.constructor.as_ref().map(|c| c.inputs.is_empty()).unwrap_or(true) &&
abi.functions().any(|func| func.name.is_test() || func.name.is_invariant_test())
{
deployable_contracts.insert(id.clone(), (abi.clone(), bytecode, libs_to_deploy));
deployable_contracts.insert(
id.clone(),
TestContract { abi: abi.clone(), bytecode, libs_to_deploy, libraries },
);
}

if let Some(bytes) = linked_contract.get_deployed_bytecode_bytes() {
Expand Down
6 changes: 5 additions & 1 deletion crates/forge/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use alloy_primitives::{Address, Log};
use foundry_common::{evm::Breakpoints, get_contract_name, get_file_name, shell};
use foundry_compilers::artifacts::Libraries;
use foundry_evm::{
coverage::HitMaps,
debug::DebugArena,
Expand Down Expand Up @@ -193,15 +194,18 @@ pub struct SuiteResult {
pub test_results: BTreeMap<String, TestResult>,
/// Generated warnings.
pub warnings: Vec<String>,
/// Libraries used to link test contract.
pub libraries: Libraries,
}

impl SuiteResult {
pub fn new(
duration: Duration,
test_results: BTreeMap<String, TestResult>,
warnings: Vec<String>,
libraries: Libraries,
) -> Self {
Self { duration, test_results, warnings }
Self { duration, test_results, warnings, libraries }
}

/// Returns an iterator over all individual succeeding tests and their names.
Expand Down
41 changes: 18 additions & 23 deletions crates/forge/src/runner.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//! The Forge test runner.
use crate::{
multi_runner::is_matching_test,
multi_runner::{is_matching_test, TestContract},
result::{SuiteResult, TestKind, TestResult, TestSetup, TestStatus},
TestFilter, TestOptions,
};
use alloy_json_abi::{Function, JsonAbi};
use alloy_primitives::{Address, Bytes, U256};
use alloy_json_abi::Function;
use alloy_primitives::{Address, U256};
use eyre::Result;
use foundry_common::{
contracts::{ContractsByAddress, ContractsByArtifact},
Expand Down Expand Up @@ -37,14 +37,10 @@ use std::{
#[derive(Clone, Debug)]
pub struct ContractRunner<'a> {
pub name: &'a str,
/// The data of the contract being ran.
pub contract: &'a TestContract,
/// The executor used by the runner.
pub executor: Executor,
/// Library contracts to be deployed before the test contract
pub predeploy_libs: &'a [Bytes],
/// The deployed contract's code
pub code: &'a Bytes,
/// The test contract's ABI
pub contract: &'a JsonAbi,
/// Revert decoder. Contains all known errors.
pub revert_decoder: &'a RevertDecoder,
/// The initial balance of the test contract
Expand All @@ -56,27 +52,22 @@ pub struct ContractRunner<'a> {
}

impl<'a> ContractRunner<'a> {
#[allow(clippy::too_many_arguments)]
pub fn new(
name: &'a str,
executor: Executor,
contract: &'a JsonAbi,
code: &'a Bytes,
contract: &'a TestContract,
initial_balance: U256,
sender: Option<Address>,
revert_decoder: &'a RevertDecoder,
predeploy_libs: &'a [Bytes],
debug: bool,
) -> Self {
Self {
name,
executor,
contract,
code,
initial_balance,
sender: sender.unwrap_or_default(),
revert_decoder,
predeploy_libs,
debug,
}
}
Expand Down Expand Up @@ -104,8 +95,8 @@ impl<'a> ContractRunner<'a> {

// Deploy libraries
let mut logs = Vec::new();
let mut traces = Vec::with_capacity(self.predeploy_libs.len());
for code in self.predeploy_libs.iter() {
let mut traces = Vec::with_capacity(self.contract.libs_to_deploy.len());
for code in self.contract.libs_to_deploy.iter() {
match self.executor.deploy(
self.sender,
code.clone(),
Expand All @@ -131,7 +122,7 @@ impl<'a> ContractRunner<'a> {
// Deploy the test contract
match self.executor.deploy(
self.sender,
self.code.clone(),
self.contract.bytecode.clone(),
U256::ZERO,
Some(self.revert_decoder),
) {
Expand Down Expand Up @@ -194,7 +185,7 @@ impl<'a> ContractRunner<'a> {
let mut warnings = Vec::new();

let setup_fns: Vec<_> =
self.contract.functions().filter(|func| func.name.is_setup()).collect();
self.contract.abi.functions().filter(|func| func.name.is_setup()).collect();

let needs_setup = setup_fns.len() == 1 && setup_fns[0].name == "setUp";

Expand All @@ -215,10 +206,11 @@ impl<'a> ContractRunner<'a> {
[("setUp()".to_string(), TestResult::fail("multiple setUp functions".to_string()))]
.into(),
warnings,
self.contract.libraries.clone(),
)
}

let has_invariants = self.contract.functions().any(|func| func.is_invariant_test());
let has_invariants = self.contract.abi.functions().any(|func| func.is_invariant_test());

// Invariant testing requires tracing to figure out what contracts were created.
let tmp_tracing = self.executor.inspector.tracer.is_none() && has_invariants && needs_setup;
Expand Down Expand Up @@ -251,6 +243,7 @@ impl<'a> ContractRunner<'a> {
)]
.into(),
warnings,
self.contract.libraries.clone(),
)
}

Expand All @@ -259,14 +252,15 @@ impl<'a> ContractRunner<'a> {
let find_timer = Instant::now();
let functions = self
.contract
.abi
.functions()
.filter(|func| is_matching_test(func, filter))
.collect::<Vec<_>>();
let find_time = find_timer.elapsed();
debug!(
"Found {} test functions out of {} in {:?}",
functions.len(),
self.contract.functions().count(),
self.contract.abi.functions().count(),
find_time,
);

Expand Down Expand Up @@ -305,7 +299,8 @@ impl<'a> ContractRunner<'a> {
.collect::<BTreeMap<_, _>>();

let duration = start.elapsed();
let suite_result = SuiteResult::new(duration, test_results, warnings);
let suite_result =
SuiteResult::new(duration, test_results, warnings, self.contract.libraries.clone());
info!(
duration=?suite_result.duration,
"done. {}/{} successful",
Expand Down Expand Up @@ -494,7 +489,7 @@ impl<'a> ContractRunner<'a> {
);

let invariant_contract =
InvariantContract { address, invariant_function: func, abi: self.contract };
InvariantContract { address, invariant_function: func, abi: &self.contract.abi };

let InvariantFuzzTestResult { error, cases, reverts, last_run_inputs, gas_report_traces } =
match evm.invariant_fuzz(invariant_contract.clone()) {
Expand Down
Loading

0 comments on commit 85cb9fb

Please sign in to comment.