Skip to content

ctb/op-deployer: Simplify superchain config/protocol versions validations#14498

Merged
mslipper merged 2 commits intodevelopfrom
bugfix/remove-pv-checks
Feb 24, 2025
Merged

ctb/op-deployer: Simplify superchain config/protocol versions validations#14498
mslipper merged 2 commits intodevelopfrom
bugfix/remove-pv-checks

Conversation

@mslipper
Copy link
Copy Markdown
Contributor

@mslipper mslipper commented Feb 24, 2025

While running the validators against newly-deployed chains, we discovered a bug where the superchain config's implementation cannot be checked unless the same proxy admin is shared between both the OP Chain and the superchain config. This only makes sense for OP Mainnet, so to simplify things I've removed the protocol version/superchain config checks from the standard validator. It will still check that the OP Chain is using the right superchain config, but it won't validate the config itself. We can use other tooling for that.

To make the OP Validator tooling work I also redeployed the contracts.

…ions

While running the validators against newly-deployed chains, we discovered a bug where the superchain config's implementation cannot be checked unless the same proxy admin is shared between both the OP Chain and the superchain config. This only makes sense for OP Mainnet, so to simplify things I've removed the protocol version/superchain config checks from the standard validator. It will still check that the OP Chain is using the right superchain config, but it won't validate the config itself. We can use other tooling for that.
@mslipper mslipper requested review from a team as code owners February 24, 2025 02:24
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.32%. Comparing base (427ed0b) to head (2b094cc).
Report is 3 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14498      +/-   ##
===========================================
- Coverage    46.59%   45.32%   -1.28%     
===========================================
  Files         1029      972      -57     
  Lines        88455    83755    -4700     
===========================================
- Hits         41215    37960    -3255     
+ Misses       44165    42902    -1263     
+ Partials      3075     2893     -182     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests 94.58% <100.00%> (+0.05%) ⬆️

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

Files with missing lines Coverage Δ
op-deployer/pkg/deployer/bootstrap/validator.go 33.05% <ø> (ø)
op-validator/pkg/validations/addresses.go 100.00% <ø> (ø)
op-validator/pkg/validations/codes.go 100.00% <ø> (ø)
...ges/contracts-bedrock/src/L1/StandardValidator.sol 91.18% <100.00%> (+0.98%) ⬆️

... and 62 files with indirect coverage changes

Copy link
Copy Markdown
Contributor

@maurelian maurelian left a comment

Choose a reason for hiding this comment

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

This LGTM.

@mslipper mslipper enabled auto-merge February 24, 2025 16:33
@mslipper mslipper disabled auto-merge February 24, 2025 16:41
@mslipper mslipper added this pull request to the merge queue Feb 24, 2025
Merged via the queue into develop with commit b0a9511 Feb 24, 2025
46 checks passed
@mslipper mslipper deleted the bugfix/remove-pv-checks branch February 24, 2025 16:52
Rjected pushed a commit to paradigmxyz/optimism that referenced this pull request Feb 25, 2025
…ions (ethereum-optimism#14498)

* ctb/op-deployer: Simplify superchain config/protocol versions validations

While running the validators against newly-deployed chains, we discovered a bug where the superchain config's implementation cannot be checked unless the same proxy admin is shared between both the OP Chain and the superchain config. This only makes sense for OP Mainnet, so to simplify things I've removed the protocol version/superchain config checks from the standard validator. It will still check that the OP Chain is using the right superchain config, but it won't validate the config itself. We can use other tooling for that.

* checks
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