From d4e47631e851453836b2a61e4bb3428eea8e92fd Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Fri, 11 Sep 2026 09:42:26 -0400 Subject: [PATCH 1/2] fix(L1): require a minimum protocol version to schedule an activation ProtocolVersions.initialize rejects an imported non-zero timestamp paired with a zero minimum protocol version, but the pairing was only enforced at that boundary. registerUpgrade and setTimestamp could still schedule an activation against a registry whose minimum version was zero, which initialize explicitly permits for an empty or zero-only schedule. Nodes attach the contract's global minimum to every timestamp they read, so any positive activation at version zero makes the whole schedule unreadable: strict modes abort startup and the runtime refresher declines to apply it. Enforce the pairing on every write path via a shared assertion. registerUpgrade counts a version supplied in the same call, so a first activation stays a single transaction. setTimestamp cannot supply one, so a registry at zero must call setMinimumProtocolVersion first; that ordering is what removes the window in which a live schedule would be unreadable. delayTimestamp is deliberately exempt: it requires an already-scheduled activation so it cannot introduce the pairing, and guarding it would strand the incident responder on a registry upgraded into this implementation while already holding a zero version. Also ship a non-zero protocolVersionsInitialMinimumVersion in the deploy config. The field is read with a zero default, so the standard deployment previously produced a registry at zero and its first hardfork would have hit the new guard. Co-authored-by: Cursor --- deploy-config/local.json | 1 + snapshots/semver-lock.json | 4 +- src/L1/ProtocolVersions.sol | 25 +++++++++- test/L1/ProtocolVersions.t.sol | 89 ++++++++++++++++++++++++++++++++++ test/deploy/DeployConfig.t.sol | 11 +++-- 5 files changed, 123 insertions(+), 7 deletions(-) diff --git a/deploy-config/local.json b/deploy-config/local.json index c38098f96..4d80f4919 100644 --- a/deploy-config/local.json +++ b/deploy-config/local.json @@ -31,6 +31,7 @@ "operatorFeeVaultWithdrawalNetwork": 0, "p2pSequencerAddress": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", "proofMaturityDelaySeconds": 604800, + "protocolVersionsInitialMinimumVersion": "0x1000000000000000000000000", "proxyAdminOwner": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", "respectedGameType": 621, "sequencerFeeVaultMinimumWithdrawalAmount": "0x8ac7230489e80000", diff --git a/snapshots/semver-lock.json b/snapshots/semver-lock.json index 3e3c23654..3bb1623cf 100644 --- a/snapshots/semver-lock.json +++ b/snapshots/semver-lock.json @@ -16,8 +16,8 @@ "sourceCodeHash": "0x811596e7486cab9ceeeb61405b9ae510d93fcbbdfa11949201a367506c53194a" }, "src/L1/ProtocolVersions.sol:ProtocolVersions": { - "initCodeHash": "0x54c2a142728d60511fd15a7d6bb962b0f33d833ee727de16189fd18b6246b492", - "sourceCodeHash": "0x53c8ad6dc42e61b43b3ef2a91e154a02e719c595c8d1481b4b738faa7ebdf13d" + "initCodeHash": "0x131c2f08ce9665aa75f25c17a837266705e77ea70758d35a2d1f3d03428b4494", + "sourceCodeHash": "0xc4fa0cdadb464286fca2c087e8602dd99b936d6865dd9d13930d095bd7a3524f" }, "src/L1/SuperchainConfig.sol:SuperchainConfig": { "initCodeHash": "0x9b1f3555b499709485d51d5d9665002c0eb1e5eb893be1fb978a30749e894858", diff --git a/src/L1/ProtocolVersions.sol b/src/L1/ProtocolVersions.sol index 78b7cf450..1eaf15063 100644 --- a/src/L1/ProtocolVersions.sol +++ b/src/L1/ProtocolVersions.sol @@ -196,7 +196,8 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable /// @param timestamp Unix activation timestamp (must be >= block.timestamp + MIN_NOTICE), or 0 to /// leave the upgrade unscheduled. /// @param minProtocolVersion New minimum protocol version to set at registration, or 0 to leave - /// the current minimum unchanged. Must fit in 128 bits if non-zero. + /// the current minimum unchanged. Must fit in 128 bits if non-zero. A non-zero + /// `timestamp` requires a non-zero minimum to already be set or supplied here. /// @return The ascending id assigned to the newly registered upgrade. function registerUpgrade(uint64 timestamp, uint256 minProtocolVersion) external returns (uint256) { _assertOnlyProxyAdminOwner(); @@ -205,6 +206,7 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable if (timestamp != 0 && timestamp < uint64(block.timestamp) + MIN_NOTICE) { revert ProtocolVersions_InsufficientNotice(timestamp); } + _assertMinimumProtocolVersionSet(timestamp, minProtocolVersion); _assertTimestampAfterPrevious(id, timestamp); _timestamps.push(0); // Reserve the link slot for this upgrade at index id + 1. @@ -242,6 +244,10 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable /// preexisting activation must still be more than FREEZE_WINDOW away. Pass 0 to remove a /// scheduled timestamp; reverts if the upgrade has already activated or is inside its /// freeze window, or if a later upgrade remains scheduled. + /// @dev Scheduling a non-zero timestamp requires `minimumProtocolVersion` to already be set. This + /// path cannot set it in the same call, so a registry still at zero must call + /// `setMinimumProtocolVersion` first; ordering it that way is what removes the window in + /// which a live schedule would be unreadable to nodes. /// @param id The upgrade to schedule. /// @param timestamp Future Unix timestamp for L2 activation (must be >= block.timestamp + MIN_NOTICE), or 0 to /// clear. @@ -254,6 +260,7 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable if (timestamp != 0 && timestamp < uint64(block.timestamp) + MIN_NOTICE) { revert ProtocolVersions_InsufficientNotice(timestamp); } + _assertMinimumProtocolVersionSet(timestamp, 0); if (timestamp == 0 || current == 0) _assertNoScheduledSuccessor(id); if (timestamp != 0) { _assertTimestampAfterPrevious(id, timestamp); @@ -382,6 +389,22 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable emit ScheduleIdUpdated(prev); } + /// @dev Requires a non-zero activation timestamp to be backed by a non-zero minimum protocol + /// version, counting one supplied in the same call. Nodes attach the global minimum to every + /// imported timestamp and reject any positive activation whose version is zero, so a schedule + /// written without one is unreadable: strict modes abort startup and the runtime refresher + /// declines to apply it. Enforcing this on every write path, not only the initializer's + /// import, keeps the pairing an invariant rather than a deployment-time convention. + /// `delayTimestamp` is deliberately exempt: it requires an already-scheduled activation, so it + /// cannot introduce the pairing, and guarding it would strand the incident responder on a + /// registry upgraded into this implementation while already holding a zero version. + function _assertMinimumProtocolVersionSet(uint64 timestamp, uint256 suppliedProtocolVersion) private view { + if (timestamp == 0) return; + if (suppliedProtocolVersion == 0 && minimumProtocolVersion == 0) { + revert ProtocolVersions_InvalidProtocolVersion(); + } + } + /// @dev Requires upgrade `id`'s scheduled activation to still be more than FREEZE_WINDOW away, /// which is the point past which an L2 block carrying that activation may already exist. function _assertNotFrozen(uint256 id, uint64 current) private view { diff --git a/test/L1/ProtocolVersions.t.sol b/test/L1/ProtocolVersions.t.sol index 19b1ddd86..8a6456bdb 100644 --- a/test/L1/ProtocolVersions.t.sol +++ b/test/L1/ProtocolVersions.t.sol @@ -59,6 +59,18 @@ abstract contract ProtocolVersions_TestInit is CommonTest { proxy.upgradeTo(impl); return IProtocolVersions(address(proxy)); } + + /// @dev Deploys a registry initialized with an empty schedule and no minimum protocol version, + /// which is the state a chain with no upgrade history is allowed to start in. The shared + /// `protocolVersions` instance carries the version from deploy config, so the write-path + /// guard against pairing an activation with a zero version needs this instance to exercise. + function _deployZeroVersionRegistry() internal returns (IProtocolVersions) { + IProtocolVersions registry = _deployUninitializedProxy(); + vm.prank(EIP1967Helper.getAdmin(address(registry))); + registry.initialize(_incidentResponder, new uint64[](0), 0); + assertEq(registry.minimumProtocolVersion(), 0); + return registry; + } } /// @title ProtocolVersions_Initialize_Test @@ -412,6 +424,41 @@ contract ProtocolVersions_RegisterUpgrade_Test is ProtocolVersions_TestInit { protocolVersions.registerUpgrade(0, type(uint128).max); assertEq(protocolVersions.minimumProtocolVersion(), type(uint128).max); } + + /// @notice Tests that scheduling at registration requires a minimum protocol version, since nodes + /// reject a positive activation timestamp paired with version zero. + function test_registerUpgrade_scheduleWithoutMinimumProtocolVersion_reverts() external { + IProtocolVersions registry = _deployZeroVersionRegistry(); + uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); + + vm.expectRevert(IProtocolVersions.ProtocolVersions_InvalidProtocolVersion.selector); + vm.prank(_owner); + registry.registerUpgrade(activation, 0); + } + + /// @notice Tests that registering without scheduling still needs no minimum protocol version. + function test_registerUpgrade_registerOnlyWithoutMinimumProtocolVersion_succeeds() external { + IProtocolVersions registry = _deployZeroVersionRegistry(); + + vm.prank(_owner); + registry.registerUpgrade(0, 0); + + assertEq(registry.getSchedule()[0], 0); + assertEq(registry.minimumProtocolVersion(), 0); + } + + /// @notice Tests that a version supplied in the same call satisfies the guard, so a first + /// activation can be scheduled atomically without a preparatory transaction. + function test_registerUpgrade_scheduleWithSuppliedMinimumProtocolVersion_succeeds() external { + IProtocolVersions registry = _deployZeroVersionRegistry(); + uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); + + vm.prank(_owner); + registry.registerUpgrade(activation, 42); + + assertEq(registry.getSchedule()[0], activation); + assertEq(registry.minimumProtocolVersion(), 42); + } } /// @title ProtocolVersions_SetMinimumProtocolVersion_Test @@ -798,6 +845,48 @@ contract ProtocolVersions_SetTimestamp_Test is ProtocolVersions_TestInit { assertEq(protocolVersions.scheduleId(), link1); } + + /// @notice Tests that scheduling an activation requires a minimum protocol version to already be + /// set, since this path cannot supply one and nodes reject a positive timestamp at zero. + function test_setTimestamp_withoutMinimumProtocolVersion_reverts() external { + IProtocolVersions registry = _deployZeroVersionRegistry(); + vm.prank(_owner); + registry.registerUpgrade(0, 0); + uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); + + vm.expectRevert(IProtocolVersions.ProtocolVersions_InvalidProtocolVersion.selector); + vm.prank(_owner); + registry.setTimestamp(CANYON, activation); + } + + /// @notice Tests that setting the minimum protocol version first unblocks scheduling, which is the + /// ordering that keeps a live schedule readable by nodes at every point in between. + function test_setTimestamp_afterMinimumProtocolVersionSet_succeeds() external { + IProtocolVersions registry = _deployZeroVersionRegistry(); + vm.prank(_owner); + registry.registerUpgrade(0, 0); + vm.prank(_owner); + registry.setMinimumProtocolVersion(42); + + uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); + vm.prank(_owner); + registry.setTimestamp(CANYON, activation); + + assertEq(registry.getSchedule()[CANYON], activation); + } + + /// @notice Tests that clearing a timestamp is unaffected by the guard: a zero activation carries + /// no minimum version requirement, so a registry at zero can still clear. + function test_setTimestamp_clearWithoutMinimumProtocolVersion_succeeds() external { + IProtocolVersions registry = _deployZeroVersionRegistry(); + vm.prank(_owner); + registry.registerUpgrade(0, 0); + + vm.prank(_owner); + registry.setTimestamp(CANYON, 0); + + assertEq(registry.getSchedule()[CANYON], 0); + } } /// @title ProtocolVersions_DelayTimestamp_Test diff --git a/test/deploy/DeployConfig.t.sol b/test/deploy/DeployConfig.t.sol index fdbdd4201..dbb84c25b 100644 --- a/test/deploy/DeployConfig.t.sol +++ b/test/deploy/DeployConfig.t.sol @@ -74,12 +74,15 @@ contract DeployConfig_Test is Test { cfg.readMinimumVersion('{"protocolVersionsInitialMinimumVersion":340282366920938463463374607431768211456}'); } - /// @notice The shipped configs describe chains without a recorded history, so they must keep the initial registry - /// state empty. - function test_read_localConfig_leavesProtocolVersionsStateEmpty_succeeds() public { + /// @notice The shipped configs describe chains without a recorded history, so the imported schedule stays empty. + /// The minimum protocol version must still ship non-zero: scheduling any activation requires one to already be + /// set, so publishing it at deploy time keeps the first hardfork a single owner transaction rather than an + /// ordered pair, and leaves no window in which the live schedule is unreadable to nodes. + function test_read_localConfig_leavesScheduleEmptyWithMinimumVersionSet_succeeds() public { cfg.read("deploy-config/local.json"); assertEq(cfg.protocolVersionsInitialSchedule().length, 0); - assertEq(cfg.protocolVersionsInitialMinimumVersion(), 0); + assertGt(cfg.protocolVersionsInitialMinimumVersion(), 0); + assertLe(cfg.protocolVersionsInitialMinimumVersion(), type(uint128).max); } } From 6f078c3b6ce51773c8fe1584b22b7c295cbbbdb8 Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Fri, 11 Sep 2026 10:25:25 -0400 Subject: [PATCH 2/2] refactor(L1): require initial protocol version Co-authored-by: Codex --- scripts/deploy/DeployConfig.s.sol | 1 + scripts/libraries/DeployUtils.sol | 5 +- scripts/libraries/Types.sol | 3 +- scripts/multiproof/README.md | 2 +- snapshots/semver-lock.json | 4 +- src/L1/ProtocolVersions.sol | 46 +++--------- test/L1/ProtocolVersions.t.sol | 118 ++---------------------------- test/L1/proofs/BaseTest.t.sol | 2 +- test/deploy/DeployConfig.t.sol | 9 +-- test/deploy/SystemDeploy.t.sol | 9 +-- 10 files changed, 29 insertions(+), 170 deletions(-) diff --git a/scripts/deploy/DeployConfig.s.sol b/scripts/deploy/DeployConfig.s.sol index 42e1bbcc8..6097a0e94 100644 --- a/scripts/deploy/DeployConfig.s.sol +++ b/scripts/deploy/DeployConfig.s.sol @@ -136,6 +136,7 @@ contract DeployConfig is Script { function _readProtocolVersionsInitialMinimumVersion(string memory _json) internal { uint256 minimumVersion = _json.readUintOr("$.protocolVersionsInitialMinimumVersion", 0); + require(minimumVersion != 0, "DeployConfig: initial minimum protocol version must be non-zero"); require(minimumVersion <= type(uint128).max, "DeployConfig: initial minimum protocol version exceeds uint128"); protocolVersionsInitialMinimumVersion = minimumVersion; } diff --git a/scripts/libraries/DeployUtils.sol b/scripts/libraries/DeployUtils.sol index 66eaa6a85..3eb15e803 100644 --- a/scripts/libraries/DeployUtils.sol +++ b/scripts/libraries/DeployUtils.sol @@ -106,7 +106,7 @@ library DeployUtils { internal view { - if (_minimumProtocolVersion > type(uint128).max) { + if (_minimumProtocolVersion == 0 || _minimumProtocolVersion > type(uint128).max) { revert IProtocolVersions.ProtocolVersions_InvalidProtocolVersion(); } @@ -117,9 +117,6 @@ library DeployUtils { uint64 previousTimestamp; for (uint256 id = 0; id < _schedule.length; id++) { uint64 timestamp = _schedule[id]; - if (timestamp != 0 && _minimumProtocolVersion == 0) { - revert IProtocolVersions.ProtocolVersions_InvalidProtocolVersion(); - } if (timestamp > currentTimestamp && timestamp < minimumFutureTimestamp) { revert IProtocolVersions.ProtocolVersions_InsufficientNotice(timestamp); } diff --git a/scripts/libraries/Types.sol b/scripts/libraries/Types.sol index 1f6f398b2..4f5d43e00 100644 --- a/scripts/libraries/Types.sol +++ b/scripts/libraries/Types.sol @@ -36,8 +36,7 @@ library Types { /// upgrade in the node's fork order, zero for unscheduled ones. Seeds /// `ProtocolVersions` at initialization, which is the only way to enter /// activations that are already in the past. - /// @custom:field initialMinimumProtocolVersion Packed semver required by non-zero timestamps in the initial - /// schedule. + /// @custom:field initialMinimumProtocolVersion Non-zero packed semver required by nodes. struct DeployInput { Roles roles; uint32 basefeeScalar; diff --git a/scripts/multiproof/README.md b/scripts/multiproof/README.md index be8f1e6af..35dec295b 100644 --- a/scripts/multiproof/README.md +++ b/scripts/multiproof/README.md @@ -56,7 +56,7 @@ Other relevant fields: | `multiproofGenesisOutputRoot` | Initial anchor output root | | `multiproofGenesisBlockNumber` | Initial anchor L2 block number | | `protocolVersionsInitialSchedule` | Hardfork activation timestamps in the node's fork order, `0` for unscheduled forks. Omit for a chain with no history; past activations cannot be added after deployment | -| `protocolVersionsInitialMinimumVersion` | Packed semver required by every non-zero initial schedule timestamp. Must be non-zero when the initial schedule contains an activation | +| `protocolVersionsInitialMinimumVersion` | Non-zero packed semver required by nodes | ### Step 2: Deploy contracts diff --git a/snapshots/semver-lock.json b/snapshots/semver-lock.json index 3bb1623cf..cf5bd40ef 100644 --- a/snapshots/semver-lock.json +++ b/snapshots/semver-lock.json @@ -16,8 +16,8 @@ "sourceCodeHash": "0x811596e7486cab9ceeeb61405b9ae510d93fcbbdfa11949201a367506c53194a" }, "src/L1/ProtocolVersions.sol:ProtocolVersions": { - "initCodeHash": "0x131c2f08ce9665aa75f25c17a837266705e77ea70758d35a2d1f3d03428b4494", - "sourceCodeHash": "0xc4fa0cdadb464286fca2c087e8602dd99b936d6865dd9d13930d095bd7a3524f" + "initCodeHash": "0xbca4c0d69dcb7a196a9ebfcc8cb77162fa090377c5dda99831d61c6c68ba4352", + "sourceCodeHash": "0x411f2590b43bf6fe9d70f393c8e0349192a1aa895ec0fbe0d7695831d3b6cb94" }, "src/L1/SuperchainConfig.sol:SuperchainConfig": { "initCodeHash": "0x9b1f3555b499709485d51d5d9665002c0eb1e5eb893be1fb978a30749e894858", diff --git a/src/L1/ProtocolVersions.sol b/src/L1/ProtocolVersions.sol index 1eaf15063..607401ef9 100644 --- a/src/L1/ProtocolVersions.sol +++ b/src/L1/ProtocolVersions.sol @@ -128,16 +128,15 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable /// already has a hardfork history can be represented faithfully at deployment, while it is /// still impossible for any proof game to have pinned a commitment from this registry. /// Future activations must provide MIN_NOTICE, matching every post-initialization write path. - /// @dev Any non-zero imported timestamp requires a non-zero packed protocol version so nodes can - /// validate the schedule immediately after deployment. + /// @dev A non-zero packed protocol version is required so nodes can validate the registry + /// immediately after deployment. /// /// @param _incidentResponder Initial incidentResponder allowed to delay activations, or address(0) to leave unset. /// @param _initialSchedule Activation timestamps for already-known upgrades, ordered by ascending /// upgrade id, using 0 for an upgrade that is registered but unscheduled. /// Future timestamps must be at least MIN_NOTICE from block.timestamp. /// Pass an empty array for a chain with no upgrade history. - /// @param _initialMinimumProtocolVersion Packed semver required by an imported activation, or 0 when - /// the initial schedule has no non-zero timestamps. + /// @param _initialMinimumProtocolVersion Packed semver required by nodes. Must be non-zero and fit in 128 bits. function initialize( address _incidentResponder, uint64[] calldata _initialSchedule, @@ -148,7 +147,7 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable { // Initialization transactions must come from the ProxyAdmin or its owner. _assertOnlyProxyAdminOrProxyAdminOwner(); - if (_initialMinimumProtocolVersion > type(uint128).max) { + if (_initialMinimumProtocolVersion == 0 || _initialMinimumProtocolVersion > type(uint128).max) { revert ProtocolVersions_InvalidProtocolVersion(); } @@ -159,9 +158,6 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable for (uint256 id = 0; id < _initialSchedule.length; id++) { uint64 timestamp = _initialSchedule[id]; - if (timestamp != 0 && _initialMinimumProtocolVersion == 0) { - revert ProtocolVersions_InvalidProtocolVersion(); - } if (timestamp > uint64(block.timestamp) && timestamp < uint64(block.timestamp) + MIN_NOTICE) { revert ProtocolVersions_InsufficientNotice(timestamp); } @@ -177,9 +173,7 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable // pass. With an empty import this just re-emits the seed as the current commitment. _refreshScheduleId(0); - if (_initialMinimumProtocolVersion != 0) { - _writeMinimumProtocolVersion(_initialMinimumProtocolVersion); - } + _writeMinimumProtocolVersion(_initialMinimumProtocolVersion); incidentResponder = _incidentResponder; emit IncidentResponderUpdated(address(0), _incidentResponder); @@ -196,8 +190,7 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable /// @param timestamp Unix activation timestamp (must be >= block.timestamp + MIN_NOTICE), or 0 to /// leave the upgrade unscheduled. /// @param minProtocolVersion New minimum protocol version to set at registration, or 0 to leave - /// the current minimum unchanged. Must fit in 128 bits if non-zero. A non-zero - /// `timestamp` requires a non-zero minimum to already be set or supplied here. + /// the current minimum unchanged. Must fit in 128 bits if non-zero. /// @return The ascending id assigned to the newly registered upgrade. function registerUpgrade(uint64 timestamp, uint256 minProtocolVersion) external returns (uint256) { _assertOnlyProxyAdminOwner(); @@ -206,7 +199,6 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable if (timestamp != 0 && timestamp < uint64(block.timestamp) + MIN_NOTICE) { revert ProtocolVersions_InsufficientNotice(timestamp); } - _assertMinimumProtocolVersionSet(timestamp, minProtocolVersion); _assertTimestampAfterPrevious(id, timestamp); _timestamps.push(0); // Reserve the link slot for this upgrade at index id + 1. @@ -234,8 +226,9 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable /// @param protocolVersion Packed semver uint256 (must be non-zero and fit in 128 bits). function setMinimumProtocolVersion(uint256 protocolVersion) external { _assertOnlyProxyAdminOwner(); - if (protocolVersion == 0) revert ProtocolVersions_InvalidProtocolVersion(); - if (protocolVersion > type(uint128).max) revert ProtocolVersions_InvalidProtocolVersion(); + if (protocolVersion == 0 || protocolVersion > type(uint128).max) { + revert ProtocolVersions_InvalidProtocolVersion(); + } _writeMinimumProtocolVersion(protocolVersion); } @@ -244,10 +237,6 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable /// preexisting activation must still be more than FREEZE_WINDOW away. Pass 0 to remove a /// scheduled timestamp; reverts if the upgrade has already activated or is inside its /// freeze window, or if a later upgrade remains scheduled. - /// @dev Scheduling a non-zero timestamp requires `minimumProtocolVersion` to already be set. This - /// path cannot set it in the same call, so a registry still at zero must call - /// `setMinimumProtocolVersion` first; ordering it that way is what removes the window in - /// which a live schedule would be unreadable to nodes. /// @param id The upgrade to schedule. /// @param timestamp Future Unix timestamp for L2 activation (must be >= block.timestamp + MIN_NOTICE), or 0 to /// clear. @@ -260,7 +249,6 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable if (timestamp != 0 && timestamp < uint64(block.timestamp) + MIN_NOTICE) { revert ProtocolVersions_InsufficientNotice(timestamp); } - _assertMinimumProtocolVersionSet(timestamp, 0); if (timestamp == 0 || current == 0) _assertNoScheduledSuccessor(id); if (timestamp != 0) { _assertTimestampAfterPrevious(id, timestamp); @@ -389,22 +377,6 @@ contract ProtocolVersions is ProxyAdminOwnedBase, Initializable, Reinitializable emit ScheduleIdUpdated(prev); } - /// @dev Requires a non-zero activation timestamp to be backed by a non-zero minimum protocol - /// version, counting one supplied in the same call. Nodes attach the global minimum to every - /// imported timestamp and reject any positive activation whose version is zero, so a schedule - /// written without one is unreadable: strict modes abort startup and the runtime refresher - /// declines to apply it. Enforcing this on every write path, not only the initializer's - /// import, keeps the pairing an invariant rather than a deployment-time convention. - /// `delayTimestamp` is deliberately exempt: it requires an already-scheduled activation, so it - /// cannot introduce the pairing, and guarding it would strand the incident responder on a - /// registry upgraded into this implementation while already holding a zero version. - function _assertMinimumProtocolVersionSet(uint64 timestamp, uint256 suppliedProtocolVersion) private view { - if (timestamp == 0) return; - if (suppliedProtocolVersion == 0 && minimumProtocolVersion == 0) { - revert ProtocolVersions_InvalidProtocolVersion(); - } - } - /// @dev Requires upgrade `id`'s scheduled activation to still be more than FREEZE_WINDOW away, /// which is the point past which an L2 block carrying that activation may already exist. function _assertNotFrozen(uint256 id, uint64 current) private view { diff --git a/test/L1/ProtocolVersions.t.sol b/test/L1/ProtocolVersions.t.sol index 8a6456bdb..21200cec3 100644 --- a/test/L1/ProtocolVersions.t.sol +++ b/test/L1/ProtocolVersions.t.sol @@ -59,18 +59,6 @@ abstract contract ProtocolVersions_TestInit is CommonTest { proxy.upgradeTo(impl); return IProtocolVersions(address(proxy)); } - - /// @dev Deploys a registry initialized with an empty schedule and no minimum protocol version, - /// which is the state a chain with no upgrade history is allowed to start in. The shared - /// `protocolVersions` instance carries the version from deploy config, so the write-path - /// guard against pairing an activation with a zero version needs this instance to exercise. - function _deployZeroVersionRegistry() internal returns (IProtocolVersions) { - IProtocolVersions registry = _deployUninitializedProxy(); - vm.prank(EIP1967Helper.getAdmin(address(registry))); - registry.initialize(_incidentResponder, new uint64[](0), 0); - assertEq(registry.minimumProtocolVersion(), 0); - return registry; - } } /// @title ProtocolVersions_Initialize_Test @@ -94,7 +82,7 @@ contract ProtocolVersions_Initialize_Test is ProtocolVersions_TestInit { vm.expectEmit(true, true, false, false, address(uninitialized)); emit IncidentResponderUpdated(address(0), _incidentResponder); vm.prank(EIP1967Helper.getAdmin(address(uninitialized))); - uninitialized.initialize(_incidentResponder, new uint64[](0), 0); + uninitialized.initialize(_incidentResponder, new uint64[](0), 1); assertEq(uninitialized.incidentResponder(), _incidentResponder); } @@ -104,7 +92,7 @@ contract ProtocolVersions_Initialize_Test is ProtocolVersions_TestInit { IProtocolVersions uninitialized = _deployUninitializedProxy(); vm.expectRevert(IProxyAdminOwnedBase.ProxyAdminOwnedBase_NotProxyAdminOrProxyAdminOwner.selector); vm.prank(_nonOwner); - uninitialized.initialize(_incidentResponder, new uint64[](0), 0); + uninitialized.initialize(_incidentResponder, new uint64[](0), 1); } /// @notice Tests that the initializer imports a preexisting schedule, building the same hash @@ -182,27 +170,12 @@ contract ProtocolVersions_Initialize_Test is ProtocolVersions_TestInit { imported.initialize(address(0), schedule, 42); } - /// @notice Tests that an imported activation cannot omit the minimum protocol version required by nodes. - function test_initialize_importWithoutMinimumProtocolVersion_reverts() external { - uint64[] memory schedule = new uint64[](1); - schedule[0] = 1; - + /// @notice Tests that the initial minimum protocol version is required even with an empty schedule. + function test_initialize_zeroMinimumProtocolVersion_reverts() external { IProtocolVersions imported = _deployUninitializedProxy(); vm.expectRevert(IProtocolVersions.ProtocolVersions_InvalidProtocolVersion.selector); vm.prank(EIP1967Helper.getAdmin(address(imported))); - imported.initialize(address(0), schedule, 0); - } - - /// @notice Tests that zero-only imports may leave the minimum protocol version unset. - function test_initialize_zeroOnlyScheduleWithoutMinimumProtocolVersion_succeeds() external { - uint64[] memory schedule = new uint64[](1); - - IProtocolVersions imported = _deployUninitializedProxy(); - vm.prank(EIP1967Helper.getAdmin(address(imported))); - imported.initialize(address(0), schedule, 0); - - assertEq(imported.getSchedule().length, 1); - assertEq(imported.minimumProtocolVersion(), 0); + imported.initialize(address(0), new uint64[](0), 0); } /// @notice Tests that the initial minimum protocol version must fit in the node's 128-bit packed semver layout. @@ -217,14 +190,14 @@ contract ProtocolVersions_Initialize_Test is ProtocolVersions_TestInit { function test_initialize_alreadyInitialized_reverts() external { vm.expectRevert("Initializable: contract is already initialized"); vm.prank(EIP1967Helper.getAdmin(address(protocolVersions))); - protocolVersions.initialize(address(0), new uint64[](0), 0); + protocolVersions.initialize(address(0), new uint64[](0), 1); } /// @notice Tests that the implementation itself cannot be initialized (initializers disabled). function test_initialize_implementationDisabled_reverts() external { IProtocolVersions impl = IProtocolVersions(EIP1967Helper.getImplementation(address(protocolVersions))); vm.expectRevert("Initializable: contract is already initialized"); - impl.initialize(address(0), new uint64[](0), 0); + impl.initialize(address(0), new uint64[](0), 1); } } @@ -424,41 +397,6 @@ contract ProtocolVersions_RegisterUpgrade_Test is ProtocolVersions_TestInit { protocolVersions.registerUpgrade(0, type(uint128).max); assertEq(protocolVersions.minimumProtocolVersion(), type(uint128).max); } - - /// @notice Tests that scheduling at registration requires a minimum protocol version, since nodes - /// reject a positive activation timestamp paired with version zero. - function test_registerUpgrade_scheduleWithoutMinimumProtocolVersion_reverts() external { - IProtocolVersions registry = _deployZeroVersionRegistry(); - uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); - - vm.expectRevert(IProtocolVersions.ProtocolVersions_InvalidProtocolVersion.selector); - vm.prank(_owner); - registry.registerUpgrade(activation, 0); - } - - /// @notice Tests that registering without scheduling still needs no minimum protocol version. - function test_registerUpgrade_registerOnlyWithoutMinimumProtocolVersion_succeeds() external { - IProtocolVersions registry = _deployZeroVersionRegistry(); - - vm.prank(_owner); - registry.registerUpgrade(0, 0); - - assertEq(registry.getSchedule()[0], 0); - assertEq(registry.minimumProtocolVersion(), 0); - } - - /// @notice Tests that a version supplied in the same call satisfies the guard, so a first - /// activation can be scheduled atomically without a preparatory transaction. - function test_registerUpgrade_scheduleWithSuppliedMinimumProtocolVersion_succeeds() external { - IProtocolVersions registry = _deployZeroVersionRegistry(); - uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); - - vm.prank(_owner); - registry.registerUpgrade(activation, 42); - - assertEq(registry.getSchedule()[0], activation); - assertEq(registry.minimumProtocolVersion(), 42); - } } /// @title ProtocolVersions_SetMinimumProtocolVersion_Test @@ -845,48 +783,6 @@ contract ProtocolVersions_SetTimestamp_Test is ProtocolVersions_TestInit { assertEq(protocolVersions.scheduleId(), link1); } - - /// @notice Tests that scheduling an activation requires a minimum protocol version to already be - /// set, since this path cannot supply one and nodes reject a positive timestamp at zero. - function test_setTimestamp_withoutMinimumProtocolVersion_reverts() external { - IProtocolVersions registry = _deployZeroVersionRegistry(); - vm.prank(_owner); - registry.registerUpgrade(0, 0); - uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); - - vm.expectRevert(IProtocolVersions.ProtocolVersions_InvalidProtocolVersion.selector); - vm.prank(_owner); - registry.setTimestamp(CANYON, activation); - } - - /// @notice Tests that setting the minimum protocol version first unblocks scheduling, which is the - /// ordering that keeps a live schedule readable by nodes at every point in between. - function test_setTimestamp_afterMinimumProtocolVersionSet_succeeds() external { - IProtocolVersions registry = _deployZeroVersionRegistry(); - vm.prank(_owner); - registry.registerUpgrade(0, 0); - vm.prank(_owner); - registry.setMinimumProtocolVersion(42); - - uint64 activation = uint64(block.timestamp) + registry.MIN_NOTICE(); - vm.prank(_owner); - registry.setTimestamp(CANYON, activation); - - assertEq(registry.getSchedule()[CANYON], activation); - } - - /// @notice Tests that clearing a timestamp is unaffected by the guard: a zero activation carries - /// no minimum version requirement, so a registry at zero can still clear. - function test_setTimestamp_clearWithoutMinimumProtocolVersion_succeeds() external { - IProtocolVersions registry = _deployZeroVersionRegistry(); - vm.prank(_owner); - registry.registerUpgrade(0, 0); - - vm.prank(_owner); - registry.setTimestamp(CANYON, 0); - - assertEq(registry.getSchedule()[CANYON], 0); - } } /// @title ProtocolVersions_DelayTimestamp_Test diff --git a/test/L1/proofs/BaseTest.t.sol b/test/L1/proofs/BaseTest.t.sol index 1f75627da..b0066d574 100644 --- a/test/L1/proofs/BaseTest.t.sol +++ b/test/L1/proofs/BaseTest.t.sol @@ -114,7 +114,7 @@ contract BaseTest is Test { ); factory.initialize(address(this)); delayedWETH.initialize(systemConfig); - protocolVersions.initialize(address(0), new uint64[](0), 0); + protocolVersions.initialize(address(0), new uint64[](0), 1); } /// @dev Rebuilds the schedule registry around a preset schedule and rebinds the verifier to it. diff --git a/test/deploy/DeployConfig.t.sol b/test/deploy/DeployConfig.t.sol index dbb84c25b..3cf73da57 100644 --- a/test/deploy/DeployConfig.t.sol +++ b/test/deploy/DeployConfig.t.sol @@ -63,10 +63,9 @@ contract DeployConfig_Test is Test { assertEq(cfg.protocolVersionsInitialMinimumVersion(), 42); } - function test_readMinimumVersion_omitted_defaultsToZero_succeeds() public { + function test_readMinimumVersion_omitted_reverts() public { + vm.expectRevert("DeployConfig: initial minimum protocol version must be non-zero"); cfg.readMinimumVersion('{"l1ChainId":1}'); - - assertEq(cfg.protocolVersionsInitialMinimumVersion(), 0); } function test_readMinimumVersion_aboveUint128_reverts() public { @@ -75,9 +74,7 @@ contract DeployConfig_Test is Test { } /// @notice The shipped configs describe chains without a recorded history, so the imported schedule stays empty. - /// The minimum protocol version must still ship non-zero: scheduling any activation requires one to already be - /// set, so publishing it at deploy time keeps the first hardfork a single owner transaction rather than an - /// ordered pair, and leaves no window in which the live schedule is unreadable to nodes. + /// The minimum protocol version is required even without recorded upgrade history. function test_read_localConfig_leavesScheduleEmptyWithMinimumVersionSet_succeeds() public { cfg.read("deploy-config/local.json"); diff --git a/test/deploy/SystemDeploy.t.sol b/test/deploy/SystemDeploy.t.sol index 7f39e14b0..02d3e0f7f 100644 --- a/test/deploy/SystemDeploy.t.sol +++ b/test/deploy/SystemDeploy.t.sol @@ -187,12 +187,9 @@ contract SystemDeploy_Test is Test, SystemDeployAssertions { } /// @notice Pins ProtocolVersions input validation before superchain deployment can broadcast. - function test_deploy_initialScheduleWithoutMinimumProtocolVersion_reverts() public { - uint64[] memory schedule = new uint64[](1); - schedule[0] = 1; - + function test_deploy_zeroInitialMinimumProtocolVersion_reverts() public { SystemDeploy.DeployInput memory input = _defaultDeployInput(); - input.opChainInput.initialUpgradeSchedule = schedule; + input.opChainInput.initialMinimumProtocolVersion = 0; input.superchainInput.superchainProxyAdminOwner = address(0); vm.expectRevert(IProtocolVersions.ProtocolVersions_InvalidProtocolVersion.selector); @@ -485,7 +482,7 @@ contract SystemDeploy_Test is Test, SystemDeployAssertions { saltMixer: "system-deploy-test", gasLimit: 60_000_000, initialUpgradeSchedule: new uint64[](0), - initialMinimumProtocolVersion: 0 + initialMinimumProtocolVersion: 1 }); }