diff --git a/.semgrep/rules/sol-rules.yaml b/.semgrep/rules/sol-rules.yaml index 3fe1754098d..1b4064a3afd 100644 --- a/.semgrep/rules/sol-rules.yaml +++ b/.semgrep/rules/sol-rules.yaml @@ -330,6 +330,7 @@ rules: - packages/contracts-bedrock/src/L2/FeeVault.sol - packages/contracts-bedrock/src/L2/OptimismMintableERC721.sol - packages/contracts-bedrock/src/L2/OptimismMintableERC721Factory.sol + - packages/contracts-bedrock/src/L2/XForkL2ContractsManager.sol - packages/contracts-bedrock/src/cannon/MIPS64.sol - packages/contracts-bedrock/src/cannon/PreimageOracle.sol - packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol diff --git a/packages/contracts-bedrock/interfaces/L2/IXForkL2ContractsManager.sol b/packages/contracts-bedrock/interfaces/L2/IXForkL2ContractsManager.sol new file mode 100644 index 00000000000..f3b7f5d573f --- /dev/null +++ b/packages/contracts-bedrock/interfaces/L2/IXForkL2ContractsManager.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { XForkL2CMTypes } from "src/libraries/XForkL2CMTypes.sol"; + +/// @title IXForkL2ContractsManager +/// @notice Interface for the XForkL2ContractsManager contract. +interface IXForkL2ContractsManager is ISemver { + /// @notice Executes the upgrade for all predeploys. + /// @dev This function MUST be called via DELEGATECALL from the L2ProxyAdmin. + function upgrade() external; + + /// @notice Constructor for the XForkL2ContractsManager contract. + /// @param _implementations The implementation struct containing the new implementation addresses for the L2 + /// predeploys. + function __constructor__(XForkL2CMTypes.Implementations memory _implementations) external; +} diff --git a/packages/contracts-bedrock/snapshots/abi/XForkL2ContractsManager.json b/packages/contracts-bedrock/snapshots/abi/XForkL2ContractsManager.json new file mode 100644 index 00000000000..26d96436393 --- /dev/null +++ b/packages/contracts-bedrock/snapshots/abi/XForkL2ContractsManager.json @@ -0,0 +1,180 @@ +[ + { + "inputs": [ + { + "components": [ + { + "internalType": "address", + "name": "storageSetterImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l2CrossDomainMessengerImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "gasPriceOracleImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l2StandardBridgeImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "sequencerFeeWalletImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "optimismMintableERC20FactoryImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l2ERC721BridgeImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l1BlockAttributesImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l1BlockAttributesCGTImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l2ToL1MessagePasserImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l2ToL1MessagePasserCGTImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "optimismMintableERC721FactoryImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "proxyAdminImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "baseFeeVaultImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l1FeeVaultImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "operatorFeeVaultImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "schemaRegistryImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "easImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "crossL2InboxImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "l2ToL2CrossDomainMessengerImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "superchainETHBridgeImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "ethLiquidityImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "optimismSuperchainERC20FactoryImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "optimismSuperchainERC20BeaconImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "superchainTokenBridgeImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "nativeAssetLiquidityImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "liquidityControllerImpl", + "type": "address" + }, + { + "internalType": "address", + "name": "feeSplitterImpl", + "type": "address" + } + ], + "internalType": "struct XForkL2CMTypes.Implementations", + "name": "_implementations", + "type": "tuple" + } + ], + "stateMutability": "nonpayable", + "type": "constructor" + }, + { + "inputs": [], + "name": "upgrade", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "version", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "XForkL2ContractsManager_OnlyDelegatecall", + "type": "error" + } +] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 4f44d9eb50c..9b4565ae6c7 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -175,6 +175,10 @@ "initCodeHash": "0xbc2cd025153720943e51b79822c2dc374d270a78b92cf47d49548c468e218e46", "sourceCodeHash": "0x734a6b2aa6406bc145d848ad6071d3af1d40852aeb8f4b2f6f51beaad476e2d3" }, + "src/L2/XForkL2ContractsManager.sol:XForkL2ContractsManager": { + "initCodeHash": "0x60d6330227c94f638d701f3311edae38e34012362f71038e9463d9411e78e976", + "sourceCodeHash": "0xc41e020e32ce453cbe2ef1bf83371180bb52979356230186f90c9039276b0110" + }, "src/cannon/MIPS64.sol:MIPS64": { "initCodeHash": "0x13196c1652a1f51cf0c16191f0092898f127eff036c773923c72b02a2823c7f4", "sourceCodeHash": "0xd745aaf4ed265be7be7bff9bca1dd040e15dfe41e3a453906d72ca09a47f2c8b" diff --git a/packages/contracts-bedrock/snapshots/storageLayout/XForkL2ContractsManager.json b/packages/contracts-bedrock/snapshots/storageLayout/XForkL2ContractsManager.json new file mode 100644 index 00000000000..0637a088a01 --- /dev/null +++ b/packages/contracts-bedrock/snapshots/storageLayout/XForkL2ContractsManager.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L2/XForkL2ContractsManager.sol b/packages/contracts-bedrock/src/L2/XForkL2ContractsManager.sol index 950d8796193..253a23a90eb 100644 --- a/packages/contracts-bedrock/src/L2/XForkL2ContractsManager.sol +++ b/packages/contracts-bedrock/src/L2/XForkL2ContractsManager.sol @@ -20,7 +20,6 @@ import { IL1Block } from "interfaces/L2/IL1Block.sol"; // Libraries import { Predeploys } from "src/libraries/Predeploys.sol"; -import { Types } from "src/libraries/Types.sol"; import { XForkL2CMTypes } from "src/libraries/XForkL2CMTypes.sol"; /// @title XForkL2ContractsManager @@ -105,6 +104,9 @@ contract XForkL2ContractsManager is ISemver { /// @notice FeeSplitter implementation. address internal immutable FEE_SPLITTER_IMPL; + /// @notice Constructor for the XForkL2ContractsManager contract. + /// @param _implementations The implementation struct containing the new implementation addresses for the L2 + /// predeploys. constructor(XForkL2CMTypes.Implementations memory _implementations) { // Store the address of this contract for DELEGATECALL enforcement. THIS_L2CM = address(this); @@ -230,9 +232,7 @@ contract XForkL2ContractsManager is ISemver { _upgradeToAndCall( Predeploys.L2_CROSS_DOMAIN_MESSENGER, L2_CROSS_DOMAIN_MESSENGER_IMPL, - abi.encodeCall( - IL2CrossDomainMessenger.initialize, (ICrossDomainMessenger(_config.crossDomainMessenger.otherMessenger)) - ), + abi.encodeCall(IL2CrossDomainMessenger.initialize, (_config.crossDomainMessenger.otherMessenger)), INITIALIZABLE_SLOT_OZ_V4, 20 // Account for CrossDomainMessengerLegacySpacer0 ); @@ -241,7 +241,7 @@ contract XForkL2ContractsManager is ISemver { _upgradeToAndCall( Predeploys.L2_STANDARD_BRIDGE, L2_STANDARD_BRIDGE_IMPL, - abi.encodeCall(IL2StandardBridge.initialize, (IStandardBridge(payable(_config.standardBridge.otherBridge)))), + abi.encodeCall(IL2StandardBridge.initialize, (_config.standardBridge.otherBridge)), INITIALIZABLE_SLOT_OZ_V4, 0 ); diff --git a/packages/contracts-bedrock/test/L2/fork/XForkL2ContractsManager.t.sol b/packages/contracts-bedrock/test/L2/XForkL2ContractsManager.t.sol similarity index 96% rename from packages/contracts-bedrock/test/L2/fork/XForkL2ContractsManager.t.sol rename to packages/contracts-bedrock/test/L2/XForkL2ContractsManager.t.sol index 4bf2d876ef4..571ca3702be 100644 --- a/packages/contracts-bedrock/test/L2/fork/XForkL2ContractsManager.t.sol +++ b/packages/contracts-bedrock/test/L2/XForkL2ContractsManager.t.sol @@ -1,15 +1,12 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import { Test } from "forge-std/Test.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; import { XForkL2ContractsManager } from "src/L2/XForkL2ContractsManager.sol"; import { XForkL2CMTypes } from "src/libraries/XForkL2CMTypes.sol"; import { CommonTest } from "test/setup/CommonTest.sol"; import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; -import { IProxy } from "interfaces/universal/IProxy.sol"; import { StorageSetter } from "src/universal/StorageSetter.sol"; -import { IL1Block } from "interfaces/L2/IL1Block.sol"; import { L2CrossDomainMessenger } from "src/L2/L2CrossDomainMessenger.sol"; import { ICrossDomainMessenger } from "interfaces/universal/ICrossDomainMessenger.sol"; import { IStandardBridge } from "interfaces/universal/IStandardBridge.sol"; @@ -21,7 +18,6 @@ import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; import { ILiquidityController } from "interfaces/L2/ILiquidityController.sol"; import { StorageSetter } from "src/universal/StorageSetter.sol"; -import { WETH } from "src/L2/WETH.sol"; import { GasPriceOracle } from "src/L2/GasPriceOracle.sol"; import { L2StandardBridge } from "src/L2/L2StandardBridge.sol"; import { OptimismMintableERC20Factory } from "src/universal/OptimismMintableERC20Factory.sol"; @@ -32,7 +28,6 @@ import { L2ToL1MessagePasser } from "src/L2/L2ToL1MessagePasser.sol"; import { L2ToL1MessagePasserCGT } from "src/L2/L2ToL1MessagePasserCGT.sol"; import { OptimismMintableERC721Factory } from "src/L2/OptimismMintableERC721Factory.sol"; import { ProxyAdmin } from "src/universal/ProxyAdmin.sol"; -import { GovernanceToken } from "src/governance/GovernanceToken.sol"; import { SuperchainETHBridge } from "src/L2/SuperchainETHBridge.sol"; import { ETHLiquidity } from "src/L2/ETHLiquidity.sol"; import { OptimismSuperchainERC20Beacon } from "src/L2/OptimismSuperchainERC20Beacon.sol"; @@ -43,7 +38,7 @@ import { Features } from "src/libraries/Features.sol"; /// @title XForkL2ContractsManager_Harness /// @notice Harness contract that exposes internal functions for testing. -contract XForkL2ContractsManager_Harness is XForkL2ContractsManager { +contract XForkL2ContractsManager_FullConfigExposer_Harness is XForkL2ContractsManager { constructor(XForkL2CMTypes.Implementations memory _implementations) XForkL2ContractsManager(_implementations) { } /// @notice Returns the full configuration for the L2 predeploys. @@ -88,8 +83,8 @@ contract XForkL2ContractsManager_Harness is XForkL2ContractsManager { /// @title XForkL2ContractsManager_Test /// @notice Test contract for the XForkL2ContractsManager contract, testing the upgrade path for this fork. -contract XForkL2ContractsManager_Test is CommonTest { - XForkL2ContractsManager_Harness internal l2cm; +contract XForkL2ContractsManager_Upgrade_Test is CommonTest { + XForkL2ContractsManager_FullConfigExposer_Harness internal l2cm; XForkL2CMTypes.Implementations internal implementations; /// @notice Struct to capture the post-upgrade state for comparison. @@ -173,7 +168,7 @@ contract XForkL2ContractsManager_Test is CommonTest { /// @notice Deploys the XForkL2ContractsManager with the loaded implementations. function _deployL2CM() internal { - l2cm = new XForkL2ContractsManager_Harness(implementations); + l2cm = new XForkL2ContractsManager_FullConfigExposer_Harness(implementations); vm.label(address(l2cm), "XForkL2ContractsManager"); } @@ -184,7 +179,7 @@ contract XForkL2ContractsManager_Test is CommonTest { address proxyAdmin = Predeploys.PROXY_ADMIN; prankDelegateCall(proxyAdmin); (bool success,) = address(l2cm).delegatecall(abi.encodeCall(XForkL2ContractsManager.upgrade, ())); - require(success, "Upgrade failed"); + require(success, "L2ContractsManager: Upgrade failed"); } /// @notice Captures the current post-upgrade state of all predeploys. @@ -339,7 +334,7 @@ contract XForkL2ContractsManager_Test is CommonTest { } /// @notice Tests that the upgrade produces identical state when called twice with the same pre-state. - function test_upgrade_producesSameState_whenCalledTwiceWithSamePreState() public { + function test_upgradeProducesSameState_whenCalledTwiceWithSamePreState_succeeds() public { // Save the pre-upgrade state uint256 snapshotId = vm.snapshotState(); @@ -363,7 +358,7 @@ contract XForkL2ContractsManager_Test is CommonTest { } /// @notice Tests that all network-specific configuration is preserved after upgrade. - function test_upgrade_preservesAllConfiguration() public { + function test_upgradePreservesAllConfiguration_succeeds() public { // Get the pre-upgrade configuration XForkL2CMTypes.FullConfig memory preUpgradeConfig = l2cm.loadFullConfig(); @@ -477,14 +472,14 @@ contract XForkL2ContractsManager_Test is CommonTest { } /// @notice Tests that calling upgrade() directly (not via DELEGATECALL) reverts. - function test_upgrade_reverts_whenCalledDirectly() public { + function test_upgrade_whenCalledDirectly_reverts() public { // Calling upgrade() directly should revert with OnlyDelegatecall error vm.expectRevert(XForkL2ContractsManager.XForkL2ContractsManager_OnlyDelegatecall.selector); l2cm.upgrade(); } /// @notice Tests that fee vault configurations with non-default values are preserved after upgrade. - function test_upgrade_preservesFeeVaultConfig_withNonDefaultValues() public { + function test_upgradePreservesFeeVaultConfig_withNonDefaultValues_succeeds() public { // Define non-default test values address customRecipient = makeAddr("customRecipient"); uint256 customMinWithdrawal = 50 ether; @@ -574,9 +569,9 @@ contract XForkL2ContractsManager_Test is CommonTest { /// @title XForkL2ContractsManager_CGT_Test /// @notice Test contract for the XForkL2ContractsManager on Custom Gas Token networks. -contract XForkL2ContractsManager_CGT_Test is XForkL2ContractsManager_Test { +contract XForkL2ContractsManager_Upgrade_CGT_Test is XForkL2ContractsManager_Upgrade_Test { /// @notice Tests that CGT-specific contracts are upgraded when CGT is enabled. - function test_upgrade_upgradesCGTContracts_whenCGTEnabled() public { + function test_upgradeUpgradesCGTContracts_whenCGTEnabled_succeeds() public { skipIfSysFeatureDisabled(Features.CUSTOM_GAS_TOKEN); // Capture pre-upgrade implementations for CGT-specific contracts @@ -630,7 +625,7 @@ contract XForkL2ContractsManager_CGT_Test is XForkL2ContractsManager_Test { } /// @notice Tests that LiquidityController config is preserved after upgrade on CGT networks. - function test_upgrade_preservesLiquidityControllerConfig_onCGTNetwork() public { + function test_upgradePreservesLiquidityControllerConfig_onCGTNetwork_succeeds() public { skipIfSysFeatureDisabled(Features.CUSTOM_GAS_TOKEN); // Capture pre-upgrade config