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: add support for revert strings #1377

Merged
merged 3 commits into from
Apr 23, 2022
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
35 changes: 17 additions & 18 deletions Cargo.lock

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

15 changes: 14 additions & 1 deletion cli/src/cmd/forge/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
opts::forge::CompilerArgs,
};
use clap::{Parser, ValueHint};
use ethers::solc::{remappings::Remapping, utils::canonicalized};
use ethers::solc::{artifacts::RevertStrings, remappings::Remapping, utils::canonicalized};
use foundry_config::{
figment::{
self,
Expand Down Expand Up @@ -138,6 +138,15 @@ pub struct CoreBuildArgs {
)]
#[serde(skip)]
pub config_path: Option<PathBuf>,

#[clap(
help_heading = "PROJECT OPTIONS",
help = r#"How to treat revert (and require) reason strings. Possible values are "default", "strip", "debug" and "verboseDebug""#,
mattsse marked this conversation as resolved.
Show resolved Hide resolved
long = "revert-strings",
value_name = "revert"
)]
#[serde(skip)]
pub revert_strings: Option<RevertStrings>,
}

impl CoreBuildArgs {
Expand Down Expand Up @@ -202,6 +211,10 @@ impl Provider for CoreBuildArgs {
dict.insert("extra_output_files".to_string(), selection.into());
}

if let Some(ref revert) = self.revert_strings {
dict.insert("revert_strings".to_string(), revert.to_string().into());
}

Ok(Map::from([(Config::selected_profile(), dict)]))
}
}
Expand Down
1 change: 1 addition & 0 deletions cli/src/cmd/forge/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ impl Cmd for FlattenArgs {
libraries: vec![],
via_ir: false,
config_path: None,
revert_strings: None,
};

let config = Config::from(&build_args);
Expand Down
1 change: 1 addition & 0 deletions cli/src/cmd/forge/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ impl VerifyArgs {
libraries: vec![],
via_ir: false,
config_path: None,
revert_strings: None,
};

let project = build_args.project()?;
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/it/config.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Contains various tests for checking forge commands related to config values
use ethers::{
prelude::artifacts::YulDetails,
solc::artifacts::RevertStrings,
types::{Address, U256},
};
use forge::executor::opts::EvmOpts;
Expand Down Expand Up @@ -83,6 +84,7 @@ forgetest!(can_extract_config_values, |prj: TestProject, mut cmd: TestCommand| {
},
no_storage_caching: true,
bytecode_hash: Default::default(),
revert_strings: Some(RevertStrings::Strip),
sparse_mode: true,
__non_exhaustive: (),
};
Expand Down
7 changes: 7 additions & 0 deletions config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ block_base_fee_per_gas = 0
block_coinbase = '0x0000000000000000000000000000000000000000'
block_timestamp = 0
block_difficulty = 0
# How to treat revert (and require) reason strings.
# Possible values are: "default", "strip", "debug" and "verboseDebug".
# "default" does not inject compiler-generated revert strings and keeps user-supplied ones.
# "strip" removes all revert strings (if possible, i.e. if literals are used) keeping side-effects
# "debug" injects strings for compiler-generated internal reverts, implemented for ABI encoders V1 and V2 for now.
# "verboseDebug" even appends further information to user-supplied revert strings (not yet implemented)
revert_strings = "default"
# caches storage retrieved locally for certain chains and endpoints
# can also be restrictied 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
15 changes: 14 additions & 1 deletion config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use crate::caching::StorageCachingConfig;
use ethers_core::types::{Address, U256};
pub use ethers_solc::artifacts::OptimizerDetails;
use ethers_solc::{
artifacts::{output_selection::ContractOutputSelection, BytecodeHash, Optimizer, Settings},
artifacts::{
output_selection::ContractOutputSelection, serde_helpers, BytecodeHash, DebuggingSettings,
Optimizer, RevertStrings, Settings,
},
cache::SOLIDITY_FILES_CACHE_FILENAME,
error::SolcError,
remappings::{RelativeRemapping, Remapping},
Expand Down Expand Up @@ -221,6 +224,9 @@ pub struct Config {
/// The metadata hash is machine dependent. By default, this is set to [BytecodeHash::None] to allow for deterministic code, See: <https://docs.soliditylang.org/en/latest/metadata.html>
#[serde(with = "from_str_lowercase")]
pub bytecode_hash: BytecodeHash,
/// How to treat revert (and require) reason strings.
#[serde(with = "serde_helpers::display_from_str_opt")]
pub revert_strings: Option<RevertStrings>,
/// Whether to compile in sparse mode
///
/// If this option is enabled, only the required contracts/files will be selected to be
Expand Down Expand Up @@ -567,6 +573,10 @@ impl Config {
evm_version: Some(self.evm_version),
libraries,
metadata: Some(self.bytecode_hash.into()),
debug: self.revert_strings.map(|revert_strings| DebuggingSettings {
revert_strings: Some(revert_strings),
debug_info: Vec::new(),
}),
..Default::default()
}
.with_extra_output(self.configured_artifacts_handler().output_selection())
Expand Down Expand Up @@ -939,6 +949,7 @@ impl Default for Config {
rpc_storage_caching: Default::default(),
no_storage_caching: false,
bytecode_hash: BytecodeHash::Ipfs,
revert_strings: None,
sparse_mode: false,
}
}
Expand Down Expand Up @@ -1643,6 +1654,7 @@ mod tests {
via_ir = true
rpc_storage_caching = { chains = [1, "optimism", 999999], endpoints = "all"}
bytecode_hash = "ipfs"
revert_strings = "strip"
"#,
)?;

Expand All @@ -1666,6 +1678,7 @@ mod tests {
endpoints: CachedEndpoints::All
},
bytecode_hash: BytecodeHash::Ipfs,
revert_strings: Some(RevertStrings::Strip),
..Config::default()
}
);
Expand Down