From bc02e90ea8f90fa9259d83325f8dc7b633fcbc1a Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 8 Jan 2025 00:26:46 -0500 Subject: [PATCH] feat: Make contract deployment names consistent with names used in OPCM (#13563) * feat: Fix contract labels and make more explicit Fixes #13391 * fix: Initializable test * update gas-snapshot * feat: fix constructor tests * fix: Initializable test * feat: remove unused import * fix: DeployOwnership script * feat: cleanup * feat: Label contracts in save() function * feat: Add _removeSuffix * Address feedback * fix OptimismPortal2Impl name --- .../contracts-bedrock/scripts/Artifacts.s.sol | 25 ++-- .../scripts/deploy/Deploy.s.sol | 76 ++++++------ .../scripts/deploy/DeployOwnership.s.sol | 3 +- .../test/L1/DataAvailabilityChallenge.t.sol | 2 +- .../test/L1/L1CrossDomainMessenger.t.sol | 2 +- .../test/L1/L1ERC721Bridge.t.sol | 2 +- .../test/L1/L1StandardBridge.t.sol | 2 +- .../test/L1/OptimismPortal2.t.sol | 2 +- .../test/L1/ProtocolVersions.t.sol | 4 +- .../test/L1/SystemConfig.t.sol | 2 +- .../test/L2/Predeploys.t.sol | 2 +- .../test/setup/ForkLive.s.sol | 9 +- .../contracts-bedrock/test/setup/Setup.sol | 32 +---- .../test/vendor/Initializable.t.sol | 109 ++++++++---------- 14 files changed, 119 insertions(+), 153 deletions(-) diff --git a/packages/contracts-bedrock/scripts/Artifacts.s.sol b/packages/contracts-bedrock/scripts/Artifacts.s.sol index e7453ba9d0d1..97bd3cbf8478 100644 --- a/packages/contracts-bedrock/scripts/Artifacts.s.sol +++ b/packages/contracts-bedrock/scripts/Artifacts.s.sol @@ -7,7 +7,6 @@ import { Vm } from "forge-std/Vm.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; import { Config } from "scripts/libraries/Config.sol"; import { StorageSlot } from "scripts/libraries/ForgeArtifacts.sol"; -import { LibString } from "@solady/utils/LibString.sol"; import { ForgeArtifacts } from "scripts/libraries/ForgeArtifacts.sol"; import { Process } from "scripts/libraries/Process.sol"; @@ -191,6 +190,8 @@ abstract contract Artifacts { _namedDeployments[_name] = deployment; _newDeployments.push(deployment); _appendDeployment(_name, _deployed); + + vm.label(_deployed, _name); } /// @notice Adds a deployment to the temp deployments file @@ -211,16 +212,18 @@ abstract contract Artifacts { } /// @notice Returns the value of the internal `_initialized` storage slot for a given contract. - function loadInitializedSlot(string memory _contractName) public returns (uint8 initialized_) { - address contractAddress = mustGetAddress(_contractName); - - // Check if the contract name ends with `Proxy` and, if so override the contract name which is used to - // retrieve the storage layout. - if (LibString.endsWith(_contractName, "Proxy")) { - _contractName = LibString.slice(_contractName, 0, bytes(_contractName).length - 5); - } - - StorageSlot memory slot = ForgeArtifacts.getInitializedSlot(_contractName); + /// @param _sourceName The name of the contract in the source code + /// @param _deploymentName The name used to save() the deployed contract + function loadInitializedSlot( + string memory _sourceName, + string memory _deploymentName + ) + public + returns (uint8 initialized_) + { + address contractAddress = mustGetAddress(_deploymentName); + + StorageSlot memory slot = ForgeArtifacts.getInitializedSlot(_sourceName); bytes32 slotVal = vm.load(contractAddress, bytes32(slot.slot)); initialized_ = uint8((uint256(slotVal) >> (slot.offset * 8)) & 0xFF); } diff --git a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol index 6a7519263985..2fd70ef7d085 100644 --- a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol @@ -130,21 +130,21 @@ contract Deploy is Deployer { } /// @notice Returns the impl addresses, not reverting if any are unset. - function _impls() internal view returns (Types.ContractSet memory proxies_) { - proxies_ = Types.ContractSet({ - L1CrossDomainMessenger: getAddress("L1CrossDomainMessenger"), - L1StandardBridge: getAddress("L1StandardBridge"), - L2OutputOracle: getAddress("L2OutputOracle"), - DisputeGameFactory: getAddress("DisputeGameFactory"), - DelayedWETH: getAddress("DelayedWETH"), - PermissionedDelayedWETH: getAddress("PermissionedDelayedWETH"), - AnchorStateRegistry: getAddress("AnchorStateRegistry"), - OptimismMintableERC20Factory: getAddress("OptimismMintableERC20Factory"), - OptimismPortal: getAddress("OptimismPortal2"), - SystemConfig: getAddress("SystemConfig"), - L1ERC721Bridge: getAddress("L1ERC721Bridge"), - ProtocolVersions: getAddress("ProtocolVersions"), - SuperchainConfig: getAddress("SuperchainConfig") + function _impls() internal view returns (Types.ContractSet memory impls_) { + impls_ = Types.ContractSet({ + L1CrossDomainMessenger: getAddress("L1CrossDomainMessengerImpl"), + L1StandardBridge: getAddress("L1StandardBridgeImpl"), + L2OutputOracle: getAddress("L2OutputOracleImpl"), + DisputeGameFactory: getAddress("DisputeGameFactoryImpl"), + DelayedWETH: getAddress("DelayedWETHImpl"), + PermissionedDelayedWETH: getAddress("PermissionedDelayedWETHImpl"), + AnchorStateRegistry: getAddress("AnchorStateRegistryImpl"), + OptimismMintableERC20Factory: getAddress("OptimismMintableERC20FactoryImpl"), + OptimismPortal: getAddress("OptimismPortal2Impl"), + SystemConfig: getAddress("SystemConfigImpl"), + L1ERC721Bridge: getAddress("L1ERC721BridgeImpl"), + ProtocolVersions: getAddress("ProtocolVersionsImpl"), + SuperchainConfig: getAddress("SuperchainConfigImpl") }); } @@ -170,11 +170,11 @@ contract Deploy is Deployer { console.log("Deploying a fresh OP Stack with existing SuperchainConfig and ProtocolVersions"); IProxy scProxy = IProxy(_superchainConfigProxy); - save("SuperchainConfig", scProxy.implementation()); + save("SuperchainConfigImpl", scProxy.implementation()); save("SuperchainConfigProxy", _superchainConfigProxy); IProxy pvProxy = IProxy(_protocolVersionsProxy); - save("ProtocolVersions", pvProxy.implementation()); + save("ProtocolVersionsImpl", pvProxy.implementation()); save("ProtocolVersionsProxy", _protocolVersionsProxy); _run({ _needsSuperchain: false }); @@ -258,9 +258,9 @@ contract Deploy is Deployer { ds.run(dsi, dso); save("SuperchainProxyAdmin", address(dso.superchainProxyAdmin())); save("SuperchainConfigProxy", address(dso.superchainConfigProxy())); - save("SuperchainConfig", address(dso.superchainConfigImpl())); + save("SuperchainConfigImpl", address(dso.superchainConfigImpl())); save("ProtocolVersionsProxy", address(dso.protocolVersionsProxy())); - save("ProtocolVersions", address(dso.protocolVersionsImpl())); + save("ProtocolVersionsImpl", address(dso.protocolVersionsImpl())); // First run assertions for the ProtocolVersions and SuperchainConfig proxy contracts. Types.ContractSet memory contracts = _proxies(); @@ -268,11 +268,11 @@ contract Deploy is Deployer { ChainAssertions.checkSuperchainConfig({ _contracts: contracts, _cfg: cfg, _isProxy: true, _isPaused: false }); // Then replace the ProtocolVersions proxy with the implementation address and run assertions on it. - contracts.ProtocolVersions = mustGetAddress("ProtocolVersions"); + contracts.ProtocolVersions = mustGetAddress("ProtocolVersionsImpl"); ChainAssertions.checkProtocolVersions({ _contracts: contracts, _cfg: cfg, _isProxy: false }); // Finally replace the SuperchainConfig proxy with the implementation address and run assertions on it. - contracts.SuperchainConfig = mustGetAddress("SuperchainConfig"); + contracts.SuperchainConfig = mustGetAddress("SuperchainConfigImpl"); ChainAssertions.checkSuperchainConfig({ _contracts: contracts, _cfg: cfg, _isPaused: false, _isProxy: false }); } @@ -304,19 +304,18 @@ contract Deploy is Deployer { } di.run(dii, dio); - save("L1CrossDomainMessenger", address(dio.l1CrossDomainMessengerImpl())); - save("OptimismMintableERC20Factory", address(dio.optimismMintableERC20FactoryImpl())); - save("SystemConfig", address(dio.systemConfigImpl())); - save("L1StandardBridge", address(dio.l1StandardBridgeImpl())); - save("L1ERC721Bridge", address(dio.l1ERC721BridgeImpl())); + save("L1CrossDomainMessengerImpl", address(dio.l1CrossDomainMessengerImpl())); + save("OptimismMintableERC20FactoryImpl", address(dio.optimismMintableERC20FactoryImpl())); + save("SystemConfigImpl", address(dio.systemConfigImpl())); + save("L1StandardBridgeImpl", address(dio.l1StandardBridgeImpl())); + save("L1ERC721BridgeImpl", address(dio.l1ERC721BridgeImpl())); // Fault proofs - save("OptimismPortal", address(dio.optimismPortalImpl())); - save("OptimismPortal2", address(dio.optimismPortalImpl())); - save("DisputeGameFactory", address(dio.disputeGameFactoryImpl())); - save("DelayedWETH", address(dio.delayedWETHImpl())); - save("PreimageOracle", address(dio.preimageOracleSingleton())); - save("Mips", address(dio.mipsSingleton())); + save("OptimismPortal2Impl", address(dio.optimismPortalImpl())); + save("DisputeGameFactoryImpl", address(dio.disputeGameFactoryImpl())); + save("DelayedWETHImpl", address(dio.delayedWETHImpl())); + save("PreimageOracleSingleton", address(dio.preimageOracleSingleton())); + save("MipsSingleton", address(dio.mipsSingleton())); save("OPContractsManager", address(dio.opcm())); Types.ContractSet memory contracts = _impls(); @@ -343,7 +342,7 @@ contract Deploy is Deployer { ChainAssertions.checkOPContractsManager({ _contracts: contracts, _opcm: OPContractsManager(mustGetAddress("OPContractsManager")), - _mips: IMIPS(mustGetAddress("Mips")) + _mips: IMIPS(mustGetAddress("MipsSingleton")) }); if (_isInterop) { ChainAssertions.checkSystemConfigInterop({ _contracts: contracts, _cfg: cfg, _isProxy: false }); @@ -376,7 +375,7 @@ contract Deploy is Deployer { save("DisputeGameFactoryProxy", address(deployOutput.disputeGameFactoryProxy)); save("PermissionedDelayedWETHProxy", address(deployOutput.delayedWETHPermissionedGameProxy)); save("AnchorStateRegistryProxy", address(deployOutput.anchorStateRegistryProxy)); - save("AnchorStateRegistry", address(deployOutput.anchorStateRegistryImpl)); + save("AnchorStateRegistryImpl", address(deployOutput.anchorStateRegistryImpl)); save("PermissionedDisputeGame", address(deployOutput.permissionedDisputeGame)); save("OptimismPortalProxy", address(deployOutput.optimismPortalProxy)); save("OptimismPortal2Proxy", address(deployOutput.optimismPortalProxy)); @@ -391,7 +390,7 @@ contract Deploy is Deployer { permissionlessGameImpl == address(0), "Deploy: The PermissionlessDelayedWETH is already set by the OPCM, it is no longer necessary to deploy it separately." ); - address delayedWETHImpl = mustGetAddress("DelayedWETH"); + address delayedWETHImpl = mustGetAddress("DelayedWETHImpl"); address delayedWETHPermissionlessGameProxy = deployERC1967ProxyWithOwner("DelayedWETHProxy", msg.sender); vm.broadcast(msg.sender); IProxy(payable(delayedWETHPermissionlessGameProxy)).upgradeToAndCall({ @@ -481,6 +480,7 @@ contract Deploy is Deployer { _save: this, _salt: _implSalt(), _name: "DataAvailabilityChallenge", + _nick: "DataAvailabilityChallengeImpl", _args: DeployUtils.encodeConstructor(abi.encodeCall(IDataAvailabilityChallenge.__constructor__, ())) }) ); @@ -495,7 +495,7 @@ contract Deploy is Deployer { function initializeSystemConfig() public broadcast { console.log("Upgrading and initializing SystemConfig proxy"); address systemConfigProxy = mustGetAddress("SystemConfigProxy"); - address systemConfig = mustGetAddress("SystemConfig"); + address systemConfig = mustGetAddress("SystemConfigImpl"); bytes32 batcherHash = bytes32(uint256(uint160(cfg.batchSenderAddress()))); @@ -543,7 +543,7 @@ contract Deploy is Deployer { function initializeDataAvailabilityChallenge() public broadcast { console.log("Upgrading and initializing DataAvailabilityChallenge proxy"); address dataAvailabilityChallengeProxy = mustGetAddress("DataAvailabilityChallengeProxy"); - address dataAvailabilityChallenge = mustGetAddress("DataAvailabilityChallenge"); + address dataAvailabilityChallenge = mustGetAddress("DataAvailabilityChallengeImpl"); address finalSystemOwner = cfg.finalSystemOwner(); uint256 daChallengeWindow = cfg.daChallengeWindow(); @@ -727,7 +727,7 @@ contract Deploy is Deployer { splitDepth: cfg.faultGameSplitDepth(), clockExtension: Duration.wrap(uint64(cfg.faultGameClockExtension())), maxClockDuration: Duration.wrap(uint64(cfg.faultGameMaxClockDuration())), - vm: IBigStepper(mustGetAddress("Mips")), + vm: IBigStepper(mustGetAddress("MipsSingleton")), weth: weth, anchorStateRegistry: IAnchorStateRegistry(mustGetAddress("AnchorStateRegistryProxy")), l2ChainId: cfg.l2ChainID() diff --git a/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol index f1c3bec3b012..7f1febf8536e 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol @@ -90,7 +90,7 @@ contract DeployOwnership is Deploy { safeConfig: SafeConfig({ threshold: 1, owners: exampleGuardianOwners }), deputyGuardianModuleConfig: DeputyGuardianModuleConfig({ deputyGuardian: mustGetAddress("FoundationOperationsSafe"), - superchainConfig: ISuperchainConfig(mustGetAddress("SuperchainConfig")) + superchainConfig: ISuperchainConfig(mustGetAddress("SuperchainConfigImpl")) }) }); } @@ -323,6 +323,7 @@ contract DeployOwnership is Deploy { _save: this, _salt: _implSalt(), _name: "SuperchainConfig", + _nick: "SuperchainConfigImpl", _args: DeployUtils.encodeConstructor(abi.encodeCall(ISuperchainConfig.__constructor__, ())) }) ); diff --git a/packages/contracts-bedrock/test/L1/DataAvailabilityChallenge.t.sol b/packages/contracts-bedrock/test/L1/DataAvailabilityChallenge.t.sol index 05daec205ad9..be574f483087 100644 --- a/packages/contracts-bedrock/test/L1/DataAvailabilityChallenge.t.sol +++ b/packages/contracts-bedrock/test/L1/DataAvailabilityChallenge.t.sol @@ -57,7 +57,7 @@ contract DataAvailabilityChallengeTest is CommonTest { // EntryPoint will revert if using amount > type(uint112).max. vm.assume(sender != Preinstalls.EntryPoint_v060); vm.assume(sender != address(dataAvailabilityChallenge)); - vm.assume(sender != deploy.mustGetAddress("DataAvailabilityChallenge")); + vm.assume(sender != deploy.mustGetAddress("DataAvailabilityChallengeImpl")); vm.deal(sender, amount); vm.prank(sender); diff --git a/packages/contracts-bedrock/test/L1/L1CrossDomainMessenger.t.sol b/packages/contracts-bedrock/test/L1/L1CrossDomainMessenger.t.sol index 3616d94f6244..ef2920fa05a1 100644 --- a/packages/contracts-bedrock/test/L1/L1CrossDomainMessenger.t.sol +++ b/packages/contracts-bedrock/test/L1/L1CrossDomainMessenger.t.sol @@ -29,7 +29,7 @@ contract L1CrossDomainMessenger_Test is CommonTest { /// @notice Marked virtual to be overridden in /// test/kontrol/deployment/DeploymentSummary.t.sol function test_constructor_succeeds() external virtual { - IL1CrossDomainMessenger impl = IL1CrossDomainMessenger(deploy.mustGetAddress("L1CrossDomainMessenger")); + IL1CrossDomainMessenger impl = IL1CrossDomainMessenger(deploy.mustGetAddress("L1CrossDomainMessengerImpl")); assertEq(address(impl.superchainConfig()), address(0)); assertEq(address(impl.PORTAL()), address(0)); assertEq(address(impl.portal()), address(0)); diff --git a/packages/contracts-bedrock/test/L1/L1ERC721Bridge.t.sol b/packages/contracts-bedrock/test/L1/L1ERC721Bridge.t.sol index 4c182adc3395..0f2698426684 100644 --- a/packages/contracts-bedrock/test/L1/L1ERC721Bridge.t.sol +++ b/packages/contracts-bedrock/test/L1/L1ERC721Bridge.t.sol @@ -69,7 +69,7 @@ contract L1ERC721Bridge_Test is CommonTest { /// @notice Marked virtual to be overridden in /// test/kontrol/deployment/DeploymentSummary.t.sol function test_constructor_succeeds() public virtual { - IL1ERC721Bridge impl = IL1ERC721Bridge(deploy.mustGetAddress("L1ERC721Bridge")); + IL1ERC721Bridge impl = IL1ERC721Bridge(deploy.mustGetAddress("L1ERC721BridgeImpl")); assertEq(address(impl.MESSENGER()), address(0)); assertEq(address(impl.messenger()), address(0)); assertEq(address(impl.superchainConfig()), address(0)); diff --git a/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol b/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol index c3e80183c6eb..07fb48e33214 100644 --- a/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol +++ b/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol @@ -38,7 +38,7 @@ contract L1StandardBridge_Initialize_Test is CommonTest { /// @notice Marked virtual to be overridden in /// test/kontrol/deployment/DeploymentSummary.t.sol function test_constructor_succeeds() external virtual { - IL1StandardBridge impl = IL1StandardBridge(deploy.mustGetAddress("L1StandardBridge")); + IL1StandardBridge impl = IL1StandardBridge(deploy.mustGetAddress("L1StandardBridgeImpl")); assertEq(address(impl.superchainConfig()), address(0)); // The constructor now uses _disableInitializers, whereas OP Mainnet has these values in storage diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 19ff94552fc1..1bbc96f3d3fd 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -42,7 +42,7 @@ contract OptimismPortal2_Test is CommonTest { /// @notice Marked virtual to be overridden in /// test/kontrol/deployment/DeploymentSummary.t.sol function test_constructor_succeeds() external virtual { - IOptimismPortal2 opImpl = IOptimismPortal2(payable(deploy.mustGetAddress("OptimismPortal2"))); + IOptimismPortal2 opImpl = IOptimismPortal2(payable(deploy.mustGetAddress("OptimismPortal2Impl"))); assertEq(address(opImpl.disputeGameFactory()), address(0)); assertEq(address(opImpl.systemConfig()), address(0)); assertEq(address(opImpl.superchainConfig()), address(0)); diff --git a/packages/contracts-bedrock/test/L1/ProtocolVersions.t.sol b/packages/contracts-bedrock/test/L1/ProtocolVersions.t.sol index 21524f2ef7bf..a4321a9f15d8 100644 --- a/packages/contracts-bedrock/test/L1/ProtocolVersions.t.sol +++ b/packages/contracts-bedrock/test/L1/ProtocolVersions.t.sol @@ -28,7 +28,7 @@ contract ProtocolVersions_Initialize_Test is ProtocolVersions_Init { skipIfForkTest( "ProtocolVersions_Initialize_Test: cannot test initialization on forked network against hardhat config" ); - IProtocolVersions protocolVersionsImpl = IProtocolVersions(deploy.mustGetAddress("ProtocolVersions")); + IProtocolVersions protocolVersionsImpl = IProtocolVersions(deploy.mustGetAddress("ProtocolVersionsImpl")); address owner = deploy.cfg().finalSystemOwner(); assertEq(ProtocolVersion.unwrap(protocolVersions.required()), ProtocolVersion.unwrap(required)); @@ -42,7 +42,7 @@ contract ProtocolVersions_Initialize_Test is ProtocolVersions_Init { /// @dev Ensures that the events are emitted during initialization. function test_initialize_events_succeeds() external { - IProtocolVersions protocolVersionsImpl = IProtocolVersions(deploy.mustGetAddress("ProtocolVersions")); + IProtocolVersions protocolVersionsImpl = IProtocolVersions(deploy.mustGetAddress("ProtocolVersionsImpl")); // Wipe out the initialized slot so the proxy can be initialized again vm.store(address(protocolVersions), bytes32(0), bytes32(0)); diff --git a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol index af96c0cce901..35815bbd6df5 100644 --- a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol +++ b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol @@ -42,7 +42,7 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init { batcherHash = bytes32(uint256(uint160(deploy.cfg().batchSenderAddress()))); gasLimit = uint64(deploy.cfg().l2GenesisBlockGasLimit()); unsafeBlockSigner = deploy.cfg().p2pSequencerAddress(); - systemConfigImpl = deploy.mustGetAddress("SystemConfig"); + systemConfigImpl = deploy.mustGetAddress("SystemConfigImpl"); optimismMintableERC20Factory = deploy.mustGetAddress("OptimismMintableERC20FactoryProxy"); } diff --git a/packages/contracts-bedrock/test/L2/Predeploys.t.sol b/packages/contracts-bedrock/test/L2/Predeploys.t.sol index 6cc36c45c9e1..ccbe0ff9fa24 100644 --- a/packages/contracts-bedrock/test/L2/Predeploys.t.sol +++ b/packages/contracts-bedrock/test/L2/Predeploys.t.sol @@ -106,7 +106,7 @@ contract PredeploysBaseTest is CommonTest { } if (_isInitializable(addr)) { - assertEq(l2Genesis.loadInitializedSlot(cname), uint8(1)); + assertEq(l2Genesis.loadInitializedSlot({ _sourceName: cname, _deploymentName: cname }), uint8(1)); } } } diff --git a/packages/contracts-bedrock/test/setup/ForkLive.s.sol b/packages/contracts-bedrock/test/setup/ForkLive.s.sol index e86b1db5353d..ea9cf0bb5600 100644 --- a/packages/contracts-bedrock/test/setup/ForkLive.s.sol +++ b/packages/contracts-bedrock/test/setup/ForkLive.s.sol @@ -66,12 +66,11 @@ contract ForkLive is Deployer { // Bridge contracts address optimismPortal = vm.parseTomlAddress(opToml, ".addresses.OptimismPortalProxy"); save("OptimismPortalProxy", optimismPortal); - save("OptimismPortal", EIP1967Helper.getImplementation(optimismPortal)); - save("OptimismPortal2", EIP1967Helper.getImplementation(optimismPortal)); + save("OptimismPortal2Impl", EIP1967Helper.getImplementation(optimismPortal)); address addressManager = vm.parseTomlAddress(opToml, ".addresses.AddressManager"); save("AddressManager", addressManager); - save("L1CrossDomainMessenger", IAddressManager(addressManager).getAddress("OVM_L1CrossDomainMessenger")); + save("L1CrossDomainMessengerImpl", IAddressManager(addressManager).getAddress("OVM_L1CrossDomainMessenger")); save("L1CrossDomainMessengerProxy", vm.parseTomlAddress(opToml, ".addresses.L1CrossDomainMessengerProxy")); saveProxyAndImpl("OptimismMintableERC20Factory", opToml, ".addresses.OptimismMintableERC20FactoryProxy"); saveProxyAndImpl("L1StandardBridge", opToml, ".addresses.L1StandardBridgeProxy"); @@ -84,7 +83,7 @@ contract ForkLive is Deployer { // Fault proof non-proxied contracts save("PreimageOracle", vm.parseTomlAddress(opToml, ".addresses.PreimageOracle")); - save("Mips", vm.parseTomlAddress(opToml, ".addresses.MIPS")); + save("MipsSingleton", vm.parseTomlAddress(opToml, ".addresses.MIPS")); IDisputeGameFactory disputeGameFactory = IDisputeGameFactory(mustGetAddress("DisputeGameFactoryProxy")); save("FaultDisputeGame", vm.parseTomlAddress(opToml, ".addresses.FaultDisputeGame")); // The PermissionedDisputeGame and PermissionedDelayedWETHProxy are not listed in the registry for OP, so we @@ -104,6 +103,6 @@ contract ForkLive is Deployer { save(string.concat(_contractName, "Proxy"), proxy); address impl = EIP1967Helper.getImplementation(proxy); require(impl != address(0), "Upgrade: Implementation address is zero"); - save(_contractName, impl); + save(string.concat(_contractName, "Impl"), impl); } } diff --git a/packages/contracts-bedrock/test/setup/Setup.sol b/packages/contracts-bedrock/test/setup/Setup.sol index eab349a659d6..f7b66a84a8b9 100644 --- a/packages/contracts-bedrock/test/setup/Setup.sol +++ b/packages/contracts-bedrock/test/setup/Setup.sol @@ -198,11 +198,12 @@ contract Setup { console.log("Setup: completed L1 deployment, registering addresses now"); optimismPortal2 = IOptimismPortal2(deploy.mustGetAddress("OptimismPortalProxy")); - disputeGameFactory = IDisputeGameFactory(deploy.mustGetAddress("DisputeGameFactoryProxy")); - delayedWeth = IDelayedWETH(deploy.mustGetAddress("DelayedWETHProxy")); systemConfig = ISystemConfig(deploy.mustGetAddress("SystemConfigProxy")); l1StandardBridge = IL1StandardBridge(deploy.mustGetAddress("L1StandardBridgeProxy")); l1CrossDomainMessenger = IL1CrossDomainMessenger(deploy.mustGetAddress("L1CrossDomainMessengerProxy")); + vm.label( + AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger)), "L1CrossDomainMessengerProxy_aliased" + ); addressManager = IAddressManager(deploy.mustGetAddress("AddressManager")); l1ERC721Bridge = IL1ERC721Bridge(deploy.mustGetAddress("L1ERC721BridgeProxy")); l1OptimismMintableERC20Factory = @@ -210,35 +211,12 @@ contract Setup { protocolVersions = IProtocolVersions(deploy.mustGetAddress("ProtocolVersionsProxy")); superchainConfig = ISuperchainConfig(deploy.mustGetAddress("SuperchainConfigProxy")); anchorStateRegistry = IAnchorStateRegistry(deploy.mustGetAddress("AnchorStateRegistryProxy")); - - vm.label(deploy.mustGetAddress("OptimismPortalProxy"), "OptimismPortalProxy"); - vm.label(address(disputeGameFactory), "DisputeGameFactory"); - vm.label(deploy.mustGetAddress("DisputeGameFactoryProxy"), "DisputeGameFactoryProxy"); - vm.label(address(delayedWeth), "DelayedWETH"); - vm.label(deploy.mustGetAddress("DelayedWETHProxy"), "DelayedWETHProxy"); - vm.label(address(systemConfig), "SystemConfig"); - vm.label(deploy.mustGetAddress("SystemConfigProxy"), "SystemConfigProxy"); - vm.label(address(l1StandardBridge), "L1StandardBridge"); - vm.label(deploy.mustGetAddress("L1StandardBridgeProxy"), "L1StandardBridgeProxy"); - vm.label(address(l1CrossDomainMessenger), "L1CrossDomainMessenger"); - vm.label(deploy.mustGetAddress("L1CrossDomainMessengerProxy"), "L1CrossDomainMessengerProxy"); - vm.label(address(addressManager), "AddressManager"); - vm.label(address(l1ERC721Bridge), "L1ERC721Bridge"); - vm.label(deploy.mustGetAddress("L1ERC721BridgeProxy"), "L1ERC721BridgeProxy"); - vm.label(address(l1OptimismMintableERC20Factory), "OptimismMintableERC20Factory"); - vm.label(deploy.mustGetAddress("OptimismMintableERC20FactoryProxy"), "OptimismMintableERC20FactoryProxy"); - vm.label(address(protocolVersions), "ProtocolVersions"); - vm.label(deploy.mustGetAddress("ProtocolVersionsProxy"), "ProtocolVersionsProxy"); - vm.label(address(superchainConfig), "SuperchainConfig"); - vm.label(deploy.mustGetAddress("SuperchainConfigProxy"), "SuperchainConfigProxy"); - vm.label(address(anchorStateRegistry), "AnchorStateRegistryProxy"); - vm.label(AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger)), "L1CrossDomainMessenger_aliased"); + disputeGameFactory = IDisputeGameFactory(deploy.mustGetAddress("DisputeGameFactoryProxy")); + delayedWeth = IDelayedWETH(deploy.mustGetAddress("DelayedWETHProxy")); if (deploy.cfg().useAltDA()) { dataAvailabilityChallenge = IDataAvailabilityChallenge(deploy.mustGetAddress("DataAvailabilityChallengeProxy")); - vm.label(address(dataAvailabilityChallenge), "DataAvailabilityChallengeProxy"); - vm.label(deploy.mustGetAddress("DataAvailabilityChallenge"), "DataAvailabilityChallenge"); } console.log("Setup: registered L1 deployments"); } diff --git a/packages/contracts-bedrock/test/vendor/Initializable.t.sol b/packages/contracts-bedrock/test/vendor/Initializable.t.sol index d8e06d29d602..382b18d61b49 100644 --- a/packages/contracts-bedrock/test/vendor/Initializable.t.sol +++ b/packages/contracts-bedrock/test/vendor/Initializable.t.sol @@ -48,12 +48,16 @@ contract Initializer_Test is CommonTest { // Initialize the `contracts` array with the addresses of the contracts to test, the // calldata used to initialize them, and the storage slot of their `_initialized` flag. + // This array should contain all initializable L1 contracts. L2 contract initialization is + // tested in Predeploys.t.sol. + // The 'name' field should be the name of the contract as it saved in the deployment + // script. // SuperchainConfigImpl contracts.push( InitializeableContract({ - name: "SuperchainConfig", - target: deploy.mustGetAddress("SuperchainConfig"), + name: "SuperchainConfigImpl", + target: deploy.mustGetAddress("SuperchainConfigImpl"), initCalldata: abi.encodeCall(superchainConfig.initialize, (address(0), false)) }) ); @@ -68,8 +72,8 @@ contract Initializer_Test is CommonTest { // L1CrossDomainMessengerImpl contracts.push( InitializeableContract({ - name: "L1CrossDomainMessenger", - target: deploy.mustGetAddress("L1CrossDomainMessenger"), + name: "L1CrossDomainMessengerImpl", + target: deploy.mustGetAddress("L1CrossDomainMessengerImpl"), initCalldata: abi.encodeCall( l1CrossDomainMessenger.initialize, (superchainConfig, optimismPortal2, systemConfig) ) @@ -88,8 +92,8 @@ contract Initializer_Test is CommonTest { // DisputeGameFactoryImpl contracts.push( InitializeableContract({ - name: "DisputeGameFactory", - target: deploy.mustGetAddress("DisputeGameFactory"), + name: "DisputeGameFactoryImpl", + target: deploy.mustGetAddress("DisputeGameFactoryImpl"), initCalldata: abi.encodeCall(disputeGameFactory.initialize, (address(0))) }) ); @@ -104,8 +108,8 @@ contract Initializer_Test is CommonTest { // DelayedWETHImpl contracts.push( InitializeableContract({ - name: "DelayedWETH", - target: deploy.mustGetAddress("DelayedWETH"), + name: "DelayedWETHImpl", + target: deploy.mustGetAddress("DelayedWETHImpl"), initCalldata: abi.encodeCall(delayedWeth.initialize, (address(0), ISuperchainConfig(address(0)))) }) ); @@ -120,8 +124,8 @@ contract Initializer_Test is CommonTest { // OptimismPortal2Impl contracts.push( InitializeableContract({ - name: "OptimismPortal2", - target: deploy.mustGetAddress("OptimismPortal2"), + name: "OptimismPortal2Impl", + target: deploy.mustGetAddress("OptimismPortal2Impl"), initCalldata: abi.encodeCall( optimismPortal2.initialize, ( @@ -133,7 +137,7 @@ contract Initializer_Test is CommonTest { ) }) ); - // OptimismPortalProxy + // OptimismPortal2Proxy contracts.push( InitializeableContract({ name: "OptimismPortal2Proxy", @@ -152,8 +156,8 @@ contract Initializer_Test is CommonTest { // SystemConfigImpl contracts.push( InitializeableContract({ - name: "SystemConfig", - target: deploy.mustGetAddress("SystemConfig"), + name: "SystemConfigImpl", + target: deploy.mustGetAddress("SystemConfigImpl"), initCalldata: abi.encodeCall( systemConfig.initialize, ( @@ -224,8 +228,8 @@ contract Initializer_Test is CommonTest { // ProtocolVersionsImpl contracts.push( InitializeableContract({ - name: "ProtocolVersions", - target: deploy.mustGetAddress("ProtocolVersions"), + name: "ProtocolVersionsImpl", + target: deploy.mustGetAddress("ProtocolVersionsImpl"), initCalldata: abi.encodeCall( protocolVersions.initialize, (address(0), ProtocolVersion.wrap(1), ProtocolVersion.wrap(2)) ) @@ -241,19 +245,11 @@ contract Initializer_Test is CommonTest { ) }) ); - // L2CrossDomainMessenger - contracts.push( - InitializeableContract({ - name: "L2CrossDomainMessenger", - target: address(l2CrossDomainMessenger), - initCalldata: abi.encodeCall(l2CrossDomainMessenger.initialize, (l1CrossDomainMessenger)) - }) - ); // L1StandardBridgeImpl contracts.push( InitializeableContract({ - name: "L1StandardBridge", - target: deploy.mustGetAddress("L1StandardBridge"), + name: "L1StandardBridgeImpl", + target: deploy.mustGetAddress("L1StandardBridgeImpl"), initCalldata: abi.encodeCall( l1StandardBridge.initialize, (l1CrossDomainMessenger, superchainConfig, systemConfig) ) @@ -269,27 +265,11 @@ contract Initializer_Test is CommonTest { ) }) ); - // L2StandardBridge - contracts.push( - InitializeableContract({ - name: "L2StandardBridge", - target: address(l2StandardBridge), - initCalldata: abi.encodeCall(l2StandardBridge.initialize, (l1StandardBridge)) - }) - ); - // L2StandardBridgeInterop - contracts.push( - InitializeableContract({ - name: "L2StandardBridgeInterop", - target: address(l2StandardBridge), - initCalldata: abi.encodeCall(l2StandardBridge.initialize, (l1StandardBridge)) - }) - ); // L1ERC721BridgeImpl contracts.push( InitializeableContract({ - name: "L1ERC721Bridge", - target: deploy.mustGetAddress("L1ERC721Bridge"), + name: "L1ERC721BridgeImpl", + target: deploy.mustGetAddress("L1ERC721BridgeImpl"), initCalldata: abi.encodeCall(l1ERC721Bridge.initialize, (l1CrossDomainMessenger, superchainConfig)) }) ); @@ -301,19 +281,11 @@ contract Initializer_Test is CommonTest { initCalldata: abi.encodeCall(l1ERC721Bridge.initialize, (l1CrossDomainMessenger, superchainConfig)) }) ); - // L2ERC721Bridge - contracts.push( - InitializeableContract({ - name: "L2ERC721Bridge", - target: address(l2ERC721Bridge), - initCalldata: abi.encodeCall(l2ERC721Bridge.initialize, (payable(address(l1ERC721Bridge)))) - }) - ); // OptimismMintableERC20FactoryImpl contracts.push( InitializeableContract({ - name: "OptimismMintableERC20Factory", - target: deploy.mustGetAddress("OptimismMintableERC20Factory"), + name: "OptimismMintableERC20FactoryImpl", + target: deploy.mustGetAddress("OptimismMintableERC20FactoryImpl"), initCalldata: abi.encodeCall(l1OptimismMintableERC20Factory.initialize, (address(l1StandardBridge))) }) ); @@ -328,8 +300,8 @@ contract Initializer_Test is CommonTest { // DataAvailabilityChallengeImpl contracts.push( InitializeableContract({ - name: "DataAvailabilityChallenge", - target: deploy.mustGetAddress("DataAvailabilityChallenge"), + name: "DataAvailabilityChallengeImpl", + target: deploy.mustGetAddress("DataAvailabilityChallengeImpl"), initCalldata: abi.encodeCall(dataAvailabilityChallenge.initialize, (address(0), 0, 0, 0, 0)) }) ); @@ -344,7 +316,7 @@ contract Initializer_Test is CommonTest { // AnchorStateRegistry contracts.push( InitializeableContract({ - name: "AnchorStateRegistry", + name: "AnchorStateRegistryImpl", target: address(anchorStateRegistry), initCalldata: abi.encodeCall( anchorStateRegistry.initialize, @@ -390,8 +362,8 @@ contract Initializer_Test is CommonTest { // TODO: Eventually remove this exclusion. Same reason as above dispute contracts. excludes[6] = "src/L1/OPContractsManager.sol"; excludes[7] = "src/L1/OPContractsManagerInterop.sol"; - // The L2OutputOracle is not always deployed (and is no longer being modified) - excludes[8] = "src/L1/L2OutputOracle.sol"; + // L2 contract initialization is tested in Predeploys.t.sol + excludes[8] = "src/L2/*"; // Get all contract names in the src directory, minus the excluded contracts. string[] memory contractNames = ForgeArtifacts.getContractNames("src/*", excludes); @@ -423,7 +395,8 @@ contract Initializer_Test is CommonTest { // Check if this contract is in the contracts array. assertTrue( - _hasMatchingContract(contractName), string.concat("Missing ", contractName, " from contracts array") + _hasMatchingContract(string.concat(contractName, "Impl")), + string.concat("Missing ", contractName, " from contracts array") ); // If the contract is proxied, check that the proxy is in the contracts array. @@ -442,11 +415,13 @@ contract Initializer_Test is CommonTest { // Attempt to re-initialize all contracts within the `contracts` array. for (uint256 i; i < contracts.length; i++) { InitializeableContract memory _contract = contracts[i]; - string memory name = _getRealContractName(_contract.name); + string memory deploymentName = _getRealContractName(_contract.name); // Grab the value of the "initialized" storage slot. - uint8 initializedSlotVal; - initializedSlotVal = deploy.loadInitializedSlot(name); + uint8 initializedSlotVal = deploy.loadInitializedSlot({ + _sourceName: _removeSuffix(deploymentName), + _deploymentName: deploymentName + }); // Assert that the contract is already initialized. assertTrue( @@ -499,4 +474,14 @@ contract Initializer_Test is CommonTest { revert("Initializer_Test: Invalid returndata format. Expected `Error(string)`"); } } + + /// @dev Removes the `Proxy` or `Impl` suffix from the contract name. + function _removeSuffix(string memory _contractName) internal pure returns (string memory deploymentName_) { + if (LibString.endsWith(_contractName, "Proxy")) { + deploymentName_ = LibString.slice(_contractName, 0, bytes(_contractName).length - 5); + } + if (LibString.endsWith(_contractName, "Impl")) { + deploymentName_ = LibString.slice(_contractName, 0, bytes(_contractName).length - 4); + } + } }