contracts-bedrock: remove ReinitializableBase

.semgrep/rules/sol-rules.yaml

@@ -343,7 +343,6 @@ rules:
         - /packages/contracts-bedrock/src/safe/LivenessGuard.sol
         - /packages/contracts-bedrock/src/safe/LivenessModule.sol
         - /packages/contracts-bedrock/src/universal/OptimismMintableERC20.sol
-        - /packages/contracts-bedrock/src/universal/ReinitializableBase.sol
 
   - id: sol-style-use-process-run
     languages: [solidity]

docs/ai/contract-dev.md

@@ -143,8 +143,9 @@ Every new implementation contract must follow this pattern:
 1. Extend OpenZeppelin's `Initializable`.
 2. Include `initialize()` with the `initializer` modifier.
 3. In the constructor: call `_disableInitializers()` and set immutables only.
-4. Extend `ReinitializableBase(N)` with the current init version.
-5. Never use `reinitializer(uint64 version)` — this codebase does not use it.
+4. Never use `reinitializer(uint64 version)` — this codebase does not use it. Upgrades
+   re-initialize by zeroing the initialized slot first (see below), so plain
+   `initializer` is sufficient.
 
 ### Upgrade Process (Atomic 3-Step)
 
@@ -207,7 +208,7 @@ import { ISemver } from "interfaces/universal/ISemver.sol";
 /// @custom:proxied true
 /// @title ContractName
 /// @notice Description
-contract ContractName is Initializable, ProxyAdminOwnedBase, ReinitializableBase, ISemver {
+contract ContractName is Initializable, ProxyAdminOwnedBase, ISemver {
     // Constants and immutables
     // Custom errors
     // Events

packages/contracts-bedrock/book/src/contributing/opcm.md

@@ -100,15 +100,11 @@ Typically a contract's `initializer()` will be modified when a new storage varia
 needs to be set. This may either be done by adding a new argument to the `initializer()`, by
 reading a value from the environment, or by reading a value from another contract.
 
-Whatever the case, a new `upgrade()` method should also be added which uses the same logic to set
-the new storage variable.
-
-In addition, the contract should inherit from `ReinitializableBase` and the `initializer()` and `upgrade()`
-methods should have the `reinitializer(reinitVersion())` modifier.
-
-The OPCM must then use `ProxyAdmin.upgradeAndCall()` to call the `upgrade()` method. Additionally, if the
-input value is not read from the chain, then it will need to be passed in as input. This will
-require a new field to be added to the `OpChainConfig` struct.
+No special handling is required in the contract itself: the OPCM upgrade flow resets the
+initialized slot (via `StorageSetter`) and then calls `initialize()` again with the full set of
+inputs, so the plain `initializer` modifier is sufficient. If the new input value is not read from
+the chain, then it will need to be passed in as input to the upgrade. This will require a new
+field to be added to the `OpChainConfig` struct.
 
 ### New derivation path events are being added
 

packages/contracts-bedrock/interfaces/L1/IETHLockbox.sol

@@ -6,9 +6,8 @@ import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
 import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol";
 import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
-import { IReinitializableBase } from "interfaces/universal/IReinitializableBase.sol";
 
-interface IETHLockbox is IProxyAdminOwnedBase, ISemver, IReinitializableBase {
+interface IETHLockbox is IProxyAdminOwnedBase, ISemver {
     error ETHLockbox_Unauthorized();
     error ETHLockbox_Paused();
     error ETHLockbox_InsufficientBalance();

packages/contracts-bedrock/interfaces/L1/IL1CrossDomainMessenger.sol

@@ -8,11 +8,9 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
 
 interface IL1CrossDomainMessenger is ICrossDomainMessenger, IProxyAdminOwnedBase {
-    error ReinitializableBase_ZeroInitVersion();
 
     function PORTAL() external view returns (IOptimismPortal);
     function initialize(ISystemConfig _systemConfig, IOptimismPortal _portal) external;
-    function initVersion() external view returns (uint8);
     function portal() external view returns (IOptimismPortal);
     function systemConfig() external view returns (ISystemConfig);
     function version() external view returns (string memory);

packages/contracts-bedrock/interfaces/L1/IL1ERC721Bridge.sol

@@ -8,9 +8,7 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
 
 interface IL1ERC721Bridge is IERC721Bridge, IProxyAdminOwnedBase {
-    error ReinitializableBase_ZeroInitVersion();
 
-    function initVersion() external view returns (uint8);
     function bridgeERC721(
         address _localToken,
         address _remoteToken,

packages/contracts-bedrock/interfaces/L1/IL1StandardBridge.sol

@@ -8,7 +8,6 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
 
 interface IL1StandardBridge is IStandardBridge, IProxyAdminOwnedBase {
-    error ReinitializableBase_ZeroInitVersion();
 
     event ERC20DepositInitiated(
         address indexed l1Token,
@@ -29,7 +28,6 @@ interface IL1StandardBridge is IStandardBridge, IProxyAdminOwnedBase {
     event ETHDepositInitiated(address indexed from, address indexed to, uint256 amount, bytes extraData);
     event ETHWithdrawalFinalized(address indexed from, address indexed to, uint256 amount, bytes extraData);
 
-    function initVersion() external view returns (uint8);
     function depositERC20(
         address _l1Token,
         address _l2Token,

packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol

@@ -16,7 +16,6 @@ interface IOptimismPortal2 is IProxyAdminOwnedBase {
     error EmptyItem();
     error InvalidDataRemainder();
     error InvalidHeader();
-    error ReinitializableBase_ZeroInitVersion();
     error OptimismPortal_AlreadyFinalized();
     error OptimismPortal_BadTarget();
     error OptimismPortal_CallPaused();
@@ -84,7 +83,6 @@ interface IOptimismPortal2 is IProxyAdminOwnedBase {
     function finalizedWithdrawals(bytes32) external view returns (bool);
     function guardian() external view returns (address);
     function initialize(ISystemConfig _systemConfig, IAnchorStateRegistry _anchorStateRegistry, IETHLockbox _ethLockbox) external;
-    function initVersion() external view returns (uint8);
     function l2Sender() external view returns (address);
     function migrateLiquidity() external;
     function migrateToSharedDisputeGame(IETHLockbox _newLockbox, IAnchorStateRegistry _newAnchorStateRegistry) external;

packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol

@@ -16,7 +16,6 @@ interface ISuperchainConfig is IProxyAdminOwnedBase {
     error SuperchainConfig_OnlyGuardian();
     error SuperchainConfig_AlreadyPaused(address identifier);
     error SuperchainConfig_NotAlreadyPaused(address identifier);
-    error ReinitializableBase_ZeroInitVersion();
 
     function guardian() external view returns (address);
     function initialize(address _guardian) external;
@@ -30,7 +29,6 @@ interface ISuperchainConfig is IProxyAdminOwnedBase {
     function version() external view returns (string memory);
     function pauseTimestamps(address) external view returns (uint256);
     function pauseExpiry() external view returns (uint256);
-    function initVersion() external view returns (uint8);
 
     function __constructor__() external;
 }

packages/contracts-bedrock/interfaces/L1/ISystemConfig.sol

@@ -27,7 +27,6 @@ interface ISystemConfig is IProxyAdminOwnedBase {
         address opcm;
     }
 
-    error ReinitializableBase_ZeroInitVersion();
     error SystemConfig_InvalidFeatureState();
 
     event ConfigUpdate(uint256 indexed version, UpdateType indexed updateType, bytes data);
@@ -66,7 +65,6 @@ interface ISystemConfig is IProxyAdminOwnedBase {
         ISuperchainConfig _superchainConfig
     )
         external;
-    function initVersion() external view returns (uint8);
     function l1CrossDomainMessenger() external view returns (address addr_);
     function l1ERC721Bridge() external view returns (address addr_);
     function l1StandardBridge() external view returns (address addr_);

packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol

@@ -11,15 +11,13 @@ import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.
 interface IAnchorStateRegistry is IProxyAdminOwnedBase {
     error AnchorStateRegistry_InvalidAnchorGame();
     error AnchorStateRegistry_Unauthorized();
-    error ReinitializableBase_ZeroInitVersion();
 
     event AnchorUpdated(IDisputeGame indexed game);
     event DisputeGameBlacklisted(IDisputeGame indexed disputeGame);
     event Initialized(uint8 version);
     event RespectedGameTypeSet(GameType gameType);
     event RetirementTimestampSet(uint256 timestamp);
 
-    function initVersion() external view returns (uint8);
     function anchorGame() external view returns (IDisputeGame);
     function anchors(GameType) external view returns (Hash, uint256);
     function blacklistDisputeGame(IDisputeGame _disputeGame) external;

packages/contracts-bedrock/interfaces/dispute/IDelayedWETH.sol

@@ -6,7 +6,6 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
 
 interface IDelayedWETH is IProxyAdminOwnedBase {
-    error ReinitializableBase_ZeroInitVersion();
 
     struct WithdrawalRequest {
         uint256 amount;
@@ -18,7 +17,6 @@ interface IDelayedWETH is IProxyAdminOwnedBase {
     fallback() external payable;
     receive() external payable;
 
-    function initVersion() external view returns (uint8);
     function systemConfig() external view returns (ISystemConfig);
     function delay() external view returns (uint256);
     function hold(address _guy) external;

packages/contracts-bedrock/interfaces/dispute/IDisputeGameFactory.sol

@@ -4,9 +4,8 @@ pragma solidity ^0.8.0;
 import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol";
 import { GameId, Timestamp, Claim, Hash, GameType } from "src/dispute/lib/Types.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
-import { IReinitializableBase } from "interfaces/universal/IReinitializableBase.sol";
 
-interface IDisputeGameFactory is IProxyAdminOwnedBase, IReinitializableBase {
+interface IDisputeGameFactory is IProxyAdminOwnedBase {
     struct GameSearchResult {
         uint256 index;
         GameId metadata;

packages/contracts-bedrock/scripts/checks/reinitializer/main.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"fmt"
-	"math"
 	"os"
 	"strconv"
 	"strings"
@@ -105,22 +104,17 @@ func getReinitializerValue(node *solc.AstNode) (uint64, error) {
 	for _, modifier := range node.Modifiers {
 		if modifier.ModifierName.Name == "reinitializer" {
 			if modifier.Arguments[0].Kind == "functionCall" {
-				if modifier.Arguments[0].Expression.Name == "initVersion" {
-					return math.MaxUint64, nil // uint64 max representing initVersion call
-				} else {
-					return 0, fmt.Errorf("reinitializer value is not a call to initVersion")
-				}
-			} else {
-				valStr, ok := modifier.Arguments[0].Value.(string)
-				if !ok {
-					return 0, fmt.Errorf("reinitializer value is not a string")
-				}
-				val, err := strconv.Atoi(valStr)
-				if err != nil {
-					return 0, fmt.Errorf("reinitializer value is not an integer")
-				}
-				return uint64(val), nil
+				return 0, fmt.Errorf("reinitializer value must be a literal integer")
 			}
+			valStr, ok := modifier.Arguments[0].Value.(string)
+			if !ok {
+				return 0, fmt.Errorf("reinitializer value is not a string")
+			}
+			val, err := strconv.Atoi(valStr)
+			if err != nil {
+				return 0, fmt.Errorf("reinitializer value is not an integer")
+			}
+			return uint64(val), nil
 		}
 	}
 

packages/contracts-bedrock/scripts/checks/reinitializer/main_test.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"math"
 	"testing"
 
 	"github.com/ethereum-optimism/optimism/op-chain-ops/solc"
@@ -157,23 +156,7 @@ func TestGetReinitializerValue(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "Valid reinitializer with initVersion call",
-			node: &solc.AstNode{
-				Modifiers: []solc.AstNode{
-					{
-						ModifierName: &solc.Expression{Name: "reinitializer"},
-						Arguments: []solc.Expression{{
-							Kind:       "functionCall",
-							Expression: &solc.Expression{Name: "initVersion"},
-						}},
-					},
-				},
-			},
-			want:    math.MaxUint64,
-			wantErr: false,
-		},
-		{
-			name: "Invalid function call - not initVersion",
+			name: "Invalid function call argument",
 			node: &solc.AstNode{
 				Modifiers: []solc.AstNode{
 					{
@@ -518,48 +501,7 @@ func TestCheckArtifact(t *testing.T) {
 			wantErr: false, // Should return nil without error
 		},
 		{
-			name: "Matching reinitializer values with initVersion",
-			artifact: &solc.ForgeArtifact{
-				Ast: solc.Ast{
-					Nodes: []solc.AstNode{
-						{
-							NodeType: "ContractDefinition",
-							Nodes: []solc.AstNode{
-								{
-									NodeType: "FunctionDefinition",
-									Name:     "initialize",
-									Modifiers: []solc.AstNode{
-										{
-											ModifierName: &solc.Expression{Name: "reinitializer"},
-											Arguments: []solc.Expression{{
-												Kind:       "functionCall",
-												Expression: &solc.Expression{Name: "initVersion"},
-											}},
-										},
-									},
-								},
-								{
-									NodeType: "FunctionDefinition",
-									Name:     "upgrade",
-									Modifiers: []solc.AstNode{
-										{
-											ModifierName: &solc.Expression{Name: "reinitializer"},
-											Arguments: []solc.Expression{{
-												Kind:       "functionCall",
-												Expression: &solc.Expression{Name: "initVersion"},
-											}},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			wantErr: false,
-		},
-		{
-			name: "Mismatched reinitializer values - one with initVersion, one with constant",
+			name: "Invalid reinitializer value - function call argument",
 			artifact: &solc.ForgeArtifact{
 				Ast: solc.Ast{
 					Nodes: []solc.AstNode{
@@ -574,7 +516,7 @@ func TestCheckArtifact(t *testing.T) {
 											ModifierName: &solc.Expression{Name: "reinitializer"},
 											Arguments: []solc.Expression{{
 												Kind:       "functionCall",
-												Expression: &solc.Expression{Name: "initVersion"},
+												Expression: &solc.Expression{Name: "someFunction"},
 											}},
 										},
 									},

packages/contracts-bedrock/scripts/libraries/DeployUtils.sol

@@ -17,7 +17,6 @@ import { IProxy } from "interfaces/universal/IProxy.sol";
 import { IAddressManager } from "interfaces/legacy/IAddressManager.sol";
 import { IL1ChugSplashProxy, IStaticL1ChugSplashProxy } from "interfaces/legacy/IL1ChugSplashProxy.sol";
 import { IResolvedDelegateProxy } from "interfaces/legacy/IResolvedDelegateProxy.sol";
-import { IReinitializableBase } from "interfaces/universal/IReinitializableBase.sol";
 
 library DeployUtils {
     Vm internal constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));
@@ -455,19 +454,7 @@ library DeployUtils {
         bytes32 slotVal = vm.load(_contractAddress, bytes32(_slot));
         uint8 val = uint8((uint256(slotVal) >> (_offset * 8)) & 0xFF);
         if (_isProxy) {
-            // Using a try/catch here to check if the contract has an initVersion() defined.
-            // EIP-150 safe because we require that we have at least 200k gas before the call which
-            // is more than enough to avoid running out of gas when 63/64 of the gas is provided to
-            // the initVersion() call (which simply reads an immutable variable). Since this is
-            // only ever triggered as part of a script, we can safely assume we'll have the gas.
-            require(gasleft() > 200_000, "DeployUtils: insufficient gas for initVersion() call");
-
-            // eip150-safe
-            try IReinitializableBase(_contractAddress).initVersion() returns (uint8 initVersion_) {
-                require(val == initVersion_, "DeployUtils: storage value is incorrect at the given slot and offset");
-            } catch {
-                require(val == 1, "DeployUtils: storage value is not set at the given slot and offset");
-            }
+            require(val == 1, "DeployUtils: storage value is not set at the given slot and offset");
         } else {
             require(val == type(uint8).max, "DeployUtils: storage value is not 0xff at the given slot and offset");
         }

packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json

@@ -148,19 +148,6 @@
     "stateMutability": "view",
     "type": "function"
   },
-  {
-    "inputs": [],
-    "name": "initVersion",
-    "outputs": [
-      {
-        "internalType": "uint8",
-        "name": "",
-        "type": "uint8"
-      }
-    ],
-    "stateMutability": "view",
-    "type": "function"
-  },
   {
     "inputs": [
       {
@@ -594,10 +581,5 @@
     "inputs": [],
     "name": "ProxyAdminOwnedBase_ProxyAdminNotFound",
     "type": "error"
-  },
-  {
-    "inputs": [],
-    "name": "ReinitializableBase_ZeroInitVersion",
-    "type": "error"
   }
 ]