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: don't request Solc JSON AST unless absolutely necessary #7197

Merged
merged 6 commits into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
13 changes: 4 additions & 9 deletions crates/chisel/src/session_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,15 +318,10 @@ impl SessionSource {
let mut sources = Sources::new();
sources.insert(self.file_name.clone(), Source::new(self.to_repl_source()));

let remappings = self.config.foundry_config.get_all_remappings().collect::<Vec<_>>();

// Include Vm.sol if forge-std remapping is not available
if !self.config.no_vm &&
!self
.config
.foundry_config
.get_all_remappings()
.into_iter()
.any(|r| r.name.starts_with("forge-std"))
{
if !self.config.no_vm && !remappings.iter().any(|r| r.name.starts_with("forge-std")) {
sources.insert(PathBuf::from("forge-std/Vm.sol"), Source::new(VM_SOURCE));
}

Expand All @@ -337,7 +332,7 @@ impl SessionSource {
.expect("Solidity source not found");

// get all remappings from the config
compiler_input.settings.remappings = self.config.foundry_config.get_all_remappings();
compiler_input.settings.remappings = remappings;

// We also need to enforce the EVM version that the user has specified.
compiler_input.settings.evm_version = Some(self.config.foundry_config.evm_version);
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub use foundry_config::utils::*;
/// Deterministic fuzzer seed used for gas snapshots and coverage reports.
///
/// The keccak256 hash of "foundry rulez"
pub static STATIC_FUZZ_SEED: [u8; 32] = [
pub const STATIC_FUZZ_SEED: [u8; 32] = [
0x01, 0x00, 0xfa, 0x69, 0xa5, 0xf1, 0x71, 0x0a, 0x95, 0xcd, 0xef, 0x94, 0x88, 0x9b, 0x02, 0x84,
0x5d, 0x64, 0x0b, 0x19, 0xad, 0xf0, 0xe3, 0x57, 0xb8, 0xd4, 0xbe, 0x7d, 0x49, 0xee, 0x70, 0xe6,
];
Expand Down
30 changes: 28 additions & 2 deletions crates/common/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

use crate::{compact_to_contract, glob::GlobMatcher, term::SpinnerReporter, TestFunctionExt};
use comfy_table::{presets::ASCII_MARKDOWN, Attribute, Cell, Color, Table};
use eyre::Result;
use eyre::{Context, Result};
use foundry_block_explorers::contract::Metadata;
use foundry_compilers::{
artifacts::{BytecodeObject, ContractBytecodeSome},
artifacts::{BytecodeObject, CompactContractBytecode, ContractBytecodeSome},
remappings::Remapping,
report::{BasicStdoutReporter, NoReporter, Report},
Artifact, ArtifactId, FileFilter, Graph, Project, ProjectCompileOutput, ProjectPathsConfig,
Expand Down Expand Up @@ -281,6 +281,32 @@ pub struct ContractSources {
}

impl ContractSources {
/// Collects the contract sources and artifacts from the project compile output.
pub fn from_project_output(
output: &ProjectCompileOutput,
root: &Path,
) -> Result<ContractSources> {
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.path);
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)?;
sources.insert(&id, file_id, source_code, contract);
} else {
warn!(id = id.identifier(), "source not found");
}
}
Ok(sources)
}

/// Inserts a contract into the sources.
pub fn insert(
&mut self,
Expand Down
10 changes: 3 additions & 7 deletions crates/common/src/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use alloy_json_abi::{Event, Function, JsonAbi};
use alloy_primitives::{hex, Address, Selector, B256};
use eyre::Result;
use foundry_compilers::{
artifacts::{CompactContractBytecode, ContractBytecodeSome},
ArtifactId, ProjectPathsConfig,
Expand Down Expand Up @@ -33,10 +34,7 @@ impl ContractsByArtifact {

/// Finds a contract which has the same contract name or identifier as `id`. If more than one is
/// found, return error.
pub fn find_by_name_or_identifier(
&self,
id: &str,
) -> eyre::Result<Option<ArtifactWithContractRef>> {
pub fn find_by_name_or_identifier(&self, id: &str) -> Result<Option<ArtifactWithContractRef>> {
let contracts = self
.iter()
.filter(|(artifact, _)| artifact.name == id || artifact.identifier() == id)
Expand Down Expand Up @@ -203,9 +201,7 @@ pub fn get_artifact_path(paths: &ProjectPathsConfig, path: &str) -> PathBuf {
}

/// Helper function to convert CompactContractBytecode ~> ContractBytecodeSome
pub fn compact_to_contract(
contract: CompactContractBytecode,
) -> eyre::Result<ContractBytecodeSome> {
pub fn compact_to_contract(contract: CompactContractBytecode) -> Result<ContractBytecodeSome> {
Ok(ContractBytecodeSome {
abi: contract.abi.ok_or_else(|| eyre::eyre!("No contract abi"))?,
bytecode: contract.bytecode.ok_or_else(|| eyre::eyre!("No contract bytecode"))?.into(),
Expand Down
53 changes: 28 additions & 25 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ impl Config {
.set_auto_detect(self.is_auto_detect())
.set_offline(self.offline)
.set_cached(cached)
.set_build_info(cached & self.build_info)
.set_build_info(cached && self.build_info)
.set_no_artifacts(no_artifacts)
.build()?;

Expand Down Expand Up @@ -768,7 +768,7 @@ impl Config {
.tests(&self.test)
.scripts(&self.script)
.artifacts(&self.out)
.libs(self.libs.clone())
.libs(self.libs.iter())
.remappings(self.get_all_remappings());

if let Some(build_info_path) = &self.build_info_path {
Expand Down Expand Up @@ -796,8 +796,8 @@ impl Config {
/// contracts/tokens/token.sol
/// contracts/math/math.sol
/// ```
pub fn get_all_remappings(&self) -> Vec<Remapping> {
self.remappings.iter().map(|m| m.clone().into()).collect()
pub fn get_all_remappings(&self) -> impl Iterator<Item = Remapping> + '_ {
self.remappings.iter().map(|m| m.clone().into())
}

/// Returns the configured rpc jwt secret
Expand Down Expand Up @@ -1025,6 +1025,7 @@ impl Config {
/// `extra_output` fields
pub fn configured_artifacts_handler(&self) -> ConfigurableArtifacts {
let mut extra_output = self.extra_output.clone();

// Sourcify verification requires solc metadata output. Since, it doesn't
// affect the UX & performance of the compiler, output the metadata files
// by default.
Expand All @@ -1034,7 +1035,7 @@ impl Config {
extra_output.push(ContractOutputSelection::Metadata);
}

ConfigurableArtifacts::new(extra_output, self.extra_output_files.clone())
ConfigurableArtifacts::new(extra_output, self.extra_output_files.iter().cloned())
}

/// Parses all libraries in the form of
Expand All @@ -1043,48 +1044,50 @@ impl Config {
Libraries::parse(&self.libraries)
}

/// Returns all libraries with applied remappings. Same as `self.solc_settings()?.libraries`.
pub fn libraries_with_remappings(&self) -> Result<Libraries, SolcError> {
Ok(self.parsed_libraries()?.with_applied_remappings(&self.project_paths()))
}

/// Returns the configured `solc` `Settings` that includes:
/// - all libraries
/// - the optimizer (including details, if configured)
/// - evm version
/// - all libraries
/// - the optimizer (including details, if configured)
/// - evm version
pub fn solc_settings(&self) -> Result<Settings, SolcError> {
let libraries = self.parsed_libraries()?.with_applied_remappings(&self.project_paths());
let optimizer = self.optimizer();

// By default if no targets are specifically selected the model checker uses all targets.
// This might be too much here, so only enable assertion checks.
// If users wish to enable all options they need to do so explicitly.
let mut model_checker = self.model_checker.clone();
if let Some(ref mut model_checker_settings) = model_checker {
if let Some(model_checker_settings) = &mut model_checker {
if model_checker_settings.targets.is_none() {
model_checker_settings.targets = Some(vec![ModelCheckerTarget::Assert]);
}
}

let mut settings = Settings {
optimizer,
Ok(Settings {
libraries: self.libraries_with_remappings()?,
optimizer: self.optimizer(),
evm_version: Some(self.evm_version),
libraries,
metadata: Some(SettingsMetadata {
use_literal_content: Some(self.use_literal_content),
bytecode_hash: Some(self.bytecode_hash),
cbor_metadata: Some(self.cbor_metadata),
}),
debug: self.revert_strings.map(|revert_strings| DebuggingSettings {
revert_strings: Some(revert_strings),
// Not used.
debug_info: Vec::new(),
}),
model_checker,
..Default::default()
}
.with_extra_output(self.configured_artifacts_handler().output_selection())
.with_ast();

if self.via_ir {
settings = settings.with_via_ir();
via_ir: Some(self.via_ir),
// Not used.
stop_after: None,
// Set in project paths.
remappings: Vec::new(),
// Set with `with_extra_output` below.
output_selection: Default::default(),
}

Ok(settings)
.with_extra_output(self.configured_artifacts_handler().output_selection()))
}

/// Returns the default figment
Expand Down Expand Up @@ -2882,7 +2885,7 @@ mod tests {

// contains additional remapping to the source dir
assert_eq!(
config.get_all_remappings(),
config.get_all_remappings().collect::<Vec<_>>(),
vec![
Remapping::from_str("ds-test/=lib/ds-test/src/").unwrap(),
Remapping::from_str("env-lib/=lib/env-lib/").unwrap(),
Expand Down
3 changes: 3 additions & 0 deletions crates/forge/bin/cmd/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ impl CoverageArgs {
project.solc_config.settings.via_ir = None;
}

// Coverage analysis requires the Solc AST output.
project.solc_config.settings = project.solc_config.settings.with_ast();

project
};

Expand Down
8 changes: 4 additions & 4 deletions crates/forge/bin/cmd/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ impl FlattenArgs {

// flatten is a subset of `BuildArgs` so we can reuse that to get the config
let build_args = CoreBuildArgs { project_paths, ..Default::default() };

let config = build_args.try_load_config_emit_warnings()?;
let mut project = config.ephemeral_no_artifacts_project()?;

let target_path = dunce::canonicalize(target_path)?;

let project = config.ephemeral_no_artifacts_project()?;
// `Flattener` uses the typed AST for better flattening results.
project.solc_config.settings = project.solc_config.settings.with_ast();

let target_path = dunce::canonicalize(target_path)?;
let compiler_output = ProjectCompiler::new().files([target_path.clone()]).compile(&project);

let flattened = match compiler_output {
Expand Down
39 changes: 6 additions & 33 deletions crates/forge/bin/cmd/script/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ use alloy_primitives::{Address, Bytes};
use eyre::{Context, ContextCompat, Result};
use forge::link::{LinkOutput, Linker};
use foundry_cli::utils::get_cached_entry_by_name;
use foundry_common::{
compact_to_contract,
compile::{self, ContractSources, ProjectCompiler},
fs,
};
use foundry_common::compile::{self, ContractSources, ProjectCompiler};
use foundry_compilers::{
artifacts::{ContractBytecode, ContractBytecodeSome, Libraries},
cache::SolFilesCache,
Expand All @@ -28,38 +24,15 @@ impl ScriptArgs {
/// Compiles the file with auto-detection and compiler params.
pub fn build(&mut self, script_config: &mut ScriptConfig) -> Result<BuildOutput> {
let (project, output) = self.get_project_and_output(script_config)?;
let output = output.with_stripped_file_prefixes(project.root());

let mut sources: ContractSources = Default::default();

let contracts = output
.into_artifacts()
.map(|(id, artifact)| -> Result<_> {
// Sources are only required for the debugger, but it *might* mean that there's
// something wrong with the build and/or artifacts.
if let Some(source) = artifact.source_file() {
let path = source
.ast
.ok_or_else(|| eyre::eyre!("source from artifact has no AST"))?
.absolute_path;
let abs_path = project.root().join(path);
let source_code = fs::read_to_string(abs_path).wrap_err_with(|| {
format!("failed to read artifact source file for `{}`", id.identifier())
})?;
let contract = artifact.clone().into_contract_bytecode();
let source_contract = compact_to_contract(contract)?;
sources.insert(&id, source.id, source_code, source_contract);
} else {
warn!(?id, "source not found");
}
Ok((id, artifact))
})
.collect::<Result<ArtifactContracts>>()?;
let root = project.root();
let output = output.with_stripped_file_prefixes(root);
let sources = ContractSources::from_project_output(&output, root)?;
let contracts = output.into_artifacts().collect();

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

let libraries = script_config.config.solc_settings()?.libraries;
let libraries = script_config.config.libraries_with_remappings()?;
let linker = Linker::new(project.root(), contracts);

let (highlevel_known_contracts, libraries, predeploy_libraries) = self.link_script_target(
Expand Down
2 changes: 1 addition & 1 deletion crates/forge/bin/cmd/script/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ impl ScriptArgs {
script_config.sender_nonce = nonce;
let target = script_config.target_contract();

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

let (highlevel_known_contracts, libraries, predeploy_libraries) =
self.link_script_target(&linker, libraries, new_sender, nonce, target.clone())?;
Expand Down
25 changes: 6 additions & 19 deletions crates/forge/bin/cmd/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use foundry_cli::{
utils::{self, LoadConfig},
};
use foundry_common::{
compact_to_contract,
compile::{ContractSources, ProjectCompiler},
evm::EvmArgs,
shell,
Expand All @@ -33,7 +32,7 @@ use foundry_config::{
};
use foundry_debugger::Debugger;
use regex::Regex;
use std::{collections::BTreeMap, fs, sync::mpsc::channel};
use std::{collections::BTreeMap, sync::mpsc::channel};
use watchexec::config::{InitConfig, RuntimeConfig};
use yansi::Paint;

Expand Down Expand Up @@ -213,28 +212,16 @@ impl TestArgs {
let outcome = self.run_tests(runner, config, verbosity, &filter, test_options).await?;

if should_debug {
let mut sources = ContractSources::default();
for (id, artifact) in output_clone.unwrap().into_artifacts() {
// Sources are only required for the debugger, but it *might* mean that there's
// something wrong with the build and/or artifacts.
if let Some(source) = artifact.source_file() {
let path = source
.ast
.ok_or_else(|| eyre::eyre!("Source from artifact has no AST."))?
.absolute_path;
let abs_path = project.root().join(&path);
let source_code = fs::read_to_string(abs_path)?;
let contract = artifact.clone().into_contract_bytecode();
let source_contract = compact_to_contract(contract)?;
sources.insert(&id, source.id, source_code, source_contract);
}
}

// There is only one test.
let Some(test) = outcome.into_tests_cloned().next() else {
return Err(eyre::eyre!("no tests were executed"));
};

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

// Run the debugger.
let mut builder = Debugger::builder()
.debug_arenas(test.result.debug.as_slice())
Expand Down
2 changes: 1 addition & 1 deletion crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ forgetest_init!(
pretty_err(&toml_file, fs::write(&toml_file, config.to_string_pretty().unwrap()));

let config = cmd.config();
let remappings = config.get_all_remappings();
let remappings = config.get_all_remappings().collect::<Vec<_>>();
pretty_assertions::assert_eq!(
remappings,
vec![
Expand Down
Loading