feat: Use @openzeppelin/[email protected] contracts in contracts we can u…

…pgrade Imports an [aliased npm package](https://forum.openzeppelin.com/t/coexist-of-v5-and-v4-contracts/38030/5) so we can use multiple oz libraries in the same solidity contracts. This way we get access to new and improved contracts. I make changes to all contracts that we can upgrade, such as SpokePools and NOT the HubPool. For example, I can use the new SafeERC20 method `forceApprove` instead of `safeIncreaseAllowance` which ensures [compatibility with tokens like USDT](OpenZeppelin/openzeppelin-contracts#4231) that make sure all approvals are set to 0 before granting a new approval. This hasn't been an issue so far because we always safeIncreaseAllowance to some number and use the complete allowance, but its worth safety checking. Other changes: - Moved `MerkleDistributor` out of uma/core into this repo to reduce dependency on this external repo - Replaced isCode() call with now [recommended](OpenZeppelin/openzeppelin-contracts#3945) explicit .code.length check - Explicitly set SpokePoolVerifier pragma to 0.8.19 and removed the overrides in hardhat.config.ts - Removed unused import in PolygonERC20Test
across-protocol · Sep 5, 2024 · a484de3 · a484de3
1 parent f56146a
commit a484de3
Show file tree

Hide file tree

Showing 56 changed files with 456 additions and 187 deletions.
diff --git a/contracts/Blast_DaiRetriever.sol b/contracts/Blast_DaiRetriever.sol
@@ -3,8 +3,8 @@ pragma solidity ^0.8.0;
 
 import "./Lockable.sol";
 
-import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
-import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
 import "@uma/core/contracts/common/implementation/MultiCaller.sol";
 
 interface USDYieldManager {
@@ -19,7 +19,7 @@ interface USDYieldManager {
  * and then an EOA can call this contract to retrieve the DAI.
  */
 contract Blast_DaiRetriever is Lockable, MultiCaller {
-    using SafeERC20Upgradeable for IERC20Upgradeable;
+    using SafeERC20 for IERC20;
 
     // Should be set to HubPool on Ethereum
     address public immutable hubPool;
@@ -28,7 +28,7 @@ contract Blast_DaiRetriever is Lockable, MultiCaller {
     USDYieldManager public immutable usdYieldManager;
 
     // Token to be retrieved.
-    IERC20Upgradeable public immutable dai;
+    IERC20 public immutable dai;
 
     /**
      * @notice Constructs USDB Retriever
@@ -39,7 +39,7 @@ contract Blast_DaiRetriever is Lockable, MultiCaller {
     constructor(
         address _hubPool,
         USDYieldManager _usdYieldManager,
-        IERC20Upgradeable _dai
+        IERC20 _dai
     ) {
         //slither-disable-next-line missing-zero-check
         hubPool = _hubPool;

diff --git a/contracts/Ethereum_SpokePool.sol b/contracts/Ethereum_SpokePool.sol
@@ -9,7 +9,7 @@ import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
  * @custom:security-contact [email protected]
  */
 contract Ethereum_SpokePool is SpokePool, OwnableUpgradeable {
-    using SafeERC20Upgradeable for IERC20Upgradeable;
+    using SafeERC20 for IERC20;
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(
@@ -35,7 +35,7 @@ contract Ethereum_SpokePool is SpokePool, OwnableUpgradeable {
      **************************************/
 
     function _bridgeTokensToHubPool(uint256 amountToReturn, address l2TokenAddress) internal override {
-        IERC20Upgradeable(l2TokenAddress).safeTransfer(hubPool, amountToReturn);
+        IERC20(l2TokenAddress).safeTransfer(hubPool, amountToReturn);
     }
 
     // The SpokePool deployed to the same network as the HubPool must be owned by the HubPool.

diff --git a/contracts/Linea_SpokePool.sol b/contracts/Linea_SpokePool.sol
@@ -6,8 +6,8 @@ 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";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
 
 /**
  * @notice Linea specific SpokePool.
@@ -154,12 +154,12 @@ contract Linea_SpokePool is SpokePool {
         }
         // If the l1Token is USDC, then we need sent it via the USDC Bridge.
         else if (l2TokenAddress == l2UsdcBridge.usdc()) {
-            IERC20(l2TokenAddress).safeIncreaseAllowance(address(l2UsdcBridge), amountToReturn);
+            IERC20(l2TokenAddress).forceApprove(address(l2UsdcBridge), amountToReturn);
             l2UsdcBridge.depositTo{ value: msg.value }(amountToReturn, hubPool);
         }
         // For other tokens, we can use the Canonical Token Bridge.
         else {
-            IERC20(l2TokenAddress).safeIncreaseAllowance(address(l2TokenBridge), amountToReturn);
+            IERC20(l2TokenAddress).forceApprove(address(l2TokenBridge), amountToReturn);
             l2TokenBridge.bridgeToken{ value: msg.value }(l2TokenAddress, amountToReturn, hubPool);
         }
     }

diff --git a/contracts/Ovm_SpokePool.sol b/contracts/Ovm_SpokePool.sol
@@ -177,7 +177,7 @@ contract Ovm_SpokePool is SpokePool, CircleCCTPAdapter {
             if (remoteL1Tokens[l2TokenAddress] != address(0)) {
                 // If there is a mapping for this L2 token to an L1 token, then use the L1 token address and
                 // call bridgeERC20To.
-                IERC20(l2TokenAddress).safeIncreaseAllowance(address(tokenBridge), amountToReturn);
+                IERC20(l2TokenAddress).forceApprove(address(tokenBridge), amountToReturn);
                 address remoteL1Token = remoteL1Tokens[l2TokenAddress];
                 tokenBridge.bridgeERC20To(
                     l2TokenAddress, // _l2Token. Address of the L2 token to bridge over.

diff --git a/contracts/PermissionSplitterProxy.sol b/contracts/PermissionSplitterProxy.sol
@@ -2,7 +2,7 @@
 pragma solidity ^0.8.0;
 
 import "@uma/core/contracts/common/implementation/MultiCaller.sol";
-import "@openzeppelin/contracts/access/AccessControl.sol";
+import "@openzeppelin/contracts5/access/AccessControl.sol";
 
 /**
  * @notice This contract is designed to own an Ownable "target" contract and gate access to specific

diff --git a/contracts/PolygonTokenBridger.sol b/contracts/PolygonTokenBridger.sol
@@ -4,8 +4,8 @@ pragma solidity ^0.8.0;
 import "./Lockable.sol";
 import "./external/interfaces/WETH9Interface.sol";
 
-import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
-import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
 
 // Polygon Registry contract that stores their addresses.
 interface PolygonRegistry {
@@ -18,7 +18,7 @@ interface PolygonERC20Predicate {
 }
 
 // ERC20s (on polygon) compatible with polygon's bridge have a withdraw method.
-interface PolygonIERC20Upgradeable is IERC20Upgradeable {
+interface PolygonIERC20Upgradeable is IERC20 {
     function withdraw(uint256 amount) external;
 }
 
@@ -40,8 +40,8 @@ interface MaticToken {
  * @custom:security-contact [email protected]
  */
 contract PolygonTokenBridger is Lockable {
-    using SafeERC20Upgradeable for PolygonIERC20Upgradeable;
-    using SafeERC20Upgradeable for IERC20Upgradeable;
+    using SafeERC20 for PolygonIERC20Upgradeable;
+    using SafeERC20 for IERC20;
 
     // Gas token for Polygon.
     MaticToken public constant MATIC = MaticToken(0x0000000000000000000000000000000000001010);
@@ -121,7 +121,7 @@ contract PolygonTokenBridger is Lockable {
      * @notice Called by someone to send tokens to the destination, which should be set to the HubPool.
      * @param token Token to send to destination.
      */
-    function retrieve(IERC20Upgradeable token) public nonReentrant onlyChainId(l1ChainId) {
+    function retrieve(IERC20 token) public nonReentrant onlyChainId(l1ChainId) {
         if (address(token) == address(l1Weth)) {
             // For WETH, there is a pre-deposit step to ensure any ETH that has been sent to the contract is captured.
             //slither-disable-next-line arbitrary-send-eth

diff --git a/contracts/PolygonZkEVM_SpokePool.sol b/contracts/PolygonZkEVM_SpokePool.sol
@@ -4,8 +4,8 @@ pragma solidity ^0.8.0;
 import "./SpokePool.sol";
 import "./external/interfaces/IPolygonZkEVMBridge.sol";
 
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
 
 /**
  * @notice Define interface for PolygonZkEVM Bridge message receiver
@@ -184,7 +184,7 @@ contract PolygonZkEVM_SpokePool is SpokePool, IBridgeMessageReceiver {
                 ""
             );
         } else {
-            IERC20(l2TokenAddress).safeIncreaseAllowance(address(l2PolygonZkEVMBridge), amountToReturn);
+            IERC20(l2TokenAddress).forceApprove(address(l2PolygonZkEVMBridge), amountToReturn);
             l2PolygonZkEVMBridge.bridgeAsset(
                 POLYGON_ZKEVM_L1_NETWORK_ID,
                 hubPool,

diff --git a/contracts/Polygon_SpokePool.sol b/contracts/Polygon_SpokePool.sol
@@ -33,7 +33,7 @@ interface IFxMessageProcessor {
  * @custom:security-contact [email protected]
  */
 contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter {
-    using SafeERC20Upgradeable for PolygonIERC20Upgradeable;
+    using SafeERC20 for PolygonIERC20Upgradeable;
 
     // Address of FxChild which sends and receives messages to and from L1.
     address public fxChild;
@@ -241,10 +241,7 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
         if (_isCCTPEnabled() && l2TokenAddress == address(usdcToken)) {
             _transferUsdc(hubPool, amountToReturn);
         } else {
-            PolygonIERC20Upgradeable(l2TokenAddress).safeIncreaseAllowance(
-                address(polygonTokenBridger),
-                amountToReturn
-            );
+            PolygonIERC20Upgradeable(l2TokenAddress).forceApprove(address(polygonTokenBridger), amountToReturn);
             // Note: WrappedNativeToken is WMATIC on matic, so this tells the tokenbridger that this is an unwrappable native token.
             polygonTokenBridger.send(PolygonIERC20Upgradeable(l2TokenAddress), amountToReturn);
         }

diff --git a/contracts/Scroll_SpokePool.sol b/contracts/Scroll_SpokePool.sol
@@ -18,7 +18,7 @@ interface IL2GatewayRouterExtended is IL2GatewayRouter {
  * @custom:security-contact [email protected]
  */
 contract Scroll_SpokePool is SpokePool {
-    using SafeERC20Upgradeable for IERC20Upgradeable;
+    using SafeERC20 for IERC20;
 
     /**
      * @notice The address of the official l2GatewayRouter contract for Scroll for bridging tokens from L2 -> L1
@@ -99,7 +99,7 @@ contract Scroll_SpokePool is SpokePool {
         // Tokens with a custom ERC20 gateway require an approval in order to withdraw.
         address erc20Gateway = l2GatewayRouter.getERC20Gateway(l2TokenAddress);
         if (erc20Gateway != l2GatewayRouter.defaultERC20Gateway()) {
-            IERC20Upgradeable(l2TokenAddress).safeIncreaseAllowance(erc20Gateway, amountToReturn);
+            IERC20(l2TokenAddress).forceApprove(erc20Gateway, amountToReturn);
         }
 
         // The scroll bridge handles arbitrary ERC20 tokens and is mindful of the official WETH address on-chain.

diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity ^0.8.0;
 
-import "./MerkleLib.sol";
+import { MerkleLib } from "./MerkleLib.sol";
 import "./external/interfaces/WETH9Interface.sol";
 import "./interfaces/SpokePoolMessageHandler.sol";
 import "./interfaces/SpokePoolInterface.sol";
@@ -10,12 +10,17 @@ import "./upgradeable/MultiCallerUpgradeable.sol";
 import "./upgradeable/EIP712CrossChainUpgradeable.sol";
 import "./upgradeable/AddressLibUpgradeable.sol";
 
-import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
-import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
-import "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts5/utils/cryptography/SignatureChecker.sol";
+import "@openzeppelin/contracts5/utils/math/SignedMath.sol";
+
+// Use 4.x version of OpenZeppelin proxy contract which match those deployed in production and we don't plan
+// to upgrade.
 import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
+// We can't use the 4.x version of ReentrancyGuardUpgradeable because it imports Initializable
+// which UUPSUpgradeable also imports so these two versions need to match.
 import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
-import "@openzeppelin/contracts/utils/math/SignedMath.sol";
 
 /**
  * @title SpokePool
@@ -35,7 +40,7 @@ abstract contract SpokePool is
     MultiCallerUpgradeable,
     EIP712CrossChainUpgradeable
 {
-    using SafeERC20Upgradeable for IERC20Upgradeable;
+    using SafeERC20 for IERC20;
     using AddressLibUpgradeable for address;
 
     // Address of the L1 contract that acts as the owner of this SpokePool. This should normally be set to the HubPool
@@ -579,7 +584,7 @@ abstract contract SpokePool is
         } else {
             // msg.value should be 0 if input token isn't the wrapped native token.
             if (msg.value != 0) revert MsgValueDoesNotMatchInputAmount();
-            IERC20Upgradeable(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
+            IERC20(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
         }
 
         emit V3FundsDeposited(
@@ -1103,7 +1108,7 @@ abstract contract SpokePool is
             // Else, it is a normal ERC20. In this case pull the token from the user's 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.
-        } else IERC20Upgradeable(originToken).safeTransferFrom(msg.sender, address(this), amount);
+        } else IERC20(originToken).safeTransferFrom(msg.sender, address(this), amount);
 
         emit V3FundsDeposited(
             originToken, // inputToken
@@ -1147,7 +1152,7 @@ abstract contract SpokePool is
         uint256 length = refundAmounts.length;
         for (uint256 i = 0; i < length; ++i) {
             uint256 amount = refundAmounts[i];
-            if (amount > 0) IERC20Upgradeable(l2TokenAddress).safeTransfer(refundAddresses[i], amount);
+            if (amount > 0) IERC20(l2TokenAddress).safeTransfer(refundAddresses[i], amount);
         }
 
         // If leaf's amountToReturn is positive, then send L2 --> L1 message to bridge tokens back via
@@ -1269,7 +1274,7 @@ abstract contract SpokePool is
     // Unwraps ETH and does a transfer to a recipient address. If the recipient is a smart contract then sends wrappedNativeToken.
     function _unwrapwrappedNativeTokenTo(address payable to, uint256 amount) internal {
         if (address(to).isContract()) {
-            IERC20Upgradeable(address(wrappedNativeToken)).safeTransfer(to, amount);
+            IERC20(address(wrappedNativeToken)).safeTransfer(to, amount);
         } else {
             wrappedNativeToken.withdraw(amount);
             AddressLibUpgradeable.sendValue(to, amount);
@@ -1357,13 +1362,13 @@ abstract contract SpokePool is
             // recipient wants wrappedNativeToken, then we can assume that wrappedNativeToken is already in the
             // contract, otherwise we'll need the user to send wrappedNativeToken to this contract. Regardless, we'll
             // need to unwrap it to native token before sending to the user.
-            if (!isSlowFill) IERC20Upgradeable(outputToken).safeTransferFrom(msg.sender, address(this), amountToSend);
+            if (!isSlowFill) IERC20(outputToken).safeTransferFrom(msg.sender, address(this), amountToSend);
             _unwrapwrappedNativeTokenTo(payable(recipientToSend), amountToSend);
             // Else, this is a normal ERC20 token. Send to recipient.
         } else {
             // Note: Similar to note above, send token directly from the contract to the user in the slow relay case.
-            if (!isSlowFill) IERC20Upgradeable(outputToken).safeTransferFrom(msg.sender, recipientToSend, amountToSend);
-            else IERC20Upgradeable(outputToken).safeTransfer(recipientToSend, amountToSend);
+            if (!isSlowFill) IERC20(outputToken).safeTransferFrom(msg.sender, recipientToSend, amountToSend);
+            else IERC20(outputToken).safeTransfer(recipientToSend, amountToSend);
         }
 
         bytes memory updatedMessage = relayExecution.updatedMessage;

diff --git a/contracts/SpokePoolVerifier.sol b/contracts/SpokePoolVerifier.sol
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: BUSL-1.1
-pragma solidity ^0.8.0;
+// NOTE: Linea and Arbitrum only support 0.8.19, so keep this contract at 19 so create2 addressses for all chains are
+// the same.
+pragma solidity ^0.8.19;
 
-import "@openzeppelin/contracts/utils/Address.sol";
 import "./interfaces/V3SpokePoolInterface.sol";
 
 /**
@@ -13,8 +14,6 @@ import "./interfaces/V3SpokePoolInterface.sol";
  * @custom:security-contact [email protected]
  */
 contract SpokePoolVerifier {
-    using Address for address;
-
     error InvalidMsgValue();
     error InvalidSpokePool();
 
@@ -54,7 +53,7 @@ contract SpokePoolVerifier {
         bytes memory message
     ) external payable {
         if (msg.value != inputAmount) revert InvalidMsgValue();
-        if (!address(spokePool).isContract()) revert InvalidSpokePool();
+        if (address(spokePool).code.length == 0) revert InvalidSpokePool();
         // Set msg.sender as the depositor so that msg.sender can speed up the deposit.
         spokePool.depositV3{ value: msg.value }(
             msg.sender,

diff --git a/contracts/SwapAndBridge.sol b/contracts/SwapAndBridge.sol
@@ -2,8 +2,8 @@
 pragma solidity ^0.8.0;
 
 import "./interfaces/V3SpokePoolInterface.sol";
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
 import "./Lockable.sol";
 import "@uma/core/contracts/common/implementation/MultiCaller.sol";
 
@@ -109,7 +109,7 @@ abstract contract SwapAndBridgeBase is Lockable, MultiCaller {
         uint256 srcBalanceBefore = _swapToken.balanceOf(address(this));
         uint256 dstBalanceBefore = _acrossInputToken.balanceOf(address(this));
 
-        _swapToken.safeIncreaseAllowance(EXCHANGE, swapTokenAmount);
+        _swapToken.forceApprove(EXCHANGE, swapTokenAmount);
         // solhint-disable-next-line avoid-low-level-calls
         (bool success, bytes memory result) = EXCHANGE.call(routerCalldata);
         require(success, string(result));
@@ -159,7 +159,7 @@ abstract contract SwapAndBridgeBase is Lockable, MultiCaller {
             depositData.outputAmount
         );
         // Deposit the swapped tokens into Across and bridge them using remainder of input params.
-        _acrossInputToken.safeIncreaseAllowance(address(SPOKE_POOL), returnAmount);
+        _acrossInputToken.forceApprove(address(SPOKE_POOL), returnAmount);
         SPOKE_POOL.depositV3(
             depositData.depositor,
             depositData.recipient,

diff --git a/contracts/chain-adapters/Arbitrum_Adapter.sol b/contracts/chain-adapters/Arbitrum_Adapter.sol
@@ -3,8 +3,8 @@ pragma solidity ^0.8.0;
 
 import "./interfaces/AdapterInterface.sol";
 
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts5/token/ERC20/utils/SafeERC20.sol";
 import "../external/interfaces/CCTPInterfaces.sol";
 import "../libraries/CircleCCTPAdapter.sol";
 
@@ -238,7 +238,7 @@ contract Arbitrum_Adapter is AdapterInterface, CircleCCTPAdapter {
             // Approve the gateway, not the router, to spend the hub pool's balance. The gateway, which is different
             // per L1 token, will temporarily escrow the tokens to be bridged and pull them from this contract.
             address erc20Gateway = L1_ERC20_GATEWAY_ROUTER.getGateway(l1Token);
-            IERC20(l1Token).safeIncreaseAllowance(erc20Gateway, amount);
+            IERC20(l1Token).forceApprove(erc20Gateway, amount);
 
             // `outboundTransfer` expects that the caller includes a bytes message as the last param that includes the
             // maxSubmissionCost to use when creating an L2 retryable ticket: https://github.com/OffchainLabs/arbitrum/blob/e98d14873dd77513b569771f47b5e05b72402c5e/packages/arb-bridge-peripherals/contracts/tokenbridge/ethereum/gateway/L1GatewayRouter.sol#L232