Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: interop launch sequence #207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

smartcontracts
Copy link
Contributor

Proposes an interop launch sequence. Will require additional feedback from other teams.

Proposes an interop launch sequence. Will require additional
feedback from other teams.
- An `ETHLockbox` contract is deployed for the chain.
- The Upgrade Controller approves the `OptimismPortal` address to use the `ETHLockbox`.
- Chains are upgraded to the new `OptimismPortal` implementation.
- Because `usingSuperRoots` is `false` by default, this will not result in any immediate impact
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add some links? What is usingSuperRoots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Context is here: #205

- A new `DisputeGameFactory` is deployed and the game implementations and init bonds for the
Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set.
- At interop launch time, in a single OPCM action:
- An `ETHLockbox` contract is deployed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to deploy a new lockbox? Can one of them be reused?

Maybe is easier this way so avoid choosing one over the other and making the script simpler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessary to deploy a new lockbox but I think it would be much nicer if all chains had to go through exactly the same upgrade process instead of one chain having a unique upgrade process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, sounds good

Copy link
Contributor

Choose a reason for hiding this comment

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

Remind me, why did we not want to just use a single lockbox for all chains, regardless if they are in the interop set? This would remove the need to deploy a new ETHLockbox and migrate ETH as part of this sequence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO this would be terrifying to manage and unclear how you'd do permissions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically key question is: who gets to upgrade the lockbox? If it's one massive shared lockbox then it's very unclear and we need different lockboxes depending on if you're using optimism governance for security or not, and then things get confusing

Comment on lines +21 to +24
- The Superchain-wide pause is activated.
- TODO: Decide if this is really the correct way to do this, or if there's some other mechanism
that works well enough. We essentially need to stop users from using the old proof system
while the new proof system is spun up.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems pretty excessive and suggests we haven't thought through the migration properly. The pause also doesn't prevent playing dispute games so if there's something that isn't working in the proofs system, bonds will be lost and games may resolve incorrectly requiring blacklisting before we unpause.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe another approach is to set the respected game type to a non-existent one, 404, this way withdrawals are effectively paused and no one can play dispute games

Comment on lines +32 to +34
- A new `AnchorStateRegistry` is deployed with a valid initial anchor state.
- TODO: Decide if the anchor state is set as an authorized input by governance or by some sort
of more automated transition process.
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to repeat this process for every chain. If it's complex we're buying ourselves another year of trying to get all chains updated. We really need to learn from the very high cost of cutting scope to just op-mainnet in fault proofs and design this to be an automated process that just works for all chains.

We can use something like https://github.com/ajsutton/migratoor to make the migration permissionless and trustless. As described, that would require making it possible for super root dispute games to play out before interop activates which is doable but would require implementation changes. But we could use that process to update a new AnchorStateRegistry and not yet have the game type implementation set, preventing creation of games. We then set the new game type only after interop activates. Potentially the migration system could be disabled when the new game type is set.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also worth noting that inevitably not all chains upgrade at the same time, so if we have to activate the superchain wide pause, we're going to wind up doing that quite a few times as we get all chains upgraded.

- TODO: Decide if the anchor state is set as an authorized input by governance or by some sort
of more automated transition process.
- A new `DisputeGameFactory` is deployed and the game implementations and init bonds for the
Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set.
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: We will not be deploying FaultDisputeGame and PermissionedDisputeGame in the migrated system. Game types 0 and 1 are retired and replaced by SuperDisputeGame and SuperPermissionedGame (final names subject to change) at game types 4 and 5.

of more automated transition process.
- A new `DisputeGameFactory` is deployed and the game implementations and init bonds for the
Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set.
- At interop launch time, in a single OPCM action:
Copy link
Contributor

Choose a reason for hiding this comment

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

It's likely not possible to execute an L1 transaction at launch time. We should define if we're doing this before or after activation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah I mean "shortly after activation"

- The Upgrade Controller approves all other lockbox contracts to migrate into the new lockbox.
- The Upgrade Controller triggers the migration function on all other lockbox contracts to
transfer ETH from those lockboxes into the single shared lockbox.
- Interop is "fully" deployed and withdrawals from the interop system can be processed on L1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit picking but we still haven't unpaused.

that works well enough. We essentially need to stop users from using the old proof system
while the new proof system is spun up.
- TODO: Needs more context from protocol & proofs.
- At interop activation time on L2:
Copy link
Contributor

@mds1 mds1 Feb 24, 2025

Choose a reason for hiding this comment

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

Before this activation time, chains will have had to update their node release and begun running op-supervisor, which we should add here. Or maybe this is actually sometime before the below "At interop launch time"?

Comment on lines +35 to +36
- A new `DisputeGameFactory` is deployed and the game implementations and init bonds for the
Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should init bonds be the same 0.08 ETH, or is there any reason to change them for interop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would assume 0.08 ETH

Copy link
Contributor

Choose a reason for hiding this comment

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

No reason to change the initial bonds.

Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set.
- At interop launch time, in a single OPCM action:
- An `ETHLockbox` contract is deployed.
- The Upgrade Controller triggers an `upgrade` function on all `OptimismPortal` contracts that
Copy link
Contributor

Choose a reason for hiding this comment

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

This flow is for all standard chains, or only chains in the interop set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only interop set

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also make sure it's clear what is happening to standard, non-initial-interop-set chains

Comment on lines +48 to +55
- At some time after interop launches, for all chains, in an OPCM action per chain:
- A new `AnchorStateRegistry` is deployed with a valid initial anchor state.
- TODO: Same question as before.
- A new `DisputeGameFactory` is deployed and the game implementations and init bonds for the
Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set.
- The Upgrade Controller for that chain triggers and `upgrade` function on the `OptimismPortal`
contract that updates the references to the `DisputeGameFactory` and `AnchorStateRegistry`
contracts, and sets the `usingSuperRoots` variable to `true`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not part of the prior sequence "At interop launch time, in a single OPCM action"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's unclear to me if the single output root Super Roots will be ready by the time interop launches or if this is something that's coming afterwards cc @ajsutton to confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we should block launching until we know how we are going to deploy to all chains smoothly. We really really don't want to wind up with chains on different configs and unable to upgrade to new hard forks again.

Copy link
Contributor

Choose a reason for hiding this comment

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

And just to clarify, the fault proof system doesn't distinguish between dependency sets with single or multiple chains - so it can handle a single chain dependency set now. The only open questions I know of are around handling the upgrade process on all chains.

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.

5 participants