Merge branch 'master' into no-fuzz-proptest

Author: grandizzy

crates/cast/src/cmd/access_list.rs

@@ -27,7 +27,7 @@ pub struct AccessListArgs {
     sig: Option<String>,
 
     /// The arguments of the function to call.
-    #[arg(value_name = "ARGS")]
+    #[arg(value_name = "ARGS", allow_negative_numbers = true)]
     args: Vec<String>,
 
     /// The block height to query at.

crates/cast/src/cmd/call.rs

@@ -72,6 +72,7 @@ pub struct CallArgs {
     sig: Option<String>,
 
     /// The arguments of the function to call.
+    #[arg(allow_negative_numbers = true)]
     args: Vec<String>,
 
     /// Raw hex-encoded data for the transaction. Used instead of \[SIG\] and \[ARGS\].
@@ -177,6 +178,7 @@ pub enum CallSubcommands {
         sig: Option<String>,
 
         /// The arguments of the constructor.
+        #[arg(allow_negative_numbers = true)]
         args: Vec<String>,
 
         /// Ether to send in the transaction.
@@ -643,4 +645,21 @@ mod tests {
             Some(vec!["0x123:0x1:0x1234".to_string(), "0x456:0x2:0x5678".to_string()])
         );
     }
+
+    #[test]
+    fn test_negative_args_with_flags() {
+        // Test that negative args work with flags
+        let args = CallArgs::parse_from([
+            "foundry-cli",
+            "--trace",
+            "0xDeaDBeeFcAfEbAbEfAcEfEeDcBaDbEeFcAfEbAbE",
+            "process(int256)",
+            "-999999",
+            "--debug",
+        ]);
+
+        assert!(args.trace);
+        assert!(args.debug);
+        assert_eq!(args.args, vec!["-999999"]);
+    }
 }

crates/cast/src/cmd/estimate.rs

@@ -22,6 +22,7 @@ pub struct EstimateArgs {
     sig: Option<String>,
 
     /// The arguments of the function to call.
+    #[arg(allow_negative_numbers = true)]
     args: Vec<String>,
 
     /// The block height to query at.
@@ -58,6 +59,7 @@ pub enum EstimateSubcommands {
         sig: Option<String>,
 
         /// Constructor arguments
+        #[arg(allow_negative_numbers = true)]
         args: Vec<String>,
 
         /// Ether to send in the transaction

crates/cast/src/cmd/mktx.rs

@@ -25,6 +25,7 @@ pub struct MakeTxArgs {
     sig: Option<String>,
 
     /// The arguments of the function to call.
+    #[arg(allow_negative_numbers = true)]
     args: Vec<String>,
 
     #[command(subcommand)]
@@ -69,6 +70,7 @@ pub enum MakeTxSubcommands {
         sig: Option<String>,
 
         /// The constructor arguments.
+        #[arg(allow_negative_numbers = true)]
         args: Vec<String>,
     },
 }

crates/cast/src/cmd/send.rs

@@ -30,6 +30,7 @@ pub struct SendTxArgs {
     sig: Option<String>,
 
     /// The arguments of the function to call.
+    #[arg(allow_negative_numbers = true)]
     args: Vec<String>,
 
     /// Only print the transaction hash and exit immediately.
@@ -80,6 +81,7 @@ pub enum SendTxSubcommands {
         sig: Option<String>,
 
         /// The arguments of the function to call.
+        #[arg(allow_negative_numbers = true)]
         args: Vec<String>,
     },
 }

crates/cast/tests/cli/main.rs

@@ -3741,3 +3741,119 @@ Transaction successfully executed.
 
 "#]]);
 });
+
+// Tests for negative number argument parsing
+// Ensures that negative numbers in function arguments are properly parsed
+// instead of being treated as command flags
+
+// Test that cast call accepts negative numbers as function arguments
+casttest!(cast_call_negative_numbers, |_prj, cmd| {
+    let rpc = next_rpc_endpoint(NamedChain::Sepolia);
+    // Test with negative int parameter - should not treat -456789 as a flag
+    cmd.args([
+        "call",
+        "0xAbCdEf1234567890aBcDeF1234567890aBcDeF12",
+        "processValue(int128)",
+        "-456789",
+        "--rpc-url",
+        rpc.as_str(),
+    ])
+    .assert_success();
+});
+
+// Test negative numbers with multiple parameters
+casttest!(cast_call_multiple_negative_numbers, |_prj, cmd| {
+    let rpc = next_rpc_endpoint(NamedChain::Sepolia);
+    cmd.args([
+        "call",
+        "--rpc-url",
+        rpc.as_str(),
+        "0xDeaDBeeFcAfEbAbEfAcEfEeDcBaDbEeFcAfEbAbE",
+        "calculateDelta(int64,int32,uint16)",
+        "-987654321",
+        "-42",
+        "65535",
+    ])
+    .assert_success();
+});
+
+// Test negative numbers mixed with flags
+casttest!(cast_call_negative_with_flags, |_prj, cmd| {
+    let rpc = next_rpc_endpoint(NamedChain::Sepolia);
+    cmd.args([
+        "call",
+        "--trace", // flag before
+        "0x9876543210FeDcBa9876543210FeDcBa98765432",
+        "updateBalance(int256)",
+        "-777888",
+        "--rpc-url",
+        rpc.as_str(), // flag after
+    ])
+    .assert_success();
+});
+
+// Test that actual invalid flags are still caught
+casttest!(cast_call_invalid_flag_still_caught, |_prj, cmd| {
+    cmd.args([
+        "call",
+        "--invalid-flag", // This should be caught as invalid
+        "0x5555555555555555555555555555555555555555",
+    ])
+    .assert_failure()
+    .stderr_eq(str![[r#"
+error: unexpected argument '--invalid-flag' found
+
+  tip: to pass '--invalid-flag' as a value, use '-- --invalid-flag'
+
+Usage: cast[..] call [OPTIONS] [TO] [SIG] [ARGS]... [COMMAND]
+
+For more information, try '--help'.
+
+"#]]);
+});
+
+// Test cast estimate with negative numbers
+casttest!(cast_estimate_negative_numbers, |_prj, cmd| {
+    let rpc = next_rpc_endpoint(NamedChain::Sepolia);
+    cmd.args([
+        "estimate",
+        "0xBbBbBbBbBbBbBbBbBbBbBbBbBbBbBbBbBbBbBbBb",
+        "rebalance(int64)",
+        "-8888",
+        "--rpc-url",
+        rpc.as_str(),
+    ])
+    .assert_success();
+});
+
+// Test cast mktx with negative numbers
+casttest!(cast_mktx_negative_numbers, |_prj, cmd| {
+    let rpc = next_rpc_endpoint(NamedChain::Sepolia);
+    cmd.args([
+        "mktx",
+        "0x1111111111111111111111111111111111111111",
+        "settleDebt(int256)",
+        "-15000",
+        "--private-key",
+        "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", // anvil wallet #0
+        "--rpc-url",
+        rpc.as_str(),
+        "--gas-limit",
+        "100000",
+    ])
+    .assert_success();
+});
+
+// Test cast access-list with negative numbers
+casttest!(cast_access_list_negative_numbers, |_prj, cmd| {
+    let rpc = next_rpc_endpoint(NamedChain::Sepolia);
+    cmd.args([
+        "access-list",
+        "0x9999999999999999999999999999999999999999",
+        "adjustPosition(int128)",
+        "-33333",
+        "--rpc-url",
+        rpc.as_str(),
+    ])
+    .assert_success();
+});

crates/forge/src/cmd/build.rs

@@ -1,6 +1,6 @@
 use super::{install, watch::WatchArgs};
 use clap::Parser;
-use eyre::Result;
+use eyre::{Result, eyre};
 use forge_lint::{linter::Linter, sol::SolidityLinter};
 use foundry_cli::{
     opts::{BuildOpts, solar_pcx_from_build_opts},
@@ -112,17 +112,21 @@ impl BuildArgs {
             sh_println!("{}", serde_json::to_string_pretty(&output.output())?)?;
         }
 
+        if !config.lint.lint_on_build {
+            return Ok(output);
+        }
+
         // Only run the `SolidityLinter` if there are no compilation errors
-        if output.output().errors.iter().all(|e| !e.is_error()) {
-            self.lint(&project, &config)?;
+        if !output.output().errors.iter().any(|e| e.is_error()) {
+            self.lint(&project, &config).map_err(|err| eyre!("Lint failed: {err}"))?;
         }
 
         Ok(output)
     }
 
     fn lint(&self, project: &Project, config: &Config) -> Result<()> {
         let format_json = shell::is_json();
-        if project.compiler.solc.is_some() && config.lint.lint_on_build && !shell::is_quiet() {
+        if project.compiler.solc.is_some() && !shell::is_quiet() {
             let linter = SolidityLinter::new(config.project_paths())
                 .with_json_emitter(format_json)
                 .with_description(!format_json)

crates/forge/tests/cli/test_cmd.rs

@@ -3941,6 +3941,7 @@ Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests)
 
 // tests proper reverts in fork mode for contracts with non-existent linked libraries.
 // <https://github.com/foundry-rs/foundry/issues/11185>
+#[cfg(not(feature = "isolate-by-default"))]
 forgetest_init!(can_fork_test_with_non_existent_linked_library, |prj, cmd| {
     prj.update_config(|config| {
         config.libraries =

crates/lint/src/sol/codesize/unwrapped_modifier_logic.rs

@@ -62,25 +62,32 @@ impl UnwrappedModifierLogic {
 
     /// Checks if a block of statements is complex and should be wrapped in a helper function.
     ///
-    /// This is true if the block contains:
+    /// This always is 'false' the modifier contains assembly. We assume that if devs know how to
+    /// use assembly, they will also know how to reduce the codesize of their contracts and they
+    /// have a good reason to use it on their modifiers.
+    ///
+    /// This is 'true' if the block contains:
     /// 1. Any statement that is not a placeholder or a valid expression.
     /// 2. More than one simple call expression.
     fn stmts_require_wrapping(&self, hir: &hir::Hir<'_>, stmts: &[hir::Stmt<'_>]) -> bool {
-        let mut has_valid_stmt = false;
+        let (mut res, mut has_valid_stmt) = (false, false);
         for stmt in stmts {
             match &stmt.kind {
                 hir::StmtKind::Placeholder => continue,
                 hir::StmtKind::Expr(expr) => {
                     if !self.is_valid_expr(hir, expr) || has_valid_stmt {
-                        return true;
+                        res = true;
                     }
                     has_valid_stmt = true;
                 }
-                _ => return true,
+                // HIR doesn't support assembly yet:
+                // <https://github.com/paradigmxyz/solar/blob/d25bf38a5accd11409318e023f701313d98b9e1e/crates/sema/src/hir/mod.rs#L977-L982>
+                hir::StmtKind::Err(_) => return false,
+                _ => res = true,
             }
         }
 
-        false
+        res
     }
 
     fn get_snippet<'a>(

crates/lint/testdata/UnwrappedModifierLogic.sol

@@ -25,6 +25,28 @@ contract UnwrappedModifierLogicTest {
     mapping(address => bool) isOwner;
     mapping(address => mapping(bytes32 => bool)) hasRole;
 
+    /// -----------------------------------------------------------------------
+    /// Exceptions (assembly block)
+    /// -----------------------------------------------------------------------
+
+    modifier freeTempMemory() {
+        uint256 m;
+        assembly ("memory-safe") {
+            m := mload(0x40)
+        }
+        _;
+        assembly ("memory-safe") {
+            mstore(0x40, m)
+        }
+    }
+
+    modifier assemblyBlock(address sender) {
+        assembly {
+            let x := sender
+        }
+        _;
+    }
+
     /// -----------------------------------------------------------------------
     /// Good patterns (only 1 valid statement before or after placeholder)
     /// -----------------------------------------------------------------------
@@ -64,7 +86,7 @@ contract UnwrappedModifierLogicTest {
     /// -----------------------------------------------------------------------
 
     // Bad because there are multiple valid function calls before the placeholder
-    modifier multipleBeforePlaceholder() { //~NOTE: wrap modifier logic to reduce code size  
+    modifier multipleBeforePlaceholder() { //~NOTE: wrap modifier logic to reduce code size
         checkPublic(msg.sender); // These should become _multipleBeforePlaceholder()
         checkPrivate(msg.sender);
         checkInternal(msg.sender);
@@ -127,14 +149,6 @@ contract UnwrappedModifierLogicTest {
         _;
     }
 
-    // Only call expressions are allowed (public/private/internal functions).
-    modifier assemblyBlock(address sender) { //~NOTE: wrap modifier logic to reduce code size
-        assembly {
-            let x := sender
-        }
-        _;
-    }
-
     // Only call expressions are allowed (public/private/internal functions).
     modifier uncheckedBlock(address sender) { //~NOTE: wrap modifier logic to reduce code size
         unchecked {
@@ -158,4 +172,4 @@ contract UnwrappedModifierLogicTest {
         c.onlyOwner(sender);
         _;
     }
-}
+}