Skip to content

Commit

Permalink
fix: normalize solc evm if set (#7096)
Browse files Browse the repository at this point in the history
* wip:fix: normalize solc evm if set

* feat: normalize evm version for forge config
  • Loading branch information
mattsse authored Feb 28, 2024
1 parent 6af18e4 commit fa5e71c
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
31 changes: 26 additions & 5 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,13 @@ impl Config {
self.evm_version = self.get_normalized_evm_version();
}

/// Returns the normalized [EvmVersion] if a [SolcReq] is set to a valid version.
/// Returns the normalized [EvmVersion] if a [SolcReq] is set to a valid version or if the solc
/// path is a valid solc binary.
///
/// Otherwise it returns the configured [EvmVersion].
pub fn get_normalized_evm_version(&self) -> EvmVersion {
if let Some(SolcReq::Version(version)) = &self.solc {
if let Some(evm_version) = self.evm_version.normalize_version(version) {
if let Some(version) = self.solc.as_ref().and_then(|solc| solc.try_version().ok()) {
if let Some(evm_version) = self.evm_version.normalize_version(&version) {
return evm_version;
}
}
Expand Down Expand Up @@ -1602,11 +1603,15 @@ impl Config {
///
/// See also <https://github.com/foundry-rs/foundry/issues/7014>
fn normalize_defaults(&mut self, figment: Figment) -> Figment {
if let Ok(version) = figment.extract_inner::<Version>("solc") {
if let Ok(solc) = figment.extract_inner::<SolcReq>("solc") {
// check if evm_version is set
// TODO: add a warning if evm_version is provided but incompatible
if figment.find_value("evm_version").is_err() {
if let Some(version) = self.evm_version.normalize_version(&version) {
if let Some(version) = solc
.try_version()
.ok()
.and_then(|version| self.evm_version.normalize_version(&version))
{
// normalize evm_version based on the provided solc version
self.evm_version = version;
}
Expand Down Expand Up @@ -1999,6 +2004,22 @@ pub enum SolcReq {
Local(PathBuf),
}

impl SolcReq {
/// Tries to get the solc version from the `SolcReq`
///
/// If the `SolcReq` is a `Version` it will return the version, if it's a path to a binary it
/// will try to get the version from the binary.
fn try_version(&self) -> Result<Version, SolcError> {
match self {
SolcReq::Version(version) => Ok(version.clone()),
SolcReq::Local(path) => Solc::new(path).version().map_err(|err| {
warn!("failed to get solc version from {}: {}", path.display(), err);
err
}),
}
}
}

impl<T: AsRef<str>> From<T> for SolcReq {
fn from(s: T) -> Self {
let s = s.as_ref();
Expand Down
5 changes: 4 additions & 1 deletion crates/forge/bin/cmd/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ impl ConfigArgs {
return Ok(())
}

let config = self.try_load_config_unsanitized_emit_warnings()?;
let config = self
.try_load_config_unsanitized_emit_warnings()?
// we explicitly normalize the version, so mimic the behavior when invoking solc
.normalized_evm_version();

let s = if self.basic {
let config = config.into_basic();
Expand Down
8 changes: 8 additions & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,3 +673,11 @@ forgetest_init!(can_resolve_symlink_fs_permissions, |prj, cmd| {
let permission = fs_permissions.find_permission(&config_path.join("config.json")).unwrap();
assert_eq!(permission, FsAccessPermission::Read);
});

// tests if evm version is normalized for config output
forgetest!(normalize_config_evm_version, |_prj, cmd| {
cmd.args(["config", "--use", "0.8.0", "--json"]);
let output = cmd.stdout_lossy();
let config: Config = serde_json::from_str(&output).unwrap();
assert_eq!(config.evm_version, EvmVersion::Istanbul);
});

0 comments on commit fa5e71c

Please sign in to comment.