feat: shared dispute game migration for interop

[stevennevins]

Description

Move migrateToSuperRoots(), migrateLiquidity(), and 3 param initialize() from OptimismPortalInterop to OptimismPortal2.

And unify OPCM_V2 to target OptimismPortal2 for the Interop migration codepath

Tests

Tests the superchain system features are properly configured
Tests that migration succeeds with the system feature is enabled

Additional context

Part of #19699

Associated design doc: https://github.com/ethereum-optimism/design-docs/blob/main/protocol/proofs/shared-dispute-game-migration.md

Metadata

Closes #19708
Closes #19710
Closes #19011

[codecov]

Codecov Report

❌ Patch coverage is 98.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.7%. Comparing base (b8a9de8) to head (7ca8a76).
⚠️ Report is 26 commits behind head on develop.

Files with missing lines	Patch %	Lines
...kages/contracts-bedrock/src/L1/OptimismPortal2.sol	96.1%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #19840      +/-   ##
===========================================
+ Coverage         0    80.7%   +80.7%     
===========================================
  Files            0      140     +140     
  Lines            0     7347    +7347     
===========================================
+ Hits             0     5936    +5936     
- Misses           0     1411    +1411     

Flag	Coverage Δ
contracts-bedrock-tests	80.7% <98.0%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/contracts-bedrock/src/L1/ETHLockbox.sol	100.0% <100.0%> (ø)
...es/contracts-bedrock/src/L1/OPContractsManager.sol	92.7% <100.0%> (ø)
...ock/src/L1/OPContractsManagerStandardValidator.sol	97.5% <100.0%> (ø)
...contracts-bedrock/src/L1/OptimismPortalInterop.sol	98.6% <100.0%> (ø)
...edrock/src/L1/opcm/OPContractsManagerContainer.sol	100.0% <ø> (ø)
...bedrock/src/L1/opcm/OPContractsManagerMigrator.sol	95.8% <100.0%> (ø)
...racts-bedrock/src/L1/opcm/OPContractsManagerV2.sol	97.7% <100.0%> (ø)
...es/contracts-bedrock/src/libraries/DevFeatures.sol	100.0% <ø> (ø)
...kages/contracts-bedrock/src/L1/OptimismPortal2.sol	98.1% <96.1%> (ø)

... and 131 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[graphite-app]

Event parameter naming convention violation: The parameter '_newLockbox' in the PortalMigrated event should not be prefixed with an underscore. According to the Optimism Solidity Style Guide naming conventions, event parameters should NOT be prefixed with an underscore, while function parameters should be prefixed with an underscore. Please rename '_newLockbox' to 'newLockbox' to comply with the style guide.

Suggested change

	IETHLockbox indexed _newLockbox,
	IETHLockbox indexed newLockbox,

Spotted by Graphite (based on custom rule: Solidity Style Guide)



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Critical Bug: Interface mismatch

This line calls migrateLiquidity() on IOptimismPortalInterop, but based on the diff, migrateLiquidity() was added to OptimismPortal2 (line 472-481 in OptimismPortal2.sol) and is only exposed in the IOptimismPortal2 interface (line 86-87 in IOptimismPortal2.sol). The IOptimismPortalInterop interface does not include this function.

Fix:

IOptimismPortal(payable(_cts.optimismPortal)).migrateLiquidity();

This matches the pattern used in OPContractsManagerMigrator.sol (line 242) where the cast was correctly changed from IOptimismPortalInterop to IOptimismPortal.

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Non-Idempotent Initializer — Acknowledgment Required

This initialize() function contains operations that are not idempotent (not safe to call multiple times with the same arguments). Since proxied contracts can be re-initialized during upgrades, this is disallowed unless explicitly acknowledged.

Please either:

1. Make the operation idempotent, or
2. Add a @notice comment on the function explaining why the non-idempotent behavior is safe given how callers use it

See docs/ai/contract-dev.md for detailed guidance.

Suggested change

	error OptimismPortal_ProofNotOldEnough();
	error OptimismPortal_InvalidLockboxState();
	/// @notice Semantic version.
	/// @custom:semver 5.4.0
	function version() public pure virtual returns (string memory) {
	return "5.4.0";
	}
	/// @notice Initializes the OptimismPortal2 contract. This function is non-idempotent and will
	/// overwrite existing state variables when called. This is acceptable because the function
	/// is only called during controlled upgrade scenarios where the new values are intended
	/// to replace the existing configuration.
	/// @param _systemConfig The SystemConfig contract.
	/// @param _anchorStateRegistry Address of the AnchorStateRegistry.
	/// @param _ethLockbox Address of the ETHLockbox contract.
	function initialize(
	ISystemConfig _systemConfig,
	IAnchorStateRegistry _anchorStateRegistry,
	IETHLockbox _ethLockbox
	)
	external
	reinitializer(initVersion())
	{
	// Now perform initialization logic.
	systemConfig = _systemConfig;
	anchorStateRegistry = _anchorStateRegistry;
	if (address(_ethLockbox) != address(0)) {
	ethLockbox = _ethLockbox;
	}
	}

Spotted by Graphite (based on custom rule: Monorepo Graphite Rules)



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The migrateLiquidity() function checks _isUsingInterop() but doesn't verify that ethLockbox is set before calling ethLockbox.lockETH(). Since initialize() conditionally sets ethLockbox only if _ethLockbox != address(0) (line 262-264), there's a scenario where INTEROP feature is enabled but ethLockbox remains uninitialized (address(0)). This will cause a revert when migrateLiquidity() is called.

function migrateLiquidity() public {
    if (!_isUsingInterop()) revert OptimismPortal_NotUsingInterop();
    // Add lockbox validation
    if (address(ethLockbox) == address(0)) revert OptimismPortal_InvalidLockboxState();
    _assertOnlyProxyAdminOwner();
    
    uint256 ethBalance = address(this).balance;
    ethLockbox.lockETH{ value: ethBalance }();
    emit ETHMigrated(address(ethLockbox), ethBalance);
}

Alternatively, update _isUsingInterop() to also check that ethLockbox != address(0), similar to how _isUsingLockbox() works at line 728.

Suggested change

	function migrateLiquidity() public {
	if (!_isUsingInterop()) revert OptimismPortal_NotUsingInterop();
	// Liquidity migration can only be triggered by the ProxyAdmin owner.
	_assertOnlyProxyAdminOwner();
	// Migrate the liquidity.
	uint256 ethBalance = address(this).balance;
	ethLockbox.lockETH{ value: ethBalance }();
	emit ETHMigrated(address(ethLockbox), ethBalance);
	}
	function migrateLiquidity() public {
	if (!_isUsingInterop()) revert OptimismPortal_NotUsingInterop();
	if (address(ethLockbox) == address(0)) revert OptimismPortal_InvalidLockboxState();
	// Liquidity migration can only be triggered by the ProxyAdmin owner.
	_assertOnlyProxyAdminOwner();
	// Migrate the liquidity.
	uint256 ethBalance = address(this).balance;
	ethLockbox.lockETH{ value: ethBalance }();
	emit ETHMigrated(address(ethLockbox), ethBalance);
	}

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The custom error OptimismPortal_NotUsingInterop violates the Optimism Solidity Style Guide naming convention for errors. Custom errors should take the format ContractName_ErrorDescription. Since this is in the IOptimismPortal2 interface, it should be named OptimismPortal2_NotUsingInterop to match the contract name.

Suggested change

	error OptimismPortal_NotUsingInterop();
	error OptimismPortal2_NotUsingInterop();

Spotted by Graphite (based on custom rule: Solidity Style Guide)



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The state change in the migrateToSharedDisputeGame function violates the Optimism Solidity Style Guide requirement that all state changing functions should emit a corresponding event. The function checks if anchorStateRegistry == _newAnchorStateRegistry and reverts, but this validation logic modifies the control flow based on state without emitting an event for this specific validation failure. While the function does emit a PortalMigrated event on success, there should be consistent event emission for all state-dependent operations.

Spotted by Graphite (based on custom rule: Solidity Style Guide)



Is this helpful? React 👍 or 👎 to let us know.