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: Proofs: AnchorStateRegistry, OptimismPortal incident response improvements #472

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

wildmolasses
Copy link

- 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
Copy link
Contributor

@smartcontracts smartcontracts Dec 5, 2024

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

Copy link
Author

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?

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.
Copy link
Author

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.
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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**
Copy link
Author

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.
Copy link
Author

@wildmolasses wildmolasses Dec 12, 2024

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.
Copy link
Contributor

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

Copy link
Contributor

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
Copy link
Contributor

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.
Copy link
Contributor

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge

Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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,
Copy link
Contributor

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
Copy link
Contributor

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`
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

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