feat: Front OPSM with Proxy and Initialize

packages/contracts-bedrock/scripts/DeployImplementations.s.sol

@@ -50,6 +50,7 @@ contract DeployImplementationsInput {
     // Outputs from DeploySuperchain.s.sol.
     SuperchainConfig internal _superchainConfigProxy;
     ProtocolVersions internal _protocolVersionsProxy;
+    ProxyAdmin internal _superchainProxyAdmin;
 
     function set(bytes4 sel, uint256 _value) public {
         require(_value != 0, "DeployImplementationsInput: cannot set zero value");
@@ -80,6 +81,7 @@ contract DeployImplementationsInput {
         require(_addr != address(0), "DeployImplementationsInput: cannot set zero address");
         if (sel == this.superchainConfigProxy.selector) _superchainConfigProxy = SuperchainConfig(_addr);
         else if (sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = ProtocolVersions(_addr);
+        else if (sel == this.superchainProxyAdmin.selector) _superchainProxyAdmin = ProxyAdmin(_addr);
         else revert("DeployImplementationsInput: unknown selector");
     }
 
@@ -130,9 +132,14 @@ contract DeployImplementationsInput {
         require(address(_protocolVersionsProxy) != address(0), "DeployImplementationsInput: not set");
         return _protocolVersionsProxy;
     }
+
+    function superchainProxyAdmin() public view returns (ProxyAdmin) {
+        require(address(_superchainProxyAdmin) != address(0), "DeployImplementationsInput: not set");
+        return _superchainProxyAdmin;
+    }
 }
 
-contract DeployImplementationsOutput {
+contract DeployImplementationsOutput is Script {
     OPStackManager internal _opsm;
     DelayedWETH internal _delayedWETHImpl;
     OptimismPortal2 internal _optimismPortalImpl;
@@ -169,7 +176,7 @@ contract DeployImplementationsOutput {
         require(false, "DeployImplementationsOutput: not implemented");
     }
 
-    function checkOutput() public view {
+    function checkOutput() public {
         address[] memory addrs = Solarray.addresses(
             address(this.opsm()),
             address(this.optimismPortalImpl()),
@@ -186,8 +193,12 @@ contract DeployImplementationsOutput {
         DeployUtils.assertValidContractAddresses(addrs);
     }
 
-    function opsm() public view returns (OPStackManager) {
+    function opsm() public returns (OPStackManager) {
         DeployUtils.assertValidContractAddress(address(_opsm));
+        // We prank as the zero address due to the Proxy's `proxyCallIfNotAdmin` modifier.
+        // Pranking inside this function also means it can no longer be considered `view`.
+        vm.prank(address(0));
+        DeployUtils.assertEIP1967Implementation(address(_opsm));
         return _opsm;
     }
 
@@ -292,24 +303,39 @@ contract DeployImplementations is Script {
         });
     }
 
+    // Deploy and initialize the OPStackManager contract that is fronted by a proxy.
     function createOPSMContract(
         DeployImplementationsInput _dii,
         DeployImplementationsOutput,
-        OPStackManager.Blueprints memory blueprints
+        OPStackManager.Blueprints memory blueprints,
+        string memory release,
+        OPStackManager.ImplementationSetter[] memory setters
     )
         internal
         virtual
-        returns (OPStackManager opsm_)
+        returns (OPStackManager opsmProxy_)
     {
         SuperchainConfig superchainConfigProxy = _dii.superchainConfigProxy();
         ProtocolVersions protocolVersionsProxy = _dii.protocolVersionsProxy();
+        ProxyAdmin proxyAdmin = _dii.superchainProxyAdmin();
 
-        vm.broadcast(msg.sender);
-        opsm_ = new OPStackManager({
-            _superchainConfig: superchainConfigProxy,
-            _protocolVersions: protocolVersionsProxy,
-            _blueprints: blueprints
-        });
+        // We broadcast as the proxyAdminOwner address due to the ProxyAdmin's `onlyOwner` modifier.
+        vm.startBroadcast(msg.sender);
+        Proxy proxy = new Proxy(address(msg.sender));
+        OPStackManager opsm = new OPStackManager(superchainConfigProxy, protocolVersionsProxy);
+
+        // TODO: Design decision, do we want to proceed with using initialize function?
+        // OPStackManager.InitializerInputs memory initializerInputs =
+        //     OPStackManager.InitializerInputs(blueprints, setters, release, true);
+        // proxy.upgradeToAndCall(address(opsm), abi.encodeWithSelector(opsm.initialize.selector, initializerInputs));
+
+        OPStackManager.SetRelease memory setRelease = OPStackManager.SetRelease(blueprints, setters, release, true);
+        proxy.upgradeToAndCall(address(opsm), abi.encodeWithSelector(opsm.setRelease.selector, setRelease));
+
+        proxy.changeAdmin(address(proxyAdmin)); // transfer ownership of Proxy contract to the ProxyAdmin contract
+        vm.stopBroadcast();
+
+        opsmProxy_ = OPStackManager(address(proxy));
     }
 
     function deployOPStackManager(DeployImplementationsInput _dii, DeployImplementationsOutput _dio) public virtual {
@@ -329,9 +355,6 @@ contract DeployImplementations is Script {
         vm.stopBroadcast();
         // forgefmt: disable-end
 
-        // This call contains a broadcast to deploy OPSM.
-        OPStackManager opsm = createOPSMContract(_dii, _dio, blueprints);
-
         OPStackManager.ImplementationSetter[] memory setters = new OPStackManager.ImplementationSetter[](6);
         setters[0] = OPStackManager.ImplementationSetter({
             name: "L1ERC721Bridge",
@@ -359,8 +382,8 @@ contract DeployImplementations is Script {
             info: OPStackManager.Implementation(address(_dio.l1StandardBridgeImpl()), L1StandardBridge.initialize.selector)
         });
 
-        vm.broadcast(msg.sender);
-        opsm.setRelease({ _release: release, _isLatest: true, _setters: setters });
+        // This call contains a broadcast to deploy OPSM which is proxied.
+        OPStackManager opsm = createOPSMContract(_dii, _dio, blueprints, release, setters);
 
         vm.label(address(opsm), "OPStackManager");
         _dio.set(_dio.opsm.selector, address(opsm));
@@ -571,21 +594,35 @@ contract DeployImplementationsInterop is DeployImplementations {
     function createOPSMContract(
         DeployImplementationsInput _dii,
         DeployImplementationsOutput,
-        OPStackManager.Blueprints memory blueprints
+        OPStackManager.Blueprints memory blueprints,
+        string memory release,
+        OPStackManager.ImplementationSetter[] memory setters
     )
         internal
+        virtual
         override
-        returns (OPStackManager opsm_)
+        returns (OPStackManager opsmProxy_)
     {
         SuperchainConfig superchainConfigProxy = _dii.superchainConfigProxy();
         ProtocolVersions protocolVersionsProxy = _dii.protocolVersionsProxy();
+        ProxyAdmin proxyAdmin = _dii.superchainProxyAdmin();
 
-        vm.broadcast(msg.sender);
-        opsm_ = new OPStackManagerInterop({
-            _superchainConfig: superchainConfigProxy,
-            _protocolVersions: protocolVersionsProxy,
-            _blueprints: blueprints
-        });
+        vm.startBroadcast(msg.sender);
+        Proxy proxy = new Proxy(address(msg.sender));
+        OPStackManager opsm = new OPStackManagerInterop(superchainConfigProxy, protocolVersionsProxy);
+
+        // TODO: Design decision, do we want to proceed with using initialize function?
+        // OPStackManager.InitializerInputs memory initializerInputs =
+        //     OPStackManager.InitializerInputs(blueprints, setters, release, true);
+        // proxy.upgradeToAndCall(address(opsm), abi.encodeWithSelector(opsm.initialize.selector, initializerInputs));
+
+        OPStackManager.SetRelease memory setRelease = OPStackManager.SetRelease(blueprints, setters, release, true);
+        proxy.upgradeToAndCall(address(opsm), abi.encodeWithSelector(opsm.setRelease.selector, setRelease));
+
+        proxy.changeAdmin(address(proxyAdmin)); // transfer ownership of Proxy contract to the ProxyAdmin contract
+        vm.stopBroadcast();
+
+        opsmProxy_ = OPStackManagerInterop(address(proxy));
     }
 
     function deployOptimismPortalImpl(

packages/contracts-bedrock/scripts/DeployOPChain.s.sol

@@ -115,6 +115,7 @@ contract DeployOPChainInput {
         return _l2ChainId;
     }
 
+    // TODO: Check that opsm is fronted by a proxy and it has an implementation.
     function opsm() public view returns (OPStackManager) {
         require(address(_opsm) != address(0), "DeployOPChainInput: not set");
         return _opsm;

packages/contracts-bedrock/scripts/libraries/DeployUtils.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+import { Proxy } from "src/universal/Proxy.sol";
 import { LibString } from "@solady/utils/LibString.sol";
 
 library DeployUtils {
@@ -18,6 +19,11 @@ library DeployUtils {
         require(_who.code.length > 0, string.concat("DeployUtils: no code at ", LibString.toHexStringChecksummed(_who)));
     }
 
+    function assertEIP1967Implementation(address _proxy) internal {
+        address implementation = Proxy(payable(_proxy)).implementation();
+        assertValidContractAddress(implementation);
+    }
+
     function assertValidContractAddresses(address[] memory _addrs) internal view {
         // Assert that all addresses are non-zero and have code.
         // We use LibString to avoid the need for adding cheatcodes to this contract.

packages/contracts-bedrock/src/L1/OPStackManager.sol

@@ -3,9 +3,13 @@ pragma solidity 0.8.15;
 
 import { Blueprint } from "src/libraries/Blueprint.sol";
 
+import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
+
 import { ISemver } from "src/universal/interfaces/ISemver.sol";
 import { Proxy } from "src/universal/Proxy.sol";
 import { ProxyAdmin } from "src/universal/ProxyAdmin.sol";
+import { SuperchainConfig } from "src/L1/SuperchainConfig.sol";
+import { ProtocolVersions } from "src/L1/ProtocolVersions.sol";
 
 import { L1ChugSplashProxy } from "src/legacy/L1ChugSplashProxy.sol";
 import { ResolvedDelegateProxy } from "src/legacy/ResolvedDelegateProxy.sol";
@@ -29,7 +33,7 @@ import { L1StandardBridge } from "src/L1/L1StandardBridge.sol";
 import { OptimismMintableERC20Factory } from "src/universal/OptimismMintableERC20Factory.sol";
 
 /// @custom:proxied true
-contract OPStackManager is ISemver {
+contract OPStackManager is ISemver, Initializable {
     // -------- Structs --------
 
     /// @notice Represents the roles that can be set when deploying a standard OP Stack chain.
@@ -97,6 +101,25 @@ contract OPStackManager is ISemver {
         address resolvedDelegateProxy;
     }
 
+    /// TODO: Design question, do we still want this setRelease functionality if we have initialize functionality?
+    /// @notice Inputs required when initializing the OPStackManager. To avoid 'StackTooDeep' errors,
+    /// all necessary inputs (excluding immutables) for initialization are bundled together in this struct.
+    struct InitializerInputs {
+        Blueprints blueprints;
+        ImplementationSetter[] setters;
+        string release;
+        bool isLatest;
+    }
+
+    /// @notice Inputs required when initializing the OPStackManager. To avoid 'StackTooDeep' errors,
+    /// all necessary inputs (excluding immutables) for initialization are bundled together in this struct.
+    struct SetRelease {
+        Blueprints blueprints;
+        ImplementationSetter[] setters;
+        string release;
+        bool isLatest;
+    }
+
     // -------- Constants and Variables --------
 
     /// @custom:semver 1.0.0-beta.3
@@ -110,7 +133,8 @@ contract OPStackManager is ISemver {
 
     /// @notice Addresses of the Blueprint contracts.
     /// This is internal because if public the autogenerated getter method would return a tuple of
-    /// addresses, but we want it to return a struct.
+    /// addresses, but we want it to return a struct. This is also set via `setRelease` because
+    /// we can't make this an immutable variable as it is a non-value type.
     Blueprints internal blueprint;
 
     /// @notice The latest release of the OP Stack Manager, as a string of the format `op-contracts/vX.Y.Z`.
@@ -151,44 +175,45 @@ contract OPStackManager is ISemver {
 
     // -------- Methods --------
 
-    /// @notice OPSM is intended to be proxied when used in production. Since we are initially
-    /// focused on an OPSM version that unblocks interop, we are not proxying OPSM for simplicity.
-    /// Later, we will `_disableInitializers` in the constructor and replace any constructor logic
-    /// with an `initialize` function, which will be a breaking change to the OPSM interface.
-    constructor(SuperchainConfig _superchainConfig, ProtocolVersions _protocolVersions, Blueprints memory _blueprints) {
-        // TODO uncomment these as we add more validations to this contract, currently this will break a few tests.
-        // assertValidContractAddress(address(_superchainConfig));
-        // assertValidContractAddress(address(_protocolVersions));
-        // assertValidContractAddress(_blueprints.addressManager);
-        // assertValidContractAddress(_blueprints.proxy);
-        // assertValidContractAddress(_blueprints.proxyAdmin);
-        // assertValidContractAddress(_blueprints.l1ChugSplashProxy);
-        // assertValidContractAddress(_blueprints.resolvedDelegateProxy);
-
+    /// @notice OPSM is proxied. Therefore the `initialize` function replaces any constructor logic for this contract.
+    constructor(SuperchainConfig _superchainConfig, ProtocolVersions _protocolVersions) {
+        assertValidContractAddress(address(_superchainConfig));
+        assertValidContractAddress(address(_protocolVersions));
         superchainConfig = _superchainConfig;
         protocolVersions = _protocolVersions;
-        blueprint = _blueprints;
+        _disableInitializers();
+    }
+
+    function initialize(InitializerInputs memory _initializerInputs) public initializer {
+        if (_initializerInputs.isLatest) latestRelease = _initializerInputs.release;
+
+        for (uint256 i = 0; i < _initializerInputs.setters.length; i++) {
+            ImplementationSetter memory setter = _initializerInputs.setters[i];
+            Implementation storage impl = implementations[_initializerInputs.release][setter.name];
+            if (impl.logic != address(0)) revert AlreadyReleased();
+
+            impl.initializer = setter.info.initializer;
+            impl.logic = setter.info.logic;
+        }
+
+        blueprint = _initializerInputs.blueprints;
     }
 
-    /// @notice Callable by the OPSM owner to release a set of implementation contracts for a given
-    /// release version. This must be called with `_isLatest` set to true before any chains can be deployed.
-    /// @param _release The release version to set implementations for, of the format `op-contracts/vX.Y.Z`.
-    /// @param _isLatest Whether the release version is the latest released version. This is
-    /// significant because the latest version is used to deploy chains in the `deploy` function.
-    /// @param _setters The set of implementations to set for the release version.
-    function setRelease(string memory _release, bool _isLatest, ImplementationSetter[] calldata _setters) external {
+    function setRelease(SetRelease memory _setRelease) external {
         // TODO Add auth to this method.
 
-        if (_isLatest) latestRelease = _release;
+        if (_setRelease.isLatest) latestRelease = _setRelease.release;
 
-        for (uint256 i = 0; i < _setters.length; i++) {
-            ImplementationSetter calldata setter = _setters[i];
-            Implementation storage impl = implementations[_release][setter.name];
+        for (uint256 i = 0; i < _setRelease.setters.length; i++) {
+            ImplementationSetter memory setter = _setRelease.setters[i];
+            Implementation storage impl = implementations[_setRelease.release][setter.name];
             if (impl.logic != address(0)) revert AlreadyReleased();
 
             impl.initializer = setter.info.initializer;
             impl.logic = setter.info.logic;
         }
+
+        blueprint = _setRelease.blueprints;
     }
 
     function deploy(DeployInput calldata _input) external returns (DeployOutput memory) {

packages/contracts-bedrock/src/L1/OPStackManagerInterop.sol

@@ -8,14 +8,13 @@ import { ResourceMetering } from "src/L1/ResourceMetering.sol";
 import { SystemConfig } from "src/L1/SystemConfig.sol";
 import { SystemConfigInterop } from "src/L1/SystemConfigInterop.sol";
 
-/// @custom:proxied TODO this is not proxied yet.
+/// @custom:proxied true
 contract OPStackManagerInterop is OPStackManager {
     constructor(
         SuperchainConfig _superchainConfig,
-        ProtocolVersions _protocolVersions,
-        Blueprints memory _blueprints
+        ProtocolVersions _protocolVersions
     )
-        OPStackManager(_superchainConfig, _protocolVersions, _blueprints)
+        OPStackManager(_superchainConfig, _protocolVersions)
     { }
 
     // The `SystemConfigInterop` contract has an extra `address _dependencyManager` argument