-
Notifications
You must be signed in to change notification settings - Fork 99
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: Proofs: AnchorStateRegistry, OptimismPortal incident response improvements #472
base: main
Are you sure you want to change the base?
feat: Proofs: AnchorStateRegistry, OptimismPortal incident response improvements #472
Conversation
ba02231
to
42d3d5d
Compare
- Withdrawal transaction's target must not be the OptimismPortal address. | ||
- Withdrawal game's root claim must be equal to the hashed outputRootProof input. | ||
- Must verify that the hash of this withdrawal is stored in the L2toL1MessagePasser contract on L2. | ||
- A withdrawal can only be proven once unless the dispute game it proved against resolves against the favor of the root |
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.
Same proof submitter cannot reprove the same withdrawal unless the dispute game previously used to prove the withdrawal is now an invalid game
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 withdrawal was not finalized"
correct?
794d889
to
af03cb3
Compare
a49c66f
to
eb311f2
Compare
eb311f2
to
dd3b97c
Compare
24c1f27
to
624f4f2
Compare
f47eab4
to
c2b80ea
Compare
ebd1aa5
to
7422c6f
Compare
are corner cases where the resolved game rebukes its platonic, game-theoretic ideal, resolving incorrectly. The anchor | ||
state registry appreciates this and affords games and their dependents probabalistic validity by enforcing a game | ||
finality delay, and adding additional dependencies like blacklisting and game retirement. These concessions improve the | ||
confidence in resolved games, and calcify the assumptions upon which withdrawals and other dependents rest. |
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.
does this section articulate the motivations correctly? is it missing anything?
|
||
#### Mitigations | ||
|
||
- Existing incentives in fault proof system design. |
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.
Very open to suggestions on how to phrase these mitigations!
|
||
- Stakeholder incentives. | ||
|
||
### aASR-001: Incorrectly resolving games will be blacklisted within the dispute game finality delay period |
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 this actually an assumption we're making? If not, what assumptions should we make about blacklisting?
|
||
- Stakeholder incentives / processes. | ||
|
||
### aASR-002: If a larger dispute game bug is found, all games will be retired before the first incorrect game's dispute game finality delay period has passed |
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 this assumption correct?
|
||
#### Impact | ||
|
||
**Severity: Critical** |
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.
just guessing at severity, please make suggestions otherwise
|
||
### `retireAllExistingGames` | ||
|
||
Retires all currently deployed games. |
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.
there's an implication in the scenario here that i'd like to double check on:
- all games are retired
getAnchorGame
returns a game that's retired.- all games will be using the retired anchor game for their root until another game resolves and waits the finality duration
- that means the anchor game won't update for at least
min(game resolution time) + finality duration delay
are we ok with an anchor game being days old?
in the above scenario, if games get retired a second time before another game became valid, the anchor state could be up to 2 * (min(game resolution time) + finality duration delay)
seconds old. when does this become a problem?
- [aASR-003](#aasr-003-the-anchorstateregistry-will-be-correctly-initialized-at-deployment) | ||
- [aSC-001](#asc-001-superchainconfig-correctly-reports-its-guardian-address) | ||
|
||
### iASR-004: The anchor game was created recently, within some bounded time period. |
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.
Ask team what exact number is
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.
Anchor state matters more, anchor game sort of an internal detail
- [aASR-003](#aasr-003-the-anchorstateregistry-will-be-correctly-initialized-at-deployment) | ||
- [aSC-001](#asc-001-superchainconfig-correctly-reports-its-guardian-address) | ||
|
||
### iASR-002: Valid withdrawals can be finalized within some bounded amount of time |
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.
Add condition that invalid withdrawals can never be finalized
- [aASR-003](#aasr-003-the-anchorstateregistry-will-be-correctly-initialized-at-deployment) | ||
- [aFDG-002](#afdg-002-fault-dispute-games-with-correct-claims-resolve-correctly-at-some-regular-rate) | ||
|
||
### iASR-005: The anchor game is a game whose claim is correct. |
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.
Anchor state
|
||
- [iASR-001](#iasr-001-games-that-make-correct-claims-about-l2-state-can-be-distinguished-from-games-that-do-not) | ||
- [aASR-001](#aasr-001-incorrectly-resolving-games-will-be-blacklisted-within-the-dispute-game-finality-delay-period) | ||
- [aASR-002](#aasr-002-if-a-larger-dispute-game-bug-is-found-all-games-will-be-retired-before-the-first-incorrect-games-dispute-game-finality-delay-period-has-passed) |
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.
Merge
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.
done in 11b3b54
|
||
### Maybe valid game | ||
|
||
A **maybe valid game** is a dispute game that correctly resolved in favor of the defender. However, the system concedes |
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.
Games that are still pending can be "maybe valid"
- Game status is not `CHALLENGER_WINS`. | ||
- Game `createdAt` timestamp is less than the **game retirement timestamp**. | ||
|
||
### Finalized game |
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 remove this distinct definition
The dispute game finality delay is the period of time between a dispute game resolving and a dispute game becoming | ||
finalized. It's set via **authorized input**. | ||
|
||
### Valid game |
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 just relist everything
|
||
### Anchor game | ||
|
||
The **anchor game** is the game whose state root is used as the **anchor state**. In the overwhelming majority of cases, |
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.
A game can be marked as the "anchor game" if that game is currently a valid game and corresponds to a claim about an L2 block number greater than the L2 block number of the current anchor game. A game remains the anchor game until replaced by another anchor game. An anchor game that becomes blacklisted is no longer considered a usable anchor game. An anchor game that becomes retired is still considered a usable anchor game.
- [aASR-002](#aasr-002-the-anchorstateregistry-will-be-correctly-initialized-at-deployment) | ||
- [aSC-001](#asc-001-superchainconfig-correctly-reports-its-guardian-address) | ||
|
||
### iASR-002: The anchor state is recent, within some bounded time period |
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 to self: understand how long this period is
|
||
Returns whether the game is a **maybe valid game**. | ||
|
||
### `isGameFinalized` |
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 remove
|
||
## Assumptions | ||
|
||
### aASR-001: AnchorStateRegistry correctly distinguishes maybe valid games |
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 change this to a dependency on the invariant
- Pending audit of `AnchorStateRegistry` | ||
- Integration testing | ||
|
||
### aASR-002: AnchorStateRegistry correctly distinguishes valid games |
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.
Same thing here
|
||
- Existing audit of `SuperchainConfig` | ||
|
||
## Top-Level Invariants |
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.
System Level
- Users can always finalize valid withdrawals within a bounded amount of time
- Users can never finalize invalid withdrawals
- Existing audit of `SuperchainConfig` | ||
|
||
## Top-Level Invariants | ||
|
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 assume that the merkle trie proof has bugs with some small non-trivial percent chance
- We must provide a mechanism by which an invalid merkle trie proof can be rejected
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.
-
A withdrawal transaction must only be finalized if it is valid
-> What does valid mean?
-> Valid means it was truly included in the state of the L2ToL1MessagePasser -
isWithdrawalValid(wd)
-> We rely on the invariant from AnchorStateRegistry that the state is valid
-> We assume that the Merkle Trie library can have bugs
-> Therefore we must provide a mechanism to invalidate bad proofs
^ draft content
Re: ethereum-optimism/optimism#12172 > ethereum-optimism/optimism#12173
Re: ethereum-optimism/optimism#12172 > ethereum-optimism/optimism#12175
This draft PR describes an AnchorStateRegistry and OptimismPortal in the spirit of changes suggested in: