Skip to content

op-deployer: fix missing Superchain Deployment after prepare - #22082

Open
0xiamflux wants to merge 2 commits into
developfrom
fix/pcd-superchaindeployment
Open

op-deployer: fix missing Superchain Deployment after prepare#22082
0xiamflux wants to merge 2 commits into
developfrom
fix/pcd-superchaindeployment

Conversation

@0xiamflux

@0xiamflux 0xiamflux commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Context

Some consumers read State.SuperchainDeployment from the prepared state.json, but prepare does not set it as it resolves the SuperchainConfigProxy only in the intent, where it feeds the prediction dry-run.

Fix

prepare reads the live Superchain off the fork it already creates, using the same script apply's live init uses, and commits SuperchainDeployment and SuperchainRoles into the state. If SuperchainConfigProxy read from the intent is not the same as the one on-chain, the whole step reverts.

@0xiamflux
0xiamflux marked this pull request as ready for review July 28, 2026 18:17
@0xiamflux
0xiamflux requested review from a team as code owners July 28, 2026 18:17
Comment thread op-deployer/pkg/deployer/prepare.go Outdated
Comment on lines +186 to +197
*intent.SuperchainConfigProxy,
)
if err != nil {
return fmt.Errorf("failed to read superchain deployment: %w", err)
}
if superDeployment.SuperchainConfigProxy != *intent.SuperchainConfigProxy {
return fmt.Errorf(
"superchain config proxy mismatch: read %s, intent %s",
superDeployment.SuperchainConfigProxy,
*intent.SuperchainConfigProxy,
)
}

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.

Claude pointed out to me that PopulateSuperchainState just copies the SuperchainConfigProxy from input to output via ReadSuperChainDeployment.

So this check is just dead code.

There is no longer an opcm.superchainConfig(), so the only possible source is the intent. Given that, I'm not sure this PR is still needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just checked the script used under the hood and the check can be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But can you elaborate a little bit more on the second point?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just for context: some consumers do an address lookup from this SuperchainDeployment object. The rationale for fixing this here and making sure SuperchainDeployment gets populated, is that if we left it unfixed in op-deployer and instead handled it in the consumer, we'd end up with two different lookup paths for no reason other than that the prepare pipeline doesn't set SuperchainDeployment while apply does.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the dead check here.

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.

2 participants