Skip to content

feat: add Super Root specific method to OptimismPortal#14532

Merged
smartcontracts merged 1 commit intofeat/interop-portal-updatesfrom
sc/pwd-super-roots
Mar 3, 2025
Merged

feat: add Super Root specific method to OptimismPortal#14532
smartcontracts merged 1 commit intofeat/interop-portal-updatesfrom
sc/pwd-super-roots

Conversation

@smartcontracts
Copy link
Contributor

Updates the OptimismPortal to include a method that allows users to prove against Super Roots for interop.

@codecov
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.41%. Comparing base (159fb05) to head (540c870).
Report is 2 commits behind head on feat/interop-portal-updates.

❗ There is a different number of reports uploaded between BASE (159fb05) and HEAD (540c870). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (159fb05) HEAD (540c870)
cannon-go-tests-32 1 0
cannon-go-tests-64 1 0
Additional details and impacted files
@@                       Coverage Diff                        @@
##           feat/interop-portal-updates   #14532       +/-   ##
================================================================
- Coverage                        81.54%   45.41%   -36.13%     
================================================================
  Files                              171      981      +810     
  Lines                             9960    84460    +74500     
================================================================
+ Hits                              8122    38359    +30237     
- Misses                            1670    43189    +41519     
- Partials                           168     2912     +2744     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests 94.62% <100.00%> (+0.04%) ⬆️

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

Files with missing lines Coverage Δ
op-chain-ops/interopgen/configs.go 25.00% <ø> (ø)
op-chain-ops/interopgen/deploy.go 72.52% <100.00%> (ø)
op-chain-ops/interopgen/recipe.go 75.00% <100.00%> (ø)
op-chain-ops/solc/types.go 12.50% <ø> (ø)
op-deployer/pkg/deployer/opcm/opchain.go 0.00% <ø> (ø)
op-deployer/pkg/deployer/pipeline/opchain.go 84.21% <100.00%> (ø)
op-deployer/pkg/deployer/standard/standard.go 27.74% <ø> (ø)
op-deployer/pkg/deployer/state/chain_intent.go 82.35% <ø> (ø)
...racts-bedrock/scripts/checks/reinitializer/main.go 74.69% <100.00%> (ø)
...es/contracts-bedrock/src/L1/OPContractsManager.sol 99.20% <100.00%> (ø)
... and 7 more

... and 914 files with indirect coverage changes

@smartcontracts smartcontracts force-pushed the sc/pwd-super-roots branch 4 times, most recently from d8e738b to e376c05 Compare February 26, 2025 15:15
@smartcontracts smartcontracts force-pushed the sc/pwd-super-roots branch 3 times, most recently from eb841eb to cc6ad30 Compare February 26, 2025 16:59
@smartcontracts smartcontracts force-pushed the sc/asr-sot branch 2 times, most recently from f9ab615 to 6f633e1 Compare February 26, 2025 19:35
@smartcontracts smartcontracts force-pushed the sc/asr-sot branch 3 times, most recently from 6db3d67 to 1040c51 Compare February 26, 2025 22:07
@smartcontracts smartcontracts force-pushed the sc/pwd-super-roots branch 3 times, most recently from 1041722 to 1a6c9c3 Compare February 27, 2025 04:19
@maurelian maurelian self-assigned this Feb 27, 2025
Comment on lines 362 to 379
if (_isProxy) {
require(val == 1, "DeployUtils: storage value is not 1 at the given slot and offset");
require(
val >= 1 && val < type(uint8).max, "DeployUtils: storage value is not set at the given slot and offset"
);
} else {
require(val == type(uint8).max, "DeployUtils: storage value is not 0xff at the given slot and offset");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

these two blocks just boil down to require(val>0), which doesn't tell us much in a world with reinitializers.

WDYT about making the reinitializer value a public constant in the bytecode, then we can check that the slotVal matches either that value of the max uint8. ie.

uint public constant initVersion;

function initialize() reinitializer(initVersion) {}
function upgrade() reinitializer(initVersion) {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved with a new dedicated contract for this

@smartcontracts smartcontracts requested review from axelKingsley and maurelian and removed request for a team February 28, 2025 01:15
@smartcontracts smartcontracts force-pushed the feat/interop-portal-updates branch 3 times, most recently from 402ded5 to 159fb05 Compare February 28, 2025 16:28
Copy link
Contributor

@ControlCplusControlV ControlCplusControlV left a comment

Choose a reason for hiding this comment

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

Adding note - reviewed the Solidity and didn't have any comments, but planning to revisit. The overall design lgtm though

Updates the OptimismPortal to include a method that allows users
to prove against Super Roots for interop.
@smartcontracts smartcontracts merged commit 60cc361 into feat/interop-portal-updates Mar 3, 2025
46 checks passed
@smartcontracts smartcontracts deleted the sc/pwd-super-roots branch March 3, 2025 21:37
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.

4 participants