contracts-bedrock: Bump OPCM major version to 8.0.0 for U20 - #22096
contracts-bedrock: Bump OPCM major version to 8.0.0 for U20#22096ajsutton wants to merge 5 commits into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
U20 is a new sequential upgrade, and the OPCM versioning rules require a major bump for each one. Without this the release would be tagged against an OPCM still advertising the U19 major. The rule is documented in book/src/policies/versioning.md and restated on version() itself.
The upgrade allowances in _isPermittedInstruction are scoped to a single upgrade by requiring version() < NEXT release. Bumping OPCM to 8.0.0 made that guard false, so the DelayedWETH permitted proxy deployment and the super-root migration startingAnchorRoot override were both rejected, failing every fork upgrade test, the ZK super-root migration tests and the op-deployer chain upgrade. Both allowances are still needed by U20, so move the guard to 9.0.0. The 7.0.0 bump in #18631 did the same. The upgrade sequence check keeps its 8.0.0 threshold: mainnet and sepolia are both on OPCM 7.1.17, so 8.0.0 is a permitted next major.
The unified DelayedWETH shipped with the 7.x OPCM, so every chain that can pass the upgrade sequence check already has a DelayedWETH proxy on its SystemConfig and the upgrade never needs to deploy one. Verified on chain: op-mainnet, op-sepolia, ink-mainnet and unichain-mainnet all report SystemConfig 3.14.2, lastUsedOPCMVersion 7.1.17 and a non-zero delayedWETH(). Closes the TODO from #18382. Drops the instruction from every caller: the fork upgrade setup, the OPCMv2 upgrade tests, op-deployer's upgrade and add-game-type tests and the devstack super-root paths. The duplicate-key test now asserts that duplicate PermittedProxyDeployment keys still skip the duplicate check and fall through to the permission check. superchain-ops' OPCMUpgradeV800 template still injects the instruction and will revert against OPCM 8.0.0; noted in the super-roots upgrade tutorial preconditions.
1e1589f to
2fa980c
Compare
Refreshes the OPContractsManagerV2 hashes after the rebase onto develop.
systemConfigFor was added in #22034 but the ABI snapshot was not regenerated. No CI job diffs the ABI snapshots, so the drift went unnoticed on develop.
|
@OptimismBot Review as the second contracts reviewer |
OptimismBot
left a comment
There was a problem hiding this comment.
Approving as second contracts reviewer. I verified the bump mechanics, the bundled allowance removal, and the snapshot changes; no blockers. Notes below are non-blocking except the description fix, which I'd like before the RC is cut.
This PR includes a major or minor version bump to OPContractsManagerV2.sol.
Reminder of OPCM versioning rules:
- Major bump: Only for a new required sequential upgrade (e.g., U16 → U17)
- Minor bump: Only for replacing an existing OPCM for the same upgrade (e.g., bug fixes, U16a)
- Patch bump: Expected for normal development work
Please confirm this version bump is intentional and follows the versioning policy.
What I checked
lt(_version(), "8.0.0")→"9.0.0"(OPContractsManagerV2.sol:362) is the correct mechanical follow-through — it keeps theoverrides.cfg.startingAnchorRootallowance scoped to exactly the 8.x upgrade, per the comment's "less than the NEXT release version" rule. The allowance stays reachable in U20 becauseSUPER_ROOT_GAMES_MIGRATIONdefaults to enabled (scripts/libraries/Config.sol:340), consistent with your reply to @maurelian.- Removing the DelayedWETH
PermittedProxyDeploymentallowance is fail-closed. Without the instruction,loadOrDeployProxyrevertsOPContractsManagerUtils_ProxyMustLoad("DelayedWETH")rather than silently deploying — and the load source issystemConfig.delayedWETH(), which is necessarily populated on any chain that completed U19 (U19 itself would have reverted withProxyMustLoadotherwise).contracts-bedrock-tests-upgradepasses on bothop-mainnetandink-mainnet, which exercises this against real forked state, so theForkL1Live.s.solchange is backed by evidence rather than just reasoning. OPContractsManagerMigrator.sol:149is unaffected — it builds its ownPERMIT_ALLarray and passes it straight to_loadOrDeployProxy; it never routes through_isPermittedInstruction, so interop migration doesn't regress.- The ABI snapshot commit is legitimate —
systemConfigForis a realexternalfunction onOPContractsManagerUtils.sol:413from #22034, and the snapshot entry matches its signature. Good catch on the undetected drift. - All 80 checks pass, 4 skipping. Nothing hardcodes the old version;
SetInteropDisputeGames.s.sol:76usesgte("7.2.1"), which8.0.0still satisfies.
One thing worth confirming before the RC
This bump silently activates isPermittedUpgradeSequence for the first time. OPContractsManagerV2.sol:1092 still reads lt(_version(), "8.0.0"), which flips from true (skip the check entirely) to false at exactly 8.0.0. That is the documented intent, but the consequence is that from U20 onward _apply calls _systemConfig.lastUsedOPCM() on every upgrade, so any chain not already upgraded by an OPCMv2 7.x reverts instead of being waved through. A 7.1.17 → 8.0.0 chain is fine via isNextMajor, and the mocked unit tests at OPContractsManagerV2.t.sol:1533+ cover the sequence matrix — but the real upgrade tests satisfy the check via isSameOPCM, not via a genuine 7.x → 8.0.0 sequence. Worth confirming the U20 target set is entirely on op-contracts/v7.0.0 before tagging.
Description drift
The body no longer matches the diff, and this is the text that ends up as the release record:
- It says
7.2.3→8.0.0; the diff is7.2.4→8.0.0(#22034 already landed ondevelop). - It says "The ABI and storage-layout snapshots are unchanged", but a471ba6 refreshes
snapshots/abi/OPContractsManagerUtils.json. - It says the bump is "not buried in a behavioural change", but the PR also removes the DelayedWETH proxy-deployment allowance and retargets the allowance gate — a behavioural change that cascades to op-deployer, op-devstack, the fork tests, and the chain-operator docs. The removal is correct and the fork tests back it, but it should be stated in the summary, along with the fact that any existing
superchain-opstemplate still injectingPermittedProxyDeploymentwill now revert.
Minor
_isMatchingInstruction (the key+data variant, OPContractsManagerUtilsCaller.sol:88) now has no callers, and the duplicate-key exemption at OPContractsManagerV2.sol:331 no longer has any permittable instruction to exempt. Both are worth keeping for the next upgrade that needs a proxy deployment — just flagging so they aren't mistaken for live logic.
Summary
Bumps
OPContractsManagerV2from7.2.3to8.0.0.OPCM does not follow the normal contract semver rules. Per
book/src/policies/versioning.md— and restated in the doc comment onversion()itself — a major bump marks a new required sequential upgrade, a minor bump marks a replacement OPCM for the same upgrade, and patch bumps are ordinary development churn. U19 shipped OPCM7.xwithop-contracts/v7.0.0; U20 is the next sequential upgrade, so OPCM needs to land on8.0.0before the release is tagged.This also settles the release version.
versioning.mdderives the monorepo release from "the highest version bump of any individual contract in the release", so an OPCM major makes U20op-contracts/v8.0.0. That is consistent with the other majors already ondevelop(SuperPermissionedDisputeGame0.7.0 → 1.0.0,ProtocolVersionsremoved) and withSystemConfig3.14.2 → 4.0.0 in #21641.Doing this as its own PR so the version bump is not buried in a behavioural change, and so the RC can be cut as soon as the remaining U20 PRs land.
Testing
just snapshotsregenerated the lock; the only change isOPContractsManagerV2'sinitCodeHashandsourceCodeHash. The ABI and storage-layout snapshots are unchanged, since"7.2.3"and"8.0.0"are both 5-byte strings and nothing else moved.just snapshots-check-no-buildis clean on re-run.just test --match-contract OPContractsManagerpasses: 98 passed, 0 failed, 47 skipped. Nothing in the repo hardcodes the old7.2.3string.Note on merge order
#21641 and #22034 both bump OPCM to
7.2.4, so whichever of the three lands last needs a trivial rebase onto8.0.0. Worth landing this one last for that reason.