From f249a038b86a0e8e0860d5202aa7f0d8c599fd43 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Thu, 2 Jan 2025 14:16:53 -0500 Subject: [PATCH] feat: add thisOpcm to access state during a delegatecall --- .../src/L1/OPContractsManager.sol | 17 +++++++++ .../test/L1/OPContractsManager.t.sol | 36 ++++++++++++++++++- .../contracts-bedrock/test/mocks/Callers.sol | 7 ++++ .../test/setup/ForkLive.s.sol | 19 ++++------ 4 files changed, 65 insertions(+), 14 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 08d1a18b15789..2d936ee333ac1 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.15; import { console2 as console } from "forge-std/console2.sol"; + // Libraries import { Blueprint } from "src/libraries/Blueprint.sol"; import { Constants } from "src/libraries/Constants.sol"; @@ -140,6 +141,10 @@ contract OPContractsManager is ISemver { /// @notice Addresses of the latest implementation contracts. Implementations internal implementation; + /// @notice The OPContractsManager contract that is currently being used. This is needed in the upgrade function + /// which is intended to be DELEGATECALLed. + OPContractsManager public immutable thisOPCM; + // -------- Events -------- /// @notice Emitted when a new OP Stack chain is deployed. @@ -197,6 +202,7 @@ contract OPContractsManager is ISemver { blueprint = _blueprints; implementation = _implementations; + thisOPCM = this; } function deploy(DeployInput calldata _input) external returns (DeployOutput memory) { @@ -375,6 +381,8 @@ contract OPContractsManager is ISemver { /// @param _proxyAdmins Array of ProxyAdmin contracts, one per chain to upgrade /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe function upgrade(ISystemConfig[] calldata _systemConfigs, IProxyAdmin[] calldata _proxyAdmins) external { + Implementations memory impls = thisOPCM.implementations(); + for (uint256 i = 0; i < _systemConfigs.length; i++) { ISystemConfig systemConfig = _systemConfigs[i]; ISystemConfig.Addresses memory opChainAddrs = ISystemConfig.Addresses({ @@ -388,6 +396,15 @@ contract OPContractsManager is ISemver { }); IProxyAdmin proxyAdmin = _proxyAdmins[i]; + proxyAdmin.upgrade(payable(address(systemConfig)), impls.systemConfigImpl); + proxyAdmin.upgrade(payable(opChainAddrs.l1CrossDomainMessenger), impls.l1CrossDomainMessengerImpl); + proxyAdmin.upgrade(payable(opChainAddrs.l1ERC721Bridge), impls.l1ERC721BridgeImpl); + proxyAdmin.upgrade(payable(opChainAddrs.l1StandardBridge), impls.l1StandardBridgeImpl); + proxyAdmin.upgrade(payable(opChainAddrs.disputeGameFactory), impls.disputeGameFactoryImpl); + proxyAdmin.upgrade(payable(opChainAddrs.optimismPortal), impls.optimismPortalImpl); + proxyAdmin.upgrade( + payable(opChainAddrs.optimismMintableERC20Factory), impls.optimismMintableERC20FactoryImpl + ); } } diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 418eebd9e64b6..b92e865ceb7bc 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -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"; -import { DeployOPChain_TestBase } from "test/opcm/DeployOPChain.t.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 { + // 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")); + } +} diff --git a/packages/contracts-bedrock/test/mocks/Callers.sol b/packages/contracts-bedrock/test/mocks/Callers.sol index b8dd70df15e14..67c724ec1b0b5 100644 --- a/packages/contracts-bedrock/test/mocks/Callers.sol +++ b/packages/contracts-bedrock/test/mocks/Callers.sol @@ -91,3 +91,10 @@ contract Reverter { revert(); } } + +contract DelegateCaller { + function dcForward(address _target, bytes calldata _data) external { + (bool success,) = _target.delegatecall(_data); + require(success, "DelegateCaller: Delegatecall failed"); + } +} diff --git a/packages/contracts-bedrock/test/setup/ForkLive.s.sol b/packages/contracts-bedrock/test/setup/ForkLive.s.sol index 3751af9bed82d..711c26a519c91 100644 --- a/packages/contracts-bedrock/test/setup/ForkLive.s.sol +++ b/packages/contracts-bedrock/test/setup/ForkLive.s.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import { console } from "forge-std/console.sol"; // Testing import { stdJson } from "forge-std/StdJson.sol"; +import { DelegateCaller } from "test/mocks/Callers.sol"; // Scripts import { Deployer } from "scripts/deploy/Deployer.sol"; @@ -129,6 +130,7 @@ contract ForkLive is Deployer { function _upgrade() internal { OPContractsManager opcm = OPContractsManager(mustGetAddress("OPContractsManager_NextVersion")); + ISystemConfig systemConfig = ISystemConfig(mustGetAddress("SystemConfigProxy")); IProxyAdmin proxyAdmin = IProxyAdmin(EIP1967Helper.getAdmin(address(systemConfig))); @@ -140,19 +142,10 @@ contract ForkLive is Deployer { IProxyAdmin[] memory proxyAdmins = new IProxyAdmin[](1); proxyAdmins[0] = proxyAdmin; - // TODO: Add support for this prank() call to forge-std - console.log("delegatecall with prank"); - (bool success,) = address(vm).call(abi.encodeWithSignature("prank(address,bool)", address(this), true)); - require(success, "ForkLive: Failed to prank"); - - (success,) = - address(opcm).delegatecall(abi.encodeCall(OPContractsManager.upgrade, (systemConfigs, proxyAdmins))); - require(success, "ForkLive: Upgrade failed"); - - console.log("delegatecall without prank"); - (success,) = - address(opcm).delegatecall(abi.encodeCall(OPContractsManager.upgrade, (systemConfigs, proxyAdmins))); - require(success, "ForkLive: Upgrade failed"); + 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