Skip to content

Commit

Permalink
test(deploy): Deploy a distinct ProxyAdmin for Superchain contracts (#…
Browse files Browse the repository at this point in the history 
…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>
  • Loading branch information
@maurelian @mds1
maurelian and mds1 authored Sep 30, 2024
1 parent b127499 commit b6e27b6
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 33 deletions.
62 changes: 30 additions & 32 deletions packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 //
////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -276,23 +266,21 @@ 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"));
if (typeHash == keccakHash) {
setupOpAltDA();
}
}

setupOpChain();
console.log("set up op chain!");
}
Expand All @@ -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
Expand All @@ -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();
}
Expand All @@ -346,7 +334,7 @@ contract Deploy is Deployer {

transferDisputeGameFactoryOwnership();
transferDelayedWETHOwnership();
transferProxyAdminOwnership();
transferProxyAdminOwnership({ _isSuperchain: false });
}

/// @notice Deploy all of the OP Chain specific contracts
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit b6e27b6

Please sign in to comment.