From afe2718237d423d268bacc13fc5f0dbf01587cf7 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 18 Oct 2024 08:51:34 +0300 Subject: [PATCH 1/4] Support suppressing zksolc errors / warnings --- .../compilers/src/compilers/zksolc/input.rs | 22 +++++++++++-- crates/compilers/src/compilers/zksolc/mod.rs | 3 +- .../src/compilers/zksolc/settings.rs | 33 ++++++++++++++++++- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/compilers/src/compilers/zksolc/input.rs b/crates/compilers/src/compilers/zksolc/input.rs index 0d83cd9c..7e570622 100644 --- a/crates/compilers/src/compilers/zksolc/input.rs +++ b/crates/compilers/src/compilers/zksolc/input.rs @@ -1,4 +1,7 @@ -use super::{settings::ZkSolcSettings, ZkSettings}; +use super::{ + settings::{ZkSolcError, ZkSolcSettings, ZkSolcWarning}, + ZkSettings, +}; use crate::{ compilers::{solc::SolcLanguage, CompilerInput}, solc, @@ -8,6 +11,8 @@ use semver::Version; use serde::{Deserialize, Serialize}; use std::{ borrow::Cow, + collections::HashSet, + mem, path::{Path, PathBuf}, }; @@ -33,7 +38,7 @@ impl CompilerInput for ZkSolcVersionedInput { version: Version, ) -> Self { let ZkSolcSettings { settings, cli_settings } = settings; - let input = ZkSolcInput { language, sources, settings }.sanitized(&version); + let input = ZkSolcInput::new(language, sources, settings).sanitized(&version); Self { solc_version: version, input, cli_settings } } @@ -65,10 +70,15 @@ impl CompilerInput for ZkSolcVersionedInput { /// Input type `zksolc` expects. #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] pub struct ZkSolcInput { pub language: SolcLanguage, pub sources: Sources, pub settings: ZkSettings, + #[serde(default, skip_serializing_if = "HashSet::is_empty")] + pub suppressed_warnings: HashSet, + #[serde(default, skip_serializing_if = "HashSet::is_empty")] + pub suppressed_errors: HashSet, } /// Default `language` field is set to `"Solidity"`. @@ -78,11 +88,19 @@ impl Default for ZkSolcInput { language: SolcLanguage::Solidity, sources: Sources::default(), settings: ZkSettings::default(), + suppressed_warnings: HashSet::default(), + suppressed_errors: HashSet::default(), } } } impl ZkSolcInput { + fn new(language: SolcLanguage, sources: Sources, mut settings: ZkSettings) -> Self { + let suppressed_warnings = mem::take(&mut settings.suppressed_warnings); + let suppressed_errors = mem::take(&mut settings.suppressed_errors); + Self { language, sources, settings, suppressed_warnings, suppressed_errors } + } + /// Removes the `base` path from all source files pub fn strip_prefix(&mut self, base: impl AsRef) { let base = base.as_ref(); diff --git a/crates/compilers/src/compilers/zksolc/mod.rs b/crates/compilers/src/compilers/zksolc/mod.rs index 153b76da..1ea4df71 100644 --- a/crates/compilers/src/compilers/zksolc/mod.rs +++ b/crates/compilers/src/compilers/zksolc/mod.rs @@ -33,8 +33,7 @@ use std::os::unix::fs::PermissionsExt; pub mod input; pub mod settings; -pub use settings::ZkSettings; -pub use settings::ZkSolcSettings; +pub use settings::{ZkSettings, ZkSolcSettings}; pub const ZKSOLC: &str = "zksolc"; pub const ZKSYNC_SOLC_RELEASE: Version = Version::new(1, 0, 1); diff --git a/crates/compilers/src/compilers/zksolc/settings.rs b/crates/compilers/src/compilers/zksolc/settings.rs index 1711f3c4..5f6fde14 100644 --- a/crates/compilers/src/compilers/zksolc/settings.rs +++ b/crates/compilers/src/compilers/zksolc/settings.rs @@ -9,12 +9,31 @@ use foundry_compilers_artifacts::{ use semver::Version; use serde::{Deserialize, Serialize}; use std::{ - collections::BTreeSet, + collections::{BTreeSet, HashSet}, fmt, path::{Path, PathBuf}, str::FromStr, }; +/// `zksolc` warnings that can be suppressed. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +#[non_exhaustive] +pub enum ZkSolcWarning { + /// `txorigin` warning: Using `tx.origin` in place of `msg.sender`. + TxOrigin, +} + +/// `zksolc` errors that can be suppressed. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +#[non_exhaustive] +pub enum ZkSolcError { + /// `sendtransfer` error: Using `send()` or `transfer()` methods on `address payable` instead + /// of `call()`. + SendTransfer, +} + /// zksolc standard json input settings. See: /// https://docs.zksync.io/zk-stack/components/compiler/toolchain/solidity.html#standard-json for differences #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -66,6 +85,12 @@ pub struct ZkSettings { /// Whether to compile via EVM assembly. #[serde(default, rename = "forceEVMLA")] pub force_evmla: bool, + /// Suppressed `zksolc` warnings. + #[serde(default, skip_serializing_if = "HashSet::is_empty")] + pub suppressed_warnings: HashSet, + /// Suppressed `zksolc` errors. + #[serde(default, skip_serializing_if = "HashSet::is_empty")] + pub suppressed_errors: HashSet, } // Analogous to SolcSettings for Zk compiler @@ -143,6 +168,8 @@ impl Default for ZkSettings { enable_eravm_extensions: false, llvm_options: Default::default(), force_evmla: false, + suppressed_errors: Default::default(), + suppressed_warnings: Default::default(), } } } @@ -168,6 +195,8 @@ impl CompilerSettings for ZkSolcSettings { enable_eravm_extensions, llvm_options, force_evmla, + suppressed_warnings, + suppressed_errors, }, .. } = self; @@ -183,6 +212,8 @@ impl CompilerSettings for ZkSolcSettings { && *enable_eravm_extensions == other.settings.enable_eravm_extensions && *llvm_options == other.settings.llvm_options && *force_evmla == other.settings.force_evmla + && *suppressed_warnings == other.settings.suppressed_warnings + && *suppressed_errors == other.settings.suppressed_errors } fn with_remappings(mut self, remappings: &[Remapping]) -> Self { From 92c9908e0ac23727062293326e5dedbf73345b74 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 18 Oct 2024 08:51:45 +0300 Subject: [PATCH 2/4] Test suppressing zksolc errors / warnings --- crates/compilers/tests/zksync.rs | 100 ++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 2 deletions(-) diff --git a/crates/compilers/tests/zksync.rs b/crates/compilers/tests/zksync.rs index efacf653..24b96138 100644 --- a/crates/compilers/tests/zksync.rs +++ b/crates/compilers/tests/zksync.rs @@ -1,11 +1,20 @@ -use std::{collections::HashMap, fs, path::PathBuf, str::FromStr}; +use std::{ + collections::{HashMap, HashSet}, + fs, + path::PathBuf, + str::FromStr, +}; use foundry_compilers::{ buildinfo::BuildInfo, cache::CompilerCache, project_util::*, resolver::parse::SolData, - zksolc::{input::ZkSolcInput, ZkSolcCompiler, ZkSolcSettings}, + zksolc::{ + input::ZkSolcInput, + settings::{ZkSolcError, ZkSolcWarning}, + ZkSolcCompiler, ZkSolcSettings, + }, zksync::{self, artifact_output::zk::ZkArtifactOutput}, Graph, ProjectBuilder, ProjectPathsConfig, }; @@ -42,6 +51,93 @@ fn zksync_can_compile_dapp_sample() { assert_eq!(cache, updated_cache); } +#[test] +fn zksync_can_compile_contract_with_suppressed_errors() { + let _ = tracing_subscriber::fmt() + .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) + .try_init() + .ok(); + let mut project = TempProject::::dapptools().unwrap(); + + project + .add_source( + "Erroneous", + r#" + // SPDX-License-Identifier: MIT OR Apache-2.0 + pragma solidity ^0.8.10; + contract Erroneous { + function distribute(address payable recipient) public { + recipient.send(1); + recipient.transfer(1); + } + } + "#, + ) + .unwrap(); + + let compiled = zksync::project_compile(project.project()).unwrap(); + assert!(compiled.has_compiler_errors()); + + project.project_mut().settings.settings.suppressed_errors = + HashSet::from([ZkSolcError::SendTransfer]); + + let compiled = zksync::project_compile(project.project()).unwrap(); + compiled.assert_success(); + assert!(compiled.find_first("Erroneous").is_some()); +} + +#[test] +fn zksync_can_compile_contract_with_suppressed_warnings() { + let _ = tracing_subscriber::fmt() + .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) + .try_init() + .ok(); + let mut project = TempProject::::dapptools().unwrap(); + + project + .add_source( + "Warning", + r#" + // SPDX-License-Identifier: MIT OR Apache-2.0 + pragma solidity ^0.8.10; + contract Warning { + function test() public view { + require(tx.origin != address(0), "what"); + } + } + "#, + ) + .unwrap(); + + let compiled = zksync::project_compile(project.project()).unwrap(); + compiled.assert_success(); + assert!( + compiled + .compiler_output + .errors + .iter() + .any(|err| err.is_warning() && err.message.contains("tx.origin")), + "{:#?}", + compiled.compiler_output.errors + ); + + project.project_mut().settings.settings.suppressed_warnings = + HashSet::from([ZkSolcWarning::TxOrigin]); + + let compiled = zksync::project_compile(project.project()).unwrap(); + compiled.assert_success(); + assert!(compiled.find_first("Warning").is_some()); + assert!( + !compiled + .compiler_output + .errors + .iter() + .any(|err| err.is_warning() && err.message.contains("tx.origin")), + "{:#?}", + compiled.compiler_output.errors + ); +} + #[test] fn zksync_can_compile_dapp_detect_changes_in_libs() { let _ = tracing_subscriber::fmt() From ba3cf92684e0874df06db2830e88bfcc515609f5 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 14:27:56 +0200 Subject: [PATCH 3/4] Update from upstream --- crates/compilers/src/compilers/zksolc/input.rs | 10 ++++++---- crates/compilers/src/compilers/zksolc/settings.rs | 1 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/compilers/src/compilers/zksolc/input.rs b/crates/compilers/src/compilers/zksolc/input.rs index 7e570622..3d6f5861 100644 --- a/crates/compilers/src/compilers/zksolc/input.rs +++ b/crates/compilers/src/compilers/zksolc/input.rs @@ -12,7 +12,6 @@ use serde::{Deserialize, Serialize}; use std::{ borrow::Cow, collections::HashSet, - mem, path::{Path, PathBuf}, }; @@ -75,6 +74,9 @@ pub struct ZkSolcInput { pub language: SolcLanguage, pub sources: Sources, pub settings: ZkSettings, + // For `zksolc` versions <1.5.7, suppressed warnings / errors were specified on the same level + // as `settings`. For `zksolc` 1.5.7+, they are specified inside `settings`. Since we want to + // support both options at the time, we duplicate fields from `settings` here. #[serde(default, skip_serializing_if = "HashSet::is_empty")] pub suppressed_warnings: HashSet, #[serde(default, skip_serializing_if = "HashSet::is_empty")] @@ -95,9 +97,9 @@ impl Default for ZkSolcInput { } impl ZkSolcInput { - fn new(language: SolcLanguage, sources: Sources, mut settings: ZkSettings) -> Self { - let suppressed_warnings = mem::take(&mut settings.suppressed_warnings); - let suppressed_errors = mem::take(&mut settings.suppressed_errors); + fn new(language: SolcLanguage, sources: Sources, settings: ZkSettings) -> Self { + let suppressed_warnings = settings.suppressed_warnings.clone(); + let suppressed_errors = settings.suppressed_errors.clone(); Self { language, sources, settings, suppressed_warnings, suppressed_errors } } diff --git a/crates/compilers/src/compilers/zksolc/settings.rs b/crates/compilers/src/compilers/zksolc/settings.rs index 5438e241..9c6d6f47 100644 --- a/crates/compilers/src/compilers/zksolc/settings.rs +++ b/crates/compilers/src/compilers/zksolc/settings.rs @@ -17,7 +17,6 @@ use std::{ /// /// The Solidity compiler codegen. -/// #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum Codegen { From c30a2642e5d8f04d6eb8d1defd4d343d77b08187 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 14:41:17 +0200 Subject: [PATCH 4/4] Test error / warning suppression for old compilers --- crates/compilers/tests/zksync.rs | 36 ++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/crates/compilers/tests/zksync.rs b/crates/compilers/tests/zksync.rs index 24b96138..95d3ef2f 100644 --- a/crates/compilers/tests/zksync.rs +++ b/crates/compilers/tests/zksync.rs @@ -13,7 +13,7 @@ use foundry_compilers::{ zksolc::{ input::ZkSolcInput, settings::{ZkSolcError, ZkSolcWarning}, - ZkSolcCompiler, ZkSolcSettings, + ZkSolc, ZkSolcCompiler, ZkSolcSettings, }, zksync::{self, artifact_output::zk::ZkArtifactOutput}, Graph, ProjectBuilder, ProjectPathsConfig, @@ -51,13 +51,13 @@ fn zksync_can_compile_dapp_sample() { assert_eq!(cache, updated_cache); } -#[test] -fn zksync_can_compile_contract_with_suppressed_errors() { +fn test_zksync_can_compile_contract_with_suppressed_errors(compiler: ZkSolcCompiler) { let _ = tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) .try_init() .ok(); let mut project = TempProject::::dapptools().unwrap(); + project.project_mut().compiler = compiler; project .add_source( @@ -87,12 +87,26 @@ fn zksync_can_compile_contract_with_suppressed_errors() { } #[test] -fn zksync_can_compile_contract_with_suppressed_warnings() { +fn zksync_can_compile_contract_with_suppressed_errors() { + test_zksync_can_compile_contract_with_suppressed_errors(ZkSolcCompiler::default()); +} + +#[test] +fn zksync_pre_1_5_7_can_compile_contract_with_suppressed_errors() { + let compiler = ZkSolcCompiler { + zksolc: ZkSolc::get_path_for_version(&semver::Version::new(1, 5, 6)).unwrap(), + solc: Default::default(), + }; + test_zksync_can_compile_contract_with_suppressed_errors(compiler); +} + +fn test_zksync_can_compile_contract_with_suppressed_warnings(compiler: ZkSolcCompiler) { let _ = tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) .try_init() .ok(); let mut project = TempProject::::dapptools().unwrap(); + project.project_mut().compiler = compiler; project .add_source( @@ -138,6 +152,20 @@ fn zksync_can_compile_contract_with_suppressed_warnings() { ); } +#[test] +fn zksync_can_compile_contract_with_suppressed_warnings() { + test_zksync_can_compile_contract_with_suppressed_warnings(ZkSolcCompiler::default()); +} + +#[test] +fn zksync_pre_1_5_7_can_compile_contract_with_suppressed_warnings() { + let compiler = ZkSolcCompiler { + zksolc: ZkSolc::get_path_for_version(&semver::Version::new(1, 5, 6)).unwrap(), + solc: Default::default(), + }; + test_zksync_can_compile_contract_with_suppressed_warnings(compiler); +} + #[test] fn zksync_can_compile_dapp_detect_changes_in_libs() { let _ = tracing_subscriber::fmt()