Skip to content

Commit 5161204

Browse files
contracts: add documentation for audit findings #2, #3, #7, #12, #15, #16 (ethereum-optimism#19271)
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 <noreply@anthropic.com>
1 parent 991d66a commit 5161204

5 files changed

Lines changed: 22 additions & 8 deletions

File tree

packages/contracts-bedrock/scripts/deploy/VerifyOPCM.s.sol

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,10 @@ contract VerifyOPCM is Script {
108108
uint256 constant MAX_INIT_CODE_SIZE = 23500;
109109

110110
/// @notice Represents a contract name and its corresponding address.
111-
/// @param field Name of the field the address was extracted from.
112-
/// @param name Name of the contract.
113-
/// @param addr Address of the contract.
111+
/// @param field Name of the field the address was extracted from.
112+
/// @param name Name of the contract.
113+
/// @param addr Address of the contract.
114+
/// @param blueprint Whether the contract is a blueprint deployment.
114115
struct OpcmContractRef {
115116
string field;
116117
string name;

packages/contracts-bedrock/snapshots/semver-lock.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@
5252
"sourceCodeHash": "0xb3184aa5d95a82109e7134d1f61941b30e25f655b9849a0e303d04bbce0cde0b"
5353
},
5454
"src/L1/opcm/OPContractsManagerV2.sol:OPContractsManagerV2": {
55-
"initCodeHash": "0x5cbc998e57035d8658824e16dacaab8c702f9e18f482e16989b9420e5a7e8190",
56-
"sourceCodeHash": "0x11678225efb1fb4593085febd8f438eeb4752c0ab3dfd2ee1c4fe47970dda953"
55+
"initCodeHash": "0x88ada0dfefb77eea33baaf11d9b5a5ad51cb8c6476611d0f2376897413074619",
56+
"sourceCodeHash": "0x1cc9dbcd4c7652f482c43e2630b324d088e825d12532711a41c636e8392636b3"
5757
},
5858
"src/L2/BaseFeeVault.sol:BaseFeeVault": {
5959
"initCodeHash": "0x838bbd7f381e84e21887f72bd1da605bfc4588b3c39aed96cbce67c09335b3ee",

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrator.sol

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ contract OPContractsManagerMigrator is OPContractsManagerUtilsCaller {
6363
/// temporary need to support the interop migration action. It will likely be removed in
6464
/// the near future once interop support is baked more directly into OPCM. It does NOT
6565
/// look or function like all of the other functions in OPCMv2.
66+
/// @dev NOTE: This function is designed exclusively for the case of N independent pre-interop
67+
/// chains merging into a single interop set. It does NOT support partial migration (i.e.,
68+
/// migrating a subset of chains that share a lockbox), re-migration of already-migrated
69+
/// chains, or any other migration scenario. Re-calling this function on already-migrated
70+
/// portals will corrupt the shared DisputeGameFactory used by all migrated chains.
6671
/// @param _input The input parameters for the migration.
6772
function migrate(MigrateInput calldata _input) public {
6873
// Check that the starting respected game type is a valid super game type.
@@ -156,6 +161,10 @@ contract OPContractsManagerMigrator is OPContractsManagerUtilsCaller {
156161
IOPContractsManagerContainer.Implementations memory impls = contractsContainer().implementations();
157162

158163
// Initialize the new ETHLockbox.
164+
// NOTE: Shared contracts (ETHLockbox, AnchorStateRegistry, DelayedWETH) are
165+
// intentionally initialized with chainSystemConfigs[0]. All chains are validated to
166+
// share the same ProxyAdmin owner and SuperchainConfig, so the first chain's
167+
// SystemConfig is used as the canonical governance reference for shared contracts.
159168
_upgrade(
160169
proxyDeployArgs.proxyAdmin,
161170
address(ethLockbox),

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
147147
/// - Major bump: New required sequential upgrade
148148
/// - Minor bump: Replacement OPCM for same upgrade
149149
/// - Patch bump: Development changes (expected for normal dev work)
150-
/// @custom:semver 7.0.8
150+
/// @custom:semver 7.0.9
151151
function version() public pure returns (string memory) {
152-
return "7.0.8";
152+
return "7.0.9";
153153
}
154154

155155
/// @param _standardValidator The standard validator for this OPCM release.
@@ -765,6 +765,10 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
765765
// ETHLockbox contract.
766766
if (isDevFeatureEnabled(DevFeatures.OPTIMISM_PORTAL_INTEROP)) {
767767
// If we haven't already enabled the ETHLockbox, enable it.
768+
// NOTE: setFeature will revert if the system is currently paused because toggling the
769+
// lockbox changes the pause identifier. This means a guardian pause will block upgrades
770+
// that enable interop. This is acceptable for now since interop is a dev feature and is
771+
// not yet production-ready.
768772
if (!_cts.systemConfig.isFeatureEnabled(Features.ETH_LOCKBOX)) {
769773
_cts.systemConfig.setFeature(Features.ETH_LOCKBOX, true);
770774
}

packages/contracts-bedrock/src/libraries/Constants.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ library Constants {
5151
string internal constant PERMITTED_PROXY_DEPLOYMENT_KEY = "PermittedProxyDeployment";
5252

5353
/// @notice Special constant value for the PermittedProxyDeployment instruction to permit all
54-
/// contracts to be deployed. Only to be used for deployments.
54+
/// contracts to be deployed. Used for both initial deployments and migrations.
5555
bytes internal constant PERMIT_ALL_CONTRACTS_INSTRUCTION = bytes("ALL");
5656

5757
/// @notice The minimum OPCM version considered to support OPCM v2.

0 commit comments

Comments
 (0)