chore(contract): applied new solhint rules across l1, l2, and system …

…contracts (#473)

.solhint.json

@@ -1,31 +1,40 @@
 {
   "extends": "solhint:recommended",
   "rules": {
-    "state-visibility": "off",
-    "func-visibility": ["warn", { "ignoreConstructors": true }],
-    "var-name-mixedcase": "off",
-    "avoid-call-value": "off",
-    "no-empty-blocks": "off",
-    "not-rely-on-time": "off",
+    "avoid-call-value": "error",
     "avoid-low-level-calls": "off",
-    "no-inline-assembly": "off",
+    "avoid-sha3": "error",
+    "check-send-result": "error",
+    "compiler-version": ["error", "^0.8.0"],
     "const-name-snakecase": "off",
-    "no-complex-fallback": "off",
-    "reason-string": "off",
+    "contract-name-camelcase": "off",
+    "gas-calldata-parameters": "error",
+    "gas-custom-errors": "error",
+    "gas-increment-by-one": "error",
+    "gas-length-in-loops": "error",
+    "gas-struct-packing": "error",
+    "explicit-types": "error",
     "func-name-mixedcase": "off",
-    "custom-errors": "off",
-    "no-unused-vars": "error",
+    "func-named-parameters": ["error", 4],
+    "func-visibility": ["error", { "ignoreConstructors": true }],
+    "imports-on-top": "error",
     "max-states-count": "off",
+    "modifier-name-mixedcase": "error",
+    "named-parameters-mapping": "off",
+    "no-complex-fallback": "off",
+    "no-console": "error",
+    "no-empty-blocks": "off",
     "no-global-import": "error",
+    "no-inline-assembly": "off",
     "no-unused-import": "error",
-    "explicit-types": "error",
-    "modifier-name-mixedcase": "error",
-    "imports-on-top": "error",
+    "no-unused-vars": "error",
+    "not-rely-on-time": "off",
     "quotes": "error",
-    "use-forbidden-name": "error",
-    "visibility-modifier-order": "error",
+    "reason-string": "error",
     "reentrancy": "error",
-    "func-named-parameters": ["error", 4],
-    "compiler-version": ["error", "^0.8.0"]
+    "state-visibility": "error",
+    "use-forbidden-name": "error",
+    "var-name-mixedcase": "off",
+    "visibility-modifier-order": "error"
   }
 }

.solhintignore

@@ -6,6 +6,8 @@ l1-contracts/cache
 l1-contracts/cache-forge
 l1-contracts/lib
 l1-contracts/node_modules
+l1-contracts/contracts/dev-contracts
+l1-contracts/test
 
 # l1-contracts-foundry
 l1-contracts-foundry/cache
@@ -18,3 +20,5 @@ l2-contracts/node_modules
 # system-contracts
 system-contracts/contracts/openzeppelin
 system-contracts/contracts/Constants.sol
+system-contracts/contracts/test-contracts
+system-contracts/contracts-preprocessed

l1-contracts-foundry/script/DeployErc20.s.sol

@@ -7,6 +7,7 @@ import {Script, console2 as console} from "forge-std/Script.sol";
 import {stdToml} from "forge-std/StdToml.sol";
 
 import {Utils} from "./Utils.sol";
+import {MintFailed} from "./ZkSyncScriptErrors.sol";
 
 contract DeployErc20Script is Script {
     using stdToml for string;
@@ -27,7 +28,7 @@ contract DeployErc20Script is Script {
         uint256 mint;
     }
 
-    Config config;
+    Config internal config;
 
     function run() public {
         console.log("Deploying ERC20 Tokens");
@@ -52,7 +53,8 @@ contract DeployErc20Script is Script {
         config.create2FactorySalt = vm.parseTomlBytes32(toml, "$.create2_factory_salt");
         string[] memory tokens = vm.parseTomlKeys(toml, "$.tokens");
 
-        for (uint256 i = 0; i < tokens.length; i++) {
+        uint256 tokensLength = tokens.length;
+        for (uint256 i = 0; i < tokensLength; ++i) {
             TokenDescription memory token;
             string memory key = string.concat("$.tokens.", tokens[i]);
             token.name = toml.readString(string.concat(key, ".name"));
@@ -66,7 +68,8 @@ contract DeployErc20Script is Script {
     }
 
     function deployTokens() internal {
-        for (uint256 i = 0; i < config.tokens.length; i++) {
+        uint256 tokensLength = config.tokens.length;
+        for (uint256 i = 0; i < tokensLength; ++i) {
             TokenDescription memory token = config.tokens[i];
             console.log("Deploying token:", token.name);
             address tokenAddress = deployErc20({
@@ -103,7 +106,9 @@ contract DeployErc20Script is Script {
             (bool success, ) = tokenAddress.call(
                 abi.encodeWithSignature("mint(address,uint256)", config.deployerAddress, mint)
             );
-            require(success, "Mint failed");
+            if (!success) {
+                revert MintFailed();
+            }
         }
 
         return tokenAddress;
@@ -114,7 +119,8 @@ contract DeployErc20Script is Script {
         vm.serializeBytes32("root", "create2_factory_salt", config.create2FactorySalt);
 
         string memory tokens = "";
-        for (uint256 i = 0; i < config.tokens.length; i++) {
+        uint256 tokensLength = config.tokens.length;
+        for (uint256 i = 0; i < tokensLength; ++i) {
             TokenDescription memory token = config.tokens[i];
             vm.serializeString(token.symbol, "name", token.name);
             vm.serializeString(token.symbol, "symbol", token.symbol);

l1-contracts-foundry/script/DeployL1.s.sol

@@ -31,13 +31,15 @@ import {FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-de
 import {L1SharedBridge} from "contracts/bridge/L1SharedBridge.sol";
 import {L1ERC20Bridge} from "contracts/bridge/L1ERC20Bridge.sol";
 import {DiamondProxy} from "contracts/state-transition/chain-deps/DiamondProxy.sol";
+import {AddressHasNoCode} from "./ZkSyncScriptErrors.sol";
 
 contract DeployL1Script is Script {
     using stdToml for string;
 
-    address constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
-    address constant DETERMINISTIC_CREATE2_ADDRESS = 0x4e59b44847b379578588920cA78FbF26c0B4956C;
+    address internal constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
+    address internal constant DETERMINISTIC_CREATE2_ADDRESS = 0x4e59b44847b379578588920cA78FbF26c0B4956C;
 
+    // solhint-disable-next-line gas-struct-packing
     struct DeployedAddresses {
         BridgehubDeployedAddresses bridgehub;
         StateTransitionDeployedAddresses stateTransition;
@@ -49,11 +51,13 @@ contract DeployL1Script is Script {
         address create2Factory;
     }
 
+    // solhint-disable-next-line gas-struct-packing
     struct BridgehubDeployedAddresses {
         address bridgehubImplementation;
         address bridgehubProxy;
     }
 
+    // solhint-disable-next-line gas-struct-packing
     struct StateTransitionDeployedAddresses {
         address stateTransitionProxy;
         address stateTransitionImplementation;
@@ -68,13 +72,15 @@ contract DeployL1Script is Script {
         address diamondProxy;
     }
 
+    // solhint-disable-next-line gas-struct-packing
     struct BridgesDeployedAddresses {
         address erc20BridgeImplementation;
         address erc20BridgeProxy;
         address sharedBridgeImplementation;
         address sharedBridgeProxy;
     }
 
+    // solhint-disable-next-line gas-struct-packing
     struct Config {
         uint256 l1ChainId;
         uint256 eraChainId;
@@ -84,6 +90,7 @@ contract DeployL1Script is Script {
         TokensConfig tokens;
     }
 
+    // solhint-disable-next-line gas-struct-packing
     struct ContractsConfig {
         bytes32 create2FactorySalt;
         address create2FactoryAddr;
@@ -112,8 +119,8 @@ contract DeployL1Script is Script {
         address tokenWethAddress;
     }
 
-    Config config;
-    DeployedAddresses addresses;
+    Config internal config;
+    DeployedAddresses internal addresses;
 
     function run() public {
         console.log("Deploying L1 contracts");
@@ -208,7 +215,7 @@ contract DeployL1Script is Script {
 
         if (isConfigured) {
             if (config.contracts.create2FactoryAddr.code.length == 0) {
-                revert("Create2Factory configured address is empty");
+                revert AddressHasNoCode(config.contracts.create2FactoryAddr);
             }
             contractAddress = config.contracts.create2FactoryAddr;
             console.log("Using configured Create2Factory address:", contractAddress);

l1-contracts-foundry/script/InitializeL2WethToken.s.sol

@@ -14,6 +14,7 @@ import {Bridgehub} from "contracts/bridgehub/Bridgehub.sol";
 contract InitializeL2WethTokenScript is Script {
     using stdToml for string;
 
+    // solhint-disable-next-line gas-struct-packing
     struct Config {
         address deployerAddress;
         address create2FactoryAddr;
@@ -31,7 +32,7 @@ contract InitializeL2WethTokenScript is Script {
         uint256 gasMultiplier;
     }
 
-    Config config;
+    Config internal config;
 
     function run() public {
         initializeConfig();

l1-contracts-foundry/script/RegisterHyperchain.s.sol

@@ -15,12 +15,13 @@ import {VerifierParams, IVerifier} from "contracts/state-transition/chain-interf
 import {FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol";
 import {InitializeDataNewChain as DiamondInitializeDataNewChain} from "contracts/state-transition/chain-interfaces/IDiamondInit.sol";
 import {ValidatorTimelock} from "contracts/state-transition/ValidatorTimelock.sol";
+import {ZksyncContract, MissingAddress, AddressHasNoCode} from "./ZkSyncScriptErrors.sol";
 
 contract RegisterHyperchainScript is Script {
     using stdToml for string;
 
-    address constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
-    bytes32 constant STATE_TRANSITION_NEW_CHAIN_HASH = keccak256("NewHyperchain(uint256,address)");
+    address internal constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
+    bytes32 internal constant STATE_TRANSITION_NEW_CHAIN_HASH = keccak256("NewHyperchain(uint256,address)");
 
     struct Config {
         ContractsConfig contracts;
@@ -49,6 +50,7 @@ contract RegisterHyperchainScript is Script {
         uint128 baseTokenGasPriceMultiplierDenominator;
     }
 
+    // solhint-disable-next-line gas-struct-packing
     struct AddressesConfig {
         address baseToken;
         address bridgehub;
@@ -64,7 +66,7 @@ contract RegisterHyperchainScript is Script {
         address newDiamondProxy;
     }
 
-    Config config;
+    Config internal config;
 
     function run() public {
         console.log("Deploying Hyperchain");
@@ -146,7 +148,7 @@ contract RegisterHyperchainScript is Script {
 
     function checkTokenAddress() internal {
         if (config.addresses.baseToken == address(0)) {
-            revert("Token address is not set");
+            revert MissingAddress(ZksyncContract.BaseToken);
         }
 
         // Check if it's ethereum address
@@ -155,7 +157,7 @@ contract RegisterHyperchainScript is Script {
         }
 
         if (config.addresses.baseToken.code.length == 0) {
-            revert("Token address is not a contract address");
+            revert AddressHasNoCode(config.addresses.baseToken);
         }
 
         console.log("Using base token address:", config.addresses.baseToken);
@@ -248,14 +250,15 @@ contract RegisterHyperchainScript is Script {
         // Get new diamond proxy address from emitted events
         Vm.Log[] memory logs = vm.getRecordedLogs();
         address diamondProxyAddress;
-        for (uint256 i = 0; i < logs.length; i++) {
+        uint256 logsLength = logs.length;
+        for (uint256 i = 0; i < logsLength; ++i) {
             if (logs[i].topics[0] == STATE_TRANSITION_NEW_CHAIN_HASH) {
                 diamondProxyAddress = address(uint160(uint256(logs[i].topics[2])));
                 break;
             }
         }
         if (diamondProxyAddress == address(0)) {
-            revert("Diamond proxy address not found");
+            revert MissingAddress(ZksyncContract.DiamondProxy);
         }
         config.addresses.newDiamondProxy = diamondProxyAddress;
     }

l1-contracts-foundry/script/Utils.sol

@@ -2,6 +2,7 @@
 pragma solidity 0.8.24;
 
 import {Vm} from "forge-std/Vm.sol";
+import {ZksyncContract, FailedToDeploy, BytecodeNotSet, FailedToDeployViaCreate2} from "./ZkSyncScriptErrors.sol";
 
 library Utils {
     // Cheatcodes address, 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D.
@@ -30,12 +31,13 @@ library Utils {
 
         // Extract selectors from the result
         string[] memory parts = vm.split(stringResult, "\n");
-        bytes4[] memory selectors = new bytes4[](parts.length);
-        for (uint256 i = 0; i < parts.length; i++) {
+        uint256 partsLength = parts.length;
+        bytes4[] memory selectors = new bytes4[](partsLength);
+        for (uint256 i = 0; i < partsLength; ++i) {
             bytes memory part = bytes(parts[i]);
             bytes memory extractedSelector = new bytes(10);
             // Selector length 10 is 0x + 4 bytes
-            for (uint256 j = 0; j < 10; j++) {
+            for (uint256 j = 0; j < 10; ++j) {
                 extractedSelector[j] = part[j];
             }
             bytes4 selector = bytes4(vm.parseBytes(string(extractedSelector)));
@@ -44,16 +46,17 @@ library Utils {
 
         // Remove `getName()` selector if existing
         bool hasGetName = false;
-        for (uint256 i = 0; i < selectors.length; i++) {
+        uint256 selectorsLength = selectors.length;
+        for (uint256 i = 0; i < selectorsLength; ++i) {
             if (selectors[i] == bytes4(keccak256("getName()"))) {
-                selectors[i] = selectors[selectors.length - 1];
+                selectors[i] = selectors[selectorsLength - 1];
                 hasGetName = true;
                 break;
             }
         }
         if (hasGetName) {
-            bytes4[] memory newSelectors = new bytes4[](selectors.length - 1);
-            for (uint256 i = 0; i < selectors.length - 1; i++) {
+            bytes4[] memory newSelectors = new bytes4[](selectorsLength - 1);
+            for (uint256 i = 0; i < selectorsLength - 1; ++i) {
                 newSelectors[i] = selectors[i];
             }
             return newSelectors;
@@ -76,10 +79,11 @@ library Utils {
      */
     function bytesToUint256(bytes memory bys) internal pure returns (uint256 value) {
         // Add left padding to 32 bytes if needed
-        if (bys.length < 32) {
+        uint256 bytesLength = bys.length;
+        if (bytesLength < 32) {
             bytes memory padded = new bytes(32);
-            for (uint256 i = 0; i < bys.length; i++) {
-                padded[i + 32 - bys.length] = bys[i];
+            for (uint256 i = 0; i < bytesLength; ++i) {
+                padded[i + 32 - bytesLength] = bys[i];
             }
             bys = padded;
         }
@@ -125,8 +129,9 @@ library Utils {
             child := create(0, add(bytecode, 0x20), mload(bytecode))
         }
         vm.stopBroadcast();
-        require(child != address(0), "Failed to deploy Create2Factory");
-        require(child.code.length > 0, "Failed to deploy Create2Factory");
+        if (child == address(0) || child.code.length == 0) {
+            revert FailedToDeploy(ZksyncContract.Create2Factory);
+        }
         return child;
     }
 
@@ -135,7 +140,7 @@ library Utils {
      */
     function deployViaCreate2(bytes memory _bytecode, bytes32 _salt, address _factory) internal returns (address) {
         if (_bytecode.length == 0) {
-            revert("Bytecode is not set");
+            revert BytecodeNotSet();
         }
         address contractAddress = vm.computeCreate2Address(_salt, keccak256(_bytecode), _factory);
         if (contractAddress.code.length != 0) {
@@ -147,7 +152,7 @@ library Utils {
         contractAddress = Utils.bytesToAddress(data);
 
         if (!success || contractAddress == address(0) || contractAddress.code.length == 0) {
-            revert("Failed to deploy contract via create2");
+            revert FailedToDeployViaCreate2();
         }
 
         return contractAddress;