From 396ee8945148b2c97421a6b8cd5d04f799a802be Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 9 Sep 2025 21:04:03 -0400 Subject: [PATCH 01/52] feat: integrate v2 implementations into opcm --- .../contracts-bedrock/interfaces/L1/IOPContractsManager.sol | 2 ++ .../scripts/deploy/DeployImplementations.s.sol | 4 +++- packages/contracts-bedrock/src/L1/OPContractsManager.sol | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol b/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol index adb75fe8c8de7..72fc2a1aaa3eb 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol @@ -216,6 +216,8 @@ interface IOPContractsManager { address anchorStateRegistryImpl; address delayedWETHImpl; address mipsImpl; + address faultDisputeGameV2Impl; + address permissionedDisputeGameV2Impl; } /// @notice The input required to identify a chain for upgrading. diff --git a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol index dced3b7d2c3f4..430c07bc6770d 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol @@ -158,7 +158,9 @@ contract DeployImplementations is Script { disputeGameFactoryImpl: address(_output.disputeGameFactoryImpl), anchorStateRegistryImpl: address(_output.anchorStateRegistryImpl), delayedWETHImpl: address(_output.delayedWETHImpl), - mipsImpl: address(_output.mipsSingleton) + mipsImpl: address(_output.mipsSingleton), + faultDisputeGameV2Impl: address(_output.faultDisputeGameV2Impl), + permissionedDisputeGameV2Impl: address(_output.permissionedDisputeGameV2Impl) }); deployOPCMBPImplsContainer(_input, _output, _blueprints, implementations); diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 424066f1e6fb1..857e004d42e1c 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -1778,6 +1778,8 @@ contract OPContractsManager is ISemver { address anchorStateRegistryImpl; address delayedWETHImpl; address mipsImpl; + address faultDisputeGameV2Impl; + address permissionedDisputeGameV2Impl; } /// @notice The input required to identify a chain for upgrading, along with new prestate hashes From 5ba217842011fe73592f223ed6f4472e298fbe4d Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 10 Sep 2025 17:24:40 -0400 Subject: [PATCH 02/52] fix: skip v2 implementations if not deployed --- .../scripts/deploy/VerifyOPCM.s.sol | 19 +++++++++++++++++ .../test/scripts/VerifyOPCM.t.sol | 21 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 39ae8d5eb02c6..2b95ffbc42ff0 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -398,6 +398,14 @@ contract VerifyOPCM is Script { console.log(string.concat(" Contract: ", _target.name)); console.log(string.concat(" Address: ", vm.toString(_target.addr))); + // Skip verification for V2 implementations based on contract name + // V2 implementations follow the pattern "*DisputeGameV2" or "*DisputeGameV2Impl" + if (_isV2Implementation(_target.name)) { + console.log(" Status: [SKIP] V2 implementation, not yet deployed"); + console.log(string.concat(" Status: [SKIP] Skipping verification for ", _target.name)); + return true; + } + // Build the expected path to the artifact file. string memory artifactPath = _buildArtifactPath(_target.name); console.log(string.concat(" Expected Runtime Artifact: ", artifactPath)); @@ -811,6 +819,17 @@ contract VerifyOPCM is Script { return string(fieldBytes); } + /// @notice Checks if a contract name corresponds to a V2 implementation. + /// @param _contractName The contract name to check. + /// @return True if this is a V2 implementation, false otherwise. + function _isV2Implementation(string memory _contractName) internal pure returns (bool) { + // V2 implementations are specifically the FaultDisputeGameV2 and PermissionedDisputeGameV2 contracts + return ( + keccak256(bytes(_contractName)) == keccak256(bytes("FaultDisputeGameV2")) + || keccak256(bytes(_contractName)) == keccak256(bytes("PermissionedDisputeGameV2")) + ); + } + /// @notice Checks if a position is inside an immutable reference. /// @param _pos The position to check. /// @param _artifact The artifact info. diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index fd64d07b948fa..f40a595250c32 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -78,6 +78,17 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { setupEnvVars(); } + /// @notice Helper function to check if a contract name corresponds to a V2 implementation. + /// @param _contractName The contract name to check. + /// @return True if this is a V2 implementation, false otherwise. + function _isV2Implementation(string memory _contractName) internal pure returns (bool) { + // V2 implementations are specifically the FaultDisputeGameV2 and PermissionedDisputeGameV2 contracts + return ( + keccak256(bytes(_contractName)) == keccak256(bytes("FaultDisputeGameV2")) + || keccak256(bytes(_contractName)) == keccak256(bytes("PermissionedDisputeGameV2")) + ); + } + /// @notice Tests that the script succeeds when no changes are introduced. function test_run_succeeds() public { // Coverage changes bytecode and causes failures, skip. @@ -126,6 +137,11 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { uint256 randomImplIndex = vm.randomUint(0, refs.length - 1); VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; + // Skip V2 implementations (not yet deployed) + if (_isV2Implementation(ref.name)) { + continue; + } + // Get the code for the implementation. bytes memory implCode = ref.addr.code; @@ -186,6 +202,11 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { uint256 randomImplIndex = vm.randomUint(0, refs.length - 1); VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; + // Skip V2 implementations (not yet deployed) + if (_isV2Implementation(ref.name)) { + continue; + } + // Get the code for the implementation. bytes memory implCode = ref.addr.code; From 97b34abacbbceb71e0e16dc37b66bab092a138cd Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 11 Sep 2025 09:37:14 -0400 Subject: [PATCH 03/52] chore: bump semver version --- .../snapshots/abi/OPContractsManager.json | 10 ++++++++++ .../OPContractsManagerContractsContainer.json | 20 +++++++++++++++++++ .../abi/OPContractsManagerDeployer.json | 10 ++++++++++ .../abi/OPContractsManagerGameTypeAdder.json | 10 ++++++++++ .../OPContractsManagerInteropMigrator.json | 10 ++++++++++ .../abi/OPContractsManagerUpgrader.json | 10 ++++++++++ .../OPContractsManagerContractsContainer.json | 2 +- 7 files changed, 71 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json index f4ef1718aa42b..db5c0b7734ded 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json @@ -530,6 +530,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerContractsContainer.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerContractsContainer.json index c49484fc7224c..6b4c6228c2736 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerContractsContainer.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerContractsContainer.json @@ -144,6 +144,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", @@ -340,6 +350,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json index 7cd7a44502c06..85e42e8fc7e0f 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json @@ -428,6 +428,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerGameTypeAdder.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerGameTypeAdder.json index 80b80512aebb9..d9ac76fa83420 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerGameTypeAdder.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerGameTypeAdder.json @@ -321,6 +321,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInteropMigrator.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInteropMigrator.json index b06cd541bb38d..e8d1dec02c225 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInteropMigrator.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInteropMigrator.json @@ -223,6 +223,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerUpgrader.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerUpgrader.json index 512a83ae75cff..a092f31799459 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerUpgrader.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerUpgrader.json @@ -223,6 +223,16 @@ "internalType": "address", "name": "mipsImpl", "type": "address" + }, + { + "internalType": "address", + "name": "faultDisputeGameV2Impl", + "type": "address" + }, + { + "internalType": "address", + "name": "permissionedDisputeGameV2Impl", + "type": "address" } ], "internalType": "struct OPContractsManager.Implementations", diff --git a/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerContractsContainer.json b/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerContractsContainer.json index d87deb94bc76b..2193053869827 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerContractsContainer.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerContractsContainer.json @@ -7,7 +7,7 @@ "type": "struct OPContractsManager.Blueprints" }, { - "bytes": "448", + "bytes": "512", "label": "implementation", "offset": 0, "slot": "13", From 961e8cd605d1e058b3d353184e2e103b5d738093 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 14:09:03 -0400 Subject: [PATCH 04/52] feat: conditionally set the v2 games based on feature flag --- .../interfaces/L1/IOPContractsManager.sol | 5 ++ .../src/L1/OPContractsManager.sol | 71 +++++++++++++++---- .../test/L1/OPContractsManager.t.sol | 43 +++++++++++ 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol b/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol index 72fc2a1aaa3eb..be3b864b94e88 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol @@ -14,6 +14,8 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol"; import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol"; +import { IFaultDisputeGameV2 } from "interfaces/dispute/v2/IFaultDisputeGameV2.sol"; +import { IPermissionedDisputeGameV2 } from "interfaces/dispute/v2/IPermissionedDisputeGameV2.sol"; import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; @@ -177,6 +179,9 @@ interface IOPContractsManager { IPermissionedDisputeGame permissionedDisputeGame; IDelayedWETH delayedWETHPermissionedGameProxy; IDelayedWETH delayedWETHPermissionlessGameProxy; + // V2 dispute game contracts (deployed when DEPLOY_V2_DISPUTE_GAMES flag is set) + IFaultDisputeGameV2 faultDisputeGameV2; + IPermissionedDisputeGameV2 permissionedDisputeGameV2; } /// @notice Addresses of ERC-5202 Blueprint contracts. There are used for deploying full size diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 857e004d42e1c..7492639dacd6e 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -26,6 +26,8 @@ import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol"; import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol"; +import { IFaultDisputeGameV2 } from "interfaces/dispute/v2/IFaultDisputeGameV2.sol"; +import { IPermissionedDisputeGameV2 } from "interfaces/dispute/v2/IPermissionedDisputeGameV2.sol"; import { ISuperFaultDisputeGame } from "interfaces/dispute/ISuperFaultDisputeGame.sol"; import { ISuperPermissionedDisputeGame } from "interfaces/dispute/ISuperPermissionedDisputeGame.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; @@ -541,15 +543,32 @@ contract OPContractsManagerGameTypeAdder is OPContractsManagerBase { ); } - // Deploy the new game type. - outputs[i].faultDisputeGame = IFaultDisputeGame( - Blueprint.deployFrom( - blueprint1, - blueprint2, - computeSalt(l2ChainId, gameConfig.saltMixer, gameContractName), - constructorData - ) - ); + // Deploy the new game type or use v2 implementation + if ( + isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES) + && gameConfig.disputeGameType.raw() == GameTypes.CANNON.raw() + && address(getImplementations().faultDisputeGameV2Impl) != address(0) + ) { + // Use v2 FaultDisputeGame implementation + outputs[i].faultDisputeGame = IFaultDisputeGame(getImplementations().faultDisputeGameV2Impl); + } else if ( + isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES) + && gameConfig.disputeGameType.raw() == GameTypes.PERMISSIONED_CANNON.raw() + && address(getImplementations().permissionedDisputeGameV2Impl) != address(0) + ) { + // Use v2 PermissionedDisputeGame implementation + outputs[i].faultDisputeGame = IFaultDisputeGame(getImplementations().permissionedDisputeGameV2Impl); + } else { + // Deploy v1 game using blueprint + outputs[i].faultDisputeGame = IFaultDisputeGame( + Blueprint.deployFrom( + blueprint1, + blueprint2, + computeSalt(l2ChainId, gameConfig.saltMixer, gameContractName), + constructorData + ) + ); + } // As a last step, register the new game type with the DisputeGameFactory. If the game // type already exists, then its implementation will be overwritten. @@ -1075,6 +1094,13 @@ contract OPContractsManagerDeployer is OPContractsManagerBase { ) ); + // Store v2 implementation addresses if the feature flag is enabled + if (isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)) { + // Store v2 implementation addresses for later registration with DisputeGameFactory + output.permissionedDisputeGameV2 = IPermissionedDisputeGameV2(implementation.permissionedDisputeGameV2Impl); + output.faultDisputeGameV2 = IFaultDisputeGameV2(implementation.faultDisputeGameV2Impl); + } + // -------- Set and Initialize Proxy Implementations -------- bytes memory data; @@ -1159,11 +1185,25 @@ contract OPContractsManagerDeployer is OPContractsManagerBase { implementation.disputeGameFactoryImpl, data ); - setDGFImplementation( - output.disputeGameFactoryProxy, - GameTypes.PERMISSIONED_CANNON, - IDisputeGame(address(output.permissionedDisputeGame)) - ); + // Register the appropriate dispute game implementation based on the feature flag + if ( + isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES) + && address(output.permissionedDisputeGameV2) != address(0) + ) { + // Register v2 implementation for PERMISSIONED_CANNON game type + setDGFImplementation( + output.disputeGameFactoryProxy, + GameTypes.PERMISSIONED_CANNON, + IDisputeGame(address(output.permissionedDisputeGameV2)) + ); + } else { + // Register v1 implementation for PERMISSIONED_CANNON game type + setDGFImplementation( + output.disputeGameFactoryProxy, + GameTypes.PERMISSIONED_CANNON, + IDisputeGame(address(output.permissionedDisputeGame)) + ); + } transferOwnership(address(output.disputeGameFactoryProxy), address(_input.roles.opChainProxyAdminOwner)); @@ -1739,6 +1779,9 @@ contract OPContractsManager is ISemver { IPermissionedDisputeGame permissionedDisputeGame; IDelayedWETH delayedWETHPermissionedGameProxy; IDelayedWETH delayedWETHPermissionlessGameProxy; + // V2 dispute game contracts (deployed when DEPLOY_V2_DISPUTE_GAMES flag is set) + IFaultDisputeGameV2 faultDisputeGameV2; + IPermissionedDisputeGameV2 permissionedDisputeGameV2; } /// @notice Addresses of ERC-5202 Blueprint contracts. There are used for deploying full size diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index eeeff41eaab90..7e211d5772cde 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -29,6 +29,8 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol"; import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol"; +import { IFaultDisputeGameV2 } from "interfaces/dispute/v2/IFaultDisputeGameV2.sol"; +import { IPermissionedDisputeGameV2 } from "interfaces/dispute/v2/IPermissionedDisputeGameV2.sol"; import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol"; import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; @@ -1785,3 +1787,44 @@ contract OPContractsManager_Version_Test is OPContractsManager_TestInit { assertNotEq(abi.encode(prestateUpdater.version()), abi.encode(0)); } } + +/// @title OPContractsManager_V2_Test +/// @notice Tests for v2 dispute game implementations in OPContractsManager +contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { + + /// @notice Test that deploy without v2 flag doesn't set v2 implementations + function test_deploy_withoutV2Flag_noV2Implementations() public { + // Convert DOI to OPCM input and deploy + IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); + vm.prank(address(this)); + IOPContractsManager.DeployOutput memory output = opcm.deploy(opcmInput); + + // Check that v2 implementations are not set (since flag is not enabled by default) + assertEq(address(output.permissionedDisputeGameV2), address(0)); + assertEq(address(output.faultDisputeGameV2), address(0)); + + // Check that v1 implementation is registered for PERMISSIONED_CANNON + address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); + assertEq(registeredImpl, address(output.permissionedDisputeGame)); + } + + /// @notice Test that deploy with v2 flag would set v2 implementations + function test_deploy_withV2Flag_concept() public { + // This test demonstrates the v2 flag concept + // In a real deployment with v2 flag enabled: + // 1. The OPContractsManagerContractsContainer would be created with DEPLOY_V2_DISPUTE_GAMES flag + // 2. The v2 implementation addresses would be non-zero + // 3. The deploy function would register v2 implementations with DisputeGameFactory + + // Get the current implementations from OPCM (these would be v2 in a v2-enabled deployment) + IOPContractsManager.Implementations memory impls = opcm.implementations(); + + // In a v2-enabled deployment, these would be non-zero addresses + // For now, we just verify they are zero since v2 flag is not enabled + assertEq(address(impls.permissionedDisputeGameV2Impl), address(0)); + assertEq(address(impls.faultDisputeGameV2Impl), address(0)); + + // Test that we can check if v2 flag is enabled (it shouldn't be in default setup) + assertFalse(opcm.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)); + } +} From 92870137534caf231d76180af6ccb77e5f4c2c8f Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 14:36:04 -0400 Subject: [PATCH 05/52] feat: add override for setup to re-deploy opcm with feature flag --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 7e211d5772cde..ff9f788911b53 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1696,7 +1696,7 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { event Deployed(uint256 indexed l2ChainId, address indexed deployer, bytes deployOutput); - function setUp() public override { + function setUp() public virtual override { DeployOPChain_TestBase.setUp(); doi.set(doi.opChainProxyAdminOwner.selector, opChainProxyAdminOwner); @@ -1723,6 +1723,7 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { // to the input struct type defined in OPContractsManager.sol. function toOPCMDeployInput(DeployOPChainInput _doi) internal + virtual view returns (IOPContractsManager.DeployInput memory) { From ef29b42c0d7ce48048d7bce8d25165004e689235 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 14:55:54 -0400 Subject: [PATCH 06/52] test: improve test by deploying with the feature flag and asserting on output --- .../test/L1/OPContractsManager.t.sol | 81 +++++++++++++++---- 1 file changed, 67 insertions(+), 14 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index ff9f788911b53..da2c7dd17ecd7 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -14,6 +14,10 @@ import { DeployUtils } from "scripts/libraries/DeployUtils.sol"; import { Deploy } from "scripts/deploy/Deploy.s.sol"; import { VerifyOPCM } from "scripts/deploy/VerifyOPCM.s.sol"; import { Config } from "scripts/libraries/Config.sol"; +import { DeployImplementations } from "scripts/deploy/DeployImplementations.s.sol"; +import { DeploySuperchain } from "scripts/deploy/DeploySuperchain.s.sol"; +import { StandardConstants } from "scripts/deploy/StandardConstants.sol"; +import { ProtocolVersion } from "src/L1/ProtocolVersions.sol"; // Libraries import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; @@ -1793,12 +1797,55 @@ contract OPContractsManager_Version_Test is OPContractsManager_TestInit { /// @notice Tests for v2 dispute game implementations in OPContractsManager contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { + /// @notice Helper function to deploy OPCM with v2 flag enabled + function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { + // Deploy Superchain contracts first + DeploySuperchain deploySuperchain = new DeploySuperchain(); + DeploySuperchain.Output memory dso = deploySuperchain.run( + DeploySuperchain.Input({ + superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), + protocolVersionsOwner: makeAddr("protocolVersionsOwner"), + guardian: makeAddr("guardian"), + paused: false, + requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), + recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) + }) + ); + + // Deploy implementations with v2 flag enabled + DeployImplementations deployImplementations = new DeployImplementations(); + DeployImplementations.Output memory dio = deployImplementations.run( + DeployImplementations.Input({ + withdrawalDelaySeconds: 100, + minProposalSizeBytes: 200, + challengePeriodSeconds: 300, + proofMaturityDelaySeconds: 400, + disputeGameFinalityDelaySeconds: 500, + mipsVersion: StandardConstants.MIPS_VERSION, + faultGameV2MaxGameDepth: 73, + faultGameV2SplitDepth: 30, + faultGameV2ClockExtension: 10800, + faultGameV2MaxClockDuration: 302400, + superchainConfigProxy: dso.superchainConfigProxy, + protocolVersionsProxy: dso.protocolVersionsProxy, + superchainProxyAdmin: dso.superchainProxyAdmin, + upgradeController: dso.superchainProxyAdmin.owner(), + proposer: proposer, + challenger: challenger, + devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here + }) + ); + + return dio.opcm; + } + /// @notice Test that deploy without v2 flag doesn't set v2 implementations function test_deploy_withoutV2Flag_noV2Implementations() public { // Convert DOI to OPCM input and deploy IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); - vm.prank(address(this)); + vm.startPrank(address(this)); IOPContractsManager.DeployOutput memory output = opcm.deploy(opcmInput); + vm.stopPrank(); // Check that v2 implementations are not set (since flag is not enabled by default) assertEq(address(output.permissionedDisputeGameV2), address(0)); @@ -1811,21 +1858,27 @@ contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { /// @notice Test that deploy with v2 flag would set v2 implementations function test_deploy_withV2Flag_concept() public { - // This test demonstrates the v2 flag concept - // In a real deployment with v2 flag enabled: - // 1. The OPContractsManagerContractsContainer would be created with DEPLOY_V2_DISPUTE_GAMES flag - // 2. The v2 implementation addresses would be non-zero - // 3. The deploy function would register v2 implementations with DisputeGameFactory + IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); - // Get the current implementations from OPCM (these would be v2 in a v2-enabled deployment) - IOPContractsManager.Implementations memory impls = opcm.implementations(); + assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); + + IOPContractsManager.Implementations memory impls = opcmV2.implementations(); + + // Verify that v2 implementations are set (non-zero) + assertTrue(address(impls.permissionedDisputeGameV2Impl) != address(0), "PermissionedDisputeGameV2 implementation should be non-zero"); + assertTrue(address(impls.faultDisputeGameV2Impl) != address(0), "FaultDisputeGameV2 implementation should be non-zero"); + + // Set up deploy input for the v2-enabled OPCM + doi.set(doi.opcm.selector, address(opcmV2)); + IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); + + vm.startPrank(address(this)); + IOPContractsManager.DeployOutput memory output = opcmV2.deploy(opcmInput); + vm.stopPrank(); - // In a v2-enabled deployment, these would be non-zero addresses - // For now, we just verify they are zero since v2 flag is not enabled - assertEq(address(impls.permissionedDisputeGameV2Impl), address(0)); - assertEq(address(impls.faultDisputeGameV2Impl), address(0)); + assertTrue(address(output.permissionedDisputeGameV2) != address(0), "PermissionedDisputeGameV2 should be deployed"); + assertTrue(address(output.faultDisputeGameV2) != address(0), "FaultDisputeGameV2 should be deployed"); - // Test that we can check if v2 flag is enabled (it shouldn't be in default setup) - assertFalse(opcm.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)); + assertTrue(address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 is still deployed, but not registered"); } } From 249ba99a697076e5a9ba404af94acba99a06dbd3 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 14:59:38 -0400 Subject: [PATCH 07/52] test: add assertion that the deployment configuration on the dgf is correct --- .../contracts-bedrock/test/L1/OPContractsManager.t.sol | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index da2c7dd17ecd7..74f026811fa11 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1876,9 +1876,16 @@ contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { IOPContractsManager.DeployOutput memory output = opcmV2.deploy(opcmInput); vm.stopPrank(); + // Verify that v2 dispute game contracts are deployed and non-zero assertTrue(address(output.permissionedDisputeGameV2) != address(0), "PermissionedDisputeGameV2 should be deployed"); assertTrue(address(output.faultDisputeGameV2) != address(0), "FaultDisputeGameV2 should be deployed"); - assertTrue(address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 is still deployed, but not registered"); + // Verify that v1 permissioned dispute game is still deployed (for backward compatibility) + assertTrue(address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 should still be deployed"); + + // Verify that the DisputeGameFactory has registered the v2 implementation for PERMISSIONED_CANNON game type + address registeredPermissionedImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); + assertEq(registeredPermissionedImpl, address(output.permissionedDisputeGameV2), + "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON"); } } From e3584b76b77aa09f87b63fec67fac5768291d663 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 15:08:08 -0400 Subject: [PATCH 08/52] chore: improve test name --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 74f026811fa11..e1b00226187e5 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1857,7 +1857,7 @@ contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { } /// @notice Test that deploy with v2 flag would set v2 implementations - function test_deploy_withV2Flag_concept() public { + function test_deploy_withV2Flag_setsV2Implementations() public { IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); From 3022729a25f98568fc32abd1da1b414184a83b30 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 16:29:19 -0400 Subject: [PATCH 09/52] refactor: add helper function for feature toggled deployment of opcm --- .../test/L1/OPContractsManager.t.sol | 117 ++++++++---------- 1 file changed, 55 insertions(+), 62 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index e1b00226187e5..f79bd42de0223 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1687,15 +1687,7 @@ contract OPContractsManager_Migrate_CannonKonaEnabled_Test is OPContractsManager } } -/// @title OPContractsManager_Deploy_Test -/// @notice Tests the `deploy` function of the `OPContractsManager` contract. -/// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This -/// is because OPContractsManager acts as a deploy script, so we start from a clean slate here -/// and work OPContractsManager's deployment into the existing test setup, instead of using -/// the existing test setup to deploy OPContractsManager. We do however inherit from -/// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly -/// to how a real deployment would happen. -contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { +contract OPContractsManager_Init is DeployOPChain_TestBase { using stdStorage for StdStorage; event Deployed(uint256 indexed l2ChainId, address indexed deployer, bytes deployOutput); @@ -1727,7 +1719,6 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { // to the input struct type defined in OPContractsManager.sol. function toOPCMDeployInput(DeployOPChainInput _doi) internal - virtual view returns (IOPContractsManager.DeployInput memory) { @@ -1755,48 +1746,6 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { }); } - function test_deploy_l2ChainIdEqualsZero_reverts() public { - IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); - deployInput.l2ChainId = 0; - vm.expectRevert(IOPContractsManager.InvalidChainId.selector); - opcm.deploy(deployInput); - } - - function test_deploy_l2ChainIdEqualsCurrentChainId_reverts() public { - IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); - deployInput.l2ChainId = block.chainid; - - vm.expectRevert(IOPContractsManager.InvalidChainId.selector); - opcm.deploy(deployInput); - } - - function test_deploy_succeeds() public { - vm.expectEmit(true, true, true, false); // TODO precompute the expected `deployOutput`. - emit Deployed(doi.l2ChainId(), address(this), bytes("")); - opcm.deploy(toOPCMDeployInput(doi)); - } -} - -/// @title OPContractsManager_Version_Test -/// @notice Tests the `version` function of the `OPContractsManager` contract. -contract OPContractsManager_Version_Test is OPContractsManager_TestInit { - IOPContractsManager internal prestateUpdater; - OPContractsManager.AddGameInput[] internal gameInput; - - function setUp() public override { - super.setUp(); - prestateUpdater = opcm; - } - - function test_semver_works() public view { - assertNotEq(abi.encode(prestateUpdater.version()), abi.encode(0)); - } -} - -/// @title OPContractsManager_V2_Test -/// @notice Tests for v2 dispute game implementations in OPContractsManager -contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { - /// @notice Helper function to deploy OPCM with v2 flag enabled function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { // Deploy Superchain contracts first @@ -1832,12 +1781,43 @@ contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { upgradeController: dso.superchainProxyAdmin.owner(), proposer: proposer, challenger: challenger, - devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here - }) + devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here + }) ); return dio.opcm; } +} + +/// @title OPContractsManager_Deploy_Test +/// @notice Tests the `deploy` function of the `OPContractsManager` contract. +/// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This +/// is because OPContractsManager acts as a deploy script, so we start from a clean slate here +/// and work OPContractsManager's deployment into the existing test setup, instead of using +/// the existing test setup to deploy OPContractsManager. We do however inherit from +/// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly +/// to how a real deployment would happen. +contract OPContractsManager_Deploy_Test is OPContractsManager_Init { + function test_deploy_l2ChainIdEqualsZero_reverts() public { + IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); + deployInput.l2ChainId = 0; + vm.expectRevert(IOPContractsManager.InvalidChainId.selector); + opcm.deploy(deployInput); + } + + function test_deploy_l2ChainIdEqualsCurrentChainId_reverts() public { + IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); + deployInput.l2ChainId = block.chainid; + + vm.expectRevert(IOPContractsManager.InvalidChainId.selector); + opcm.deploy(deployInput); + } + + function test_deploy_succeeds() public { + vm.expectEmit(true, true, true, false); // TODO precompute the expected `deployOutput`. + emit Deployed(doi.l2ChainId(), address(this), bytes("")); + opcm.deploy(toOPCMDeployInput(doi)); + } /// @notice Test that deploy without v2 flag doesn't set v2 implementations function test_deploy_withoutV2Flag_noV2Implementations() public { @@ -1865,8 +1845,13 @@ contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { IOPContractsManager.Implementations memory impls = opcmV2.implementations(); // Verify that v2 implementations are set (non-zero) - assertTrue(address(impls.permissionedDisputeGameV2Impl) != address(0), "PermissionedDisputeGameV2 implementation should be non-zero"); - assertTrue(address(impls.faultDisputeGameV2Impl) != address(0), "FaultDisputeGameV2 implementation should be non-zero"); + assertTrue( + address(impls.permissionedDisputeGameV2Impl) != address(0), + "PermissionedDisputeGameV2 implementation should be non-zero" + ); + assertTrue( + address(impls.faultDisputeGameV2Impl) != address(0), "FaultDisputeGameV2 implementation should be non-zero" + ); // Set up deploy input for the v2-enabled OPCM doi.set(doi.opcm.selector, address(opcmV2)); @@ -1877,15 +1862,23 @@ contract OPContractsManager_V2_Test is OPContractsManager_Deploy_Test { vm.stopPrank(); // Verify that v2 dispute game contracts are deployed and non-zero - assertTrue(address(output.permissionedDisputeGameV2) != address(0), "PermissionedDisputeGameV2 should be deployed"); + assertTrue( + address(output.permissionedDisputeGameV2) != address(0), "PermissionedDisputeGameV2 should be deployed" + ); assertTrue(address(output.faultDisputeGameV2) != address(0), "FaultDisputeGameV2 should be deployed"); // Verify that v1 permissioned dispute game is still deployed (for backward compatibility) - assertTrue(address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 should still be deployed"); + assertTrue( + address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 should still be deployed" + ); // Verify that the DisputeGameFactory has registered the v2 implementation for PERMISSIONED_CANNON game type - address registeredPermissionedImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); - assertEq(registeredPermissionedImpl, address(output.permissionedDisputeGameV2), - "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON"); + address registeredPermissionedImpl = + address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); + assertEq( + registeredPermissionedImpl, + address(output.permissionedDisputeGameV2), + "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" + ); } -} +} \ No newline at end of file From 78656fd1dc65c284458ddf46d98cd7782f23530c Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 15 Sep 2025 20:06:16 -0400 Subject: [PATCH 10/52] fix: bytecode size check failing --- packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 2b95ffbc42ff0..647cc073ff471 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -896,6 +896,12 @@ contract VerifyOPCM is Script { sourceName = sourceNameOverrides[_contractName]; } + // Check if the dispute profile artifact exists and should be used. + string memory disputePath = string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); + if (vm.exists(disputePath)) { + return disputePath; + } + // Return computed path, relative to the contracts-bedrock directory. return string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".json"); } From 3749303f60a91bb8069eefd1796feb1a2d14be3c Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 16 Sep 2025 08:13:56 -0400 Subject: [PATCH 11/52] chore: run forge fmt --- packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol | 3 ++- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 647cc073ff471..e60d97816ec20 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -897,7 +897,8 @@ contract VerifyOPCM is Script { } // Check if the dispute profile artifact exists and should be used. - string memory disputePath = string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); + string memory disputePath = + string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); if (vm.exists(disputePath)) { return disputePath; } diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index f79bd42de0223..1eebe94396b28 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1881,4 +1881,4 @@ contract OPContractsManager_Deploy_Test is OPContractsManager_Init { "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" ); } -} \ No newline at end of file +} From ddd42de55732172578184747ba24940cc2c0abbd Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 16 Sep 2025 09:05:44 -0400 Subject: [PATCH 12/52] fix: address CI naming and semver bump --- .../snapshots/abi/OPContractsManager.json | 10 ++++++++++ .../snapshots/abi/OPContractsManagerDeployer.json | 10 ++++++++++ .../contracts-bedrock/test/L1/OPContractsManager.t.sol | 10 ++++------ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json index db5c0b7734ded..7af882bfa8638 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManager.json @@ -432,6 +432,16 @@ "internalType": "contract IDelayedWETH", "name": "delayedWETHPermissionlessGameProxy", "type": "address" + }, + { + "internalType": "contract IFaultDisputeGameV2", + "name": "faultDisputeGameV2", + "type": "address" + }, + { + "internalType": "contract IPermissionedDisputeGameV2", + "name": "permissionedDisputeGameV2", + "type": "address" } ], "internalType": "struct OPContractsManager.DeployOutput", diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json index 85e42e8fc7e0f..28e694d5e4f5f 100644 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json +++ b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerDeployer.json @@ -330,6 +330,16 @@ "internalType": "contract IDelayedWETH", "name": "delayedWETHPermissionlessGameProxy", "type": "address" + }, + { + "internalType": "contract IFaultDisputeGameV2", + "name": "faultDisputeGameV2", + "type": "address" + }, + { + "internalType": "contract IPermissionedDisputeGameV2", + "name": "permissionedDisputeGameV2", + "type": "address" } ], "internalType": "struct OPContractsManager.DeployOutput", diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 1eebe94396b28..57146dbaee274 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -33,8 +33,6 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol"; import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol"; -import { IFaultDisputeGameV2 } from "interfaces/dispute/v2/IFaultDisputeGameV2.sol"; -import { IPermissionedDisputeGameV2 } from "interfaces/dispute/v2/IPermissionedDisputeGameV2.sol"; import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol"; import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; @@ -1687,7 +1685,7 @@ contract OPContractsManager_Migrate_CannonKonaEnabled_Test is OPContractsManager } } -contract OPContractsManager_Init is DeployOPChain_TestBase { +contract OPContractsManager_DeployBase is DeployOPChain_TestBase { using stdStorage for StdStorage; event Deployed(uint256 indexed l2ChainId, address indexed deployer, bytes deployOutput); @@ -1797,7 +1795,7 @@ contract OPContractsManager_Init is DeployOPChain_TestBase { /// the existing test setup to deploy OPContractsManager. We do however inherit from /// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly /// to how a real deployment would happen. -contract OPContractsManager_Deploy_Test is OPContractsManager_Init { +contract OPContractsManager_Deploy_Test is OPContractsManager_DeployBase { function test_deploy_l2ChainIdEqualsZero_reverts() public { IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); deployInput.l2ChainId = 0; @@ -1820,7 +1818,7 @@ contract OPContractsManager_Deploy_Test is OPContractsManager_Init { } /// @notice Test that deploy without v2 flag doesn't set v2 implementations - function test_deploy_withoutV2Flag_noV2Implementations() public { + function test_deployWithoutV2Flag_succeeds() public { // Convert DOI to OPCM input and deploy IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); vm.startPrank(address(this)); @@ -1837,7 +1835,7 @@ contract OPContractsManager_Deploy_Test is OPContractsManager_Init { } /// @notice Test that deploy with v2 flag would set v2 implementations - function test_deploy_withV2Flag_setsV2Implementations() public { + function test_deployWithV2Flag_succeeds() public { IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); From 015c220e6c7a52757b1cbb2f8dd0092a37243b1e Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 16 Sep 2025 11:35:22 -0400 Subject: [PATCH 13/52] fix: rename setup contract name to align with ci checks --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 57146dbaee274..349cc04699e18 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1685,7 +1685,7 @@ contract OPContractsManager_Migrate_CannonKonaEnabled_Test is OPContractsManager } } -contract OPContractsManager_DeployBase is DeployOPChain_TestBase { +contract OPContractsManager_Deploy_TestInit is DeployOPChain_TestBase { using stdStorage for StdStorage; event Deployed(uint256 indexed l2ChainId, address indexed deployer, bytes deployOutput); @@ -1795,7 +1795,7 @@ contract OPContractsManager_DeployBase is DeployOPChain_TestBase { /// the existing test setup to deploy OPContractsManager. We do however inherit from /// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly /// to how a real deployment would happen. -contract OPContractsManager_Deploy_Test is OPContractsManager_DeployBase { +contract OPContractsManager_Deploy_Test is OPContractsManager_Deploy_TestInit { function test_deploy_l2ChainIdEqualsZero_reverts() public { IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); deployInput.l2ChainId = 0; From 71374afc6a6f8947f3fffb7f0e91b3b764a0950a Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 13:17:55 -0400 Subject: [PATCH 14/52] chore: comment out added fix --- .../scripts/deploy/VerifyOPCM.s.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index e60d97816ec20..1d598b933dcc5 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -896,12 +896,12 @@ contract VerifyOPCM is Script { sourceName = sourceNameOverrides[_contractName]; } - // Check if the dispute profile artifact exists and should be used. - string memory disputePath = - string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); - if (vm.exists(disputePath)) { - return disputePath; - } + // // Check if the dispute profile artifact exists and should be used. + // string memory disputePath = + // string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); + // if (vm.exists(disputePath)) { + // return disputePath; + // } // Return computed path, relative to the contracts-bedrock directory. return string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".json"); From d491a4bd043d574294ee99f06307bc02caec7ac6 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 13:50:19 -0400 Subject: [PATCH 15/52] fix: verify opcm --- .../scripts/deploy/VerifyOPCM.s.sol | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 1d598b933dcc5..adcb1fcf3c596 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -113,6 +113,8 @@ contract VerifyOPCM is Script { fieldNameOverrides["optimismPortalInteropImpl"] = "OptimismPortalInterop"; fieldNameOverrides["mipsImpl"] = "MIPS64"; fieldNameOverrides["ethLockboxImpl"] = "ETHLockbox"; + fieldNameOverrides["faultDisputeGameV2Impl"] = "FaultDisputeGameV2"; + fieldNameOverrides["permissionedDisputeGameV2Impl"] = "PermissionedDisputeGameV2"; fieldNameOverrides["permissionlessDisputeGame1"] = "FaultDisputeGame"; fieldNameOverrides["permissionlessDisputeGame2"] = "FaultDisputeGame"; fieldNameOverrides["permissionedDisputeGame1"] = "PermissionedDisputeGame"; @@ -896,12 +898,12 @@ contract VerifyOPCM is Script { sourceName = sourceNameOverrides[_contractName]; } - // // Check if the dispute profile artifact exists and should be used. - // string memory disputePath = - // string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); - // if (vm.exists(disputePath)) { - // return disputePath; - // } + // Check if the dispute profile artifact exists and should be used. + string memory disputePath = + string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); + if (vm.exists(disputePath)) { + return disputePath; + } // Return computed path, relative to the contracts-bedrock directory. return string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".json"); From 2ebc280a15f1b908f900941aeea109047aec78e6 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 14:12:23 -0400 Subject: [PATCH 16/52] fix: naming convention for test contracts --- .../test/L1/OPContractsManager.t.sol | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 349cc04699e18..b67968c928dc5 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1685,7 +1685,15 @@ contract OPContractsManager_Migrate_CannonKonaEnabled_Test is OPContractsManager } } -contract OPContractsManager_Deploy_TestInit is DeployOPChain_TestBase { +/// @title OPContractsManager_Deploy_Test +/// @notice Tests the `deploy` function of the `OPContractsManager` contract. +/// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This +/// is because OPContractsManager acts as a deploy script, so we start from a clean slate here +/// and work OPContractsManager's deployment into the existing test setup, instead of using +/// the existing test setup to deploy OPContractsManager. We do however inherit from +/// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly +/// to how a real deployment would happen. +contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { using stdStorage for StdStorage; event Deployed(uint256 indexed l2ChainId, address indexed deployer, bytes deployOutput); @@ -1785,17 +1793,7 @@ contract OPContractsManager_Deploy_TestInit is DeployOPChain_TestBase { return dio.opcm; } -} -/// @title OPContractsManager_Deploy_Test -/// @notice Tests the `deploy` function of the `OPContractsManager` contract. -/// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This -/// is because OPContractsManager acts as a deploy script, so we start from a clean slate here -/// and work OPContractsManager's deployment into the existing test setup, instead of using -/// the existing test setup to deploy OPContractsManager. We do however inherit from -/// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly -/// to how a real deployment would happen. -contract OPContractsManager_Deploy_Test is OPContractsManager_Deploy_TestInit { function test_deploy_l2ChainIdEqualsZero_reverts() public { IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); deployInput.l2ChainId = 0; From 67194d8d97c6a7860e95abb6fb50a31dd7ba9f7f Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 14:34:50 -0400 Subject: [PATCH 17/52] fix: move the updates for addGameType to the addGameType pr --- .../src/L1/OPContractsManager.sol | 35 +++++-------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 7492639dacd6e..132a61638eca1 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -543,32 +543,15 @@ contract OPContractsManagerGameTypeAdder is OPContractsManagerBase { ); } - // Deploy the new game type or use v2 implementation - if ( - isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES) - && gameConfig.disputeGameType.raw() == GameTypes.CANNON.raw() - && address(getImplementations().faultDisputeGameV2Impl) != address(0) - ) { - // Use v2 FaultDisputeGame implementation - outputs[i].faultDisputeGame = IFaultDisputeGame(getImplementations().faultDisputeGameV2Impl); - } else if ( - isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES) - && gameConfig.disputeGameType.raw() == GameTypes.PERMISSIONED_CANNON.raw() - && address(getImplementations().permissionedDisputeGameV2Impl) != address(0) - ) { - // Use v2 PermissionedDisputeGame implementation - outputs[i].faultDisputeGame = IFaultDisputeGame(getImplementations().permissionedDisputeGameV2Impl); - } else { - // Deploy v1 game using blueprint - outputs[i].faultDisputeGame = IFaultDisputeGame( - Blueprint.deployFrom( - blueprint1, - blueprint2, - computeSalt(l2ChainId, gameConfig.saltMixer, gameContractName), - constructorData - ) - ); - } + // Deploy the new game type. + outputs[i].faultDisputeGame = IFaultDisputeGame( + Blueprint.deployFrom( + blueprint1, + blueprint2, + computeSalt(l2ChainId, gameConfig.saltMixer, gameContractName), + constructorData + ) + ); // As a last step, register the new game type with the DisputeGameFactory. If the game // type already exists, then its implementation will be overwritten. From 3b769e000d4277a91612438439d8c594d4e49003 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 24 Sep 2025 14:36:08 -0400 Subject: [PATCH 18/52] fix: proposer removed --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index b67968c928dc5..a57b4a1dcf1d3 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1785,7 +1785,6 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { protocolVersionsProxy: dso.protocolVersionsProxy, superchainProxyAdmin: dso.superchainProxyAdmin, upgradeController: dso.superchainProxyAdmin.owner(), - proposer: proposer, challenger: challenger, devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here }) From 8734e9d6aad5a2b396c8bbd8ba8bbfabf04af326 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 24 Sep 2025 15:03:33 -0400 Subject: [PATCH 19/52] fix: rename function for clarity in VerifyOPCM script Rename _isV2Implementation to _isV2DisputeGameImplementation for better clarity about what types of contracts this function checks for. --- .../contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol | 8 ++++---- packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index adcb1fcf3c596..3632546bc928f 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -402,7 +402,7 @@ contract VerifyOPCM is Script { // Skip verification for V2 implementations based on contract name // V2 implementations follow the pattern "*DisputeGameV2" or "*DisputeGameV2Impl" - if (_isV2Implementation(_target.name)) { + if (_isV2DisputeGameImplementation(_target.name)) { console.log(" Status: [SKIP] V2 implementation, not yet deployed"); console.log(string.concat(" Status: [SKIP] Skipping verification for ", _target.name)); return true; @@ -821,10 +821,10 @@ contract VerifyOPCM is Script { return string(fieldBytes); } - /// @notice Checks if a contract name corresponds to a V2 implementation. + /// @notice Checks if a contract name corresponds to a V2 dispute game implementation. /// @param _contractName The contract name to check. - /// @return True if this is a V2 implementation, false otherwise. - function _isV2Implementation(string memory _contractName) internal pure returns (bool) { + /// @return True if this is a V2 dispute game implementation, false otherwise. + function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { // V2 implementations are specifically the FaultDisputeGameV2 and PermissionedDisputeGameV2 contracts return ( keccak256(bytes(_contractName)) == keccak256(bytes("FaultDisputeGameV2")) diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index f40a595250c32..4019098adb265 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -81,7 +81,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { /// @notice Helper function to check if a contract name corresponds to a V2 implementation. /// @param _contractName The contract name to check. /// @return True if this is a V2 implementation, false otherwise. - function _isV2Implementation(string memory _contractName) internal pure returns (bool) { + function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { // V2 implementations are specifically the FaultDisputeGameV2 and PermissionedDisputeGameV2 contracts return ( keccak256(bytes(_contractName)) == keccak256(bytes("FaultDisputeGameV2")) @@ -138,7 +138,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 implementations (not yet deployed) - if (_isV2Implementation(ref.name)) { + if (_isV2DisputeGameImplementation(ref.name)) { continue; } @@ -203,7 +203,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 implementations (not yet deployed) - if (_isV2Implementation(ref.name)) { + if (_isV2DisputeGameImplementation(ref.name)) { continue; } From 1fb7ca5362720135ebc8b78c70df5e74a7bba6de Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 24 Sep 2025 15:52:50 -0400 Subject: [PATCH 20/52] fix: add proper gameArgs for V2 dispute game registration and fix setting output implementation on outupt struct --- .../src/L1/OPContractsManager.sol | 99 +++++++++++-------- 1 file changed, 59 insertions(+), 40 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 132a61638eca1..441e0a703b0b8 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -364,8 +364,23 @@ abstract contract OPContractsManagerBase { } /// @notice Sets a game implementation on the dispute game factory - function setDGFImplementation(IDisputeGameFactory _dgf, GameType _gameType, IDisputeGame _newGame) internal { - _dgf.setImplementation(_gameType, _newGame); + /// @param _dgf The dispute game factory + /// @param _gameType The game type + /// @param _newGame The new game implementation + /// @param _gameArgs Optional game arguments. If empty, calls without gameArgs + function setDGFImplementation( + IDisputeGameFactory _dgf, + GameType _gameType, + IDisputeGame _newGame, + bytes memory _gameArgs + ) + internal + { + if (_gameArgs.length > 0) { + _dgf.setImplementation(_gameType, _newGame, _gameArgs); + } else { + _dgf.setImplementation(_gameType, _newGame); + } } } @@ -555,7 +570,9 @@ contract OPContractsManagerGameTypeAdder is OPContractsManagerBase { // As a last step, register the new game type with the DisputeGameFactory. If the game // type already exists, then its implementation will be overwritten. - setDGFImplementation(dgf, gameConfig.disputeGameType, IDisputeGame(address(outputs[i].faultDisputeGame))); + setDGFImplementation( + dgf, gameConfig.disputeGameType, IDisputeGame(address(outputs[i].faultDisputeGame)), bytes("") + ); dgf.setInitBond(gameConfig.disputeGameType, gameConfig.initialBond); // Emit event for the newly added game type with the new and old implementations. @@ -942,7 +959,7 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase { IDisputeGameFactory dgf = IDisputeGameFactory(_opChainConfig.systemConfigProxy.disputeGameFactory()); // Set the new implementation. - setDGFImplementation(dgf, _gameType, IDisputeGame(newGame)); + setDGFImplementation(dgf, _gameType, IDisputeGame(newGame), bytes("")); } } @@ -1052,36 +1069,30 @@ contract OPContractsManagerDeployer is OPContractsManagerBase { ) ); - // While not a proxy, we deploy the PermissionedDisputeGame here as well because it's bespoke per chain. - output.permissionedDisputeGame = IPermissionedDisputeGame( - Blueprint.deployFrom( - blueprint.permissionedDisputeGame1, - blueprint.permissionedDisputeGame2, - computeSalt(_input.l2ChainId, _input.saltMixer, "PermissionedDisputeGame"), - encodePermissionedFDGConstructor( - IFaultDisputeGame.GameConstructorParams({ - gameType: GameTypes.PERMISSIONED_CANNON, - absolutePrestate: _input.disputeAbsolutePrestate, - maxGameDepth: _input.disputeMaxGameDepth, - splitDepth: _input.disputeSplitDepth, - clockExtension: _input.disputeClockExtension, - maxClockDuration: _input.disputeMaxClockDuration, - vm: IBigStepper(implementation.mipsImpl), - weth: IDelayedWETH(payable(address(output.delayedWETHPermissionedGameProxy))), - anchorStateRegistry: IAnchorStateRegistry(address(output.anchorStateRegistryProxy)), - l2ChainId: _input.l2ChainId - }), - _input.roles.proposer, - _input.roles.challenger + if (!isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)) { + output.permissionedDisputeGame = IPermissionedDisputeGame( + Blueprint.deployFrom( + blueprint.permissionedDisputeGame1, + blueprint.permissionedDisputeGame2, + computeSalt(_input.l2ChainId, _input.saltMixer, "PermissionedDisputeGame"), + encodePermissionedFDGConstructor( + IFaultDisputeGame.GameConstructorParams({ + gameType: GameTypes.PERMISSIONED_CANNON, + absolutePrestate: _input.disputeAbsolutePrestate, + maxGameDepth: _input.disputeMaxGameDepth, + splitDepth: _input.disputeSplitDepth, + clockExtension: _input.disputeClockExtension, + maxClockDuration: _input.disputeMaxClockDuration, + vm: IBigStepper(implementation.mipsImpl), + weth: IDelayedWETH(payable(address(output.delayedWETHPermissionedGameProxy))), + anchorStateRegistry: IAnchorStateRegistry(address(output.anchorStateRegistryProxy)), + l2ChainId: _input.l2ChainId + }), + _input.roles.proposer, + _input.roles.challenger + ) ) - ) - ); - - // Store v2 implementation addresses if the feature flag is enabled - if (isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)) { - // Store v2 implementation addresses for later registration with DisputeGameFactory - output.permissionedDisputeGameV2 = IPermissionedDisputeGameV2(implementation.permissionedDisputeGameV2Impl); - output.faultDisputeGameV2 = IFaultDisputeGameV2(implementation.faultDisputeGameV2Impl); + ); } // -------- Set and Initialize Proxy Implementations -------- @@ -1169,22 +1180,30 @@ contract OPContractsManagerDeployer is OPContractsManagerBase { data ); // Register the appropriate dispute game implementation based on the feature flag - if ( - isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES) - && address(output.permissionedDisputeGameV2) != address(0) - ) { - // Register v2 implementation for PERMISSIONED_CANNON game type + if (isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)) { + // Register v2 implementation for PERMISSIONED_CANNON game type with CWIA args + bytes memory gameArgs = abi.encodePacked( + _input.disputeAbsolutePrestate, // 32 bytes + implementation.mipsImpl, // 20 bytes + address(output.anchorStateRegistryProxy), // 20 bytes + address(output.delayedWETHPermissionedGameProxy), // 20 bytes + _input.l2ChainId, // 32 bytes + _input.roles.proposer, // 20 bytes + _input.roles.challenger // 20 bytes + ); setDGFImplementation( output.disputeGameFactoryProxy, GameTypes.PERMISSIONED_CANNON, - IDisputeGame(address(output.permissionedDisputeGameV2)) + IDisputeGame(implementation.permissionedDisputeGameV2Impl), + gameArgs ); } else { // Register v1 implementation for PERMISSIONED_CANNON game type setDGFImplementation( output.disputeGameFactoryProxy, GameTypes.PERMISSIONED_CANNON, - IDisputeGame(address(output.permissionedDisputeGame)) + IDisputeGame(address(output.permissionedDisputeGame)), + bytes("") ); } From cef1fc317a4b1b9b37de606a8f7d178ed7371a9f Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 07:00:42 -0400 Subject: [PATCH 21/52] test: add helper functin for creating game proxies --- .../test/L1/OPContractsManager.t.sol | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index a57b4a1dcf1d3..41a36cd5a5ba8 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1793,6 +1793,58 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { return dio.opcm; } + /// @notice Helper function to create a permissioned game v2 through the factory + function _createPermissionedGameV2( + IDisputeGameFactory factory, + IOPContractsManager.DeployOutput memory output, + address proposer + ) + internal + returns (IFaultDisputeGame) + { + // Check if there's an init bond required for the game type + uint256 initBond = factory.initBonds(GameTypes.PERMISSIONED_CANNON); + + // Fund the proposer if needed + if (initBond > 0) { + vm.deal(proposer, initBond); + } + + // We use vm.startPrank to set both msg.sender and tx.origin to the proposer + vm.startPrank(proposer, proposer); + + IDisputeGame gameProxy = factory.create{ value: initBond }( + GameTypes.PERMISSIONED_CANNON, Claim.wrap(bytes32(uint256(1))), abi.encode(bytes32(uint256(2))) + ); + + vm.stopPrank(); + + return IFaultDisputeGame(address(gameProxy)); + } + + /// @notice Helper function to create a fault dispute game v2 (non-permissioned) through the factory + function _createFaultDisputeGameV2( + IDisputeGameFactory factory, + IOPContractsManager.DeployOutput memory output + ) + internal + returns (IFaultDisputeGame) + { + // Check if there's an init bond required for the game type + uint256 initBond = factory.initBonds(GameTypes.CANNON); + + // Fund the test contract if needed + if (initBond > 0) { + vm.deal(address(this), initBond); + } + + IDisputeGame gameProxy = factory.create{ value: initBond }( + GameTypes.CANNON, Claim.wrap(bytes32(uint256(1))), abi.encode(bytes32(uint256(2))) + ); + + return IFaultDisputeGame(address(gameProxy)); + } + function test_deploy_l2ChainIdEqualsZero_reverts() public { IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); deployInput.l2ChainId = 0; From 503835f8bf47e2dc4e929530966e4887a2081d28 Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 10:37:49 -0400 Subject: [PATCH 22/52] test: refactor helper functions for test and remove impl from output struct --- .../src/L1/OPContractsManager.sol | 47 ++++++++++++------- .../test/L1/OPContractsManager.t.sol | 18 +------ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 441e0a703b0b8..517c6cd9c5a3f 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -311,6 +311,35 @@ abstract contract OPContractsManagerBase { return _disputeGame.challenger(); } + /// @notice Helper function to register permissioned game V2 implementation + /// @dev Extracted to avoid stack too deep error + /// @param _input The deployment input data containing all necessary parameters + /// @param _implementation The implementation addresses struct + /// @param _output The deployment output containing proxy addresses + function _registerPermissionedGameV2( + OPContractsManager.DeployInput calldata _input, + OPContractsManager.Implementations memory _implementation, + OPContractsManager.DeployOutput memory _output + ) + internal + { + bytes memory gameArgs = abi.encodePacked( + _input.disputeAbsolutePrestate, // 32 bytes + _implementation.mipsImpl, // 20 bytes + address(_output.anchorStateRegistryProxy), // 20 bytes + address(_output.delayedWETHPermissionedGameProxy), // 20 bytes + _input.l2ChainId, // 32 bytes + _input.roles.proposer, // 20 bytes + _input.roles.challenger // 20 bytes + ); + setDGFImplementation( + _output.disputeGameFactoryProxy, + GameTypes.PERMISSIONED_CANNON, + IDisputeGame(_implementation.permissionedDisputeGameV2Impl), + gameArgs + ); + } + /// @notice Retrieves the DisputeGameFactory address for a given SystemConfig function getDisputeGameFactory(ISystemConfig _systemConfig) internal view returns (IDisputeGameFactory) { return IDisputeGameFactory(_systemConfig.disputeGameFactory()); @@ -1181,22 +1210,8 @@ contract OPContractsManagerDeployer is OPContractsManagerBase { ); // Register the appropriate dispute game implementation based on the feature flag if (isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES)) { - // Register v2 implementation for PERMISSIONED_CANNON game type with CWIA args - bytes memory gameArgs = abi.encodePacked( - _input.disputeAbsolutePrestate, // 32 bytes - implementation.mipsImpl, // 20 bytes - address(output.anchorStateRegistryProxy), // 20 bytes - address(output.delayedWETHPermissionedGameProxy), // 20 bytes - _input.l2ChainId, // 32 bytes - _input.roles.proposer, // 20 bytes - _input.roles.challenger // 20 bytes - ); - setDGFImplementation( - output.disputeGameFactoryProxy, - GameTypes.PERMISSIONED_CANNON, - IDisputeGame(implementation.permissionedDisputeGameV2Impl), - gameArgs - ); + // Extracted to helper function to avoid stack too deep error + _registerPermissionedGameV2(_input, implementation, output); } else { // Register v1 implementation for PERMISSIONED_CANNON game type setDGFImplementation( diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 41a36cd5a5ba8..e318e6b74da4c 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1903,28 +1903,14 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { // Set up deploy input for the v2-enabled OPCM doi.set(doi.opcm.selector, address(opcmV2)); IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); - - vm.startPrank(address(this)); IOPContractsManager.DeployOutput memory output = opcmV2.deploy(opcmInput); - vm.stopPrank(); - - // Verify that v2 dispute game contracts are deployed and non-zero - assertTrue( - address(output.permissionedDisputeGameV2) != address(0), "PermissionedDisputeGameV2 should be deployed" - ); - assertTrue(address(output.faultDisputeGameV2) != address(0), "FaultDisputeGameV2 should be deployed"); - - // Verify that v1 permissioned dispute game is still deployed (for backward compatibility) - assertTrue( - address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 should still be deployed" - ); // Verify that the DisputeGameFactory has registered the v2 implementation for PERMISSIONED_CANNON game type address registeredPermissionedImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); - assertEq( + assertNotEq( registeredPermissionedImpl, - address(output.permissionedDisputeGameV2), + address(0), "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" ); } From 30ef2f36a9f2ad179bec5aea21cb4d27cb0b44f2 Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 11:49:45 -0400 Subject: [PATCH 23/52] test: test args correct on created proxy games --- .../test/L1/OPContractsManager.t.sol | 74 +++++++++++++++---- 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index e318e6b74da4c..2e06825355750 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1796,11 +1796,10 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { /// @notice Helper function to create a permissioned game v2 through the factory function _createPermissionedGameV2( IDisputeGameFactory factory, - IOPContractsManager.DeployOutput memory output, address proposer ) internal - returns (IFaultDisputeGame) + returns (IPermissionedDisputeGame) { // Check if there's an init bond required for the game type uint256 initBond = factory.initBonds(GameTypes.PERMISSIONED_CANNON); @@ -1819,17 +1818,11 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { vm.stopPrank(); - return IFaultDisputeGame(address(gameProxy)); + return IPermissionedDisputeGame(address(gameProxy)); } /// @notice Helper function to create a fault dispute game v2 (non-permissioned) through the factory - function _createFaultDisputeGameV2( - IDisputeGameFactory factory, - IOPContractsManager.DeployOutput memory output - ) - internal - returns (IFaultDisputeGame) - { + function _createFaultDisputeGameV2(IDisputeGameFactory factory) internal returns (IFaultDisputeGame) { // Check if there's an init bond required for the game type uint256 initBond = factory.initBonds(GameTypes.CANNON); @@ -1866,13 +1859,12 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { opcm.deploy(toOPCMDeployInput(doi)); } - /// @notice Test that deploy without v2 flag doesn't set v2 implementations - function test_deployWithoutV2Flag_succeeds() public { + /// @notice Test that deploy without v2 flag doesn't set v2 implementations for PERMISSIONED_CANNON + function test_deployPermissionedWithoutV2Flag_succeeds() public { // Convert DOI to OPCM input and deploy IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); - vm.startPrank(address(this)); IOPContractsManager.DeployOutput memory output = opcm.deploy(opcmInput); - vm.stopPrank(); + IOPContractsManager.Implementations memory impls = opcm.implementations(); // Check that v2 implementations are not set (since flag is not enabled by default) assertEq(address(output.permissionedDisputeGameV2), address(0)); @@ -1881,10 +1873,38 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { // Check that v1 implementation is registered for PERMISSIONED_CANNON address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); assertEq(registeredImpl, address(output.permissionedDisputeGame)); + + address registeredPermissionedImpl = + address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); + assertNotEq( + registeredPermissionedImpl, + address(0), + "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" + ); + assertEq( + registeredPermissionedImpl, address(output.permissionedDisputeGame), "Should be using v2 implementation" + ); + + // Create a game proxy to test immutable fields + IPermissionedDisputeGame gameV2Proxy = + _createPermissionedGameV2(output.disputeGameFactoryProxy, opcmInput.roles.proposer); + + // Verify immutable fields on the game proxy + assertEq(address(gameV2Proxy.vm()), address(impls.mipsImpl), "VM should match MIPS implementation"); + assertEq( + address(gameV2Proxy.anchorStateRegistry()), address(output.anchorStateRegistryProxy), "ASR should match" + ); + assertEq(address(gameV2Proxy.weth()), address(output.delayedWETHPermissionedGameProxy), "WETH should match"); + assertEq(gameV2Proxy.l2ChainId(), opcmInput.l2ChainId, "L2 chain ID should match"); + + // For permissioned game, check proposer and challenger + IPermissionedDisputeGame permissionedProxy = IPermissionedDisputeGame(address(gameV2Proxy)); + assertEq(permissionedProxy.proposer(), opcmInput.roles.proposer, "Proposer should match"); + assertEq(permissionedProxy.challenger(), opcmInput.roles.challenger, "Challenger should match"); } - /// @notice Test that deploy with v2 flag would set v2 implementations - function test_deployWithV2Flag_succeeds() public { + /// @notice Test that deploy with v2 flag would set v2 implementations for PERMISSIONED_CANNON + function test_deployPermissionedWithV2Flag_succeeds() public { IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); @@ -1913,5 +1933,27 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { address(0), "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" ); + assertEq( + registeredPermissionedImpl, + address(impls.permissionedDisputeGameV2Impl), + "Should be using v2 implementation" + ); + + // Create a game proxy to test immutable fields + IPermissionedDisputeGame gameV2Proxy = + _createPermissionedGameV2(output.disputeGameFactoryProxy, opcmInput.roles.proposer); + + // Verify immutable fields on the game proxy + assertEq(address(gameV2Proxy.vm()), address(impls.mipsImpl), "VM should match MIPS implementation"); + assertEq( + address(gameV2Proxy.anchorStateRegistry()), address(output.anchorStateRegistryProxy), "ASR should match" + ); + assertEq(address(gameV2Proxy.weth()), address(output.delayedWETHPermissionedGameProxy), "WETH should match"); + assertEq(gameV2Proxy.l2ChainId(), opcmInput.l2ChainId, "L2 chain ID should match"); + + // For permissioned game, check proposer and challenger + IPermissionedDisputeGame permissionedProxy = IPermissionedDisputeGame(address(gameV2Proxy)); + assertEq(permissionedProxy.proposer(), opcmInput.roles.proposer, "Proposer should match"); + assertEq(permissionedProxy.challenger(), opcmInput.roles.challenger, "Challenger should match"); } } From bcbe98f9f9260b3f8700ccbf03d598cefd0af144 Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 12:09:28 -0400 Subject: [PATCH 24/52] chore: remove unused helper for now --- .../test/L1/OPContractsManager.t.sol | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 2e06825355750..6283e822bb52a 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1821,23 +1821,6 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { return IPermissionedDisputeGame(address(gameProxy)); } - /// @notice Helper function to create a fault dispute game v2 (non-permissioned) through the factory - function _createFaultDisputeGameV2(IDisputeGameFactory factory) internal returns (IFaultDisputeGame) { - // Check if there's an init bond required for the game type - uint256 initBond = factory.initBonds(GameTypes.CANNON); - - // Fund the test contract if needed - if (initBond > 0) { - vm.deal(address(this), initBond); - } - - IDisputeGame gameProxy = factory.create{ value: initBond }( - GameTypes.CANNON, Claim.wrap(bytes32(uint256(1))), abi.encode(bytes32(uint256(2))) - ); - - return IFaultDisputeGame(address(gameProxy)); - } - function test_deploy_l2ChainIdEqualsZero_reverts() public { IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); deployInput.l2ChainId = 0; From 78f65b60602606dd06f87a9e660082f9b6a2f4f2 Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 16:04:13 -0400 Subject: [PATCH 25/52] fix: remove guard clause from local issue --- packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 3632546bc928f..155bd3baa7c15 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -898,13 +898,6 @@ contract VerifyOPCM is Script { sourceName = sourceNameOverrides[_contractName]; } - // Check if the dispute profile artifact exists and should be used. - string memory disputePath = - string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".dispute.json"); - if (vm.exists(disputePath)) { - return disputePath; - } - // Return computed path, relative to the contracts-bedrock directory. return string.concat("forge-artifacts/", sourceName, ".sol/", _contractName, ".json"); } From 0a464cda0a177db0c4f1b755841aba0a77b553f7 Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 16:04:21 -0400 Subject: [PATCH 26/52] chore: bump semver --- .../contracts-bedrock/src/L1/OPContractsManager.sol | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 517c6cd9c5a3f..6992480928344 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -719,8 +719,7 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase { /// @notice Upgrades a set of chains to the latest implementation contracts /// @param _opChainConfigs Array of OpChain structs, one per chain to upgrade - /// @dev This function is intended to be DELEGATECALLed by an address that is the common owner of every chain in - /// `_opChainConfigs`'s ProxyAdmin. + /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. /// @dev This function requires that each chain's superchainConfig is already upgraded. function upgrade(OPContractsManager.OpChainConfig[] memory _opChainConfigs) external virtual { // Grab the implementations. @@ -888,7 +887,7 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase { /// @notice Upgrades the SuperchainConfig contract. /// @param _superchainConfig The SuperchainConfig contract to upgrade. /// @param _superchainProxyAdmin The ProxyAdmin contract to use for the upgrade. - /// @dev This function is intended to be DELEGATECALLed by the superchainConfig's ProxyAdminOwner. + /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. /// @dev This function will revert if the SuperchainConfig is already at or above the target version. function upgradeSuperchainConfig(ISuperchainConfig _superchainConfig, IProxyAdmin _superchainProxyAdmin) external { // Only upgrade the superchainConfig if the current version is less than the target version. @@ -2011,8 +2010,7 @@ contract OPContractsManager is ISemver { /// @notice Upgrades a set of chains to the latest implementation contracts /// @param _opChainConfigs Array of OpChain structs, one per chain to upgrade - /// @dev This function is intended to be DELEGATECALLed by an address that is the common owner of every chain in - /// `_opChainConfigs`'s ProxyAdmin. + /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. /// @dev This function requires that each chain's superchainConfig is already upgraded. function upgrade(OpChainConfig[] memory _opChainConfigs) external virtual { if (address(this) == address(thisOPCM)) revert OnlyDelegatecall(); @@ -2024,7 +2022,7 @@ contract OPContractsManager is ISemver { /// @notice Upgrades the SuperchainConfig contract. /// @param _superchainConfig The SuperchainConfig contract to upgrade. /// @param _superchainProxyAdmin The ProxyAdmin contract to use for the upgrade. - /// @dev This function is intended to be DELEGATECALLed by the superchainConfig's ProxyAdminOwner. + /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. /// @dev This function will revert if the SuperchainConfig is already at or above the target version. function upgradeSuperchainConfig(ISuperchainConfig _superchainConfig, IProxyAdmin _superchainProxyAdmin) external { if (address(this) == address(thisOPCM)) revert OnlyDelegatecall(); From ac768a5fcfd3dfc2f7ab99f2c4f05646fb408cdd Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 16:17:26 -0400 Subject: [PATCH 27/52] chore: revert changes to ignore v2 implementations --- .../scripts/deploy/VerifyOPCM.s.sol | 19 ---------------- .../test/scripts/VerifyOPCM.t.sol | 22 +++---------------- 2 files changed, 3 insertions(+), 38 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 155bd3baa7c15..acb666bbcb1d9 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -400,14 +400,6 @@ contract VerifyOPCM is Script { console.log(string.concat(" Contract: ", _target.name)); console.log(string.concat(" Address: ", vm.toString(_target.addr))); - // Skip verification for V2 implementations based on contract name - // V2 implementations follow the pattern "*DisputeGameV2" or "*DisputeGameV2Impl" - if (_isV2DisputeGameImplementation(_target.name)) { - console.log(" Status: [SKIP] V2 implementation, not yet deployed"); - console.log(string.concat(" Status: [SKIP] Skipping verification for ", _target.name)); - return true; - } - // Build the expected path to the artifact file. string memory artifactPath = _buildArtifactPath(_target.name); console.log(string.concat(" Expected Runtime Artifact: ", artifactPath)); @@ -821,17 +813,6 @@ contract VerifyOPCM is Script { return string(fieldBytes); } - /// @notice Checks if a contract name corresponds to a V2 dispute game implementation. - /// @param _contractName The contract name to check. - /// @return True if this is a V2 dispute game implementation, false otherwise. - function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { - // V2 implementations are specifically the FaultDisputeGameV2 and PermissionedDisputeGameV2 contracts - return ( - keccak256(bytes(_contractName)) == keccak256(bytes("FaultDisputeGameV2")) - || keccak256(bytes(_contractName)) == keccak256(bytes("PermissionedDisputeGameV2")) - ); - } - /// @notice Checks if a position is inside an immutable reference. /// @param _pos The position to check. /// @param _artifact The artifact info. diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 4019098adb265..7094d573c90cf 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -78,17 +78,6 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { setupEnvVars(); } - /// @notice Helper function to check if a contract name corresponds to a V2 implementation. - /// @param _contractName The contract name to check. - /// @return True if this is a V2 implementation, false otherwise. - function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { - // V2 implementations are specifically the FaultDisputeGameV2 and PermissionedDisputeGameV2 contracts - return ( - keccak256(bytes(_contractName)) == keccak256(bytes("FaultDisputeGameV2")) - || keccak256(bytes(_contractName)) == keccak256(bytes("PermissionedDisputeGameV2")) - ); - } - /// @notice Tests that the script succeeds when no changes are introduced. function test_run_succeeds() public { // Coverage changes bytecode and causes failures, skip. @@ -137,11 +126,6 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { uint256 randomImplIndex = vm.randomUint(0, refs.length - 1); VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; - // Skip V2 implementations (not yet deployed) - if (_isV2DisputeGameImplementation(ref.name)) { - continue; - } - // Get the code for the implementation. bytes memory implCode = ref.addr.code; @@ -203,9 +187,9 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 implementations (not yet deployed) - if (_isV2DisputeGameImplementation(ref.name)) { - continue; - } + // if (_isV2DisputeGameImplementation(ref.name)) { + // continue; + // } // Get the code for the implementation. bytes memory implCode = ref.addr.code; From 5fda712204c8c743098ca690451eecb49ae2c4e4 Mon Sep 17 00:00:00 2001 From: steven Date: Thu, 25 Sep 2025 21:24:03 -0400 Subject: [PATCH 28/52] fix: check bitmap and contract name for ref --- .../scripts/deploy/VerifyOPCM.s.sol | 41 +++++++++++++++++++ .../test/scripts/VerifyOPCM.t.sol | 26 +++++++++--- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index acb666bbcb1d9..a90e5da547e8d 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -12,6 +12,7 @@ import { LibString } from "@solady/utils/LibString.sol"; import { Process } from "scripts/libraries/Process.sol"; import { Config } from "scripts/libraries/Config.sol"; import { Bytes } from "src/libraries/Bytes.sol"; +import { DevFeatures } from "src/libraries/DevFeatures.sol"; // Interfaces import { IOPContractsManager } from "interfaces/L1/IOPContractsManager.sol"; @@ -106,6 +107,8 @@ contract VerifyOPCM is Script { /// @notice Setup flag. bool internal ready; + /// @notice The OPCM address being verified, stored to access during contract verification. + address internal currentOpcmAddress; /// @notice Populates override mappings. function setUp() public { // Overrides for situations where field names do not cleanly map to contract names. @@ -201,6 +204,9 @@ contract VerifyOPCM is Script { console.log(" Do NOT do this in production"); } + // Store OPCM address for use in verification functions + currentOpcmAddress = _opcmAddress; + // Fetch Implementations & Blueprints from OPCM IOPContractsManager opcm = IOPContractsManager(_opcmAddress); @@ -396,6 +402,25 @@ contract VerifyOPCM is Script { { console.log(); console.log(string.concat("Checking Contract: ", _target.field)); + // Check if this is a V2 dispute game that should be skipped + if (_isV2DisputeGameImplementation(_target.name)) { + IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); + + if (!_isV2DisputeGamesEnabled(opcm)) { + if (_target.addr == address(0)) { + console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); + console.log(string.concat(" Contract: ", _target.name)); + return true; // Consider this "verified" when feature is off + } else { + console.log(" [FAIL] V2 dispute game deployed but feature disabled"); + console.log(string.concat(" Contract: ", _target.name)); + console.log(string.concat(" Address: ", vm.toString(_target.addr))); + return false; + } + } + // If feature is enabled, continue with normal verification + } + console.log(string.concat(" Type: ", _target.blueprint ? "Blueprint" : "Implementation")); console.log(string.concat(" Contract: ", _target.name)); console.log(string.concat(" Address: ", vm.toString(_target.addr))); @@ -500,6 +525,22 @@ contract VerifyOPCM is Script { return success; } + /// @notice Checks if V2 dispute games feature is enabled in the dev feature bitmap. + /// @param _opcm The OPContractsManager to check. + /// @return True if V2 dispute games are enabled. + function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { + bytes32 bitmap = _opcm.devFeatureBitmap(); + return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + } + + /// @notice Checks if a contract is a V2 dispute game implementation. + /// @param _contractName The name to check. + /// @return True if this is a V2 dispute game. + function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { + return LibString.eq(_contractName, "FaultDisputeGameV2") || + LibString.eq(_contractName, "PermissionedDisputeGameV2"); + } + /// @notice Verifies that the immutable variables in the OPCM contract match expected values. /// @param _opcm The OPCM contract to verify immutable variables for. /// @return True if all immutable variables are verified, false otherwise. diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 7094d573c90cf..7856a07bccee0 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.15; // Libraries import { LibString } from "@solady/utils/LibString.sol"; +import { DevFeatures } from "src/libraries/DevFeatures.sol"; // Tests import { OPContractsManager_TestInit } from "test/L1/OPContractsManager.t.sol"; @@ -120,12 +121,22 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { // Grab the list of implementations. VerifyOPCM.OpcmContractRef[] memory refs = harness.getOpcmContractRefs(opcm, "implementations", false); + // Check if V2 dispute games feature is enabled + bytes32 bitmap = opcm.devFeatureBitmap(); + bool v2FeatureEnabled = DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + // Change 256 bytes at random. - for (uint8 i = 0; i < 255; i++) { + for (uint256 i = 0; i < 255; i++) { // Pick a random implementation to change. uint256 randomImplIndex = vm.randomUint(0, refs.length - 1); VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; + // Skip V2 dispute games when feature disabled + bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + if (isV2DisputeGame && !v2FeatureEnabled) { + continue; + } + // Get the code for the implementation. bytes memory implCode = ref.addr.code; @@ -180,16 +191,21 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { // Grab the list of implementations. VerifyOPCM.OpcmContractRef[] memory refs = harness.getOpcmContractRefs(opcm, "implementations", false); + // Check if V2 dispute games feature is enabled + bytes32 bitmap = opcm.devFeatureBitmap(); + bool v2FeatureEnabled = DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + // Change 256 bytes at random. for (uint8 i = 0; i < 255; i++) { // Pick a random implementation to change. uint256 randomImplIndex = vm.randomUint(0, refs.length - 1); VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; - // Skip V2 implementations (not yet deployed) - // if (_isV2DisputeGameImplementation(ref.name)) { - // continue; - // } + // Skip V2 dispute games when feature disabled + bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + if (isV2DisputeGame && !v2FeatureEnabled) { + continue; + } // Get the code for the implementation. bytes memory implCode = ref.addr.code; From d6881869db29c9758c9f604d16e8962a592a8cb6 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 26 Sep 2025 05:32:31 -0400 Subject: [PATCH 29/52] test: with feature toggle on --- .../scripts/deploy/VerifyOPCM.s.sol | 71 ++++----- .../test/scripts/VerifyOPCM.t.sol | 145 +++++++++++++++++- 2 files changed, 179 insertions(+), 37 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index a90e5da547e8d..371eb0118d358 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -107,9 +107,10 @@ contract VerifyOPCM is Script { /// @notice Setup flag. bool internal ready; - /// @notice The OPCM address being verified, stored to access during contract verification. - address internal currentOpcmAddress; + /// @notice The OPCM address being verified, stored to access during contract verification. + address internal currentOpcmAddress; /// @notice Populates override mappings. + function setUp() public { // Overrides for situations where field names do not cleanly map to contract names. fieldNameOverrides["optimismPortalImpl"] = "OptimismPortal2"; @@ -205,7 +206,7 @@ contract VerifyOPCM is Script { } // Store OPCM address for use in verification functions - currentOpcmAddress = _opcmAddress; + currentOpcmAddress = _opcmAddress; // Fetch Implementations & Blueprints from OPCM IOPContractsManager opcm = IOPContractsManager(_opcmAddress); @@ -402,24 +403,24 @@ contract VerifyOPCM is Script { { console.log(); console.log(string.concat("Checking Contract: ", _target.field)); - // Check if this is a V2 dispute game that should be skipped - if (_isV2DisputeGameImplementation(_target.name)) { - IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); - - if (!_isV2DisputeGamesEnabled(opcm)) { - if (_target.addr == address(0)) { - console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); - console.log(string.concat(" Contract: ", _target.name)); - return true; // Consider this "verified" when feature is off - } else { - console.log(" [FAIL] V2 dispute game deployed but feature disabled"); - console.log(string.concat(" Contract: ", _target.name)); - console.log(string.concat(" Address: ", vm.toString(_target.addr))); - return false; - } - } - // If feature is enabled, continue with normal verification - } + // Check if this is a V2 dispute game that should be skipped + if (_isV2DisputeGameImplementation(_target.name)) { + IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); + + if (!_isV2DisputeGamesEnabled(opcm)) { + if (_target.addr == address(0)) { + console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); + console.log(string.concat(" Contract: ", _target.name)); + return true; // Consider this "verified" when feature is off + } else { + console.log(" [FAIL] V2 dispute game deployed but feature disabled"); + console.log(string.concat(" Contract: ", _target.name)); + console.log(string.concat(" Address: ", vm.toString(_target.addr))); + return false; + } + } + // If feature is enabled, continue with normal verification + } console.log(string.concat(" Type: ", _target.blueprint ? "Blueprint" : "Implementation")); console.log(string.concat(" Contract: ", _target.name)); @@ -526,20 +527,20 @@ contract VerifyOPCM is Script { } /// @notice Checks if V2 dispute games feature is enabled in the dev feature bitmap. - /// @param _opcm The OPContractsManager to check. - /// @return True if V2 dispute games are enabled. - function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { - bytes32 bitmap = _opcm.devFeatureBitmap(); - return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); - } - - /// @notice Checks if a contract is a V2 dispute game implementation. - /// @param _contractName The name to check. - /// @return True if this is a V2 dispute game. - function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { - return LibString.eq(_contractName, "FaultDisputeGameV2") || - LibString.eq(_contractName, "PermissionedDisputeGameV2"); - } + /// @param _opcm The OPContractsManager to check. + /// @return True if V2 dispute games are enabled. + function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { + bytes32 bitmap = _opcm.devFeatureBitmap(); + return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + } + + /// @notice Checks if a contract is a V2 dispute game implementation. + /// @param _contractName The name to check. + /// @return True if this is a V2 dispute game. + function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { + return LibString.eq(_contractName, "FaultDisputeGameV2") + || LibString.eq(_contractName, "PermissionedDisputeGameV2"); + } /// @notice Verifies that the immutable variables in the OPCM contract match expected values. /// @param _opcm The OPCM contract to verify immutable variables for. diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 7856a07bccee0..55e0930163ffe 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -4,12 +4,16 @@ pragma solidity 0.8.15; // Libraries import { LibString } from "@solady/utils/LibString.sol"; import { DevFeatures } from "src/libraries/DevFeatures.sol"; +import { ProtocolVersion } from "src/L1/ProtocolVersions.sol"; // Tests import { OPContractsManager_TestInit } from "test/L1/OPContractsManager.t.sol"; // Scripts import { VerifyOPCM } from "scripts/deploy/VerifyOPCM.s.sol"; +import { DeploySuperchain } from "scripts/deploy/DeploySuperchain.s.sol"; +import { DeployImplementations } from "scripts/deploy/DeployImplementations.s.sol"; +import { StandardConstants } from "scripts/deploy/StandardConstants.sol"; // Interfaces import { IOPContractsManager, IOPContractsManagerUpgrader } from "interfaces/L1/IOPContractsManager.sol"; @@ -69,6 +73,48 @@ contract VerifyOPCM_TestInit is OPContractsManager_TestInit { harness = new VerifyOPCM_Harness(); harness.setUp(); } + + /// @notice Deploys a new OPCM with V2 dispute games feature enabled. + /// @return The deployed OPCM with V2 contracts. + function deployOPCMWithV2Enabled() internal returns (IOPContractsManager) { + // Deploy Superchain contracts first + DeploySuperchain deploySuperchain = new DeploySuperchain(); + DeploySuperchain.Output memory dso = deploySuperchain.run( + DeploySuperchain.Input({ + superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), + protocolVersionsOwner: makeAddr("protocolVersionsOwner"), + guardian: makeAddr("guardian"), + paused: false, + requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), + recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) + }) + ); + + // Deploy implementations with v2 flag enabled + DeployImplementations deployImplementations = new DeployImplementations(); + DeployImplementations.Output memory dio = deployImplementations.run( + DeployImplementations.Input({ + withdrawalDelaySeconds: 100, + minProposalSizeBytes: 200, + challengePeriodSeconds: 300, + proofMaturityDelaySeconds: 400, + disputeGameFinalityDelaySeconds: 500, + mipsVersion: StandardConstants.MIPS_VERSION, + faultGameV2MaxGameDepth: 73, + faultGameV2SplitDepth: 30, + faultGameV2ClockExtension: 10800, + faultGameV2MaxClockDuration: 302400, + superchainConfigProxy: dso.superchainConfigProxy, + protocolVersionsProxy: dso.protocolVersionsProxy, + superchainProxyAdmin: dso.superchainProxyAdmin, + upgradeController: dso.superchainProxyAdmin.owner(), + challenger: makeAddr("challenger"), + devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here + }) + ); + + return dio.opcm; + } } /// @title VerifyOPCM_Run_Test @@ -132,7 +178,8 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 dispute games when feature disabled - bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + bool isV2DisputeGame = + LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); if (isV2DisputeGame && !v2FeatureEnabled) { continue; } @@ -202,7 +249,8 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 dispute games when feature disabled - bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + bool isV2DisputeGame = + LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); if (isV2DisputeGame && !v2FeatureEnabled) { continue; } @@ -476,4 +524,97 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { vm.expectRevert(abi.encodeWithSelector(VerifyOPCM.VerifyOPCM_UnaccountedGetters.selector, expectedUnaccounted)); harness.validateAllGettersAccounted(); } + + /// @notice Tests that the script succeeds when V2 dispute games are deployed (feature enabled). + /// @dev This test will fail with ProtocolVersions bytecode mismatch - this is a known issue. + function test_run_withV2DisputeGamesEnabled_succeeds() public { + // Coverage changes bytecode and causes failures, skip. + skipIfCoverage(); + + // Deploy OPCM with V2 feature enabled + IOPContractsManager opcmV2 = deployOPCMWithV2Enabled(); + + // Verify that V2 dispute games feature is enabled + assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); + + // Verify that V2 contracts are deployed (not address(0)) + IOPContractsManager.Implementations memory impls = opcmV2.implementations(); + assertTrue( + address(impls.faultDisputeGameV2Impl) != address(0), + "FaultDisputeGameV2 implementation should be non-zero" + ); + assertTrue( + address(impls.permissionedDisputeGameV2Impl) != address(0), + "PermissionedDisputeGameV2 implementation should be non-zero" + ); + + // Update environment variables for the new OPCM deployment + vm.setEnv("EXPECTED_SUPERCHAIN_CONFIG", vm.toString(address(opcmV2.superchainConfig()))); + vm.setEnv("EXPECTED_PROTOCOL_VERSIONS", vm.toString(address(opcmV2.protocolVersions()))); + vm.setEnv("EXPECTED_SUPERCHAIN_PROXY_ADMIN", vm.toString(address(opcmV2.superchainProxyAdmin()))); + vm.setEnv("EXPECTED_UPGRADE_CONTROLLER", vm.toString(opcmV2.upgradeController())); + + // Run verification - should succeed WITH V2 contracts properly verified + harness.run(address(opcmV2), true); + } + + /// @notice Tests that the script reverts when V2 contracts have non-immutable modifications. + function test_run_v2EnabledNonImmutableModifications_reverts() public { + // Coverage changes bytecode and causes failures, skip. + skipIfCoverage(); + + // Deploy OPCM with V2 feature enabled + IOPContractsManager opcmV2 = deployOPCMWithV2Enabled(); + + // Update environment variables for the new OPCM deployment + vm.setEnv("EXPECTED_SUPERCHAIN_CONFIG", vm.toString(address(opcmV2.superchainConfig()))); + vm.setEnv("EXPECTED_PROTOCOL_VERSIONS", vm.toString(address(opcmV2.protocolVersions()))); + vm.setEnv("EXPECTED_SUPERCHAIN_PROXY_ADMIN", vm.toString(address(opcmV2.superchainProxyAdmin()))); + vm.setEnv("EXPECTED_UPGRADE_CONTROLLER", vm.toString(opcmV2.upgradeController())); + + // Grab the list of implementations including V2 + VerifyOPCM.OpcmContractRef[] memory refs = harness.getOpcmContractRefs(opcmV2, "implementations", false); + + // Find and modify a V2 contract outside immutable references + for (uint256 i = 0; i < refs.length; i++) { + if (LibString.eq(refs[i].name, "FaultDisputeGameV2")) { + // Get the code for the implementation + bytes memory implCode = refs[i].addr.code; + + // Grab the artifact info for the implementation + VerifyOPCM.ArtifactInfo memory artifact = harness.loadArtifactInfo(harness.buildArtifactPath(refs[i].name)); + + // Find a byte that's NOT in an immutable reference + bool inImmutable = true; + uint256 modifyPos = 0; + while (inImmutable && modifyPos < implCode.length) { + inImmutable = false; + for (uint256 j = 0; j < artifact.immutableRefs.length; j++) { + VerifyOPCM.ImmutableRef memory immRef = artifact.immutableRefs[j]; + if (modifyPos >= immRef.offset && modifyPos < immRef.offset + immRef.length) { + inImmutable = true; + break; + } + } + if (inImmutable) { + modifyPos++; + } + } + + // Modify the byte outside immutable references + if (modifyPos < implCode.length) { + bytes1 existingByte = implCode[modifyPos]; + bytes1 newByte = bytes1(uint8(uint256(uint8(existingByte)) + 1)); + implCode[modifyPos] = newByte; + vm.etch(refs[i].addr, implCode); + } + + break; // Only modify one V2 contract for this test + } + } + + // Run verification - should revert due to non-immutable modification + vm.expectRevert(VerifyOPCM.VerifyOPCM_Failed.selector); + harness.run(address(opcmV2), true); + } } From 5a585a34aec09c260726c980e08dea5eab0eebac Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 26 Sep 2025 06:00:03 -0400 Subject: [PATCH 30/52] chore: forge fmt --- packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 55e0930163ffe..0ce0986489057 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -110,7 +110,7 @@ contract VerifyOPCM_TestInit is OPContractsManager_TestInit { upgradeController: dso.superchainProxyAdmin.owner(), challenger: makeAddr("challenger"), devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here - }) + }) ); return dio.opcm; @@ -540,8 +540,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { // Verify that V2 contracts are deployed (not address(0)) IOPContractsManager.Implementations memory impls = opcmV2.implementations(); assertTrue( - address(impls.faultDisputeGameV2Impl) != address(0), - "FaultDisputeGameV2 implementation should be non-zero" + address(impls.faultDisputeGameV2Impl) != address(0), "FaultDisputeGameV2 implementation should be non-zero" ); assertTrue( address(impls.permissionedDisputeGameV2Impl) != address(0), @@ -582,7 +581,8 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { bytes memory implCode = refs[i].addr.code; // Grab the artifact info for the implementation - VerifyOPCM.ArtifactInfo memory artifact = harness.loadArtifactInfo(harness.buildArtifactPath(refs[i].name)); + VerifyOPCM.ArtifactInfo memory artifact = + harness.loadArtifactInfo(harness.buildArtifactPath(refs[i].name)); // Find a byte that's NOT in an immutable reference bool inImmutable = true; From 3c4c5290b6580a8ea737604e9ce0a45f08597992 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 26 Sep 2025 06:30:59 -0400 Subject: [PATCH 31/52] Revert "chore: forge fmt" This reverts commit d029fdb8dd12445a839026972e3859be7aae1470. --- packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 0ce0986489057..55e0930163ffe 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -110,7 +110,7 @@ contract VerifyOPCM_TestInit is OPContractsManager_TestInit { upgradeController: dso.superchainProxyAdmin.owner(), challenger: makeAddr("challenger"), devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here - }) + }) ); return dio.opcm; @@ -540,7 +540,8 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { // Verify that V2 contracts are deployed (not address(0)) IOPContractsManager.Implementations memory impls = opcmV2.implementations(); assertTrue( - address(impls.faultDisputeGameV2Impl) != address(0), "FaultDisputeGameV2 implementation should be non-zero" + address(impls.faultDisputeGameV2Impl) != address(0), + "FaultDisputeGameV2 implementation should be non-zero" ); assertTrue( address(impls.permissionedDisputeGameV2Impl) != address(0), @@ -581,8 +582,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { bytes memory implCode = refs[i].addr.code; // Grab the artifact info for the implementation - VerifyOPCM.ArtifactInfo memory artifact = - harness.loadArtifactInfo(harness.buildArtifactPath(refs[i].name)); + VerifyOPCM.ArtifactInfo memory artifact = harness.loadArtifactInfo(harness.buildArtifactPath(refs[i].name)); // Find a byte that's NOT in an immutable reference bool inImmutable = true; From e6e7888796943e9681a0a515b209a7e85ef28102 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 26 Sep 2025 06:31:10 -0400 Subject: [PATCH 32/52] Revert "test: with feature toggle on" This reverts commit a932eb608d25f9acf486ce49aeb5318742c666de. --- .../scripts/deploy/VerifyOPCM.s.sol | 71 +++++---- .../test/scripts/VerifyOPCM.t.sol | 145 +----------------- 2 files changed, 37 insertions(+), 179 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index 371eb0118d358..a90e5da547e8d 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -107,10 +107,9 @@ contract VerifyOPCM is Script { /// @notice Setup flag. bool internal ready; - /// @notice The OPCM address being verified, stored to access during contract verification. - address internal currentOpcmAddress; + /// @notice The OPCM address being verified, stored to access during contract verification. + address internal currentOpcmAddress; /// @notice Populates override mappings. - function setUp() public { // Overrides for situations where field names do not cleanly map to contract names. fieldNameOverrides["optimismPortalImpl"] = "OptimismPortal2"; @@ -206,7 +205,7 @@ contract VerifyOPCM is Script { } // Store OPCM address for use in verification functions - currentOpcmAddress = _opcmAddress; + currentOpcmAddress = _opcmAddress; // Fetch Implementations & Blueprints from OPCM IOPContractsManager opcm = IOPContractsManager(_opcmAddress); @@ -403,24 +402,24 @@ contract VerifyOPCM is Script { { console.log(); console.log(string.concat("Checking Contract: ", _target.field)); - // Check if this is a V2 dispute game that should be skipped - if (_isV2DisputeGameImplementation(_target.name)) { - IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); - - if (!_isV2DisputeGamesEnabled(opcm)) { - if (_target.addr == address(0)) { - console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); - console.log(string.concat(" Contract: ", _target.name)); - return true; // Consider this "verified" when feature is off - } else { - console.log(" [FAIL] V2 dispute game deployed but feature disabled"); - console.log(string.concat(" Contract: ", _target.name)); - console.log(string.concat(" Address: ", vm.toString(_target.addr))); - return false; - } - } - // If feature is enabled, continue with normal verification - } + // Check if this is a V2 dispute game that should be skipped + if (_isV2DisputeGameImplementation(_target.name)) { + IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); + + if (!_isV2DisputeGamesEnabled(opcm)) { + if (_target.addr == address(0)) { + console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); + console.log(string.concat(" Contract: ", _target.name)); + return true; // Consider this "verified" when feature is off + } else { + console.log(" [FAIL] V2 dispute game deployed but feature disabled"); + console.log(string.concat(" Contract: ", _target.name)); + console.log(string.concat(" Address: ", vm.toString(_target.addr))); + return false; + } + } + // If feature is enabled, continue with normal verification + } console.log(string.concat(" Type: ", _target.blueprint ? "Blueprint" : "Implementation")); console.log(string.concat(" Contract: ", _target.name)); @@ -527,20 +526,20 @@ contract VerifyOPCM is Script { } /// @notice Checks if V2 dispute games feature is enabled in the dev feature bitmap. - /// @param _opcm The OPContractsManager to check. - /// @return True if V2 dispute games are enabled. - function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { - bytes32 bitmap = _opcm.devFeatureBitmap(); - return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); - } - - /// @notice Checks if a contract is a V2 dispute game implementation. - /// @param _contractName The name to check. - /// @return True if this is a V2 dispute game. - function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { - return LibString.eq(_contractName, "FaultDisputeGameV2") - || LibString.eq(_contractName, "PermissionedDisputeGameV2"); - } + /// @param _opcm The OPContractsManager to check. + /// @return True if V2 dispute games are enabled. + function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { + bytes32 bitmap = _opcm.devFeatureBitmap(); + return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + } + + /// @notice Checks if a contract is a V2 dispute game implementation. + /// @param _contractName The name to check. + /// @return True if this is a V2 dispute game. + function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { + return LibString.eq(_contractName, "FaultDisputeGameV2") || + LibString.eq(_contractName, "PermissionedDisputeGameV2"); + } /// @notice Verifies that the immutable variables in the OPCM contract match expected values. /// @param _opcm The OPCM contract to verify immutable variables for. diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 55e0930163ffe..7856a07bccee0 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -4,16 +4,12 @@ pragma solidity 0.8.15; // Libraries import { LibString } from "@solady/utils/LibString.sol"; import { DevFeatures } from "src/libraries/DevFeatures.sol"; -import { ProtocolVersion } from "src/L1/ProtocolVersions.sol"; // Tests import { OPContractsManager_TestInit } from "test/L1/OPContractsManager.t.sol"; // Scripts import { VerifyOPCM } from "scripts/deploy/VerifyOPCM.s.sol"; -import { DeploySuperchain } from "scripts/deploy/DeploySuperchain.s.sol"; -import { DeployImplementations } from "scripts/deploy/DeployImplementations.s.sol"; -import { StandardConstants } from "scripts/deploy/StandardConstants.sol"; // Interfaces import { IOPContractsManager, IOPContractsManagerUpgrader } from "interfaces/L1/IOPContractsManager.sol"; @@ -73,48 +69,6 @@ contract VerifyOPCM_TestInit is OPContractsManager_TestInit { harness = new VerifyOPCM_Harness(); harness.setUp(); } - - /// @notice Deploys a new OPCM with V2 dispute games feature enabled. - /// @return The deployed OPCM with V2 contracts. - function deployOPCMWithV2Enabled() internal returns (IOPContractsManager) { - // Deploy Superchain contracts first - DeploySuperchain deploySuperchain = new DeploySuperchain(); - DeploySuperchain.Output memory dso = deploySuperchain.run( - DeploySuperchain.Input({ - superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), - protocolVersionsOwner: makeAddr("protocolVersionsOwner"), - guardian: makeAddr("guardian"), - paused: false, - requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), - recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) - }) - ); - - // Deploy implementations with v2 flag enabled - DeployImplementations deployImplementations = new DeployImplementations(); - DeployImplementations.Output memory dio = deployImplementations.run( - DeployImplementations.Input({ - withdrawalDelaySeconds: 100, - minProposalSizeBytes: 200, - challengePeriodSeconds: 300, - proofMaturityDelaySeconds: 400, - disputeGameFinalityDelaySeconds: 500, - mipsVersion: StandardConstants.MIPS_VERSION, - faultGameV2MaxGameDepth: 73, - faultGameV2SplitDepth: 30, - faultGameV2ClockExtension: 10800, - faultGameV2MaxClockDuration: 302400, - superchainConfigProxy: dso.superchainConfigProxy, - protocolVersionsProxy: dso.protocolVersionsProxy, - superchainProxyAdmin: dso.superchainProxyAdmin, - upgradeController: dso.superchainProxyAdmin.owner(), - challenger: makeAddr("challenger"), - devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here - }) - ); - - return dio.opcm; - } } /// @title VerifyOPCM_Run_Test @@ -178,8 +132,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 dispute games when feature disabled - bool isV2DisputeGame = - LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); if (isV2DisputeGame && !v2FeatureEnabled) { continue; } @@ -249,8 +202,7 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 dispute games when feature disabled - bool isV2DisputeGame = - LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); if (isV2DisputeGame && !v2FeatureEnabled) { continue; } @@ -524,97 +476,4 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { vm.expectRevert(abi.encodeWithSelector(VerifyOPCM.VerifyOPCM_UnaccountedGetters.selector, expectedUnaccounted)); harness.validateAllGettersAccounted(); } - - /// @notice Tests that the script succeeds when V2 dispute games are deployed (feature enabled). - /// @dev This test will fail with ProtocolVersions bytecode mismatch - this is a known issue. - function test_run_withV2DisputeGamesEnabled_succeeds() public { - // Coverage changes bytecode and causes failures, skip. - skipIfCoverage(); - - // Deploy OPCM with V2 feature enabled - IOPContractsManager opcmV2 = deployOPCMWithV2Enabled(); - - // Verify that V2 dispute games feature is enabled - assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); - - // Verify that V2 contracts are deployed (not address(0)) - IOPContractsManager.Implementations memory impls = opcmV2.implementations(); - assertTrue( - address(impls.faultDisputeGameV2Impl) != address(0), - "FaultDisputeGameV2 implementation should be non-zero" - ); - assertTrue( - address(impls.permissionedDisputeGameV2Impl) != address(0), - "PermissionedDisputeGameV2 implementation should be non-zero" - ); - - // Update environment variables for the new OPCM deployment - vm.setEnv("EXPECTED_SUPERCHAIN_CONFIG", vm.toString(address(opcmV2.superchainConfig()))); - vm.setEnv("EXPECTED_PROTOCOL_VERSIONS", vm.toString(address(opcmV2.protocolVersions()))); - vm.setEnv("EXPECTED_SUPERCHAIN_PROXY_ADMIN", vm.toString(address(opcmV2.superchainProxyAdmin()))); - vm.setEnv("EXPECTED_UPGRADE_CONTROLLER", vm.toString(opcmV2.upgradeController())); - - // Run verification - should succeed WITH V2 contracts properly verified - harness.run(address(opcmV2), true); - } - - /// @notice Tests that the script reverts when V2 contracts have non-immutable modifications. - function test_run_v2EnabledNonImmutableModifications_reverts() public { - // Coverage changes bytecode and causes failures, skip. - skipIfCoverage(); - - // Deploy OPCM with V2 feature enabled - IOPContractsManager opcmV2 = deployOPCMWithV2Enabled(); - - // Update environment variables for the new OPCM deployment - vm.setEnv("EXPECTED_SUPERCHAIN_CONFIG", vm.toString(address(opcmV2.superchainConfig()))); - vm.setEnv("EXPECTED_PROTOCOL_VERSIONS", vm.toString(address(opcmV2.protocolVersions()))); - vm.setEnv("EXPECTED_SUPERCHAIN_PROXY_ADMIN", vm.toString(address(opcmV2.superchainProxyAdmin()))); - vm.setEnv("EXPECTED_UPGRADE_CONTROLLER", vm.toString(opcmV2.upgradeController())); - - // Grab the list of implementations including V2 - VerifyOPCM.OpcmContractRef[] memory refs = harness.getOpcmContractRefs(opcmV2, "implementations", false); - - // Find and modify a V2 contract outside immutable references - for (uint256 i = 0; i < refs.length; i++) { - if (LibString.eq(refs[i].name, "FaultDisputeGameV2")) { - // Get the code for the implementation - bytes memory implCode = refs[i].addr.code; - - // Grab the artifact info for the implementation - VerifyOPCM.ArtifactInfo memory artifact = harness.loadArtifactInfo(harness.buildArtifactPath(refs[i].name)); - - // Find a byte that's NOT in an immutable reference - bool inImmutable = true; - uint256 modifyPos = 0; - while (inImmutable && modifyPos < implCode.length) { - inImmutable = false; - for (uint256 j = 0; j < artifact.immutableRefs.length; j++) { - VerifyOPCM.ImmutableRef memory immRef = artifact.immutableRefs[j]; - if (modifyPos >= immRef.offset && modifyPos < immRef.offset + immRef.length) { - inImmutable = true; - break; - } - } - if (inImmutable) { - modifyPos++; - } - } - - // Modify the byte outside immutable references - if (modifyPos < implCode.length) { - bytes1 existingByte = implCode[modifyPos]; - bytes1 newByte = bytes1(uint8(uint256(uint8(existingByte)) + 1)); - implCode[modifyPos] = newByte; - vm.etch(refs[i].addr, implCode); - } - - break; // Only modify one V2 contract for this test - } - } - - // Run verification - should revert due to non-immutable modification - vm.expectRevert(VerifyOPCM.VerifyOPCM_Failed.selector); - harness.run(address(opcmV2), true); - } } From 3bd2507e9c09dec9f6af2f04d35d7993759500b8 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 26 Sep 2025 06:31:50 -0400 Subject: [PATCH 33/52] chore: forge fmt --- .../scripts/deploy/VerifyOPCM.s.sol | 71 ++++++++++--------- .../test/scripts/VerifyOPCM.t.sol | 6 +- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index a90e5da547e8d..371eb0118d358 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -107,9 +107,10 @@ contract VerifyOPCM is Script { /// @notice Setup flag. bool internal ready; - /// @notice The OPCM address being verified, stored to access during contract verification. - address internal currentOpcmAddress; + /// @notice The OPCM address being verified, stored to access during contract verification. + address internal currentOpcmAddress; /// @notice Populates override mappings. + function setUp() public { // Overrides for situations where field names do not cleanly map to contract names. fieldNameOverrides["optimismPortalImpl"] = "OptimismPortal2"; @@ -205,7 +206,7 @@ contract VerifyOPCM is Script { } // Store OPCM address for use in verification functions - currentOpcmAddress = _opcmAddress; + currentOpcmAddress = _opcmAddress; // Fetch Implementations & Blueprints from OPCM IOPContractsManager opcm = IOPContractsManager(_opcmAddress); @@ -402,24 +403,24 @@ contract VerifyOPCM is Script { { console.log(); console.log(string.concat("Checking Contract: ", _target.field)); - // Check if this is a V2 dispute game that should be skipped - if (_isV2DisputeGameImplementation(_target.name)) { - IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); - - if (!_isV2DisputeGamesEnabled(opcm)) { - if (_target.addr == address(0)) { - console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); - console.log(string.concat(" Contract: ", _target.name)); - return true; // Consider this "verified" when feature is off - } else { - console.log(" [FAIL] V2 dispute game deployed but feature disabled"); - console.log(string.concat(" Contract: ", _target.name)); - console.log(string.concat(" Address: ", vm.toString(_target.addr))); - return false; - } - } - // If feature is enabled, continue with normal verification - } + // Check if this is a V2 dispute game that should be skipped + if (_isV2DisputeGameImplementation(_target.name)) { + IOPContractsManager opcm = IOPContractsManager(currentOpcmAddress); + + if (!_isV2DisputeGamesEnabled(opcm)) { + if (_target.addr == address(0)) { + console.log(" [SKIP] V2 dispute game not deployed (feature disabled)"); + console.log(string.concat(" Contract: ", _target.name)); + return true; // Consider this "verified" when feature is off + } else { + console.log(" [FAIL] V2 dispute game deployed but feature disabled"); + console.log(string.concat(" Contract: ", _target.name)); + console.log(string.concat(" Address: ", vm.toString(_target.addr))); + return false; + } + } + // If feature is enabled, continue with normal verification + } console.log(string.concat(" Type: ", _target.blueprint ? "Blueprint" : "Implementation")); console.log(string.concat(" Contract: ", _target.name)); @@ -526,20 +527,20 @@ contract VerifyOPCM is Script { } /// @notice Checks if V2 dispute games feature is enabled in the dev feature bitmap. - /// @param _opcm The OPContractsManager to check. - /// @return True if V2 dispute games are enabled. - function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { - bytes32 bitmap = _opcm.devFeatureBitmap(); - return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); - } - - /// @notice Checks if a contract is a V2 dispute game implementation. - /// @param _contractName The name to check. - /// @return True if this is a V2 dispute game. - function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { - return LibString.eq(_contractName, "FaultDisputeGameV2") || - LibString.eq(_contractName, "PermissionedDisputeGameV2"); - } + /// @param _opcm The OPContractsManager to check. + /// @return True if V2 dispute games are enabled. + function _isV2DisputeGamesEnabled(IOPContractsManager _opcm) internal view returns (bool) { + bytes32 bitmap = _opcm.devFeatureBitmap(); + return DevFeatures.isDevFeatureEnabled(bitmap, DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + } + + /// @notice Checks if a contract is a V2 dispute game implementation. + /// @param _contractName The name to check. + /// @return True if this is a V2 dispute game. + function _isV2DisputeGameImplementation(string memory _contractName) internal pure returns (bool) { + return LibString.eq(_contractName, "FaultDisputeGameV2") + || LibString.eq(_contractName, "PermissionedDisputeGameV2"); + } /// @notice Verifies that the immutable variables in the OPCM contract match expected values. /// @param _opcm The OPCM contract to verify immutable variables for. diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 7856a07bccee0..31dac9fa35d47 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -132,7 +132,8 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 dispute games when feature disabled - bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + bool isV2DisputeGame = + LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); if (isV2DisputeGame && !v2FeatureEnabled) { continue; } @@ -202,7 +203,8 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { VerifyOPCM.OpcmContractRef memory ref = refs[randomImplIndex]; // Skip V2 dispute games when feature disabled - bool isV2DisputeGame = LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); + bool isV2DisputeGame = + LibString.eq(ref.name, "FaultDisputeGameV2") || LibString.eq(ref.name, "PermissionedDisputeGameV2"); if (isV2DisputeGame && !v2FeatureEnabled) { continue; } From a253141c80894541322b0e2ae1c7c032a15ff633 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 29 Sep 2025 15:02:48 -0400 Subject: [PATCH 34/52] fix: test compiler restriction fix --- packages/contracts-bedrock/foundry.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 12d9cb5f206ad..284e2e2d5010a 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -147,7 +147,7 @@ compilation_restrictions = [ { paths = "src/L1/OPContractsManager.sol", optimizer_runs = 0 }, { paths = "src/L1/OPContractsManagerStandardValidator.sol", optimizer_runs = 0 }, { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 0 }, - { paths = "src/L1/ProtocolVersions.sol", optimizer_runs = 0 }, + { paths = "src/L1/ProtocolVersions.sol", optimizer_runs = 0 } ] ################################################################ From df9a7673e1f2a6f37d24daafb2800571fb17b07a Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 29 Sep 2025 15:08:25 -0400 Subject: [PATCH 35/52] Revert "fix: test compiler restriction fix" This reverts commit 50282cf4fbf32b2a3615a4d9537e0887436f682d. --- packages/contracts-bedrock/foundry.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 284e2e2d5010a..e834a9b0ec167 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -28,8 +28,7 @@ compilation_restrictions = [ { paths = "src/dispute/v2/PermissionedDisputeGameV2.sol", optimizer_runs = 5000 }, { paths = "src/L1/OPContractsManager.sol", optimizer_runs = 5000 }, { paths = "src/L1/OPContractsManagerStandardValidator.sol", optimizer_runs = 5000 }, - { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 5000 }, - { paths = "src/L1/ProtocolVersions.sol", optimizer_runs = 5000 } + { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 5000 } ] extra_output = ['devdoc', 'userdoc', 'metadata', 'storageLayout'] @@ -147,7 +146,6 @@ compilation_restrictions = [ { paths = "src/L1/OPContractsManager.sol", optimizer_runs = 0 }, { paths = "src/L1/OPContractsManagerStandardValidator.sol", optimizer_runs = 0 }, { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 0 }, - { paths = "src/L1/ProtocolVersions.sol", optimizer_runs = 0 } ] ################################################################ From 7adc269c5a330c488e7d965d8ba8d329eb3423f4 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 29 Sep 2025 15:13:23 -0400 Subject: [PATCH 36/52] fix: compiler bump --- packages/contracts-bedrock/foundry.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index e834a9b0ec167..9a1d5901f09a8 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -28,7 +28,8 @@ compilation_restrictions = [ { paths = "src/dispute/v2/PermissionedDisputeGameV2.sol", optimizer_runs = 5000 }, { paths = "src/L1/OPContractsManager.sol", optimizer_runs = 5000 }, { paths = "src/L1/OPContractsManagerStandardValidator.sol", optimizer_runs = 5000 }, - { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 5000 } + { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 5000 }, + { paths = "src/L1/ProtocolVersions.sol", optimizer_runs = 5000 } ] extra_output = ['devdoc', 'userdoc', 'metadata', 'storageLayout'] @@ -146,6 +147,7 @@ compilation_restrictions = [ { paths = "src/L1/OPContractsManager.sol", optimizer_runs = 0 }, { paths = "src/L1/OPContractsManagerStandardValidator.sol", optimizer_runs = 0 }, { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 0 }, + { paths = "src/L1/ProtocolVerions.sol", optimizer_runs = 0 }, ] ################################################################ From 05f87503a5ce2aa7dacf9f7fdcfb99814c4d34ae Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 29 Sep 2025 15:15:23 -0400 Subject: [PATCH 37/52] fix: typo --- packages/contracts-bedrock/foundry.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 9a1d5901f09a8..12d9cb5f206ad 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -147,7 +147,7 @@ compilation_restrictions = [ { paths = "src/L1/OPContractsManager.sol", optimizer_runs = 0 }, { paths = "src/L1/OPContractsManagerStandardValidator.sol", optimizer_runs = 0 }, { paths = "src/L1/OptimismPortal2.sol", optimizer_runs = 0 }, - { paths = "src/L1/ProtocolVerions.sol", optimizer_runs = 0 }, + { paths = "src/L1/ProtocolVersions.sol", optimizer_runs = 0 }, ] ################################################################ From 113651bb705b5d464ac1a442ba1a4612bdd8d8db Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 29 Sep 2025 15:57:59 -0400 Subject: [PATCH 38/52] test: add test for verify opcm with v2 dispute games deployed --- .../test/scripts/VerifyOPCM.t.sol | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol index 31dac9fa35d47..a2714ffd00218 100644 --- a/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol +++ b/packages/contracts-bedrock/test/scripts/VerifyOPCM.t.sol @@ -479,3 +479,22 @@ contract VerifyOPCM_Run_Test is VerifyOPCM_TestInit { harness.validateAllGettersAccounted(); } } + +/// @title VerifyOPCM_Run_V2DisputeGamesEnabled_Test +/// @notice Tests the `run` function with V2 dispute games enabled. +contract VerifyOPCM_Run_V2DisputeGamesEnabled_Test is VerifyOPCM_TestInit { + function setUp() public override { + setDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + super.setUp(); + setupEnvVars(); + } + + /// @notice Tests that the script succeeds when V2 dispute games are enabled. + function test_run_succeeds() public { + // Coverage changes bytecode and causes failures, skip. + skipIfCoverage(); + + // Run the script with V2-enabled OPCM. + harness.run(address(opcm), true); + } +} From 8e8d8a5d84ac5f35754c75d77faf47d2a5727b12 Mon Sep 17 00:00:00 2001 From: steven Date: Mon, 29 Sep 2025 17:25:22 -0400 Subject: [PATCH 39/52] test: add skips for v1 tests if v2 deployed --- .../contracts-bedrock/test/dispute/FaultDisputeGame.t.sol | 4 ++++ .../test/dispute/PermissionedDisputeGame.t.sol | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol index 686b4489f5dad..e08a0da2130b6 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol @@ -21,6 +21,7 @@ import { LibClock } from "src/dispute/lib/LibUDT.sol"; import { LibPosition } from "src/dispute/lib/LibPosition.sol"; import "src/dispute/lib/Types.sol"; import "src/dispute/lib/Errors.sol"; +import { DevFeatures } from "src/libraries/DevFeatures.sol"; // Interfaces import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; @@ -150,6 +151,9 @@ contract FaultDisputeGame_TestInit is BaseFaultDisputeGame_TestInit { super.setUp(); + // Skip V1 tests when V2 dispute games are enabled to avoid game type conflicts + skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + // Get the actual anchor roots (Hash root, uint256 l2Bn) = anchorStateRegistry.getAnchorRoot(); validL2BlockNumber = l2Bn + 1; diff --git a/packages/contracts-bedrock/test/dispute/PermissionedDisputeGame.t.sol b/packages/contracts-bedrock/test/dispute/PermissionedDisputeGame.t.sol index 30c0e547a2981..d49459df52ef8 100644 --- a/packages/contracts-bedrock/test/dispute/PermissionedDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/dispute/PermissionedDisputeGame.t.sol @@ -7,6 +7,7 @@ import { AlphabetVM } from "test/mocks/AlphabetVM.sol"; // Libraries import "src/dispute/lib/Types.sol"; import "src/dispute/lib/Errors.sol"; +import { DevFeatures } from "src/libraries/DevFeatures.sol"; // Interfaces import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol"; @@ -95,6 +96,9 @@ contract PermissionedDisputeGame_TestInit is DisputeGameFactory_TestInit { super.setUp(); + // Skip V1 tests when V2 dispute games are enabled to avoid game type conflicts + skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + // Get the actual anchor roots (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); validL2BlockNumber = l2BlockNumber + 1; From c2a4b665b7c993e5e746543744d05c23f812562a Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 30 Sep 2025 13:58:18 -0400 Subject: [PATCH 40/52] fix: skip standard validator until its implemented --- .../test/L1/OPContractsManagerStandardValidator.t.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol index 8bbbd9e8c2cd7..7d219ca0de053 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol @@ -10,6 +10,7 @@ import { GameTypes, Duration, Claim } from "src/dispute/lib/Types.sol"; import { DeployUtils } from "scripts/libraries/DeployUtils.sol"; import { ForgeArtifacts } from "scripts/libraries/ForgeArtifacts.sol"; import { Features } from "src/libraries/Features.sol"; +import { DevFeatures } from "src/libraries/DevFeatures.sol"; // Interfaces import { IOPContractsManager } from "interfaces/L1/IOPContractsManager.sol"; @@ -102,6 +103,9 @@ contract OPContractsManagerStandardValidator_TestInit is CommonTest { function setUp() public virtual override { super.setUp(); + // Skip V1 StandardValidator tests when V2 dispute games are enabled + skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + // Grab the deploy input for later use. deployInput = deploy.getDeployInput(); From 35ffb7a892baf2534bb134555e071428184a3df2 Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 30 Sep 2025 14:22:25 -0400 Subject: [PATCH 41/52] fix: skip addGameType until its implemented --- .../contracts-bedrock/test/L1/OPContractsManager.t.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 6283e822bb52a..c88f46b61f532 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -436,6 +436,13 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { uint256 indexed l2ChainId, GameType indexed gameType, IDisputeGame newDisputeGame, IDisputeGame oldDisputeGame ); + function setUp() public virtual override { + super.setUp(); + + // Skip AddGameType tests when V2 dispute games are enabled + skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + } + /// @notice Tests that we can add a PermissionedDisputeGame implementation with addGameType. function test_addGameType_permissioned_succeeds() public { // Create the input for the Permissioned game type. From c9aebaee8e718117c74b7b9dc03054e8d9b2b85e Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 30 Sep 2025 14:56:20 -0400 Subject: [PATCH 42/52] fix: skip updatePrestate until its implemented --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index c88f46b61f532..6731300fc12c2 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -795,6 +795,11 @@ contract OPContractsManager_UpdatePrestate_Test is OPContractsManager_TestInit { function setUp() public override { super.setUp(); + + // Skip UpdatePrestate tests when V2 dispute games enabled + // UpdatePrestate feature not yet implemented for V2 + skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); + prestateUpdater = opcm; } From d2d8781705baa3b52b506718d9d09d848f53b341 Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 30 Sep 2025 15:23:02 -0400 Subject: [PATCH 43/52] fix: remove diff in natspec --- .../contracts-bedrock/src/L1/OPContractsManager.sol | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OPContractsManager.sol b/packages/contracts-bedrock/src/L1/OPContractsManager.sol index 6992480928344..586a7fc056290 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManager.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManager.sol @@ -719,7 +719,8 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase { /// @notice Upgrades a set of chains to the latest implementation contracts /// @param _opChainConfigs Array of OpChain structs, one per chain to upgrade - /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. + /// @dev This function is intended to be DELEGATECALLed by an address that is the common owner of every chain in + /// `_opChainConfigs`'s ProxyAdmin. /// @dev This function requires that each chain's superchainConfig is already upgraded. function upgrade(OPContractsManager.OpChainConfig[] memory _opChainConfigs) external virtual { // Grab the implementations. @@ -887,7 +888,7 @@ contract OPContractsManagerUpgrader is OPContractsManagerBase { /// @notice Upgrades the SuperchainConfig contract. /// @param _superchainConfig The SuperchainConfig contract to upgrade. /// @param _superchainProxyAdmin The ProxyAdmin contract to use for the upgrade. - /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. + /// @dev This function is intended to be DELEGATECALLed by the superchainConfig's ProxyAdminOwner. /// @dev This function will revert if the SuperchainConfig is already at or above the target version. function upgradeSuperchainConfig(ISuperchainConfig _superchainConfig, IProxyAdmin _superchainProxyAdmin) external { // Only upgrade the superchainConfig if the current version is less than the target version. @@ -2010,7 +2011,8 @@ contract OPContractsManager is ISemver { /// @notice Upgrades a set of chains to the latest implementation contracts /// @param _opChainConfigs Array of OpChain structs, one per chain to upgrade - /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. + /// @dev This function is intended to be DELEGATECALLed by an address that is the common owner of every chain in + /// `_opChainConfigs`'s ProxyAdmin. /// @dev This function requires that each chain's superchainConfig is already upgraded. function upgrade(OpChainConfig[] memory _opChainConfigs) external virtual { if (address(this) == address(thisOPCM)) revert OnlyDelegatecall(); @@ -2022,7 +2024,8 @@ contract OPContractsManager is ISemver { /// @notice Upgrades the SuperchainConfig contract. /// @param _superchainConfig The SuperchainConfig contract to upgrade. /// @param _superchainProxyAdmin The ProxyAdmin contract to use for the upgrade. - /// @dev This function is intended to be called via DELEGATECALL from the Upgrade Controller Safe. + /// @dev This function is intended to be DELEGATECALLed by an address that is the common owner of every chain in + /// `_opChainConfigs`'s ProxyAdmin. /// @dev This function will revert if the SuperchainConfig is already at or above the target version. function upgradeSuperchainConfig(ISuperchainConfig _superchainConfig, IProxyAdmin _superchainProxyAdmin) external { if (address(this) == address(thisOPCM)) revert OnlyDelegatecall(); From ebb8185c0751d7ada6ce9ef05a34294ee061a3dd Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 30 Sep 2025 15:27:52 -0400 Subject: [PATCH 44/52] chore: add TODO comments with issue tracking to skips --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 2 ++ .../test/L1/OPContractsManagerStandardValidator.t.sol | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 6731300fc12c2..02c44a7a5e6b1 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -440,6 +440,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { super.setUp(); // Skip AddGameType tests when V2 dispute games are enabled + // TODO(#17260): Remove skip when V2 dispute game support for addGameType implemented skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); } @@ -798,6 +799,7 @@ contract OPContractsManager_UpdatePrestate_Test is OPContractsManager_TestInit { // Skip UpdatePrestate tests when V2 dispute games enabled // UpdatePrestate feature not yet implemented for V2 + // TODO(#17261): Remove skip when V2 dispute game support for updatePrestate implemented skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); prestateUpdater = opcm; diff --git a/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol index 7d219ca0de053..c516d46843fb7 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol @@ -104,6 +104,7 @@ contract OPContractsManagerStandardValidator_TestInit is CommonTest { super.setUp(); // Skip V1 StandardValidator tests when V2 dispute games are enabled + // TODO(#17267): Remove skip when V2 dispute game support added to the StandardValidator is implemented skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); // Grab the deploy input for later use. From 8bf14ec263038f09f761c087485ebefbb7863831 Mon Sep 17 00:00:00 2001 From: steven Date: Tue, 30 Sep 2025 15:57:57 -0400 Subject: [PATCH 45/52] fix: bump semver --- packages/contracts-bedrock/snapshots/semver-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 4d4f5835d4fe4..6d15c3221fd5a 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,8 +20,8 @@ "sourceCodeHash": "0xfca613b5d055ffc4c3cbccb0773ddb9030abedc1aa6508c9e2e7727cc0cd617b" }, "src/L1/OPContractsManager.sol:OPContractsManager": { - "initCodeHash": "0xf414fb79a8941d3ff24f7c6ae65ccee0927c78bd444b20540db9e3376cf2e9e2", - "sourceCodeHash": "0x74a5c3a080db33be6780c9b9bff37c0ce46b4335d6a35148e080a8c2096cd2c8" + "initCodeHash": "0xfea70b2646110201bc6e5cdc33033a764c9bdc1f1c8696c6100f1da5767c3e18", + "sourceCodeHash": "0x49d0020391f7814c2063f33aafc28dfd94cd6e0314e3c05b9ad8fa951d811506" }, "src/L1/OPContractsManagerStandardValidator.sol:OPContractsManagerStandardValidator": { "initCodeHash": "0xcc5dacb9e1c2b9395aa5f9c300f03c18af1ff5a9efd6a7ce4d5135dfbe7b1e2b", From df5f3b15ade734e6f2cc71e6cfb5d7adba442204 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 15:00:09 -0400 Subject: [PATCH 46/52] fix: override inheritance for helper function --- .../test/L1/OPContractsManager.t.sol | 676 +++++++++++------- 1 file changed, 407 insertions(+), 269 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 02c44a7a5e6b1..1748e2266b3d2 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -310,6 +310,125 @@ contract OPContractsManager_Upgrade_Harness is CommonTest { } } +/// @title OPContractsManager_Init +/// @notice Base contract for OPContractsManager tests that need v2 functionality +contract OPContractsManager_Init is DeployOPChain_TestBase { + IOPContractsManager.DeployOutput internal chainDeployOutput1; + IOPContractsManager.DeployOutput internal chainDeployOutput2; + + function setUp() public virtual override { + DeployOPChain_TestBase.setUp(); + + // Initialize DOI with necessary values + doi.set(doi.opChainProxyAdminOwner.selector, opChainProxyAdminOwner); + doi.set(doi.systemConfigOwner.selector, systemConfigOwner); + doi.set(doi.batcher.selector, batcher); + doi.set(doi.unsafeBlockSigner.selector, unsafeBlockSigner); + doi.set(doi.proposer.selector, proposer); + doi.set(doi.challenger.selector, challenger); + doi.set(doi.basefeeScalar.selector, basefeeScalar); + doi.set(doi.blobBaseFeeScalar.selector, blobBaseFeeScalar); + doi.set(doi.l2ChainId.selector, l2ChainId); + doi.set(doi.opcm.selector, address(opcm)); + doi.set(doi.gasLimit.selector, gasLimit); + doi.set(doi.disputeGameType.selector, disputeGameType); + doi.set(doi.disputeAbsolutePrestate.selector, disputeAbsolutePrestate); + doi.set(doi.disputeMaxGameDepth.selector, disputeMaxGameDepth); + doi.set(doi.disputeSplitDepth.selector, disputeSplitDepth); + doi.set(doi.disputeClockExtension.selector, disputeClockExtension); + doi.set(doi.disputeMaxClockDuration.selector, disputeMaxClockDuration); + + chainDeployOutput1 = createChainContracts(100); + chainDeployOutput2 = createChainContracts(101); + + vm.deal(address(chainDeployOutput1.ethLockboxProxy), 100 ether); + vm.deal(address(chainDeployOutput2.ethLockboxProxy), 100 ether); + } + + /// @notice Helper function to deploy OPCM with v2 flag enabled + function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { + // Deploy Superchain contracts first + DeploySuperchain deploySuperchain = new DeploySuperchain(); + DeploySuperchain.Output memory dso = deploySuperchain.run( + DeploySuperchain.Input({ + superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), + protocolVersionsOwner: makeAddr("protocolVersionsOwner"), + guardian: makeAddr("guardian"), + paused: false, + requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), + recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) + }) + ); + + // Deploy implementations with v2 flag enabled + DeployImplementations deployImplementations = new DeployImplementations(); + DeployImplementations.Output memory dio = deployImplementations.run( + DeployImplementations.Input({ + withdrawalDelaySeconds: 100, + minProposalSizeBytes: 200, + challengePeriodSeconds: 300, + proofMaturityDelaySeconds: 400, + disputeGameFinalityDelaySeconds: 500, + mipsVersion: StandardConstants.MIPS_VERSION, + faultGameV2MaxGameDepth: 73, + faultGameV2SplitDepth: 30, + faultGameV2ClockExtension: 10800, + faultGameV2MaxClockDuration: 302400, + superchainConfigProxy: dso.superchainConfigProxy, + protocolVersionsProxy: dso.protocolVersionsProxy, + superchainProxyAdmin: dso.superchainProxyAdmin, + upgradeController: dso.superchainProxyAdmin.owner(), + proposer: proposer, + challenger: challenger, + devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here + }) + ); + + return dio.opcm; + } + + /// @notice Helper function to convert DOI to OPCM deploy input + function toOPCMDeployInput(DeployOPChainInput _doi) + internal + view + virtual + returns (IOPContractsManager.DeployInput memory) + { + return IOPContractsManager.DeployInput({ + roles: IOPContractsManager.Roles({ + opChainProxyAdminOwner: _doi.opChainProxyAdminOwner(), + systemConfigOwner: _doi.systemConfigOwner(), + batcher: _doi.batcher(), + unsafeBlockSigner: _doi.unsafeBlockSigner(), + proposer: _doi.proposer(), + challenger: _doi.challenger() + }), + basefeeScalar: _doi.basefeeScalar(), + blobBasefeeScalar: _doi.blobBaseFeeScalar(), + l2ChainId: _doi.l2ChainId(), + startingAnchorRoot: _doi.startingAnchorRoot(), + saltMixer: _doi.saltMixer(), + gasLimit: _doi.gasLimit(), + disputeGameType: _doi.disputeGameType(), + disputeAbsolutePrestate: _doi.disputeAbsolutePrestate(), + disputeMaxGameDepth: _doi.disputeMaxGameDepth(), + disputeSplitDepth: _doi.disputeSplitDepth(), + disputeClockExtension: _doi.disputeClockExtension(), + disputeMaxClockDuration: _doi.disputeMaxClockDuration() + }); + } + + /// @notice Helper function to deploy a new set of L1 contracts via OPCM. + /// @param _l2ChainId The L2 chain ID to deploy the contracts for. + function createChainContracts(uint256 _l2ChainId) internal returns (IOPContractsManager.DeployOutput memory) { + doi.set(doi.l2ChainId.selector, _l2ChainId); + IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); + deployInput.saltMixer = string.concat("test-", vm.toString(_l2ChainId)); + + return opcm.deploy(deployInput); + } +} + /// @title OPContractsManager_TestInit /// @notice Reusable test initialization for `OPContractsManager` tests. contract OPContractsManager_TestInit is CommonTest { @@ -431,23 +550,15 @@ contract OPContractsManager_ChainIdToBatchInboxAddress_Test is Test { /// @title OPContractsManager_AddGameType_Test /// @notice Tests the `addGameType` function of the `OPContractsManager` contract. -contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { +contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { event GameTypeAdded( uint256 indexed l2ChainId, GameType indexed gameType, IDisputeGame newDisputeGame, IDisputeGame oldDisputeGame ); - function setUp() public virtual override { - super.setUp(); - - // Skip AddGameType tests when V2 dispute games are enabled - // TODO(#17260): Remove skip when V2 dispute game support for addGameType implemented - skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); - } - /// @notice Tests that we can add a PermissionedDisputeGame implementation with addGameType. function test_addGameType_permissioned_succeeds() public { // Create the input for the Permissioned game type. - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.PERMISSIONED_CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(true); // Run the addGameType call. IOPContractsManager.AddGameOutput memory output = addGameType(input); @@ -466,9 +577,9 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { } /// @notice Tests that we can add a FaultDisputeGame implementation with addGameType. - function test_addGameType_cannon_succeeds() public { + function test_addGameType_permissionless_succeeds() public { // Create the input for the Permissionless game type. - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(false); // Run the addGameType call. IOPContractsManager.AddGameOutput memory output = addGameType(input); @@ -488,7 +599,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { /// @notice Tests that we can add a SuperPermissionedDisputeGame implementation with addGameType. function test_addGameType_permissionedSuper_succeeds() public { // Create the input for the Super game type. - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.SUPER_PERMISSIONED_CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(true); input.disputeGameType = GameTypes.SUPER_PERMISSIONED_CANNON; // Since OPCM will start with the standard Permissioned (non-Super) game type we won't have @@ -527,9 +638,10 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { } /// @notice Tests that we can add a SuperFaultDisputeGame implementation with addGameType. - function test_addGameType_superCannon_succeeds() public { + function test_addGameType_permissionlessSuper_succeeds() public { // Create the input for the Super game type. - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.SUPER_CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(false); + input.disputeGameType = GameTypes.SUPER_CANNON; // Run the addGameType call. IOPContractsManager.AddGameOutput memory output = addGameType(input); @@ -549,31 +661,8 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { /// @notice Tests that addGameType will revert if the game type is not supported. function test_addGameType_unsupportedGameType_reverts() public { - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameType.wrap(2000)); - - // Run the addGameType call, should revert. - IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](1); - inputs[0] = input; - (bool success,) = address(opcm).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (inputs))); - assertFalse(success, "addGameType should have failed"); - } - - /// @notice Tests that addGameType will revert if the game type is cannon-kona and the dev feature is not enabled - function test_addGameType_cannonKonaGameTypeDisabled_reverts() public { - skipIfDevFeatureEnabled(DevFeatures.CANNON_KONA); - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.CANNON_KONA); - - // Run the addGameType call, should revert. - IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](1); - inputs[0] = input; - (bool success,) = address(opcm).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (inputs))); - assertFalse(success, "addGameType should have failed"); - } - - /// @notice Tests that addGameType will revert if the game type is cannon-kona and the dev feature is not enabled - function test_addGameType_superCannonKonaGameTypeDisabled_reverts() public { - skipIfDevFeatureEnabled(DevFeatures.CANNON_KONA); - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.SUPER_CANNON_KONA); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(false); + input.disputeGameType = GameType.wrap(2000); // Run the addGameType call, should revert. IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](1); @@ -594,7 +683,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { ) ); vm.etch(address(delayedWETH), hex"01"); - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(false); input.delayedWETH = delayedWETH; IOPContractsManager.AddGameOutput memory output = addGameType(input); assertValidGameType(input, output); @@ -602,8 +691,10 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { } function test_addGameType_outOfOrderInputs_reverts() public { - IOPContractsManager.AddGameInput memory input1 = newGameInputFactory(GameType.wrap(2)); - IOPContractsManager.AddGameInput memory input2 = newGameInputFactory(GameType.wrap(1)); + IOPContractsManager.AddGameInput memory input1 = newGameInputFactory(false); + input1.disputeGameType = GameType.wrap(2); + IOPContractsManager.AddGameInput memory input2 = newGameInputFactory(false); + input2.disputeGameType = GameType.wrap(1); IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](2); inputs[0] = input1; inputs[1] = input2; @@ -614,7 +705,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { } function test_addGameType_duplicateGameType_reverts() public { - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(false); IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](2); inputs[0] = input; inputs[1] = input; @@ -636,7 +727,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { } function test_addGameType_notDelegateCall_reverts() public { - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.PERMISSIONED_CANNON); + IOPContractsManager.AddGameInput memory input = newGameInputFactory(true); IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](1); inputs[0] = input; @@ -644,6 +735,166 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { opcm.addGameType(inputs); } + /// @notice Test that addGameType with v2 flag uses v2 implementation for PERMISSIONED_CANNON + function test_addGameType_withV2Flag_permissionedCannon_succeeds() public { + // Deploy OPCM with v2 flag enabled + IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); + + // Deploy a chain first with v2-enabled OPCM + IOPContractsManager.DeployInput memory deployInput = IOPContractsManager.DeployInput({ + roles: IOPContractsManager.Roles({ + opChainProxyAdminOwner: makeAddr("opChainProxyAdminOwner"), + systemConfigOwner: makeAddr("systemConfigOwner"), + batcher: makeAddr("batcher"), + unsafeBlockSigner: makeAddr("unsafeBlockSigner"), + proposer: makeAddr("proposer"), + challenger: makeAddr("challenger") + }), + basefeeScalar: 100, + blobBasefeeScalar: 200, + l2ChainId: 10001, + startingAnchorRoot: abi.encode(bytes32(uint256(0x123))), + saltMixer: "test-salt", + gasLimit: 30_000_000, + disputeGameType: GameTypes.PERMISSIONED_CANNON, + disputeAbsolutePrestate: Claim.wrap(bytes32(uint256(0x456))), + disputeMaxGameDepth: 73, + disputeSplitDepth: 30, + disputeClockExtension: Duration.wrap(uint64(3 hours)), + disputeMaxClockDuration: Duration.wrap(uint64(3.5 days)) + }); + + vm.startPrank(address(this)); + IOPContractsManager.DeployOutput memory output = opcmV2.deploy(deployInput); + vm.stopPrank(); + + // Get the v2 implementation address from OPCM + IOPContractsManager.Implementations memory impls = opcmV2.implementations(); + + // Prepare to add PERMISSIONED_CANNON game type with a different prestate + IOPContractsManager.AddGameInput[] memory gameConfigs = new IOPContractsManager.AddGameInput[](1); + gameConfigs[0] = IOPContractsManager.AddGameInput({ + disputeGameType: GameTypes.PERMISSIONED_CANNON, + systemConfig: output.systemConfigProxy, + proxyAdmin: output.opChainProxyAdmin, + delayedWETH: IDelayedWETH(payable(address(0))), // Will deploy new one + disputeAbsolutePrestate: Claim.wrap(bytes32(uint256(0x789))), // Different prestate + disputeMaxGameDepth: 73, + disputeSplitDepth: 30, + disputeClockExtension: Duration.wrap(uint64(3 hours)), + disputeMaxClockDuration: Duration.wrap(uint64(3.5 days)), + initialBond: 1 ether, + vm: IBigStepper(makeAddr("vm")), + permissioned: true, + saltMixer: "salt1" + }); + + // Add the game type + (bool success, bytes memory rawGameOut) = + address(opcmV2).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (gameConfigs))); + assertTrue(success, "addGameType failed"); + + IOPContractsManager.AddGameOutput[] memory addGameOutputs = + abi.decode(rawGameOut, (IOPContractsManager.AddGameOutput[])); + + // Verify v2 implementation is registered in DisputeGameFactory + address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); + + // Verify implementation address matches permissionedDisputeGameV2Impl + assertEq( + registeredImpl, + address(impls.permissionedDisputeGameV2Impl), + "DisputeGameFactory should have v2 PermissionedDisputeGame implementation registered" + ); + + // Verify that the returned fault dispute game is the v2 implementation + assertEq( + address(addGameOutputs[0].faultDisputeGame), + address(impls.permissionedDisputeGameV2Impl), + "addGameType should return v2 PermissionedDisputeGame implementation" + ); + } + + /// @notice Test that addGameType with v2 flag uses v2 implementation for CANNON (permissionless) + function test_addGameType_withV2Flag_permissionlessCannon_succeeds() public { + // Deploy OPCM with v2 flag enabled + IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); + + // Deploy a chain first with v2-enabled OPCM + IOPContractsManager.DeployInput memory deployInput = IOPContractsManager.DeployInput({ + roles: IOPContractsManager.Roles({ + opChainProxyAdminOwner: makeAddr("opChainProxyAdminOwner"), + systemConfigOwner: makeAddr("systemConfigOwner"), + batcher: makeAddr("batcher"), + unsafeBlockSigner: makeAddr("unsafeBlockSigner"), + proposer: makeAddr("proposer"), + challenger: makeAddr("challenger") + }), + basefeeScalar: 100, + blobBasefeeScalar: 200, + l2ChainId: 10002, + startingAnchorRoot: abi.encode(bytes32(uint256(0x123))), + saltMixer: "test-salt-2", + gasLimit: 30_000_000, + disputeGameType: GameTypes.PERMISSIONED_CANNON, + disputeAbsolutePrestate: Claim.wrap(bytes32(uint256(0x456))), + disputeMaxGameDepth: 73, + disputeSplitDepth: 30, + disputeClockExtension: Duration.wrap(uint64(3 hours)), + disputeMaxClockDuration: Duration.wrap(uint64(3.5 days)) + }); + + vm.startPrank(address(this)); + IOPContractsManager.DeployOutput memory output = opcmV2.deploy(deployInput); + vm.stopPrank(); + + // Get the v2 implementation address from OPCM + IOPContractsManager.Implementations memory impls = opcmV2.implementations(); + + // Prepare to add CANNON game type (permissionless) + IOPContractsManager.AddGameInput[] memory gameConfigs = new IOPContractsManager.AddGameInput[](1); + gameConfigs[0] = IOPContractsManager.AddGameInput({ + disputeGameType: GameTypes.CANNON, + systemConfig: output.systemConfigProxy, + proxyAdmin: output.opChainProxyAdmin, + delayedWETH: IDelayedWETH(payable(address(0))), // Will deploy new one + disputeAbsolutePrestate: Claim.wrap(bytes32(uint256(0xabc))), // Different prestate + disputeMaxGameDepth: 73, + disputeSplitDepth: 30, + disputeClockExtension: Duration.wrap(uint64(3 hours)), + disputeMaxClockDuration: Duration.wrap(uint64(3.5 days)), + initialBond: 1 ether, + vm: IBigStepper(makeAddr("vm")), + permissioned: false, // Permissionless CANNON + saltMixer: "salt2" + }); + + // Add the game type + (bool success, bytes memory rawGameOut) = + address(opcmV2).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (gameConfigs))); + assertTrue(success, "addGameType failed"); + + IOPContractsManager.AddGameOutput[] memory addGameOutputs = + abi.decode(rawGameOut, (IOPContractsManager.AddGameOutput[])); + + // Verify v2 implementation is registered in DisputeGameFactory + address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.CANNON)); + + // Verify implementation address matches faultDisputeGameV2Impl + assertEq( + registeredImpl, + address(impls.faultDisputeGameV2Impl), + "DisputeGameFactory should have v2 FaultDisputeGame implementation registered" + ); + + // Verify that the returned fault dispute game is the v2 implementation + assertEq( + address(addGameOutputs[0].faultDisputeGame), + address(impls.faultDisputeGameV2Impl), + "addGameType should return v2 FaultDisputeGame implementation" + ); + } + function addGameType(IOPContractsManager.AddGameInput memory input) internal returns (IOPContractsManager.AddGameOutput memory) @@ -669,13 +920,13 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { return addGameOutAll[0]; } - function newGameInputFactory(GameType _gameType) internal view returns (IOPContractsManager.AddGameInput memory) { + function newGameInputFactory(bool permissioned) internal view returns (IOPContractsManager.AddGameInput memory) { return IOPContractsManager.AddGameInput({ saltMixer: "hello", systemConfig: chainDeployOutput1.systemConfigProxy, proxyAdmin: chainDeployOutput1.opChainProxyAdmin, delayedWETH: IDelayedWETH(payable(address(0))), - disputeGameType: _gameType, + disputeGameType: GameType.wrap(permissioned ? 1 : 0), disputeAbsolutePrestate: Claim.wrap(bytes32(hex"deadbeef1234")), disputeMaxGameDepth: 73, disputeSplitDepth: 30, @@ -683,8 +934,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { disputeMaxClockDuration: Duration.wrap(302400), initialBond: 1 ether, vm: IBigStepper(address(opcm.implementations().mipsImpl)), - permissioned: _gameType.raw() == GameTypes.PERMISSIONED_CANNON.raw() - || _gameType.raw() == GameTypes.SUPER_PERMISSIONED_CANNON.raw() + permissioned: permissioned }); } @@ -738,56 +988,6 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_TestInit { } } -/// @title OPContractsManager_AddGameTypeCannonKonaEnabled_Test -/// @notice Tests the `addGameType` function of the `OPContractsManager` contract with CANNON_KONA enabled. -contract OPContractsManager_AddGameType_CannonKonaEnabled_Test is OPContractsManager_AddGameType_Test { - function setUp() public override { - setDevFeatureEnabled(DevFeatures.CANNON_KONA); - super.setUp(); - } - - /// @notice Tests that addGameType will revert if the game type is cannon-kona and the dev feature is not enabled - function test_addGameType_cannonKonaGameType_succeeds() public { - // Create the input for the cannon-kona game type. - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.CANNON_KONA); - - // Run the addGameType call. - IOPContractsManager.AddGameOutput memory output = addGameType(input); - assertValidGameType(input, output); - - // Check the values on the new game type. - IPermissionedDisputeGame notPDG = IPermissionedDisputeGame(address(output.faultDisputeGame)); - - // Proposer call should revert because this is a permissionless game. - vm.expectRevert(); // nosemgrep: sol-safety-expectrevert-no-args - notPDG.proposer(); - - // L2 chain ID call should not revert because this is not a Super game. - assertNotEq(notPDG.l2ChainId(), 0, "l2ChainId should not be zero"); - } - - /// @notice Tests that addGameType will revert if the game type is cannon-kona and the dev feature is not enabled - function test_addGameType_superCannonKonaGameType_succeeds() public { - // Create the input for the cannon-kona game type. - IOPContractsManager.AddGameInput memory input = newGameInputFactory(GameTypes.SUPER_CANNON_KONA); - - // Run the addGameType call. - IOPContractsManager.AddGameOutput memory output = addGameType(input); - assertValidGameType(input, output); - - // Grab the new game type. - IPermissionedDisputeGame notPDG = IPermissionedDisputeGame(address(output.faultDisputeGame)); - - // Proposer should fail, this is a permissionless game. - vm.expectRevert(); // nosemgrep: sol-safety-expectrevert-no-args - notPDG.proposer(); - - // Super games don't have the l2ChainId function. - vm.expectRevert(); // nosemgrep: sol-safety-expectrevert-no-args - notPDG.l2ChainId(); - } -} - /// @title OPContractsManager_UpdatePrestate_Test /// @notice Tests the `updatePrestate` function of the `OPContractsManager` contract. contract OPContractsManager_UpdatePrestate_Test is OPContractsManager_TestInit { @@ -796,12 +996,6 @@ contract OPContractsManager_UpdatePrestate_Test is OPContractsManager_TestInit { function setUp() public override { super.setUp(); - - // Skip UpdatePrestate tests when V2 dispute games enabled - // UpdatePrestate feature not yet implemented for V2 - // TODO(#17261): Remove skip when V2 dispute game support for updatePrestate implemented - skipIfDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES); - prestateUpdater = opcm; } @@ -1296,7 +1490,7 @@ contract OPContractsManager_Migrate_Test is OPContractsManager_TestInit { Claim absolutePrestate2 = Claim.wrap(bytes32(hex"DEAD")); /// @notice Function requires interop portal. - function setUp() public virtual override { + function setUp() public override { super.setUp(); skipIfDevFeatureDisabled(DevFeatures.OPTIMISM_PORTAL_INTEROP); } @@ -1371,11 +1565,6 @@ contract OPContractsManager_Migrate_Test is OPContractsManager_TestInit { assertEq(address(_disputeGameFactory.gameImpls(GameTypes.SUPER_CANNON)), address(0)); assertEq(address(_disputeGameFactory.gameImpls(GameTypes.PERMISSIONED_CANNON)), address(0)); assertEq(address(_disputeGameFactory.gameImpls(GameTypes.SUPER_PERMISSIONED_CANNON)), address(0)); - if (isDevFeatureEnabled(DevFeatures.CANNON_KONA)) { - // Only explicitly zeroed out if feature is enabled. Otherwise left unchanged (which may still be 0). - assertEq(address(_disputeGameFactory.gameImpls(GameTypes.CANNON_KONA)), address(0)); - assertEq(address(_disputeGameFactory.gameImpls(GameTypes.SUPER_CANNON_KONA)), address(0)); - } } /// @notice Tests that the migration function succeeds when requesting to use the @@ -1668,46 +1857,7 @@ contract OPContractsManager_Migrate_Test is OPContractsManager_TestInit { } } -/// @title OPContractsManager_Migrate_CannonKonaEnabled_Test -/// @notice Tests the `migrate` function of the `OPContractsManager` contract. -contract OPContractsManager_Migrate_CannonKonaEnabled_Test is OPContractsManager_Migrate_Test { - function setUp() public override { - setDevFeatureEnabled(DevFeatures.CANNON_KONA); - super.setUp(); - } - - function test_migrate_zerosOutCannonKonaGameTypes_succeeds() public { - IOPContractsManagerInteropMigrator.MigrateInput memory input = _getDefaultInput(); - - // Grab the existing DisputeGameFactory for each chain. - IDisputeGameFactory oldDisputeGameFactory1 = - IDisputeGameFactory(payable(chainDeployOutput1.systemConfigProxy.disputeGameFactory())); - IDisputeGameFactory oldDisputeGameFactory2 = - IDisputeGameFactory(payable(chainDeployOutput2.systemConfigProxy.disputeGameFactory())); - // Ensure cannon kona games have implementations - oldDisputeGameFactory1.setImplementation(GameTypes.CANNON_KONA, IDisputeGame(address(1))); - oldDisputeGameFactory2.setImplementation(GameTypes.CANNON_KONA, IDisputeGame(address(1))); - oldDisputeGameFactory1.setImplementation(GameTypes.SUPER_CANNON_KONA, IDisputeGame(address(2))); - oldDisputeGameFactory2.setImplementation(GameTypes.SUPER_CANNON_KONA, IDisputeGame(address(2))); - - // Execute the migration. - _doMigration(input); - - // Assert that the old game implementations are now zeroed out. - _assertOldGamesZeroed(oldDisputeGameFactory1); - _assertOldGamesZeroed(oldDisputeGameFactory2); - } -} - -/// @title OPContractsManager_Deploy_Test -/// @notice Tests the `deploy` function of the `OPContractsManager` contract. -/// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This -/// is because OPContractsManager acts as a deploy script, so we start from a clean slate here -/// and work OPContractsManager's deployment into the existing test setup, instead of using -/// the existing test setup to deploy OPContractsManager. We do however inherit from -/// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly -/// to how a real deployment would happen. -contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { +contract OPContractsManager_DeployBase is DeployOPChain_TestBase { using stdStorage for StdStorage; event Deployed(uint256 indexed l2ChainId, address indexed deployer, bytes deployOutput); @@ -1740,6 +1890,7 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { function toOPCMDeployInput(DeployOPChainInput _doi) internal view + virtual returns (IOPContractsManager.DeployInput memory) { return IOPContractsManager.DeployInput({ @@ -1766,75 +1917,98 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { }); } - /// @notice Helper function to deploy OPCM with v2 flag enabled - function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { - // Deploy Superchain contracts first - DeploySuperchain deploySuperchain = new DeploySuperchain(); - DeploySuperchain.Output memory dso = deploySuperchain.run( - DeploySuperchain.Input({ - superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), - protocolVersionsOwner: makeAddr("protocolVersionsOwner"), - guardian: makeAddr("guardian"), - paused: false, - requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), - recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) - }) - ); - - // Deploy implementations with v2 flag enabled - DeployImplementations deployImplementations = new DeployImplementations(); - DeployImplementations.Output memory dio = deployImplementations.run( - DeployImplementations.Input({ - withdrawalDelaySeconds: 100, - minProposalSizeBytes: 200, - challengePeriodSeconds: 300, - proofMaturityDelaySeconds: 400, - disputeGameFinalityDelaySeconds: 500, - mipsVersion: StandardConstants.MIPS_VERSION, - faultGameV2MaxGameDepth: 73, - faultGameV2SplitDepth: 30, - faultGameV2ClockExtension: 10800, - faultGameV2MaxClockDuration: 302400, - superchainConfigProxy: dso.superchainConfigProxy, - protocolVersionsProxy: dso.protocolVersionsProxy, - superchainProxyAdmin: dso.superchainProxyAdmin, - upgradeController: dso.superchainProxyAdmin.owner(), - challenger: challenger, - devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here - }) - ); - - return dio.opcm; + function test_deploy_l2ChainIdEqualsZero_reverts() public { + IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); + deployInput.l2ChainId = 0; + vm.expectRevert(IOPContractsManager.InvalidChainId.selector); + opcm.deploy(deployInput); } - /// @notice Helper function to create a permissioned game v2 through the factory - function _createPermissionedGameV2( - IDisputeGameFactory factory, - address proposer - ) - internal - returns (IPermissionedDisputeGame) - { - // Check if there's an init bond required for the game type - uint256 initBond = factory.initBonds(GameTypes.PERMISSIONED_CANNON); + function test_deploy_l2ChainIdEqualsCurrentChainId_reverts() public { + IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); + deployInput.l2ChainId = block.chainid; - // Fund the proposer if needed - if (initBond > 0) { - vm.deal(proposer, initBond); - } + vm.expectRevert(IOPContractsManager.InvalidChainId.selector); + opcm.deploy(deployInput); + } - // We use vm.startPrank to set both msg.sender and tx.origin to the proposer - vm.startPrank(proposer, proposer); + function test_deploy_succeeds() public { + vm.expectEmit(true, true, true, false); // TODO precompute the expected `deployOutput`. + emit Deployed(doi.l2ChainId(), address(this), bytes("")); + opcm.deploy(toOPCMDeployInput(doi)); + } +} - IDisputeGame gameProxy = factory.create{ value: initBond }( - GameTypes.PERMISSIONED_CANNON, Claim.wrap(bytes32(uint256(1))), abi.encode(bytes32(uint256(2))) - ); +/// @title OPContractsManager_Version_Test +/// @notice Tests the `version` function of the `OPContractsManager` contract. +contract OPContractsManager_Version_Test is OPContractsManager_TestInit { + IOPContractsManager internal prestateUpdater; + OPContractsManager.AddGameInput[] internal gameInput; - vm.stopPrank(); + function setUp() public override { + super.setUp(); + prestateUpdater = opcm; + } - return IPermissionedDisputeGame(address(gameProxy)); + function test_semver_works() public view { + assertNotEq(abi.encode(prestateUpdater.version()), abi.encode(0)); } +} + +/// @title OPContractsManager_V2_Test +/// @notice Tests for v2 dispute game implementations in OPContractsManager +contract OPContractsManager_V2_Test is OPContractsManager_Init { + /// @notice Helper function to deploy OPCM with v2 flag enabled + // function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { + // // Deploy Superchain contracts first + // DeploySuperchain deploySuperchain = new DeploySuperchain(); + // DeploySuperchain.Output memory dso = deploySuperchain.run( + // DeploySuperchain.Input({ + // superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), + // protocolVersionsOwner: makeAddr("protocolVersionsOwner"), + // guardian: makeAddr("guardian"), + // paused: false, + // requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), + // recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) + // }) + // ); + + // // Deploy implementations with v2 flag enabled + // DeployImplementations deployImplementations = new DeployImplementations(); + // DeployImplementations.Output memory dio = deployImplementations.run( + // DeployImplementations.Input({ + // withdrawalDelaySeconds: 100, + // minProposalSizeBytes: 200, + // challengePeriodSeconds: 300, + // proofMaturityDelaySeconds: 400, + // disputeGameFinalityDelaySeconds: 500, + // mipsVersion: StandardConstants.MIPS_VERSION, + // faultGameV2MaxGameDepth: 73, + // faultGameV2SplitDepth: 30, + // faultGameV2ClockExtension: 10800, + // faultGameV2MaxClockDuration: 302400, + // superchainConfigProxy: dso.superchainConfigProxy, + // protocolVersionsProxy: dso.protocolVersionsProxy, + // superchainProxyAdmin: dso.superchainProxyAdmin, + // upgradeController: dso.superchainProxyAdmin.owner(), + // proposer: proposer, + // challenger: challenger, + // devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here + // }) + // ); + + // return dio.opcm; + // } +/// @title OPContractsManager_Deploy_Test +/// @notice Tests the `deploy` function of the `OPContractsManager` contract. +/// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This +/// is because OPContractsManager acts as a deploy script, so we start from a clean slate here +/// and work OPContractsManager's deployment into the existing test setup, instead of using +/// the existing test setup to deploy OPContractsManager. We do however inherit from +/// DeployOPChain_TestBase so we can use its setup to deploy the implementations similarly +/// to how a real deployment would happen. +contract OPContractsManager_Deploy_Test is OPContractsManager_DeployBase { function test_deploy_l2ChainIdEqualsZero_reverts() public { IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); deployInput.l2ChainId = 0; @@ -1855,13 +2029,13 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { emit Deployed(doi.l2ChainId(), address(this), bytes("")); opcm.deploy(toOPCMDeployInput(doi)); } - - /// @notice Test that deploy without v2 flag doesn't set v2 implementations for PERMISSIONED_CANNON - function test_deployPermissionedWithoutV2Flag_succeeds() public { + /// @notice Test that deploy without v2 flag doesn't set v2 implementations + function test_deployWithoutV2Flag_succeeds() public { // Convert DOI to OPCM input and deploy IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); + vm.startPrank(address(this)); IOPContractsManager.DeployOutput memory output = opcm.deploy(opcmInput); - IOPContractsManager.Implementations memory impls = opcm.implementations(); + vm.stopPrank(); // Check that v2 implementations are not set (since flag is not enabled by default) assertEq(address(output.permissionedDisputeGameV2), address(0)); @@ -1870,38 +2044,10 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { // Check that v1 implementation is registered for PERMISSIONED_CANNON address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); assertEq(registeredImpl, address(output.permissionedDisputeGame)); - - address registeredPermissionedImpl = - address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); - assertNotEq( - registeredPermissionedImpl, - address(0), - "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" - ); - assertEq( - registeredPermissionedImpl, address(output.permissionedDisputeGame), "Should be using v2 implementation" - ); - - // Create a game proxy to test immutable fields - IPermissionedDisputeGame gameV2Proxy = - _createPermissionedGameV2(output.disputeGameFactoryProxy, opcmInput.roles.proposer); - - // Verify immutable fields on the game proxy - assertEq(address(gameV2Proxy.vm()), address(impls.mipsImpl), "VM should match MIPS implementation"); - assertEq( - address(gameV2Proxy.anchorStateRegistry()), address(output.anchorStateRegistryProxy), "ASR should match" - ); - assertEq(address(gameV2Proxy.weth()), address(output.delayedWETHPermissionedGameProxy), "WETH should match"); - assertEq(gameV2Proxy.l2ChainId(), opcmInput.l2ChainId, "L2 chain ID should match"); - - // For permissioned game, check proposer and challenger - IPermissionedDisputeGame permissionedProxy = IPermissionedDisputeGame(address(gameV2Proxy)); - assertEq(permissionedProxy.proposer(), opcmInput.roles.proposer, "Proposer should match"); - assertEq(permissionedProxy.challenger(), opcmInput.roles.challenger, "Challenger should match"); } - /// @notice Test that deploy with v2 flag would set v2 implementations for PERMISSIONED_CANNON - function test_deployPermissionedWithV2Flag_succeeds() public { + /// @notice Test that deploy with v2 flag would set v2 implementations + function test_deployWithV2Flag_succeeds() public { IOPContractsManager opcmV2 = _deployOPCMWithV2Flag(); assertTrue(opcmV2.isDevFeatureEnabled(DevFeatures.DEPLOY_V2_DISPUTE_GAMES), "V2 flag should be enabled"); @@ -1920,37 +2066,29 @@ contract OPContractsManager_Deploy_Test is DeployOPChain_TestBase { // Set up deploy input for the v2-enabled OPCM doi.set(doi.opcm.selector, address(opcmV2)); IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi); + + vm.startPrank(address(this)); IOPContractsManager.DeployOutput memory output = opcmV2.deploy(opcmInput); + vm.stopPrank(); + + // Verify that v2 dispute game contracts are deployed and non-zero + assertTrue( + address(output.permissionedDisputeGameV2) != address(0), "PermissionedDisputeGameV2 should be deployed" + ); + assertTrue(address(output.faultDisputeGameV2) != address(0), "FaultDisputeGameV2 should be deployed"); + + // Verify that v1 permissioned dispute game is still deployed (for backward compatibility) + assertTrue( + address(output.permissionedDisputeGame) != address(0), "PermissionedDisputeGame v1 should still be deployed" + ); // Verify that the DisputeGameFactory has registered the v2 implementation for PERMISSIONED_CANNON game type address registeredPermissionedImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); - assertNotEq( - registeredPermissionedImpl, - address(0), - "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" - ); assertEq( registeredPermissionedImpl, - address(impls.permissionedDisputeGameV2Impl), - "Should be using v2 implementation" - ); - - // Create a game proxy to test immutable fields - IPermissionedDisputeGame gameV2Proxy = - _createPermissionedGameV2(output.disputeGameFactoryProxy, opcmInput.roles.proposer); - - // Verify immutable fields on the game proxy - assertEq(address(gameV2Proxy.vm()), address(impls.mipsImpl), "VM should match MIPS implementation"); - assertEq( - address(gameV2Proxy.anchorStateRegistry()), address(output.anchorStateRegistryProxy), "ASR should match" + address(output.permissionedDisputeGameV2), + "DisputeGameFactory should have v2 PermissionedDisputeGame registered for PERMISSIONED_CANNON" ); - assertEq(address(gameV2Proxy.weth()), address(output.delayedWETHPermissionedGameProxy), "WETH should match"); - assertEq(gameV2Proxy.l2ChainId(), opcmInput.l2ChainId, "L2 chain ID should match"); - - // For permissioned game, check proposer and challenger - IPermissionedDisputeGame permissionedProxy = IPermissionedDisputeGame(address(gameV2Proxy)); - assertEq(permissionedProxy.proposer(), opcmInput.roles.proposer, "Proposer should match"); - assertEq(permissionedProxy.challenger(), opcmInput.roles.challenger, "Challenger should match"); } } From 7bb7ca46a43de3bcba7166cf0225676da382ae80 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 15:24:04 -0400 Subject: [PATCH 47/52] chore: remove commented function --- .../test/L1/OPContractsManager.t.sol | 42 +------------------ 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 1748e2266b3d2..16a07d1f91fda 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -1958,47 +1958,7 @@ contract OPContractsManager_Version_Test is OPContractsManager_TestInit { /// @title OPContractsManager_V2_Test /// @notice Tests for v2 dispute game implementations in OPContractsManager contract OPContractsManager_V2_Test is OPContractsManager_Init { - /// @notice Helper function to deploy OPCM with v2 flag enabled - // function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { - // // Deploy Superchain contracts first - // DeploySuperchain deploySuperchain = new DeploySuperchain(); - // DeploySuperchain.Output memory dso = deploySuperchain.run( - // DeploySuperchain.Input({ - // superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), - // protocolVersionsOwner: makeAddr("protocolVersionsOwner"), - // guardian: makeAddr("guardian"), - // paused: false, - // requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), - // recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) - // }) - // ); - - // // Deploy implementations with v2 flag enabled - // DeployImplementations deployImplementations = new DeployImplementations(); - // DeployImplementations.Output memory dio = deployImplementations.run( - // DeployImplementations.Input({ - // withdrawalDelaySeconds: 100, - // minProposalSizeBytes: 200, - // challengePeriodSeconds: 300, - // proofMaturityDelaySeconds: 400, - // disputeGameFinalityDelaySeconds: 500, - // mipsVersion: StandardConstants.MIPS_VERSION, - // faultGameV2MaxGameDepth: 73, - // faultGameV2SplitDepth: 30, - // faultGameV2ClockExtension: 10800, - // faultGameV2MaxClockDuration: 302400, - // superchainConfigProxy: dso.superchainConfigProxy, - // protocolVersionsProxy: dso.protocolVersionsProxy, - // superchainProxyAdmin: dso.superchainProxyAdmin, - // upgradeController: dso.superchainProxyAdmin.owner(), - // proposer: proposer, - // challenger: challenger, - // devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here - // }) - // ); - - // return dio.opcm; - // } +} /// @title OPContractsManager_Deploy_Test /// @notice Tests the `deploy` function of the `OPContractsManager` contract. From 2ae7354e45e338ed6e7752bd31a36d7a21340fc7 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 18:49:51 -0400 Subject: [PATCH 48/52] fix: test setup --- .../test/L1/OPContractsManager.t.sol | 51 +++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 16a07d1f91fda..06c4044fb1d2c 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -315,12 +315,14 @@ contract OPContractsManager_Upgrade_Harness is CommonTest { contract OPContractsManager_Init is DeployOPChain_TestBase { IOPContractsManager.DeployOutput internal chainDeployOutput1; IOPContractsManager.DeployOutput internal chainDeployOutput2; + IDelayedWETH internal standaloneDelayedWETH; function setUp() public virtual override { DeployOPChain_TestBase.setUp(); // Initialize DOI with necessary values - doi.set(doi.opChainProxyAdminOwner.selector, opChainProxyAdminOwner); + // Set the test contract as the owner for delegatecall compatibility + doi.set(doi.opChainProxyAdminOwner.selector, address(this)); doi.set(doi.systemConfigOwner.selector, systemConfigOwner); doi.set(doi.batcher.selector, batcher); doi.set(doi.unsafeBlockSigner.selector, unsafeBlockSigner); @@ -343,6 +345,27 @@ contract OPContractsManager_Init is DeployOPChain_TestBase { vm.deal(address(chainDeployOutput1.ethLockboxProxy), 100 ether); vm.deal(address(chainDeployOutput2.ethLockboxProxy), 100 ether); + + // Deploy a standalone DelayedWETH for tests to avoid ProxyAdmin ownership issues + standaloneDelayedWETH = deployStandaloneDelayedWETH(); + } + + /// @notice Deploy a standalone DelayedWETH contract for testing + function deployStandaloneDelayedWETH() internal returns (IDelayedWETH) { + // Deploy proxy + address delayedWETHProxy = DeployUtils.create1({ + _name: "Proxy", + _args: DeployUtils.encodeConstructor(abi.encodeCall(IProxy.__constructor__, (address(this)))) + }); + + // Deploy implementation + address delayedWETHImpl = address(opcm.implementations().delayedWETHImpl); + + // Initialize the proxy with the implementation + IProxy(payable(delayedWETHProxy)).upgradeTo(delayedWETHImpl); + IDelayedWETH(payable(delayedWETHProxy)).initialize(chainDeployOutput1.systemConfigProxy); + + return IDelayedWETH(payable(delayedWETHProxy)); } /// @notice Helper function to deploy OPCM with v2 flag enabled @@ -434,6 +457,7 @@ contract OPContractsManager_Init is DeployOPChain_TestBase { contract OPContractsManager_TestInit is CommonTest { IOPContractsManager.DeployOutput internal chainDeployOutput1; IOPContractsManager.DeployOutput internal chainDeployOutput2; + IDelayedWETH internal standaloneDelayedWETH; function setUp() public virtual override { super.setUp(); @@ -443,6 +467,27 @@ contract OPContractsManager_TestInit is CommonTest { vm.deal(address(chainDeployOutput1.ethLockboxProxy), 100 ether); vm.deal(address(chainDeployOutput2.ethLockboxProxy), 100 ether); + + // Deploy a standalone DelayedWETH for tests to avoid ProxyAdmin ownership issues + standaloneDelayedWETH = deployStandaloneDelayedWETH(); + } + + /// @notice Deploy a standalone DelayedWETH contract for testing + function deployStandaloneDelayedWETH() internal returns (IDelayedWETH) { + // Deploy proxy + address delayedWETHProxy = DeployUtils.create1({ + _name: "Proxy", + _args: DeployUtils.encodeConstructor(abi.encodeCall(IProxy.__constructor__, (address(this)))) + }); + + // Deploy implementation + address delayedWETHImpl = address(opcm.implementations().delayedWETHImpl); + + // Initialize the proxy with the implementation + IProxy(payable(delayedWETHProxy)).upgradeTo(delayedWETHImpl); + IDelayedWETH(payable(delayedWETHProxy)).initialize(chainDeployOutput1.systemConfigProxy); + + return IDelayedWETH(payable(delayedWETHProxy)); } /// @notice Sets up the environment variables for the VerifyOPCM test. @@ -925,7 +970,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { saltMixer: "hello", systemConfig: chainDeployOutput1.systemConfigProxy, proxyAdmin: chainDeployOutput1.opChainProxyAdmin, - delayedWETH: IDelayedWETH(payable(address(0))), + delayedWETH: standaloneDelayedWETH, disputeGameType: GameType.wrap(permissioned ? 1 : 0), disputeAbsolutePrestate: Claim.wrap(bytes32(hex"deadbeef1234")), disputeMaxGameDepth: 73, @@ -1232,7 +1277,7 @@ contract OPContractsManager_UpdatePrestate_Test is OPContractsManager_TestInit { saltMixer: "hello", systemConfig: chainDeployOutput1.systemConfigProxy, proxyAdmin: chainDeployOutput1.opChainProxyAdmin, - delayedWETH: IDelayedWETH(payable(address(0))), + delayedWETH: standaloneDelayedWETH, disputeGameType: GameType.wrap(permissioned ? 1 : 0), disputeAbsolutePrestate: Claim.wrap(bytes32(hex"deadbeef1234")), disputeMaxGameDepth: 73, From b25ecac3c18df42c84f3aee574639718f3cda244 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 17 Sep 2025 21:44:14 -0400 Subject: [PATCH 49/52] fix: add game type v2 test --- .../test/L1/OPContractsManager.t.sol | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 06c4044fb1d2c..ff5489533d22d 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -798,7 +798,10 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { basefeeScalar: 100, blobBasefeeScalar: 200, l2ChainId: 10001, - startingAnchorRoot: abi.encode(bytes32(uint256(0x123))), + startingAnchorRoot: abi.encode(Proposal({ + root: Hash.wrap(bytes32(uint256(0x123))), + l2SequenceNumber: 1 + })), saltMixer: "test-salt", gasLimit: 30_000_000, disputeGameType: GameTypes.PERMISSIONED_CANNON, @@ -822,7 +825,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { disputeGameType: GameTypes.PERMISSIONED_CANNON, systemConfig: output.systemConfigProxy, proxyAdmin: output.opChainProxyAdmin, - delayedWETH: IDelayedWETH(payable(address(0))), // Will deploy new one + delayedWETH: output.delayedWETHPermissionedGameProxy, // Use existing DelayedWETH to avoid proxy upgrade disputeAbsolutePrestate: Claim.wrap(bytes32(uint256(0x789))), // Different prestate disputeMaxGameDepth: 73, disputeSplitDepth: 30, @@ -834,7 +837,12 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { saltMixer: "salt1" }); - // Add the game type + // Transfer DisputeGameFactory ownership to test contract temporarily for delegatecall + // This is needed because addGameType uses delegatecall and needs to call setImplementation + vm.prank(makeAddr("opChainProxyAdminOwner")); + output.disputeGameFactoryProxy.transferOwnership(address(this)); + + // Add the game type via delegatecall (bool success, bytes memory rawGameOut) = address(opcmV2).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (gameConfigs))); assertTrue(success, "addGameType failed"); @@ -842,6 +850,9 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { IOPContractsManager.AddGameOutput[] memory addGameOutputs = abi.decode(rawGameOut, (IOPContractsManager.AddGameOutput[])); + // Transfer DisputeGameFactory ownership back to the original owner + output.disputeGameFactoryProxy.transferOwnership(makeAddr("opChainProxyAdminOwner")); + // Verify v2 implementation is registered in DisputeGameFactory address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.PERMISSIONED_CANNON)); @@ -878,7 +889,10 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { basefeeScalar: 100, blobBasefeeScalar: 200, l2ChainId: 10002, - startingAnchorRoot: abi.encode(bytes32(uint256(0x123))), + startingAnchorRoot: abi.encode(Proposal({ + root: Hash.wrap(bytes32(uint256(0x123))), + l2SequenceNumber: 1 + })), saltMixer: "test-salt-2", gasLimit: 30_000_000, disputeGameType: GameTypes.PERMISSIONED_CANNON, @@ -902,7 +916,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { disputeGameType: GameTypes.CANNON, systemConfig: output.systemConfigProxy, proxyAdmin: output.opChainProxyAdmin, - delayedWETH: IDelayedWETH(payable(address(0))), // Will deploy new one + delayedWETH: output.delayedWETHPermissionedGameProxy, // Use existing DelayedWETH to avoid proxy upgrade disputeAbsolutePrestate: Claim.wrap(bytes32(uint256(0xabc))), // Different prestate disputeMaxGameDepth: 73, disputeSplitDepth: 30, @@ -914,7 +928,12 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { saltMixer: "salt2" }); - // Add the game type + // Transfer DisputeGameFactory ownership to test contract temporarily for delegatecall + // This is needed because addGameType uses delegatecall and needs to call setImplementation + vm.prank(makeAddr("opChainProxyAdminOwner")); + output.disputeGameFactoryProxy.transferOwnership(address(this)); + + // Add the game type via delegatecall (bool success, bytes memory rawGameOut) = address(opcmV2).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (gameConfigs))); assertTrue(success, "addGameType failed"); @@ -922,6 +941,9 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { IOPContractsManager.AddGameOutput[] memory addGameOutputs = abi.decode(rawGameOut, (IOPContractsManager.AddGameOutput[])); + // Transfer DisputeGameFactory ownership back to the original owner + output.disputeGameFactoryProxy.transferOwnership(makeAddr("opChainProxyAdminOwner")); + // Verify v2 implementation is registered in DisputeGameFactory address registeredImpl = address(output.disputeGameFactoryProxy.gameImpls(GameTypes.CANNON)); From e9170b2941eaa167e8bcb16bf1ebd8ecfe1b0078 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 19 Sep 2025 10:32:42 -0400 Subject: [PATCH 50/52] fix: correct v2 dispute game test setup for addGameType --- .../contracts-bedrock/test/L1/OPContractsManager.t.sol | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index ff5489533d22d..cee8c8d3cb7cd 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -798,10 +798,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { basefeeScalar: 100, blobBasefeeScalar: 200, l2ChainId: 10001, - startingAnchorRoot: abi.encode(Proposal({ - root: Hash.wrap(bytes32(uint256(0x123))), - l2SequenceNumber: 1 - })), + startingAnchorRoot: abi.encode(Proposal({ root: Hash.wrap(bytes32(uint256(0x123))), l2SequenceNumber: 1 })), saltMixer: "test-salt", gasLimit: 30_000_000, disputeGameType: GameTypes.PERMISSIONED_CANNON, @@ -889,10 +886,7 @@ contract OPContractsManager_AddGameType_Test is OPContractsManager_Init { basefeeScalar: 100, blobBasefeeScalar: 200, l2ChainId: 10002, - startingAnchorRoot: abi.encode(Proposal({ - root: Hash.wrap(bytes32(uint256(0x123))), - l2SequenceNumber: 1 - })), + startingAnchorRoot: abi.encode(Proposal({ root: Hash.wrap(bytes32(uint256(0x123))), l2SequenceNumber: 1 })), saltMixer: "test-salt-2", gasLimit: 30_000_000, disputeGameType: GameTypes.PERMISSIONED_CANNON, From 681da743a5110fda034500225ef14d9f264aad02 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 19 Sep 2025 11:24:54 -0400 Subject: [PATCH 51/52] fix: conflict from duplicate helper --- .../test/L1/OPContractsManager.t.sol | 56 +++++++++++++------ 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index cee8c8d3cb7cd..2a456b0d1d79b 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -29,6 +29,7 @@ import { DevFeatures } from "src/libraries/DevFeatures.sol"; import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol"; import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol"; import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; +import { IProxy } from "interfaces/universal/IProxy.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol"; @@ -1978,25 +1979,46 @@ contract OPContractsManager_DeployBase is DeployOPChain_TestBase { }); } - function test_deploy_l2ChainIdEqualsZero_reverts() public { - IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); - deployInput.l2ChainId = 0; - vm.expectRevert(IOPContractsManager.InvalidChainId.selector); - opcm.deploy(deployInput); - } - - function test_deploy_l2ChainIdEqualsCurrentChainId_reverts() public { - IOPContractsManager.DeployInput memory deployInput = toOPCMDeployInput(doi); - deployInput.l2ChainId = block.chainid; + /// @notice Helper function to deploy OPCM with v2 flag enabled + function _deployOPCMWithV2Flag() internal returns (IOPContractsManager) { + // Deploy Superchain contracts first + DeploySuperchain deploySuperchain = new DeploySuperchain(); + DeploySuperchain.Output memory dso = deploySuperchain.run( + DeploySuperchain.Input({ + superchainProxyAdminOwner: makeAddr("superchainProxyAdminOwner"), + protocolVersionsOwner: makeAddr("protocolVersionsOwner"), + guardian: makeAddr("guardian"), + paused: false, + requiredProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(1))), + recommendedProtocolVersion: bytes32(ProtocolVersion.unwrap(ProtocolVersion.wrap(2))) + }) + ); - vm.expectRevert(IOPContractsManager.InvalidChainId.selector); - opcm.deploy(deployInput); - } + // Deploy implementations with v2 flag enabled + DeployImplementations deployImplementations = new DeployImplementations(); + DeployImplementations.Output memory dio = deployImplementations.run( + DeployImplementations.Input({ + withdrawalDelaySeconds: 100, + minProposalSizeBytes: 200, + challengePeriodSeconds: 300, + proofMaturityDelaySeconds: 400, + disputeGameFinalityDelaySeconds: 500, + mipsVersion: StandardConstants.MIPS_VERSION, + faultGameV2MaxGameDepth: 73, + faultGameV2SplitDepth: 30, + faultGameV2ClockExtension: 10800, + faultGameV2MaxClockDuration: 302400, + superchainConfigProxy: dso.superchainConfigProxy, + protocolVersionsProxy: dso.protocolVersionsProxy, + superchainProxyAdmin: dso.superchainProxyAdmin, + upgradeController: dso.superchainProxyAdmin.owner(), + proposer: proposer, + challenger: challenger, + devFeatureBitmap: DevFeatures.DEPLOY_V2_DISPUTE_GAMES // Enable v2 flag here + }) + ); - function test_deploy_succeeds() public { - vm.expectEmit(true, true, true, false); // TODO precompute the expected `deployOutput`. - emit Deployed(doi.l2ChainId(), address(this), bytes("")); - opcm.deploy(toOPCMDeployInput(doi)); + return dio.opcm; } } From 1005541fd38ce27872e9e671665192ce7bb3fe32 Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 19 Sep 2025 11:33:08 -0400 Subject: [PATCH 52/52] fix: remove empty contract --- packages/contracts-bedrock/test/L1/OPContractsManager.t.sol | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol index 2a456b0d1d79b..447b70dfb8502 100644 --- a/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L1/OPContractsManager.t.sol @@ -2038,11 +2038,6 @@ contract OPContractsManager_Version_Test is OPContractsManager_TestInit { } } -/// @title OPContractsManager_V2_Test -/// @notice Tests for v2 dispute game implementations in OPContractsManager -contract OPContractsManager_V2_Test is OPContractsManager_Init { -} - /// @title OPContractsManager_Deploy_Test /// @notice Tests the `deploy` function of the `OPContractsManager` contract. /// @dev Unlike other test suites, we intentionally do not inherit from CommonTest or Setup. This @@ -2073,6 +2068,7 @@ contract OPContractsManager_Deploy_Test is OPContractsManager_DeployBase { opcm.deploy(toOPCMDeployInput(doi)); } /// @notice Test that deploy without v2 flag doesn't set v2 implementations + function test_deployWithoutV2Flag_succeeds() public { // Convert DOI to OPCM input and deploy IOPContractsManager.DeployInput memory opcmInput = toOPCMDeployInput(doi);