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: Linking rewrite #7027

Merged
merged 30 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 14 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
4 changes: 2 additions & 2 deletions Cargo.lock

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

9 changes: 6 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ resolver = "2"
[workspace.package]
version = "0.2.0"
edition = "2021"
rust-version = "1.74" # Remember to update clippy.toml as well
rust-version = "1.74" # Remember to update clippy.toml as well
klkvr marked this conversation as resolved.
Show resolved Hide resolved
authors = ["Foundry Contributors"]
license = "MIT OR Apache-2.0"
homepage = "https://github.com/foundry-rs/foundry"
Expand Down Expand Up @@ -127,7 +127,7 @@ foundry-test-utils = { path = "crates/test-utils" }

# solc & compilation utilities
foundry-block-explorers = { version = "0.2.3", default-features = false }
foundry-compilers = { version = "0.3.2", default-features = false }
foundry-compilers = { version = "0.3.3", default-features = false }

## revm
# no default features to avoid c-kzg
Expand Down Expand Up @@ -173,7 +173,10 @@ alloy-rlp = "0.3.3"
solang-parser = "=0.3.3"

## misc
chrono = { version = "0.4", default-features = false, features = ["clock", "std"] }
chrono = { version = "0.4", default-features = false, features = [
"clock",
"std",
] }
color-eyre = "0.6"
derive_more = "0.99"
eyre = "0.6"
Expand Down
190 changes: 72 additions & 118 deletions crates/forge/bin/cmd/script/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::*;
use alloy_primitives::{Address, Bytes};
use eyre::{Context, ContextCompat, Result};
use forge::link::{link_with_nonce_or_address, PostLinkInput, ResolvedDependency};
use eyre::{Context, ContextCompat, OptionExt, Result};
use forge::link::{link_with_nonce_or_address, LinkOutput};
use foundry_cli::utils::get_cached_entry_by_name;
use foundry_common::{
compact_to_contract,
Expand All @@ -15,7 +15,7 @@ use foundry_compilers::{
info::ContractInfo,
ArtifactId, Project, ProjectCompileOutput,
};
use std::{collections::BTreeMap, str::FromStr};
use std::str::FromStr;

impl ScriptArgs {
/// Compiles the file or project and the verify metadata.
Expand Down Expand Up @@ -60,32 +60,30 @@ impl ScriptArgs {
})
.collect::<Result<ArtifactContracts>>()?;

let mut output = self.link(
let target = self.find_target(&project, &contracts)?.clone();
script_config.target_contract = Some(target.clone());

let libraries = script_config.config.solc_settings()?.libraries;

let mut output = self.link_script_target(
project,
contracts,
script_config.config.parsed_libraries()?,
libraries,
script_config.evm_opts.sender,
script_config.sender_nonce,
target,
)?;

output.sources = sources;
script_config.target_contract = Some(output.target.clone());

Ok(output)
}

pub fn link(
pub fn find_target<'a>(
klkvr marked this conversation as resolved.
Show resolved Hide resolved
&self,
project: Project,
contracts: ArtifactContracts,
libraries_addresses: Libraries,
sender: Address,
nonce: u64,
) -> Result<BuildOutput> {
let mut run_dependencies = vec![];
let mut contract = CompactContractBytecode::default();
let mut highlevel_known_contracts = BTreeMap::new();

project: &Project,
contracts: &'a ArtifactContracts,
) -> Result<&'a ArtifactId> {
let mut target_fname = dunce::canonicalize(&self.path)
.wrap_err("Couldn't convert contract path to absolute path.")?
.strip_prefix(project.root())
Expand All @@ -101,113 +99,79 @@ impl ScriptArgs {
true
};

let mut extra_info = ExtraLinkingInfo {
no_target_name,
target_fname: target_fname.clone(),
contract: &mut contract,
dependencies: &mut run_dependencies,
matched: false,
target_id: None,
};

// link_with_nonce_or_address expects absolute paths
let mut libs = libraries_addresses.clone();
for (file, libraries) in libraries_addresses.libs.iter() {
if file.is_relative() {
let mut absolute_path = project.root().clone();
absolute_path.push(file);
libs.libs.insert(absolute_path, libraries.clone());
}
}

link_with_nonce_or_address(
contracts.clone(),
&mut highlevel_known_contracts,
libs,
sender,
nonce,
&mut extra_info,
|post_link_input| {
let PostLinkInput {
contract,
known_contracts: highlevel_known_contracts,
id,
extra,
dependencies,
} = post_link_input;
let mut target = None;

fn unique_deps(deps: Vec<ResolvedDependency>) -> Vec<(String, Bytes)> {
let mut filtered = Vec::new();
let mut seen = HashSet::new();
for dep in deps {
if !seen.insert(dep.id.clone()) {
continue
}
filtered.push((dep.id, dep.bytecode));
for (id, contract) in contracts.iter() {
if no_target_name {
// Match artifact source, and ignore interfaces
if id.source == std::path::Path::new(&target_fname) &&
contract.bytecode.as_ref().map_or(false, |b| b.object.bytes_len() > 0)
{
if target.is_some() {
eyre::bail!("Multiple contracts in the target path. Please specify the contract name with `--tc ContractName`")
}

filtered
target = Some(id);
}

// if it's the target contract, grab the info
if extra.no_target_name {
// Match artifact source, and ignore interfaces
if id.source == std::path::Path::new(&extra.target_fname) &&
contract.bytecode.as_ref().map_or(false, |b| b.object.bytes_len() > 0)
{
if extra.matched {
eyre::bail!("Multiple contracts in the target path. Please specify the contract name with `--tc ContractName`")
}
*extra.dependencies = unique_deps(dependencies);
*extra.contract = contract.clone();
extra.matched = true;
extra.target_id = Some(id.clone());
}
} else {
let (path, name) = extra
.target_fname
.rsplit_once(':')
.expect("The target specifier is malformed.");
let path = std::path::Path::new(path);
if path == id.source && name == id.name {
*extra.dependencies = unique_deps(dependencies);
*extra.contract = contract.clone();
extra.matched = true;
extra.target_id = Some(id.clone());
}
} else {
let (path, name) =
target_fname.rsplit_once(':').expect("The target specifier is malformed.");
let path = std::path::Path::new(path);
if path == id.source && name == id.name {
target = Some(id);
}
}
}

if let Ok(tc) = ContractBytecode::from(contract).try_into() {
highlevel_known_contracts.insert(id, tc);
}
target.ok_or_eyre(format!("Could not find target contract: {}", target_fname))
}

Ok(())
},
pub fn link_script_target(
klkvr marked this conversation as resolved.
Show resolved Hide resolved
&self,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need &self here or can this also be a standalone function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really need &self here, however I believe that highlevel_known_contracts and Libraries logic is specific to scripts so we can make it a standalone function but still in scope of the scripts

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if we can give this a different name e.g link_script for clarity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Evalir you mean separate it from ScriptArgs and rename?

Copy link
Member

@Evalir Evalir Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@klkvr personally don't mind either separating it or keeping it on ScriptArgs—just think the fn name can be more specific 😄

If we can make this a standalone fn outside ScriptArgs I think it would be nice, as it shows this is explictly a script-related helper.

project: Project,
contracts: ArtifactContracts,
libraries: Libraries,
sender: Address,
nonce: u64,
target: ArtifactId,
) -> Result<BuildOutput> {
let LinkOutput {
libs_to_deploy: predeploy_libraries,
contracts: linked_contracts,
libraries,
} = link_with_nonce_or_address(
&contracts,
libraries,
sender,
nonce,
&target,
project.root(),
)?;

let target = extra_info
.target_id
.ok_or_else(|| eyre::eyre!("Could not find target contract: {}", target_fname))?;

let (new_libraries, predeploy_libraries): (Vec<_>, Vec<_>) =
run_dependencies.into_iter().unzip();

// Merge with user provided libraries
let mut new_libraries = Libraries::parse(&new_libraries)?;
for (file, libraries) in libraries_addresses.libs.into_iter() {
new_libraries.libs.entry(file).or_default().extend(libraries)
}
// Get linked target artifact
let contract = linked_contracts
.get(&target)
.ok_or_eyre("Target contract not found in artifacts")?
.clone();

// Collect all linked contracts
let highlevel_known_contracts = linked_contracts
.iter()
.filter_map(|(id, contract)| {
ContractBytecodeSome::try_from(ContractBytecode::from(contract.clone()))
.ok()
.map(|tc| (id.clone(), tc))
})
.filter(|(_, tc)| !tc.bytecode.object.is_unlinked())
.collect();

Ok(BuildOutput {
target,
contract,
known_contracts: contracts,
highlevel_known_contracts: ArtifactContracts(highlevel_known_contracts),
highlevel_known_contracts,
predeploy_libraries,
sources: Default::default(),
project,
libraries: new_libraries,
libraries,
})
}

Expand Down Expand Up @@ -267,18 +231,8 @@ impl ScriptArgs {
}
}

struct ExtraLinkingInfo<'a> {
no_target_name: bool,
target_fname: String,
contract: &'a mut CompactContractBytecode,
dependencies: &'a mut Vec<(String, Bytes)>,
matched: bool,
target_id: Option<ArtifactId>,
}

pub struct BuildOutput {
pub project: Project,
pub target: ArtifactId,
pub contract: CompactContractBytecode,
pub known_contracts: ArtifactContracts,
pub highlevel_known_contracts: ArtifactContracts<ContractBytecodeSome>,
Expand Down
31 changes: 22 additions & 9 deletions crates/forge/bin/cmd/script/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,16 +272,25 @@ impl ScriptArgs {
}

if self.verify {
let target = self.find_target(&project, &default_known_contracts)?.clone();
let libraries = Libraries::parse(&deployment_sequence.libraries)?
.with_stripped_file_prefixes(project.root());
// We might have predeployed libraries from the broadcasting, so we need to
// relink the contracts with them, since their mapping is
// not included in the solc cache files.
let BuildOutput { highlevel_known_contracts, .. } = self.link(
project,
default_known_contracts,
Libraries::parse(&deployment_sequence.libraries)?,
script_config.config.sender, // irrelevant, since we're not creating any
0, // irrelevant, since we're not creating any
)?;
let BuildOutput { highlevel_known_contracts, predeploy_libraries, .. } = self
.link_script_target(
project,
default_known_contracts,
libraries,
script_config.config.sender, // irrelevant, since we're not creating any
0, // irrelevant, since we're not creating any
target,
)?;

if !predeploy_libraries.is_empty() {
eyre::bail!("Incomplete set of libraries in deployment artifact.");
}

verify.known_contracts = flatten_contracts(&highlevel_known_contracts, false);

Expand Down Expand Up @@ -311,15 +320,19 @@ impl ScriptArgs {
)
.await?;
script_config.sender_nonce = nonce;
let target = self.find_target(&project, &default_known_contracts)?.clone();

let libraries = script_config.config.solc_settings()?.libraries;

let BuildOutput {
libraries, contract, highlevel_known_contracts, predeploy_libraries, ..
} = self.link(
} = self.link_script_target(
project,
default_known_contracts,
script_config.config.parsed_libraries()?,
libraries,
new_sender,
nonce,
target,
)?;

let mut txs = self.create_deploy_transactions(
Expand Down
Loading
Loading