contracts-bedrock: remove ReinitializableBase - #22124
Draft
claude[bot] wants to merge 2 commits into
Draft
Conversation
ReinitializableBase existed so that contracts could force re-initialization during upgrades by bumping an INIT_VERSION constant passed to the reinitializer() modifier. Since OPCM v2 (#18079), upgrades reset the initialized slot via StorageSetter before calling initialize(), so the version value no longer gates anything and the plain initializer modifier is sufficient. - Delete ReinitializableBase, IReinitializableBase, and their tests. - Replace reinitializer(initVersion()) with initializer in the 10 inheriting contracts and drop the initVersion() getter from their interfaces (minor semver bumps). - Simplify DeployUtils.assertInitialized to expect an initialized value of exactly 1 for proxies. - Drop the initVersion() special case from scripts/checks/reinitializer. - Remove the semgrep immutables exclusion for ReinitializableBase. - Update contract-dev and OPCM contributing docs accordingly. - Regenerate ABI snapshots and semver-lock. Co-Authored-By: Claude Co-authored-by: John Mardlin <john@oplabs.co>
Collapse the initialize() signatures of ETHLockbox, L1ERC721Bridge, and L1StandardBridge onto a single line now that dropping the reinitializer(initVersion()) modifier makes them fit, matching forge fmt. Whitespace-only: initCodeHash values are unchanged, only sourceCodeHash entries in the semver lock move. Co-Authored-By: Claude Co-authored-by: John Mardlin <john@oplabs.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by John Mardlin · Slack thread
Description
Removes the
ReinitializableBasecontract, which is vestigial now that OPCM upgrades reset the initialized slot instead of bumping reinitializer versions.Before: every proxied L1/dispute contract inherited
ReinitializableBase(N)and guardedinitialize()withreinitializer(initVersion()). The version constant existed so that upgrades could force re-initialization by bumpingNpast the value stored in the proxy's_initializedslot.After:
initialize()uses the plaininitializermodifier and the base contract, its interface, and theinitVersion()getter are gone. This is safe because the OPCMv2 upgrade flow (OPContractsManagerUtils.upgrade) zeroes the_initializedbyte viaStorageSetterbefore callinginitialize(), so any reinitializer version ≥ 1 behaved identically; the version value no longer gated anything. Fresh proxy deployments start with_initialized = 0, so they are unaffected. Access control oninitialize()(ProxyAdmin / ProxyAdmin owner checks) is unchanged.How
src/universal/ReinitializableBase.sol,interfaces/universal/IReinitializableBase.sol, andtest/universal/ReinitializableBase.t.sol.reinitializer(initVersion())withinitializer(minor version bumps, matching the convention used when theupgrade()functions were removed in feat: set up OPCM for U17 #17406):SystemConfig3.14.2 → 3.15.0OptimismPortal25.8.0 → 5.9.0L1CrossDomainMessenger2.11.1 → 2.12.0L1ERC721Bridge2.9.1 → 2.10.0L1StandardBridge2.8.2 → 2.9.0SuperchainConfig2.4.3 → 2.5.0ETHLockbox1.3.1 → 1.4.0AnchorStateRegistry3.9.0 → 3.10.0DelayedWETH1.5.1 → 1.6.0DisputeGameFactory1.6.1 → 1.7.0initVersion()/ReinitializableBase_ZeroInitVersion()from the corresponding interfaces.DeployUtils.assertInitializednow expects the proxy_initializedvalue to be exactly 1 (previously matchedinitVersion()with a fallback to 1).scripts/checks/reinitializer: removed thereinitializer(initVersion())special case; literal reinitializer values are still checked for upgrade/initialize parity.ReinitializableBase.solexclusion from the semgrep immutable-variables rule.docs/ai/contract-dev.md(contract pattern) andbook/src/contributing/opcm.md(upgrade guidance, which still described the pre-U17upgrade()+reinitializerflow).initVersion()to assert it equals 1.semver-lock.json.Intentionally untouched:
op-challenger/game/fault/contracts/abis/DisputeGameFactory-1.2.0.json(frozen historical ABI) and the legacy generated bindingop-e2e/bindings/systemconfig.go(no callers ofInitVersion).History
cbe992160b) introducedReinitializableBase(interop portal updates); rolled out further in Stage 1 changes implementation #15174 and feat: standardize upgradeable L1 contracts #15615.c09c867e49) is an example of the old pattern: bumpedINIT_VERSION2 → 3 onSystemConfigandL1StandardBridgewhen initializer inputs changed.33efd658f4, U17 OPCM setup) deleted the per-contractupgrade() external reinitializer(initVersion())functions.ec8ce8753d, OPCM v2) introduced the reset-the-initialized-slot upgrade pattern, after which noINIT_VERSIONwas ever bumped again and all new contracts used version 1.Generated by Claude Code