OPCM Upgrade implementations only #13574
+225
−8
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,18 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity 0.8.15; | ||
|
|
||
| // Testing | ||
| import { Test, stdStorage, StdStorage } from "forge-std/Test.sol"; | ||
| import { CommonTest } from "test/setup/CommonTest.sol"; | ||
| import { DeployOPChain_TestBase } from "test/opcm/DeployOPChain.t.sol"; | ||
|
|
||
| // Scripts | ||
| import { DeployOPChainInput } from "scripts/deploy/DeployOPChain.s.sol"; | ||
|
|
||
| // Libraries | ||
| import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; | ||
|
|
||
| // Contracts | ||
| import { OPContractsManager } from "src/L1/OPContractsManager.sol"; | ||
| import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; | ||
| import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; | ||
|
|
@@ -148,3 +155,30 @@ contract OPContractsManager_InternalMethods_Test is Test { | |
| vm.assertEq(expected, actual); | ||
| } | ||
| } | ||
|
|
||
| contract OPContractsManager_Upgrade_Test is CommonTest { | ||
| function setUp() public override { | ||
| super.setUp(); | ||
| if (!isForkTest()) { | ||
| // This test is only supported in forked tests, as we are testing the upgrade. | ||
| vm.skip(true); | ||
| } | ||
| } | ||
|
|
||
| function test_upgrade_succeeds() public view { | ||
| // TODO: make a state var for opcm in CommonTest | ||
| OPContractsManager opcm = OPContractsManager(deploy.mustGetAddress("OPContractsManager_NextVersion")); | ||
| OPContractsManager.Implementations memory impls = opcm.implementations(); | ||
| assertEq(impls.systemConfigImpl, EIP1967Helper.getImplementation(address(systemConfig))); | ||
| assertEq(impls.l1ERC721BridgeImpl, EIP1967Helper.getImplementation(address(l1ERC721Bridge))); | ||
| assertEq(impls.disputeGameFactoryImpl, EIP1967Helper.getImplementation(address(disputeGameFactory))); | ||
| assertEq(impls.optimismPortalImpl, EIP1967Helper.getImplementation(address(optimismPortal2))); | ||
| assertEq( | ||
| impls.optimismMintableERC20FactoryImpl, | ||
| EIP1967Helper.getImplementation(address(l1OptimismMintableERC20Factory)) | ||
| ); | ||
| assertEq(impls.l1StandardBridgeImpl, EIP1967Helper.getImplementation(address(l1StandardBridge))); | ||
|
|
||
| assertEq(impls.l1CrossDomainMessengerImpl, addressManager.getAddress("OVM_L1CrossDomainMessenger")); | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,3 +91,10 @@ contract Reverter { | |
| revert(); | ||
| } | ||
| } | ||
|
|
||
| contract DelegateCaller { | ||
|
There was a problem hiding this comment. Can we add natspec explaining why we have this contract? |
||
| function dcForward(address _target, bytes calldata _data) external { | ||
| (bool success,) = _target.delegatecall(_data); | ||
| require(success, "DelegateCaller: Delegatecall failed"); | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ pragma solidity ^0.8.0; | |
|
|
||
| // Testing | ||
| import { stdJson } from "forge-std/StdJson.sol"; | ||
| import { DelegateCaller } from "test/mocks/Callers.sol"; | ||
|
|
||
| // Scripts | ||
| import { Deployer } from "scripts/deploy/Deployer.sol"; | ||
|
|
@@ -16,6 +17,9 @@ import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; | |
| import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol"; | ||
| import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; | ||
| import { IAddressManager } from "interfaces/legacy/IAddressManager.sol"; | ||
| import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; | ||
| import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; | ||
| import { OPContractsManager } from "src/L1/OPContractsManager.sol"; | ||
|
|
||
| /// @title ForkLive | ||
| /// @notice This script is called by Setup.sol as a preparation step for the foundry test suite, and is run as an | ||
|
|
@@ -96,20 +100,53 @@ contract ForkLive is Deployer { | |
|
|
||
| // Now deploy the updated OPCM and implementations of the contracts | ||
| _deployNewImplementations(); | ||
|
|
||
| // Now upgrade the contracts | ||
| _upgrade(); | ||
| } | ||
|
|
||
| /// @notice Etches a new Deploy.s.sol contract at a deterministic address, sets up the environment, | ||
| /// and deploys new implementations with a "_NextVersion" suffix. This suffix is necessary to avoid | ||
| /// naming collisions with the implementations saved above. | ||
| function _deployNewImplementations() internal { | ||
| // Etch a copy of the Deploy.s.sol contract so that we can use its deployImplementations() function | ||
| Deploy deployNew = Deploy(address(uint160(uint256(keccak256(abi.encode("optimism.deploy.new")))))); | ||
| vm.etch(address(deployNew), vm.getDeployedCode("Deploy.s.sol:Deploy")); | ||
| vm.label(address(deployNew), "DeployNew"); | ||
| vm.allowCheatcodes(address(deployNew)); | ||
|
|
||
| // Set the CONTRACT_ADDRESSES_PATH environment variable | ||
| // This is used in Deploy.setUp() to read and save the addresses it needs (SuperchainConfigProxy, | ||
| // ProtocolVersionsProxy) | ||
| string memory jsonPath = string.concat(vm.projectRoot(), "/deployments/1-deploy.json"); | ||
| vm.setEnv("CONTRACT_ADDRESSES_PATH", jsonPath); | ||
| deployNew.setUp(); | ||
| deployNew.deployImplementations({ _isInterop: false, _suffix: "_NextVersion" }); | ||
|
|
||
| // deployNew.deployImplementations() saves the addresses to its own storage as well as to the json file at | ||
| // CONTRACT_ADDRESSES_PATH. In order to use the addresses in the json file, we now need to load them into | ||
| // local storage. | ||
| loadAddresses(jsonPath); | ||
| } | ||
|
|
||
| function _upgrade() internal { | ||
| OPContractsManager opcm = OPContractsManager(mustGetAddress("OPContractsManager_NextVersion")); | ||
|
|
||
| ISystemConfig systemConfig = ISystemConfig(mustGetAddress("SystemConfigProxy")); | ||
| IProxyAdmin proxyAdmin = IProxyAdmin(EIP1967Helper.getAdmin(address(systemConfig))); | ||
|
|
||
| address upgrader = proxyAdmin.owner(); | ||
| vm.label(upgrader, "ProxyAdmin Owner"); | ||
|
|
||
| ISystemConfig[] memory systemConfigs = new ISystemConfig[](1); | ||
| systemConfigs[0] = systemConfig; | ||
| IProxyAdmin[] memory proxyAdmins = new IProxyAdmin[](1); | ||
| proxyAdmins[0] = proxyAdmin; | ||
|
Comment on lines
+141
to
+144
There was a problem hiding this comment. Consider adding helpers for this to our vendored |
||
|
|
||
| vm.etch(upgrader, vm.getDeployedCode("test/mocks/Callers.sol:DelegateCaller")); | ||
| DelegateCaller(upgrader).dcForward( | ||
| address(opcm), abi.encodeCall(OPContractsManager.upgrade, (systemConfigs, proxyAdmins)) | ||
| ); | ||
| } | ||
|
|
||
| /// @notice Saves the proxy and implementation addresses for a contract name | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -781,6 +781,7 @@ contract Specification_Test is CommonTest { | |
| _addSpec({ _name: "OPContractsManager", _sel: OPContractsManager.blueprints.selector }); | ||
| _addSpec({ _name: "OPContractsManager", _sel: OPContractsManager.chainIdToBatchInboxAddress.selector }); | ||
| _addSpec({ _name: "OPContractsManager", _sel: OPContractsManager.implementations.selector }); | ||
| _addSpec({ _name: "OPContractsManager", _sel: OPContractsManager.upgrade.selector }); | ||
|
|
||
| // OPContractsManagerInterop | ||
| _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("version()") }); | ||
|
|
@@ -792,6 +793,7 @@ contract Specification_Test is CommonTest { | |
| _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.blueprints.selector }); | ||
| _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.chainIdToBatchInboxAddress.selector }); | ||
| _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.implementations.selector }); | ||
| _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.upgrade.selector }); | ||
|
|
||
| // DeputyGuardianModule | ||
| _addSpec({ | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we are not emitting (or testing for) the
Upgradedevent