Finish most remaining TODOs

contracts/Arbitrum_SpokePool.sol

@@ -82,7 +82,10 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool {
     }
 
     // l1 addresses are transformed during l1->l2 calls. See https://developer.offchainlabs.com/docs/l1_l2_messages#address-aliasing for more information.
+    // This cannot be pulled directly from Arbitrum contracts because their contracts are not 0.8.X compatible and this operation takes advantage of
+    // overflows, whose behavior changed in 0.8.0.
     function _applyL1ToL2Alias(address l1Address) internal pure returns (address l2Address) {
+        // Allows overflows as explained above.
         unchecked {
             l2Address = address(uint160(l1Address) + uint160(0x1111000000000000000000000000000000001111));
         }

contracts/HubPool.sol

@@ -359,6 +359,8 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
         bytes32 relayerRefundRoot,
         bytes32 slowRelayRoot
     ) public override nonReentrant noActiveRequests {
+        // Note: this is to prevent "empty block" style attacks where someone can make empty proposals that are
+        // technically valid but not useful. This could also potentially be enforced at the UMIP-level.
         require(poolRebalanceLeafCount > 0, "Bundle must have at least 1 leaf");
 
         uint64 requestExpirationTimestamp = uint64(getCurrentTime() + liveness);
@@ -398,6 +400,9 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
             "Bad Proof"
         );
 
+        // Before interacting with a particular chain's adapter, ensure that the adapter is set.
+        require(address(crossChainContracts[poolRebalanceLeaf.chainId].adapter) != address(0), "No adapter for chain");
+
         // Set the leafId in the claimed bitmap.
         rootBundleProposal.claimedBitMap = MerkleLib.setClaimed1D(
             rootBundleProposal.claimedBitMap,

contracts/SpokePool.sol

@@ -87,10 +87,10 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
     );
     event FilledRelay(
         bytes32 indexed relayHash,
-        uint256 totalRelayAmount,
+        uint256 amount,
         uint256 totalFilledAmount,
         uint256 fillAmount,
-        uint256 indexed repaymentChain,
+        uint256 indexed repaymentChainId,
         uint256 originChainId,
         uint64 relayerFeePct,
         uint64 realizedLpFeePct,
@@ -102,7 +102,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
     );
     event ExecutedSlowRelayRoot(
         bytes32 indexed relayHash,
-        uint256 totalRelayAmount,
+        uint256 amount,
         uint256 totalFilledAmount,
         uint256 fillAmount,
         uint256 originChainId,
@@ -215,7 +215,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         uint32 quoteTimestamp
     ) public payable onlyEnabledRoute(originToken, destinationChainId) nonReentrant {
         // We limit the relay fees to prevent the user spending all their funds on fees.
-        require(relayerFeePct <= 0.5e18, "invalid relayer fee");
+        require(relayerFeePct < 0.5e18, "invalid relayer fee");
         // Note We assume that L2 timing cannot be compared accurately and consistently to L1 timing. Therefore,
         // `block.timestamp` is different from the L1 EVM's. Therefore, the quoteTimestamp must be within a configurable
         // buffer to allow for this variance.
@@ -231,12 +231,10 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         if (originToken == address(weth) && msg.value > 0) {
             require(msg.value == amount, "msg.value must match amount");
             weth.deposit{ value: msg.value }();
-        } else {
             // Else, it is a normal ERC20. In this case pull the token from the users wallet as per normal.
             // Note: this includes the case where the L2 user has WETH (already wrapped ETH) and wants to bridge them. In
             // this case the msg.value will be set to 0, indicating a "normal" ERC20 bridging action.
-            IERC20(originToken).safeTransferFrom(msg.sender, address(this), amount);
-        }
+        } else IERC20(originToken).safeTransferFrom(msg.sender, address(this), amount);
 
         emit FundsDeposited(
             amount,
@@ -260,9 +258,9 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         address depositor,
         address recipient,
         address destinationToken,
-        uint256 totalRelayAmount,
+        uint256 amount,
         uint256 maxTokensToSend,
-        uint256 repaymentChain,
+        uint256 repaymentChainId,
         uint256 originChainId,
         uint64 realizedLpFeePct,
         uint64 relayerFeePct,
@@ -275,7 +273,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
             depositor: depositor,
             recipient: recipient,
             destinationToken: destinationToken,
-            relayAmount: totalRelayAmount,
+            amount: amount,
             realizedLpFeePct: realizedLpFeePct,
             relayerFeePct: relayerFeePct,
             depositId: depositId,
@@ -285,16 +283,16 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
 
         uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, maxTokensToSend, relayerFeePct, false);
 
-        _emitFillRelay(relayHash, fillAmountPreFees, repaymentChain, relayerFeePct, relayData);
+        _emitFillRelay(relayHash, fillAmountPreFees, repaymentChainId, relayerFeePct, relayData);
     }
 
     function fillRelayWithUpdatedFee(
         address depositor,
         address recipient,
         address destinationToken,
-        uint256 totalRelayAmount,
+        uint256 amount,
         uint256 maxTokensToSend,
-        uint256 repaymentChain,
+        uint256 repaymentChainId,
         uint256 originChainId,
         uint64 realizedLpFeePct,
         uint64 relayerFeePct,
@@ -328,7 +326,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
             depositor: depositor,
             recipient: recipient,
             destinationToken: destinationToken,
-            relayAmount: totalRelayAmount,
+            amount: amount,
             realizedLpFeePct: realizedLpFeePct,
             relayerFeePct: relayerFeePct,
             depositId: depositId,
@@ -337,7 +335,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         bytes32 relayHash = _getRelayHash(relayData);
         uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, maxTokensToSend, newRelayerFeePct, false);
 
-        _emitFillRelay(relayHash, fillAmountPreFees, repaymentChain, newRelayerFeePct, relayData);
+        _emitFillRelay(relayHash, fillAmountPreFees, repaymentChainId, newRelayerFeePct, relayData);
     }
 
     /**************************************
@@ -347,7 +345,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         address depositor,
         address recipient,
         address destinationToken,
-        uint256 totalRelayAmount,
+        uint256 amount,
         uint256 originChainId,
         uint64 realizedLpFeePct,
         uint64 relayerFeePct,
@@ -359,7 +357,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
             depositor,
             recipient,
             destinationToken,
-            totalRelayAmount,
+            amount,
             originChainId,
             realizedLpFeePct,
             relayerFeePct,
@@ -408,7 +406,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         // Verify the leafId in the leaf has not yet been claimed.
         require(!MerkleLib.isClaimed(rootBundle.claimedBitmap, relayerRefundLeaf.leafId), "Already claimed");
 
-        // Set leaf as claimed in bitmap.
+        // Set leaf as claimed in bitmap. This is passed by reference to the storage rootBundle.
         MerkleLib.setClaimed(rootBundle.claimedBitmap, relayerRefundLeaf.leafId);
 
         // Send each relayer refund address the associated refundAmount for the L2 token address.
@@ -449,7 +447,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         address depositor,
         address recipient,
         address destinationToken,
-        uint256 totalRelayAmount,
+        uint256 amount,
         uint256 originChainId,
         uint64 realizedLpFeePct,
         uint64 relayerFeePct,
@@ -461,7 +459,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
             depositor: depositor,
             recipient: recipient,
             destinationToken: destinationToken,
-            relayAmount: totalRelayAmount,
+            amount: amount,
             originChainId: originChainId,
             realizedLpFeePct: realizedLpFeePct,
             relayerFeePct: relayerFeePct,
@@ -477,7 +475,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
 
         // Note: use relayAmount as the max amount to send, so the relay is always completely filled by the contract's
         // funds in all cases.
-        uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, relayData.relayAmount, relayerFeePct, true);
+        uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, relayData.amount, relayerFeePct, true);
 
         _emitExecutedSlowRelayRoot(relayHash, fillAmountPreFees, relayData);
     }
@@ -541,7 +539,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
         RelayData memory relayData,
         uint256 maxTokensToSend,
         uint64 updatableRelayerFeePct,
-        bool isSlowRelay
+        bool useContractFunds
     ) internal returns (uint256 fillAmountPreFees) {
         // We limit the relay fees to prevent the user spending all their funds on fees. Note that 0.5e18 (i.e. 50%)
         // fees are just magic numbers. The important point is to prevent the total fee from being 100%, otherwise
@@ -550,57 +548,56 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
 
         // Check that the relay has not already been completely filled. Note that the `relays` mapping will point to
         // the amount filled so far for a particular `relayHash`, so this will start at 0 and increment with each fill.
-        require(relayFills[relayHash] < relayData.relayAmount, "relay filled");
+        require(relayFills[relayHash] < relayData.amount, "relay filled");
 
         // Stores the equivalent amount to be sent by the relayer before fees have been taken out.
-        fillAmountPreFees = 0;
+        if (maxTokensToSend == 0) return 0;
 
-        // Adding brackets "stack too deep" solidity error.
-        if (maxTokensToSend > 0) {
-            fillAmountPreFees = _computeAmountPreFees(
-                maxTokensToSend,
-                (relayData.realizedLpFeePct + updatableRelayerFeePct)
+        // todo: consider if this code block could be simplified. improve the branching comments and perhaps change the
+        // variable names.
+        fillAmountPreFees = _computeAmountPreFees(
+            maxTokensToSend,
+            (relayData.realizedLpFeePct + updatableRelayerFeePct)
+        );
+        // If user's specified max amount to send is greater than the amount of the relay remaining pre-fees,
+        // we'll pull exactly enough tokens to complete the relay.
+        uint256 amountToSend = maxTokensToSend;
+        if (relayData.amount - relayFills[relayHash] < fillAmountPreFees) {
+            fillAmountPreFees = relayData.amount - relayFills[relayHash];
+            amountToSend = _computeAmountPostFees(
+                fillAmountPreFees,
+                relayData.realizedLpFeePct + updatableRelayerFeePct
             );
-            // If user's specified max amount to send is greater than the amount of the relay remaining pre-fees,
-            // we'll pull exactly enough tokens to complete the relay.
-            uint256 amountToSend = maxTokensToSend;
-            if (relayData.relayAmount - relayFills[relayHash] < fillAmountPreFees) {
-                fillAmountPreFees = relayData.relayAmount - relayFills[relayHash];
-                amountToSend = _computeAmountPostFees(
-                    fillAmountPreFees,
-                    relayData.realizedLpFeePct + updatableRelayerFeePct
-                );
-            }
-            relayFills[relayHash] += fillAmountPreFees;
-            // If relay token is weth then unwrap and send eth.
-            if (relayData.destinationToken == address(weth)) {
-                // Note: WETH is already in the contract in the slow relay case.
-                if (!isSlowRelay)
-                    IERC20(relayData.destinationToken).safeTransferFrom(msg.sender, address(this), amountToSend);
-                _unwrapWETHTo(payable(relayData.recipient), amountToSend);
-                // Else, this is a normal ERC20 token. Send to recipient.
-            } else {
-                // Note: send token directly from the contract to the user in the slow relay case.
-                if (!isSlowRelay)
-                    IERC20(relayData.destinationToken).safeTransferFrom(msg.sender, relayData.recipient, amountToSend);
-                else IERC20(relayData.destinationToken).safeTransfer(relayData.recipient, amountToSend);
-            }
+        }
+        relayFills[relayHash] += fillAmountPreFees;
+        // If relay token is weth then unwrap and send eth.
+        if (relayData.destinationToken == address(weth)) {
+            // Note: WETH is already in the contract in the slow relay case.
+            if (!useContractFunds)
+                IERC20(relayData.destinationToken).safeTransferFrom(msg.sender, address(this), amountToSend);
+            _unwrapWETHTo(payable(relayData.recipient), amountToSend);
+            // Else, this is a normal ERC20 token. Send to recipient.
+        } else {
+            // Note: send token directly from the contract to the user in the slow relay case.
+            if (!useContractFunds)
+                IERC20(relayData.destinationToken).safeTransferFrom(msg.sender, relayData.recipient, amountToSend);
+            else IERC20(relayData.destinationToken).safeTransfer(relayData.recipient, amountToSend);
         }
     }
 
     function _emitFillRelay(
         bytes32 relayHash,
         uint256 fillAmount,
-        uint256 repaymentChain,
+        uint256 repaymentChainId,
         uint64 relayerFeePct,
         RelayData memory relayData
     ) internal {
         emit FilledRelay(
             relayHash,
-            relayData.relayAmount,
+            relayData.amount,
             relayFills[relayHash],
             fillAmount,
-            repaymentChain,
+            repaymentChainId,
             relayData.originChainId,
             relayerFeePct,
             relayData.realizedLpFeePct,
@@ -619,7 +616,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
     ) internal {
         emit ExecutedSlowRelayRoot(
             relayHash,
-            relayData.relayAmount,
+            relayData.amount,
             relayFills[relayHash],
             fillAmount,
             relayData.originChainId,

contracts/SpokePoolInterface.sol

@@ -31,7 +31,7 @@ interface SpokePoolInterface {
         // The corresponding token address on the destination chain.
         address destinationToken;
         // The total relay amount before fees are taken out.
-        uint256 relayAmount;
+        uint256 amount;
         // Origin chain id.
         uint256 originChainId;
         // The LP Fee percentage computed by the relayer based on the deposit's quote timestamp

test/SpokePool.Fixture.ts

@@ -66,7 +66,7 @@ export interface RelayData {
   depositor: string;
   recipient: string;
   destinationToken: string;
-  relayAmount: string;
+  amount: string;
   realizedLpFeePct: string;
   relayerFeePct: string;
   depositId: string;
@@ -78,15 +78,15 @@ export function getRelayHash(
   _depositId: number,
   _originChainId: number,
   _destinationToken: string,
-  _relayAmount?: string,
+  _amount?: string,
   _realizedLpFeePct?: string,
   _relayerFeePct?: string
 ): { relayHash: string; relayData: RelayData } {
   const relayData = {
     depositor: _depositor,
     recipient: _recipient,
     destinationToken: _destinationToken,
-    relayAmount: _relayAmount || consts.amountToDeposit.toString(),
+    amount: _amount || consts.amountToDeposit.toString(),
     originChainId: _originChainId.toString(),
     realizedLpFeePct: _realizedLpFeePct || consts.realizedLpFeePct.toString(),
     relayerFeePct: _relayerFeePct || consts.depositRelayerFeePct.toString(),
@@ -131,7 +131,7 @@ export function getFillRelayParams(
     _relayData.depositor,
     _relayData.recipient,
     _relayData.destinationToken,
-    _relayData.relayAmount,
+    _relayData.amount,
     _maxTokensToSend.toString(),
     _repaymentChain ? _repaymentChain.toString() : consts.repaymentChainId.toString(),
     _relayData.originChainId,
@@ -152,7 +152,7 @@ export function getFillRelayUpdatedFeeParams(
     _relayData.depositor,
     _relayData.recipient,
     _relayData.destinationToken,
-    _relayData.relayAmount,
+    _relayData.amount,
     _maxTokensToSend.toString(),
     _repaymentChain ? _repaymentChain.toString() : consts.repaymentChainId.toString(),
     _relayData.originChainId,
@@ -168,7 +168,7 @@ export function getExecuteSlowRelayParams(
   _depositor: string,
   _recipient: string,
   _destToken: string,
-  _amountToRelay: BigNumber,
+  _amount: BigNumber,
   _originChainId: number,
   _realizedLpFeePct: BigNumber,
   _relayerFeePct: BigNumber,
@@ -180,7 +180,7 @@ export function getExecuteSlowRelayParams(
     _depositor,
     _recipient,
     _destToken,
-    _amountToRelay.toString(),
+    _amount.toString(),
     _originChainId.toString(),
     _realizedLpFeePct.toString(),
     _relayerFeePct.toString(),

test/SpokePool.Relay.ts

@@ -43,7 +43,7 @@ describe("SpokePool Relayer Logic", async function () {
       .to.emit(spokePool, "FilledRelay")
       .withArgs(
         relayHash,
-        relayData.relayAmount,
+        relayData.amount,
         consts.amountToRelayPreFees,
         consts.amountToRelayPreFees,
         consts.repaymentChainId,