Repository navigation
refactor(deploy): remove unused SystemDeploy upgrade path [CHAIN-5392] - #452
Merged
Merged
Conversation
Collaborator
✅ Heimdall Review Status
|
Collaborator
🟡 Heimdall Review Status
|
jackchuma
marked this pull request as ready for review
October 13, 2026 13:35
Contributor
|
LGTM !!! |
leopoldjoy
reviewed
Oct 13, 2026
leopoldjoy
approved these changes
Oct 13, 2026
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
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.
Linear: https://linear.app/coinbase/issue/CHAIN-5392/remove-unused-upgrade-path-from-systemdeploy
What changed? Why?
Remove the unused script-level SystemDeploy upgrade API. Actual system upgrades use supported external tooling; keeping a second path required separate helpers and tests that could diverge.
Notes to reviewers
Fresh deployment proxy initialization and implementation artifact saving are preserved. The unreachable MissingImplementations error and check are also removed; the sole validation caller already rejects the empty branch by selecting fresh implementation deployment. The invalid NitroValidator test now exercises supplied deployment implementations rather than the removed upgrade API.
SuperchainConfigNeedsUpgrade was already absent on main. The SystemConfig documentation no longer recommends the removed API. That comment changes its source hash in the regenerated semver lock; all production init-code hashes, ABI snapshots, and storage layouts are unchanged.
The useOpsRepo getter remains for existing test skip behavior. Fork tests without alloc input now fail clearly instead of running local upgrades or reading an unmodified live fork.
The local alloc regression uses a process-wide environment variable. The supported fork recipes restrict tests to
test/L1/**, excluding this regression, so it cannot change their alloc input. Both independent reviewers reported no findings; the acceptance-criteria check confirmed all four criteria.How has it been tested?
Initial implementation validated on commit
17f26133with pinned Foundry 1.5.1:mise exec -- just test --match-path test/deploy/SystemDeploy.t.sol: 18 passed.mise exec -- just test --match-path test/setup/ForkLive.t.sol: 1 regression passed, covering missing input and alloc loading before address derivation.mise exec -- just test: 1,219 passed, 0 failed, 1 existing skip.mise exec -- just snapshots: passed with no uncommitted snapshot changes.mise exec -- just lint-checkandgit diff --check: passed.Live fork validation could not run because ETH_RPC_URL and SUPERCHAIN_OPS_ALLOCS_PATH are unset. The alloc-loading regression uses locally generated state, without live RPC. The unused-import checker is absent from this checkout; manual import-reference checks passed.
Follow-up dead-code cleanup in
561ec068passed all 18 deployment tests,mise exec -- just lint-check, andgit diff --check. No new test was added for removing an unreachable branch.Generated with Toshi