-
Notifications
You must be signed in to change notification settings - Fork 36
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
base: main
Are you sure you want to change the base?
Conversation
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 |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sounds good
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
- 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
- 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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"?
- A new `DisputeGameFactory` is deployed and the game implementations and init bonds for the | ||
Super Root versions of the `FaultDisputeGame` and `PermissionedDisputeGame` are set. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only interop set
There was a problem hiding this comment.
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
- 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`. |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Proposes an interop launch sequence. Will require additional feedback from other teams.