address most of the PR comments about contracts

Signed-off-by: Ihor Farion <[email protected]>

contracts/AlephZero_SpokePool.sol

@@ -19,8 +19,7 @@ contract AlephZero_SpokePool is Arbitrum_SpokePool {
         IERC20 _l2Usdc,
         ITokenMessenger _cctpTokenMessenger,
         IERC20 _l2Usdt,
-        IOFT _oftMessenger,
-        uint32 _ethereumUsdtDstEid
+        IOFT _oftMessenger
     )
         Arbitrum_SpokePool(
             _wrappedNativeTokenAddress,
@@ -29,8 +28,7 @@ contract AlephZero_SpokePool is Arbitrum_SpokePool {
             _l2Usdc,
             _cctpTokenMessenger,
             _l2Usdt,
-            _oftMessenger,
-            _ethereumUsdtDstEid
+            _oftMessenger
         )
     {} // solhint-disable-line no-empty-blocks
 }

contracts/Arbitrum_SpokePool.sol

@@ -30,12 +30,11 @@ contract Arbitrum_SpokePool is SpokePool, CircleCCTPAdapter, OFTTransportAdapter
         IERC20 _l2Usdc,
         ITokenMessenger _cctpTokenMessenger,
         IERC20 _l2Usdt,
-        IOFT _oftMessenger,
-        uint32 _ethereumUsdtDstEid
+        IOFT _oftMessenger
     )
         SpokePool(_wrappedNativeTokenAddress, _depositQuoteTimeBuffer, _fillDeadlineBuffer)
         CircleCCTPAdapter(_l2Usdc, _cctpTokenMessenger, CircleDomainIds.Ethereum)
-        OFTTransportAdapter(_l2Usdt, _oftMessenger, _ethereumUsdtDstEid)
+        OFTTransportAdapter(_l2Usdt, _oftMessenger, OFTEIds.Ethereum)
     {} // solhint-disable-line no-empty-blocks
 
     /**

contracts/chain-adapters/Arbitrum_Adapter.sol

@@ -64,7 +64,6 @@ contract Arbitrum_Adapter is AdapterInterface, CircleCCTPAdapter, OFTTransportAd
      * @param _cctpTokenMessenger TokenMessenger contract to bridge via CCTP.
      * @param _l1Usdt USDT address on L1.
      * @param _oftTransport OFTAdapter proxy address on L1 to bridge via OFT.
-     * @param _arbitrumUsdtDstEid destination endpoint id of USDT OFTAdater on Arbitrum.
      */
     constructor(
         ArbitrumL1InboxLike _l1ArbitrumInbox,
@@ -73,11 +72,10 @@ contract Arbitrum_Adapter is AdapterInterface, CircleCCTPAdapter, OFTTransportAd
         IERC20 _l1Usdc,
         ITokenMessenger _cctpTokenMessenger,
         IERC20 _l1Usdt,
-        IOFT _oftTransport,
-        uint32 _arbitrumUsdtDstEid
+        IOFT _oftTransport
     )
         CircleCCTPAdapter(_l1Usdc, _cctpTokenMessenger, CircleDomainIds.Arbitrum)
-        OFTTransportAdapter(_l1Usdt, _oftTransport, _arbitrumUsdtDstEid)
+        OFTTransportAdapter(_l1Usdt, _oftTransport, OFTEIds.Arbitrum)
     {
         L1_INBOX = _l1ArbitrumInbox;
         L1_ERC20_GATEWAY_ROUTER = _l1ERC20GatewayRouter;

contracts/libraries/OFTTransportAdapter.sol

@@ -3,9 +3,21 @@ pragma solidity ^0.8.0;
 
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import { IOFT, SendParam, MessagingFee } from "@layerzerolabs/oft-evm/contracts/interfaces/IOFT.sol";
+import { IOFT, SendParam, MessagingFee, OFTReceipt } from "@layerzerolabs/oft-evm/contracts/interfaces/IOFT.sol";
 import { AddressToBytes32 } from "../libraries/AddressConverters.sol";
 
+/**
+ * @notice List of OFT endpoint ids for different chains.
+ * @dev source https://docs.layerzero.network/v2/developers/evm/technical-reference/deployed-contracts.
+ */
+library OFTEIds {
+    uint32 public constant Ethereum = 30101;
+    uint32 public constant Arbitrum = 30110;
+    // Use this value for placeholder purposes only for adapters that extend this adapter but haven't yet been
+    // assigned a domain ID by OFT messaging protocol.
+    uint32 public constant UNINITIALIZED = type(uint32).max;
+}
+
 /**
  * @notice Facilitate bridging USDT via LayerZero's OFT.
  * @dev This contract is intended to be inherited by other chain-specific adapters and spoke pools. This contract is built for 1-to-1 relationship with USDT to facilitate USDT0. When adding other contracts, we'd need to add a token mapping instead of the immutable token address.
@@ -19,35 +31,39 @@ contract OFTTransportAdapter {
     address public constant ZERO_ADDRESS = address(0);
 
     // USDT address on current chain.
-    IERC20 public immutable usdt;
+    IERC20 public immutable USDT;
     // Mailbox address for OFT cross-chain transfers.
-    IOFT public immutable oftMessenger;
+    IOFT public immutable OFT_MESSENGER;
 
     /**
      * @notice The destination endpoint id in the OFT messaging protocol.
      * @dev Source https://docs.layerzero.network/v2/developers/evm/technical-reference/deployed-contracts.
      * @dev Can also be found on target chain OFTAdapter -> endpoint() -> eid().
      */
-    uint32 public immutable dstEid;
+    uint32 public immutable DST_EID;
 
     /**
      * @notice intiailizes the OFTTransportAdapter contract.
      * @param _usdt USDT address on the current chain.
      * @param _oftMessenger OFTAdapter contract to bridge to the other chain. If address is set to zero, OFT bridging will be disabled.
-     * @param _dstEid The endpoint ID that OFT will transfer funds to.
+     * @param _oftDstEid The endpoint ID that OFT will transfer funds to.
      */
-    constructor(IERC20 _usdt, IOFT _oftMessenger, uint32 _dstEid) {
-        usdt = _usdt;
-        oftMessenger = _oftMessenger;
-        dstEid = _dstEid;
+    constructor(
+        IERC20 _usdt,
+        IOFT _oftMessenger,
+        uint32 _oftDstEid
+    ) {
+        USDT = _usdt;
+        OFT_MESSENGER = _oftMessenger;
+        DST_EID = _oftDstEid;
     }
 
     /**
      * @notice Returns whether or not the OFT bridge is enabled.
      * @dev If the IOFT is the zero address, OFT bridging is disabled.
      */
     function _isOFTEnabled(address _token) internal view returns (bool) {
-        return address(oftMessenger) != address(0) && address(usdt) == _token;
+        return address(OFT_MESSENGER) != address(0) && address(USDT) == _token;
     }
 
     /**
@@ -58,34 +74,36 @@ contract OFTTransportAdapter {
     function _transferUsdt(address _to, uint256 _amount) internal {
         bytes32 to = _to.toBytes32();
 
-        // todo: should probably just comment these 2 vars and use `_amount` in send call below
-        // `amountLD` and `minAmountLD` have a subtle relationship. OFT "removes dust" on .send, which should not affect USDT transfer because of how OFT works internally with 6-decimal tokens.
-        // Setting these two equal protects us from dust subtraction on the OFT side. If any dust is subtracted, the later .send should revert. Should be cautious with this logic.
-        uint256 amountLD = _amount;
-        uint256 minAmountLD = _amount;
-
         SendParam memory sendParam = SendParam(
-            dstEid,
+            DST_EID,
             to,
-            amountLD,
-            minAmountLD,
+            /**
+             * _amount, _amount here specify `amountLD` and `minAmountLD`. These 2 have a subtle relationship
+             * OFT "removes dust" on .send, which should not affect USDT transfer because of how OFT works internally with 6-decimal tokens.
+             * Setting them both to `_amount` protects us from dust subtraction on the OFT side. If any dust is subtracted, the later .send should revert.
+             */
+            _amount,
+            _amount,
+            /**
+             * EMPTY_MSG_BYTES, EMPTY_MSG_BYTES, EMPTY_MSG_BYTES here specify `extraOptions`, `composeMsg` and `oftCmd`.
+             * These can be set to empty bytes arrays for the purposes of sending a simple cross-chain transfer.
+             */
             EMPTY_MSG_BYTES,
             EMPTY_MSG_BYTES,
             EMPTY_MSG_BYTES
         );
 
-        MessagingFee memory fee = oftMessenger.quoteSend(sendParam, false);
+        // `false` in the 2nd param here refers to `bool _payInLzToken`. We will pay in native token, so set to `false`
+        MessagingFee memory fee = OFT_MESSENGER.quoteSend(sendParam, false);
 
-        // todo: here, we're relying on `oftMessenger` to use the full allowance. Otherwise, next .safeApprove will revert.
-        usdt.safeApprove(address(this), _amount);
+        // approve the exact _amount for `OFT_MESSENGER` to spend. Fee will be paid in native token
+        USDT.forceApprove(address(OFT_MESSENGER), _amount);
 
         // todo: not really sure if we should blindly trust the fee.nativeFee here and just send it away. Should we check it against some sane cap?
-        // setting refundAddress to zero addr here, because we calculate the fees precicely and we can save gas this way
-        oftMessenger.send{ value: fee.nativeFee }(sendParam, fee, ZERO_ADDRESS);
+        // setting `refundAddress` to `ZERO_ADDRESS` here, because we calculate the fees precicely and we can save gas this way
+        (, OFTReceipt memory oftReceipt) = OFT_MESSENGER.send{ value: fee.nativeFee }(sendParam, fee, ZERO_ADDRESS);
 
-        // todo: OFTAdapter enforces this, but should we check anyway?
-        // return vals from .send: (MessagingReceipt memory msgReceipt, OFTReceipt memory oftReceipt)
-        // require(amount == oftReceipt.amountSentLD);
-        // require(amount == oftReceipt.amountReceivedLD);
+        // we require that received amount of this transfer at destination exactly matches the sent amount
+        require(_amount == oftReceipt.amountReceivedLD, "incorrect amountReceivedLD");
     }
 }

package.json

@@ -140,6 +140,5 @@
   "resolutions": {
     "**/eccrypto/secp256k1": "3.8.1",
     "**/eth-crypto/secp256k1": "5.0.1"
-  },
-  "packageManager": "[email protected]+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
+  }
 }