fix allow deployer != admin

issue: We NEED to update the DeployInit script as it is broken for when we use distinct Admin and deployer variables. Currently if we set admin to a different address than the deployer, then when the deployer goes to call `initialize` on the lxly contracts it will fail because it is not the admin. The Admin needs to be changed AFTER all of the contract setup has happened
BuildOnPolygon · Aug 29, 2023 · 2290f2c · 2290f2c
1 parent 705e7a1
commit 2290f2c
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 74 deletions.
diff --git a/.env.example b/.env.example
@@ -1,9 +1,9 @@
 # private key for the executor of the scripts (account must be funded)
-DEPLOYER_PRIVATE_KEY=0x2a871d0798f97d79848a013d4936a73bf4cc922c825d33c1cf7073dff6d409c6 # TODO: change this
+DEPLOYER_PRIVATE_KEY=0x2a871d0798f97d79848a013d4936a73bf4cc922c825d33c1cf7073dff6d409c6 # TODO: change this (0xa0Ee7A142d267C1f36714E4a8F75612F20a79720)
 # address of the account that will have admin rights over the proxies
-ADDRESS_PROXY_ADMIN=0xa0Ee7A142d267C1f36714E4a8F75612F20a79720 # TODO: change this
+ADDRESS_PROXY_ADMIN=0x14dC79964da2C08b23698B3D3cc7Ca32193d9955 # TODO: change this
 # address of the account that will be the owner of the contracts
-ADDRESS_OWNER=0xa0Ee7A142d267C1f36714E4a8F75612F20a79720 # TODO: change this
+ADDRESS_OWNER=0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f # TODO: change this
 
 # bridge and token address
 ADDRESS_LXLY_BRIDGE=0x2a3DD3EB832aF982ec71669E178424b10Dca2EDe # NOTE: same address for both L1/L2 when eth/zkevm

diff --git a/scripts/DeployInit.s.sol b/scripts/DeployInit.s.sol
@@ -34,7 +34,7 @@ contract DeployInit is Script {
         // deploy L1 contract
         vm.selectFork(l1ForkId);
         vm.startBroadcast(vm.envUint("DEPLOYER_PRIVATE_KEY"));
-        address l1EscrowProxy = LibDeployInit.deployL1Contracts(admin);
+        address l1EscrowProxy = LibDeployInit.deployL1Contracts();
         vm.stopBroadcast();
 
         // deploy L2 contracts
@@ -43,14 +43,15 @@ contract DeployInit is Script {
         (
             address minterBurnerProxy,
             address nativeConverterProxy
-        ) = LibDeployInit.deployL2Contracts(admin);
+        ) = LibDeployInit.deployL2Contracts();
         vm.stopBroadcast();
 
         // init L1 contract
         vm.selectFork(l1ForkId);
         vm.startBroadcast(vm.envUint("DEPLOYER_PRIVATE_KEY"));
         L1EscrowImpl l1Escrow = LibDeployInit.initL1Contracts(
             owner,
+            admin,
             l2NetworkId,
             bridge,
             l1EscrowProxy,
@@ -67,6 +68,7 @@ contract DeployInit is Script {
             NativeConverterImpl nativeConverter
         ) = LibDeployInit.initL2Contracts(
                 owner,
+                admin,
                 l1NetworkId,
                 bridge,
                 l1EscrowProxy,

diff --git a/scripts/DeployInitHelpers.sol b/scripts/DeployInitHelpers.sol
@@ -11,14 +11,11 @@ import "../src/ZkMinterBurnerProxy.sol";
 import "../src/ZkMinterBurnerImpl.sol";
 
 library LibDeployInit {
-    function deployL1Contracts(
-        address admin
-    ) internal returns (address l1eProxy) {
+    function deployL1Contracts() internal returns (address l1eProxy) {
         // deploy implementation
         L1EscrowImpl l1EscrowImpl = new L1EscrowImpl();
         // deploy proxy
         L1EscrowProxy l1EscrowProxy = new L1EscrowProxy(
-            admin,
             address(l1EscrowImpl),
             ""
         );
@@ -27,14 +24,14 @@ library LibDeployInit {
         l1eProxy = address(l1EscrowProxy);
     }
 
-    function deployL2Contracts(
-        address admin
-    ) internal returns (address mbProxy, address ncProxy) {
+    function deployL2Contracts()
+        internal
+        returns (address mbProxy, address ncProxy)
+    {
         // deploy implementation
         ZkMinterBurnerImpl minterBurnerImpl = new ZkMinterBurnerImpl();
         // deploy proxy
         ZkMinterBurnerProxy minterBurnerProxy = new ZkMinterBurnerProxy(
-            admin,
             address(minterBurnerImpl),
             ""
         );
@@ -43,7 +40,6 @@ library LibDeployInit {
         NativeConverterImpl nativeConverterImpl = new NativeConverterImpl();
         // deploy proxy
         NativeConverterProxy nativeConverterProxy = new NativeConverterProxy(
-            admin,
             address(nativeConverterImpl),
             ""
         );
@@ -55,6 +51,7 @@ library LibDeployInit {
 
     function initL1Contracts(
         address owner,
+        address admin,
         uint32 l2NetworkId,
         address bridge,
         address l1EscrowProxy,
@@ -65,6 +62,7 @@ library LibDeployInit {
         l1Escrow = L1EscrowImpl(l1EscrowProxy);
         l1Escrow.initialize(
             owner,
+            admin,
             bridge,
             l2NetworkId,
             minterBurnerProxy,
@@ -74,6 +72,7 @@ library LibDeployInit {
 
     function initL2Contracts(
         address owner,
+        address admin,
         uint32 l1NetworkId,
         address bridge,
         address l1EscrowProxy,
@@ -92,6 +91,7 @@ library LibDeployInit {
         minterBurner = ZkMinterBurnerImpl(minterBurnerProxy);
         minterBurner.initialize(
             owner,
+            admin,
             bridge,
             l1NetworkId,
             l1EscrowProxy,
@@ -102,6 +102,7 @@ library LibDeployInit {
         nativeConverter = NativeConverterImpl(nativeConverterProxy);
         nativeConverter.initialize(
             owner,
+            admin,
             bridge,
             l1NetworkId,
             l1EscrowProxy,

diff --git a/src/L1EscrowImpl.sol b/src/L1EscrowImpl.sol
@@ -30,19 +30,22 @@ contract L1EscrowImpl is IBridgeMessageReceiver, CommonAdminOwner {
 
     function initialize(
         address owner_,
+        address admin_,
         address bridge_,
         uint32 zkNetworkId_,
         address zkMinterBurnerProxy_,
         address l1Usdc_
     ) external onlyProxy onlyAdmin initializer {
-        require(bridge_ != address(0), "INVALID_ADDRESS");
-        require(zkMinterBurnerProxy_ != address(0), "INVALID_ADDRESS");
-        require(l1Usdc_ != address(0), "INVALID_ADDRESS");
-        require(owner_ != address(0), "INVALID_ADDRESS");
+        require(bridge_ != address(0), "INVALID_BRIDGE");
+        require(zkMinterBurnerProxy_ != address(0), "INVALID_MB");
+        require(l1Usdc_ != address(0), "INVALID_L1_USDC");
+        require(owner_ != address(0), "INVALID_OWNER");
+        require(admin_ != address(0), "INVALID_ADMIN");
 
         __CommonAdminOwner_init();
 
         _transferOwnership(owner_);
+        _changeAdmin(admin_);
 
         bridge = IPolygonZkEVMBridge(bridge_);
         zkNetworkId = zkNetworkId_;

diff --git a/src/L1EscrowProxy.sol b/src/L1EscrowProxy.sol
@@ -5,10 +5,10 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
 contract L1EscrowProxy is ERC1967Proxy {
     constructor(
-        address admin_,
         address impl_,
         bytes memory data_
     ) payable ERC1967Proxy(impl_, data_) {
-        _changeAdmin(admin_);
+        // set admin=deployer, this is changed on the subsequent call to init
+        _changeAdmin(msg.sender);
     }
 }
diff --git a/src/NativeConverterImpl.sol b/src/NativeConverterImpl.sol
@@ -37,25 +37,28 @@ contract NativeConverterImpl is CommonAdminOwner {
 
     function initialize(
         address owner_,
+        address admin_,
         address bridge_,
         uint32 l1NetworkId_,
-        address l1Escrow_,
+        address l1EscrowProxy_,
         address zkUSDCe_,
         address zkBWUSDC_
     ) external onlyProxy onlyAdmin initializer {
-        require(bridge_ != address(0), "INVALID_ADDRESS");
-        require(l1Escrow_ != address(0), "INVALID_ADDRESS");
-        require(zkUSDCe_ != address(0), "INVALID_ADDRESS");
-        require(zkBWUSDC_ != address(0), "INVALID_ADDRESS");
-        require(owner_ != address(0), "INVALID_ADDRESS");
+        require(bridge_ != address(0), "INVALID_BRIDGE");
+        require(l1EscrowProxy_ != address(0), "INVALID_L1ESCROW");
+        require(zkUSDCe_ != address(0), "INVALID_USDC_E");
+        require(zkBWUSDC_ != address(0), "INVALID_BW_UDSC");
+        require(owner_ != address(0), "INVALID_OWNER");
+        require(admin_ != address(0), "INVALID_ADMIN");
 
         __CommonAdminOwner_init();
 
         _transferOwnership(owner_);
+        _changeAdmin(admin_);
 
         bridge = IPolygonZkEVMBridge(bridge_);
         l1NetworkId = l1NetworkId_;
-        l1Escrow = l1Escrow_;
+        l1Escrow = l1EscrowProxy_;
         zkUSDCe = IUSDC(zkUSDCe_);
         zkBWUSDC = IUSDC(zkBWUSDC_);
     }

diff --git a/src/NativeConverterProxy.sol b/src/NativeConverterProxy.sol
@@ -5,10 +5,10 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
 contract NativeConverterProxy is ERC1967Proxy {
     constructor(
-        address admin_,
         address impl,
         bytes memory data
     ) payable ERC1967Proxy(impl, data) {
-        _changeAdmin(admin_);
+        // set admin=deployer, this is changed on the subsequent call to init
+        _changeAdmin(msg.sender);
     }
 }
diff --git a/src/ZkMinterBurnerImpl.sol b/src/ZkMinterBurnerImpl.sol
@@ -36,19 +36,22 @@ contract ZkMinterBurnerImpl is IBridgeMessageReceiver, CommonAdminOwner {
 
     function initialize(
         address owner_,
+        address admin_,
         address bridge_,
         uint32 l1NetworkId_,
         address l1EscrowProxy_,
         address zkUSDCe_
     ) external onlyProxy onlyAdmin initializer {
-        require(bridge_ != address(0), "INVALID_ADDRESS");
-        require(l1EscrowProxy_ != address(0), "INVALID_ADDRESS");
-        require(zkUSDCe_ != address(0), "INVALID_ADDRESS");
-        require(owner_ != address(0), "INVALID_ADDRESS");
+        require(bridge_ != address(0), "INVALID_BRIDGE");
+        require(l1EscrowProxy_ != address(0), "INVALID_L1ESCROW");
+        require(zkUSDCe_ != address(0), "INVALID_USDC_E");
+        require(owner_ != address(0), "INVALID_OWNER");
+        require(admin_ != address(0), "INVALID_ADMIN");
 
         __CommonAdminOwner_init();
 
         _transferOwnership(owner_);
+        _changeAdmin(admin_);
 
         bridge = IPolygonZkEVMBridge(bridge_);
         l1NetworkId = l1NetworkId_;

diff --git a/src/ZkMinterBurnerProxy.sol b/src/ZkMinterBurnerProxy.sol
@@ -5,10 +5,10 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
 contract ZkMinterBurnerProxy is ERC1967Proxy {
     constructor(
-        address admin_,
         address impl,
         bytes memory data
     ) payable ERC1967Proxy(impl, data) {
-        _changeAdmin(admin_);
+        // set admin=deployer, this is changed on the subsequent call to init
+        _changeAdmin(msg.sender);
     }
 }
diff --git a/test/Base.sol b/test/Base.sol
@@ -58,10 +58,10 @@ contract Base is Test {
     address internal _dan;
     address internal _erin;
     address internal _frank;
-    address internal _grace;
-    address internal _henry;
 
-    address internal _deployerOwnerAdmin;
+    address internal _deployer;
+    address internal _owner;
+    address internal _admin;
     address internal _bridge;
     address internal _l1Usdc;
     address internal _l2Usdc;
@@ -96,15 +96,16 @@ contract Base is Test {
         _erc20L2Usdc = IERC20(_l2Usdc);
         _erc20L2Wusdc = IERC20(_l2Wusdc);
 
-        _deployerOwnerAdmin = vm.addr(8);
+        _deployer = vm.addr(9);
+        _owner = vm.addr(8);
+        _admin = vm.addr(7);
         _alice = vm.addr(1);
         _bob = vm.addr(2);
         _carol = vm.addr(3);
         _dan = vm.addr(4);
         _erin = vm.addr(5);
         _frank = vm.addr(6);
-        _grace = vm.addr(7);
-        _actors = [_alice, _bob, _carol, _dan, _erin, _frank, _grace];
+        _actors = [_alice, _bob, _carol, _dan, _erin, _frank];
 
         // deploy and initialize contracts
         _deployMockBridge();
@@ -255,27 +256,24 @@ contract Base is Test {
     }
 
     function _deployInitContracts() internal {
-        vm.startPrank(_deployerOwnerAdmin);
+        vm.startPrank(_deployer);
 
         // deploy L1 contract
         vm.selectFork(_l1Fork);
-        address l1EscrowProxy = LibDeployInit.deployL1Contracts(
-            _deployerOwnerAdmin // admin
-        );
+        address l1EscrowProxy = LibDeployInit.deployL1Contracts();
 
         // deploy L2 contracts
         vm.selectFork(_l2Fork);
         (
             address minterBurnerProxy,
             address nativeConverterProxy
-        ) = LibDeployInit.deployL2Contracts(
-                _deployerOwnerAdmin // admin
-            );
+        ) = LibDeployInit.deployL2Contracts();
 
         // init L1 contract
         vm.selectFork(_l1Fork);
         _l1Escrow = LibDeployInit.initL1Contracts(
-            _deployerOwnerAdmin,
+            _owner,
+            _admin,
             _l2NetworkId,
             _bridge,
             l1EscrowProxy,
@@ -286,7 +284,8 @@ contract Base is Test {
         // init L2 contracts
         vm.selectFork(_l2Fork);
         (_minterBurner, _nativeConverter) = LibDeployInit.initL2Contracts(
-            _deployerOwnerAdmin,
+            _owner,
+            _admin,
             _l1NetworkId,
             _bridge,
             l1EscrowProxy,