Skip to content

Commit

Permalink
feat: adding proxy infront of OPStackManager.
Browse files Browse the repository at this point in the history
  • Loading branch information
@blmalone
blmalone committed Sep 13, 2024
1 parent 2283e70 commit d0c30be
Show file tree
Hide file tree
Showing 12 changed files with 192 additions and 96 deletions.
90 changes: 58 additions & 32 deletions packages/contracts-bedrock/scripts/DeployImplementations.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity 0.8.15;

import { Script } from "forge-std/Script.sol";
import "forge-std/console.sol";

import { LibString } from "@solady/utils/LibString.sol";

Expand Down Expand Up @@ -51,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");
Expand Down Expand Up @@ -81,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");
}

Expand Down Expand Up @@ -131,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;
Expand Down Expand Up @@ -170,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()),
Expand All @@ -187,9 +193,12 @@ contract DeployImplementationsOutput {
DeployUtils.assertValidContractAddresses(addrs);
}

function opsm() public view returns (OPStackManager) {
function opsm() public returns (OPStackManager) {
DeployUtils.assertValidContractAddress(address(_opsm));
DeployUtils.assertEIP1967ImplementationSet(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;
}

Expand Down Expand Up @@ -294,32 +303,38 @@ 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 opsmProxy_)
{
SuperchainConfig superchainConfigProxy = _dii.superchainConfigProxy();
ProtocolVersions protocolVersionsProxy = _dii.protocolVersionsProxy();
ProxyAdmin proxyAdmin = _dii.superchainProxyAdmin();

// In this case, DeployImplementations script needs to deploy a proxy contract and the opsm contract.
// To deploy a proxy contract, you need to know the proxy admin.
// However, this proxy admin is not deployed until the OPStackManager.
// Not using blueprint because we don't have access to l2ChainId.
vm.broadcast(msg.sender);
Proxy proxy = new Proxy(msg.sender); // Setting proxy admin to msg.sender because ProxyAdmin is not deployed
// yet.
OPStackManager opsm = new OPStackManager({
_superchainConfig: superchainConfigProxy,
_protocolVersions: protocolVersionsProxy,
_blueprints: blueprints
});
proxy.upgradeTo(address(opsm));
// 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));
}

Expand All @@ -340,9 +355,6 @@ contract DeployImplementations is Script {
vm.stopBroadcast();
// forgefmt: disable-end

// This call contains a broadcast to deploy OPSM which is proxied.
OPStackManager opsm = createOPSMContract(_dii, _dio, blueprints);

OPStackManager.ImplementationSetter[] memory setters = new OPStackManager.ImplementationSetter[](6);
setters[0] = OPStackManager.ImplementationSetter({
name: "L1ERC721Bridge",
Expand Down Expand Up @@ -370,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));
Expand Down Expand Up @@ -582,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(
Expand Down
1 change: 1 addition & 0 deletions packages/contracts-bedrock/scripts/DeployOPChain.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 3 additions & 8 deletions packages/contracts-bedrock/scripts/libraries/DeployUtils.sol
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

import { Proxy } from "src/universal/Proxy.sol";
import { LibString } from "@solady/utils/LibString.sol";
import "forge-std/console.sol";

library DeployUtils {
// This takes a sender and an identifier and returns a deterministic address based on the two.
Expand All @@ -19,13 +19,8 @@ library DeployUtils {
require(_who.code.length > 0, string.concat("DeployUtils: no code at ", LibString.toHexStringChecksummed(_who)));
}

function assertEIP1967ImplementationSet(address _proxy) internal view {
(bool success, bytes memory result) = _proxy.staticcall(abi.encodeWithSignature("implementation()"));
console.logBool(success);
require(success, "DeployUtils: EIP1967 implementation check failed");
console.log("success is true");
console.logBytes(result);
address implementation = abi.decode(result, (address));
function assertEIP1967Implementation(address _proxy) internal {
address implementation = Proxy(payable(_proxy)).implementation();
assertValidContractAddress(implementation);
}

Expand Down
81 changes: 53 additions & 28 deletions packages/contracts-bedrock/src/L1/OPStackManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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`.
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions packages/contracts-bedrock/src/L1/OPStackManagerInterop.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 1 addition & 8 deletions packages/contracts-bedrock/src/universal/Proxy.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity 0.8.15;

import { Constants } from "src/libraries/Constants.sol";
import "forge-std/console.sol";

/// @title Proxy
/// @notice Proxy is a transparent proxy that passes through the call if the caller is the owner or
Expand Down Expand Up @@ -96,10 +95,7 @@ contract Proxy {
//// @notice Queries the implementation address.
/// @return Implementation address.
function implementation() public virtual proxyCallIfNotAdmin returns (address) {
address impl = _getImplementation();
console.log("inside proxy.sol implementation()");
console.log(impl);
return impl;
return _getImplementation();
}

/// @notice Sets the implementation address.
Expand Down Expand Up @@ -153,12 +149,9 @@ contract Proxy {
function _getImplementation() internal view returns (address) {
address impl;
bytes32 proxyImplementation = Constants.PROXY_IMPLEMENTATION_ADDRESS;
console.logBytes32(proxyImplementation);
assembly {
impl := sload(proxyImplementation)
}
console.log("inside proxy.sol _getImplementation()");
console.log(impl);
return impl;
}

Expand Down
Loading

0 comments on commit d0c30be

Please sign in to comment.