Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove DisputeGameFactory input from SystemConfig #14820

Merged
merged 8 commits into from
Mar 13, 2025
Merged

feat: remove DisputeGameFactory input from SystemConfig #14820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9454b78
feat: remove DisputeGameFactory input from SystemConfig
smartcontracts Mar 12, 2025
17ddd57
fix: make tests work
smartcontracts Mar 12, 2025
5050402
fix: remove console log
smartcontracts Mar 12, 2025
1447c93
fix: move address fetch
smartcontracts Mar 12, 2025
139cd76
fix: address pr feedback
smartcontracts Mar 12, 2025
1084c44
fix: missing semicolon
smartcontracts Mar 12, 2025
8b519d6
chore: remove DISPUTE_GAME_FACTORY_SLOT
mds1 Mar 12, 2025
cc16b2f
test: fix abi spec
mds1 Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/contracts-bedrock/interfaces/L1/ISystemConfig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ interface ISystemConfig {
address l1CrossDomainMessenger;
address l1ERC721Bridge;
address l1StandardBridge;
address disputeGameFactory;
address optimismPortal;
address optimismMintableERC20Factory;
}
Expand All @@ -29,7 +28,6 @@ interface ISystemConfig {
event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

function BATCH_INBOX_SLOT() external view returns (bytes32);
function DISPUTE_GAME_FACTORY_SLOT() external view returns (bytes32);
function L1_CROSS_DOMAIN_MESSENGER_SLOT() external view returns (bytes32);
function L1_ERC_721_BRIDGE_SLOT() external view returns (bytes32);
function L1_STANDARD_BRIDGE_SLOT() external view returns (bytes32);
Expand Down
2 changes: 0 additions & 2 deletions packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ library ChainAssertions {
require(config.l1CrossDomainMessenger() == _contracts.L1CrossDomainMessenger, "CHECK-SCFG-160");
require(config.l1ERC721Bridge() == _contracts.L1ERC721Bridge, "CHECK-SCFG-170");
require(config.l1StandardBridge() == _contracts.L1StandardBridge, "CHECK-SCFG-180");
require(config.disputeGameFactory() == _contracts.DisputeGameFactory, "CHECK-SCFG-190");
require(config.optimismPortal() == _contracts.OptimismPortal, "CHECK-SCFG-200");
require(config.optimismMintableERC20Factory() == _contracts.OptimismMintableERC20Factory, "CHECK-SCFG-210");
} else {
Expand All @@ -116,7 +115,6 @@ library ChainAssertions {
require(config.l1CrossDomainMessenger() == address(0), "CHECK-SCFG-380");
require(config.l1ERC721Bridge() == address(0), "CHECK-SCFG-390");
require(config.l1StandardBridge() == address(0), "CHECK-SCFG-400");
require(config.disputeGameFactory() == address(0), "CHECK-SCFG-410");
require(config.optimismPortal() == address(0), "CHECK-SCFG-420");
require(config.optimismMintableERC20Factory() == address(0), "CHECK-SCFG-430");
}
Expand Down
1 change: 0 additions & 1 deletion packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,6 @@ contract Deploy is Deployer {
l1CrossDomainMessenger: artifacts.mustGetAddress("L1CrossDomainMessengerProxy"),
l1ERC721Bridge: artifacts.mustGetAddress("L1ERC721BridgeProxy"),
l1StandardBridge: artifacts.mustGetAddress("L1StandardBridgeProxy"),
disputeGameFactory: artifacts.mustGetAddress("DisputeGameFactoryProxy"),
optimismPortal: artifacts.mustGetAddress("OptimismPortalProxy"),
optimismMintableERC20Factory: artifacts.mustGetAddress("OptimismMintableERC20FactoryProxy")
}),
Expand Down
5 changes: 2 additions & 3 deletions packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,8 @@ contract DeployImplementationsOutput is BaseDeployIO {
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");
require(systemConfig.optimismPortal() == address(0), "SYSCON-200");
require(systemConfig.optimismMintableERC20Factory() == address(0), "SYSCON-210");
}

function assertValidL1CrossDomainMessengerImpl(DeployImplementationsInput) internal view {
Expand Down
5 changes: 2 additions & 3 deletions packages/contracts-bedrock/scripts/deploy/DeployOPChain.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,11 +562,10 @@ contract DeployOPChain is Script {
require(systemConfig.l1CrossDomainMessenger() == address(_doo.l1CrossDomainMessengerProxy()), "SYSCON-160");
require(systemConfig.l1ERC721Bridge() == address(_doo.l1ERC721BridgeProxy()), "SYSCON-170");
require(systemConfig.l1StandardBridge() == address(_doo.l1StandardBridgeProxy()), "SYSCON-180");
require(systemConfig.disputeGameFactory() == address(_doo.disputeGameFactoryProxy()), "SYSCON-190");
require(systemConfig.optimismPortal() == address(_doo.optimismPortalProxy()), "SYSCON-200");
require(systemConfig.optimismPortal() == address(_doo.optimismPortalProxy()), "SYSCON-190");
require(
systemConfig.optimismMintableERC20Factory() == address(_doo.optimismMintableERC20FactoryProxy()),
"SYSCON-210"
"SYSCON-200"
);
}

Expand Down
23 changes: 0 additions & 23 deletions packages/contracts-bedrock/snapshots/abi/SystemConfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,6 @@
"stateMutability": "view",
"type": "function"
},
{
"inputs": [],
"name": "DISPUTE_GAME_FACTORY_SLOT",
"outputs": [
{
"internalType": "bytes32",
"name": "",
"type": "bytes32"
}
],
"stateMutability": "view",
"type": "function"
},
{
"inputs": [],
"name": "L1_CROSS_DOMAIN_MESSENGER_SLOT",
Expand Down Expand Up @@ -259,11 +246,6 @@
"name": "l1StandardBridge",
"type": "address"
},
{
"internalType": "address",
"name": "disputeGameFactory",
"type": "address"
},
{
"internalType": "address",
"name": "optimismPortal",
Expand Down Expand Up @@ -387,11 +369,6 @@
"name": "l1StandardBridge",
"type": "address"
},
{
"internalType": "address",
"name": "disputeGameFactory",
"type": "address"
},
{
"internalType": "address",
"name": "optimismPortal",
Expand Down
8 changes: 4 additions & 4 deletions packages/contracts-bedrock/snapshots/semver-lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
"sourceCodeHash": "0x44797707aea8c63dec049a02d69ea056662a06e5cf320028ab8b388634bf1c67"
},
"src/L1/OPContractsManager.sol:OPContractsManager": {
"initCodeHash": "0x04ecca6917f73f77cf808097e48bf2fb025393907982438ae6fd9810f0f271b7",
"sourceCodeHash": "0x75ed03f4fbf1039e0f4df0c36de6944f341aa980d1e6d9087f8baceb3b19c851"
"initCodeHash": "0xaa8ca04a37dfcc419f5fc53f0411e5a7bd772b88d4df5da7558aaceea26547d1",
"sourceCodeHash": "0x11f9627893eabb8f81c538a79d55abffb0c37bef6e8d1121b085fac4008726c9"
},
"src/L1/OptimismPortal2.sol:OptimismPortal2": {
"initCodeHash": "0xbc7db7b1016025228d99a40db1760290b333bb90563dff5514fd253fd91019ba",
Expand All @@ -36,8 +36,8 @@
"sourceCodeHash": "0xfd56e63e76b1f203cceeb9bbb14396ae803cbbbf7e80ca0ee11fb586321812af"
},
"src/L1/SystemConfig.sol:SystemConfig": {
"initCodeHash": "0x471ac69544fdee81d0734b87151297ad4cf91ca1d43a2c95e9e644c0424eed65",
"sourceCodeHash": "0x8c3ccb8f3718c00c3f9bf7c866a22d87ea18a87a6e9454c31d1c97638107434c"
"initCodeHash": "0x1edc0fd719aaa39eef32e3b29bacec77b512a3494904643bab2a21a855507bbc",
"sourceCodeHash": "0xfa541a4f124ef5568f80ddefbcb257a9204b94ebef921c854598b8b510ea14d1"
},
"src/L2/BaseFeeVault.sol:BaseFeeVault": {
"initCodeHash": "0xc403d4c555d8e69a2699e01d192ae7327136701fa02da10a6d75a584b3c364c9",
Expand Down
63 changes: 30 additions & 33 deletions packages/contracts-bedrock/src/L1/OPContractsManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,27 +530,17 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {

for (uint256 i = 0; i < _opChainConfigs.length; i++) {
assertValidOpChainConfig(_opChainConfigs[i]);
ISystemConfig.Addresses memory opChainAddrs = _opChainConfigs[i].systemConfigProxy.getAddresses();

// Use the SystemConfig to grab the DisputeGameFactory address.
IDisputeGameFactory dgf = IDisputeGameFactory(_opChainConfigs[i].systemConfigProxy.disputeGameFactory());

// All chains have the PermissionedDisputeGame, grab that.
IPermissionedDisputeGame permissionedDisputeGame = IPermissionedDisputeGame(
address(
getGameImplementation(
IDisputeGameFactory(opChainAddrs.disputeGameFactory), GameTypes.PERMISSIONED_CANNON
)
)
);
IPermissionedDisputeGame permissionedDisputeGame =
IPermissionedDisputeGame(address(getGameImplementation(dgf, GameTypes.PERMISSIONED_CANNON)));

// Grab the L2 chain ID from the PermissionedDisputeGame.
uint256 l2ChainId = getL2ChainId(IFaultDisputeGame(address(permissionedDisputeGame)));

// Grab the current respectedGameType from the OptimismPortal contract before the upgrade.
GameType respectedGameType = IOptimismPortal(payable(opChainAddrs.optimismPortal)).respectedGameType();

// Grab the current SuperchainConfig from the OptimismPortal contract before the upgrade.
ISuperchainConfig superchainConfig =
IOptimismPortal(payable(opChainAddrs.optimismPortal)).superchainConfig();

// Start by upgrading the SystemConfig contract to have the l2ChainId.
upgradeToAndCall(
_opChainConfigs[i].proxyAdmin,
Expand All @@ -559,6 +549,17 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {
abi.encodeCall(ISystemConfig.upgrade, (l2ChainId))
);

// Grab chain addresses here. We need to do this after the SystemConfig upgrade or the
// addresses will be incorrect.
ISystemConfig.Addresses memory opChainAddrs = _opChainConfigs[i].systemConfigProxy.getAddresses();

// Grab the current respectedGameType from the OptimismPortal contract before the upgrade.
GameType respectedGameType = IOptimismPortal(payable(opChainAddrs.optimismPortal)).respectedGameType();

// Grab the current SuperchainConfig from the OptimismPortal contract before the upgrade.
ISuperchainConfig superchainConfig =
IOptimismPortal(payable(opChainAddrs.optimismPortal)).superchainConfig();

// Separate context to avoid stack too deep.
IAnchorStateRegistry newAnchorStateRegistryProxy;
{
Expand Down Expand Up @@ -591,7 +592,7 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {
IAnchorStateRegistry.initialize,
(
superchainConfig,
IDisputeGameFactory(opChainAddrs.disputeGameFactory),
dgf,
OutputRoot({ root: root, l2BlockNumber: l2BlockNumber }),
respectedGameType
)
Expand Down Expand Up @@ -641,19 +642,15 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {
_disputeGame: IDisputeGame(address(permissionedDisputeGame)),
_newAnchorStateRegistryProxy: newAnchorStateRegistryProxy,
_gameType: GameTypes.PERMISSIONED_CANNON,
_opChainConfig: _opChainConfigs[i],
_opChainAddrs: opChainAddrs
_opChainConfig: _opChainConfigs[i]
});
}

// Separate context to avoid stack too deep.
{
// Now retrieve the permissionless game.
IFaultDisputeGame permissionlessDisputeGame = IFaultDisputeGame(
address(
getGameImplementation(IDisputeGameFactory(opChainAddrs.disputeGameFactory), GameTypes.CANNON)
)
);
IFaultDisputeGame permissionlessDisputeGame =
IFaultDisputeGame(address(getGameImplementation(dgf, GameTypes.CANNON)));

// If it exists, replace its implementation.
if (address(permissionlessDisputeGame) != address(0)) {
Expand All @@ -663,8 +660,7 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {
_disputeGame: IDisputeGame(address(permissionlessDisputeGame)),
_newAnchorStateRegistryProxy: newAnchorStateRegistryProxy,
_gameType: GameTypes.CANNON,
_opChainConfig: _opChainConfigs[i],
_opChainAddrs: opChainAddrs
_opChainConfig: _opChainConfigs[i]
});
}
}
Expand Down Expand Up @@ -700,14 +696,12 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {
/// @param _newAnchorStateRegistryProxy The new anchor state registry proxy
/// @param _gameType The type of game to deploy
/// @param _opChainConfig The OP chain configuration
/// @param _opChainAddrs The OP chain addresses
function deployAndSetNewGameImpl(
uint256 _l2ChainId,
IDisputeGame _disputeGame,
IAnchorStateRegistry _newAnchorStateRegistryProxy,
GameType _gameType,
OPContractsManager.OpChainConfig memory _opChainConfig,
ISystemConfig.Addresses memory _opChainAddrs
OPContractsManager.OpChainConfig memory _opChainConfig
)
internal
{
Expand Down Expand Up @@ -755,7 +749,12 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase {
)
);
}
setDGFImplementation(IDisputeGameFactory(_opChainAddrs.disputeGameFactory), _gameType, IDisputeGame(newGame));

// Grab the DisputeGameFactory from the SystemConfig.
IDisputeGameFactory dgf = IDisputeGameFactory(_opChainConfig.systemConfigProxy.disputeGameFactory());

// Set the new implementation.
setDGFImplementation(dgf, _gameType, IDisputeGame(newGame));
}
}

Expand Down Expand Up @@ -988,15 +987,13 @@ contract OPContractsManagerDeployer is OPContractsManagerBase {
l1CrossDomainMessenger: address(_output.l1CrossDomainMessengerProxy),
l1ERC721Bridge: address(_output.l1ERC721BridgeProxy),
l1StandardBridge: address(_output.l1StandardBridgeProxy),
disputeGameFactory: address(_output.disputeGameFactoryProxy),
optimismPortal: address(_output.optimismPortalProxy),
optimismMintableERC20Factory: address(_output.optimismMintableERC20FactoryProxy)
});

assertValidContractAddress(opChainAddrs_.l1CrossDomainMessenger);
assertValidContractAddress(opChainAddrs_.l1ERC721Bridge);
assertValidContractAddress(opChainAddrs_.l1StandardBridge);
assertValidContractAddress(opChainAddrs_.disputeGameFactory);
assertValidContractAddress(opChainAddrs_.optimismPortal);
assertValidContractAddress(opChainAddrs_.optimismMintableERC20Factory);
}
Expand Down Expand Up @@ -1294,9 +1291,9 @@ contract OPContractsManager is ISemver {

// -------- Constants and Variables --------

/// @custom:semver 1.11.1
/// @custom:semver 1.12.0
function version() public pure virtual returns (string memory) {
return "1.11.1";
return "1.12.0";
}

OPContractsManagerGameTypeAdder public immutable opcmGameTypeAdder;
Expand Down
20 changes: 10 additions & 10 deletions packages/contracts-bedrock/src/L1/SystemConfig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Storage } from "src/libraries/Storage.sol";
// Interfaces
import { ISemver } from "interfaces/universal/ISemver.sol";
import { IResourceMetering } from "interfaces/L1/IResourceMetering.sol";
import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol";

/// @custom:proxied true
/// @title SystemConfig
Expand Down Expand Up @@ -40,7 +41,6 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {
address l1CrossDomainMessenger;
address l1ERC721Bridge;
address l1StandardBridge;
address disputeGameFactory;
address optimismPortal;
address optimismMintableERC20Factory;
}
Expand Down Expand Up @@ -81,10 +81,6 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {
/// @notice Storage slot for block at which the op-node can start searching for logs from.
bytes32 public constant START_BLOCK_SLOT = bytes32(uint256(keccak256("systemconfig.startBlock")) - 1);

/// @notice Storage slot for the DisputeGameFactory address.
bytes32 public constant DISPUTE_GAME_FACTORY_SLOT =
bytes32(uint256(keccak256("systemconfig.disputegamefactory")) - 1);

/// @notice The maximum gas limit that can be set for L2 blocks. This limit is used to enforce that the blocks
/// on L2 are not too large to process and prove. Over time, this value can be increased as various
/// optimizations and improvements are made to the system at large.
Expand Down Expand Up @@ -140,9 +136,9 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {
event ConfigUpdate(uint256 indexed version, UpdateType indexed updateType, bytes data);

/// @notice Semantic version.
/// @custom:semver 2.6.0
/// @custom:semver 3.0.0
function version() public pure virtual returns (string memory) {
return "2.6.0";
return "3.0.0";
}

/// @notice Constructs the SystemConfig contract.
Expand Down Expand Up @@ -194,7 +190,6 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {
Storage.setAddress(L1_CROSS_DOMAIN_MESSENGER_SLOT, _addresses.l1CrossDomainMessenger);
Storage.setAddress(L1_ERC_721_BRIDGE_SLOT, _addresses.l1ERC721Bridge);
Storage.setAddress(L1_STANDARD_BRIDGE_SLOT, _addresses.l1StandardBridge);
Storage.setAddress(DISPUTE_GAME_FACTORY_SLOT, _addresses.disputeGameFactory);
Storage.setAddress(OPTIMISM_PORTAL_SLOT, _addresses.optimismPortal);
Storage.setAddress(OPTIMISM_MINTABLE_ERC20_FACTORY_SLOT, _addresses.optimismMintableERC20Factory);

Expand All @@ -208,7 +203,12 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {
/// @notice Upgrades the SystemConfig by setting the L2 chain ID variable.
/// @param _l2ChainId The L2 chain ID that this SystemConfig configures.
function upgrade(uint256 _l2ChainId) external reinitializer(initVersion()) {
// Set the L2 chain ID.
l2ChainId = _l2ChainId;

// Clear out the old dispute game factory address, it's derived now.
bytes32 disputeGameFactorySlot = bytes32(uint256(keccak256("systemconfig.disputegamefactory")) - 1);
Storage.setAddress(disputeGameFactorySlot, address(0));
}

/// @notice Returns the minimum L2 gas limit that can be safely set for the system to
Expand Down Expand Up @@ -254,7 +254,8 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {

/// @notice Getter for the DisputeGameFactory address.
function disputeGameFactory() public view returns (address addr_) {
addr_ = Storage.getAddress(DISPUTE_GAME_FACTORY_SLOT);
IOptimismPortal2 portal = IOptimismPortal2(payable(Storage.getAddress(OPTIMISM_PORTAL_SLOT)));
addr_ = address(portal.disputeGameFactory());
}

/// @notice Getter for the OptimismPortal address.
Expand All @@ -273,7 +274,6 @@ contract SystemConfig is OwnableUpgradeable, ReinitializableBase, ISemver {
l1CrossDomainMessenger: l1CrossDomainMessenger(),
l1ERC721Bridge: l1ERC721Bridge(),
l1StandardBridge: l1StandardBridge(),
disputeGameFactory: disputeGameFactory(),
optimismPortal: optimismPortal(),
optimismMintableERC20Factory: optimismMintableERC20Factory()
});
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/test/L1/OPContractsManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ contract OPContractsManager_Upgrade_Harness is CommonTest {

// Make sure that the SystemConfig is upgraded to the right version. It must also have the
// right l2ChainId and must be properly initialized.
assertEq(ISemver(address(systemConfig)).version(), "2.6.0");
assertEq(ISemver(address(systemConfig)).version(), "3.0.0");
assertEq(impls.systemConfigImpl, EIP1967Helper.getImplementation(address(systemConfig)));
assertEq(systemConfig.l2ChainId(), l2ChainId);
DeployUtils.assertInitialized({ _contractAddress: address(systemConfig), _isProxy: true, _slot: 0, _offset: 0 });
Expand Down
Loading