solidity: update custom warnings and version validation (#193)

This commit is contained in:
xermicus
2025-02-05 11:42:35 +01:00
committed by GitHub
parent de3e7bf253
commit 35419e8202
8 changed files with 112 additions and 143 deletions
+2
View File
@@ -14,12 +14,14 @@ Supported `polkadot-sdk` rev: `274a781e8ca1a9432c7ec87593bd93214abbff50`
- The `resolc` executable prints the help by default.
- Removed support for legacy EVM assembly (EVMLA) translation.
- integration: identify cached code blobs on source code to fix potential confusions.
- Setting base, include or allow paths in emscripten is now a hard error.
### Fixed
- Solidity: Add the solc `--libraries` files to sources.
- A data race in tests.
- Fix `broken pipe` errors.
- llvm-builder: Allow warnings.
- solidity: Fix the custom compiler warning messages.
## v0.1.0-dev.9
+1 -1
View File
@@ -103,7 +103,7 @@ fn main_inner() -> anyhow::Result<()> {
let mut solc = {
#[cfg(target_os = "emscripten")]
{
revive_solidity::SoljsonCompiler { version: None }
revive_solidity::SoljsonCompiler
}
#[cfg(not(target_os = "emscripten"))]
-3
View File
@@ -19,9 +19,6 @@ use self::version::Version;
/// The first version of `solc` with the support of standard JSON interface.
pub const FIRST_SUPPORTED_VERSION: semver::Version = semver::Version::new(0, 8, 0);
/// The first version of `solc`, where `--via-ir` codegen mode is supported.
pub const FIRST_VIA_IR_VERSION: semver::Version = semver::Version::new(0, 8, 13);
/// The last supported version of `solc`.
pub const LAST_SUPPORTED_VERSION: semver::Version = semver::Version::new(0, 8, 28);
+4 -36
View File
@@ -1,4 +1,4 @@
//! The Solidity compiler.
//! The Solidity compiler solc interface.
use std::io::Write;
use std::path::Path;
@@ -8,7 +8,6 @@ use crate::solc::combined_json::CombinedJson;
use crate::solc::standard_json::input::Input as StandardJsonInput;
use crate::solc::standard_json::output::Output as StandardJsonOutput;
use crate::solc::version::Version;
use crate::solc::FIRST_INCLUDE_PATH_VERSION;
use super::Compiler;
@@ -16,8 +15,6 @@ use super::Compiler;
pub struct SolcCompiler {
/// The binary executable name.
pub executable: String,
/// The lazily-initialized compiler version.
pub version: Option<Version>,
}
impl SolcCompiler {
@@ -34,10 +31,7 @@ impl SolcCompiler {
error
);
}
Ok(Self {
executable,
version: None,
})
Ok(Self { executable })
}
}
@@ -50,11 +44,7 @@ impl Compiler for SolcCompiler {
include_paths: Vec<String>,
allow_paths: Option<String>,
) -> anyhow::Result<StandardJsonOutput> {
let version = self.version()?.default;
if !include_paths.is_empty() && version < FIRST_INCLUDE_PATH_VERSION {
anyhow::bail!("--include-path is not supported in solc {version}");
}
let version = self.version()?.validate(&include_paths)?.default;
let mut command = std::process::Command::new(self.executable.as_str());
command.stdin(std::process::Stdio::piped());
@@ -225,10 +215,6 @@ impl Compiler for SolcCompiler {
/// The `solc --version` mini-parser.
fn version(&mut self) -> anyhow::Result<Version> {
if let Some(version) = self.version.as_ref() {
return Ok(version.to_owned());
}
let mut command = std::process::Command::new(self.executable.as_str());
command.arg("--version");
let output = command.output().map_err(|error| {
@@ -274,24 +260,6 @@ impl Compiler for SolcCompiler {
.and_then(|line| line.split('-').nth(1))
.and_then(|version| version.parse().ok());
let version = Version::new(long, default, l2_revision);
if version.default < super::FIRST_SUPPORTED_VERSION {
anyhow::bail!(
"`solc` versions <{} are not supported, found {}",
super::FIRST_SUPPORTED_VERSION,
version.default
);
}
if version.default > super::LAST_SUPPORTED_VERSION {
anyhow::bail!(
"`solc` versions >{} are not supported, found {}",
super::LAST_SUPPORTED_VERSION,
version.default
);
}
self.version = Some(version.clone());
Ok(version)
Ok(Version::new(long, default, l2_revision))
}
}
+19 -31
View File
@@ -1,4 +1,4 @@
//! The Solidity compiler.
//! The Solidity compiler solJson interface.
use std::path::Path;
use std::path::PathBuf;
@@ -18,22 +18,30 @@ extern "C" {
}
/// The Solidity compiler.
pub struct SoljsonCompiler {
/// The lazily-initialized compiler version.
pub version: Option<Version>,
}
pub struct SoljsonCompiler;
impl Compiler for SoljsonCompiler {
/// Compiles the Solidity `--standard-json` input into Yul IR.
fn standard_json(
&mut self,
mut input: StandardJsonInput,
_base_path: Option<String>,
_include_paths: Vec<String>,
_allow_paths: Option<String>,
base_path: Option<String>,
include_paths: Vec<String>,
allow_paths: Option<String>,
) -> anyhow::Result<StandardJsonOutput> {
let version = self.version()?;
input.normalize(&version.default);
if !include_paths.is_empty() {
anyhow::bail!("configuring include paths is not supported with solJson")
}
if base_path.is_some() {
anyhow::bail!("configuring the base path is not supported with solJson")
}
if allow_paths.is_some() {
anyhow::bail!("configuring allow paths is not supported with solJson")
}
let version = self.version()?.validate(&include_paths)?.default;
input.normalize(&version);
let suppressed_warnings = input.suppressed_warnings.take().unwrap_or_default();
let input_json = serde_json::to_string(&input).expect("Always valid");
@@ -74,31 +82,11 @@ impl Compiler for SoljsonCompiler {
.ok_or_else(|| anyhow::anyhow!("Soljson version parsing: metadata dropping"))?
.parse()
.map_err(|error| anyhow::anyhow!("Soljson version parsing: {}", error))?;
let l2_revision: Option<semver::Version> = version
.split('-')
.nth(1)
.and_then(|version| version.parse().ok());
let version = Version::new(long, default, l2_revision);
if version.default < super::FIRST_SUPPORTED_VERSION {
anyhow::bail!(
"`Soljson` versions <{} are not supported, found {}",
super::FIRST_SUPPORTED_VERSION,
version.default
);
}
if version.default > super::LAST_SUPPORTED_VERSION {
anyhow::bail!(
"`Soljson` versions >{} are not supported, found {}",
super::LAST_SUPPORTED_VERSION,
version.default
);
}
self.version = Some(version.clone());
Ok(version)
Ok(Version::new(long, default, l2_revision))
}
}
@@ -33,13 +33,12 @@ impl Error {
/// Returns the `ecrecover` function usage warning.
pub fn message_ecrecover(src: Option<&str>) -> Self {
let message = r#"
┌──────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Warning: It looks like you are using 'ecrecover' to validate a signature of a user account. │
│ Polkadot comes with native account abstraction support, therefore it is highly recommended NOT │
│ to rely on the fact that the account has an ECDSA private key attached to it since accounts might│
│ implement other signature schemes. │
└──────────────────────────────────────────────────────────────────────────────────────────────────┘"#
.to_owned();
Warning: It looks like you are using 'ecrecover' to validate a signature of a user account.
Polkadot comes with native account abstraction support, therefore it is highly recommended NOT
to rely on the fact that the account has an ECDSA private key attached to it since accounts might
implement other signature schemes.
"#
.to_owned();
Self {
component: "general".to_owned(),
@@ -55,17 +54,13 @@ impl Error {
/// Returns the `<address payable>`'s `send` and `transfer` methods usage error.
pub fn message_send_and_transfer(src: Option<&str>) -> Self {
let message = r#"
┌──────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Warning: It looks like you are using '<address payable>.send/transfer(<X>)' without providing │
│ the gas amount. Such calls will fail depending on the pubdata costs. │
│ This might be a false positive if you are using an interface (like IERC20) instead of the │
│ native Solidity `send/transfer`. │
│ Please use 'payable(<address>).call{value: <X>}("")' instead, but be careful with the reentrancy │
│ attack. `send` and `transfer` send limited amount of gas that prevents reentrancy, whereas │
│ `<address>.call{value: <X>}` sends all gas to the callee. Learn more on │
│ https://docs.soliditylang.org/en/latest/security-considerations.html#reentrancy │
└──────────────────────────────────────────────────────────────────────────────────────────────────┘"#
.to_owned();
Warning: It looks like you are using '<address payable>.send/transfer(<X>)'. Such balance
transfer calls will supply all remaining gas and disable call re-entrancy instead of
supplying the 2300 gas stipend. However, the compiler uses a heuristic to detect the expected
2300 gas stipend. You are advised to carefully test this to ensure the desired behavior.
Learn more on https://docs.soliditylang.org/en/latest/security-considerations.html#reentrancy
"#
.to_owned();
Self {
component: "general".to_owned(),
@@ -81,16 +76,15 @@ impl Error {
/// Returns the `extcodesize` instruction usage warning.
pub fn message_extcodesize(src: Option<&str>) -> Self {
let message = r#"
┌──────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Warning: Your code or one of its dependencies uses the 'extcodesize' instruction, which is │
│ usually needed in the following cases: │
1. To detect whether an address belongs to a smart contract. │
│ 2. To detect whether the deploy code execution has finished. │
│ Polkadot comes with native account abstraction support (so smart contracts are just accounts │
│ coverned by code), and you should avoid differentiating between contracts and non-contract |
| addresses. │
└──────────────────────────────────────────────────────────────────────────────────────────────────┘"#
.to_owned();
Warning: Your code or one of its dependencies uses the 'extcodesize' instruction, which is
usually needed in the following cases:
1. To detect whether an address belongs to a smart contract.
2. To detect whether the deploy code execution has finished.
Polkadot comes with native account abstraction support (so smart contracts are just accounts
coverned by code), and you should avoid differentiating between contracts and non-contract
addresses.
"#
.to_owned();
Self {
component: "general".to_owned(),
@@ -106,13 +100,12 @@ impl Error {
/// Returns the `origin` instruction usage warning.
pub fn message_tx_origin(src: Option<&str>) -> Self {
let message = r#"
┌──────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Warning: You are checking for 'tx.origin' in your code, which might lead to unexpected behavior. │
│ Polkadot comes with native account abstraction support, and therefore the initiator of a │
│ transaction might be different from the contract calling your code. It is highly recommended NOT │
│ to rely on tx.origin, but use msg.sender instead. │
└──────────────────────────────────────────────────────────────────────────────────────────────────┘"#
.to_owned();
Warning: You are checking for 'tx.origin' in your code, which might lead to unexpected behavior.
Polkadot comes with native account abstraction support, and therefore the initiator of a
transaction might be different from the contract calling your code. It is highly recommended NOT
to rely on tx.origin, but use msg.sender instead.
"#
.to_owned();
Self {
component: "general".to_owned(),
+22
View File
@@ -36,4 +36,26 @@ impl Version {
l2_revision: None,
}
}
pub fn validate(self, include_paths: &[String]) -> anyhow::Result<Self> {
if self.default < super::FIRST_SUPPORTED_VERSION {
anyhow::bail!(
"`solc` versions <{} are not supported, found {}",
super::FIRST_SUPPORTED_VERSION,
self.default
);
}
if self.default > super::LAST_SUPPORTED_VERSION {
anyhow::bail!(
"`solc` versions >{} are not supported, found {}",
super::LAST_SUPPORTED_VERSION,
self.default
);
}
if !include_paths.is_empty() && self.default < super::FIRST_INCLUDE_PATH_VERSION {
anyhow::bail!("--include-path is not supported in solc {}", self.default);
}
Ok(self)
}
}
+36 -37
View File
@@ -64,32 +64,33 @@ contract SendExample {
require(success, "Failed to send Ether");
}
}
"#;
"#;
pub const BALANCE_CALLS_MESSAGE: &str =
"Warning: It looks like you are using '<address payable>.send/transfer(<X>)'";
#[test]
fn send() {
assert!(
super::check_solidity_warning(
SEND_TEST_SOURCE,
"Warning: It looks like you are using '<address payable>.send/transfer(<X>)' without providing",
BTreeMap::new(),
false,
None,
).expect("Test failure")
);
assert!(super::check_solidity_warning(
SEND_TEST_SOURCE,
BALANCE_CALLS_MESSAGE,
BTreeMap::new(),
false,
None,
)
.expect("Test failure"));
}
#[test]
fn send_suppressed() {
assert!(
!super::check_solidity_warning(
SEND_TEST_SOURCE,
"Warning: It looks like you are using '<address payable>.send/transfer(<X>)' without providing",
BTreeMap::new(),
false,
Some(vec![Warning::SendTransfer]),
).expect("Test failure")
);
assert!(!super::check_solidity_warning(
SEND_TEST_SOURCE,
BALANCE_CALLS_MESSAGE,
BTreeMap::new(),
false,
Some(vec![Warning::SendTransfer]),
)
.expect("Test failure"));
}
pub const TRANSFER_TEST_SOURCE: &str = r#"
@@ -111,28 +112,26 @@ contract TransferExample {
#[test]
fn transfer() {
assert!(
super::check_solidity_warning(
TRANSFER_TEST_SOURCE,
"Warning: It looks like you are using '<address payable>.send/transfer(<X>)' without providing",
BTreeMap::new(),
false,
None,
).expect("Test failure")
);
assert!(super::check_solidity_warning(
TRANSFER_TEST_SOURCE,
BALANCE_CALLS_MESSAGE,
BTreeMap::new(),
false,
None,
)
.expect("Test failure"));
}
#[test]
fn transfer_suppressed() {
assert!(
!super::check_solidity_warning(
TRANSFER_TEST_SOURCE,
"Warning: It looks like you are using '<address payable>.send/transfer(<X>)' without providing",
BTreeMap::new(),
false,
Some(vec![Warning::SendTransfer]),
).expect("Test failure")
);
assert!(!super::check_solidity_warning(
TRANSFER_TEST_SOURCE,
BALANCE_CALLS_MESSAGE,
BTreeMap::new(),
false,
Some(vec![Warning::SendTransfer]),
)
.expect("Test failure"));
}
pub const EXTCODESIZE_TEST_SOURCE: &str = r#"