Skip to content

fix(L1): seed imported schedule protocol version - #420

Open
jackchuma wants to merge 3 commits into
mainfrom
fix/protocol-versions-initial-minimum-version
Open

fix(L1): seed imported schedule protocol version#420
jackchuma wants to merge 3 commits into
mainfrom
fix/protocol-versions-initial-minimum-version

Conversation

@jackchuma

@jackchuma jackchuma commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What changed? Why?

  • Extend ProtocolVersions.initialize to set the minimum packed protocol version atomically with an imported schedule and reject non-zero activation timestamps when that version is zero.
  • Plumb protocolVersionsInitialMinimumVersion through the standard and multiproof deployment paths, retaining the config parser's packed-version bound check.
  • Preflight the imported schedule/version invariant before SystemDeploy sends any broadcast transaction, preventing invalid configuration from stranding a partially deployed deterministic system.
  • Add regression and deployment coverage, document the new config field, and refresh the ABI and semver snapshots.

This keeps imported activation schedules immediately consumable by nodes, which reject positive activation timestamps paired with protocol version zero.

Notes to reviewers

  • Empty and zero-only initial schedules may still leave the minimum protocol version at zero.
  • Non-zero initial versions retain the existing 128-bit packed-semver bound.
  • minimumProtocolVersion remains outside the scheduleId commitment.
  • The initializer remains the contract-level trust boundary. SystemDeploy mirrors the invariant only as a preflight because script broadcasts are separate transactions and a later revert cannot roll back earlier broadcasts.
  • The deployment integration test verifies the resulting schedule and minimum version; there is no duplicate post-deployment readback in the script.

How has it been tested?

  • just test
  • just test --match-path test/deploy/SystemDeploy.t.sol
  • just test --match-path test/L1/ProtocolVersions.t.sol
  • just build-no-tests
  • just test --match-test test_cannotReinitialize_succeeds
  • just snapshots
  • forge fmt --check
  • just lint-forge-tests-check-no-build
  • just validate-spacers-no-build

Require imported activations to initialize their minimum packed version atomically so nodes never read a scheduled upgrade with version zero.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

jackchuma and others added 2 commits August 24, 2026 17:52
Rely on the atomic initializer for schedule validation and keep deployment-state verification in the integration test.

Co-authored-by: Cursor <cursoragent@cursor.com>
Validate the imported schedule and minimum version before any broadcast so a bad config cannot strand a partially deployed deterministic system.

Co-authored-by: Cursor <cursoragent@cursor.com>
revert IProtocolVersions.ProtocolVersions_InvalidProtocolVersion();
}
if (_input.initialMinimumProtocolVersion == 0) {
for (uint256 i = 0; i < _input.initialUpgradeSchedule.length; i++) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also validate that non-zero timestamps are ordered here, since an unordered schedule passes this preflight and only reverts in initialize after earlier deployment transactions have been broadcast?

abi.encodeCall(IProtocolVersions.initialize, (address(0), cfg.protocolVersionsInitialSchedule()))
abi.encodeCall(
IProtocolVersions.initialize,
(address(0), cfg.protocolVersionsInitialSchedule(), cfg.protocolVersionsInitialMinimumVersion())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mirror the imported schedule validation in _preflight() before startBroadcast(), otherwise an invalid schedule/minimum-version pair would revert only after the infrastructure and TEE transactions have been sent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants