improve: address solhint warnings

.github/workflows/solhint.yml

@@ -0,0 +1,20 @@
+name: CI
+on: [push]
+jobs:
+  build:
+    name: Solhint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@v2
+
+      - name: Use Node ${{ matrix.node }}
+        uses: actions/setup-node@v2
+        with:
+          node-version: ${{ matrix.node }}
+
+      - name: Install deps and build (with cache)
+        uses: bahmutov/npm-install@v1
+
+      - name: Solhint
+        run: yarn lint-contracts

contracts/HubPool.sol

@@ -662,6 +662,10 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
         if (groupIndex == 0) {
             // Relay root bundles to spoke pool on destination chain by
             // performing delegatecall to use the adapter's code with this contract's context.
+
+            // We are ok with this low-level call since the adapter address is set by the admin and we've
+            // already checked that its not the zero address.
+            // solhint-disable-next-line avoid-low-level-calls
             (bool success, ) = adapter.delegatecall(
                 abi.encodeWithSignature(
                     "relayMessage(address,bytes)",
@@ -817,7 +821,9 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
      * messages to Arbitrum. Relaying messages to Arbitrum requires that this contract has an ETH balance, so in this
      * case the caller would need to pre-load this contract with ETH before multicall-executing the leaf.
      */
-    function loadEthForL2Calls() public payable override {}
+    function loadEthForL2Calls() public payable override {
+        /* solhint-disable-line no-empty-blocks */
+    }
 
     /*************************************************
      *              INTERNAL FUNCTIONS               *
@@ -866,6 +872,10 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
             if (netSendAmounts[i] > 0) {
                 // Perform delegatecall to use the adapter's code with this contract's context. Opt for delegatecall's
                 // complexity in exchange for lower gas costs.
+
+                // We are ok with this low-level call since the adapter address is set by the admin and we've
+                // already checked that its not the zero address.
+                // solhint-disable-next-line avoid-low-level-calls
                 (bool success, ) = adapter.delegatecall(
                     abi.encodeWithSignature(
                         "relayTokens(address,address,uint256,address)",
@@ -992,6 +1002,10 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
         (address adapter, address spokePool) = _getInitializedCrossChainContracts(chainId);
 
         // Perform delegatecall to use the adapter's code with this contract's context.
+
+        // We are ok with this low-level call since the adapter address is set by the admin and we've
+        // already checked that its not the zero address.
+        // solhint-disable-next-line avoid-low-level-calls
         (bool success, ) = adapter.delegatecall(
             abi.encodeWithSignature(
                 "relayMessage(address,bytes)",

contracts/chain-adapters/Arbitrum_Adapter.sol

@@ -39,6 +39,8 @@ interface ArbitrumL1ERC20GatewayLike {
  * For example, the HubPool will delegatecall these functions, therefore its only necessary that the HubPool's methods
  * that call this contract's logic guard against reentrancy.
  */
+
+// solhint-disable-next-line contract-name-camelcase
 contract Arbitrum_Adapter is AdapterInterface {
     using SafeERC20 for IERC20;
 

contracts/chain-adapters/CrossDomainEnabled.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity >0.5.0 <0.9.0;
+pragma solidity ^0.8.0;
 
 /* Interface Imports */
 import { ICrossDomainMessenger } from "@eth-optimism/contracts/libraries/bridge/ICrossDomainMessenger.sol";
@@ -27,11 +27,11 @@ contract CrossDomainEnabled {
      *  authenticated to call this function.
      */
     modifier onlyFromCrossDomainAccount(address _sourceDomainAccount) {
-        require(msg.sender == address(getCrossDomainMessenger()), "OVM_XCHAIN: messenger contract unauthenticated");
+        require(msg.sender == address(getCrossDomainMessenger()), "invalid cross domain messenger");
 
         require(
             getCrossDomainMessenger().xDomainMessageSender() == _sourceDomainAccount,
-            "OVM_XCHAIN: wrong sender of cross-domain message"
+            "invalid cross domain sender"
         );
 
         _;

contracts/chain-adapters/Ethereum_Adapter.sol

@@ -17,6 +17,8 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
  * For example, the HubPool will delegatecall these functions, therefore its only necessary that the HubPool's methods
  * that call this contract's logic guard against reentrancy.
  */
+
+// solhint-disable-next-linecontract-name-camelcase
 contract Ethereum_Adapter is AdapterInterface {
     using SafeERC20 for IERC20;
 
@@ -54,9 +56,10 @@ contract Ethereum_Adapter is AdapterInterface {
     // Note: this snippet of code is copied from Governor.sol. Source: https://github.com/UMAprotocol/protocol/blob/5b37ea818a28479c01e458389a83c3e736306b17/packages/core/contracts/oracle/implementation/Governor.sol#L190-L207
     function _executeCall(address to, bytes memory data) private {
         // Note: this snippet of code is copied from Governor.sol and modified to not include any "value" field.
-        // solhint-disable-next-line no-inline-assembly
 
         bool success;
+
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             let inputData := add(data, 0x20)
             let inputDataSize := mload(data)

contracts/chain-adapters/Mock_Adapter.sol

@@ -7,6 +7,8 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 /**
  * @notice Contract used for testing communication between HubPool and Adapter.
  */
+
+// solhint-disable-next-line contract-name-camelcase
 contract Mock_Adapter is AdapterInterface {
     event RelayMessageCalled(address target, bytes message, address caller);
 
@@ -37,11 +39,13 @@ contract Mock_Adapter is AdapterInterface {
 
 // This contract is intended to "act like" a simple version of an L2 bridge.
 // It's primarily meant to better reflect how a true L2 bridge interaction might work to give better gas estimates.
+
+// solhint-disable-next-line contract-name-camelcase
 contract Mock_Bridge {
     event BridgedTokens(address token, uint256 amount);
     event BridgedMessage(address target, bytes message);
 
-    mapping(address => uint256) deposits;
+    mapping(address => uint256) public deposits;
 
     function bridgeTokens(address token, uint256 amount) public {
         IERC20(token).transferFrom(msg.sender, address(this), amount);

contracts/chain-adapters/Optimism_Adapter.sol

@@ -19,6 +19,8 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
  * For example, the HubPool will delegatecall these functions, therefore its only necessary that the HubPool's methods
  * that call this contract's logic guard against reentrancy.
  */
+
+// solhint-disable-next-linecontract-name-camelcase
 contract Optimism_Adapter is CrossDomainEnabled, AdapterInterface {
     using SafeERC20 for IERC20;
     uint32 public immutable l2GasLimit = 2_000_000;

contracts/chain-adapters/Polygon_Adapter.sol

@@ -28,6 +28,8 @@ interface IFxStateSender {
  * For example, the HubPool will delegatecall these functions, therefore its only necessary that the HubPool's methods
  * that call this contract's logic guard against reentrancy.
  */
+
+// solhint-disable-next-linecontract-name-camelcase
 contract Polygon_Adapter is AdapterInterface {
     using SafeERC20 for IERC20;
     IRootChainManager public immutable rootChainManager;

contracts/test/ArbitrumMocks.sol

@@ -3,17 +3,17 @@ pragma solidity ^0.8.0;
 
 contract ArbitrumMockErc20GatewayRouter {
     function outboundTransfer(
-        address _token,
-        address _to,
-        uint256 _amount,
-        uint256 _maxGas,
-        uint256 _gasPriceBid,
+        address,
+        address,
+        uint256,
+        uint256,
+        uint256,
         bytes calldata _data
     ) external payable returns (bytes memory) {
         return _data;
     }
 
-    function getGateway(address _token) external view returns (address) {
+    function getGateway(address) external view returns (address) {
         return address(this);
     }
 }

contracts/test/MockSpokePool.sol

@@ -9,18 +9,20 @@ import "../SpokePoolInterface.sol";
  * @notice Implements abstract contract for testing.
  */
 contract MockSpokePool is SpokePool {
-    uint256 chainId_;
+    uint256 private chainId_;
 
+    // solhint-disable-next-line no-empty-blocks
     constructor(
         address _crossDomainAdmin,
         address _hubPool,
         address _wethAddress,
         address timerAddress
-    ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress) {}
+    ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress) {} // solhint-disable-line no-empty-blocks
 
+    // solhint-disable-next-line no-empty-blocks
     function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override {}
 
-    function _requireAdminSender() internal override {}
+    function _requireAdminSender() internal override {} // solhint-disable-line no-empty-blocks
 
     function chainId() public view override(SpokePool) returns (uint256) {
         // If chainId_ is set then return it, else do nothing and return the parent chainId().

contracts/test/PolygonERC20Test.sol

@@ -8,7 +8,7 @@ import "../PolygonTokenBridger.sol";
  * @notice Simulated Polygon ERC20 for use in testing PolygonTokenBridger.
  */
 contract PolygonERC20Test is ExpandedERC20, PolygonIERC20 {
-    constructor() ExpandedERC20("Polygon Test", "POLY_TEST", 18) {}
+    constructor() ExpandedERC20("Polygon Test", "POLY_TEST", 18) {} // solhint-disable-line no-empty-blocks
 
     function withdraw(uint256 amount) public {
         _burn(msg.sender, amount);

contracts/test/PolygonMocks.sol

@@ -2,15 +2,16 @@
 pragma solidity ^0.8.0;
 
 contract RootChainManagerMock {
-    function depositEtherFor(address user) external payable {}
+    function depositEtherFor(address user) external payable {} // solhint-disable-line no-empty-blocks
 
     function depositFor(
         address user,
         address rootToken,
         bytes calldata depositData
-    ) external {}
+    ) external {} // solhint-disable-line no-empty-blocks
 }
 
 contract FxStateSenderMock {
+    // solhint-disable-next-line no-empty-blocks
     function sendMessageToChild(address _receiver, bytes calldata _data) external {}
 }

package.json

@@ -15,6 +15,7 @@
   "types": "dist/index.d.ts",
   "main": "dist/index.js",
   "scripts": {
+    "lint-contracts": "yarn solhint ./contracts/**/*.sol",
     "lint": "yarn prettier --list-different",
     "lint-fix": "yarn prettier --write",
     "prettier": "prettier .",