feat: Proofs: AnchorStateRegistry, OptimismPortal incident response improvements #472
Conversation
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
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.
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.
"and withdrawal was not finalized"
correct?
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
794d889 to
af03cb3
Compare
|
|
||
| ### `getLatestValidGame` | ||
|
|
||
| Returns **latest valid game**, or reverts if there is no **latest valid game**. |
There was a problem hiding this comment.
Returns the anchor game if the anchor game is a valid game. Reverts if the anchor game is an invalid game
| - Calling game must only register itself (and not some other game). | ||
| - TODO: determine any invariants around registry ordering. | ||
|
|
||
| ### `tryUpdateLatestValidGame` |
There was a problem hiding this comment.
| ### `tryUpdateLatestValidGame` | |
| ### `tryUpdateAnchorGame` |
|
|
||
| Proves a withdrawal transaction. | ||
|
|
||
| - Withdrawal game must not be a **maybe valid game**. |
There was a problem hiding this comment.
| - Withdrawal game must not be a **maybe valid game**. | |
| - Withdrawal game must be a **maybe valid game**. |
|
|
||
| ## Top-Level Invariants | ||
|
|
||
| - The contract will only assert **valid games** are valid. |
There was a problem hiding this comment.
Can be expanded a bit
- Must always correctly represent the state of a given game (valid, invalid, maybe valid)
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
a49c66f to
eb311f2
Compare
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
eb311f2 to
dd3b97c
Compare
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
24c1f27 to
624f4f2
Compare
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
f47eab4 to
c2b80ea
Compare
wildmolasses
force-pushed
the
11-08-proofs-iri-asr
branch
from
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.
does this section articulate the motivations correctly? is it missing anything?
|
|
||
| > See [Fault Dispute Game](fault-dispute-game.md) | ||
|
|
||
| A dispute game is a contract that resolves an L2 state claim. |
There was a problem hiding this comment.
If something is (or should) be defined elsewhere, does it still make sense to define it here w/ link?
| - Game was created by the dispute game factory. | ||
| - Game is not **blacklisted**. | ||
| - Game was created while it was the respected game type. | ||
| - Game status is not `CHALLENGER_WINS`. |
There was a problem hiding this comment.
Notice I removed the "validity timestamp" condition here -- that created somewhat frustrating clashes when trying to define "anchor game" and "recent valid game," so I refactored the timestamp stuff out to a "retired game" concept where:
- an anchor game must be a valid game (must not be retired when set but can get retired and will still be valid as its claim and anchor state are still correct)
- the "recent valid game" is the anchor game but if it's been retired it will throw
does that feel any cleaner?
However, the interface would have to change: instead of isGameValid, we'd use isGameValidAndNotRetired which seems a little clunkier. Any thoughts?
|
|
||
| - Existing audit on `FaultDisputeGame`. Note: Existing audit does not yet cover the second property above (that a game | ||
| correctly reports whether its game type was the respected game type when created). | ||
| - Integration testing. |
There was a problem hiding this comment.
Is it appropriate to call integration testing a mitigation?
| correctly reports whether its game type was the respected game type when created). | ||
| - Integration testing. | ||
|
|
||
| ### aFDG-002: Fault dispute games with correct claims resolve correctly at some regular rate |
There was a problem hiding this comment.
Keen on your perspective here, is this a needed dependency for tryUpdateAnchorGame?
|
|
||
| #### Mitigations | ||
|
|
||
| - Existing incentives in fault proof system design. |
There was a problem hiding this comment.
Very open to suggestions on how to phrase these mitigations!
| - Existing audit on the `DisputeGameFactory`. | ||
| - Integration testing. | ||
|
|
||
| ### aDGF-002: Games created by the DisputeGameFactory will be monitored |
There was a problem hiding this comment.
This assumption factors heavily into the 2 assumptions that follow it. Is this correctly articulated?
|
|
||
| - Stakeholder incentives. | ||
|
|
||
| ### aASR-001: Incorrectly resolving games will be blacklisted within the dispute game finality delay period |
There was a problem hiding this comment.
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.
Is this assumption correct?
|
|
||
| #### Impact | ||
|
|
||
| **Severity: Critical** |
There was a problem hiding this comment.
just guessing at severity, please make suggestions otherwise
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: