From 6fbf0177a06fd6b0a17e1aef91be5e86f6ecb511 Mon Sep 17 00:00:00 2001 From: evalir Date: Thu, 13 Jul 2023 10:40:01 -0400 Subject: [PATCH] fix(`linking`): correctly handle duplicated libraries, make library paths unique and fix path issues (#5364) * fix: use correct nonce to resolve lib address * fix: adjust test * chore: rm stray code * test: add more runner tests for linking * fix: correct nonce diagnostic * fix: adjust test * chore: forge fmt * chore: forge fmt * fix: resolve libraries correctly on dupe name * test: add more tests * refactor: simplify * chore: clippy * chore: properly strip prefix from all paths on scripting * chore: fmt --------- Co-authored-by: Oliver Nordbjerg --- Cargo.lock | 1 - cli/src/cmd/forge/script/build.rs | 14 +- forge/src/multi_runner.rs | 28 +- forge/tests/it/core.rs | 40 +- testdata/cheats/Broadcast.t.sol | 17 +- testdata/linking/duplicate/Duplicate.t.sol | 126 ++++ .../nested/Nested.t.sol} | 4 +- testdata/linking/simple/Simple.t.sol | 30 + utils/Cargo.toml | 1 - utils/src/lib.rs | 557 ++++++++++++------ 10 files changed, 619 insertions(+), 199 deletions(-) create mode 100644 testdata/linking/duplicate/Duplicate.t.sol rename testdata/{core/LibraryLinking.t.sol => linking/nested/Nested.t.sol} (88%) create mode 100644 testdata/linking/simple/Simple.t.sol diff --git a/Cargo.lock b/Cargo.lock index f9bf668a1da2..aafe5c043ff5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2485,7 +2485,6 @@ dependencies = [ name = "foundry-utils" version = "0.2.0" dependencies = [ - "ethers", "ethers-addressbook", "ethers-contract", "ethers-core", diff --git a/cli/src/cmd/forge/script/build.rs b/cli/src/cmd/forge/script/build.rs index f7f3dbbb4cb2..e8de948d7b32 100644 --- a/cli/src/cmd/forge/script/build.rs +++ b/cli/src/cmd/forge/script/build.rs @@ -14,7 +14,7 @@ use ethers::{ }; use eyre::{Context, ContextCompat}; use foundry_common::compile; -use foundry_utils::PostLinkInput; +use foundry_utils::{PostLinkInput, ResolvedDependency}; use std::{collections::BTreeMap, fs, str::FromStr}; use tracing::{trace, warn}; @@ -29,6 +29,7 @@ impl ScriptArgs { /// Compiles the file with auto-detection and compiler params. pub fn build(&mut self, script_config: &mut ScriptConfig) -> eyre::Result { let (project, output) = self.get_project_and_output(script_config)?; + let output = output.with_stripped_file_prefixes(project.root()); let mut sources: BTreeMap = BTreeMap::new(); @@ -80,6 +81,8 @@ impl ScriptArgs { let mut target_fname = dunce::canonicalize(&self.path) .wrap_err("Couldn't convert contract path to absolute path.")? + .strip_prefix(project.root()) + .wrap_err("Couldn't strip project root from contract path.")? .to_str() .wrap_err("Bad path to string.")? .to_string(); @@ -117,7 +120,6 @@ impl ScriptArgs { sender, nonce, &mut extra_info, - |file, key| (format!("{key}.json:{key}"), file, key), |post_link_input| { let PostLinkInput { contract, @@ -127,14 +129,14 @@ impl ScriptArgs { dependencies, } = post_link_input; - fn unique_deps(deps: Vec<(String, Bytes)>) -> Vec<(String, Bytes)> { + fn unique_deps(deps: Vec) -> Vec<(String, Bytes)> { let mut filtered = Vec::new(); let mut seen = HashSet::new(); - for (dep, bytes) in deps { - if !seen.insert(dep.clone()) { + for dep in deps { + if !seen.insert(dep.id.clone()) { continue } - filtered.push((dep, bytes)); + filtered.push((dep.id, dep.bytecode)); } filtered diff --git a/forge/src/multi_runner.rs b/forge/src/multi_runner.rs index 0e41aa4678be..10b8be22c892 100644 --- a/forge/src/multi_runner.rs +++ b/forge/src/multi_runner.rs @@ -14,10 +14,14 @@ use foundry_evm::{ }, revm, }; -use foundry_utils::PostLinkInput; +use foundry_utils::{PostLinkInput, ResolvedDependency}; use rayon::prelude::*; use revm::primitives::SpecId; -use std::{collections::BTreeMap, path::Path, sync::mpsc::Sender}; +use std::{ + collections::{BTreeMap, HashSet}, + path::Path, + sync::mpsc::Sender, +}; pub type DeployableContracts = BTreeMap)>; @@ -242,6 +246,19 @@ impl MultiContractRunnerBuilder { // create a mapping of name => (abi, deployment code, Vec) let mut deployable_contracts = DeployableContracts::default(); + fn unique_deps(deps: Vec) -> Vec { + let mut filtered = Vec::new(); + let mut seen = HashSet::new(); + for dep in deps { + if !seen.insert(dep.id.clone()) { + continue + } + filtered.push(dep); + } + + filtered + } + foundry_utils::link_with_nonce_or_address( ArtifactContracts::from_iter(contracts), &mut known_contracts, @@ -249,7 +266,6 @@ impl MultiContractRunnerBuilder { evm_opts.sender, U256::one(), &mut deployable_contracts, - |file, key| (format!("{key}.json:{key}"), file, key), |post_link_input| { let PostLinkInput { contract, @@ -258,6 +274,7 @@ impl MultiContractRunnerBuilder { extra: deployable_contracts, dependencies, } = post_link_input; + let dependencies = unique_deps(dependencies); // get bytes let bytecode = @@ -278,10 +295,7 @@ impl MultiContractRunnerBuilder { ( abi.clone(), bytecode, - dependencies - .into_iter() - .map(|(_, bytecode)| bytecode) - .collect::>(), + dependencies.into_iter().map(|dep| dep.bytecode).collect::>(), ), ); } diff --git a/forge/tests/it/core.rs b/forge/tests/it/core.rs index bb61eb624b70..beadfb55ad88 100644 --- a/forge/tests/it/core.rs +++ b/forge/tests/it/core.rs @@ -61,13 +61,6 @@ async fn test_core() { "core/PaymentFailure.t.sol:PaymentFailureTest", vec![("testCantPay()", false, Some("EvmError: Revert".to_string()), None, None)], ), - ( - "core/LibraryLinking.t.sol:LibraryLinkingTest", - vec![ - ("testDirect()", true, None, None, None), - ("testNested()", true, None, None, None), - ], - ), ("core/Abstract.t.sol:AbstractTest", vec![("testSomething()", true, None, None, None)]), ( "core/FailingTestAfterFailedSetup.t.sol:FailingTestAfterFailedSetupTest", @@ -83,6 +76,39 @@ async fn test_core() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn test_linking() { + let mut runner = runner().await; + let results = runner.test(&Filter::new(".*", ".*", ".*linking"), None, test_opts()).await; + + assert_multiple( + &results, + BTreeMap::from([ + ( + "linking/simple/Simple.t.sol:SimpleLibraryLinkingTest", + vec![("testCall()", true, None, None, None)], + ), + ( + "linking/nested/Nested.t.sol:NestedLibraryLinkingTest", + vec![ + ("testDirect()", true, None, None, None), + ("testNested()", true, None, None, None), + ], + ), + ( + "linking/duplicate/Duplicate.t.sol:DuplicateLibraryLinkingTest", + vec![ + ("testA()", true, None, None, None), + ("testB()", true, None, None, None), + ("testC()", true, None, None, None), + ("testD()", true, None, None, None), + ("testE()", true, None, None, None), + ], + ), + ]), + ); +} + #[tokio::test(flavor = "multi_thread")] async fn test_logs() { let mut runner = runner().await; diff --git a/testdata/cheats/Broadcast.t.sol b/testdata/cheats/Broadcast.t.sol index 83ab74e44e59..c7d05acb7c50 100644 --- a/testdata/cheats/Broadcast.t.sol +++ b/testdata/cheats/Broadcast.t.sol @@ -1,9 +1,15 @@ // SPDX-License-Identifier: Unlicense -pragma solidity 0.8.19; +pragma solidity 0.8.18; import "ds-test/test.sol"; import "./Vm.sol"; +library F { + function t2() public pure returns (uint256) { + return 1; + } +} + contract Test is DSTest { uint256 public changed = 0; @@ -27,12 +33,6 @@ contract Test is DSTest { } } -library F { - function t2() public pure returns (uint256) { - return 1; - } -} - contract BroadcastTest is DSTest { Vm constant vm = Vm(HEVM_ADDRESS); @@ -148,10 +148,9 @@ contract BroadcastTest is DSTest { } function deployNoArgs() public { - vm.broadcast(); + vm.startBroadcast(); Test test1 = new Test(); - vm.startBroadcast(); Test test2 = new Test(); vm.stopBroadcast(); } diff --git a/testdata/linking/duplicate/Duplicate.t.sol b/testdata/linking/duplicate/Duplicate.t.sol new file mode 100644 index 000000000000..215e85e34eb5 --- /dev/null +++ b/testdata/linking/duplicate/Duplicate.t.sol @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.18; + +import "ds-test/test.sol"; + +// Linking scenario: contract has many dependencies, some of which appear to the linker +// more than once. +// +// Each library should only have its address computed once + +library A { + function increment(uint256 number, uint256 number2) external pure returns (uint256) { + return number + number2; + } +} + +library B { + function subtract(uint256 number) external pure returns (uint256) { + return number - 1; + } +} + +library C { + function double(uint256 number) external pure returns (uint256) { + return A.increment(number, 0) + A.increment(number, 0); + } +} + +library D { + function half(uint256 number) external pure returns (uint256) { + return number / 2; + } + + function sub2(uint256 number) external pure returns (uint256) { + return B.subtract(number); + } +} + +library E { + function pow(uint256 number, uint256 exponent) external pure returns (uint256) { + return number ** exponent; + } + + function quadruple(uint256 number) external pure returns (uint256) { + return C.double(number) + C.double(number); + } +} + +contract LibraryConsumer { + uint256 public number; + + function setNumber(uint256 newNumber) public { + number = newNumber; + } + + function increment() public { + number++; + } + + function add(uint256 num) external returns (uint256) { + number = num; + return A.increment(num, 1); + } + + function sub(uint256 num) external returns (uint256) { + number = num; + return B.subtract(num); + } + + function mul(uint256 num) external returns (uint256) { + number = num; + return C.double(num); + } + + function div(uint256 num) external returns (uint256) { + number = num; + return D.half(num); + } + + function pow(uint256 num, uint256 exponent) external returns (uint256) { + number = num; + return E.pow(num, exponent); + } + + function sub2(uint256 num) external returns (uint256) { + number = num; + return D.sub2(num); + } + + function quadruple(uint256 num) external returns (uint256) { + number = num; + return E.quadruple(num); + } +} + +contract DuplicateLibraryLinkingTest is DSTest { + LibraryConsumer consumer; + + function setUp() public { + consumer = new LibraryConsumer(); + } + + function testA() public { + assertEq(consumer.add(1), 2, "library call failed"); + } + + function testB() public { + consumer.setNumber(1); + assertEq(consumer.sub(1), 0, "library call failed"); + } + + function testC() public { + consumer.setNumber(2); + assertEq(consumer.mul(2), 4, "library call failed"); + } + + function testD() public { + consumer.setNumber(2); + assertEq(consumer.div(2), 1, "library call failed"); + } + + function testE() public { + consumer.setNumber(2); + assertEq(consumer.quadruple(2), 8, "library call failed"); + } +} diff --git a/testdata/core/LibraryLinking.t.sol b/testdata/linking/nested/Nested.t.sol similarity index 88% rename from testdata/core/LibraryLinking.t.sol rename to testdata/linking/nested/Nested.t.sol index 92d92ec3abdd..b9b85c614a3a 100644 --- a/testdata/core/LibraryLinking.t.sol +++ b/testdata/linking/nested/Nested.t.sol @@ -3,6 +3,8 @@ pragma solidity 0.8.18; import "ds-test/test.sol"; +// Linking scenario: contract with a library that depends on a library + library Lib { function plus100(uint256 a) public pure returns (uint256) { return a + 100; @@ -25,7 +27,7 @@ contract LibraryConsumer { } } -contract LibraryLinkingTest is DSTest { +contract NestedLibraryLinkingTest is DSTest { LibraryConsumer consumer; function setUp() public { diff --git a/testdata/linking/simple/Simple.t.sol b/testdata/linking/simple/Simple.t.sol new file mode 100644 index 000000000000..af8363509b62 --- /dev/null +++ b/testdata/linking/simple/Simple.t.sol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.18; + +import "ds-test/test.sol"; + +// Linking scenario: contract with one library + +library Lib { + function plus100(uint256 a) public pure returns (uint256) { + return a + 100; + } +} + +contract LibraryConsumer { + function consume(uint256 a) public pure returns (uint256) { + return Lib.plus100(a); + } +} + +contract SimpleLibraryLinkingTest is DSTest { + LibraryConsumer consumer; + + function setUp() public { + consumer = new LibraryConsumer(); + } + + function testCall() public { + assertEq(consumer.consume(1), 101, "library call failed"); + } +} diff --git a/utils/Cargo.toml b/utils/Cargo.toml index cbf58c040f7a..3bceb68ecfca 100644 --- a/utils/Cargo.toml +++ b/utils/Cargo.toml @@ -32,5 +32,4 @@ tracing = "0.1" [dev-dependencies] foundry-common = { path = "./../common" } -ethers = { workspace = true, features = ["solc-full"] } pretty_assertions = "1.3.0" diff --git a/utils/src/lib.rs b/utils/src/lib.rs index ff8db14b69f0..7088c2da680f 100644 --- a/utils/src/lib.rs +++ b/utils/src/lib.rs @@ -13,7 +13,7 @@ use futures::future::BoxFuture; use std::{ collections::{BTreeMap, HashMap}, env::VarError, - fmt::Write, + fmt::{Formatter, Write}, path::PathBuf, str::FromStr, time::Duration, @@ -25,27 +25,33 @@ pub mod error; pub mod glob; pub mod rpc; +/// Data passed to the post link handler of the linker for each linked artifact. #[derive(Debug)] pub struct PostLinkInput<'a, T, U> { + /// The fully linked bytecode of the artifact pub contract: CompactContractBytecode, + /// All artifacts passed to the linker pub known_contracts: &'a mut BTreeMap, + /// The ID of the artifact pub id: ArtifactId, + /// Extra data passed to the handler, which can be used as a scratch space. pub extra: &'a mut U, - pub dependencies: Vec<(String, Bytes)>, + /// Each dependency of the contract in their resolved form. + pub dependencies: Vec, } -/// A possible link for a `target` +/// Dependencies for an artifact. #[derive(Debug)] struct ArtifactDependencies { - /// All references of the artifact's bytecode + /// All references to dependencies in the artifact's unlinked bytecode. dependencies: Vec, - /// identifier of the artifact + /// The ID of the artifact artifact_id: ArtifactId, } +/// A dependency of an artifact. #[derive(Debug)] struct ArtifactDependency { - file_name: String, file: String, key: String, } @@ -63,35 +69,33 @@ impl std::fmt::Debug for ArtifactCode { #[derive(Debug)] struct AllArtifactsBySlug { - /// all artifacts grouped by slug + /// all artifacts grouped by identifier inner: BTreeMap, } impl AllArtifactsBySlug { /// Finds the code for the target of the artifact and the matching key. - /// - /// In multi-version builds the identifier also includes the version number. So we try to find - /// that artifact first. If there's no matching, versioned artifact we continue with the - /// `target_slug` - fn find_code( - &self, - artifact: &ArtifactId, - target_slug: &str, - key: &str, - ) -> Option<(String, CompactContractBytecode)> { - // try to find by versioned slug first to match the exact version - let major = artifact.version.major; - let minor = artifact.version.minor; - let patch = artifact.version.patch; - let version_slug = - format!("{key}.{major}.{minor}.{patch}.json:{key}.{major}.{minor}.{patch}"); - let (identifier, code) = if let Some(code) = self.inner.get(&version_slug) { - (version_slug, code) - } else { - let code = self.inner.get(target_slug)?; - (target_slug.to_string(), code) - }; - Some((identifier, code.code.clone())) + fn find_code(&self, identifier: &String) -> Option { + trace!(target : "forge::link", identifier, "fetching artifact by identifier"); + let code = self.inner.get(identifier)?; + + Some(code.code.clone()) + } +} + +#[derive(Debug)] +pub struct ResolvedDependency { + /// The address the linker resolved + pub address: Address, + /// The nonce used to resolve the dependency + pub nonce: U256, + pub id: String, + pub bytecode: Bytes, +} + +impl std::fmt::Display for ResolvedDependency { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{} @ {} (resolved with nonce {})", self.id, self.address, self.nonce) } } @@ -132,21 +136,18 @@ pub fn link_with_nonce_or_address( sender: Address, nonce: U256, extra: &mut U, - link_key_construction: impl Fn(String, String) -> (String, String, String), post_link: impl Fn(PostLinkInput) -> eyre::Result<()>, ) -> Result<()> { // create a mapping of fname => Vec<(fname, file, key)>, let link_tree: BTreeMap = contracts .iter() .map(|(id, contract)| { - let key = id.slug(); + let key = id.identifier(); let references = contract .all_link_references() .iter() - .flat_map(|(file, link)| { - link.keys().map(|key| link_key_construction(file.to_string(), key.to_string())) - }) - .map(|(file_name, file, key)| ArtifactDependency { file_name, file, key }) + .flat_map(|(file, link)| link.keys().map(|key| (file.to_string(), key.to_string()))) + .map(|(file, key)| ArtifactDependency { file, key }) .collect(); let references = @@ -160,7 +161,7 @@ pub fn link_with_nonce_or_address( .iter() .map(|(artifact_id, c)| { ( - artifact_id.slug(), + artifact_id.identifier(), ArtifactCode { code: c.clone(), artifact_id: artifact_id.clone() }, ) }) @@ -189,18 +190,18 @@ pub fn link_with_nonce_or_address( match bytecode.object { BytecodeObject::Unlinked(_) => { - trace!(target : "forge::link", target=id.slug(), version=?id.version, "unlinked contract"); + trace!(target : "forge::link", target=id.identifier(), version=?id.version, "unlinked contract"); // link needed recurse_link( - id.slug(), + id.identifier(), (&mut target_bytecode, &mut target_bytecode_runtime), &artifacts_by_slug, &link_tree, &mut dependencies, &mut internally_deployed_libraries, &deployed_library_addresses, - nonce, + &mut nonce.clone(), sender, ); } @@ -242,14 +243,14 @@ fn recurse_link<'a>( // fname => Vec<(fname, file, key)> dependency_tree: &'a BTreeMap, // library deployment vector (file:contract:address, bytecode) - deployment: &'a mut Vec<(String, Bytes)>, + deployment: &'a mut Vec, // libraries we have already deployed during the linking process. // the key is `file:contract` and the value is the address we computed - internally_deployed_libraries: &'a mut HashMap, + internally_deployed_libraries: &'a mut HashMap, // deployed library addresses fname => adddress deployed_library_addresses: &'a Libraries, // nonce to start at - init_nonce: U256, + nonce: &mut U256, // sender sender: Address, ) { @@ -259,11 +260,12 @@ fn recurse_link<'a>( // for each dependency, try to link dependencies.dependencies.iter().for_each(|dep| { - let ArtifactDependency { file_name: next_target, file, key } = dep; + let ArtifactDependency { file, key, .. } = dep; + let next_target = format!("{file}:{key}"); // get the dependency trace!(target : "forge::link", dependency = next_target, file, key, version=?dependencies.artifact_id.version, "get dependency"); - let (next_identifier, artifact) = artifacts - .find_code(&dependencies.artifact_id, next_target, key) + let artifact = artifacts + .find_code(&next_target) .unwrap_or_else(|| panic!("No target contract named {next_target}")) ; let mut next_target_bytecode = artifact @@ -276,18 +278,20 @@ fn recurse_link<'a>( .expect("No target runtime"); // make sure dependency is fully linked - if let Some(deps) = dependency_tree.get(&next_identifier) { + if let Some(deps) = dependency_tree.get(&format!("{file}:{key}")) { if !deps.dependencies.is_empty() { + trace!(target : "forge::link", dependency = next_target, file, key, version=?dependencies.artifact_id.version, "dependency has dependencies"); + // actually link the nested dependencies to this dependency recurse_link( - next_identifier, + format!("{file}:{key}"), (&mut next_target_bytecode, &mut next_target_runtime_bytecode), artifacts, dependency_tree, deployment, internally_deployed_libraries, deployed_library_addresses, - init_nonce, + nonce, sender, ); } @@ -306,32 +310,43 @@ fn recurse_link<'a>( } let address = if let Some(deployed_address) = deployed_address { - // the user specified the library address + trace!(target : "forge::link", dependency = next_target, file, key, "dependency has pre-defined address"); + // the user specified the library address deployed_address - } else if let Some(deployed_address) = internally_deployed_libraries.get(&format!("{file}:{key}")) { + } else if let Some((cached_nonce, deployed_address)) = internally_deployed_libraries.get(&format!("{file}:{key}")) { + trace!(target : "forge::link", dependency = next_target, file, key, "dependency was previously deployed"); + // we previously deployed the library let library = format!("{file}:{key}:0x{}", hex::encode(deployed_address)); // push the dependency into the library deployment vector - deployment.push(( - library, - next_target_bytecode.object.into_bytes().unwrap_or_else(|| panic!( "Bytecode should be linked for {next_target}")), - )); + deployment.push(ResolvedDependency { + id: library, + address: *deployed_address, + nonce: *cached_nonce, + bytecode: next_target_bytecode.object.into_bytes().unwrap_or_else(|| panic!( "Bytecode should be linked for {next_target}")), + }); *deployed_address } else { + trace!(target : "forge::link", dependency = next_target, file, key, "dependency has to be deployed"); + // we need to deploy the library - let computed_address = ethers_core::utils::get_contract_address(sender, init_nonce + deployment.len()); + let used_nonce = *nonce; + let computed_address = ethers_core::utils::get_contract_address(sender, used_nonce); + *nonce += 1.into(); let library = format!("{file}:{key}:0x{}", hex::encode(computed_address)); // push the dependency into the library deployment vector - deployment.push(( - library, - next_target_bytecode.object.into_bytes().unwrap_or_else(|| panic!( "Bytecode should be linked for {next_target}")), - )); + deployment.push(ResolvedDependency { + id: library, + address: computed_address, + nonce: used_nonce, + bytecode: next_target_bytecode.object.into_bytes().unwrap_or_else(|| panic!( "Bytecode should be linked for {next_target}")), + }); // remember this library for later - internally_deployed_libraries.insert(format!("{file}:{key}"), computed_address); + internally_deployed_libraries.insert(format!("{file}:{key}"), (used_nonce, computed_address)); computed_address }; @@ -339,6 +354,7 @@ fn recurse_link<'a>( // link the dependency to the target target_bytecode.0.link(file.clone(), key.clone(), address); target_bytecode.1.link(file.clone(), key.clone(), address); + trace!(target : "forge::link", ?target, dependency = next_target, file, key, "linking dependency done"); }); } } @@ -462,117 +478,324 @@ pub async fn next_nonce( #[cfg(test)] mod tests { use super::*; - use ethers::{ - abi::Abi, - solc::{Project, ProjectPathsConfig}, - types::{Address, Bytes}, - }; + use ethers_core::types::Address; + use ethers_solc::{Project, ProjectPathsConfig}; use foundry_common::ContractsByArtifact; - #[test] - fn test_linking() { - let mut contract_names = [ - "DSTest.json:DSTest", - "Lib.json:Lib", - "LibraryConsumer.json:LibraryConsumer", - "LibraryLinkingTest.json:LibraryLinkingTest", - "NestedLib.json:NestedLib", - ]; - contract_names.sort_unstable(); - - let paths = ProjectPathsConfig::builder() - .root("../testdata") - .sources("../testdata/core") - .build() - .unwrap(); - - let project = Project::builder().paths(paths).ephemeral().no_artifacts().build().unwrap(); - - let output = project.compile().unwrap(); - let contracts = output - .into_artifacts() - .filter(|(i, _)| contract_names.contains(&i.slug().as_str())) - .map(|(id, c)| (id, c.into_contract_bytecode())) - .collect::(); - - let mut known_contracts = ContractsByArtifact::default(); - let mut deployable_contracts: BTreeMap)> = - Default::default(); - - let mut res = contracts.keys().map(|i| i.slug()).collect::>(); - res.sort_unstable(); - assert_eq!(&res[..], &contract_names[..]); - - let lib_linked = hex::encode( - &contracts - .iter() - .find(|(i, _)| i.slug() == "Lib.json:Lib") + struct LinkerTest { + contracts: ArtifactContracts, + dependency_assertions: HashMap>, + } + + impl LinkerTest { + fn new(path: impl Into) -> Self { + let path = path.into(); + let paths = ProjectPathsConfig::builder() + .root("../testdata/linking") + .lib("../testdata/lib") + .sources(path.clone()) + .tests(path) + .build() + .unwrap(); + + let project = + Project::builder().paths(paths).ephemeral().no_artifacts().build().unwrap(); + let contracts = project + .compile() .unwrap() - .1 - .bytecode - .clone() - .expect("library had no bytecode") - .object - .into_bytes() - .expect("could not get bytecode as bytes"), - ); - let nested_lib_unlinked = &contracts - .iter() - .find(|(i, _)| i.slug() == "NestedLib.json:NestedLib") - .unwrap() - .1 - .bytecode - .as_ref() - .expect("nested library had no bytecode") - .object - .as_str() - .expect("could not get bytecode as str") - .to_string(); - - link_with_nonce_or_address( - contracts, - &mut known_contracts, - Default::default(), - Address::default(), - U256::one(), - &mut deployable_contracts, - |file, key| (format!("{key}.json:{key}"), file, key), - |post_link_input| { - match post_link_input.id.slug().as_str() { - "DSTest.json:DSTest" => { - assert_eq!(post_link_input.dependencies.len(), 0); - } - "LibraryLinkingTest.json:LibraryLinkingTest" => { - assert_eq!(post_link_input.dependencies.len(), 3); - assert_eq!(hex::encode(&post_link_input.dependencies[0].1), lib_linked); - assert_eq!(hex::encode(&post_link_input.dependencies[1].1), lib_linked); - assert_ne!( - hex::encode(&post_link_input.dependencies[2].1), - *nested_lib_unlinked - ); - } - "Lib.json:Lib" => { - assert_eq!(post_link_input.dependencies.len(), 0); - } - "NestedLib.json:NestedLib" => { - assert_eq!(post_link_input.dependencies.len(), 1); - assert_eq!(hex::encode(&post_link_input.dependencies[0].1), lib_linked); + .with_stripped_file_prefixes(project.root()) + .into_artifacts() + .map(|(id, c)| (id, c.into_contract_bytecode())) + .collect::(); + + Self { contracts, dependency_assertions: HashMap::new() } + } + + fn assert_dependencies( + mut self, + artifact_id: String, + deps: Vec<(String, U256, Address)>, + ) -> Self { + self.dependency_assertions.insert(artifact_id, deps); + self + } + + fn test_with_sender_and_nonce(self, sender: Address, initial_nonce: U256) { + let mut called_once = false; + link_with_nonce_or_address( + self.contracts, + &mut ContractsByArtifact::default(), + Default::default(), + sender, + initial_nonce, + &mut called_once, + |post_link_input| { + *post_link_input.extra = true; + let identifier = post_link_input.id.identifier(); + + // Skip ds-test as it always has no dependencies etc. (and the path is outside root so is not sanitized) + if identifier.contains("DSTest") { + return Ok(()) } - "LibraryConsumer.json:LibraryConsumer" => { - assert_eq!(post_link_input.dependencies.len(), 3); - assert_eq!(hex::encode(&post_link_input.dependencies[0].1), lib_linked); - assert_eq!(hex::encode(&post_link_input.dependencies[1].1), lib_linked); - assert_ne!( - hex::encode(&post_link_input.dependencies[2].1), - *nested_lib_unlinked - ); + + let assertions = self + .dependency_assertions + .get(&identifier) + .unwrap_or_else(|| panic!("Unexpected artifact: {identifier}")); + + assert_eq!( + post_link_input.dependencies.len(), + assertions.len(), + "artifact {identifier} has more/less dependencies than expected ({} vs {}): {:#?}", + post_link_input.dependencies.len(), + assertions.len(), + post_link_input.dependencies + ); + + for (expected, actual) in assertions.iter().zip(post_link_input.dependencies.iter()) { + let expected_lib_id = format!("{}:{:?}", expected.0, expected.2); + assert_eq!(expected_lib_id, actual.id, "unexpected dependency, expected: {}, got: {}", expected_lib_id, actual.id); + assert_eq!(actual.nonce, expected.1, "nonce wrong for dependency, expected: {}, got: {}", expected.1, actual.nonce); + assert_eq!(actual.address, expected.2, "address wrong for dependency, expected: {}, got: {}", expected.2, actual.address); } - s => panic!("unexpected slug {s}"), - } - Ok(()) - }, - ) - .unwrap(); + + Ok(()) + }, + ) + .expect("Linking failed"); + + assert!(called_once, "linker did nothing"); + } + } + + #[test] + fn link_simple() { + LinkerTest::new("../testdata/linking/simple") + .assert_dependencies("simple/Simple.t.sol:Lib".to_string(), vec![]) + .assert_dependencies( + "simple/Simple.t.sol:LibraryConsumer".to_string(), + vec![( + "simple/Simple.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + )], + ) + .assert_dependencies( + "simple/Simple.t.sol:SimpleLibraryLinkingTest".to_string(), + vec![( + "simple/Simple.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + )], + ) + .test_with_sender_and_nonce(Address::default(), U256::one()); + } + + #[test] + fn link_nested() { + LinkerTest::new("../testdata/linking/nested") + .assert_dependencies("nested/Nested.t.sol:Lib".to_string(), vec![]) + .assert_dependencies( + "nested/Nested.t.sol:NestedLib".to_string(), + vec![( + "nested/Nested.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + )], + ) + .assert_dependencies( + "nested/Nested.t.sol:LibraryConsumer".to_string(), + vec![ + // Lib shows up here twice, because the linker sees it twice, but it should + // have the same address and nonce. + ( + "nested/Nested.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "nested/Nested.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "nested/Nested.t.sol:NestedLib".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ], + ) + .assert_dependencies( + "nested/Nested.t.sol:NestedLibraryLinkingTest".to_string(), + vec![ + ( + "nested/Nested.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "nested/Nested.t.sol:Lib".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "nested/Nested.t.sol:NestedLib".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ], + ) + .test_with_sender_and_nonce(Address::default(), U256::one()); + } + + /// This test ensures that complicated setups with many libraries, some of which are referenced + /// in more than one place, result in correct linking. + /// + /// Each `assert_dependencies` should be considered in isolation, i.e. read it as "if I wanted + /// to deploy this contract, I would have to deploy the dependencies in this order with this + /// nonce". + /// + /// A library may show up more than once, but it should *always* have the same nonce and address + /// with respect to the single `assert_dependencies` call. There should be no gaps in the nonce + /// otherwise, i.e. whenever a new dependency is encountered, the nonce should be a single + /// increment larger than the previous largest nonce. + #[test] + fn link_duplicate() { + LinkerTest::new("../testdata/linking/duplicate") + .assert_dependencies("duplicate/Duplicate.t.sol:A".to_string(), vec![]) + .assert_dependencies("duplicate/Duplicate.t.sol:B".to_string(), vec![]) + .assert_dependencies( + "duplicate/Duplicate.t.sol:C".to_string(), + vec![( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + )], + ) + .assert_dependencies( + "duplicate/Duplicate.t.sol:D".to_string(), + vec![( + "duplicate/Duplicate.t.sol:B".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + )], + ) + .assert_dependencies( + "duplicate/Duplicate.t.sol:E".to_string(), + vec![ + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:C".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ], + ) + .assert_dependencies( + "duplicate/Duplicate.t.sol:LibraryConsumer".to_string(), + vec![ + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:B".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:C".to_string(), + U256::from(3), + Address::from_str("0x8be503bcded90ed42eff31f56199399b2b0154ca").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:B".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:D".to_string(), + U256::from(4), + Address::from_str("0x47c5e40890bce4a473a49d7501808b9633f29782").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:C".to_string(), + U256::from(3), + Address::from_str("0x8be503bcded90ed42eff31f56199399b2b0154ca").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:E".to_string(), + U256::from(5), + Address::from_str("0x29b2440db4a256b0c1e6d3b4cdcaa68e2440a08f").unwrap(), + ), + ], + ) + .assert_dependencies( + "duplicate/Duplicate.t.sol:DuplicateLibraryLinkingTest".to_string(), + vec![ + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:B".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:C".to_string(), + U256::from(3), + Address::from_str("0x8be503bcded90ed42eff31f56199399b2b0154ca").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:B".to_string(), + U256::from(2), + Address::from_str("0x47e9fbef8c83a1714f1951f142132e6e90f5fa5d").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:D".to_string(), + U256::from(4), + Address::from_str("0x47c5e40890bce4a473a49d7501808b9633f29782").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:A".to_string(), + U256::one(), + Address::from_str("0x5a443704dd4b594b382c22a083e2bd3090a6fef3").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:C".to_string(), + U256::from(3), + Address::from_str("0x8be503bcded90ed42eff31f56199399b2b0154ca").unwrap(), + ), + ( + "duplicate/Duplicate.t.sol:E".to_string(), + U256::from(5), + Address::from_str("0x29b2440db4a256b0c1e6d3b4cdcaa68e2440a08f").unwrap(), + ), + ], + ) + .test_with_sender_and_nonce(Address::default(), U256::one()); } #[test]