!feat: Make SpokePools upgradeable

.gitignore

@@ -12,5 +12,8 @@ artifacts
 cache-zk
 artifacts-zk
 
+# Upgradeability files
+.openzeppelin
+
 # IDE
 .idea

contracts/Arbitrum_SpokePool.sol

@@ -35,16 +35,17 @@ contract Arbitrum_SpokePool is SpokePool {
      * @param _crossDomainAdmin Cross domain admin to set. Can be changed by admin.
      * @param _hubPool Hub pool address to set. Can be changed by admin.
      * @param _wethAddress Weth address for this network to set.
-     * @param timerAddress Timer address to set.
+     * @param _timerAddress Timer address to set.
      */
-    constructor(
+    function initialize(
         uint32 _initialDepositId,
         address _l2GatewayRouter,
         address _crossDomainAdmin,
         address _hubPool,
         address _wethAddress,
-        address timerAddress
-    ) SpokePool(_initialDepositId, _crossDomainAdmin, _hubPool, _wethAddress, timerAddress) {
+        address _timerAddress
+    ) public initializer {
+        __SpokePool_init(_initialDepositId, _crossDomainAdmin, _hubPool, _wethAddress, _timerAddress);
         _setL2GatewayRouter(_l2GatewayRouter);
     }
 

contracts/Boba_SpokePool.sol

@@ -13,21 +13,21 @@ contract Boba_SpokePool is Ovm_SpokePool {
      * relay hash collisions.
      * @param _crossDomainAdmin Cross domain admin to set. Can be changed by admin.
      * @param _hubPool Hub pool address to set. Can be changed by admin.
-     * @param timerAddress Timer address to set.
+     * @param _timerAddress Timer address to set.
      */
-    constructor(
+    function initialize(
         uint32 _initialDepositId,
         address _crossDomainAdmin,
         address _hubPool,
-        address timerAddress
-    )
-        Ovm_SpokePool(
+        address _timerAddress
+    ) public initializer {
+        __OvmSpokePool_init(
             _initialDepositId,
             _crossDomainAdmin,
             _hubPool,
             0x4200000000000000000000000000000000000006,
             0xDeadDeAddeAddEAddeadDEaDDEAdDeaDDeAD0000,
-            timerAddress
-        )
-    {}
+            _timerAddress
+        );
+    }
 }

contracts/Ethereum_SpokePool.sol

@@ -2,15 +2,13 @@
 pragma solidity ^0.8.0;
 
 import "./SpokePool.sol";
-import "@openzeppelin/contracts/access/Ownable.sol";
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 
 /**
  * @notice Ethereum L1 specific SpokePool. Used on Ethereum L1 to facilitate L2->L1 transfers.
  */
-contract Ethereum_SpokePool is SpokePool, Ownable {
-    using SafeERC20 for IERC20;
+contract Ethereum_SpokePool is SpokePool, OwnableUpgradeable {
+    using SafeERC20Upgradeable for IERC20Upgradeable;
 
     /**
      * @notice Construct the Ethereum SpokePool.
@@ -19,21 +17,24 @@ contract Ethereum_SpokePool is SpokePool, Ownable {
      * relay hash collisions.
      * @param _hubPool Hub pool address to set. Can be changed by admin.
      * @param _wethAddress Weth address for this network to set.
-     * @param timerAddress Timer address to set.
+     * @param _timerAddress Timer address to set.
      */
-    constructor(
+    function initialize(
         uint32 _initialDepositId,
         address _hubPool,
         address _wethAddress,
-        address timerAddress
-    ) SpokePool(_initialDepositId, _hubPool, _hubPool, _wethAddress, timerAddress) {}
+        address _timerAddress
+    ) public initializer {
+        __Ownable_init();
+        __SpokePool_init(_initialDepositId, _hubPool, _hubPool, _wethAddress, _timerAddress);
+    }
 
     /**************************************
      *          INTERNAL FUNCTIONS           *
      **************************************/
 
     function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override {
-        IERC20(relayerRefundLeaf.l2TokenAddress).safeTransfer(hubPool, relayerRefundLeaf.amountToReturn);
+        IERC20Upgradeable(relayerRefundLeaf.l2TokenAddress).safeTransfer(hubPool, relayerRefundLeaf.amountToReturn);
     }
 
     // The SpokePool deployed to the same network as the HubPool must be owned by the HubPool.

contracts/Optimism_SpokePool.sol

@@ -14,21 +14,21 @@ contract Optimism_SpokePool is Ovm_SpokePool {
      * relay hash collisions.
      * @param _crossDomainAdmin Cross domain admin to set. Can be changed by admin.
      * @param _hubPool Hub pool address to set. Can be changed by admin.
-     * @param timerAddress Timer address to set.
+     * @param _timerAddress Timer address to set.
      */
-    constructor(
+    function initialize(
         uint32 _initialDepositId,
         address _crossDomainAdmin,
         address _hubPool,
-        address timerAddress
-    )
-        Ovm_SpokePool(
+        address _timerAddress
+    ) public initializer {
+        __OvmSpokePool_init(
             _initialDepositId,
             _crossDomainAdmin,
             _hubPool,
             Lib_PredeployAddresses.OVM_ETH,
             0x4200000000000000000000000000000000000006,
-            timerAddress
-        )
-    {}
+            _timerAddress
+        );
+    }
 }

contracts/Ovm_SpokePool.sol

@@ -4,7 +4,7 @@ pragma solidity ^0.8.0;
 import "./SpokePool.sol";
 import "./interfaces/WETH9Interface.sol";
 
-import "@eth-optimism/contracts/libraries/bridge/CrossDomainEnabled.sol";
+import "@openzeppelin/contracts-upgradeable/crosschain/optimism/LibOptimismUpgradeable.sol";
 import "@eth-optimism/contracts/libraries/constants/Lib_PredeployAddresses.sol";
 import "@eth-optimism/contracts/L2/messaging/IL2ERC20Bridge.sol";
 
@@ -16,13 +16,21 @@ interface SynthetixBridgeToBase {
 /**
  * @notice OVM specific SpokePool. Uses OVM cross-domain-enabled logic to implement admin only access to functions. * Optimism and Boba each implement this spoke pool and set their chain specific contract addresses for l2Eth and l2Weth.
  */
-contract Ovm_SpokePool is CrossDomainEnabled, SpokePool {
+contract Ovm_SpokePool is SpokePool {
     // "l1Gas" parameter used in call to bridge tokens from this contract back to L1 via IL2ERC20Bridge. Currently
     // unused by bridge but included for future compatibility.
-    uint32 public l1Gas = 5_000_000;
+    uint32 public l1Gas;
 
     // ETH is an ERC20 on OVM.
-    address public immutable l2Eth;
+    address public l2Eth;
+
+    // Address of the Optimism L2 messenger.
+    address public messenger;
+
+    // Reserve storage slots for future versions of this base contract to add state variables without
+    // affecting the storage layout of child contracts. Decrement the size of __gap whenever state variables
+    // are added.
+    uint256[1000] private __gap;
 
     // Stores alternative token bridges to use for L2 tokens that don't go over the standard bridge. This is needed
     // to support non-standard ERC20 tokens on Optimism, such as DIA and SNX which both use custom bridges.
@@ -38,19 +46,19 @@ contract Ovm_SpokePool is CrossDomainEnabled, SpokePool {
      * relay hash collisions.
      * @param _crossDomainAdmin Cross domain admin to set. Can be changed by admin.
      * @param _hubPool Hub pool address to set. Can be changed by admin.
-     * @param timerAddress Timer address to set.
+     * @param _timerAddress Timer address to set.
      */
-    constructor(
+    function __OvmSpokePool_init(
         uint32 _initialDepositId,
         address _crossDomainAdmin,
         address _hubPool,
         address _l2Eth,
         address _wrappedNativeToken,
-        address timerAddress
-    )
-        CrossDomainEnabled(Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER)
-        SpokePool(_initialDepositId, _crossDomainAdmin, _hubPool, _wrappedNativeToken, timerAddress)
-    {
+        address _timerAddress
+    ) public onlyInitializing {
+        l1Gas = 5_000_000;
+        __SpokePool_init(_initialDepositId, _crossDomainAdmin, _hubPool, _wrappedNativeToken, _timerAddress);
+        messenger = Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
         l2Eth = _l2Eth;
     }
 
@@ -172,5 +180,10 @@ contract Ovm_SpokePool is CrossDomainEnabled, SpokePool {
     }
 
     // Apply OVM-specific transformation to cross domain admin address on L1.
-    function _requireAdminSender() internal override onlyFromCrossDomainAccount(crossDomainAdmin) {}
+    function _requireAdminSender() internal view override {
+        require(
+            LibOptimismUpgradeable.crossChainSender(messenger) == crossDomainAdmin,
+            "OVM_XCHAIN: wrong sender of cross-domain message"
+        );
+    }
 }

contracts/PolygonTokenBridger.sol

@@ -4,8 +4,8 @@ pragma solidity ^0.8.0;
 import "./Lockable.sol";
 import "./interfaces/WETH9Interface.sol";
 
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
+import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.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 PolygonIERC20 is IERC20 {
+interface PolygonIERC20Upgradeable is IERC20Upgradeable {
     function withdraw(uint256 amount) external;
 }
 
@@ -39,8 +39,8 @@ interface MaticToken {
  * sender.
  */
 contract PolygonTokenBridger is Lockable {
-    using SafeERC20 for PolygonIERC20;
-    using SafeERC20 for IERC20;
+    using SafeERC20Upgradeable for PolygonIERC20Upgradeable;
+    using SafeERC20Upgradeable for IERC20Upgradeable;
 
     // Gas token for Polygon.
     MaticToken public constant maticToken = MaticToken(0x0000000000000000000000000000000000001010);
@@ -104,7 +104,7 @@ contract PolygonTokenBridger is Lockable {
      * @param token Token to bridge.
      * @param amount Amount to bridge.
      */
-    function send(PolygonIERC20 token, uint256 amount) public nonReentrant onlyChainId(l2ChainId) {
+    function send(PolygonIERC20Upgradeable token, uint256 amount) public nonReentrant onlyChainId(l2ChainId) {
         token.safeTransferFrom(msg.sender, address(this), amount);
 
         // In the wMatic case, this unwraps. For other ERC20s, this is the burn/send action.
@@ -119,7 +119,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(IERC20 token) public nonReentrant onlyChainId(l1ChainId) {
+    function retrieve(IERC20Upgradeable 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.
             l1Weth.deposit{ value: address(this).balance }();

contracts/Polygon_SpokePool.sol

@@ -6,9 +6,6 @@ import "./PolygonTokenBridger.sol";
 import "./interfaces/WETH9Interface.sol";
 import "./SpokePoolInterface.sol";
 
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-
 // IFxMessageProcessor represents interface to process messages.
 interface IFxMessageProcessor {
     function processMessageFromRoot(
@@ -22,7 +19,7 @@ interface IFxMessageProcessor {
  * @notice Polygon specific SpokePool.
  */
 contract Polygon_SpokePool is IFxMessageProcessor, SpokePool {
-    using SafeERC20 for PolygonIERC20;
+    using SafeERC20Upgradeable for PolygonIERC20Upgradeable;
 
     // Address of FxChild which sends and receives messages to and from L1.
     address public fxChild;
@@ -33,7 +30,7 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool {
 
     // Internal variable that only flips temporarily to true upon receiving messages from L1. Used to authenticate that
     // the caller is the fxChild AND that the fxChild called processMessageFromRoot
-    bool private callValidated = false;
+    bool private callValidated;
 
     event PolygonTokensBridged(address indexed token, address indexed receiver, uint256 amount);
     event SetFxChild(address indexed newFxChild);
@@ -69,17 +66,19 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool {
      * @param _hubPool Hub pool address to set. Can be changed by admin.
      * @param _wmaticAddress Replaces wrappedNativeToken for this network since MATIC is the native currency on polygon.
      * @param _fxChild FxChild contract, changeable by Admin.
-     * @param timerAddress Timer address to set.
+     * @param _timerAddress Timer address to set.
      */
-    constructor(
+    function initialize(
         uint32 _initialDepositId,
         PolygonTokenBridger _polygonTokenBridger,
         address _crossDomainAdmin,
         address _hubPool,
         address _wmaticAddress, // Note: wmatic is used here since it is the token sent via msg.value on polygon.
         address _fxChild,
-        address timerAddress
-    ) SpokePool(_initialDepositId, _crossDomainAdmin, _hubPool, _wmaticAddress, timerAddress) {
+        address _timerAddress
+    ) public initializer {
+        callValidated = false;
+        __SpokePool_init(_initialDepositId, _crossDomainAdmin, _hubPool, _wmaticAddress, _timerAddress);
         polygonTokenBridger = _polygonTokenBridger;
         fxChild = _fxChild;
     }
@@ -126,6 +125,9 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool {
         require(rootMessageSender == crossDomainAdmin, "Not from mainnet admin");
 
         // This uses delegatecall to take the information in the message and process it as a function call on this contract.
+        /// This is a safe delegatecall because its made to address(this) so there is no risk of delegating to a
+        /// selfdestruct().
+        /// @custom:oz-upgrades-unsafe-allow delegatecall
         (bool success, ) = address(this).delegatecall(data);
         require(success, "delegatecall failed");
     }
@@ -209,13 +211,16 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool {
      **************************************/
 
     function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override {
-        PolygonIERC20(relayerRefundLeaf.l2TokenAddress).safeIncreaseAllowance(
+        PolygonIERC20Upgradeable(relayerRefundLeaf.l2TokenAddress).safeIncreaseAllowance(
             address(polygonTokenBridger),
             relayerRefundLeaf.amountToReturn
         );
 
         // Note: WrappedNativeToken is WMATIC on matic, so this tells the tokenbridger that this is an unwrappable native token.
-        polygonTokenBridger.send(PolygonIERC20(relayerRefundLeaf.l2TokenAddress), relayerRefundLeaf.amountToReturn);
+        polygonTokenBridger.send(
+            PolygonIERC20Upgradeable(relayerRefundLeaf.l2TokenAddress),
+            relayerRefundLeaf.amountToReturn
+        );
 
         emit PolygonTokensBridged(relayerRefundLeaf.l2TokenAddress, address(this), relayerRefundLeaf.amountToReturn);
     }