From 887e380ebf84357c2a660a190903d908e319055b Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Tue, 17 Sep 2024 12:59:55 -0700 Subject: [PATCH 1/8] refactor: rename var for clarity --- .../scripts/DeploySuperchain.s.sol | 22 +++++++++--------- .../test/DeployOPChain.t.sol | 4 ++-- .../test/DeploySuperchain.t.sol | 23 ++++++++++--------- .../fixtures/test-deploy-superchain-in.toml | 4 ++-- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol index 800833ccaa4f..5ff3abcff7e8 100644 --- a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol @@ -81,7 +81,7 @@ contract DeploySuperchainInput is BaseDeployIO { // Role inputs. address internal _guardian; address internal _protocolVersionsOwner; - address internal _proxyAdminOwner; + address internal _superchainProxyAdminOwner; // Other inputs. bool internal _paused; @@ -94,7 +94,7 @@ contract DeploySuperchainInput is BaseDeployIO { require(_address != address(0), "DeploySuperchainInput: cannot set zero address"); if (_sel == this.guardian.selector) _guardian = _address; else if (_sel == this.protocolVersionsOwner.selector) _protocolVersionsOwner = _address; - else if (_sel == this.proxyAdminOwner.selector) _proxyAdminOwner = _address; + else if (_sel == this.superchainProxyAdminOwner.selector) _superchainProxyAdminOwner = _address; else revert("DeploySuperchainInput: unknown selector"); } @@ -119,7 +119,7 @@ contract DeploySuperchainInput is BaseDeployIO { // Parse and set role inputs. set(this.guardian.selector, toml.readAddress(".roles.guardian")); set(this.protocolVersionsOwner.selector, toml.readAddress(".roles.protocolVersionsOwner")); - set(this.proxyAdminOwner.selector, toml.readAddress(".roles.proxyAdminOwner")); + set(this.superchainProxyAdminOwner.selector, toml.readAddress(".roles.superchainProxyAdminOwner")); // Parse and set other inputs. set(this.paused.selector, toml.readBool(".paused")); @@ -136,9 +136,9 @@ contract DeploySuperchainInput is BaseDeployIO { // validate that each input is set before accessing it. With getter methods, we can automatically // validate that each input is set before allowing any field to be accessed. - function proxyAdminOwner() public view returns (address) { - require(_proxyAdminOwner != address(0), "DeploySuperchainInput: proxyAdminOwner not set"); - return _proxyAdminOwner; + function superchainProxyAdminOwner() public view returns (address) { + require(_superchainProxyAdminOwner != address(0), "DeploySuperchainInput: superchainProxyAdminOwner not set"); + return _superchainProxyAdminOwner; } function protocolVersionsOwner() public view returns (address) { @@ -210,7 +210,7 @@ contract DeploySuperchainOutput is BaseDeployIO { // This function can be called to ensure all outputs are correct. Similar to `writeOutputFile`, // it fetches the output values using external calls to the getter methods for safety. - function checkOutput(DeploySuperchainInput) public { + function checkOutput(DeploySuperchainInput _dsi) public { address[] memory addrs = Solarray.addresses( address(this.superchainProxyAdmin()), address(this.superchainConfigImpl()), @@ -302,7 +302,7 @@ contract DeploySuperchain is Script { deployAndInitializeProtocolVersions(_dsi, _dso); // Transfer ownership of the ProxyAdmin from the deployer to the specified owner. - transferProxyAdminOwnership(_dsi, _dso); + transferSuperchainProxyAdminOwnership(_dsi, _dso); // Output assertions, to make sure outputs were assigned correctly. _dso.checkOutput(_dsi); @@ -380,14 +380,14 @@ contract DeploySuperchain is Script { _dso.set(_dso.protocolVersionsProxy.selector, address(protocolVersionsProxy)); } - function transferProxyAdminOwnership(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { - address proxyAdminOwner = _dsi.proxyAdminOwner(); + function transferSuperchainProxyAdminOwnership(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { + address superchainProxyAdminOwner = _dsi.superchainProxyAdminOwner(); ProxyAdmin superchainProxyAdmin = _dso.superchainProxyAdmin(); DeployUtils.assertValidContractAddress(address(superchainProxyAdmin)); vm.broadcast(msg.sender); - superchainProxyAdmin.transferOwnership(proxyAdminOwner); + superchainProxyAdmin.transferOwnership(superchainProxyAdminOwner); } // -------- Utilities -------- diff --git a/packages/contracts-bedrock/test/DeployOPChain.t.sol b/packages/contracts-bedrock/test/DeployOPChain.t.sol index b0a4cb15507e..643e97c9edda 100644 --- a/packages/contracts-bedrock/test/DeployOPChain.t.sol +++ b/packages/contracts-bedrock/test/DeployOPChain.t.sol @@ -306,7 +306,7 @@ contract DeployOPChain_TestBase is Test { DeployOPChainOutput doo; // Define default inputs for DeploySuperchain. - address proxyAdminOwner = makeAddr("defaultProxyAdminOwner"); + address superchainProxyAdminOwner = makeAddr("defaultProxyAdminOwner"); address protocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner"); address guardian = makeAddr("defaultGuardian"); bool paused = false; @@ -342,7 +342,7 @@ contract DeployOPChain_TestBase is Test { // Initialize deploy scripts. DeploySuperchain deploySuperchain = new DeploySuperchain(); (DeploySuperchainInput dsi, DeploySuperchainOutput dso) = deploySuperchain.etchIOContracts(); - dsi.set(dsi.proxyAdminOwner.selector, proxyAdminOwner); + dsi.set(dsi.superchainProxyAdminOwner.selector, superchainProxyAdminOwner); dsi.set(dsi.protocolVersionsOwner.selector, protocolVersionsOwner); dsi.set(dsi.guardian.selector, guardian); dsi.set(dsi.paused.selector, paused); diff --git a/packages/contracts-bedrock/test/DeploySuperchain.t.sol b/packages/contracts-bedrock/test/DeploySuperchain.t.sol index eaf7fe3988a7..066f08b96e06 100644 --- a/packages/contracts-bedrock/test/DeploySuperchain.t.sol +++ b/packages/contracts-bedrock/test/DeploySuperchain.t.sol @@ -13,7 +13,7 @@ import { DeploySuperchainInput, DeploySuperchain, DeploySuperchainOutput } from contract DeploySuperchainInput_Test is Test { DeploySuperchainInput dsi; - address proxyAdminOwner = makeAddr("defaultProxyAdminOwner"); + address superchainProxyAdminOwner = makeAddr("defaultSuperchainProxyAdminOwner"); address protocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner"); address guardian = makeAddr("defaultGuardian"); bool paused = false; @@ -30,7 +30,7 @@ contract DeploySuperchainInput_Test is Test { dsi.loadInputFile(path); - assertEq(proxyAdminOwner, dsi.proxyAdminOwner(), "100"); + assertEq(superchainProxyAdminOwner, dsi.superchainProxyAdminOwner(), "100"); assertEq(protocolVersionsOwner, dsi.protocolVersionsOwner(), "200"); assertEq(guardian, dsi.guardian(), "300"); assertEq(paused, dsi.paused(), "400"); @@ -48,7 +48,7 @@ contract DeploySuperchainInput_Test is Test { function test_getters_whenNotSet_revert() public { vm.expectRevert("DeploySuperchainInput: proxyAdminOwner not set"); - dsi.proxyAdminOwner(); + dsi.superchainProxyAdminOwner(); vm.expectRevert("DeploySuperchainInput: protocolVersionsOwner not set"); dsi.protocolVersionsOwner(); @@ -186,7 +186,7 @@ contract DeploySuperchain_Test is Test { DeploySuperchainOutput dso; // Define default input variables for testing. - address defaultProxyAdminOwner = makeAddr("defaultProxyAdminOwner"); + address defaultSuperchainProxyAdminOwner = makeAddr("defaultSuperchainProxyAdminOwner"); address defaultProtocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner"); address defaultGuardian = makeAddr("defaultGuardian"); bool defaultPaused = false; @@ -210,7 +210,7 @@ contract DeploySuperchain_Test is Test { // Generate random input values from the seed. This doesn't give us the benefit of the forge // fuzzer's dictionary, but that's ok because we are just testing that values are set and // passed correctly. - address proxyAdminOwner = address(uint160(uint256(hash(_seed, 0)))); + address superchainProxyAdminOwner = address(uint160(uint256(hash(_seed, 0)))); address protocolVersionsOwner = address(uint160(uint256(hash(_seed, 1)))); address guardian = address(uint160(uint256(hash(_seed, 2)))); bool paused = bool(uint8(uint256(hash(_seed, 3))) % 2 == 0); @@ -218,7 +218,7 @@ contract DeploySuperchain_Test is Test { ProtocolVersion recommendedProtocolVersion = ProtocolVersion.wrap(uint256(hash(_seed, 5))); // Set the input values on the input contract. - dsi.set(dsi.proxyAdminOwner.selector, proxyAdminOwner); + dsi.set(dsi.superchainProxyAdminOwner.selector, superchainProxyAdminOwner); dsi.set(dsi.protocolVersionsOwner.selector, protocolVersionsOwner); dsi.set(dsi.guardian.selector, guardian); dsi.set(dsi.paused.selector, paused); @@ -229,7 +229,7 @@ contract DeploySuperchain_Test is Test { deploySuperchain.run(dsi, dso); // Assert inputs were properly passed through to the contract initializers. - assertEq(address(dso.superchainProxyAdmin().owner()), proxyAdminOwner, "100"); + assertEq(address(dso.superchainProxyAdmin().owner()), superchainProxyAdminOwner, "100"); assertEq(address(dso.protocolVersionsProxy().owner()), protocolVersionsOwner, "200"); assertEq(address(dso.superchainConfigProxy().guardian()), guardian, "300"); assertEq(dso.superchainConfigProxy().paused(), paused, "400"); @@ -270,7 +270,7 @@ contract DeploySuperchain_Test is Test { function test_run_NullInput_reverts() public { // Set default values for all inputs. - dsi.set(dsi.proxyAdminOwner.selector, defaultProxyAdminOwner); + dsi.set(dsi.superchainProxyAdminOwner.selector, defaultSuperchainProxyAdminOwner); dsi.set(dsi.protocolVersionsOwner.selector, defaultProtocolVersionsOwner); dsi.set(dsi.guardian.selector, defaultGuardian); dsi.set(dsi.paused.selector, defaultPaused); @@ -281,10 +281,11 @@ contract DeploySuperchain_Test is Test { // methods to set the zero address, so we use StdStorage. We can't use the `checked_write` // method, because it does a final call to test that the value was set correctly, but for us // that would revert. Therefore we use StdStorage to find the slot, then we write to it. - uint256 slot = zeroOutSlotForSelector(dsi.proxyAdminOwner.selector); - vm.expectRevert("DeploySuperchainInput: proxyAdminOwner not set"); + uint256 slot = zeroOutSlotForSelector(dsi.superchainProxyAdminOwner.selector); + vm.expectRevert("DeploySuperchainInput: superchainProxyAdminOwner not set"); deploySuperchain.run(dsi, dso); - vm.store(address(dsi), bytes32(slot), bytes32(uint256(uint160(defaultProxyAdminOwner)))); // Restore the value + vm.store(address(dsi), bytes32(slot), bytes32(uint256(uint160(defaultSuperchainProxyAdminOwner)))); // Restore + // the value // we just tested. slot = zeroOutSlotForSelector(dsi.protocolVersionsOwner.selector); diff --git a/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml b/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml index 4cbcce25d004..0334814500cd 100644 --- a/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml +++ b/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml @@ -3,6 +3,6 @@ requiredProtocolVersion = 1 recommendedProtocolVersion = 2 [roles] -proxyAdminOwner = "0x51f0348a9fA2aAbaB45E82825Fbd13d406e04497" +superchainProxyAdminOwner = "0x51f0348a9fA2aAbaB45E82825Fbd13d406e04497" protocolVersionsOwner = "0xeEB4cc05dC0dE43c465f97cfc703D165418CA93A" -guardian = "0xE5DbA98c65F4B9EB0aeEBb3674fE64f88509a1eC" \ No newline at end of file +guardian = "0xE5DbA98c65F4B9EB0aeEBb3674fE64f88509a1eC" From ebd761ce6b86ca3127e071abc75df35f2991a931 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Wed, 18 Sep 2024 15:49:05 -0700 Subject: [PATCH 2/8] test: add more assertions from ChainAssertions, and some new ones --- .../scripts/DeployImplementations.s.sol | 172 +++++++++++++++++- .../scripts/DeployOPChain.s.sol | 2 +- .../scripts/DeploySuperchain.s.sol | 55 ++++++ .../scripts/libraries/DeployUtils.sol | 10 +- 4 files changed, 232 insertions(+), 7 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol index 4c413bfe3edd..c7aa3d89dafb 100644 --- a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol @@ -5,6 +5,12 @@ import { Script } from "forge-std/Script.sol"; import { LibString } from "@solady/utils/LibString.sol"; +import { IResourceMetering } from "src/L1/interfaces/IResourceMetering.sol"; +import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; + +import { Constants } from "src/libraries/Constants.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; + import { ProxyAdmin } from "src/universal/ProxyAdmin.sol"; import { Proxy } from "src/universal/Proxy.sol"; import { L1ChugSplashProxy } from "src/legacy/L1ChugSplashProxy.sol"; @@ -191,7 +197,7 @@ contract DeployImplementationsOutput is BaseDeployIO { require(false, "DeployImplementationsOutput: not implemented"); } - function checkOutput(DeployImplementationsInput) public { + function checkOutput(DeployImplementationsInput _dii) public { address[] memory addrs = Solarray.addresses( address(this.opsmProxy()), address(this.optimismPortalImpl()), @@ -207,7 +213,7 @@ contract DeployImplementationsOutput is BaseDeployIO { ); DeployUtils.assertValidContractAddresses(addrs); - // TODO Also add the assertions for the implementation contracts from ChainAssertions.sol + assertValidDeploy(_dii); } function opsmProxy() public returns (OPStackManager) { @@ -265,6 +271,168 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertValidContractAddress(address(_disputeGameFactoryImpl)); return _disputeGameFactoryImpl; } + + // -------- Deployment Assertions -------- + function assertValidDeploy(DeployImplementationsInput _dii) public { + assertValidOpsmProxy(_dii); + assertValidOptimismPortalImpl(_dii); + assertValidDelayedWETHImpl(_dii); + assertValidPreimageOracleSingleton(_dii); + assertValidMipsSingleton(_dii); + assertValidSystemConfigImpl(_dii); + assertValidL1CrossDomainMessengerImpl(_dii); + assertValidL1ERC721BridgeImpl(_dii); + assertValidL1StandardBridgeImpl(_dii); + assertValidOptimismMintableERC20FactoryImpl(_dii); + assertValidDisputeGameFactoryImpl(_dii); + } + + function assertValidOpsmProxy(DeployImplementationsInput _dii) internal { + // First we check the proxy as itself. + Proxy proxy = Proxy(payable(address(opsm()))); + vm.prank(address(0)); + address admin = proxy.admin(); + require(admin == address(_dii.superchainProxyAdmin()), "OPSM-10"); + + // Then we check the proxy as OPSM. + DeployUtils.assertInitialized({ _contractAddress: address(opsm()), _slot: 0, _offset: 0 }); + require(address(opsm().superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-20"); + require(address(opsm().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-30"); + require(LibString.eq(opsm().latestRelease(), _dii.release()), "OPSM-50"); // Initial release must be the latest. + + // Lastly we check it's implementation. + vm.prank(address(0)); + OPStackManager impl = OPStackManager(proxy.implementation()); + DeployUtils.assertInitialized({ _contractAddress: address(impl), _slot: 0, _offset: 0 }); + require(address(impl.superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-60"); + require(address(impl.protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-70"); + } + + function assertValidOptimismPortalImpl(DeployImplementationsInput) internal view { + OptimismPortal2 portal = optimismPortalImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(portal), _slot: 0, _offset: 0 }); + + require(address(portal.disputeGameFactory()) == address(0), "OP-10"); + require(address(portal.systemConfig()) == address(0), "OP-20"); + require(address(portal.superchainConfig()) == address(0), "OP-30"); + require(portal.l2Sender() == Constants.DEFAULT_L2_SENDER, "OP-40"); + + // This slot is the custom gas token _balance and this check ensures + // that it stays unset for forwards compatibility with custom gas token. + require(vm.load(address(portal), bytes32(uint256(61))) == bytes32(0), "OP-50"); + } + + function assertValidDelayedWETHImpl(DeployImplementationsInput _dii) internal view { + DelayedWETH delayedWETH = delayedWETHImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(delayedWETH), _slot: 0, _offset: 0 }); + + require(delayedWETH.owner() == address(0), "DW-10"); + require(delayedWETH.delay() == _dii.withdrawalDelaySeconds(), "DW-20"); + require(delayedWETH.config() == ISuperchainConfig(address(0)), "DW-30"); + } + + function assertValidPreimageOracleSingleton(DeployImplementationsInput _dii) internal view { + PreimageOracle oracle = preimageOracleSingleton(); + + require(oracle.minProposalSize() == _dii.minProposalSizeBytes(), "PO-10"); + require(oracle.challengePeriod() == _dii.challengePeriodSeconds(), "PO-20"); + } + + function assertValidMipsSingleton(DeployImplementationsInput) internal view { + MIPS mips = mipsSingleton(); + + require(address(mips.oracle()) == address(preimageOracleSingleton()), "MIPS-10"); + } + + function assertValidSystemConfigImpl(DeployImplementationsInput) internal view { + SystemConfig systemConfig = systemConfigImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(systemConfig), _slot: 0, _offset: 0 }); + + require(systemConfig.owner() == address(0xdead), "SC-10"); + require(systemConfig.overhead() == 0, "SC-20"); + require(systemConfig.scalar() == uint256(0x01) << 248, "SC-30"); + require(systemConfig.basefeeScalar() == 0, "SC-40"); + require(systemConfig.blobbasefeeScalar() == 0, "SC-50"); + require(systemConfig.batcherHash() == bytes32(0), "SC-60"); + require(systemConfig.gasLimit() == 1, "SC-70"); + require(systemConfig.unsafeBlockSigner() == address(0), "SC-80"); + + IResourceMetering.ResourceConfig memory resourceConfig = systemConfig.resourceConfig(); + require(resourceConfig.maxResourceLimit == 1, "SC-90"); + require(resourceConfig.elasticityMultiplier == 1, "SC-100"); + require(resourceConfig.baseFeeMaxChangeDenominator == 2, "SC-110"); + require(resourceConfig.systemTxMaxGas == 0, "SC-120"); + require(resourceConfig.minimumBaseFee == 0, "SC-130"); + require(resourceConfig.maximumBaseFee == 0, "SC-140"); + + require(systemConfig.startBlock() == type(uint256).max, "SC-150"); + require(systemConfig.batchInbox() == address(0), "SC-160"); + require(systemConfig.l1CrossDomainMessenger() == address(0), "SC-170"); + require(systemConfig.l1ERC721Bridge() == address(0), "SC-180"); + require(systemConfig.l1StandardBridge() == address(0), "SC-190"); + require(systemConfig.disputeGameFactory() == address(0), "SC-200"); + require(systemConfig.optimismPortal() == address(0), "SC-210"); + require(systemConfig.optimismMintableERC20Factory() == address(0), "SC-220"); + } + + function assertValidL1CrossDomainMessengerImpl(DeployImplementationsInput) internal view { + L1CrossDomainMessenger messenger = l1CrossDomainMessengerImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(messenger), _slot: 0, _offset: 20 }); + + require(address(messenger.OTHER_MESSENGER()) == Predeploys.L2_CROSS_DOMAIN_MESSENGER, "L1xDM-10"); + require(address(messenger.otherMessenger()) == Predeploys.L2_CROSS_DOMAIN_MESSENGER, "L1xDM-20"); + require(address(messenger.PORTAL()) == address(0), "L1xDM-30"); + require(address(messenger.portal()) == address(0), "L1xDM-40"); + require(address(messenger.superchainConfig()) == address(0), "L1xDM-50"); + + bytes32 xdmSenderSlot = vm.load(address(messenger), bytes32(uint256(204))); + require(address(uint160(uint256(xdmSenderSlot))) == Constants.DEFAULT_L2_SENDER, "L1xDM-60"); + } + + function assertValidL1ERC721BridgeImpl(DeployImplementationsInput) internal view { + L1ERC721Bridge bridge = l1ERC721BridgeImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 }); + + require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_ERC721_BRIDGE, "LEB-10"); + require(address(bridge.otherBridge()) == Predeploys.L2_ERC721_BRIDGE, "LEB-20"); + require(address(bridge.MESSENGER()) == address(0), "LEB-30"); + require(address(bridge.messenger()) == address(0), "LEB-40"); + require(address(bridge.superchainConfig()) == address(0), "LEB-50"); + } + + function assertValidL1StandardBridgeImpl(DeployImplementationsInput) internal view { + L1StandardBridge bridge = l1StandardBridgeImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 }); + + require(address(bridge.MESSENGER()) == address(0), "LSB-10"); + require(address(bridge.messenger()) == address(0), "LSB-20"); + require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_STANDARD_BRIDGE, "LSB-30"); + require(address(bridge.otherBridge()) == Predeploys.L2_STANDARD_BRIDGE, "LSB-40"); + require(address(bridge.superchainConfig()) == address(0), "LSB-50"); + } + + function assertValidOptimismMintableERC20FactoryImpl(DeployImplementationsInput) internal view { + OptimismMintableERC20Factory factory = optimismMintableERC20FactoryImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(factory), _slot: 0, _offset: 0 }); + + require(address(factory.BRIDGE()) == address(0), "OM-10"); + require(address(factory.bridge()) == address(0), "OM-20"); + } + + function assertValidDisputeGameFactoryImpl(DeployImplementationsInput) internal view { + DisputeGameFactory factory = disputeGameFactoryImpl(); + + DeployUtils.assertInitialized({ _contractAddress: address(factory), _slot: 0, _offset: 0 }); + + require(address(factory.owner()) == address(0), "DG-10"); + } } contract DeployImplementations is Script { diff --git a/packages/contracts-bedrock/scripts/DeployOPChain.s.sol b/packages/contracts-bedrock/scripts/DeployOPChain.s.sol index fbf8b8054531..e8afcf46ad25 100644 --- a/packages/contracts-bedrock/scripts/DeployOPChain.s.sol +++ b/packages/contracts-bedrock/scripts/DeployOPChain.s.sol @@ -274,7 +274,7 @@ contract DeployOPChainOutput is BaseDeployIO { return _delayedWETHPermissionlessGameProxy; } - // -------- Assertions on chain architecture -------- + // -------- Deployment Assertions -------- function assertValidDeploy(DeployOPChainInput _doi) internal view { assertValidSystemConfig(_doi); diff --git a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol index 5ff3abcff7e8..7769f70f3d4c 100644 --- a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol @@ -230,6 +230,7 @@ contract DeploySuperchainOutput is BaseDeployIO { require(actualProtocolVersionsImpl == address(_protocolVersionsImpl), "200"); // TODO Also add the assertions for the implementation contracts from ChainAssertions.sol + assertValidDeploy(_dsi); } function superchainProxyAdmin() public view returns (ProxyAdmin) { @@ -256,6 +257,60 @@ contract DeploySuperchainOutput is BaseDeployIO { DeployUtils.assertValidContractAddress(address(_protocolVersionsProxy)); return _protocolVersionsProxy; } + + // -------- Deployment Assertions -------- + function assertValidDeploy(DeploySuperchainInput _dsi) public { + assertValidSuperchainProxyAdmin(_dsi); + assertValidSuperchainConfig(_dsi); + assertValidProtocolVersions(_dsi); + } + + function assertValidSuperchainProxyAdmin(DeploySuperchainInput _dsi) internal view { + require(superchainProxyAdmin().owner() == _dsi.superchainProxyAdminOwner(), "SPA-10"); + } + + function assertValidSuperchainConfig(DeploySuperchainInput _dsi) internal { + // Proxy checks. + SuperchainConfig superchainConfig = superchainConfigProxy(); + DeployUtils.assertInitialized({ _contractAddress: address(superchainConfig), _slot: 0, _offset: 0 }); + require(superchainConfig.guardian() == _dsi.guardian(), "SC-10"); + require(superchainConfig.paused() == _dsi.paused(), "SC-20"); + + vm.startPrank(address(0)); + require(Proxy(payable(address(superchainConfig))).implementation() == address(superchainConfigImpl()), "SC-30"); + require(Proxy(payable(address(superchainConfig))).admin() == address(superchainProxyAdmin()), "SC-40"); + vm.stopPrank(); + + // Implementation checks + superchainConfig = superchainConfigImpl(); + require(superchainConfig.guardian() == address(0), "SC-50"); + require(superchainConfig.paused() == false, "SC-60"); + } + + function assertValidProtocolVersions(DeploySuperchainInput _dsi) internal { + // Proxy checks. + ProtocolVersions pv = protocolVersionsProxy(); + DeployUtils.assertInitialized({ _contractAddress: address(pv), _slot: 0, _offset: 0 }); + require(pv.owner() == _dsi.protocolVersionsOwner(), "PV-10"); + require( + ProtocolVersion.unwrap(pv.required()) == ProtocolVersion.unwrap(_dsi.requiredProtocolVersion()), "PV-20" + ); + require( + ProtocolVersion.unwrap(pv.recommended()) == ProtocolVersion.unwrap(_dsi.recommendedProtocolVersion()), + "PV-30" + ); + + vm.startPrank(address(0)); + require(Proxy(payable(address(pv))).implementation() == address(protocolVersionsImpl()), "PV-40"); + require(Proxy(payable(address(pv))).admin() == address(superchainProxyAdmin()), "PV-50"); + vm.stopPrank(); + + // Implementation checks. + pv = protocolVersionsImpl(); + require(pv.owner() == address(0xdead), "PV-60"); + require(ProtocolVersion.unwrap(pv.required()) == 0, "PV-70"); + require(ProtocolVersion.unwrap(pv.recommended()) == 0, "PV-80"); + } } // For all broadcasts in this script we explicitly specify the deployer as `msg.sender` because for diff --git a/packages/contracts-bedrock/scripts/libraries/DeployUtils.sol b/packages/contracts-bedrock/scripts/libraries/DeployUtils.sol index d3586d1b7232..cf8eaf7cb69f 100644 --- a/packages/contracts-bedrock/scripts/libraries/DeployUtils.sol +++ b/packages/contracts-bedrock/scripts/libraries/DeployUtils.sol @@ -48,13 +48,15 @@ library DeployUtils { } } - // Asserts that for a given contract the value of a storage slot at an offset is 1. This - // is used to assert that a contract is initialized. + // Asserts that for a given contract the value of a storage slot at an offset is 1 or + // `type(uint8).max`. The value is set to 1 when a contract is initialized, and set to + // `type(uint8).max` when `_disableInitializers` is called. function assertInitialized(address _contractAddress, uint256 _slot, uint256 _offset) internal view { bytes32 slotVal = vm.load(_contractAddress, bytes32(_slot)); + uint8 value = uint8((uint256(slotVal) >> (_offset * 8)) & 0xFF); require( - uint8((uint256(slotVal) >> (_offset * 8)) & 0xFF) == uint8(1), - "Storage value is not 1 at the given slot and offset" + value == 1 || value == type(uint8).max, + "Value at the given slot and offset does not indicate initialization" ); } } From fbc396dac98cb93cedfcbd60466e046c5581562c Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Wed, 18 Sep 2024 15:56:57 -0700 Subject: [PATCH 3/8] chore: ensure unique, clear, consistent revert string IDs also sorts methods alphabetically for clarity --- .../scripts/DeployImplementations.s.sol | 92 +++++++++---------- .../scripts/DeployOPChain.s.sol | 83 +++++++++-------- .../scripts/DeploySuperchain.s.sol | 14 +-- 3 files changed, 97 insertions(+), 92 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol index c7aa3d89dafb..3d8ccae75320 100644 --- a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol @@ -274,17 +274,17 @@ contract DeployImplementationsOutput is BaseDeployIO { // -------- Deployment Assertions -------- function assertValidDeploy(DeployImplementationsInput _dii) public { - assertValidOpsmProxy(_dii); - assertValidOptimismPortalImpl(_dii); assertValidDelayedWETHImpl(_dii); - assertValidPreimageOracleSingleton(_dii); - assertValidMipsSingleton(_dii); - assertValidSystemConfigImpl(_dii); + assertValidDisputeGameFactoryImpl(_dii); assertValidL1CrossDomainMessengerImpl(_dii); assertValidL1ERC721BridgeImpl(_dii); assertValidL1StandardBridgeImpl(_dii); + assertValidMipsSingleton(_dii); + assertValidOpsmProxy(_dii); assertValidOptimismMintableERC20FactoryImpl(_dii); - assertValidDisputeGameFactoryImpl(_dii); + assertValidOptimismPortalImpl(_dii); + assertValidPreimageOracleSingleton(_dii); + assertValidSystemConfigImpl(_dii); } function assertValidOpsmProxy(DeployImplementationsInput _dii) internal { @@ -313,14 +313,14 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(portal), _slot: 0, _offset: 0 }); - require(address(portal.disputeGameFactory()) == address(0), "OP-10"); - require(address(portal.systemConfig()) == address(0), "OP-20"); - require(address(portal.superchainConfig()) == address(0), "OP-30"); - require(portal.l2Sender() == Constants.DEFAULT_L2_SENDER, "OP-40"); + require(address(portal.disputeGameFactory()) == address(0), "PORTAL-10"); + require(address(portal.systemConfig()) == address(0), "PORTAL-20"); + require(address(portal.superchainConfig()) == address(0), "PORTAL-30"); + require(portal.l2Sender() == Constants.DEFAULT_L2_SENDER, "PORTAL-40"); // This slot is the custom gas token _balance and this check ensures // that it stays unset for forwards compatibility with custom gas token. - require(vm.load(address(portal), bytes32(uint256(61))) == bytes32(0), "OP-50"); + require(vm.load(address(portal), bytes32(uint256(61))) == bytes32(0), "PORTAL-50"); } function assertValidDelayedWETHImpl(DeployImplementationsInput _dii) internal view { @@ -351,31 +351,31 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(systemConfig), _slot: 0, _offset: 0 }); - require(systemConfig.owner() == address(0xdead), "SC-10"); - require(systemConfig.overhead() == 0, "SC-20"); - require(systemConfig.scalar() == uint256(0x01) << 248, "SC-30"); - require(systemConfig.basefeeScalar() == 0, "SC-40"); - require(systemConfig.blobbasefeeScalar() == 0, "SC-50"); - require(systemConfig.batcherHash() == bytes32(0), "SC-60"); - require(systemConfig.gasLimit() == 1, "SC-70"); - require(systemConfig.unsafeBlockSigner() == address(0), "SC-80"); + require(systemConfig.owner() == address(0xdead), "SYSCON-10"); + require(systemConfig.overhead() == 0, "SYSCON-20"); + require(systemConfig.scalar() == uint256(0x01) << 248, "SYSCON-30"); + require(systemConfig.basefeeScalar() == 0, "SYSCON-40"); + require(systemConfig.blobbasefeeScalar() == 0, "SYSCON-50"); + require(systemConfig.batcherHash() == bytes32(0), "SYSCON-60"); + require(systemConfig.gasLimit() == 1, "SYSCON-70"); + require(systemConfig.unsafeBlockSigner() == address(0), "SYSCON-80"); IResourceMetering.ResourceConfig memory resourceConfig = systemConfig.resourceConfig(); - require(resourceConfig.maxResourceLimit == 1, "SC-90"); - require(resourceConfig.elasticityMultiplier == 1, "SC-100"); - require(resourceConfig.baseFeeMaxChangeDenominator == 2, "SC-110"); - require(resourceConfig.systemTxMaxGas == 0, "SC-120"); - require(resourceConfig.minimumBaseFee == 0, "SC-130"); - require(resourceConfig.maximumBaseFee == 0, "SC-140"); - - require(systemConfig.startBlock() == type(uint256).max, "SC-150"); - require(systemConfig.batchInbox() == address(0), "SC-160"); - require(systemConfig.l1CrossDomainMessenger() == address(0), "SC-170"); - require(systemConfig.l1ERC721Bridge() == address(0), "SC-180"); - require(systemConfig.l1StandardBridge() == address(0), "SC-190"); - require(systemConfig.disputeGameFactory() == address(0), "SC-200"); - require(systemConfig.optimismPortal() == address(0), "SC-210"); - require(systemConfig.optimismMintableERC20Factory() == address(0), "SC-220"); + require(resourceConfig.maxResourceLimit == 1, "SYSCON-90"); + require(resourceConfig.elasticityMultiplier == 1, "SYSCON-100"); + require(resourceConfig.baseFeeMaxChangeDenominator == 2, "SYSCON-110"); + require(resourceConfig.systemTxMaxGas == 0, "SYSCON-120"); + require(resourceConfig.minimumBaseFee == 0, "SYSCON-130"); + require(resourceConfig.maximumBaseFee == 0, "SYSCON-140"); + + require(systemConfig.startBlock() == type(uint256).max, "SYSCON-150"); + require(systemConfig.batchInbox() == address(0), "SYSCON-160"); + require(systemConfig.l1CrossDomainMessenger() == address(0), "SYSCON-170"); + require(systemConfig.l1ERC721Bridge() == address(0), "SYSCON-180"); + require(systemConfig.l1StandardBridge() == address(0), "SYSCON-190"); + require(systemConfig.disputeGameFactory() == address(0), "SYSCON-200"); + require(systemConfig.optimismPortal() == address(0), "SYSCON-210"); + require(systemConfig.optimismMintableERC20Factory() == address(0), "SYSCON-220"); } function assertValidL1CrossDomainMessengerImpl(DeployImplementationsInput) internal view { @@ -398,11 +398,11 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 }); - require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_ERC721_BRIDGE, "LEB-10"); - require(address(bridge.otherBridge()) == Predeploys.L2_ERC721_BRIDGE, "LEB-20"); - require(address(bridge.MESSENGER()) == address(0), "LEB-30"); - require(address(bridge.messenger()) == address(0), "LEB-40"); - require(address(bridge.superchainConfig()) == address(0), "LEB-50"); + require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_ERC721_BRIDGE, "L721B-10"); + require(address(bridge.otherBridge()) == Predeploys.L2_ERC721_BRIDGE, "L721B-20"); + require(address(bridge.MESSENGER()) == address(0), "L721B-30"); + require(address(bridge.messenger()) == address(0), "L721B-40"); + require(address(bridge.superchainConfig()) == address(0), "L721B-50"); } function assertValidL1StandardBridgeImpl(DeployImplementationsInput) internal view { @@ -410,11 +410,11 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 }); - require(address(bridge.MESSENGER()) == address(0), "LSB-10"); - require(address(bridge.messenger()) == address(0), "LSB-20"); - require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_STANDARD_BRIDGE, "LSB-30"); - require(address(bridge.otherBridge()) == Predeploys.L2_STANDARD_BRIDGE, "LSB-40"); - require(address(bridge.superchainConfig()) == address(0), "LSB-50"); + require(address(bridge.MESSENGER()) == address(0), "L1SB-10"); + require(address(bridge.messenger()) == address(0), "L1SB-20"); + require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_STANDARD_BRIDGE, "L1SB-30"); + require(address(bridge.otherBridge()) == Predeploys.L2_STANDARD_BRIDGE, "L1SB-40"); + require(address(bridge.superchainConfig()) == address(0), "L1SB-50"); } function assertValidOptimismMintableERC20FactoryImpl(DeployImplementationsInput) internal view { @@ -422,8 +422,8 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(factory), _slot: 0, _offset: 0 }); - require(address(factory.BRIDGE()) == address(0), "OM-10"); - require(address(factory.bridge()) == address(0), "OM-20"); + require(address(factory.BRIDGE()) == address(0), "MERC20F-10"); + require(address(factory.bridge()) == address(0), "MERC20F-20"); } function assertValidDisputeGameFactoryImpl(DeployImplementationsInput) internal view { diff --git a/packages/contracts-bedrock/scripts/DeployOPChain.s.sol b/packages/contracts-bedrock/scripts/DeployOPChain.s.sol index e8afcf46ad25..1c0aed3d0807 100644 --- a/packages/contracts-bedrock/scripts/DeployOPChain.s.sol +++ b/packages/contracts-bedrock/scripts/DeployOPChain.s.sol @@ -277,13 +277,14 @@ contract DeployOPChainOutput is BaseDeployIO { // -------- Deployment Assertions -------- function assertValidDeploy(DeployOPChainInput _doi) internal view { - assertValidSystemConfig(_doi); + assertValidDelayedWETHs(_doi); + assertValidDisputeGameFactory(_doi); assertValidL1CrossDomainMessenger(_doi); + assertValidL1ERC721Bridge(_doi); assertValidL1StandardBridge(_doi); assertValidOptimismMintableERC20Factory(_doi); assertValidOptimismPortal(_doi); - assertValidDisputeGameFactory(_doi); - assertValidDelayedWETHs(_doi); + assertValidSystemConfig(_doi); // TODO Other FP assertions like the dispute games, anchor state registry, etc. // TODO add initialization assertions } @@ -293,34 +294,36 @@ contract DeployOPChainOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(systemConfig), _slot: 0, _offset: 0 }); - require(systemConfig.owner() == _doi.systemConfigOwner(), "SC-10"); - require(systemConfig.basefeeScalar() == _doi.basefeeScalar(), "SC-20"); - require(systemConfig.blobbasefeeScalar() == _doi.blobBaseFeeScalar(), "SC-30"); - require(systemConfig.batcherHash() == bytes32(uint256(uint160(_doi.batcher()))), "SC-40"); - require(systemConfig.gasLimit() == uint64(30000000), "SC-50"); // TODO allow other gas limits? - require(systemConfig.unsafeBlockSigner() == _doi.unsafeBlockSigner(), "SC-60"); - require(systemConfig.scalar() >> 248 == 1, "SC-70"); + require(systemConfig.owner() == _doi.systemConfigOwner(), "SYSCON-10"); + require(systemConfig.basefeeScalar() == _doi.basefeeScalar(), "SYSCON-20"); + require(systemConfig.blobbasefeeScalar() == _doi.blobBaseFeeScalar(), "SYSCON-30"); + require(systemConfig.batcherHash() == bytes32(uint256(uint160(_doi.batcher()))), "SYSCON-40"); + require(systemConfig.gasLimit() == uint64(30000000), "SYSCON-50"); // TODO allow other gas limits? + require(systemConfig.unsafeBlockSigner() == _doi.unsafeBlockSigner(), "SYSCON-60"); + require(systemConfig.scalar() >> 248 == 1, "SYSCON-70"); IResourceMetering.ResourceConfig memory rConfig = Constants.DEFAULT_RESOURCE_CONFIG(); IResourceMetering.ResourceConfig memory outputConfig = systemConfig.resourceConfig(); - require(outputConfig.maxResourceLimit == rConfig.maxResourceLimit, "SC-80"); - require(outputConfig.elasticityMultiplier == rConfig.elasticityMultiplier, "SC-90"); - require(outputConfig.baseFeeMaxChangeDenominator == rConfig.baseFeeMaxChangeDenominator, "SC-100"); - require(outputConfig.systemTxMaxGas == rConfig.systemTxMaxGas, "SC-110"); - require(outputConfig.minimumBaseFee == rConfig.minimumBaseFee, "SC-120"); - require(outputConfig.maximumBaseFee == rConfig.maximumBaseFee, "SC-130"); - - require(systemConfig.startBlock() == block.number, "SC-140"); - require(systemConfig.batchInbox() == _doi.opsmProxy().chainIdToBatchInboxAddress(_doi.l2ChainId()), "SC-150"); - - require(systemConfig.l1CrossDomainMessenger() == address(l1CrossDomainMessengerProxy()), "SC-160"); - require(systemConfig.l1ERC721Bridge() == address(l1ERC721BridgeProxy()), "SC-170"); - require(systemConfig.l1StandardBridge() == address(l1StandardBridgeProxy()), "SC-180"); - require(systemConfig.disputeGameFactory() == address(disputeGameFactoryProxy()), "SC-190"); - require(systemConfig.optimismPortal() == address(optimismPortalProxy()), "SC-200"); - require(systemConfig.optimismMintableERC20Factory() == address(optimismMintableERC20FactoryProxy()), "SC-210"); + require(outputConfig.maxResourceLimit == rConfig.maxResourceLimit, "SYSCON-80"); + require(outputConfig.elasticityMultiplier == rConfig.elasticityMultiplier, "SYSCON-90"); + require(outputConfig.baseFeeMaxChangeDenominator == rConfig.baseFeeMaxChangeDenominator, "SYSCON-100"); + require(outputConfig.systemTxMaxGas == rConfig.systemTxMaxGas, "SYSCON-110"); + require(outputConfig.minimumBaseFee == rConfig.minimumBaseFee, "SYSCON-120"); + require(outputConfig.maximumBaseFee == rConfig.maximumBaseFee, "SYSCON-130"); + + require(systemConfig.startBlock() == block.number, "SYSCON-140"); + require(systemConfig.batchInbox() == _doi.opsmProxy().chainIdToBatchInboxAddress(_doi.l2ChainId()), "SYSCON-150"); + + require(systemConfig.l1CrossDomainMessenger() == address(l1CrossDomainMessengerProxy()), "SYSCON-160"); + require(systemConfig.l1ERC721Bridge() == address(l1ERC721BridgeProxy()), "SYSCON-170"); + require(systemConfig.l1StandardBridge() == address(l1StandardBridgeProxy()), "SYSCON-180"); + require(systemConfig.disputeGameFactory() == address(disputeGameFactoryProxy()), "SYSCON-190"); + require(systemConfig.optimismPortal() == address(optimismPortalProxy()), "SYSCON-200"); + require( + systemConfig.optimismMintableERC20Factory() == address(optimismMintableERC20FactoryProxy()), "SYSCON-210" + ); (address gasPayingToken,) = systemConfig.gasPayingToken(); - require(gasPayingToken == Constants.ETHER, "SC-220"); + require(gasPayingToken == Constants.ETHER, "SYSCON-220"); } function assertValidL1CrossDomainMessenger(DeployOPChainInput _doi) internal view { @@ -357,8 +360,8 @@ contract DeployOPChainOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(factory), _slot: 0, _offset: 0 }); - require(factory.BRIDGE() == address(l1StandardBridgeProxy()), "OMEF-10"); - require(factory.bridge() == address(l1StandardBridgeProxy()), "OMEF-20"); + require(factory.BRIDGE() == address(l1StandardBridgeProxy()), "MERC20F-10"); + require(factory.bridge() == address(l1StandardBridgeProxy()), "MERC20F-20"); } function assertValidL1ERC721Bridge(DeployOPChainInput _doi) internal view { @@ -366,24 +369,24 @@ contract DeployOPChainOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 }); - require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_ERC721_BRIDGE, "LEB-10"); - require(address(bridge.otherBridge()) == Predeploys.L2_ERC721_BRIDGE, "LEB-20"); + require(address(bridge.OTHER_BRIDGE()) == Predeploys.L2_ERC721_BRIDGE, "L721B-10"); + require(address(bridge.otherBridge()) == Predeploys.L2_ERC721_BRIDGE, "L721B-20"); - require(address(bridge.MESSENGER()) == address(l1CrossDomainMessengerProxy()), "LEB-30"); - require(address(bridge.messenger()) == address(l1CrossDomainMessengerProxy()), "LEB-40"); - require(address(bridge.superchainConfig()) == address(_doi.opsmProxy().superchainConfig()), "LEB-50"); + require(address(bridge.MESSENGER()) == address(l1CrossDomainMessengerProxy()), "L721B-30"); + require(address(bridge.messenger()) == address(l1CrossDomainMessengerProxy()), "L721B-40"); + require(address(bridge.superchainConfig()) == address(_doi.opsmProxy().superchainConfig()), "L721B-50"); } function assertValidOptimismPortal(DeployOPChainInput _doi) internal view { OptimismPortal2 portal = optimismPortalProxy(); ISuperchainConfig superchainConfig = ISuperchainConfig(address(_doi.opsmProxy().superchainConfig())); - require(address(portal.disputeGameFactory()) == address(disputeGameFactoryProxy()), "OP-10"); - require(address(portal.systemConfig()) == address(systemConfigProxy()), "OP-20"); - require(address(portal.superchainConfig()) == address(superchainConfig), "OP-30"); - require(portal.guardian() == superchainConfig.guardian(), "OP-40"); - require(portal.paused() == superchainConfig.paused(), "OP-50"); - require(portal.l2Sender() == Constants.DEFAULT_L2_SENDER, "OP-60"); + require(address(portal.disputeGameFactory()) == address(disputeGameFactoryProxy()), "PORTAL-10"); + require(address(portal.systemConfig()) == address(systemConfigProxy()), "PORTAL-20"); + require(address(portal.superchainConfig()) == address(superchainConfig), "PORTAL-30"); + require(portal.guardian() == superchainConfig.guardian(), "PORTAL-40"); + require(portal.paused() == superchainConfig.paused(), "PORTAL-50"); + require(portal.l2Sender() == Constants.DEFAULT_L2_SENDER, "PORTAL-60"); // This slot is the custom gas token _balance and this check ensures // that it stays unset for forwards compatibility with custom gas token. diff --git a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol index 7769f70f3d4c..0f22174829a1 100644 --- a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol @@ -273,18 +273,20 @@ contract DeploySuperchainOutput is BaseDeployIO { // Proxy checks. SuperchainConfig superchainConfig = superchainConfigProxy(); DeployUtils.assertInitialized({ _contractAddress: address(superchainConfig), _slot: 0, _offset: 0 }); - require(superchainConfig.guardian() == _dsi.guardian(), "SC-10"); - require(superchainConfig.paused() == _dsi.paused(), "SC-20"); + require(superchainConfig.guardian() == _dsi.guardian(), "SUPCON-10"); + require(superchainConfig.paused() == _dsi.paused(), "SUPCON-20"); vm.startPrank(address(0)); - require(Proxy(payable(address(superchainConfig))).implementation() == address(superchainConfigImpl()), "SC-30"); - require(Proxy(payable(address(superchainConfig))).admin() == address(superchainProxyAdmin()), "SC-40"); + require( + Proxy(payable(address(superchainConfig))).implementation() == address(superchainConfigImpl()), "SUPCON-30" + ); + require(Proxy(payable(address(superchainConfig))).admin() == address(superchainProxyAdmin()), "SUPCON-40"); vm.stopPrank(); // Implementation checks superchainConfig = superchainConfigImpl(); - require(superchainConfig.guardian() == address(0), "SC-50"); - require(superchainConfig.paused() == false, "SC-60"); + require(superchainConfig.guardian() == address(0), "SUPCON-50"); + require(superchainConfig.paused() == false, "SUPCON-60"); } function assertValidProtocolVersions(DeploySuperchainInput _dsi) internal { From f51a387c625e32a4d61d4c4120b629b9019d6904 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Wed, 18 Sep 2024 16:02:21 -0700 Subject: [PATCH 4/8] fix: var name after rebase --- .../scripts/DeployImplementations.s.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol index 3d8ccae75320..a4478a588bc0 100644 --- a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol @@ -289,16 +289,16 @@ contract DeployImplementationsOutput is BaseDeployIO { function assertValidOpsmProxy(DeployImplementationsInput _dii) internal { // First we check the proxy as itself. - Proxy proxy = Proxy(payable(address(opsm()))); + Proxy proxy = Proxy(payable(address(opsmProxy()))); vm.prank(address(0)); address admin = proxy.admin(); require(admin == address(_dii.superchainProxyAdmin()), "OPSM-10"); // Then we check the proxy as OPSM. - DeployUtils.assertInitialized({ _contractAddress: address(opsm()), _slot: 0, _offset: 0 }); - require(address(opsm().superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-20"); - require(address(opsm().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-30"); - require(LibString.eq(opsm().latestRelease(), _dii.release()), "OPSM-50"); // Initial release must be the latest. + DeployUtils.assertInitialized({ _contractAddress: address(opsmProxy()), _slot: 0, _offset: 0 }); + require(address(opsmProxy().superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-20"); + require(address(opsmProxy().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-30"); + require(LibString.eq(opsmProxy().latestRelease(), _dii.release()), "OPSM-50"); // Initial release must be the latest. // Lastly we check it's implementation. vm.prank(address(0)); From 0d705670672124c8ab752b0499d386eec34725ae Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Wed, 18 Sep 2024 16:17:07 -0700 Subject: [PATCH 5/8] style: forge fmt --- .../contracts-bedrock/scripts/DeployImplementations.s.sol | 2 +- packages/contracts-bedrock/scripts/DeployOPChain.s.sol | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol index a4478a588bc0..2575d074a421 100644 --- a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol @@ -298,7 +298,7 @@ contract DeployImplementationsOutput is BaseDeployIO { DeployUtils.assertInitialized({ _contractAddress: address(opsmProxy()), _slot: 0, _offset: 0 }); require(address(opsmProxy().superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-20"); require(address(opsmProxy().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-30"); - require(LibString.eq(opsmProxy().latestRelease(), _dii.release()), "OPSM-50"); // Initial release must be the latest. + require(LibString.eq(opsmProxy().latestRelease(), _dii.release()), "OPSM-50"); // Initial release is latest. // Lastly we check it's implementation. vm.prank(address(0)); diff --git a/packages/contracts-bedrock/scripts/DeployOPChain.s.sol b/packages/contracts-bedrock/scripts/DeployOPChain.s.sol index 1c0aed3d0807..a13c1e581eac 100644 --- a/packages/contracts-bedrock/scripts/DeployOPChain.s.sol +++ b/packages/contracts-bedrock/scripts/DeployOPChain.s.sol @@ -312,7 +312,9 @@ contract DeployOPChainOutput is BaseDeployIO { require(outputConfig.maximumBaseFee == rConfig.maximumBaseFee, "SYSCON-130"); require(systemConfig.startBlock() == block.number, "SYSCON-140"); - require(systemConfig.batchInbox() == _doi.opsmProxy().chainIdToBatchInboxAddress(_doi.l2ChainId()), "SYSCON-150"); + require( + systemConfig.batchInbox() == _doi.opsmProxy().chainIdToBatchInboxAddress(_doi.l2ChainId()), "SYSCON-150" + ); require(systemConfig.l1CrossDomainMessenger() == address(l1CrossDomainMessengerProxy()), "SYSCON-160"); require(systemConfig.l1ERC721Bridge() == address(l1ERC721BridgeProxy()), "SYSCON-170"); From e874a0d3cc76e2d44c3a16478069ce81dc3dfee0 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Wed, 18 Sep 2024 16:18:40 -0700 Subject: [PATCH 6/8] revert proxyAdmin -> superchainProxyAdmin name change for now --- .../scripts/DeploySuperchain.s.sol | 22 +++++++++---------- .../test/DeployOPChain.t.sol | 4 ++-- .../test/DeploySuperchain.t.sol | 22 +++++++++---------- .../fixtures/test-deploy-superchain-in.toml | 2 +- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol index 0f22174829a1..f1cfdbcf571d 100644 --- a/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/DeploySuperchain.s.sol @@ -81,7 +81,7 @@ contract DeploySuperchainInput is BaseDeployIO { // Role inputs. address internal _guardian; address internal _protocolVersionsOwner; - address internal _superchainProxyAdminOwner; + address internal _proxyAdminOwner; // Other inputs. bool internal _paused; @@ -94,7 +94,7 @@ contract DeploySuperchainInput is BaseDeployIO { require(_address != address(0), "DeploySuperchainInput: cannot set zero address"); if (_sel == this.guardian.selector) _guardian = _address; else if (_sel == this.protocolVersionsOwner.selector) _protocolVersionsOwner = _address; - else if (_sel == this.superchainProxyAdminOwner.selector) _superchainProxyAdminOwner = _address; + else if (_sel == this.proxyAdminOwner.selector) _proxyAdminOwner = _address; else revert("DeploySuperchainInput: unknown selector"); } @@ -119,7 +119,7 @@ contract DeploySuperchainInput is BaseDeployIO { // Parse and set role inputs. set(this.guardian.selector, toml.readAddress(".roles.guardian")); set(this.protocolVersionsOwner.selector, toml.readAddress(".roles.protocolVersionsOwner")); - set(this.superchainProxyAdminOwner.selector, toml.readAddress(".roles.superchainProxyAdminOwner")); + set(this.proxyAdminOwner.selector, toml.readAddress(".roles.proxyAdminOwner")); // Parse and set other inputs. set(this.paused.selector, toml.readBool(".paused")); @@ -136,9 +136,9 @@ contract DeploySuperchainInput is BaseDeployIO { // validate that each input is set before accessing it. With getter methods, we can automatically // validate that each input is set before allowing any field to be accessed. - function superchainProxyAdminOwner() public view returns (address) { - require(_superchainProxyAdminOwner != address(0), "DeploySuperchainInput: superchainProxyAdminOwner not set"); - return _superchainProxyAdminOwner; + function proxyAdminOwner() public view returns (address) { + require(_proxyAdminOwner != address(0), "DeploySuperchainInput: proxyAdminOwner not set"); + return _proxyAdminOwner; } function protocolVersionsOwner() public view returns (address) { @@ -266,7 +266,7 @@ contract DeploySuperchainOutput is BaseDeployIO { } function assertValidSuperchainProxyAdmin(DeploySuperchainInput _dsi) internal view { - require(superchainProxyAdmin().owner() == _dsi.superchainProxyAdminOwner(), "SPA-10"); + require(superchainProxyAdmin().owner() == _dsi.proxyAdminOwner(), "SPA-10"); } function assertValidSuperchainConfig(DeploySuperchainInput _dsi) internal { @@ -359,7 +359,7 @@ contract DeploySuperchain is Script { deployAndInitializeProtocolVersions(_dsi, _dso); // Transfer ownership of the ProxyAdmin from the deployer to the specified owner. - transferSuperchainProxyAdminOwnership(_dsi, _dso); + transferProxyAdminOwnership(_dsi, _dso); // Output assertions, to make sure outputs were assigned correctly. _dso.checkOutput(_dsi); @@ -437,14 +437,14 @@ contract DeploySuperchain is Script { _dso.set(_dso.protocolVersionsProxy.selector, address(protocolVersionsProxy)); } - function transferSuperchainProxyAdminOwnership(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { - address superchainProxyAdminOwner = _dsi.superchainProxyAdminOwner(); + function transferProxyAdminOwnership(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { + address proxyAdminOwner = _dsi.proxyAdminOwner(); ProxyAdmin superchainProxyAdmin = _dso.superchainProxyAdmin(); DeployUtils.assertValidContractAddress(address(superchainProxyAdmin)); vm.broadcast(msg.sender); - superchainProxyAdmin.transferOwnership(superchainProxyAdminOwner); + superchainProxyAdmin.transferOwnership(proxyAdminOwner); } // -------- Utilities -------- diff --git a/packages/contracts-bedrock/test/DeployOPChain.t.sol b/packages/contracts-bedrock/test/DeployOPChain.t.sol index 643e97c9edda..b0a4cb15507e 100644 --- a/packages/contracts-bedrock/test/DeployOPChain.t.sol +++ b/packages/contracts-bedrock/test/DeployOPChain.t.sol @@ -306,7 +306,7 @@ contract DeployOPChain_TestBase is Test { DeployOPChainOutput doo; // Define default inputs for DeploySuperchain. - address superchainProxyAdminOwner = makeAddr("defaultProxyAdminOwner"); + address proxyAdminOwner = makeAddr("defaultProxyAdminOwner"); address protocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner"); address guardian = makeAddr("defaultGuardian"); bool paused = false; @@ -342,7 +342,7 @@ contract DeployOPChain_TestBase is Test { // Initialize deploy scripts. DeploySuperchain deploySuperchain = new DeploySuperchain(); (DeploySuperchainInput dsi, DeploySuperchainOutput dso) = deploySuperchain.etchIOContracts(); - dsi.set(dsi.superchainProxyAdminOwner.selector, superchainProxyAdminOwner); + dsi.set(dsi.proxyAdminOwner.selector, proxyAdminOwner); dsi.set(dsi.protocolVersionsOwner.selector, protocolVersionsOwner); dsi.set(dsi.guardian.selector, guardian); dsi.set(dsi.paused.selector, paused); diff --git a/packages/contracts-bedrock/test/DeploySuperchain.t.sol b/packages/contracts-bedrock/test/DeploySuperchain.t.sol index 066f08b96e06..50a6ae85491f 100644 --- a/packages/contracts-bedrock/test/DeploySuperchain.t.sol +++ b/packages/contracts-bedrock/test/DeploySuperchain.t.sol @@ -13,7 +13,7 @@ import { DeploySuperchainInput, DeploySuperchain, DeploySuperchainOutput } from contract DeploySuperchainInput_Test is Test { DeploySuperchainInput dsi; - address superchainProxyAdminOwner = makeAddr("defaultSuperchainProxyAdminOwner"); + address proxyAdminOwner = makeAddr("defaultProxyAdminOwner"); address protocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner"); address guardian = makeAddr("defaultGuardian"); bool paused = false; @@ -30,7 +30,7 @@ contract DeploySuperchainInput_Test is Test { dsi.loadInputFile(path); - assertEq(superchainProxyAdminOwner, dsi.superchainProxyAdminOwner(), "100"); + assertEq(proxyAdminOwner, dsi.proxyAdminOwner(), "100"); assertEq(protocolVersionsOwner, dsi.protocolVersionsOwner(), "200"); assertEq(guardian, dsi.guardian(), "300"); assertEq(paused, dsi.paused(), "400"); @@ -48,7 +48,7 @@ contract DeploySuperchainInput_Test is Test { function test_getters_whenNotSet_revert() public { vm.expectRevert("DeploySuperchainInput: proxyAdminOwner not set"); - dsi.superchainProxyAdminOwner(); + dsi.proxyAdminOwner(); vm.expectRevert("DeploySuperchainInput: protocolVersionsOwner not set"); dsi.protocolVersionsOwner(); @@ -186,7 +186,7 @@ contract DeploySuperchain_Test is Test { DeploySuperchainOutput dso; // Define default input variables for testing. - address defaultSuperchainProxyAdminOwner = makeAddr("defaultSuperchainProxyAdminOwner"); + address defaultProxyAdminOwner = makeAddr("defaultProxyAdminOwner"); address defaultProtocolVersionsOwner = makeAddr("defaultProtocolVersionsOwner"); address defaultGuardian = makeAddr("defaultGuardian"); bool defaultPaused = false; @@ -210,7 +210,7 @@ contract DeploySuperchain_Test is Test { // Generate random input values from the seed. This doesn't give us the benefit of the forge // fuzzer's dictionary, but that's ok because we are just testing that values are set and // passed correctly. - address superchainProxyAdminOwner = address(uint160(uint256(hash(_seed, 0)))); + address proxyAdminOwner = address(uint160(uint256(hash(_seed, 0)))); address protocolVersionsOwner = address(uint160(uint256(hash(_seed, 1)))); address guardian = address(uint160(uint256(hash(_seed, 2)))); bool paused = bool(uint8(uint256(hash(_seed, 3))) % 2 == 0); @@ -218,7 +218,7 @@ contract DeploySuperchain_Test is Test { ProtocolVersion recommendedProtocolVersion = ProtocolVersion.wrap(uint256(hash(_seed, 5))); // Set the input values on the input contract. - dsi.set(dsi.superchainProxyAdminOwner.selector, superchainProxyAdminOwner); + dsi.set(dsi.proxyAdminOwner.selector, proxyAdminOwner); dsi.set(dsi.protocolVersionsOwner.selector, protocolVersionsOwner); dsi.set(dsi.guardian.selector, guardian); dsi.set(dsi.paused.selector, paused); @@ -229,7 +229,7 @@ contract DeploySuperchain_Test is Test { deploySuperchain.run(dsi, dso); // Assert inputs were properly passed through to the contract initializers. - assertEq(address(dso.superchainProxyAdmin().owner()), superchainProxyAdminOwner, "100"); + assertEq(address(dso.superchainProxyAdmin().owner()), proxyAdminOwner, "100"); assertEq(address(dso.protocolVersionsProxy().owner()), protocolVersionsOwner, "200"); assertEq(address(dso.superchainConfigProxy().guardian()), guardian, "300"); assertEq(dso.superchainConfigProxy().paused(), paused, "400"); @@ -270,7 +270,7 @@ contract DeploySuperchain_Test is Test { function test_run_NullInput_reverts() public { // Set default values for all inputs. - dsi.set(dsi.superchainProxyAdminOwner.selector, defaultSuperchainProxyAdminOwner); + dsi.set(dsi.proxyAdminOwner.selector, defaultProxyAdminOwner); dsi.set(dsi.protocolVersionsOwner.selector, defaultProtocolVersionsOwner); dsi.set(dsi.guardian.selector, defaultGuardian); dsi.set(dsi.paused.selector, defaultPaused); @@ -281,10 +281,10 @@ contract DeploySuperchain_Test is Test { // methods to set the zero address, so we use StdStorage. We can't use the `checked_write` // method, because it does a final call to test that the value was set correctly, but for us // that would revert. Therefore we use StdStorage to find the slot, then we write to it. - uint256 slot = zeroOutSlotForSelector(dsi.superchainProxyAdminOwner.selector); - vm.expectRevert("DeploySuperchainInput: superchainProxyAdminOwner not set"); + uint256 slot = zeroOutSlotForSelector(dsi.proxyAdminOwner.selector); + vm.expectRevert("DeploySuperchainInput: proxyAdminOwner not set"); deploySuperchain.run(dsi, dso); - vm.store(address(dsi), bytes32(slot), bytes32(uint256(uint160(defaultSuperchainProxyAdminOwner)))); // Restore + vm.store(address(dsi), bytes32(slot), bytes32(uint256(uint160(defaultProxyAdminOwner)))); // Restore // the value // we just tested. diff --git a/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml b/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml index 0334814500cd..0900e71635d7 100644 --- a/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml +++ b/packages/contracts-bedrock/test/fixtures/test-deploy-superchain-in.toml @@ -3,6 +3,6 @@ requiredProtocolVersion = 1 recommendedProtocolVersion = 2 [roles] -superchainProxyAdminOwner = "0x51f0348a9fA2aAbaB45E82825Fbd13d406e04497" +proxyAdminOwner = "0x51f0348a9fA2aAbaB45E82825Fbd13d406e04497" protocolVersionsOwner = "0xeEB4cc05dC0dE43c465f97cfc703D165418CA93A" guardian = "0xE5DbA98c65F4B9EB0aeEBb3674fE64f88509a1eC" From 9d81cc1a7a1029d7f7d51ee536265c43bf0c3fea Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Thu, 19 Sep 2024 07:21:57 -0700 Subject: [PATCH 7/8] Update packages/contracts-bedrock/scripts/DeployImplementations.s.sol Co-authored-by: Maurelian --- packages/contracts-bedrock/scripts/DeployImplementations.s.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol index 2575d074a421..a5627db90948 100644 --- a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol @@ -300,7 +300,7 @@ contract DeployImplementationsOutput is BaseDeployIO { require(address(opsmProxy().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-30"); require(LibString.eq(opsmProxy().latestRelease(), _dii.release()), "OPSM-50"); // Initial release is latest. - // Lastly we check it's implementation. + // Lastly we check its implementation. vm.prank(address(0)); OPStackManager impl = OPStackManager(proxy.implementation()); DeployUtils.assertInitialized({ _contractAddress: address(impl), _slot: 0, _offset: 0 }); From 7318db7b42bd48f71882ab6042969e26de241fc2 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Thu, 19 Sep 2024 07:26:50 -0700 Subject: [PATCH 8/8] chore: small tweaks from pr feedback --- .../scripts/DeployImplementations.s.sol | 17 ++++++++++------- .../test/DeploySuperchain.t.sol | 5 ++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol index a5627db90948..b68b4243c195 100644 --- a/packages/contracts-bedrock/scripts/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/DeployImplementations.s.sol @@ -281,6 +281,7 @@ contract DeployImplementationsOutput is BaseDeployIO { assertValidL1StandardBridgeImpl(_dii); assertValidMipsSingleton(_dii); assertValidOpsmProxy(_dii); + assertValidOpsmImpl(_dii); assertValidOptimismMintableERC20FactoryImpl(_dii); assertValidOptimismPortalImpl(_dii); assertValidPreimageOracleSingleton(_dii); @@ -292,20 +293,22 @@ contract DeployImplementationsOutput is BaseDeployIO { Proxy proxy = Proxy(payable(address(opsmProxy()))); vm.prank(address(0)); address admin = proxy.admin(); - require(admin == address(_dii.superchainProxyAdmin()), "OPSM-10"); + require(admin == address(_dii.superchainProxyAdmin()), "OPSMP-10"); // Then we check the proxy as OPSM. DeployUtils.assertInitialized({ _contractAddress: address(opsmProxy()), _slot: 0, _offset: 0 }); - require(address(opsmProxy().superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-20"); - require(address(opsmProxy().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-30"); - require(LibString.eq(opsmProxy().latestRelease(), _dii.release()), "OPSM-50"); // Initial release is latest. + require(address(opsmProxy().superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSMP-20"); + require(address(opsmProxy().protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSMP-30"); + require(LibString.eq(opsmProxy().latestRelease(), _dii.release()), "OPSMP-50"); // Initial release is latest. + } - // Lastly we check its implementation. + function assertValidOpsmImpl(DeployImplementationsInput _dii) internal { + Proxy proxy = Proxy(payable(address(opsmProxy()))); vm.prank(address(0)); OPStackManager impl = OPStackManager(proxy.implementation()); DeployUtils.assertInitialized({ _contractAddress: address(impl), _slot: 0, _offset: 0 }); - require(address(impl.superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSM-60"); - require(address(impl.protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSM-70"); + require(address(impl.superchainConfig()) == address(_dii.superchainConfigProxy()), "OPSMI-10"); + require(address(impl.protocolVersions()) == address(_dii.protocolVersionsProxy()), "OPSMI-20"); } function assertValidOptimismPortalImpl(DeployImplementationsInput) internal view { diff --git a/packages/contracts-bedrock/test/DeploySuperchain.t.sol b/packages/contracts-bedrock/test/DeploySuperchain.t.sol index 50a6ae85491f..1d6650b1cce1 100644 --- a/packages/contracts-bedrock/test/DeploySuperchain.t.sol +++ b/packages/contracts-bedrock/test/DeploySuperchain.t.sol @@ -284,9 +284,8 @@ contract DeploySuperchain_Test is Test { uint256 slot = zeroOutSlotForSelector(dsi.proxyAdminOwner.selector); vm.expectRevert("DeploySuperchainInput: proxyAdminOwner not set"); deploySuperchain.run(dsi, dso); - vm.store(address(dsi), bytes32(slot), bytes32(uint256(uint160(defaultProxyAdminOwner)))); // Restore - // the value - // we just tested. + // Restore the value we just tested. + vm.store(address(dsi), bytes32(slot), bytes32(uint256(uint160(defaultProxyAdminOwner)))); slot = zeroOutSlotForSelector(dsi.protocolVersionsOwner.selector); vm.expectRevert("DeploySuperchainInput: protocolVersionsOwner not set");