Skip to content

Commit

Permalink
feat: add thisOpcm to access state during a delegatecall
Browse files Browse the repository at this point in the history
  • Loading branch information
maurelian committed Jan 4, 2025
1 parent 3a2accf commit f249a03
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 14 deletions.
17 changes: 17 additions & 0 deletions packages/contracts-bedrock/src/L1/OPContractsManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -197,6 +202,7 @@ contract OPContractsManager is ISemver {

blueprint = _blueprints;
implementation = _implementations;
thisOPCM = this;
}

function deploy(DeployInput calldata _input) external returns (DeployOutput memory) {
Expand Down Expand Up @@ -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({
Expand All @@ -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
);
}
}

Expand Down
36 changes: 35 additions & 1 deletion packages/contracts-bedrock/test/L1/OPContractsManager.t.sol
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";
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";
Expand Down Expand Up @@ -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"));
}
}
7 changes: 7 additions & 0 deletions packages/contracts-bedrock/test/mocks/Callers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
19 changes: 6 additions & 13 deletions packages/contracts-bedrock/test/setup/ForkLive.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)));

Expand All @@ -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
Expand Down

0 comments on commit f249a03

Please sign in to comment.