diff --git a/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol b/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol index 41d19d9f56ef3..fde972fcd4608 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol @@ -30,15 +30,25 @@ interface IOptimismPortal2 is IProxyAdminOwnedBase { error OptimismPortal_InvalidOutputRootProof(); error OptimismPortal_InvalidProofTimestamp(); error OptimismPortal_InvalidRootClaim(); + error OptimismPortal_MigratingToSameRegistry(); error OptimismPortal_NoReentrancy(); + error OptimismPortal_NotUsingInterop(); error OptimismPortal_ProofNotOldEnough(); error OptimismPortal_Unproven(); + error OptimismPortal_InvalidInteropState(); error OptimismPortal_InvalidLockboxState(); error OutOfGas(); error UnexpectedList(); error UnexpectedString(); event Initialized(uint8 version); + event ETHMigrated(address indexed lockbox, uint256 balance); + event PortalMigrated( + IETHLockbox oldLockbox, + IETHLockbox newLockbox, + IAnchorStateRegistry oldAnchorStateRegistry, + IAnchorStateRegistry newAnchorStateRegistry + ); event TransactionDeposited(address indexed from, address indexed to, uint256 indexed version, bytes opaqueData); event WithdrawalFinalized(bytes32 indexed withdrawalHash, bool success); event WithdrawalProven(bytes32 indexed withdrawalHash, address indexed from, address indexed to); @@ -71,9 +81,11 @@ interface IOptimismPortal2 is IProxyAdminOwnedBase { external; function finalizedWithdrawals(bytes32) external view returns (bool); function guardian() external view returns (address); - function initialize(ISystemConfig _systemConfig, IAnchorStateRegistry _anchorStateRegistry) external; + function initialize(ISystemConfig _systemConfig, IAnchorStateRegistry _anchorStateRegistry, IETHLockbox _ethLockbox) external; function initVersion() external view returns (uint8); function l2Sender() external view returns (address); + function migrateLiquidity() external; + function migrateToSharedDisputeGame(IETHLockbox _newLockbox, IAnchorStateRegistry _newAnchorStateRegistry) external; function minimumGasLimit(uint64 _byteCount) external pure returns (uint64); function numProofSubmitters(bytes32 _withdrawalHash) external view returns (uint256); function params() external view returns (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum); // nosemgrep diff --git a/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol b/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol index 4f3917a70d228..dad8ff3e72cf3 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol @@ -11,6 +11,7 @@ import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.so import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol"; import { IETHLockbox } from "interfaces/L1/IETHLockbox.sol"; +/// TODO(#19709) remove this file and migrate fully to the OptimismPortal2 interface IOptimismPortalInterop is IProxyAdminOwnedBase { error ContentLengthMismatch(); error EmptyItem(); @@ -64,7 +65,7 @@ interface IOptimismPortalInterop is IProxyAdminOwnedBase { function disputeGameFinalityDelaySeconds() external view returns (uint256); function donateETH() external payable; function superchainConfig() external view returns (ISuperchainConfig); - function migrateToSuperRoots(IETHLockbox _newLockbox, IAnchorStateRegistry _newAnchorStateRegistry) external; + function migrateToSharedDisputeGame(IETHLockbox _newLockbox, IAnchorStateRegistry _newAnchorStateRegistry) external; function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external; function finalizeWithdrawalTransactionExternalProof( Types.WithdrawalTransaction memory _tx, diff --git a/packages/contracts-bedrock/interfaces/L1/opcm/IOPContractsManagerContainer.sol b/packages/contracts-bedrock/interfaces/L1/opcm/IOPContractsManagerContainer.sol index 03ba33e9fce9f..a249073962f65 100644 --- a/packages/contracts-bedrock/interfaces/L1/opcm/IOPContractsManagerContainer.sol +++ b/packages/contracts-bedrock/interfaces/L1/opcm/IOPContractsManagerContainer.sol @@ -10,6 +10,7 @@ interface IOPContractsManagerContainer { address resolvedDelegateProxy; } + // TODO(#19709): Remove the reference to optimismPortalInteropImpl when we remove OptimismPortalInterop from src struct Implementations { address superchainConfigImpl; address protocolVersionsImpl; diff --git a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol index b2c4a44fedb05..f1630f86eb054 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol @@ -370,6 +370,8 @@ contract DeployImplementations is Script { _output.optimismPortalImpl = impl; } + + /// TODO(#19709) remove this file and migrate fully to the OptimismPortal2 function deployOptimismPortalInteropImpl(Input memory _input, Output memory _output) private { uint256 proofMaturityDelaySeconds = _input.proofMaturityDelaySeconds; IOptimismPortalInterop impl = IOptimismPortalInterop( diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json index 9d19d18dbe5db..602251f628829 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json @@ -294,6 +294,11 @@ "internalType": "contract IAnchorStateRegistry", "name": "_anchorStateRegistry", "type": "address" + }, + { + "internalType": "contract IETHLockbox", + "name": "_ethLockbox", + "type": "address" } ], "name": "initialize", @@ -314,6 +319,31 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "migrateLiquidity", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract IETHLockbox", + "name": "_newLockbox", + "type": "address" + }, + { + "internalType": "contract IAnchorStateRegistry", + "name": "_newAnchorStateRegistry", + "type": "address" + } + ], + "name": "migrateToSharedDisputeGame", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { @@ -627,6 +657,25 @@ "stateMutability": "pure", "type": "function" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "lockbox", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "balance", + "type": "uint256" + } + ], + "name": "ETHMigrated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -640,6 +689,37 @@ "name": "Initialized", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "contract IETHLockbox", + "name": "oldLockbox", + "type": "address" + }, + { + "indexed": false, + "internalType": "contract IETHLockbox", + "name": "newLockbox", + "type": "address" + }, + { + "indexed": false, + "internalType": "contract IAnchorStateRegistry", + "name": "oldAnchorStateRegistry", + "type": "address" + }, + { + "indexed": false, + "internalType": "contract IAnchorStateRegistry", + "name": "newAnchorStateRegistry", + "type": "address" + } + ], + "name": "PortalMigrated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -794,6 +874,11 @@ "name": "OptimismPortal_InvalidDisputeGame", "type": "error" }, + { + "inputs": [], + "name": "OptimismPortal_InvalidInteropState", + "type": "error" + }, { "inputs": [], "name": "OptimismPortal_InvalidLockboxState", @@ -819,6 +904,11 @@ "name": "OptimismPortal_InvalidRootClaim", "type": "error" }, + { + "inputs": [], + "name": "OptimismPortal_MigratingToSameRegistry", + "type": "error" + }, { "inputs": [], "name": "OptimismPortal_NoReentrancy", @@ -829,6 +919,11 @@ "name": "OptimismPortal_NotAllowedOnCGTMode", "type": "error" }, + { + "inputs": [], + "name": "OptimismPortal_NotUsingInterop", + "type": "error" + }, { "inputs": [], "name": "OptimismPortal_ProofNotOldEnough", diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json index 88c5cce91cfa0..3f3066c5a8986 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json @@ -339,7 +339,7 @@ "type": "address" } ], - "name": "migrateToSuperRoots", + "name": "migrateToSharedDisputeGame", "outputs": [], "stateMutability": "nonpayable", "type": "function" diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index e0490008eb99c..2aa1c7200be2a 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -4,8 +4,8 @@ "sourceCodeHash": "0xe772f7db8033e4a738850cb28ac4849d3a454c93732135a8a10d4f7cb498088e" }, "src/L1/ETHLockbox.sol:ETHLockbox": { - "initCodeHash": "0x781079a80d379658eb4553622a9da86f7532ffa424f1e8957a82680ee9435f66", - "sourceCodeHash": "0xf4d9f6adc3d99d65b70df3255976980d36d37f8a4514ecc24d786dd03efdb7be" + "initCodeHash": "0xb2ff8426ab2eb36352f790748963c8e1f7a91caf16bee6a035c2c41bac532836", + "sourceCodeHash": "0x870004d5acc24a704277680f09ccca51a020a512e6c6d48cca583a23a0de43a2" }, "src/L1/FeesDepositor.sol:FeesDepositor": { "initCodeHash": "0xe2ca240d728f711df438b7aeb3589c95ad11a97d742539a692ddafaf1365eb54", @@ -24,16 +24,16 @@ "sourceCodeHash": "0x9d16e900a764cd7f19db3656cf7a9e555b23b9c7e018641ed21566657847a314" }, "src/L1/OPContractsManagerStandardValidator.sol:OPContractsManagerStandardValidator": { - "initCodeHash": "0x233f5f4b424bc2aabe170cf758c9ff80841ceab4c78e37edfe3a7bc660c5577d", - "sourceCodeHash": "0x7c0cb663f82b07da8dec8a7497cf2fa56a335fb5bdc57b612c86462f8527d4d5" + "initCodeHash": "0xad0549a36d8c093d6971b12577639a6d6a4a823fdd06ce4187ad8efd09a7e7bc", + "sourceCodeHash": "0xcee723c26da263433affef4c07b4a7384397f49ee78b04029f79d757013819bc" }, "src/L1/OptimismPortal2.sol:OptimismPortal2": { - "initCodeHash": "0x8c296124bc1b1468cf301a434eebf3f0d9a194cde06876b993a8672577f08187", - "sourceCodeHash": "0xb14d8bceab135616e55fd560a077a4cc66fc3b535f09931d3b9167ee940fa62f" + "initCodeHash": "0xbce33e9c36303e2130a3389b63a781228bf3b0de44834d3ae73cd88ccf3ced0c", + "sourceCodeHash": "0xbcd2d7f8b3c530bd46d777912fdb46063fa7e4ab0b0b56496a922ea24d3e3229" }, "src/L1/OptimismPortalInterop.sol:OptimismPortalInterop": { - "initCodeHash": "0xbafd0b80deb0a834335052e32a4199a96121148d9bda05acb62535ac18bd9909", - "sourceCodeHash": "0x24373f3fd28c5c6ae93cc32e2a213bb47458bc0f36e81b2a7b20a7b6b0a97119" + "initCodeHash": "0xa7262e160851c0de73b96943a28141ea23d3337494f1c507ecfc66d8a8d83024", + "sourceCodeHash": "0xa73d3099cb3a5e2e3a51323c091e77c8a46ef489d65756d8fa6d64879851ee6a" }, "src/L1/ProtocolVersions.sol:ProtocolVersions": { "initCodeHash": "0xcb59ad9a5ec2a0831b7f4daa74bdacba82ffa03035dafb499a732c641e017f4e", @@ -48,8 +48,8 @@ "sourceCodeHash": "0xb09cb2f7cbde8585fad5c5beb6811fa9044b156b4203da8005d3f6a7a68c30b2" }, "src/L1/opcm/OPContractsManagerV2.sol:OPContractsManagerV2": { - "initCodeHash": "0x6c8af9dac0ff4dc0c783fcf8af06bde4d444ebab065c907785a24fd4f65f2414", - "sourceCodeHash": "0x937e16a99db4a376c8855b3df8eb529d19614c0fa3d5d7dbe334006bad1452a3" + "initCodeHash": "0x177c9727d6a6a450e279becd2a14103401ab3b177d820cc2af887aa3fe01c1f6", + "sourceCodeHash": "0x263e1d09fe1c1d7d4ed1f557866441bb256c82983815cb63c29f4d6a7ea73e88" }, "src/L2/BaseFeeVault.sol:BaseFeeVault": { "initCodeHash": "0xf1fb169c6dd4eceb5cec6ed6dfa3affc45970e5a01e00827d06af1f9e8df026d", diff --git a/packages/contracts-bedrock/src/L1/ETHLockbox.sol b/packages/contracts-bedrock/src/L1/ETHLockbox.sol index 5cb777150b9f6..63c470a2ca664 100644 --- a/packages/contracts-bedrock/src/L1/ETHLockbox.sol +++ b/packages/contracts-bedrock/src/L1/ETHLockbox.sol @@ -73,9 +73,9 @@ contract ETHLockbox is ProxyAdminOwnedBase, Initializable, ReinitializableBase, mapping(IETHLockbox => bool) public authorizedLockboxes; /// @notice Semantic version. - /// @custom:semver 1.2.1 + /// @custom:semver 1.3.1 function version() public view virtual returns (string memory) { - return "1.2.1"; + return "1.3.1"; } /// @notice Constructs the ETHLockbox contract. @@ -195,7 +195,7 @@ contract ETHLockbox is ProxyAdminOwnedBase, Initializable, ReinitializableBase, } /// @notice Migrates liquidity from the current ETH lockbox to another. - /// @dev Must be called atomically with `OptimismPortal.migrateToSuperRoots()` in the same + /// @dev Must be called atomically with `OptimismPortal.migrateToSharedDisputeGame()` in the same /// transaction batch, or otherwise the OptimismPortal may not be able to unlock ETH /// from the ETHLockbox on finalized withdrawals. /// @param _lockbox The address of the ETH lockbox to migrate liquidity to. diff --git a/packages/contracts-bedrock/src/L1/OPContractsManagerStandardValidator.sol b/packages/contracts-bedrock/src/L1/OPContractsManagerStandardValidator.sol index 7310b8d0ed3c9..63bb5f9d1a791 100644 --- a/packages/contracts-bedrock/src/L1/OPContractsManagerStandardValidator.sol +++ b/packages/contracts-bedrock/src/L1/OPContractsManagerStandardValidator.sol @@ -40,8 +40,8 @@ import { IBigStepper } from "interfaces/dispute/IBigStepper.sol"; /// before and after an upgrade. contract OPContractsManagerStandardValidator is ISemver { /// @notice The semantic version of the OPContractsManagerStandardValidator contract. - /// @custom:semver 2.5.0 - string public constant version = "2.5.0"; + /// @custom:semver 2.6.0 + string public constant version = "2.6.0"; /// @notice The SuperchainConfig contract. ISuperchainConfig public superchainConfig; @@ -437,23 +437,12 @@ contract OPContractsManagerStandardValidator is ISemver { { IOptimismPortal2 _portal = IOptimismPortal2(payable(_sysCfg.optimismPortal())); - if (DevFeatures.isDevFeatureEnabled(devFeatureBitmap, DevFeatures.OPTIMISM_PORTAL_INTEROP)) { - _errors = internalRequire( - LibString.eq(getVersion(address(_portal)), string.concat(getVersion(optimismPortalInteropImpl))), - "PORTAL-10", - _errors - ); - _errors = internalRequire( - getProxyImplementation(_admin, address(_portal)) == optimismPortalInteropImpl, "PORTAL-20", _errors - ); - } else { - _errors = internalRequire( - LibString.eq(getVersion(address(_portal)), getVersion(optimismPortalImpl)), "PORTAL-10", _errors - ); - _errors = internalRequire( - getProxyImplementation(_admin, address(_portal)) == optimismPortalImpl, "PORTAL-20", _errors - ); - } + _errors = internalRequire( + LibString.eq(getVersion(address(_portal)), getVersion(optimismPortalImpl)), "PORTAL-10", _errors + ); + _errors = internalRequire( + getProxyImplementation(_admin, address(_portal)) == optimismPortalImpl, "PORTAL-20", _errors + ); IDisputeGameFactory _dgf = IDisputeGameFactory(_sysCfg.disputeGameFactory()); _errors = internalRequire(address(_portal.disputeGameFactory()) == address(_dgf), "PORTAL-30", _errors); diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 9117301b9d7dd..21ebb0d89ca7b 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -130,6 +130,18 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase /// @custom:spacer superRootsActive bool private spacer_63_20_1; + /// @notice Emitted when the Portal is migrated to be shared by + /// @param oldLockbox The lockbox before the migration + /// @param newLockbox The shared lockbox + /// @param oldAnchorStateRegistry The anchorStateRegistry used before the migration + /// @param newAnchorStateRegistry The anchorStateRegistry used after the migration + event PortalMigrated( + IETHLockbox oldLockbox, + IETHLockbox newLockbox, + IAnchorStateRegistry oldAnchorStateRegistry, + IAnchorStateRegistry newAnchorStateRegistry + ); + /// @notice Emitted when a transaction is deposited from L1 to L2. The parameters of this event /// are read by the rollup node and used to derive deposit transactions on L2. /// @param from Address that triggered the deposit transaction. @@ -168,6 +180,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase /// @notice Thrown when the portal is paused. error OptimismPortal_CallPaused(); + event ETHMigrated(address indexed lockbox, uint256 balance); + /// @notice Migrates the total ETH balance to the ETHLockbox. + /// @notice Thrown when a CGT withdrawal is not allowed. error OptimismPortal_NotAllowedOnCGTMode(); @@ -183,6 +198,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase /// @notice Thrown when a withdrawal has not been proven against a valid dispute game. error OptimismPortal_InvalidDisputeGame(); + /// @notice Thrown when Interop is set without the lockbox feature flag + error OptimismPortal_InvalidInteropState(); + /// @notice Thrown when a withdrawal has not been proven against a valid merkle proof. error OptimismPortal_InvalidMerkleProof(); @@ -195,9 +213,17 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase /// @notice Thrown when the root claim of a dispute game is invalid. error OptimismPortal_InvalidRootClaim(); + /// @notice Thrown when migrating to the registry that was previously + /// set on the OptimismPortal prior to the migration + error OptimismPortal_MigratingToSameRegistry(); + /// @notice Thrown when a withdrawal is being finalized by a reentrant call. error OptimismPortal_NoReentrancy(); + /// @notice Thrown when calling a function that is only available when INTEROP + /// is enabled + error OptimismPortal_NotUsingInterop(); + /// @notice Thrown when a withdrawal has not been proven for long enough. error OptimismPortal_ProofNotOldEnough(); @@ -208,9 +234,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase error OptimismPortal_InvalidLockboxState(); /// @notice Semantic version. - /// @custom:semver 5.3.0 + /// @custom:semver 5.5.0 function version() public pure virtual returns (string memory) { - return "5.3.0"; + return "5.5.0"; } /// @param _proofMaturityDelaySeconds The proof maturity delay in seconds. @@ -224,7 +250,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase /// @param _anchorStateRegistry Address of the AnchorStateRegistry. function initialize( ISystemConfig _systemConfig, - IAnchorStateRegistry _anchorStateRegistry + IAnchorStateRegistry _anchorStateRegistry, + IETHLockbox _ethLockbox ) external reinitializer(initVersion()) @@ -235,7 +262,11 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase // Now perform initialization logic. systemConfig = _systemConfig; anchorStateRegistry = _anchorStateRegistry; + if (address(_ethLockbox) != address(0)) { + ethLockbox = _ethLockbox; + } + _assertValidInteropState(); // Assert that the lockbox state is valid. _assertValidLockboxState(); @@ -444,6 +475,66 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase finalizeWithdrawalTransactionExternalProof(_tx, msg.sender); } + 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); + } + + /// @notice Allows the owner of the ProxyAdmin to migrate the OptimismPortal to use a new + /// lockbox, point at a new AnchorStateRegistry, and start to use the Super Roots proof + /// method. Primarily used for OptimismPortal instances to join the interop set, but + /// can also be used to swap the proof method from Output Roots to Super Roots if the + /// provided lockbox is the same as the current one. + /// @dev It is possible to change lockboxes without migrating liquidity. This can cause one + /// of the OptimismPortal instances connected to the new lockbox to not be able to + /// unlock sufficient ETH to finalize withdrawals which would trigger reverts. To avoid + /// this issue, guarantee that this function is called atomically alongside the + /// ETHLockbox.migrateLiquidity() function within the same transaction. + /// @param _newLockbox The address of the new ETHLockbox contract. + /// @param _newAnchorStateRegistry The address of the new AnchorStateRegistry contract. + + function migrateToSharedDisputeGame( + IETHLockbox _newLockbox, + IAnchorStateRegistry _newAnchorStateRegistry + ) + external + { + if (!_isUsingInterop()) revert OptimismPortal_NotUsingInterop(); + // Migration can only be triggered when the system is not paused because the migration can + // potentially unpause the system as a result of the modified ETHLockbox address. + _assertNotPaused(); + + // Migration can only be triggered by the ProxyAdmin owner. + _assertOnlyProxyAdminOwner(); + + // Chains can use this method to swap the proof method from Output Roots to Super Roots + // without joining the interop set. In this case, the old and new lockboxes will be the + // same. However, whether or not a chain is joining the interop set, all chains will need a + // new AnchorStateRegistry when migrating to Super Roots. We therefore check that the new + // AnchorStateRegistry is different than the old one to prevent this function from being + // accidentally misused. + if (anchorStateRegistry == _newAnchorStateRegistry) { + revert OptimismPortal_MigratingToSameRegistry(); + } + + // Update the ETHLockbox. + IETHLockbox oldLockbox = ethLockbox; + ethLockbox = _newLockbox; + + // Update the AnchorStateRegistry. + IAnchorStateRegistry oldAnchorStateRegistry = anchorStateRegistry; + anchorStateRegistry = _newAnchorStateRegistry; + + // Emit a PortalMigrated event. + emit PortalMigrated(oldLockbox, _newLockbox, oldAnchorStateRegistry, _newAnchorStateRegistry); + } + /// @notice Finalizes a withdrawal transaction, using an external proof submitter. /// @param _tx Withdrawal transaction to finalize. /// @param _proofSubmitter Address of the proof submitter. @@ -641,8 +732,14 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase return systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX) && address(ethLockbox) != address(0); } + /// @notice Checks if the Interop feature is enabled. + /// @return bool True if the Interop feature is enabled. + function _isUsingInterop() internal view returns (bool) { + return systemConfig.isFeatureEnabled(Features.INTEROP); + } /// @notice Checks if the Custom Gas Token feature is enabled. /// @return bool True if the Custom Gas Token feature is enabled. + function _isUsingCustomGasToken() internal view returns (bool) { // NOTE: Chains are not supposed to enable Custom Gas Token (CGT) mode after initial deployment. // Enabling CGT post-deployment is strongly discouraged and may lead to unexpected behavior. @@ -656,6 +753,13 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ReinitializableBase } } + /// @notice Asserts the ETHLockbox feature flag must be set if INTEROP is set + function _assertValidInteropState() internal view { + if (systemConfig.isFeatureEnabled(Features.INTEROP) && !systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX)) { + revert OptimismPortal_InvalidInteropState(); + } + } + /// @notice Asserts that the ETHLockbox is set/unset correctly depending on the feature flag. function _assertValidLockboxState() internal view { if ( diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index fda0ae386602a..81292373d1142 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -32,6 +32,7 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; /// @notice The OptimismPortal is a low-level contract responsible for passing messages between L1 /// and L2. Messages sent directly to the OptimismPortal have no form of replayability. /// Users are encouraged to use the L1CrossDomainMessenger for a higher-level interface. +/// TODO(#19709) remove this file and migrate fully to the OptimismPortal2 contract OptimismPortalInterop is Initializable, ResourceMetering, ReinitializableBase, ProxyAdminOwnedBase, ISemver { /// @notice Represents a proven withdrawal. /// @custom:field disputeGameProxy Game that the withdrawal was proven against. @@ -217,9 +218,9 @@ contract OptimismPortalInterop is Initializable, ResourceMetering, Reinitializab error OptimismPortal_MigratingToSameRegistry(); /// @notice Semantic version. - /// @custom:semver 5.3.1+interop + /// @custom:semver 5.4.1+interop function version() public pure virtual returns (string memory) { - return "5.3.1+interop"; + return "5.4.1+interop"; } /// @param _proofMaturityDelaySeconds The proof maturity delay in seconds. @@ -378,7 +379,12 @@ contract OptimismPortalInterop is Initializable, ResourceMetering, Reinitializab /// ETHLockbox.migrateLiquidity() function within the same transaction. /// @param _newLockbox The address of the new ETHLockbox contract. /// @param _newAnchorStateRegistry The address of the new AnchorStateRegistry contract. - function migrateToSuperRoots(IETHLockbox _newLockbox, IAnchorStateRegistry _newAnchorStateRegistry) external { + function migrateToSharedDisputeGame( + IETHLockbox _newLockbox, + IAnchorStateRegistry _newAnchorStateRegistry + ) + external + { // Migration can only be triggered when the system is not paused because the migration can // potentially unpause the system as a result of the modified ETHLockbox address. _assertNotPaused(); diff --git a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerContainer.sol b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerContainer.sol index 5315096a0eeed..dbbe550be7c31 100644 --- a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerContainer.sol +++ b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerContainer.sol @@ -21,6 +21,7 @@ contract OPContractsManagerContainer { } /// @notice Addresses of the implementation contracts. + // TODO(#19709): Remove the reference to optimismPortalInteropImpl when we remove OptimismPortalInterop from src struct Implementations { address superchainConfigImpl; address protocolVersionsImpl; diff --git a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol index d0e9e70312a2b..0f0ef8e3c796c 100644 --- a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol +++ b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol @@ -17,7 +17,6 @@ import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { IOptimismPortal2 as IOptimismPortal } from "interfaces/L1/IOptimismPortal2.sol"; -import { IOptimismPortalInterop } from "interfaces/L1/IOptimismPortalInterop.sol"; import { IETHLockbox } from "interfaces/L1/IETHLockbox.sol"; import { IOPContractsManagerContainer } from "interfaces/L1/opcm/IOPContractsManagerContainer.sol"; import { IOPContractsManagerUtils } from "interfaces/L1/opcm/IOPContractsManagerUtils.sol"; @@ -240,7 +239,7 @@ contract OPContractsManagerMigrator is OPContractsManagerUtilsCaller { internal { // Convert portal to interop portal interface, and grab existing ETHLockbox and DGF. - IOptimismPortalInterop portal = IOptimismPortalInterop(payable(_systemConfig.optimismPortal())); + IOptimismPortal portal = IOptimismPortal(payable(_systemConfig.optimismPortal())); IETHLockbox existingLockbox = IETHLockbox(payable(address(portal.ethLockbox()))); IDisputeGameFactory existingDGF = IDisputeGameFactory(payable(address(portal.disputeGameFactory()))); @@ -272,11 +271,11 @@ contract OPContractsManagerMigrator is OPContractsManagerUtilsCaller { } // Migrate the portal to the new ETHLockbox and AnchorStateRegistry. - // This also sets superRootsActive = true. // NOTE: This requires the portal to already be upgraded to the interop version - // (OptimismPortalInterop). If the portal is not on the interop version, this call will + // (OptimismPortal2). And it requires the feature flag for INTEROP to be enabled + // If the portal is not on the interop version, this call will // fail. - portal.migrateToSuperRoots(_newLockbox, _newASR); + portal.migrateToSharedDisputeGame(_newLockbox, _newASR); } /// @notice Returns the contracts container. diff --git a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol index 440f104211eb8..a1d3d151521b4 100644 --- a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol +++ b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol @@ -24,7 +24,6 @@ import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IOptimismPortal2 as IOptimismPortal } from "interfaces/L1/IOptimismPortal2.sol"; -import { IOptimismPortalInterop } from "interfaces/L1/IOptimismPortalInterop.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { IL1CrossDomainMessenger } from "interfaces/L1/IL1CrossDomainMessenger.sol"; import { IL1ERC721Bridge } from "interfaces/L1/IL1ERC721Bridge.sol"; @@ -153,9 +152,9 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller { /// - Major bump: New required sequential upgrade /// - Minor bump: Replacement OPCM for same upgrade /// - Patch bump: Development changes (expected for normal dev work) - /// @custom:semver 7.0.14 + /// @custom:semver 7.1.14 function version() public pure returns (string memory) { - return "7.0.14"; + return "7.1.14"; } /// @param _standardValidator The standard validator for this OPCM release. @@ -783,13 +782,21 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller { ); // Update the OptimismPortal. + // When interop is enabled, the ETH_LOCKBOX feature must be set on SystemConfig before + // upgrading the portal. Unlike OptimismPortalInterop which blindly sets the ethLockbox, + // OptimismPortal2.initialize() calls _assertValidLockboxState() which requires the + // ETH_LOCKBOX feature flag and ethLockbox address to be consistent. Otherwise we end up in a state where we + // have a lockbox and the feature flag is off if (isDevFeatureEnabled(DevFeatures.OPTIMISM_PORTAL_INTEROP)) { + if (!_cts.systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX)) { + _cts.systemConfig.setFeature(Features.ETH_LOCKBOX, true); + } _upgrade( _cts.proxyAdmin, address(_cts.optimismPortal), - impls.optimismPortalInteropImpl, + impls.optimismPortalImpl, abi.encodeCall( - IOptimismPortalInterop.initialize, (_cts.systemConfig, _cts.anchorStateRegistry, _cts.ethLockbox) + IOptimismPortal.initialize, (_cts.systemConfig, _cts.anchorStateRegistry, _cts.ethLockbox) ) ); } else { @@ -797,7 +804,9 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller { _cts.proxyAdmin, address(_cts.optimismPortal), impls.optimismPortalImpl, - abi.encodeCall(IOptimismPortal.initialize, (_cts.systemConfig, _cts.anchorStateRegistry)) + abi.encodeCall( + IOptimismPortal.initialize, (_cts.systemConfig, _cts.anchorStateRegistry, IETHLockbox(address(0))) + ) ); } @@ -828,9 +837,12 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller { if (!_cts.systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX)) { _cts.systemConfig.setFeature(Features.ETH_LOCKBOX, true); } + if (!_cts.systemConfig.isFeatureEnabled(Features.INTEROP)) { + _cts.systemConfig.setFeature(Features.INTEROP, true); + } // Migrate any ETH into the ETHLockbox. - IOptimismPortalInterop(payable(_cts.optimismPortal)).migrateLiquidity(); + IOptimismPortal(payable(_cts.optimismPortal)).migrateLiquidity(); } // Update the L1CrossDomainMessenger. diff --git a/packages/contracts-bedrock/src/libraries/DevFeatures.sol b/packages/contracts-bedrock/src/libraries/DevFeatures.sol index b91b29f7bda4a..d677dbdac7382 100644 --- a/packages/contracts-bedrock/src/libraries/DevFeatures.sol +++ b/packages/contracts-bedrock/src/libraries/DevFeatures.sol @@ -10,7 +10,7 @@ pragma solidity ^0.8.0; /// etc. /// We'll expand to using all available bits if we need more than 64 concurrent features. library DevFeatures { - /// @notice The feature that enables the OptimismPortalInterop contract. + /// @notice The feature that enables the Interop migration functions on the OptimismPortal2 contract. bytes32 public constant OPTIMISM_PORTAL_INTEROP = bytes32(0x0000000000000000000000000000000000000000000000000000000000000001); diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 2d30b3459208e..f30f5bd056369 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -28,7 +28,6 @@ import { UnknownChainId } from "src/dispute/lib/Errors.sol"; // Interfaces import { IResourceMetering } from "interfaces/L1/IResourceMetering.sol"; import { IOptimismPortal2 as IOptimismPortal } from "interfaces/L1/IOptimismPortal2.sol"; -import { IOptimismPortalInterop } from "interfaces/L1/IOptimismPortalInterop.sol"; import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; import { IProxy } from "interfaces/universal/IProxy.sol"; @@ -162,24 +161,6 @@ abstract contract OptimismPortal2_TestInit is DisputeGameFactory_TestInit { assertFalse(optimismPortal2.finalizedWithdrawals(Hashing.hashWithdrawal(_defaultTx))); } - /// @notice Sets the supeRootsActive variable to the provided value. - /// @param _superRootsActive The value to set the superRootsActive variable to. - function setSuperRootsActive(bool _superRootsActive) public { - // Get the slot for superRootsActive. - StorageSlot memory slot = ForgeArtifacts.getSlot("OptimismPortalInterop", "superRootsActive"); - - // Load the existing storage slot value. - bytes32 existingValue = vm.load(address(optimismPortal2), bytes32(slot.slot)); - - // Inject the bool into the existing storage slot value with a bitwise OR. - // Shift the bool left by the offset of the storage slot and OR with existing value. - bytes32 newValue = - bytes32(uint256(uint8(_superRootsActive ? 1 : 0)) << slot.offset * 8 | uint256(existingValue)); - - // Store the new value at the correct slot/offset. - vm.store(address(optimismPortal2), bytes32(slot.slot), newValue); - } - /// @notice Checks if the ETHLockbox feature is enabled. /// @return bool True if the ETHLockbox feature is enabled. function isUsingLockbox() public view returns (bool) { @@ -316,7 +297,7 @@ contract OptimismPortal2_Initialize_Test is OptimismPortal2_TestInit { // Call the `initialize` function with the sender vm.prank(_sender); - IOptimismPortalInterop(payable(optimismPortal2)).initialize(systemConfig, anchorStateRegistry, ethLockbox); + optimismPortal2.initialize(systemConfig, anchorStateRegistry, ethLockbox); } /// @notice Tests that the initialize function reverts when lockbox state is invalid. @@ -344,7 +325,7 @@ contract OptimismPortal2_Initialize_Test is OptimismPortal2_TestInit { // Call the `initialize` function vm.prank(address(proxyAdmin)); - optimismPortal2.initialize(systemConfig, anchorStateRegistry); + optimismPortal2.initialize(systemConfig, anchorStateRegistry, IETHLockbox(address(0))); } /// @notice Tests that the initialize function reverts if called by a non-proxy admin or owner. @@ -366,131 +347,7 @@ contract OptimismPortal2_Initialize_Test is OptimismPortal2_TestInit { // Call the `initialize` function with the sender vm.prank(_sender); - optimismPortal2.initialize(systemConfig, anchorStateRegistry); - } -} - -/// @title OptimismPortal2_UpgradeInterop_Test -/// @notice Reusable test for the current upgrade() function in the OptimismPortal2 contract. If -/// the upgrade() function is changed, tests inside of this contract should be updated to -/// reflect the new function. If the upgrade() function is removed, remove the -/// corresponding tests but leave this contract in place so it's easy to add tests back -/// in the future. -contract OptimismPortal2_UpgradeInterop_Test is CommonTest { - function setUp() public virtual override { - super.setUp(); - skipIfDevFeatureDisabled(DevFeatures.OPTIMISM_PORTAL_INTEROP); - } - - /// @notice Tests that the upgrade() function succeeds. - function testFuzz_upgrade_interop_succeeds(address _newAnchorStateRegistry, uint256 _balance) external { - // Prevent overflow on an upgrade context - _balance = bound(_balance, 0, type(uint256).max - address(ethLockbox).balance); - - // Get the slot for _initialized. - StorageSlot memory slot = ForgeArtifacts.getSlot("OptimismPortal2", "_initialized"); - - // Set the initialized slot to 0. - vm.store(address(optimismPortal2), bytes32(slot.slot), bytes32(0)); - - // Set the balance of the portal and get the lockbox balance before the upgrade. - deal(address(optimismPortal2), _balance); - uint256 lockboxBalanceBefore = address(ethLockbox).balance; - - // Expect the ETH to be migrated to the lockbox. - vm.expectCall(address(ethLockbox), _balance, abi.encodeCall(ethLockbox.lockETH, ())); - - // Call the upgrade function. - vm.prank(address(optimismPortal2.proxyAdmin())); - IOptimismPortalInterop(payable(optimismPortal2)).upgrade( - IAnchorStateRegistry(_newAnchorStateRegistry), IETHLockbox(ethLockbox) - ); - - // Verify that the initialized slot was updated. - bytes32 initializedSlotAfter = vm.load(address(optimismPortal2), bytes32(slot.slot)); - assertEq(initializedSlotAfter, bytes32(uint256(optimismPortal2.initVersion()))); - - // Assert the portal is properly upgraded. - assertEq(address(optimismPortal2.ethLockbox()), address(ethLockbox)); - assertEq(address(optimismPortal2.anchorStateRegistry()), _newAnchorStateRegistry); - - // Balance has not updated. - assertEq(address(optimismPortal2).balance, _balance); - assertEq(address(ethLockbox).balance, lockboxBalanceBefore); - - // Now we migrate liquidity. - vm.prank(proxyAdminOwner); - IOptimismPortalInterop(payable(optimismPortal2)).migrateLiquidity(); - - // Balance has been updated. - assertEq(address(optimismPortal2).balance, 0); - assertEq(address(ethLockbox).balance, lockboxBalanceBefore + _balance); - } - - /// @notice Tests that the upgrade() function reverts if called a second time. - function test_upgrade_upgradeTwice_reverts() external { - // Get the slot for _initialized. - StorageSlot memory slot = ForgeArtifacts.getSlot("OptimismPortal2", "_initialized"); - - // Set the initialized slot to 0. - vm.store(address(optimismPortal2), bytes32(slot.slot), bytes32(0)); - - // Trigger first upgrade. - vm.prank(address(optimismPortal2.proxyAdmin())); - IOptimismPortalInterop(payable(optimismPortal2)).upgrade( - IAnchorStateRegistry(address(0xdeadbeef)), IETHLockbox(ethLockbox) - ); - - // Try to trigger second upgrade. - vm.prank(address(optimismPortal2.proxyAdmin())); - vm.expectRevert("Initializable: contract is already initialized"); - IOptimismPortalInterop(payable(optimismPortal2)).upgrade( - IAnchorStateRegistry(address(0xdeadbeef)), IETHLockbox(ethLockbox) - ); - } - - /// @notice Tests that the upgrade() function reverts if called after initialization. - function test_upgrade_afterInitialization_reverts() external { - // Get the slot for _initialized. - StorageSlot memory slot = ForgeArtifacts.getSlot("OptimismPortal2", "_initialized"); - - // Slot value should be set to already initialized. - bytes32 initializedSlotBefore = vm.load(address(optimismPortal2), bytes32(slot.slot)); - assertEq(initializedSlotBefore, bytes32(uint256(optimismPortal2.initVersion()))); - - // AnchorStateRegistry address should be non-zero. - assertNotEq(address(optimismPortal2.anchorStateRegistry()), address(0)); - - // SystemConfig address should be non-zero. - assertNotEq(address(optimismPortal2.systemConfig()), address(0)); - - // Try to trigger upgrade(). - vm.expectRevert("Initializable: contract is already initialized"); - IOptimismPortalInterop(payable(optimismPortal2)).upgrade( - IAnchorStateRegistry(address(0xdeadbeef)), IETHLockbox(ethLockbox) - ); - } - - /// @notice Tests that the upgrade() function reverts if called by a non-proxy admin or owner. - /// @param _sender The address of the sender to test. - function testFuzz_upgrade_notProxyAdminOrProxyAdminOwner_reverts(address _sender) public { - // Prank as the not ProxyAdmin or ProxyAdmin owner. - vm.assume(_sender != address(proxyAdmin) && _sender != proxyAdminOwner); - - // Get the slot for _initialized. - StorageSlot memory slot = ForgeArtifacts.getSlot("OptimismPortal2", "_initialized"); - - // Set the initialized slot to 0. - vm.store(address(optimismPortal2), bytes32(slot.slot), bytes32(0)); - - // Expect the revert with `ProxyAdminOwnedBase_NotProxyAdminOrProxyAdminOwner` selector. - vm.expectRevert(IProxyAdminOwnedBase.ProxyAdminOwnedBase_NotProxyAdminOrProxyAdminOwner.selector); - - // Call the `upgrade` function with the sender - vm.prank(_sender); - IOptimismPortalInterop(payable(optimismPortal2)).upgrade( - IAnchorStateRegistry(address(0xdeadbeef)), IETHLockbox(ethLockbox) - ); + optimismPortal2.initialize(systemConfig, anchorStateRegistry, IETHLockbox(address(0))); } } @@ -827,7 +684,7 @@ contract OptimismPortal2_MigrateLiquidity_Test is CommonTest { vm.assume(_caller != optimismPortal2.proxyAdminOwner()); vm.expectRevert(IProxyAdminOwnedBase.ProxyAdminOwnedBase_NotProxyAdminOwner.selector); vm.prank(_caller); - IOptimismPortalInterop(payable(optimismPortal2)).migrateLiquidity(); + optimismPortal2.migrateLiquidity(); } /// @notice Tests that the liquidity migration from the portal to the lockbox succeeds. @@ -844,37 +701,35 @@ contract OptimismPortal2_MigrateLiquidity_Test is CommonTest { emit ETHMigrated(address(ethLockbox), _portalBalance); vm.prank(proxyAdminOwner); - IOptimismPortalInterop(payable(optimismPortal2)).migrateLiquidity(); + optimismPortal2.migrateLiquidity(); assertEq(address(optimismPortal2).balance, 0); assertEq(address(ethLockbox).balance, lockboxBalanceBefore + _portalBalance); } } -/// @title OptimismPortal2_MigrateToSuperRoots_Test -/// @notice Test contract for OptimismPortal2 `migrateToSuperRoots` function. -contract OptimismPortal2_MigrateToSuperRoots_Test is OptimismPortal2_TestInit { +/// @title OptimismPortal2_migrateToSharedDisputeGame_Test +/// @notice Test contract for OptimismPortal2 `migrateToSharedDisputeGame` function. +contract OptimismPortal2_migrateToSharedDisputeGame_Test is OptimismPortal2_TestInit { function setUp() public override { super.setUp(); skipIfDevFeatureDisabled(DevFeatures.OPTIMISM_PORTAL_INTEROP); } - /// @notice Tests that `migrateToSuperRoots` reverts if the caller is not the proxy admin + /// @notice Tests that `migrateToSharedDisputeGame` reverts if the caller is not the proxy admin /// owner. - function testFuzz_migrateToSuperRoots_notProxyAdminOwner_reverts(address _caller) external { + function testFuzz_migrateToSharedDisputeGame_notProxyAdminOwner_reverts(address _caller) external { vm.assume(_caller != optimismPortal2.proxyAdminOwner()); vm.expectRevert(IProxyAdminOwnedBase.ProxyAdminOwnedBase_NotProxyAdminOwner.selector); vm.prank(_caller); - IOptimismPortalInterop(payable(optimismPortal2)).migrateToSuperRoots( - IETHLockbox(address(1)), IAnchorStateRegistry(address(1)) - ); + optimismPortal2.migrateToSharedDisputeGame(IETHLockbox(address(1)), IAnchorStateRegistry(address(1))); } - /// @notice Tests that `migrateToSuperRoots` reverts if the new registry is the same as the + /// @notice Tests that `migrateToSharedDisputeGame` reverts if the new registry is the same as the /// current one. /// @param _newLockbox The new ETHLockbox to migrate to. - function testFuzz_migrateToSuperRoots_usingSameRegistry_reverts(address _newLockbox) external { + function testFuzz_migrateToSharedDisputeGame_usingSameRegistry_reverts(address _newLockbox) external { vm.assume(_newLockbox != address(optimismPortal2.ethLockbox())); // Use the same registry as the current one. @@ -884,18 +739,21 @@ contract OptimismPortal2_MigrateToSuperRoots_Test is OptimismPortal2_TestInit { address caller = optimismPortal2.proxyAdminOwner(); // Expect the migration to revert. - vm.expectRevert(IOptimismPortalInterop.OptimismPortal_MigratingToSameRegistry.selector); + vm.expectRevert(IOptimismPortal.OptimismPortal_MigratingToSameRegistry.selector); vm.prank(caller); - IOptimismPortalInterop(payable(optimismPortal2)).migrateToSuperRoots( - IETHLockbox(_newLockbox), newAnchorStateRegistry - ); + optimismPortal2.migrateToSharedDisputeGame(IETHLockbox(_newLockbox), newAnchorStateRegistry); } - /// @notice Tests that `migrateToSuperRoots` updates the ETHLockbox contract, updates the + /// @notice Tests that `migrateToSharedDisputeGame` updates the ETHLockbox contract, updates the /// AnchorStateRegistry, and sets the superRootsActive flag to true. /// @param _newLockbox The new ETHLockbox to migrate to. /// @param _newAnchorStateRegistry The new AnchorStateRegistry to migrate to. - function testFuzz_migrateToSuperRoots_succeeds(address _newLockbox, address _newAnchorStateRegistry) external { + function testFuzz_migrateToSharedDisputeGame_succeeds( + address _newLockbox, + address _newAnchorStateRegistry + ) + external + { address oldLockbox = address(optimismPortal2.ethLockbox()); address oldAnchorStateRegistry = address(optimismPortal2.anchorStateRegistry()); vm.assume(_newLockbox != oldLockbox); @@ -905,17 +763,17 @@ contract OptimismPortal2_MigrateToSuperRoots_Test is OptimismPortal2_TestInit { emit PortalMigrated(oldLockbox, _newLockbox, oldAnchorStateRegistry, _newAnchorStateRegistry); vm.prank(optimismPortal2.proxyAdminOwner()); - IOptimismPortalInterop(payable(optimismPortal2)).migrateToSuperRoots( + optimismPortal2.migrateToSharedDisputeGame( IETHLockbox(_newLockbox), IAnchorStateRegistry(_newAnchorStateRegistry) ); assertEq(address(optimismPortal2.ethLockbox()), _newLockbox); assertEq(address(optimismPortal2.anchorStateRegistry()), _newAnchorStateRegistry); - assertTrue(IOptimismPortalInterop(payable(optimismPortal2)).superRootsActive()); + assertTrue(systemConfig.isFeatureEnabled(Features.INTEROP)); } - /// @notice Tests that `migrateToSuperRoots` reverts when the system is paused. - function test_migrateToSuperRoots_paused_reverts() external { + /// @notice Tests that `migrateToSharedDisputeGame` reverts when the system is paused. + function test_migrateToSharedDisputeGame_paused_reverts() external { vm.startPrank(optimismPortal2.guardian()); systemConfig.superchainConfig().pause(address(0)); vm.stopPrank(); @@ -923,15 +781,19 @@ contract OptimismPortal2_MigrateToSuperRoots_Test is OptimismPortal2_TestInit { address caller = optimismPortal2.proxyAdminOwner(); vm.expectRevert(IOptimismPortal.OptimismPortal_CallPaused.selector); vm.prank(caller); - IOptimismPortalInterop(payable(optimismPortal2)).migrateToSuperRoots( - IETHLockbox(address(1)), IAnchorStateRegistry(address(1)) - ); + optimismPortal2.migrateToSharedDisputeGame(IETHLockbox(address(1)), IAnchorStateRegistry(address(1))); } } /// @title OptimismPortal2_ProveWithdrawalTransaction_Test /// @notice Test contract for OptimismPortal2 `proveWithdrawalTransaction` function. contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_TestInit { + /// @notice Enables super root behavior on the portal for testing. + function _enableSuperRoots() internal { + // OptimismPortal2 uses stateless GameTypes.isSuperGame() check. + vm.mockCall(address(game), abi.encodeCall(game.gameType, ()), abi.encode(GameTypes.SUPER_CANNON_KONA)); + } + /// @notice Tests that `proveWithdrawalTransaction` reverts when paused. function test_proveWithdrawalTransaction_paused_reverts() external { vm.startPrank(optimismPortal2.guardian()); @@ -1195,13 +1057,12 @@ contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_Test }); } - /// @notice Tests that `proveWithdrawalTransaction` reverts when superRootsActive is true + /// @notice Tests that `proveWithdrawalTransaction` reverts when super roots are active /// and the game's rootClaimByChainId reverts with UnknownChainId. function test_proveWithdrawalTransaction_superRootsVersionBadChainId_reverts() external { skipIfDevFeatureDisabled(DevFeatures.OPTIMISM_PORTAL_INTEROP); - // Enable super roots. - setSuperRootsActive(true); + _enableSuperRoots(); // Mock rootClaimByChainId to revert with UnknownChainId. vm.mockCallRevert( @@ -1212,7 +1073,7 @@ contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_Test // Should revert because chainId not found in super root. vm.expectRevert(UnknownChainId.selector); - IOptimismPortalInterop(payable(optimismPortal2)).proveWithdrawalTransaction({ + optimismPortal2.proveWithdrawalTransaction({ _tx: _defaultTx, _disputeGameIndex: _proposedGameIndex, _outputRootProof: _outputRootProof, @@ -1220,13 +1081,12 @@ contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_Test }); } - /// @notice Tests that `proveWithdrawalTransaction` reverts when superRootsActive is true + /// @notice Tests that `proveWithdrawalTransaction` reverts when super roots are active /// and the output root proof doesn't match the game's rootClaimByChainId. function test_proveWithdrawalTransaction_superRootsVersionBadOutputRootProof_reverts() external { skipIfDevFeatureDisabled(DevFeatures.OPTIMISM_PORTAL_INTEROP); - // Enable super roots. - setSuperRootsActive(true); + _enableSuperRoots(); // Mock rootClaimByChainId to return a different output root (wrong one). bytes32 wrongOutputRoot = keccak256(abi.encode(_outputRoot)); @@ -1237,8 +1097,8 @@ contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_Test ); // Should revert because the output root proof doesn't match. - vm.expectRevert(IOptimismPortalInterop.OptimismPortal_InvalidOutputRootProof.selector); - IOptimismPortalInterop(payable(optimismPortal2)).proveWithdrawalTransaction({ + vm.expectRevert(IOptimismPortal.OptimismPortal_InvalidOutputRootProof.selector); + optimismPortal2.proveWithdrawalTransaction({ _tx: _defaultTx, _disputeGameIndex: _proposedGameIndex, _outputRootProof: _outputRootProof, @@ -1246,13 +1106,12 @@ contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_Test }); } - /// @notice Tests that `proveWithdrawalTransaction` succeeds when superRootsActive is true + /// @notice Tests that `proveWithdrawalTransaction` succeeds when super roots are active /// and all parameters are valid. function test_proveWithdrawalTransaction_superRootsVersion_succeeds() external { skipIfDevFeatureDisabled(DevFeatures.OPTIMISM_PORTAL_INTEROP); - // Enable super roots. - setSuperRootsActive(true); + _enableSuperRoots(); // Mock rootClaimByChainId to return the correct output root. vm.mockCall( @@ -1262,7 +1121,7 @@ contract OptimismPortal2_ProveWithdrawalTransaction_Test is OptimismPortal2_Test ); // Should succeed. - IOptimismPortalInterop(payable(optimismPortal2)).proveWithdrawalTransaction({ + optimismPortal2.proveWithdrawalTransaction({ _tx: _defaultTx, _disputeGameIndex: _proposedGameIndex, _outputRootProof: _outputRootProof, diff --git a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol index e4635d1b6c9e9..b9964dacb86ff 100644 --- a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol +++ b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol @@ -10,6 +10,7 @@ import { ForgeArtifacts, StorageSlot } from "scripts/libraries/ForgeArtifacts.so // Libraries import { Constants } from "src/libraries/Constants.sol"; import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; +import { DevFeatures } from "src/libraries/DevFeatures.sol"; import { Features } from "src/libraries/Features.sol"; // Interfaces @@ -100,6 +101,16 @@ contract SystemConfig_Initialize_Test is SystemConfig_TestInit { skipIfForkTest("SystemConfig_Initialize_Test: cannot test initialization on forked network"); } + function test_initialize_interopFlag_succeeds() external view { + if (isDevFeatureEnabled(DevFeatures.OPTIMISM_PORTAL_INTEROP)) { + /// if devfeature flag is on, check in system config is on + vm.assertTrue(systemConfig.isFeatureEnabled(Features.INTEROP)); + } else { + /// if dev feature flag is off, check system config is off + vm.assertFalse(systemConfig.isFeatureEnabled(Features.INTEROP)); + } + } + /// @notice Tests that initialization sets the correct values. function test_initialize_succeeds() external view { assertEq(systemConfig.owner(), owner); @@ -883,6 +894,12 @@ contract SystemConfig_IsFeatureEnabled_Test is SystemConfig_TestInit { systemConfig.setFeature(Features.CUSTOM_GAS_TOKEN, false); } + // Normalize INTEROP to avoid environment-dependent state + if (systemConfig.isFeatureEnabled(Features.INTEROP)) { + vm.prank(address(systemConfig.proxyAdmin())); + systemConfig.setFeature(Features.INTEROP, false); + } + assertFalse(systemConfig.isFeatureEnabled(_feature)); } diff --git a/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerContainer.t.sol b/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerContainer.t.sol index 6ce881b617171..6f3e8a91d953c 100644 --- a/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerContainer.t.sol +++ b/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerContainer.t.sol @@ -25,6 +25,7 @@ contract OPContractsManagerContainer_TestInit is Test { resolvedDelegateProxy: makeAddr("resolvedDelegateProxy") }); + // TODO(#19709): Remove the reference to OptimismPortalImpl when we remove OptimismPortalInterop from src implementations = OPContractsManagerContainer.Implementations({ superchainConfigImpl: makeAddr("superchainConfigImpl"), protocolVersionsImpl: makeAddr("protocolVersionsImpl"), diff --git a/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerUtils.t.sol b/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerUtils.t.sol index c725605e9782f..9840c5017350b 100644 --- a/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerUtils.t.sol +++ b/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerUtils.t.sol @@ -100,6 +100,7 @@ contract OPContractsManagerUtils_TestInit is Test { }); // Set up implementations - use real StorageSetter, mocks for the rest. + // TODO(#19709): Remove the reference to OptimismPortalImpl when we remove OptimismPortalInterop from src implementations = OPContractsManagerContainer.Implementations({ superchainConfigImpl: makeAddr("superchainConfigImpl"), protocolVersionsImpl: makeAddr("protocolVersionsImpl"), diff --git a/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerV2.t.sol b/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerV2.t.sol index d3eb2c62a91b9..b5e0a9f92fe34 100644 --- a/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerV2.t.sol +++ b/packages/contracts-bedrock/test/L1/opcm/OPContractsManagerV2.t.sol @@ -29,7 +29,6 @@ import { IOPContractsManagerUtils } from "interfaces/L1/opcm/IOPContractsManager import { IOPContractsManagerContainer } from "interfaces/L1/opcm/IOPContractsManagerContainer.sol"; import { IOPContractsManagerMigrator } from "interfaces/L1/opcm/IOPContractsManagerMigrator.sol"; import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol"; -import { IOptimismPortalInterop } from "interfaces/L1/IOptimismPortalInterop.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol"; import { IETHLockbox } from "interfaces/L1/IETHLockbox.sol"; @@ -2041,16 +2040,15 @@ contract OPContractsManagerV2_Migrate_Test is OPContractsManagerV2_TestInit { assertTrue(newLockbox.authorizedPortals(portal1), "ETHLockbox does not have portal 1 authorized"); assertTrue(newLockbox.authorizedPortals(portal2), "ETHLockbox does not have portal 2 authorized"); - // Check that superRootsActive is true on both portals. + // Check that the ETH_LOCKBOX feature is enabled on both SystemConfigs. assertTrue( - IOptimismPortalInterop(payable(address(portal1))).superRootsActive(), - "Portal 1 superRootsActive should be true" + chainContracts1.systemConfig.isFeatureEnabled(Features.INTEROP), + "Chain 1 ETH_LOCKBOX feature should be enabled" ); assertTrue( - IOptimismPortalInterop(payable(address(portal2))).superRootsActive(), - "Portal 2 superRootsActive should be true" + chainContracts2.systemConfig.isFeatureEnabled(Features.INTEROP), + "Chain 2 ETH_LOCKBOX feature should be enabled" ); - // Check that the ETH_LOCKBOX feature is enabled on both SystemConfigs. assertTrue( chainContracts1.systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX), diff --git a/packages/contracts-bedrock/test/L2/L2DevFeatureFlags.t.sol b/packages/contracts-bedrock/test/L2/L2DevFeatureFlags.t.sol index 0e277961dc8fe..88a3248148cd6 100644 --- a/packages/contracts-bedrock/test/L2/L2DevFeatureFlags.t.sol +++ b/packages/contracts-bedrock/test/L2/L2DevFeatureFlags.t.sol @@ -85,7 +85,7 @@ contract L2DevFeatureFlags_IsDevFeatureEnabled_Test is L2DevFeatureFlags_TestIni } /// @notice Tests that `isDevFeatureEnabled` works correctly with the known OPTIMISM_PORTAL_INTEROP feature. - function test_isDevFeatureEnabled_optimismPortalInterop_succeeds() public { + function test_isDevFeatureEnabled_interop_succeeds() public { vm.prank(Constants.DEPOSITOR_ACCOUNT); l2DevFeatureFlags.setDevFeatureBitmap(DevFeatures.OPTIMISM_PORTAL_INTEROP); diff --git a/packages/contracts-bedrock/test/vendor/Initializable.t.sol b/packages/contracts-bedrock/test/vendor/Initializable.t.sol index e135a75dc9063..a6137501368b7 100644 --- a/packages/contracts-bedrock/test/vendor/Initializable.t.sol +++ b/packages/contracts-bedrock/test/vendor/Initializable.t.sol @@ -15,6 +15,7 @@ import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; import { DevFeatures } from "src/libraries/DevFeatures.sol"; // Interfaces +import { IETHLockbox } from "interfaces/L1/IETHLockbox.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IResourceMetering } from "interfaces/L1/IResourceMetering.sol"; @@ -22,7 +23,6 @@ import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.so import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { ProtocolVersion } from "interfaces/L1/IProtocolVersions.sol"; import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol"; -import { IOptimismPortalInterop } from "interfaces/L1/IOptimismPortalInterop.sol"; /// @title Initializer_Test /// @dev Ensures that the `initialize()` function on contracts cannot be called more than @@ -123,14 +123,13 @@ contract Initializer_Test is CommonTest { ); if (isDevFeatureEnabled(DevFeatures.OPTIMISM_PORTAL_INTEROP)) { - // OptimismPortal2Impl + // TODO(#19709): Remove this branching logic when we remove the OptimismPortalInterop from src contracts.push( InitializeableContract({ name: "OptimismPortal2Impl", target: EIP1967Helper.getImplementation(address(optimismPortal2)), initCalldata: abi.encodeCall( - IOptimismPortalInterop(payable(optimismPortal2)).initialize, - (systemConfig, anchorStateRegistry, ethLockbox) + optimismPortal2.initialize, (systemConfig, anchorStateRegistry, ethLockbox) ) }) ); @@ -140,18 +139,18 @@ contract Initializer_Test is CommonTest { name: "OptimismPortal2Proxy", target: address(optimismPortal2), initCalldata: abi.encodeCall( - IOptimismPortalInterop(payable(optimismPortal2)).initialize, - (systemConfig, anchorStateRegistry, ethLockbox) + optimismPortal2.initialize, (systemConfig, anchorStateRegistry, ethLockbox) ) }) ); } else { - // OptimismPortal2Impl contracts.push( InitializeableContract({ name: "OptimismPortal2Impl", target: EIP1967Helper.getImplementation(address(optimismPortal2)), - initCalldata: abi.encodeCall(optimismPortal2.initialize, (systemConfig, anchorStateRegistry)) + initCalldata: abi.encodeCall( + optimismPortal2.initialize, (systemConfig, anchorStateRegistry, IETHLockbox(address(0))) + ) }) ); // OptimismPortal2Proxy @@ -159,7 +158,9 @@ contract Initializer_Test is CommonTest { InitializeableContract({ name: "OptimismPortal2Proxy", target: address(optimismPortal2), - initCalldata: abi.encodeCall(optimismPortal2.initialize, (systemConfig, anchorStateRegistry)) + initCalldata: abi.encodeCall( + optimismPortal2.initialize, (systemConfig, anchorStateRegistry, IETHLockbox(address(0))) + ) }) ); } @@ -399,6 +400,7 @@ contract Initializer_Test is CommonTest { excludes[j++] = "src/dispute/SuperPermissionedDisputeGame.sol"; excludes[j++] = "src/dispute/zk/ZKDisputeGame.sol"; // TODO: Eventually remove this exclusion. Same reason as above dispute contracts. + // TODO(#19709): Remove this exclusion as part of the OptimismPortalInterop removal excludes[j++] = "src/L1/OptimismPortalInterop.sol"; // L2 contract initialization is tested in Predeploys.t.sol excludes[j++] = "src/L2/*";