Skip to content

Commit

Permalink
feat: don't request Solc JSON AST unless absolutely necessary (foundr…
Browse files Browse the repository at this point in the history
…y-rs#7197)

* feat: don't request Solc JSON AST unless absolutely necessary

* fix: don't require AST just for the path of a file

* feat: add `--abi` and `abi = <bool>` config values

* fmt

* fix config

* fix: keep AST in build_info
  • Loading branch information
DaniPopes authored and zeroXbrock committed Feb 28, 2024
1 parent 524721a commit cf1ca62
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 133 deletions.
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
4 changes: 4 additions & 0 deletions crates/cli/src/opts/build/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ impl Provider for CoreBuildArgs {
dict.insert("build_info".to_string(), self.build_info.into());
}

if self.compiler.ast {
dict.insert("ast".to_string(), true.into());
}

if self.compiler.optimize {
dict.insert("optimizer".to_string(), self.compiler.optimize.into());
}
Expand Down
5 changes: 5 additions & 0 deletions crates/cli/src/opts/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ pub use self::paths::ProjectPathsArgs;
#[derive(Clone, Debug, Default, Serialize, Parser)]
#[clap(next_help_heading = "Compiler options")]
pub struct CompilerArgs {
/// Includes the AST as JSON in the compiler output.
#[clap(long, help_heading = "Compiler options")]
#[serde(skip)]
pub ast: bool,

/// The target EVM version.
#[clap(long, value_name = "VERSION")]
#[serde(skip_serializing_if = "Option::is_none")]
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
1 change: 1 addition & 0 deletions crates/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ extra_output_files = []
names = false
sizes = false
via_ir = false
ast = false
# caches storage retrieved locally for certain chains and endpoints
# can also be restricted to `chains = ["optimism", "mainnet"]`
# by default all endpoints will be cached, alternative options are "remote" for only caching non localhost endpoints and "<regex>"
Expand Down
53 changes: 33 additions & 20 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ pub struct Config {
/// If set to true, changes compilation pipeline to go through the Yul intermediate
/// representation.
pub via_ir: bool,
/// Whether to include the AST as JSON in the compiler output.
pub ast: bool,
/// RPC storage caching settings determines what chains and endpoints to cache
pub rpc_storage_caching: StorageCachingConfig,
/// Disables storage caching entirely. This overrides any settings made in
Expand Down Expand Up @@ -670,7 +672,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 @@ -770,7 +772,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 @@ -798,8 +800,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 @@ -1027,6 +1029,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 @@ -1036,7 +1039,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 @@ -1045,45 +1048,54 @@ 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,
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()
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(),
}
.with_extra_output(self.configured_artifacts_handler().output_selection())
.with_ast();
.with_extra_output(self.configured_artifacts_handler().output_selection());

if self.via_ir {
settings = settings.with_via_ir();
// We're keeping AST in `--build-info` for backwards compatibility with HardHat.
if self.ast || self.build_info {
settings = settings.with_ast();
}

Ok(settings)
Expand Down Expand Up @@ -1877,6 +1889,7 @@ impl Default for Config {
ignored_file_paths: vec![],
deny_warnings: false,
via_ir: false,
ast: false,
rpc_storage_caching: Default::default(),
rpc_endpoints: Default::default(),
etherscan: Default::default(),
Expand Down Expand Up @@ -2885,7 +2898,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
67 changes: 32 additions & 35 deletions crates/forge/bin/cmd/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ impl CoverageArgs {
// Set fuzz seed so coverage reports are deterministic
config.fuzz.seed = Some(U256::from_be_bytes(STATIC_FUZZ_SEED));

// Coverage analysis requires the Solc AST output.
config.ast = true;

let (project, output) = self.build(&config)?;
p_println!(!self.opts.silent => "Analysing contracts...");
let report = self.prepare(&config, output.clone())?;
Expand All @@ -99,47 +102,41 @@ impl CoverageArgs {
/// Builds the project.
fn build(&self, config: &Config) -> Result<(Project, ProjectCompileOutput)> {
// Set up the project
let project = {
let mut project = config.ephemeral_no_artifacts_project()?;

if self.ir_minimum {
// TODO: How to detect solc version if the user does not specify a solc version in
// config case1: specify local installed solc ?
// case2: multiple solc versions used and auto_detect_solc == true
if let Some(SolcReq::Version(version)) = &config.solc {
if *version < Version::new(0, 8, 13) {
return Err(eyre::eyre!(
let mut project = config.ephemeral_no_artifacts_project()?;
if self.ir_minimum {
// TODO: How to detect solc version if the user does not specify a solc version in
// config case1: specify local installed solc ?
// case2: multiple solc versions used and auto_detect_solc == true
if let Some(SolcReq::Version(version)) = &config.solc {
if *version < Version::new(0, 8, 13) {
return Err(eyre::eyre!(
"viaIR with minimum optimization is only available in Solidity 0.8.13 and above."
));
}
}
}

// print warning message
p_println!(!self.opts.silent => "{}",
Paint::yellow(
concat!(
"Warning! \"--ir-minimum\" flag enables viaIR with minimum optimization, which can result in inaccurate source mappings.\n",
// print warning message
let msg = Paint::yellow(concat!(
"Warning! \"--ir-minimum\" flag enables viaIR with minimum optimization, \
which can result in inaccurate source mappings.\n",
"Only use this flag as a workaround if you are experiencing \"stack too deep\" errors.\n",
"Note that \"viaIR\" is only available in Solidity 0.8.13 and above.\n",
"See more:\n",
"https://github.com/foundry-rs/foundry/issues/3357\n"
)));

// Enable viaIR with minimum optimization
// https://github.com/ethereum/solidity/issues/12533#issuecomment-1013073350
// And also in new releases of solidity:
// https://github.com/ethereum/solidity/issues/13972#issuecomment-1628632202
project.solc_config.settings =
project.solc_config.settings.with_via_ir_minimum_optimization()
} else {
project.solc_config.settings.optimizer.disable();
project.solc_config.settings.optimizer.runs = None;
project.solc_config.settings.optimizer.details = None;
project.solc_config.settings.via_ir = None;
}

project
};
"See more: https://github.com/foundry-rs/foundry/issues/3357",
));
p_println!(!self.opts.silent => "{msg}");

// Enable viaIR with minimum optimization
// https://github.com/ethereum/solidity/issues/12533#issuecomment-1013073350
// And also in new releases of solidity:
// https://github.com/ethereum/solidity/issues/13972#issuecomment-1628632202
project.solc_config.settings =
project.solc_config.settings.with_via_ir_minimum_optimization()
} else {
project.solc_config.settings.optimizer.disable();
project.solc_config.settings.optimizer.runs = None;
project.solc_config.settings.optimizer.details = None;
project.solc_config.settings.via_ir = None;
}

let output = ProjectCompiler::default()
.compile(&project)?
Expand Down
9 changes: 4 additions & 5 deletions crates/forge/bin/cmd/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,12 @@ 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 target_path = dunce::canonicalize(target_path)?;

let mut config = build_args.try_load_config_emit_warnings()?;
// `Flattener` uses the typed AST for better flattening results.
config.ast = true;
let project = config.ephemeral_no_artifacts_project()?;

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
Loading

0 comments on commit cf1ca62

Please sign in to comment.