From 8d80aac5355b80563ad2636155d593d6448f3c87 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Sun, 22 Feb 2026 12:21:32 -0500 Subject: [PATCH] contracts: add documentation for audit findings #2, #3, #7, #12, #15, #16 Add missing @param blueprint NatSpec to OpcmContractRef struct (#2). Add comments about pause blocking interop upgrades (#3). Document migrate() scope limitations and re-migration risks (#7, #15). Update PERMIT_ALL_CONTRACTS_INSTRUCTION comment (#12). Document intentional use of chainSystemConfigs[0] for shared contracts (#16). Co-Authored-By: Claude Opus 4.6 --- .../contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol | 7 ++++--- packages/contracts-bedrock/snapshots/semver-lock.json | 4 ++-- .../src/L1/opcm/OPContractsManagerMigrator.sol | 9 +++++++++ .../src/L1/opcm/OPContractsManagerV2.sol | 8 ++++++-- packages/contracts-bedrock/src/libraries/Constants.sol | 2 +- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol index b727cd26aa4..8677c9cb2e3 100644 --- a/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol @@ -108,9 +108,10 @@ contract VerifyOPCM is Script { uint256 constant MAX_INIT_CODE_SIZE = 23500; /// @notice Represents a contract name and its corresponding address. - /// @param field Name of the field the address was extracted from. - /// @param name Name of the contract. - /// @param addr Address of the contract. + /// @param field Name of the field the address was extracted from. + /// @param name Name of the contract. + /// @param addr Address of the contract. + /// @param blueprint Whether the contract is a blueprint deployment. struct OpcmContractRef { string field; string name; diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 476ad492fac..93125d7a2bb 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -52,8 +52,8 @@ "sourceCodeHash": "0xb3184aa5d95a82109e7134d1f61941b30e25f655b9849a0e303d04bbce0cde0b" }, "src/L1/opcm/OPContractsManagerV2.sol:OPContractsManagerV2": { - "initCodeHash": "0x5cbc998e57035d8658824e16dacaab8c702f9e18f482e16989b9420e5a7e8190", - "sourceCodeHash": "0x11678225efb1fb4593085febd8f438eeb4752c0ab3dfd2ee1c4fe47970dda953" + "initCodeHash": "0x88ada0dfefb77eea33baaf11d9b5a5ad51cb8c6476611d0f2376897413074619", + "sourceCodeHash": "0x1cc9dbcd4c7652f482c43e2630b324d088e825d12532711a41c636e8392636b3" }, "src/L2/BaseFeeVault.sol:BaseFeeVault": { "initCodeHash": "0x838bbd7f381e84e21887f72bd1da605bfc4588b3c39aed96cbce67c09335b3ee", diff --git a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol index a156638c31a..289cdbd7312 100644 --- a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol +++ b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol @@ -63,6 +63,11 @@ contract OPContractsManagerMigrator is OPContractsManagerUtilsCaller { /// temporary need to support the interop migration action. It will likely be removed in /// the near future once interop support is baked more directly into OPCM. It does NOT /// look or function like all of the other functions in OPCMv2. + /// @dev NOTE: This function is designed exclusively for the case of N independent pre-interop + /// chains merging into a single interop set. It does NOT support partial migration (i.e., + /// migrating a subset of chains that share a lockbox), re-migration of already-migrated + /// chains, or any other migration scenario. Re-calling this function on already-migrated + /// portals will corrupt the shared DisputeGameFactory used by all migrated chains. /// @param _input The input parameters for the migration. function migrate(MigrateInput calldata _input) public { // Check that the starting respected game type is a valid super game type. @@ -156,6 +161,10 @@ contract OPContractsManagerMigrator is OPContractsManagerUtilsCaller { IOPContractsManagerContainer.Implementations memory impls = contractsContainer().implementations(); // Initialize the new ETHLockbox. + // NOTE: Shared contracts (ETHLockbox, AnchorStateRegistry, DelayedWETH) are + // intentionally initialized with chainSystemConfigs[0]. All chains are validated to + // share the same ProxyAdmin owner and SuperchainConfig, so the first chain's + // SystemConfig is used as the canonical governance reference for shared contracts. _upgrade( proxyDeployArgs.proxyAdmin, address(ethLockbox), diff --git a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol index c17aa044d23..55c15c74117 100644 --- a/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol +++ b/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol @@ -147,9 +147,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.8 + /// @custom:semver 7.0.9 function version() public pure returns (string memory) { - return "7.0.8"; + return "7.0.9"; } /// @param _standardValidator The standard validator for this OPCM release. @@ -765,6 +765,10 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller { // ETHLockbox contract. if (isDevFeatureEnabled(DevFeatures.OPTIMISM_PORTAL_INTEROP)) { // If we haven't already enabled the ETHLockbox, enable it. + // NOTE: setFeature will revert if the system is currently paused because toggling the + // lockbox changes the pause identifier. This means a guardian pause will block upgrades + // that enable interop. This is acceptable for now since interop is a dev feature and is + // not yet production-ready. if (!_cts.systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX)) { _cts.systemConfig.setFeature(Features.ETH_LOCKBOX, true); } diff --git a/packages/contracts-bedrock/src/libraries/Constants.sol b/packages/contracts-bedrock/src/libraries/Constants.sol index 820a90d2a23..9627f48b229 100644 --- a/packages/contracts-bedrock/src/libraries/Constants.sol +++ b/packages/contracts-bedrock/src/libraries/Constants.sol @@ -51,7 +51,7 @@ library Constants { string internal constant PERMITTED_PROXY_DEPLOYMENT_KEY = "PermittedProxyDeployment"; /// @notice Special constant value for the PermittedProxyDeployment instruction to permit all - /// contracts to be deployed. Only to be used for deployments. + /// contracts to be deployed. Used for both initial deployments and migrations. bytes internal constant PERMIT_ALL_CONTRACTS_INSTRUCTION = bytes("ALL"); /// @notice The minimum OPCM version considered to support OPCM v2.