improve: N01-N16 Various Fixes (#23) (#441)

Changes by Nick.

Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>

Author: pxrl

contracts/Arbitrum_SpokePool.sol

@@ -39,7 +39,7 @@ contract Arbitrum_SpokePool is SpokePool, CircleCCTPAdapter {
         ITokenMessenger _cctpTokenMessenger
     )
         SpokePool(_wrappedNativeTokenAddress, _depositQuoteTimeBuffer, _fillDeadlineBuffer)
-        CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, 0)
+        CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, CircleDomainIds.Ethereum)
     {} // solhint-disable-line no-empty-blocks
 
     /**

contracts/BondToken.sol

@@ -24,7 +24,7 @@ interface ExtendedHubPoolInterface is HubPoolInterface {
 contract BondToken is WETH9, Ownable {
     using Address for address;
 
-    ExtendedHubPoolInterface public immutable hubPool;
+    ExtendedHubPoolInterface public immutable HUB_POOL;
 
     /**
      * @notice Addresses that are permitted to make HubPool root bundle proposals.
@@ -43,7 +43,7 @@ contract BondToken is WETH9, Ownable {
     constructor(ExtendedHubPoolInterface _hubPool) {
         name = "Across Bond Token";
         symbol = "ABT";
-        hubPool = _hubPool;
+        HUB_POOL = _hubPool;
     }
 
     /**
@@ -75,8 +75,8 @@ contract BondToken is WETH9, Ownable {
         address dst,
         uint256 amt
     ) public override returns (bool) {
-        if (dst == address(hubPool)) {
-            require(proposers[src] || hubPool.rootBundleProposal().proposer != src, "Transfer not permitted");
+        if (dst == address(HUB_POOL)) {
+            require(proposers[src] || HUB_POOL.rootBundleProposal().proposer != src, "Transfer not permitted");
         }
         return super.transferFrom(src, dst, amt);
     }

contracts/Linea_SpokePool.sol

@@ -5,82 +5,10 @@
 pragma solidity 0.8.19;
 
 import "./SpokePool.sol";
+import { IMessageService, ITokenBridge, IUSDCBridge } from "./external/interfaces/LineaInterfaces.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 
-/**
- * @notice Interface of Linea's Canonical Message Service
- * See https://github.com/Consensys/linea-contracts/blob/3cf85529fd4539eb06ba998030c37e47f98c528a/contracts/interfaces/IMessageService.sol
- */
-interface IMessageService {
-    /**
-     * @notice Sends a message for transporting from the given chain.
-     * @dev This function should be called with a msg.value = _value + _fee. The fee will be paid on the destination chain.
-     * @param _to The destination address on the destination chain.
-     * @param _fee The message service fee on the origin chain.
-     * @param _calldata The calldata used by the destination message service to call the destination contract.
-     */
-    function sendMessage(
-        address _to,
-        uint256 _fee,
-        bytes calldata _calldata
-    ) external payable;
-
-    /**
-     * @notice Returns the original sender of the message on the origin layer.
-     */
-    function sender() external view returns (address);
-
-    /**
-     * @notice Minimum fee to use when sending a message. Currently, only exists on L2MessageService.
-     * See https://github.com/Consensys/linea-contracts/blob/3cf85529fd4539eb06ba998030c37e47f98c528a/contracts/messageService/l2/L2MessageService.sol#L37
-     */
-    function minimumFeeInWei() external view returns (uint256);
-}
-
-/**
- * @notice Interface of Linea's Canonical Token Bridge
- * See https://github.com/Consensys/linea-contracts/blob/3cf85529fd4539eb06ba998030c37e47f98c528a/contracts/tokenBridge/interfaces/ITokenBridge.sol
- */
-interface ITokenBridge {
-    /**
-     * @notice This function is the single entry point to bridge tokens to the
-     *   other chain, both for native and already bridged tokens. You can use it
-     *   to bridge any ERC20. If the token is bridged for the first time an ERC20
-     *   (BridgedToken.sol) will be automatically deployed on the target chain.
-     * @dev User should first allow the bridge to transfer tokens on his behalf.
-     *   Alternatively, you can use `bridgeTokenWithPermit` to do so in a single
-     *   transaction. If you want the transfer to be automatically executed on the
-     *   destination chain. You should send enough ETH to pay the postman fees.
-     *   Note that Linea can reserve some tokens (which use a dedicated bridge).
-     *   In this case, the token cannot be bridged. Linea can only reserve tokens
-     *   that have not been bridged yet.
-     *   Linea can pause the bridge for security reason. In this case new bridge
-     *   transaction would revert.
-     * @param _token The address of the token to be bridged.
-     * @param _amount The amount of the token to be bridged.
-     * @param _recipient The address that will receive the tokens on the other chain.
-     */
-    function bridgeToken(
-        address _token,
-        uint256 _amount,
-        address _recipient
-    ) external payable;
-}
-
-interface IUSDCBridge {
-    function usdc() external view returns (address);
-
-    /**
-     * @dev Sends the sender's USDC from L1 to the recipient on L2, locks the USDC sent
-     * in this contract and sends a message to the message bridge
-     * contract to mint the equivalent USDC on L2
-     * @param amount The amount of USDC to send
-     * @param to The recipient's address to receive the funds
-     */
-    function depositTo(uint256 amount, address to) external payable;
-}
-
 /**
  * @notice Linea specific SpokePool.
  */
@@ -153,7 +81,7 @@ contract Linea_SpokePool is SpokePool {
      * sending L2->L1 messages.
      */
     function minimumFeeInWei() public view returns (uint256) {
-        return IMessageService(l2MessageService).minimumFeeInWei();
+        return l2MessageService.minimumFeeInWei();
     }
 
     /****************************************
@@ -218,7 +146,7 @@ contract Linea_SpokePool is SpokePool {
         // send ETH directly via the Canonical Message Service.
         if (l2TokenAddress == address(wrappedNativeToken)) {
             WETH9Interface(l2TokenAddress).withdraw(amountToReturn); // Unwrap into ETH.
-            IMessageService(l2MessageService).sendMessage{ value: amountToReturn + msg.value }(hubPool, msg.value, "");
+            l2MessageService.sendMessage{ value: amountToReturn + msg.value }(hubPool, msg.value, "");
         }
         // If the l1Token is USDC, then we need sent it via the USDC Bridge.
         else if (l2TokenAddress == l2UsdcBridge.usdc()) {
@@ -234,7 +162,7 @@ contract Linea_SpokePool is SpokePool {
 
     function _requireAdminSender() internal view override {
         require(
-            IMessageService(l2MessageService).sender() == crossDomainAdmin && msg.sender == address(l2MessageService),
+            l2MessageService.sender() == crossDomainAdmin && msg.sender == address(l2MessageService),
             "ONLY_COUNTERPART_GATEWAY"
         );
     }

contracts/Ovm_SpokePool.sol

@@ -36,7 +36,7 @@ contract Ovm_SpokePool is SpokePool, CircleCCTPAdapter {
     address public l2Eth;
 
     // Address of the Optimism L2 messenger.
-    address public messenger;
+    address public constant MESSENGER = Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
 
     // Address of custom bridge used to bridge Synthetix-related assets like SNX.
     address private constant SYNTHETIX_BRIDGE = 0x136b1EC699c62b0606854056f02dC7Bb80482d63;
@@ -60,7 +60,7 @@ contract Ovm_SpokePool is SpokePool, CircleCCTPAdapter {
         ITokenMessenger _cctpTokenMessenger
     )
         SpokePool(_wrappedNativeTokenAddress, _depositQuoteTimeBuffer, _fillDeadlineBuffer)
-        CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, 0)
+        CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, CircleDomainIds.Ethereum)
     {} // solhint-disable-line no-empty-blocks
 
     /**
@@ -80,7 +80,6 @@ contract Ovm_SpokePool is SpokePool, CircleCCTPAdapter {
     ) public onlyInitializing {
         l1Gas = 5_000_000;
         __SpokePool_init(_initialDepositId, _crossDomainAdmin, _hubPool);
-        messenger = Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
         //slither-disable-next-line missing-zero-check
         l2Eth = _l2Eth;
     }
@@ -170,7 +169,7 @@ contract Ovm_SpokePool is SpokePool, CircleCCTPAdapter {
     // Apply OVM-specific transformation to cross domain admin address on L1.
     function _requireAdminSender() internal view override {
         require(
-            LibOptimismUpgradeable.crossChainSender(messenger) == crossDomainAdmin,
+            LibOptimismUpgradeable.crossChainSender(MESSENGER) == crossDomainAdmin,
             "OVM_XCHAIN: wrong sender of cross-domain message"
         );
     }

contracts/PolygonTokenBridger.sol

@@ -43,7 +43,7 @@ contract PolygonTokenBridger is Lockable {
     using SafeERC20Upgradeable for IERC20Upgradeable;
 
     // Gas token for Polygon.
-    MaticToken public constant maticToken = MaticToken(0x0000000000000000000000000000000000001010);
+    MaticToken public constant MATIC = MaticToken(0x0000000000000000000000000000000000001010);
 
     // Should be set to HubPool on Ethereum, or unused on Polygon.
     address public immutable destination;
@@ -113,8 +113,7 @@ contract PolygonTokenBridger is Lockable {
         token.withdraw(token.balanceOf(address(this)));
 
         // This takes the token that was withdrawn and calls withdraw on the "native" ERC20.
-        if (address(token) == l2WrappedMatic)
-            maticToken.withdraw{ value: address(this).balance }(address(this).balance);
+        if (address(token) == l2WrappedMatic) MATIC.withdraw{ value: address(this).balance }(address(this).balance);
     }
 
     /**

contracts/PolygonZkEVM_SpokePool.sol

@@ -35,7 +35,7 @@ contract PolygonZkEVM_SpokePool is SpokePool, IBridgeMessageReceiver {
     IPolygonZkEVMBridge public l2PolygonZkEVMBridge;
 
     // Polygon zkEVM's internal network id for L1.
-    uint32 public constant l1NetworkId = 0;
+    uint32 public constant POLYGON_ZKEVM_L1_NETWORK_ID = 0;
 
     // Warning: this variable should _never_ be touched outside of this contract. It is intentionally set to be
     // private. Leaving it set to true can permanently disable admin calls.
@@ -136,7 +136,7 @@ contract PolygonZkEVM_SpokePool is SpokePool, IBridgeMessageReceiver {
         if (_originAddress != crossDomainAdmin) {
             revert OriginSenderNotCrossDomain();
         }
-        if (_originNetwork != l1NetworkId) {
+        if (_originNetwork != POLYGON_ZKEVM_L1_NETWORK_ID) {
             revert SourceChainNotHubChain();
         }
 
@@ -175,7 +175,7 @@ contract PolygonZkEVM_SpokePool is SpokePool, IBridgeMessageReceiver {
         if (l2TokenAddress == address(wrappedNativeToken)) {
             WETH9Interface(l2TokenAddress).withdraw(amountToReturn); // Unwrap into ETH.
             l2PolygonZkEVMBridge.bridgeAsset{ value: amountToReturn }(
-                l1NetworkId,
+                POLYGON_ZKEVM_L1_NETWORK_ID,
                 hubPool,
                 amountToReturn,
                 address(0),
@@ -185,7 +185,7 @@ contract PolygonZkEVM_SpokePool is SpokePool, IBridgeMessageReceiver {
         } else {
             IERC20(l2TokenAddress).safeIncreaseAllowance(address(l2PolygonZkEVMBridge), amountToReturn);
             l2PolygonZkEVMBridge.bridgeAsset(
-                l1NetworkId,
+                POLYGON_ZKEVM_L1_NETWORK_ID,
                 hubPool,
                 amountToReturn,
                 l2TokenAddress,

contracts/Polygon_SpokePool.sol

@@ -81,7 +81,7 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
         ITokenMessenger _cctpTokenMessenger
     )
         SpokePool(_wrappedNativeTokenAddress, _depositQuoteTimeBuffer, _fillDeadlineBuffer)
-        CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, 0)
+        CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, CircleDomainIds.Ethereum)
     {} // solhint-disable-line no-empty-blocks
 
     /**
@@ -102,9 +102,9 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
     ) public initializer {
         callValidated = false;
         __SpokePool_init(_initialDepositId, _crossDomainAdmin, _hubPool);
-        polygonTokenBridger = _polygonTokenBridger;
+        _setPolygonTokenBridger(payable(_polygonTokenBridger));
         //slither-disable-next-line missing-zero-check
-        fxChild = _fxChild;
+        _setFxChild(_fxChild);
     }
 
     /********************************************************
@@ -116,18 +116,15 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
      * @param newFxChild New FxChild.
      */
     function setFxChild(address newFxChild) public onlyAdmin nonReentrant {
-        //slither-disable-next-line missing-zero-check
-        fxChild = newFxChild;
-        emit SetFxChild(newFxChild);
+        _setFxChild(newFxChild);
     }
 
     /**
      * @notice Change polygonTokenBridger address. Callable only by admin via processMessageFromRoot.
      * @param newPolygonTokenBridger New Polygon Token Bridger contract.
      */
     function setPolygonTokenBridger(address payable newPolygonTokenBridger) public onlyAdmin nonReentrant {
-        polygonTokenBridger = PolygonTokenBridger(newPolygonTokenBridger);
-        emit SetPolygonTokenBridger(address(newPolygonTokenBridger));
+        _setPolygonTokenBridger(newPolygonTokenBridger);
     }
 
     /**
@@ -216,6 +213,17 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
      *        INTERNAL FUNCTIONS          *
      **************************************/
 
+    function _setFxChild(address _fxChild) internal {
+        //slither-disable-next-line missing-zero-check
+        fxChild = _fxChild;
+        emit SetFxChild(_fxChild);
+    }
+
+    function _setPolygonTokenBridger(address payable _polygonTokenBridger) internal {
+        polygonTokenBridger = PolygonTokenBridger(_polygonTokenBridger);
+        emit SetPolygonTokenBridger(address(_polygonTokenBridger));
+    }
+
     function _preExecuteLeafHook(address) internal override {
         // Wraps MATIC --> WMATIC before distributing tokens from this contract.
         _wrap();

contracts/Scroll_SpokePool.sol

@@ -57,8 +57,8 @@ contract Scroll_SpokePool is SpokePool {
         address _hubPool
     ) public initializer {
         __SpokePool_init(_initialDepositId, _crossDomainAdmin, _hubPool);
-        l2GatewayRouter = _l2GatewayRouter;
-        l2ScrollMessenger = _l2ScrollMessenger;
+        _setL2GatewayRouter(_l2GatewayRouter);
+        _setL2MessageService(_l2ScrollMessenger);
     }
 
     /**

contracts/SpokePool.sol

@@ -45,6 +45,7 @@ interface AcrossMessageHandler {
  * on the destination chain. Locked source chain tokens are later sent over the canonical token bridge to L1 HubPool.
  * Relayers are refunded with destination tokens out of this contract after another off-chain actor, a "data worker",
  * submits a proof that the relayer correctly submitted a relay on this SpokePool.
+ * @custom:security-contact bugs@across.to
  */
 abstract contract SpokePool is
     V3SpokePoolInterface,
@@ -161,6 +162,11 @@ abstract contract SpokePool is
             "UpdateDepositDetails(uint32 depositId,uint256 originChainId,uint256 updatedOutputAmount,address updatedRecipient,bytes updatedMessage)"
         );
 
+    // Default chain Id used to signify that no repayment is requested, for example when executing a slow fill.
+    uint256 public constant EMPTY_REPAYMENT_CHAIN_ID = 0;
+    // Default address used to signify that no relayer should be credited with a refund, for example
+    // when executing a slow fill.
+    address public constant EMPTY_RELAYER = address(0);
     // This is the magic value that signals to the off-chain validator
     // that this deposit can never expire. A deposit with this fill deadline should always be eligible for a
     // slow fill, meaning that its output token and input token must be "equivalent". Therefore, this value is only
@@ -215,18 +221,6 @@ abstract contract SpokePool is
         bytes message,
         RelayExecutionInfo updatableRelayData
     );
-    /// @custom:audit FOLLOWING EVENT TO BE DEPRECATED
-    event RefundRequested(
-        address indexed relayer,
-        address refundToken,
-        uint256 amount,
-        uint256 indexed originChainId,
-        uint256 destinationChainId,
-        int64 realizedLpFeePct,
-        uint32 indexed depositId,
-        uint256 fillBlock,
-        uint256 previousIdenticalRequests
-    );
     event RelayedRootBundle(
         uint32 indexed rootBundleId,
         bytes32 indexed relayerRefundRoot,
@@ -552,7 +546,7 @@ abstract contract SpokePool is
      * @notice This function is intended for multisig depositors who can accept some LP fee uncertainty in order to lift
      * the quoteTimestamp buffer constraint.
      * @dev Re-orgs may produce invalid fills if the quoteTimestamp moves across a change in HubPool utilisation.
-     * @dev The existing function modifiers are already enforced by _deposit(), so no additional modifiers are imposed.
+     * @dev The existing function modifiers are already enforced by deposit(), so no additional modifiers are imposed.
      * @param recipient Address to receive funds at on destination chain.
      * @param originToken Token to lock into this contract to initiate deposit.
      * @param amount Amount of tokens to deposit. Will be amount of tokens to receive less fees.
@@ -589,7 +583,7 @@ abstract contract SpokePool is
      * @notice This function is intended for multisig depositors who can accept some LP fee uncertainty in order to lift
      * the quoteTimestamp buffer constraint.
      * @dev Re-orgs may produce invalid fills if the quoteTimestamp moves across a change in HubPool utilisation.
-     * @dev The existing function modifiers are already enforced by _deposit(), so no additional modifiers are imposed.
+     * @dev The existing function modifiers are already enforced by depositFor(), so no additional modifiers are imposed.
      * @param depositor Address who is credited for depositing funds on origin chain and can speed up the deposit.
      * @param recipient Address to receive funds at on destination chain.
      * @param originToken Token to lock into this contract to initiate deposit.
@@ -707,7 +701,7 @@ abstract contract SpokePool is
      * ERC20.
      * @param inputAmount The amount of input tokens to pull from the caller's account and lock into this contract.
      * This amount will be sent to the relayer on their repayment chain of choice as a refund following an optimistic
-     * challenge window in the HubPool, plus a system fee.
+     * challenge window in the HubPool, less a system fee.
      * @param outputAmount The amount of output tokens that the relayer will send to the recipient on the destination.
      * @param destinationChainId The destination chain identifier. Must be enabled along with the input token
      * as a valid deposit route from this spoke pool or this transaction will revert.
@@ -1283,13 +1277,13 @@ abstract contract SpokePool is
             updatedOutputAmount: slowFillLeaf.updatedOutputAmount,
             updatedRecipient: relayData.recipient,
             updatedMessage: relayData.message,
-            repaymentChainId: 0 // Hardcoded to 0 for slow fills
+            repaymentChainId: EMPTY_REPAYMENT_CHAIN_ID // Repayment not relevant for slow fills.
         });
 
         _verifyV3SlowFill(relayExecution, rootBundleId, proof);
 
-        // - 0x0 hardcoded as relayer for slow fill execution.
-        _fillRelayV3(relayExecution, address(0), true);
+        // - No relayer to refund for slow fill executions.
+        _fillRelayV3(relayExecution, EMPTY_RELAYER, true);
     }
 
     /**