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 all 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
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 @@ -138,6 +138,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 @@ -318,6 +318,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 @@ -668,7 +670,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 +770,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 +798,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 +1027,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 +1037,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,45 +1046,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 @@ -1874,6 +1886,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 @@ -2882,7 +2895,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
Loading