Skip to content

chore: Simplify op-deployer scripts: DeploySuperchain [1/N]#15014

Merged
janjakubnanista merged 14 commits intodevelopfrom
jan/op-deployer--001
Apr 1, 2025
Merged

chore: Simplify op-deployer scripts: DeploySuperchain [1/N]#15014
janjakubnanista merged 14 commits intodevelopfrom
jan/op-deployer--001

Conversation

@janjakubnanista
Copy link
Contributor

@janjakubnanista janjakubnanista commented Mar 24, 2025

Description

An investigation PR focused on reducing the complexity of deploy scripts, taking DeploySuperchain as a test subject. Instead of modifying the original script, DeploySuperchain2 has been introduced to allow for step-by-step migration and side-by-side comparison.

  • Simplifies the interface
    • Removes the input/output contracts and replaces them with input/output structs defined per script contract
    • Places the assertion logic closer to the deployment logic, directly on the deploy script
    • Marks all but the run methods as internal. Any reusable deployment logic should be placed into a separate deploy script that only exposes the run method

Relates to #14976

Notes

  • Currently, the deploy script contract (DeploySuperchain) has partial public functions besides the main entrypoint (and partial assertion functions). To impose a cleaner interface, I propose no such partial functions - instead, separate deployment script contracts should be defined (could be in the same file), each with only one public function (run). This way, the convention runs all the way through the codebase.

@codecov
Copy link

codecov bot commented Mar 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.63%. Comparing base (3ad9a11) to head (d8a4a47).
Report is 22 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #15014       +/-   ##
============================================
+ Coverage    46.42%   85.63%   +39.21%     
============================================
  Files         1183      121     -1062     
  Lines       101251     6238    -95013     
============================================
- Hits         47002     5342    -41660     
+ Misses       50904      884    -50020     
+ Partials      3345       12     -3333     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests 94.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1070 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@janjakubnanista janjakubnanista marked this pull request as ready for review March 28, 2025 19:35
@janjakubnanista janjakubnanista requested a review from a team as a code owner March 28, 2025 19:35
Copy link
Collaborator

@mslipper mslipper left a comment

Choose a reason for hiding this comment

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

this looks good to me - what's the migration plan for the script? is the idea to merge this stack then replace deploysuperchain with deploysuperchain2?

@janjakubnanista janjakubnanista added this pull request to the merge queue Apr 1, 2025
Merged via the queue into develop with commit d8ef500 Apr 1, 2025
50 checks passed
@janjakubnanista janjakubnanista deleted the jan/op-deployer--001 branch April 1, 2025 15:38
@janjakubnanista janjakubnanista self-assigned this Apr 1, 2025
bitwiseguy pushed a commit that referenced this pull request Jun 16, 2025
* chore: Base deploy script

* wip: DeploySuperchain

* chore: Drop unused error

* chore: Rename DeployBase to BaseDeploy

* fix: Typo

* chore: Finish renaming, remove unused imports

* chore: Lint

* chore: Simplify the script

* fix: Lint

* chore: Forgotten import

* chore: Fix typo

* chore: Use bytes32 for ProtocolVersion externally

* chore: Comments

* chore: Lint
iquidus pushed a commit to Layr-Labs/optimism that referenced this pull request Jul 24, 2025
…-optimism#15014)

* chore: Base deploy script

* wip: DeploySuperchain

* chore: Drop unused error

* chore: Rename DeployBase to BaseDeploy

* fix: Typo

* chore: Finish renaming, remove unused imports

* chore: Lint

* chore: Simplify the script

* fix: Lint

* chore: Forgotten import

* chore: Fix typo

* chore: Use bytes32 for ProtocolVersion externally

* chore: Comments

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments