feat: add sol-style-malformed-require locally (#12340)

Adds sol-style-malformed-require to local semgrep and actually
fixes the rule so that it works. Fixes a couple of findings that
the rule found.

.semgrep/sol-rules.yaml

@@ -2,14 +2,14 @@ rules:
   - id: sol-safety-deployutils-args
     languages: [solidity]
     severity: ERROR
-    pattern-regex: DeployUtils\.(create1|create2|create1AndSave|create2AndSave)\s*\(\s*\{[^}]*?_args\s*:\s*(?!\s*DeployUtils\.encodeConstructor\()\s*[^}]*?\}\s*\)
     message: _args parameter should be wrapped with DeployUtils.encodeConstructor
+    pattern-regex: DeployUtils\.(create1|create2|create1AndSave|create2AndSave)\s*\(\s*\{[^}]*?_args\s*:\s*(?!\s*DeployUtils\.encodeConstructor\()\s*[^}]*?\}\s*\)
 
   - id: sol-style-input-arg-fmt
     languages: [solidity]
     severity: ERROR
-    pattern-regex: function\s+\w+\s*\(\s*([^)]*?\b\w+\s+(?!_)(?!memory\b)(?!calldata\b)(?!storage\b)(?!payable\b)\w+\s*(?=,|\)))
     message: Named inputs to functions must be prepended with an underscore
+    pattern-regex: function\s+\w+\s*\(\s*([^)]*?\b\w+\s+(?!_)(?!memory\b)(?!calldata\b)(?!storage\b)(?!payable\b)\w+\s*(?=,|\)))
     paths:
       exclude:
         - op-chain-ops/script/testdata/scripts/ScriptExample.s.sol
@@ -25,8 +25,8 @@ rules:
   - id: sol-style-return-arg-fmt
     languages: [solidity]
     severity: ERROR
-    pattern-regex: returns\s*(\w+\s*)?\(\s*([^)]*?\b\w+\s+(?!memory\b)(?!calldata\b)(?!storage\b)(?!payable\b)\w+(?<!_)\s*(?=,|\)))
     message: Named return arguments to functions must be appended with an underscore
+    pattern-regex: returns\s*(\w+\s*)?\(\s*([^)]*?\b\w+\s+(?!memory\b)(?!calldata\b)(?!storage\b)(?!payable\b)\w+(?<!_)\s*(?=,|\)))
     paths:
       exclude:
         - op-chain-ops/script/testdata/scripts/ScriptExample.s.sol
@@ -47,3 +47,24 @@ rules:
     message: vm.expectRevert() must specify the revert reason
     patterns:
       - pattern: vm.expectRevert()
+
+  - id: sol-style-malformed-require
+    languages: [solidity]
+    severity: ERROR
+    message: Require statement style is malformed
+    patterns:
+      - pattern: require(..., $ERR);
+      - pattern-not: require($ERR);
+      - focus-metavariable: $ERR
+      - pattern-not-regex: \"(\w+:\s[^"]+)\"
+      - pattern-not-regex: string\.concat\(\"(\w+:\s[^"]+)\"\,[^"]+\)
+      - pattern-not-regex: \"([a-zA-Z0-9\s]+-[a-zA-Z0-9\s]+)\"
+      - pattern-not-regex: \"([a-zA-Z0-9\s]+-[a-zA-Z0-9\s]+-[a-zA-Z0-9\s]+)\"
+    paths:
+      exclude:
+        - packages/contracts-bedrock/src/libraries/Bytes.sol
+        - packages/contracts-bedrock/src/legacy/LegacyMintableERC20.sol
+        - packages/contracts-bedrock/src/cannon/MIPS.sol
+        - packages/contracts-bedrock/src/cannon/MIPS2.sol
+        - packages/contracts-bedrock/src/cannon/libraries/MIPSMemory.sol
+        - packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol

packages/contracts-bedrock/scripts/DeploySuperchain.s.sol

@@ -192,8 +192,10 @@ contract DeploySuperchainOutput is BaseDeployIO {
         address actualProtocolVersionsImpl = IProxy(payable(address(_protocolVersionsProxy))).implementation();
         vm.stopPrank();
 
-        require(actualSuperchainConfigImpl == address(_superchainConfigImpl), "100");
-        require(actualProtocolVersionsImpl == address(_protocolVersionsImpl), "200");
+        require(actualSuperchainConfigImpl == address(_superchainConfigImpl), "100"); // nosemgrep:
+            // sol-style-malformed-require
+        require(actualProtocolVersionsImpl == address(_protocolVersionsImpl), "200"); // nosemgrep:
+            // sol-style-malformed-require
 
         assertValidDeploy(_dsi);
     }

packages/contracts-bedrock/scripts/autogen/SemverLock.s.sol

@@ -48,7 +48,7 @@ contract SemverLock is Script {
             string memory artifactFiles = string(Process.run(commands));
 
             string[] memory files = stdJson.readStringArray(artifactFiles, "");
-            require(files.length > 0, string.concat("No artifacts found for ", contractName));
+            require(files.length > 0, string.concat("SemverLock: no artifacts found for ", contractName));
             string memory fileName = files[0];
 
             // Parse the artifact to get the contract's initcode hash.

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol

@@ -584,6 +584,9 @@ library ChainAssertions {
     function assertInitializedSlotIsSet(address _contractAddress, uint256 _slot, uint256 _offset) internal view {
         bytes32 slotVal = vm.load(_contractAddress, bytes32(_slot));
         uint8 val = uint8((uint256(slotVal) >> (_offset * 8)) & 0xFF);
-        require(val == uint8(1) || val == uint8(0xff), "Storage value is not 1 or 0xff at the given slot and offset");
+        require(
+            val == uint8(1) || val == uint8(0xff),
+            "ChainAssertions: storage value is not 1 or 0xff at the given slot and offset"
+        );
     }
 }

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

@@ -236,8 +236,8 @@ contract Deploy is Deployer {
     )
         public
     {
-        require(_superchainConfigProxy != address(0), "must specify address for superchain config proxy");
-        require(_protocolVersionsProxy != address(0), "must specify address for protocol versions proxy");
+        require(_superchainConfigProxy != address(0), "Deploy: must specify address for superchain config proxy");
+        require(_protocolVersionsProxy != address(0), "Deploy: must specify address for protocol versions proxy");
 
         vm.chainId(cfg.l1ChainID());
 

packages/contracts-bedrock/scripts/deploy/DeployConfig.s.sol

@@ -97,7 +97,7 @@ contract DeployConfig is Script {
         try vm.readFile(_path) returns (string memory data_) {
             _json = data_;
         } catch {
-            require(false, string.concat("Cannot find deploy config file at ", _path));
+            require(false, string.concat("DeployConfig: cannot find deploy config file at ", _path));
         }
 
         finalSystemOwner = stdJson.readAddress(_json, "$.finalSystemOwner");

packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol

@@ -374,7 +374,7 @@ contract DeployOwnership is Deploy {
         address[] memory owners = safe.getOwners();
         require(
             safe.getThreshold() == LivenessModule(livenessModule).getRequiredThreshold(owners.length),
-            "Safe threshold must be equal to the LivenessModule's required threshold"
+            "DeployOwnership: safe threshold must be equal to the LivenessModule's required threshold"
         );
 
         addr_ = address(safe);

packages/contracts-bedrock/scripts/libraries/DeployUtils.sol

@@ -204,7 +204,7 @@ library DeployUtils {
     /// @param _data constructor arguments prefixed with a psuedo-constructor function signature
     /// @return encodedData_ constructor arguments without the psuedo-constructor function signature prefix
     function encodeConstructor(bytes memory _data) internal pure returns (bytes memory encodedData_) {
-        require(_data.length >= 4, "encodeConstructor takes in _data of length >= 4");
+        require(_data.length >= 4, "DeployUtils: encodeConstructor takes in _data of length >= 4");
         encodedData_ = Bytes.slice(_data, 4);
     }
 
@@ -238,9 +238,12 @@ library DeployUtils {
         // All addresses should be unique.
         for (uint256 i = 0; i < _addrs.length; i++) {
             for (uint256 j = i + 1; j < _addrs.length; j++) {
-                string memory err =
-                    string.concat("check failed: duplicates at ", LibString.toString(i), ",", LibString.toString(j));
-                require(_addrs[i] != _addrs[j], err);
+                require(
+                    _addrs[i] != _addrs[j],
+                    string.concat(
+                        "DeployUtils: check failed, duplicates at ", LibString.toString(i), ",", LibString.toString(j)
+                    )
+                );
             }
         }
     }
@@ -253,7 +256,7 @@ library DeployUtils {
         uint8 value = uint8((uint256(slotVal) >> (_offset * 8)) & 0xFF);
         require(
             value == 1 || value == type(uint8).max,
-            "Value at the given slot and offset does not indicate initialization"
+            "DeployUtils: value at the given slot and offset does not indicate initialization"
         );
     }
 }

packages/contracts-bedrock/scripts/periphery/deploy/DeployPeriphery.s.sol

@@ -277,7 +277,7 @@ contract DeployPeriphery is Script, Artifacts {
             assembly {
                 addr_ := create2(0, add(initCode, 0x20), mload(initCode), salt)
             }
-            require(addr_ != address(0), "deployment failed");
+            require(addr_ != address(0), "DeployPeriphery: deployment failed");
             save(_name, addr_);
             console.log("%s deployed at %s", _name, addr_);
         }

packages/contracts-bedrock/semver-lock.json

@@ -140,8 +140,8 @@
     "sourceCodeHash": "0x16614cc0e6abf7e81e1e5dc2c0773ee7101cb38af40e0907a8800ca7eddd3b5a"
   },
   "src/cannon/PreimageOracle.sol": {
-    "initCodeHash": "0xa0b19e18561da9990c95ebea9750dd901f73147b32b8b234eca0f35073c5a970",
-    "sourceCodeHash": "0x6235d602f84c4173e7a58666791e3db4c9e9651eaccb20db5aed2f898b76e896"
+    "initCodeHash": "0x64ea814bf9769257c91da57928675d3f8462374b0c23bdf860ccfc79f41f7801",
+    "sourceCodeHash": "0xac290a77986d54efe404c73ee58d11429e1b1fb47e4d06d4c38b6ecc20751d78"
   },
   "src/dispute/AnchorStateRegistry.sol": {
     "initCodeHash": "0x13d00eef8c3f769863fc766180acc8586f5da309ca0a098e67d4d90bd3243341",

packages/contracts-bedrock/src/cannon/PreimageOracle.sol

@@ -33,8 +33,8 @@ contract PreimageOracle is ISemver {
     uint256 public constant PRECOMPILE_CALL_RESERVED_GAS = 100_000;
 
     /// @notice The semantic version of the Preimage Oracle contract.
-    /// @custom:semver 1.1.3-beta.4
-    string public constant version = "1.1.3-beta.4";
+    /// @custom:semver 1.1.3-beta.5
+    string public constant version = "1.1.3-beta.5";
 
     ////////////////////////////////////////////////////////////////
     //                 Authorized Preimage Parts                  //
@@ -97,7 +97,7 @@ contract PreimageOracle is ISemver {
         // Make sure challenge period fits within uint64 so that it can safely be used within the
         // FaultDisputeGame contract to compute clock extensions. Adding this check is simpler than
         // changing the existing contract ABI.
-        require(_challengePeriod <= type(uint64).max, "challenge period too large");
+        require(_challengePeriod <= type(uint64).max, "PreimageOracle: challenge period too large");
 
         // Compute hashes in empty sparse Merkle tree. The first hash is not set, and kept as zero as the identity.
         for (uint256 height = 0; height < KECCAK_TREE_DEPTH - 1; height++) {

packages/contracts-bedrock/test/cannon/PreimageOracle.t.sol

@@ -24,7 +24,7 @@ contract PreimageOracle_Test is Test {
     /// @param _challengePeriod The challenge period to test.
     function testFuzz_constructor_challengePeriodTooLarge_reverts(uint256 _challengePeriod) public {
         _challengePeriod = bound(_challengePeriod, uint256(type(uint64).max) + 1, type(uint256).max);
-        vm.expectRevert("challenge period too large");
+        vm.expectRevert("PreimageOracle: challenge period too large");
         new PreimageOracle(0, _challengePeriod);
     }
 
@@ -860,6 +860,7 @@ contract PreimageOracle_LargePreimageProposals_Test is Test {
 
     /// @notice Tests that squeezing a large preimage proposal after the challenge period has passed always succeeds and
     ///         persists the correct data.
+    /// forge-config: ciheavy.fuzz.runs = 512
     function testFuzz_squeezeLPP_succeeds(uint256 _numBlocks, uint32 _partOffset) public {
         _numBlocks = bound(_numBlocks, 1, 2 ** 8);
         _partOffset = uint32(bound(_partOffset, 0, _numBlocks * LibKeccak.BLOCK_SIZE_BYTES + 8 - 1));
@@ -1057,6 +1058,7 @@ contract PreimageOracle_LargePreimageProposals_Test is Test {
 
     /// @notice Tests that challenging the first divergence in a large preimage proposal at an arbitrary location
     ///         in the leaf values always succeeds.
+    /// forge-config: ciheavy.fuzz.runs = 512
     function testFuzz_challenge_arbitraryLocation_succeeds(uint256 _lastCorrectLeafIdx, uint256 _numBlocks) public {
         _numBlocks = bound(_numBlocks, 1, 2 ** 8);
         _lastCorrectLeafIdx = bound(_lastCorrectLeafIdx, 0, _numBlocks - 1);
@@ -1109,6 +1111,7 @@ contract PreimageOracle_LargePreimageProposals_Test is Test {
     }
 
     /// @notice Tests that challenging the a divergence in a large preimage proposal at the first leaf always succeeds.
+    /// forge-config: ciheavy.fuzz.runs = 1024
     function testFuzz_challengeFirst_succeeds(uint256 _numBlocks) public {
         _numBlocks = bound(_numBlocks, 1, 2 ** 8);