Merge pull request #114 from safe-global/4337m-preparations-lint-setup

Audit preparations: Update Solidity linting setup, fix TS linting
safe-global · Oct 26, 2023 · 53349e2 · 53349e2
2 parents 94fe0c1 + dcd582f
commit 53349e2
Show file tree

Hide file tree

Showing 14 changed files with 3,713 additions and 8,810 deletions.
diff --git a/4337/.eslintrc.js b/4337/.eslintrc.js
@@ -0,0 +1,18 @@
+module.exports = {
+  env: {
+    browser: true,
+    es2021: true,
+    node: true,
+  },
+  extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'plugin:prettier/recommended'],
+  overrides: [],
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    ecmaVersion: 'latest',
+    sourceType: 'module',
+  },
+  plugins: ['@typescript-eslint', 'no-only-tests'],
+  rules: {
+    '@typescript-eslint/no-explicit-any': 'warn',
+  },
+}
diff --git a/4337/.prettierrc b/4337/.prettierrc
@@ -1,4 +1,5 @@
 {
+  "plugins": ["prettier-plugin-solidity"],
   "overrides": [
     {
       "files": "*.sol",

diff --git a/4337/.solhint.json b/4337/.solhint.json
@@ -1,6 +1,6 @@
 {
   "extends": "solhint:recommended",
-  "plugins": ["prettier"],
+  "plugins": [],
   "rules": {
     "compiler-version": "off",
     "func-visibility": [
@@ -9,10 +9,10 @@
         "ignoreConstructors": true
       }
     ],
-    "prettier/prettier": "error",
     "not-rely-on-time": "off",
     "reason-string": "off",
     "no-empty-blocks": "off",
-    "avoid-low-level-calls": "off"
+    "avoid-low-level-calls": "off",
+    "custom-errors": "off"
   }
 }
diff --git a/4337/contracts/EIP4337Module.sol b/4337/contracts/EIP4337Module.sol
@@ -3,7 +3,7 @@ pragma solidity >=0.8.0 <0.9.0;
 
 import {HandlerContext} from "@safe-global/safe-contracts/contracts/handler/HandlerContext.sol";
 import {CompatibilityFallbackHandler} from "@safe-global/safe-contracts/contracts/handler/CompatibilityFallbackHandler.sol";
-import {IAccount, INonceManager, UserOperation} from "./interfaces/ERC4337.sol";
+import {IAccount, UserOperation} from "./interfaces/ERC4337.sol";
 import {ISafe} from "./interfaces/Safe.sol";
 
 /**
@@ -22,10 +22,10 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
             "SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)"
         );
 
-    address public immutable supportedEntryPoint;
+    address public immutable SUPPORTED_ENTRYPOINT;
 
     constructor(address entryPoint) {
-        supportedEntryPoint = entryPoint;
+        SUPPORTED_ENTRYPOINT = entryPoint;
     }
 
     /**
@@ -34,11 +34,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
      * @param requiredPrefund Required prefund to execute the operation.
      * @return validationResult An integer indicating the result of the validation.
      */
-    function validateUserOp(
-        UserOperation calldata userOp,
-        bytes32,
-        uint256 requiredPrefund
-    ) external returns (uint256 validationResult) {
+    function validateUserOp(UserOperation calldata userOp, bytes32, uint256 requiredPrefund) external returns (uint256 validationResult) {
         address payable safeAddress = payable(userOp.sender);
         // The entryPoint address is appended to the calldata in `HandlerContext` contract
         // Because of this, the relayer may manipulate the entryPoint address, therefore we have to verify that
@@ -53,7 +49,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
         );
 
         address entryPoint = _msgSender();
-        require(entryPoint == supportedEntryPoint, "Unsupported entry point");
+        require(entryPoint == SUPPORTED_ENTRYPOINT, "Unsupported entry point");
 
         // The userOp nonce is validated in the Entrypoint (for 0.6.0+), therefore we will not check it again
         validationResult = validateSignatures(entryPoint, userOp);
@@ -72,14 +68,9 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
      * @param data Data payload of the user operation.
      * @param operation Operation type of the user operation.
      */
-    function executeUserOp(
-        address to,
-        uint256 value,
-        bytes memory data,
-        uint8 operation
-    ) external {
+    function executeUserOp(address to, uint256 value, bytes memory data, uint8 operation) external {
         address entryPoint = _msgSender();
-        require(entryPoint == supportedEntryPoint, "Unsupported entry point");
+        require(entryPoint == SUPPORTED_ENTRYPOINT, "Unsupported entry point");
 
         require(ISafe(msg.sender).execTransactionFromModule(to, value, data, operation), "Execution failed");
     }
@@ -91,14 +82,9 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
      * @param data Data payload of the user operation.
      * @param operation Operation type of the user operation.
      */
-    function executeUserOpWithErrorString(
-        address to,
-        uint256 value,
-        bytes memory data,
-        uint8 operation
-    ) external {
+    function executeUserOpWithErrorString(address to, uint256 value, bytes memory data, uint8 operation) external {
         address entryPoint = _msgSender();
-        require(entryPoint == supportedEntryPoint, "Unsupported entry point");
+        require(entryPoint == SUPPORTED_ENTRYPOINT, "Unsupported entry point");
 
         (bool success, bytes memory returnData) = ISafe(msg.sender).execTransactionFromModuleReturnData(to, value, data, operation);
         if (!success) {

diff --git a/4337/contracts/interfaces/ERC4337.sol b/4337/contracts/interfaces/ERC4337.sol
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-3.0
+/* solhint-disable one-contract-per-file */
 pragma solidity >=0.7.0 <0.9.0;
 
 interface INonceManager {

diff --git a/4337/contracts/test/SafeMock.sol b/4337/contracts/test/SafeMock.sol
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: LGPL-3.0-only
+/* solhint-disable one-contract-per-file */
 pragma solidity >=0.8.0;
 
 import "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
@@ -8,7 +9,7 @@ import {INonceManager, UserOperation} from "../interfaces/ERC4337.sol";
 import {UserOperationLib} from "./UserOperationLib.sol";
 
 contract SafeMock {
-    address public immutable supportedEntryPoint;
+    address public immutable SUPPORTED_ENTRYPOINT;
 
     address public singleton;
     address public owner;
@@ -17,26 +18,18 @@ contract SafeMock {
 
     constructor(address entryPoint) {
         owner = msg.sender;
-        supportedEntryPoint = entryPoint;
+        SUPPORTED_ENTRYPOINT = entryPoint;
     }
 
     function setup(address _fallbackHandler, address _module) public virtual {
         require(owner == address(0), "Already setup");
         owner = msg.sender;
         fallbackHandler = _fallbackHandler;
         modules[_module] = true;
-        modules[supportedEntryPoint] = true;
+        modules[SUPPORTED_ENTRYPOINT] = true;
     }
 
-    function signatureSplit(bytes memory signature)
-        internal
-        pure
-        returns (
-            uint8 v,
-            bytes32 r,
-            bytes32 s
-        )
-    {
+    function signatureSplit(bytes memory signature) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
         // solhint-disable-next-line no-inline-assembly
         assembly {
             r := mload(add(signature, 0x20))
@@ -45,11 +38,7 @@ contract SafeMock {
         }
     }
 
-    function checkSignatures(
-        bytes32 dataHash,
-        bytes memory,
-        bytes memory signature
-    ) public view {
+    function checkSignatures(bytes32 dataHash, bytes memory, bytes memory signature) public view {
         uint8 v;
         bytes32 r;
         bytes32 s;
@@ -123,13 +112,9 @@ contract Safe4337Mock is SafeMock {
     /// @dev Validates user operation provided by the entry point
     /// @param userOp User operation struct
     /// @param requiredPrefund Required prefund to execute the operation
-    function validateUserOp(
-        UserOperation calldata userOp,
-        bytes32,
-        uint256 requiredPrefund
-    ) external returns (uint256) {
+    function validateUserOp(UserOperation calldata userOp, bytes32, uint256 requiredPrefund) external returns (uint256) {
         address entryPoint = msg.sender;
-        require(entryPoint == supportedEntryPoint, "Unsupported entry point");
+        require(entryPoint == SUPPORTED_ENTRYPOINT, "Unsupported entry point");
 
         validateReplayProtection(userOp);
 
@@ -157,14 +142,9 @@ contract Safe4337Mock is SafeMock {
     /// @param value Ether value of the user operation.
     /// @param data Data payload of the user operation.
     /// @param operation Operation type of the user operation.
-    function executeUserOp(
-        address to,
-        uint256 value,
-        bytes memory data,
-        uint8 operation
-    ) external {
+    function executeUserOp(address to, uint256 value, bytes memory data, uint8 operation) external {
         address entryPoint = msg.sender;
-        require(entryPoint == supportedEntryPoint, "Unsupported entry point");
+        require(entryPoint == SUPPORTED_ENTRYPOINT, "Unsupported entry point");
 
         bool success;
         if (operation == 1) (success, ) = to.delegatecall(data);
@@ -178,14 +158,9 @@ contract Safe4337Mock is SafeMock {
     /// @param value Ether value of the user operation.
     /// @param data Data payload of the user operation.
     /// @param operation Operation type of the user operation.
-    function executeUserOpWithErrorString(
-        address to,
-        uint256 value,
-        bytes memory data,
-        uint8 operation
-    ) external {
+    function executeUserOpWithErrorString(address to, uint256 value, bytes memory data, uint8 operation) external {
         address entryPoint = msg.sender;
-        require(entryPoint == supportedEntryPoint, "Unsupported entry point");
+        require(entryPoint == SUPPORTED_ENTRYPOINT, "Unsupported entry point");
 
         bool success;
         bytes memory returnData;
@@ -293,7 +268,7 @@ contract Safe4337Mock is SafeMock {
         // The entrypoints handles the increase of the nonce
         // Right shifting fills up with 0s from the left
         uint192 key = uint192(userOp.nonce >> 64);
-        uint256 safeNonce = INonceManager(supportedEntryPoint).getNonce(userOp.sender, key);
+        uint256 safeNonce = INonceManager(SUPPORTED_ENTRYPOINT).getNonce(userOp.sender, key);
 
         // Check returned nonce against the user operation nonce
         require(safeNonce == userOp.nonce, "Invalid Nonce");

diff --git a/4337/contracts/test/TestEntryPoint.sol b/4337/contracts/test/TestEntryPoint.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: LGPL-3.0-only
+/* solhint-disable one-contract-per-file */
 pragma solidity >=0.8.0;
 import {INonceManager, IAccount, UserOperation} from "../interfaces/ERC4337.sol";
-import {UserOperationLib} from "./UserOperationLib.sol";
 
 /**
  * helper contract for EntryPoint, to call userOp.initCode from a "neutral" address,
@@ -32,12 +32,12 @@ contract TestEntryPoint is INonceManager {
     error NotEnoughFunds(uint256 expected, uint256 available);
     error InvalidNonce(uint256 userNonce);
     event UserOpReverted(bytes reason);
-    SenderCreator public immutable senderCreator;
+    SenderCreator public immutable SENDER_CREATOR;
     mapping(address => uint256) public balances;
     mapping(address => mapping(uint192 => uint256)) public nonceSequenceNumber;
 
     constructor() {
-        senderCreator = new SenderCreator();
+        SENDER_CREATOR = new SenderCreator();
     }
 
     receive() external payable {
@@ -47,7 +47,7 @@ contract TestEntryPoint is INonceManager {
     function executeUserOp(UserOperation calldata userOp, uint256 requiredPrefund) external {
         if (userOp.sender.code.length == 0) {
             require(userOp.initCode.length >= 20, "Invalid initCode provided");
-            require(userOp.sender == senderCreator.createSender(userOp.initCode), "Could not create expected account");
+            require(userOp.sender == SENDER_CREATOR.createSender(userOp.initCode), "Could not create expected account");
         }
 
         require(gasleft() > userOp.verificationGasLimit, "Not enough gas for verification");

diff --git a/4337/contracts/test/TestToken.sol b/4337/contracts/test/TestToken.sol
@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 pragma solidity >=0.8.0;
 
-import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
+import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
 
 contract HariWillibaldToken is ERC20, ERC20Permit {
     constructor(address initialMintDest) ERC20("Hari Willibald Token", "HWT") ERC20Permit("Hari Willibald Token") {
-        _mint(initialMintDest, 1000000 * 10**decimals());
+        _mint(initialMintDest, 1000000 * 10 ** decimals());
     }
 }