Add solhint (#54)

Add solhint & semgrep rules
PufferFinance · Oct 24, 2024 · 4703bcc · 4703bcc
1 parent 3e2db5f
commit 4703bcc
Show file tree

Hide file tree

Showing 138 changed files with 25,305 additions and 35 deletions.
diff --git a/.solhint.json b/.solhint.json
@@ -0,0 +1,63 @@
+{
+  "extends": "solhint:recommended",
+  "rules": {
+    "no-console": "warn",
+    "avoid-low-level-calls": "off",
+    "code-complexity": ["error", 9],
+    "compiler-version": ["error", ">=0.8.0 <0.9.0"],
+    "const-name-snakecase": "off",
+    "gas-custom-errors": "off",
+    "func-name-mixedcase": "error",
+    "immutable-vars-naming": "error",
+    "func-visibility": ["error", { "ignoreConstructors": true }],
+    "max-line-length": ["error", 123],
+    "named-parameters-mapping": "warn",
+    "no-global-import": "warn",
+    "imports-order": "off",
+    "no-unused-import": "error",
+    "avoid-call-value": "error",
+    "avoid-low-level-calls": "error",
+    "no-complex-fallback": "error",
+    "avoid-tx-origin": "error",
+    "check-send-result": "error",
+    "compiler-version": "error",
+    "no-inline-assembly": "error",
+    "not-rely-on-block-hash": "error",
+    "reentrancy": "error",
+    "modifier-name-mixedcase": "error",
+    "state-visibility": "error",
+    "multiple-sends": "error",
+    "named-parameters-mapping": "error",
+    "visibility-modifier-order": "warn",
+    "gas-calldata-parameters": "warn",
+    "quotes": "warn",
+    "gas-indexed-events": "off",
+    "gas-strict-inequalities": "off",
+    "gas-struct-packing": "error",
+    "gas-increment-by-one": "warn",
+    "gas-length-in-loops": "off",
+    "no-unused-vars": "error",
+    "no-empty-blocks": "off",
+    "not-rely-on-time": "off",
+    "one-contract-per-file": "off",
+    "var-name-mixedcase": "warn",
+    "func-order": "off",
+    "max-line-length": "off",
+    "reason-string": "off",
+    "func-named-parameters": "error",
+    "imports-on-top": "error",
+    "ordering": "off",
+    "func-param-name-mixedcase": "error",
+    "modifier-name-mixedcase": "error",
+    "private-vars-leading-underscore": "error",
+    "use-forbidden-name": "error",
+    "code-complexity": "error",
+    "explicit-types": "error",
+    "constructor-syntax": "error",
+    "interface-starts-with-i": "error",
+    "const-name-snakecase": "error",
+    "contract-name-camelcase": "error",
+    "event-name-camelcase": "error",
+    "payable-fallback": "error"
+  }
+}
diff --git a/mainnet-contracts/.semgrepignore b/mainnet-contracts/.semgrepignore
@@ -0,0 +1 @@
+test/*
diff --git a/mainnet-contracts/script/DeployFWR.s.sol b/mainnet-contracts/script/DeployFWR.s.sol
@@ -85,7 +85,7 @@ contract DeployFWR is DeployerHelper {
 
         // L1RewardManager
         L1RewardManager l1ReeardManagerImpl = new L1RewardManager({
-            XpufETH: _getXPufETH(),
+            xPufETH: _getXPufETH(),
             pufETH: _getPufferVault(),
             lockbox: _getLockbox(),
             l2RewardsManager: l2RewardManagerProxy

diff --git a/mainnet-contracts/script/DeployPufETHBridging.s.sol b/mainnet-contracts/script/DeployPufETHBridging.s.sol
@@ -64,7 +64,7 @@ contract DeployPufETHBridging is BaseScript {
 
         // L1RewardManager
         L1RewardManager l1RewardManagerImpl = new L1RewardManager({
-            XpufETH: address(xPufETHProxy),
+            xPufETH: address(xPufETHProxy),
             pufETH: deployment.pufferVault,
             lockbox: address(xERC20Lockbox),
             l2RewardsManager: address(l2RewardsManagerProxy)

diff --git a/mainnet-contracts/script/UpgradeL2RewardManager.s.sol b/mainnet-contracts/script/UpgradeL2RewardManager.s.sol
@@ -36,7 +36,7 @@ contract UpgradeL2RewardManager is DeployerHelper {
 
         // L1RewardManager
         L1RewardManager l1RewardManagerImpl = new L1RewardManager({
-            XpufETH: _getXPufETH(),
+            xPufETH: _getXPufETH(),
             pufETH: _getPufferVault(),
             lockbox: _getLockbox(),
             l2RewardsManager: l2RewardsManagerProxy

diff --git a/mainnet-contracts/src/GuardianModule.sol b/mainnet-contracts/src/GuardianModule.sol
@@ -142,7 +142,7 @@ contract GuardianModule is AccessManaged, IGuardianModule {
      */
     function validateProvisionNode(
         uint256 pufferModuleIndex,
-        bytes memory pubKey,
+        bytes calldata pubKey,
         bytes calldata signature,
         bytes calldata withdrawalCredentials,
         bytes32 depositDataRoot,

diff --git a/mainnet-contracts/src/L1RewardManager.sol b/mainnet-contracts/src/L1RewardManager.sol
@@ -50,8 +50,8 @@ contract L1RewardManager is
     /**
      * @custom:oz-upgrades-unsafe-allow constructor
      */
-    constructor(address XpufETH, address lockbox, address pufETH, address l2RewardsManager) {
-        XPUFETH = IERC20(XpufETH);
+    constructor(address xPufETH, address lockbox, address pufETH, address l2RewardsManager) {
+        XPUFETH = IERC20(xPufETH);
         LOCKBOX = IXERC20Lockbox(lockbox);
         PUFFER_VAULT = PufferVaultV3(payable(pufETH));
         L2_REWARDS_MANAGER = l2RewardsManager;
@@ -168,7 +168,7 @@ contract L1RewardManager is
      * @notice This contract receives XPufETH from the L2RewardManager via the bridge, unwraps it to pufETH and then burns the pufETH, reverting the original mintAndBridge call
      * @dev Restricted access to `ROLE_ID_BRIDGE`
      */
-    function xReceive(bytes32, uint256, address, address originSender, uint32 originDomainId, bytes memory callData)
+    function xReceive(bytes32, uint256, address, address originSender, uint32 originDomainId, bytes calldata callData)
         external
         override(IXReceiver)
         restricted
@@ -212,7 +212,7 @@ contract L1RewardManager is
      * @param bridgeData The updated bridge data.
      * @dev Restricted access to `ROLE_ID_DAO`
      */
-    function updateBridgeData(address bridge, BridgeData memory bridgeData) external restricted {
+    function updateBridgeData(address bridge, BridgeData calldata bridgeData) external restricted {
         RewardManagerStorage storage $ = _getRewardManagerStorage();
         if (bridge == address(0)) {
             revert InvalidAddress();

diff --git a/mainnet-contracts/src/LibBeaconchainContract.sol b/mainnet-contracts/src/LibBeaconchainContract.sol
@@ -16,7 +16,7 @@ library LibBeaconchainContract {
      * @param withdrawalCredentials is the withdrawal credentials
      * @return the deposit data root
      */
-    function getDepositDataRoot(bytes memory pubKey, bytes calldata signature, bytes memory withdrawalCredentials)
+    function getDepositDataRoot(bytes calldata pubKey, bytes calldata signature, bytes calldata withdrawalCredentials)
         external
         pure
         returns (bytes32)

diff --git a/mainnet-contracts/src/NoImplementation.sol b/mainnet-contracts/src/NoImplementation.sol
@@ -2,17 +2,16 @@
 pragma solidity >=0.8.0 <0.9.0;
 
 import { UUPSUpgradeable } from "@openzeppelin-contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
+import { Unauthorized } from "./Errors.sol";
 
 contract NoImplementation is UUPSUpgradeable {
-    address immutable upgrader;
+    address public immutable UPGRADER;
 
     constructor() {
-        upgrader = msg.sender;
+        UPGRADER = msg.sender;
     }
 
     function _authorizeUpgrade(address) internal virtual override {
-        // solhint-disable-next-line custom-errors
-        require(msg.sender == upgrader, "Unauthorized");
-        // anybody can steal this proxy
+        require(msg.sender == UPGRADER, Unauthorized());
     }
 }
diff --git a/mainnet-contracts/src/PufLocker.sol b/mainnet-contracts/src/PufLocker.sol
@@ -5,7 +5,6 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import { AccessManagedUpgradeable } from
     "@openzeppelin-contracts-upgradeable/access/manager/AccessManagedUpgradeable.sol";
-import { Address } from "@openzeppelin/contracts/utils/Address.sol";
 import { PufLockerStorage } from "./PufLockerStorage.sol";
 import { IPufLocker } from "./interface/IPufLocker.sol";
 import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";

diff --git a/mainnet-contracts/src/PufToken.sol b/mainnet-contracts/src/PufToken.sol
@@ -128,6 +128,7 @@ contract PufToken is IPufStakingPool, ERC20, ERC20Permit {
     /**
      * @inheritdoc IPufStakingPool
      */
+    // solhint-disable-next-line gas-calldata-parameters
     function migrateWithSignature(
         address depositor,
         address migratorContract,

diff --git a/mainnet-contracts/src/PufferDepositorV2.sol b/mainnet-contracts/src/PufferDepositorV2.sol
@@ -28,7 +28,7 @@ contract PufferDepositorV2 is IPufferDepositorV2, PufferDepositorStorage, Access
     /**
      * @dev Wallet that transferred pufETH to the PufferDepositor by mistake.
      */
-    address private constant PUFFER = 0x8A0C1e5cEA8e0F6dF341C005335E7fe5ed18A0a0;
+    address private constant _PUFFER = 0x8A0C1e5cEA8e0F6dF341C005335E7fe5ed18A0a0;
 
     /**
      * @dev The Puffer Vault contract address
@@ -46,7 +46,7 @@ contract PufferDepositorV2 is IPufferDepositorV2, PufferDepositorStorage, Access
     function initialize() public reinitializer(2) {
         // https://etherscan.io/token/0xd9a442856c234a39a81a089c06451ebaa4306a72?a=0x4aa799c5dfc01ee7d790e3bf1a7c2257ce1dceff
         // slither-disable-next-line unchecked-transfer
-        PUFFER_VAULT.transfer(PUFFER, 0.201 ether);
+        PUFFER_VAULT.transfer(_PUFFER, 0.201 ether);
     }
 
     /**

diff --git a/mainnet-contracts/src/PufferL2Depositor.sol b/mainnet-contracts/src/PufferL2Depositor.sol
@@ -163,7 +163,13 @@ contract PufferL2Depositor is IPufferL2Depositor, AccessManaged {
             pufToken.deposit(depositor, account, amount);
         }
 
-        emit DepositedToken(token, msg.sender, account, amount, referralCode);
+        emit DepositedToken({
+            token: token,
+            depositor: msg.sender,
+            account: account,
+            tokenAmount: amount,
+            referralCode: referralCode
+        });
     }
 
     function _addNewToken(address token) internal {

diff --git a/mainnet-contracts/src/PufferModule.sol b/mainnet-contracts/src/PufferModule.sol
@@ -260,6 +260,7 @@ contract PufferModule is IPufferModule, Initializable, AccessManagedUpgradeable
     /**
      * @inheritdoc IPufferModule
      */
+    // solhint-disable-next-line func-name-mixedcase
     function NAME() external view returns (bytes32) {
         ModuleStorage storage $ = _getPufferModuleStorage();
         return $.moduleName;

diff --git a/mainnet-contracts/src/PufferModuleManager.sol b/mainnet-contracts/src/PufferModuleManager.sol
@@ -104,8 +104,8 @@ contract PufferModuleManager is IPufferModuleManager, AccessManagedUpgradeable,
 
         uint256 sharesWithdrawn;
 
-        for (uint256 i = 0; i < withdrawals.length; i++) {
-            for (uint256 j = 0; j < withdrawals[i].shares.length; j++) {
+        for (uint256 i = 0; i < withdrawals.length; ++i) {
+            for (uint256 j = 0; j < withdrawals[i].shares.length; ++j) {
                 sharesWithdrawn += withdrawals[i].shares[j];
             }
         }
@@ -146,7 +146,8 @@ contract PufferModuleManager is IPufferModuleManager, AccessManagedUpgradeable,
     {
         uint256 totalRewardsAmount;
 
-        for (uint256 i = 0; i < modules.length; i++) {
+        for (uint256 i = 0; i < modules.length; ++i) {
+            //solhint-disable-next-line avoid-low-level-calls
             (bool success,) = IPufferModule(modules[i]).call(address(this), rewardsAmounts[i], "");
             if (!success) {
                 revert InvalidAmount();
@@ -362,7 +363,7 @@ contract PufferModuleManager is IPufferModuleManager, AccessManagedUpgradeable,
      * @dev Restricted to the DAO
      */
     function callStartCheckpoint(address[] calldata moduleAddresses) external virtual restricted {
-        for (uint256 i = 0; i < moduleAddresses.length; i++) {
+        for (uint256 i = 0; i < moduleAddresses.length; ++i) {
             // reverts if supplied with a duplicate module address
             IPufferModule(moduleAddresses[i]).startCheckpoint();
         }
@@ -389,7 +390,7 @@ contract PufferModuleManager is IPufferModuleManager, AccessManagedUpgradeable,
     function callUpdateOperatorAVSSocket(
         IRestakingOperator restakingOperator,
         address avsRegistryCoordinator,
-        string memory socket
+        string calldata socket
     ) external virtual restricted {
         restakingOperator.updateOperatorAVSSocket(avsRegistryCoordinator, socket);
 

diff --git a/mainnet-contracts/src/PufferOracleV2.sol b/mainnet-contracts/src/PufferOracleV2.sol
@@ -3,6 +3,7 @@ pragma solidity >=0.8.0 <0.9.0;
 
 import { IGuardianModule } from "./interface/IGuardianModule.sol";
 import { IPufferOracleV2 } from "./interface/IPufferOracleV2.sol";
+//solhint-disable-next-line no-unused-import
 import { IPufferOracle } from "./interface/IPufferOracle.sol";
 import { AccessManaged } from "@openzeppelin/contracts/access/manager/AccessManaged.sol";
 

diff --git a/mainnet-contracts/src/PufferProtocol.sol b/mainnet-contracts/src/PufferProtocol.sol
@@ -364,6 +364,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad
             // The excess is the rewards amount for that Node Operator
             uint256 transferAmount =
                 validatorInfos[i].withdrawalAmount > 32 ether ? 32 ether : validatorInfos[i].withdrawalAmount;
+            //solhint-disable-next-line avoid-low-level-calls
             (bool success,) = IPufferModule(validatorInfos[i].module).call(address(PUFFER_VAULT), transferAmount, "");
             if (!success) {
                 revert Failed();
@@ -843,7 +844,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad
     }
 
     function _decreaseNumberOfRegisteredValidators(ProtocolStorage storage $, bytes32 moduleName) internal {
-        $.moduleLimits[moduleName].numberOfRegisteredValidators -= 1;
+        --$.moduleLimits[moduleName].numberOfRegisteredValidators;
         emit NumberOfRegisteredValidatorsChanged(moduleName, $.moduleLimits[moduleName].numberOfRegisteredValidators);
     }
 

diff --git a/mainnet-contracts/src/PufferVaultV2.sol b/mainnet-contracts/src/PufferVaultV2.sol
@@ -47,8 +47,8 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
     /**
      * @dev Two wallets that transferred pufETH to the PufferVault by mistake.
      */
-    address private constant WHALE_PUFFER = 0xe6957D9b493b2f2634c8898AC09dc14Cb24BE222;
-    address private constant PUFFER = 0x34c912C13De7953530DBE4c32F597d1bAF77889b;
+    address private constant _WHALE_PUFFER = 0xe6957D9b493b2f2634c8898AC09dc14Cb24BE222;
+    address private constant _PUFFER = 0x34c912C13De7953530DBE4c32F597d1bAF77889b;
 
     constructor(
         IStETH stETH,
@@ -91,10 +91,10 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
 
             // https://etherscan.io/tx/0x2e02a00dbc8ba48cd65a6802d174c210d0c4869806a564cca0088e42d382b2ff
             // slither-disable-next-line unchecked-transfer
-            this.transfer(WHALE_PUFFER, 299.864287100672938618 ether);
+            this.transfer(_WHALE_PUFFER, 299.864287100672938618 ether);
             // https://etherscan.io/tx/0x7d309dc26cb3f0226e480e0d4c598707faee59d58bfc68bedb75cf5055ac274a
             // slither-disable-next-line unchecked-transfer
-            this.transfer(PUFFER, 25426113577506618);
+            this.transfer(_PUFFER, 25426113577506618);
         }
     }
 
@@ -600,13 +600,15 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
     }
 
     modifier markDeposit() virtual {
+        //solhint-disable-next-line no-inline-assembly
         assembly {
             tstore(_DEPOSIT_TRACKER_LOCATION, 1) // Store `1` in the deposit tracker location
         }
         _;
     }
 
     modifier revertIfDeposited() virtual {
+        //solhint-disable-next-line no-inline-assembly
         assembly {
             // If the deposit tracker location is set to `1`, revert with `DepositAndWithdrawalForbidden()`
             if tload(_DEPOSIT_TRACKER_LOCATION) {

diff --git a/mainnet-contracts/src/PufferVaultV3.sol b/mainnet-contracts/src/PufferVaultV3.sol
@@ -10,8 +10,6 @@ import { IDelegationManager } from "./interface/EigenLayer/IDelegationManager.so
 import { IWETH } from "./interface/Other/IWETH.sol";
 import { IPufferVaultV3 } from "./interface/IPufferVaultV3.sol";
 import { IPufferOracle } from "./interface/IPufferOracle.sol";
-import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
 
 /**

diff --git a/mainnet-contracts/src/PufferWithdrawalManager.sol b/mainnet-contracts/src/PufferWithdrawalManager.sol
@@ -3,7 +3,6 @@ pragma solidity >=0.8.0 <0.9.0;
 
 import { PufferVaultV3 } from "./PufferVaultV3.sol";
 import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
-import { Address } from "@openzeppelin/contracts/utils/Address.sol";
 import { IPufferWithdrawalManager } from "./interface/IPufferWithdrawalManager.sol";
 import { PufferWithdrawalManagerStorage } from "./PufferWithdrawalManagerStorage.sol";
 import { AccessManagedUpgradeable } from
@@ -71,13 +70,15 @@ contract PufferWithdrawalManager is
      * @notice Only one withdrawal request per transaction is allowed
      */
     modifier oneWithdrawalRequestAllowed() virtual {
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             // If the deposit tracker location is set to `1`, revert with `MultipleWithdrawalsAreForbidden()`
             if tload(_WITHDRAWAL_REQUEST_TRACKER_LOCATION) {
                 mstore(0x00, 0x0eca04b2) // Store the error signature `0x0eca04b2` for `error MultipleWithdrawalsAreForbidden()` in memory.
                 revert(0x1c, 0x04) // Revert by returning those 4 bytes. `revert MultipleWithdrawalsAreForbidden()`
             }
         }
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             tstore(_WITHDRAWAL_REQUEST_TRACKER_LOCATION, 1) // Store `1` in the deposit tracker location
         }

diff --git a/mainnet-contracts/src/RestakingOperator.sol b/mainnet-contracts/src/RestakingOperator.sol
@@ -154,7 +154,7 @@ contract RestakingOperator is IRestakingOperator, IERC1271, Initializable, Acces
         bytes calldata quorumNumbers,
         string calldata socket,
         IBLSApkRegistry.PubkeyRegistrationParams calldata params,
-        ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature
+        ISignatureUtils.SignatureWithSaltAndExpiry calldata operatorSignature
     ) external virtual onlyPufferModuleManager {
         IRegistryCoordinatorExtended(avsRegistryCoordinator).registerOperator({
             quorumNumbers: quorumNumbers,
@@ -174,8 +174,8 @@ contract RestakingOperator is IRestakingOperator, IERC1271, Initializable, Acces
         string calldata socket,
         IBLSApkRegistry.PubkeyRegistrationParams calldata params,
         IRegistryCoordinator.OperatorKickParam[] calldata operatorKickParams,
-        ISignatureUtils.SignatureWithSaltAndExpiry memory churnApproverSignature,
-        ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature
+        ISignatureUtils.SignatureWithSaltAndExpiry calldata churnApproverSignature,
+        ISignatureUtils.SignatureWithSaltAndExpiry calldata operatorSignature
     ) external virtual onlyPufferModuleManager {
         IRegistryCoordinatorExtended(avsRegistryCoordinator).registerOperatorWithChurn({
             quorumNumbers: quorumNumbers,
@@ -216,7 +216,7 @@ contract RestakingOperator is IRestakingOperator, IERC1271, Initializable, Acces
      * @inheritdoc IRestakingOperator
      * @dev Restricted to the PufferModuleManager
      */
-    function updateOperatorAVSSocket(address avsRegistryCoordinator, string memory socket)
+    function updateOperatorAVSSocket(address avsRegistryCoordinator, string calldata socket)
         external
         virtual
         onlyPufferModuleManager