test(deploy): Deploy a distinct ProxyAdmin for Superchain contracts (#12130)

* test(deploy): Deploy a distinct ProxyAdmin for Superchain contracts

* fix: only set address manager for OP Chain's ProxyAdmin

* fix: typo

* test: fix create2 collision between ProxyAdmins

* fix: lint

* fix: move setupOpChainAdmin() before setupOpAltDA

The DA challenge contract needs a proxy admin too

* feat: use create2AndSave for ProxyAdmin

* fix: Do not double save the superchain ProxyAdmin

* Fix whitespace
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

---------
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

@@ -191,8 +191,9 @@ contract Deploy is Deployer {
     ////////////////////////////////////////////////////////////////
 
     /// @notice Transfer ownership of the ProxyAdmin contract to the final system owner
-    function transferProxyAdminOwnership() public broadcast {
-        IProxyAdmin proxyAdmin = IProxyAdmin(mustGetAddress("ProxyAdmin"));
+    function transferProxyAdminOwnership(bool _isSuperchain) public broadcast {
+        string memory proxyAdminName = _isSuperchain ? "SuperchainProxyAdmin" : "ProxyAdmin";
+        IProxyAdmin proxyAdmin = IProxyAdmin(mustGetAddress(proxyAdminName));
         address owner = proxyAdmin.owner();
 
         address finalSystemOwner = cfg.finalSystemOwner();
@@ -202,17 +203,6 @@ contract Deploy is Deployer {
         }
     }
 
-    /// @notice Transfer ownership of a Proxy to the ProxyAdmin contract
-    ///         This is expected to be used in conjusting with deployERC1967ProxyWithOwner after setup actions
-    ///         have been performed on the proxy.
-    /// @param _name The name of the proxy to transfer ownership of.
-    function transferProxyToProxyAdmin(string memory _name) public broadcast {
-        IProxy proxy = IProxy(mustGetAddress(_name));
-        address proxyAdmin = mustGetAddress("ProxyAdmin");
-        proxy.changeAdmin(proxyAdmin);
-        console.log("Proxy %s ownership transferred to ProxyAdmin at: %s", _name, proxyAdmin);
-    }
-
     ////////////////////////////////////////////////////////////////
     //                    SetUp and Run                           //
     ////////////////////////////////////////////////////////////////
@@ -276,16 +266,13 @@ contract Deploy is Deployer {
     function _run(bool _needsSuperchain) internal {
         console.log("start of L1 Deploy!");
 
-        // Deploy a new ProxyAdmin and AddressManager
-        // This proxy will be used on the SuperchainConfig and ProtocolVersions contracts, as well as the contracts
-        // in the OP Chain system.
-        setupAdmin();
-
         if (_needsSuperchain) {
+            deployProxyAdmin({ _isSuperchain: true });
             setupSuperchain();
             console.log("set up superchain!");
         }
 
+        setupOpChainAdmin();
         if (cfg.useAltDA()) {
             bytes32 typeHash = keccak256(bytes(cfg.daCommitmentType()));
             bytes32 keccakHash = keccak256(bytes("KeccakCommitment"));
@@ -293,6 +280,7 @@ contract Deploy is Deployer {
                 setupOpAltDA();
             }
         }
+
         setupOpChain();
         console.log("set up op chain!");
     }
@@ -302,9 +290,9 @@ contract Deploy is Deployer {
     ////////////////////////////////////////////////////////////////
 
     /// @notice Deploy the address manager and proxy admin contracts.
-    function setupAdmin() public {
+    function setupOpChainAdmin() public {
         deployAddressManager();
-        deployProxyAdmin();
+        deployProxyAdmin({ _isSuperchain: false });
     }
 
     /// @notice Deploy a full system with a new SuperchainConfig
@@ -315,12 +303,12 @@ contract Deploy is Deployer {
         console.log("Setting up Superchain");
 
         // Deploy the SuperchainConfigProxy
-        deployERC1967Proxy("SuperchainConfigProxy");
+        deployERC1967ProxyWithOwner("SuperchainConfigProxy", mustGetAddress("SuperchainProxyAdmin"));
         deploySuperchainConfig();
         initializeSuperchainConfig();
 
         // Deploy the ProtocolVersionsProxy
-        deployERC1967Proxy("ProtocolVersionsProxy");
+        deployERC1967ProxyWithOwner("ProtocolVersionsProxy", mustGetAddress("SuperchainProxyAdmin"));
         deployProtocolVersions();
         initializeProtocolVersions();
     }
@@ -346,7 +334,7 @@ contract Deploy is Deployer {
 
         transferDisputeGameFactoryOwnership();
         transferDelayedWETHOwnership();
-        transferProxyAdminOwnership();
+        transferProxyAdminOwnership({ _isSuperchain: false });
     }
 
     /// @notice Deploy all of the OP Chain specific contracts
@@ -441,23 +429,33 @@ contract Deploy is Deployer {
     }
 
     /// @notice Deploy the ProxyAdmin
-    function deployProxyAdmin() public broadcast returns (address addr_) {
+    function deployProxyAdmin(bool _isSuperchain) public broadcast returns (address addr_) {
+        string memory proxyAdminName = _isSuperchain ? "SuperchainProxyAdmin" : "ProxyAdmin";
+
+        console.log("Deploying %s", proxyAdminName);
+
+        // Include the proxyAdminName in the salt to prevent a create2 collision when both the Superchain and an OP
+        // Chain are being setup.
         IProxyAdmin admin = IProxyAdmin(
             DeployUtils.create2AndSave({
                 _save: this,
-                _salt: _implSalt(),
+                _salt: keccak256(abi.encode(_implSalt(), proxyAdminName)),
                 _name: "ProxyAdmin",
+                _nick: proxyAdminName,
                 _args: DeployUtils.encodeConstructor(abi.encodeCall(IProxyAdmin.__constructor__, (msg.sender)))
             })
         );
         require(admin.owner() == msg.sender);
 
-        IAddressManager addressManager = IAddressManager(mustGetAddress("AddressManager"));
-        if (admin.addressManager() != addressManager) {
-            admin.setAddressManager(addressManager);
+        // The AddressManager is only required for OP Chains
+        if (!_isSuperchain) {
+            IAddressManager addressManager = IAddressManager(mustGetAddress("AddressManager"));
+            if (admin.addressManager() != addressManager) {
+                admin.setAddressManager(addressManager);
+            }
+            require(admin.addressManager() == addressManager);
         }
-
-        require(admin.addressManager() == addressManager);
+        console.log("%s deployed at %s", proxyAdminName, address(admin));
         addr_ = address(admin);
     }
 
@@ -933,7 +931,7 @@ contract Deploy is Deployer {
         address payable superchainConfigProxy = mustGetAddress("SuperchainConfigProxy");
         address payable superchainConfig = mustGetAddress("SuperchainConfig");
 
-        IProxyAdmin proxyAdmin = IProxyAdmin(payable(mustGetAddress("ProxyAdmin")));
+        IProxyAdmin proxyAdmin = IProxyAdmin(payable(mustGetAddress("SuperchainProxyAdmin")));
         proxyAdmin.upgradeAndCall({
             _proxy: superchainConfigProxy,
             _implementation: superchainConfig,
@@ -1345,7 +1343,7 @@ contract Deploy is Deployer {
         uint256 requiredProtocolVersion = cfg.requiredProtocolVersion();
         uint256 recommendedProtocolVersion = cfg.recommendedProtocolVersion();
 
-        IProxyAdmin proxyAdmin = IProxyAdmin(payable(mustGetAddress("ProxyAdmin")));
+        IProxyAdmin proxyAdmin = IProxyAdmin(payable(mustGetAddress("SuperchainProxyAdmin")));
         proxyAdmin.upgradeAndCall({
             _proxy: payable(protocolVersionsProxy),
             _implementation: protocolVersions,

packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol

@@ -19,8 +19,8 @@ import { DeputyGuardianModule } from "src/safe/DeputyGuardianModule.sol";
 import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol";
 
 import { Deploy } from "./Deploy.s.sol";
-/// @notice Configuration for a Safe
 
+/// @notice Configuration for a Safe
 struct SafeConfig {
     uint256 threshold;
     address[] owners;