fix(ctb): solc warnings in ProxyAdmin.sol (#2952)

Addresses solc warnings in ProxyAdmin.sol. Also changes the input type
from Proxy to address payable because it is *not* guaranteed that the
input will actually be of Proxy type.

.changeset/fresh-pugs-remain.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Fix solc warnings in ProxyAdmin

packages/contracts-bedrock/contracts/test/ProxyAdmin.t.sol

@@ -103,18 +103,18 @@ contract ProxyAdmin_Test is Test {
     }
 
     function test_erc1967GetProxyImplementation() external {
-        getProxyImplementation(proxy);
+        getProxyImplementation(payable(proxy));
     }
 
     function test_chugsplashGetProxyImplementation() external {
-        getProxyImplementation(Proxy(payable(chugsplash)));
+        getProxyImplementation(payable(chugsplash));
     }
 
     function test_delegateResolvedGetProxyImplementation() external {
-        getProxyImplementation(Proxy(payable(resolved)));
+        getProxyImplementation(payable(resolved));
     }
 
-    function getProxyImplementation(Proxy _proxy) internal {
+    function getProxyImplementation(address payable _proxy) internal {
         {
             address impl = admin.getProxyImplementation(_proxy);
             assertEq(impl, address(0));
@@ -130,35 +130,35 @@ contract ProxyAdmin_Test is Test {
     }
 
     function test_erc1967GetProxyAdmin() external {
-        getProxyAdmin(proxy);
+        getProxyAdmin(payable(proxy));
     }
 
     function test_chugsplashGetProxyAdmin() external {
-        getProxyAdmin(Proxy(payable(chugsplash)));
+        getProxyAdmin(payable(chugsplash));
     }
 
     function test_delegateResolvedGetProxyAdmin() external {
-        getProxyAdmin(Proxy(payable(resolved)));
+        getProxyAdmin(payable(resolved));
     }
 
-    function getProxyAdmin(Proxy _proxy) internal {
+    function getProxyAdmin(address payable _proxy) internal {
         address owner = admin.getProxyAdmin(_proxy);
         assertEq(owner, address(admin));
     }
 
     function test_erc1967ChangeProxyAdmin() external {
-        changeProxyAdmin(proxy);
+        changeProxyAdmin(payable(proxy));
     }
 
     function test_chugsplashChangeProxyAdmin() external {
-        changeProxyAdmin(Proxy(payable(chugsplash)));
+        changeProxyAdmin(payable(chugsplash));
     }
 
     function test_delegateResolvedChangeProxyAdmin() external {
-        changeProxyAdmin(Proxy(payable(resolved)));
+        changeProxyAdmin(payable(resolved));
     }
 
-    function changeProxyAdmin(Proxy _proxy) internal {
+    function changeProxyAdmin(address payable _proxy) internal {
         ProxyAdmin.ProxyType proxyType = admin.proxyType(address(_proxy));
 
         vm.prank(alice);
@@ -184,7 +184,7 @@ contract ProxyAdmin_Test is Test {
         // Different proxy types have different interfaces.
         vm.prank(address(128));
         if (proxyType == ProxyAdmin.ProxyType.ERC1967) {
-            assertEq(_proxy.admin(), address(128));
+            assertEq(Proxy(payable(_proxy)).admin(), address(128));
         } else if (proxyType == ProxyAdmin.ProxyType.Chugsplash) {
             assertEq(
                 L1ChugSplashProxy(payable(_proxy)).getOwner(),
@@ -201,18 +201,18 @@ contract ProxyAdmin_Test is Test {
     }
 
     function test_erc1967Upgrade() external {
-        upgrade(proxy);
+        upgrade(payable(proxy));
     }
 
     function test_chugsplashUpgrade() external {
-        upgrade(Proxy(payable(chugsplash)));
+        upgrade(payable(chugsplash));
     }
 
     function test_delegateResolvedUpgrade() external {
-        upgrade(Proxy(payable(resolved)));
+        upgrade(payable(resolved));
     }
 
-    function upgrade(Proxy _proxy) internal {
+    function upgrade(address payable _proxy) internal {
         vm.prank(alice);
         admin.upgrade(_proxy, address(implementation));
 
@@ -221,18 +221,18 @@ contract ProxyAdmin_Test is Test {
     }
 
     function test_erc1967UpgradeAndCall() external {
-        upgradeAndCall(proxy);
+        upgradeAndCall(payable(proxy));
     }
 
     function test_chugsplashUpgradeAndCall() external {
-        upgradeAndCall(Proxy(payable(chugsplash)));
+        upgradeAndCall(payable(chugsplash));
     }
 
     function test_delegateResolvedUpgradeAndCall() external {
-        upgradeAndCall(Proxy(payable(resolved)));
+        upgradeAndCall(payable(resolved));
     }
 
-    function upgradeAndCall(Proxy _proxy) internal {
+    function upgradeAndCall(address payable _proxy) internal {
         vm.prank(alice);
         admin.upgradeAndCall(
             _proxy,
@@ -249,13 +249,13 @@ contract ProxyAdmin_Test is Test {
 
     function test_onlyOwner() external {
         vm.expectRevert("UNAUTHORIZED");
-        admin.changeProxyAdmin(proxy, address(0));
+        admin.changeProxyAdmin(payable(proxy), address(0));
 
         vm.expectRevert("UNAUTHORIZED");
-        admin.upgrade(proxy, address(implementation));
+        admin.upgrade(payable(proxy), address(implementation));
 
         vm.expectRevert("UNAUTHORIZED");
-        admin.upgradeAndCall(proxy, address(implementation), hex"");
+        admin.upgradeAndCall(payable(proxy), address(implementation), hex"");
     }
 
     function test_isUpgrading() external {

packages/contracts-bedrock/contracts/universal/ProxyAdmin.sol

@@ -152,15 +152,14 @@ contract ProxyAdmin is Owned {
      *
      * @return Address of the implementation of the proxy.
      */
-    function getProxyImplementation(Proxy _proxy) external view returns (address) {
-        ProxyType proxyType = proxyType[address(_proxy)];
-
-        if (proxyType == ProxyType.ERC1967) {
-            return IStaticERC1967Proxy(address(_proxy)).implementation();
-        } else if (proxyType == ProxyType.Chugsplash) {
-            return IStaticL1ChugSplashProxy(address(_proxy)).getImplementation();
-        } else if (proxyType == ProxyType.ResolvedDelegate) {
-            return addressManager.getAddress(implementationName[address(_proxy)]);
+    function getProxyImplementation(address _proxy) external view returns (address) {
+        ProxyType ptype = proxyType[_proxy];
+        if (ptype == ProxyType.ERC1967) {
+            return IStaticERC1967Proxy(_proxy).implementation();
+        } else if (ptype == ProxyType.Chugsplash) {
+            return IStaticL1ChugSplashProxy(_proxy).getImplementation();
+        } else if (ptype == ProxyType.ResolvedDelegate) {
+            return addressManager.getAddress(implementationName[_proxy]);
         } else {
             revert("ProxyAdmin: unknown proxy type");
         }
@@ -173,14 +172,13 @@ contract ProxyAdmin is Owned {
      *
      * @return Address of the admin of the proxy.
      */
-    function getProxyAdmin(Proxy _proxy) external view returns (address) {
-        ProxyType proxyType = proxyType[address(_proxy)];
-
-        if (proxyType == ProxyType.ERC1967) {
-            return IStaticERC1967Proxy(address(_proxy)).admin();
-        } else if (proxyType == ProxyType.Chugsplash) {
-            return IStaticL1ChugSplashProxy(address(_proxy)).getOwner();
-        } else if (proxyType == ProxyType.ResolvedDelegate) {
+    function getProxyAdmin(address payable _proxy) external view returns (address) {
+        ProxyType ptype = proxyType[_proxy];
+        if (ptype == ProxyType.ERC1967) {
+            return IStaticERC1967Proxy(_proxy).admin();
+        } else if (ptype == ProxyType.Chugsplash) {
+            return IStaticL1ChugSplashProxy(_proxy).getOwner();
+        } else if (ptype == ProxyType.ResolvedDelegate) {
             return addressManager.owner();
         } else {
             revert("ProxyAdmin: unknown proxy type");
@@ -193,14 +191,13 @@ contract ProxyAdmin is Owned {
      * @param _proxy    Address of the proxy to update.
      * @param _newAdmin Address of the new proxy admin.
      */
-    function changeProxyAdmin(Proxy _proxy, address _newAdmin) external onlyOwner {
-        ProxyType proxyType = proxyType[address(_proxy)];
-
-        if (proxyType == ProxyType.ERC1967) {
-            _proxy.changeAdmin(_newAdmin);
-        } else if (proxyType == ProxyType.Chugsplash) {
-            L1ChugSplashProxy(payable(_proxy)).setOwner(_newAdmin);
-        } else if (proxyType == ProxyType.ResolvedDelegate) {
+    function changeProxyAdmin(address payable _proxy, address _newAdmin) external onlyOwner {
+        ProxyType ptype = proxyType[_proxy];
+        if (ptype == ProxyType.ERC1967) {
+            Proxy(_proxy).changeAdmin(_newAdmin);
+        } else if (ptype == ProxyType.Chugsplash) {
+            L1ChugSplashProxy(_proxy).setOwner(_newAdmin);
+        } else if (ptype == ProxyType.ResolvedDelegate) {
             addressManager.transferOwnership(_newAdmin);
         } else {
             revert("ProxyAdmin: unknown proxy type");
@@ -213,18 +210,17 @@ contract ProxyAdmin is Owned {
      * @param _proxy          Address of the proxy to upgrade.
      * @param _implementation Address of the new implementation address.
      */
-    function upgrade(Proxy _proxy, address _implementation) public onlyOwner {
-        ProxyType proxyType = proxyType[address(_proxy)];
-
-        if (proxyType == ProxyType.ERC1967) {
-            _proxy.upgradeTo(_implementation);
-        } else if (proxyType == ProxyType.Chugsplash) {
-            L1ChugSplashProxy(payable(_proxy)).setStorage(
+    function upgrade(address payable _proxy, address _implementation) public onlyOwner {
+        ProxyType ptype = proxyType[_proxy];
+        if (ptype == ProxyType.ERC1967) {
+            Proxy(_proxy).upgradeTo(_implementation);
+        } else if (ptype == ProxyType.Chugsplash) {
+            L1ChugSplashProxy(_proxy).setStorage(
                 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc,
                 bytes32(uint256(uint160(_implementation)))
             );
-        } else if (proxyType == ProxyType.ResolvedDelegate) {
-            string memory name = implementationName[address(_proxy)];
+        } else if (ptype == ProxyType.ResolvedDelegate) {
+            string memory name = implementationName[_proxy];
             addressManager.setAddress(name, _implementation);
         } else {
             revert("ProxyAdmin: unknown proxy type");
@@ -240,19 +236,18 @@ contract ProxyAdmin is Owned {
      * @param _data           Data to trigger the new implementation with.
      */
     function upgradeAndCall(
-        Proxy _proxy,
+        address payable _proxy,
         address _implementation,
         bytes memory _data
     ) external payable onlyOwner {
-        ProxyType proxyType = proxyType[address(_proxy)];
-
-        if (proxyType == ProxyType.ERC1967) {
-            _proxy.upgradeToAndCall{ value: msg.value }(_implementation, _data);
+        ProxyType ptype = proxyType[_proxy];
+        if (ptype == ProxyType.ERC1967) {
+            Proxy(_proxy).upgradeToAndCall{ value: msg.value }(_implementation, _data);
         } else {
             // reverts if proxy type is unknown
             upgrade(_proxy, _implementation);
-            (bool success, ) = address(_proxy).call{ value: msg.value }(_data);
-            require(success);
+            (bool success, ) = _proxy.call{ value: msg.value }(_data);
+            require(success, "ProxyAdmin: call to proxy after upgrade failed");
         }
     }
 }