Implement ownable directly into base contract

OffchainLabs · May 23, 2024 · 799d31b · 799d31b
1 parent 4b60ef6
commit 799d31b
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 7 deletions.
diff --git a/contracts/tokenbridge/ethereum/gateway/L1USDCCustomGateway.sol b/contracts/tokenbridge/ethereum/gateway/L1USDCCustomGateway.sol
@@ -4,14 +4,14 @@ pragma solidity ^0.8.4;
 
 import "./L1ArbitrumExtendedGateway.sol";
 import {L2USDCCustomGateway} from "../../arbitrum/gateway/L2USDCCustomGateway.sol";
-import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 
 /**
  * @title Custom gateway for USDC bridging.
  */
-contract L1USDCCustomGateway is L1ArbitrumExtendedGateway, OwnableUpgradeable {
+contract L1USDCCustomGateway is L1ArbitrumExtendedGateway {
     address public l1USDC;
     address public l2USDC;
+    address public owner;
     bool public depositsPaused;
 
     event DepositsPaused();
@@ -22,6 +22,15 @@ contract L1USDCCustomGateway is L1ArbitrumExtendedGateway, OwnableUpgradeable {
     error L1USDCCustomGateway_DepositsNotPaused();
     error L1USDCCustomGateway_InvalidL1USDC();
     error L1USDCCustomGateway_InvalidL2USDC();
+    error L1USDCCustomGateway_NotOwner();
+    error L1USDCCustomGateway_InvalidOwner();
+
+    modifier onlyOwner() {
+        if (msg.sender != owner) {
+            revert L1USDCCustomGateway_NotOwner();
+        }
+        _;
+    }
 
     function initialize(
         address _l2Counterpart,
@@ -30,18 +39,20 @@ contract L1USDCCustomGateway is L1ArbitrumExtendedGateway, OwnableUpgradeable {
         address _l1USDC,
         address _l2USDC,
         address _owner
-    ) public initializer {
+    ) public {
         if (_l1USDC == address(0)) {
             revert L1USDCCustomGateway_InvalidL1USDC();
         }
         if (_l2USDC == address(0)) {
             revert L1USDCCustomGateway_InvalidL2USDC();
         }
-        __Ownable_init();
+        if (_owner == address(0)) {
+            revert L1USDCCustomGateway_InvalidOwner();
+        }
         L1ArbitrumGateway._initialize(_l2Counterpart, _l1Router, _inbox);
         l1USDC = _l1USDC;
         l2USDC = _l2USDC;
-        transferOwnership(_owner);
+        owner = _owner;
     }
 
     function burnLockedUSDC() external onlyOwner {
@@ -83,6 +94,13 @@ contract L1USDCCustomGateway is L1ArbitrumExtendedGateway, OwnableUpgradeable {
         });
     }
 
+    function setOwner(address newOwner) external onlyOwner {
+        if (newOwner == address(0)) {
+            revert L1USDCCustomGateway_InvalidOwner();
+        }
+        owner = newOwner;
+    }
+
     function outboundTransferCustomRefund(
         address _l1Token,
         address _refundTo,

diff --git a/test-foundry/L1USDCCustomGateway.t.sol b/test-foundry/L1USDCCustomGateway.t.sol
@@ -55,7 +55,9 @@ contract L1USDCCustomGatewayTest is L1ArbitrumExtendedGatewayTest {
     }
 
     function test_burnLockedUSDC_revert_NotOwner() public {
-        vm.expectRevert("Ownable: caller is not the owner");
+        vm.expectRevert(
+            abi.encodeWithSelector(L1USDCCustomGateway.L1USDCCustomGateway_NotOwner.selector)
+        );
         usdcGateway.burnLockedUSDC();
     }
 
@@ -107,6 +109,14 @@ contract L1USDCCustomGatewayTest is L1ArbitrumExtendedGatewayTest {
         gateway.initialize(l2Gateway, router, inbox, L1_USDC, address(0), owner);
     }
 
+    function test_initialize_revert_InvalidOwner() public {
+        L1USDCCustomGateway gateway = new L1USDCCustomGateway();
+        vm.expectRevert(
+            abi.encodeWithSelector(L1USDCCustomGateway.L1USDCCustomGateway_InvalidOwner.selector)
+        );
+        gateway.initialize(l2Gateway, router, inbox, L1_USDC, L2_USDC, address(0));
+    }
+
     function test_finalizeInboundTransfer() public override {
         uint256 withdrawalAmount = 100_000_000;
         deal(L1_USDC, address(l1Gateway), withdrawalAmount);
@@ -322,7 +332,9 @@ contract L1USDCCustomGatewayTest is L1ArbitrumExtendedGatewayTest {
     }
 
     function test_pauseDeposits_revert_NotOwner() public {
-        vm.expectRevert("Ownable: caller is not the owner");
+        vm.expectRevert(
+            abi.encodeWithSelector(L1USDCCustomGateway.L1USDCCustomGateway_NotOwner.selector)
+        );
         usdcGateway.pauseDeposits{value: retryableCost}(
             maxGas, gasPriceBid, maxSubmissionCost, creditBackAddress
         );
@@ -345,6 +357,29 @@ contract L1USDCCustomGatewayTest is L1ArbitrumExtendedGatewayTest {
         );
     }
 
+    function test_setOwner() public {
+        address newOwner = makeAddr("new-owner");
+        vm.prank(owner);
+        usdcGateway.setOwner(newOwner);
+
+        assertEq(usdcGateway.owner(), newOwner, "Invalid owner");
+    }
+
+    function test_setOwner_revert_InvalidOwner() public {
+        vm.expectRevert(
+            abi.encodeWithSelector(L1USDCCustomGateway.L1USDCCustomGateway_InvalidOwner.selector)
+        );
+        vm.prank(owner);
+        usdcGateway.setOwner(address(0));
+    }
+
+    function test_setOwner_revert_NotOwner() public {
+        vm.expectRevert(
+            abi.encodeWithSelector(L1USDCCustomGateway.L1USDCCustomGateway_NotOwner.selector)
+        );
+        usdcGateway.setOwner(owner);
+    }
+
     ////
     // Event declarations
     ////