feat: Easy Game Rotation

[clabby]

Overview

Currently, the OptimismPortal2 contract respects a single dispute game type at a time, as outlined in the specification for the OptimismPortal modifications made as a part of the Stage 1 fault proofs release.

The issue with this is that changing the respected game type issues a large user experience burden to the users of the chain by delaying their withdrawals for a week. This process happens in this codepath, where during withdrawal finalization, the dispute game that the withdrawal was proven against is checked to both be of the current respected game type and created after the most recent time the respected game type was updated.

// The game type of the dispute game must be the respected game type. This was also checked in
// `proveWithdrawalTransaction`, but we check it again in case the respected game type has changed since
// the withdrawal was proven.
if (disputeGameProxy.gameType().raw() != respectedGameType.raw()) revert InvalidGameType();

// The game must have been created after `respectedGameTypeUpdatedAt`. This is to prevent users from creating
// invalid disputes against a deployed game type while the off-chain challenge agents are not watching.
require(
    createdAt >= respectedGameTypeUpdatedAt,
    "OptimismPortal: dispute game created before respected game type was updated"
);

Desired Change

The new behavior here should be such that the Guardian can optionally invalidate, rather than always having to invalidate, the proofs of ongoing user withdrawals.

The behavior should enable a "rollover period," where withdrawals proven against dispute games of the previous game type are allowed, if the Guardian did not specify that old games were to be invalidated. No functionality around the rest of the withdrawal path should change.

[wildmolasses]

hi @clabby, understanding the problem this creates for checkWithdrawal. Spitballing one initial impl idea just to get things started:

when calling setRespectedGameType we could save a previousRespectedGameType to storage, save the last game's respectedGameTypeUpdatedAt as validFrom, and set a validUntil on it.

struct PreviousRespectedGameType {
  GameType gameType;
  uint64 validFrom;
  uint64 validUntil;
}

we could then expand on the conditional branches a bit in checkWithdrawal to accommodate this previousRespectedGameType:

 if (disputeGameProxy.gameType().raw() != respectedGameType.raw()) {
  if(disputeGameProxy.gameType().raw() != previousRespectedGameType.gameType.raw() {
    revert InvalidGameType();
  }
  require(
    createdAt >= previousRespectedGameType.validFrom && block.timestamp <= previousRespectedGameType.validUntil
  );
}
...

if the guardian didn't want the previous respected game type to hang around, they could specify that, and the previousRespectedGameType could be set to 0,0,0.

a few questions to start:

• will we ever want to support multiple previous respectedGameTypes?
• is it ok to let the guardian set their own validUntil timestamp on the PreviousRespectedGameType? (perhaps it could be bounded using DISPUTE_GAME_FINALITY_DELAY_SECONDS and/or PROOF_MATURITY_DELAY_SECONDS)
• is it ok to let the guardian also "set" the lastRespectedGameType (in the case where a bad game type is mistakenly set, and another one is set immediately after that?)

keen to hear from you, would be happy to hear suggestions or considerations to keep in mind.

[ajsutton]

Does it simplify anything if we thought of this as just respecting already proven withdrawals rather than trying to continue respecting the previous game type? e.g

1. Respected game type is 0
2. User A proves against a game of type 0
3. Respected game type is switched to 1
4. User A can still finalise their withdrawal (assuming the game resolved as DefenderWins and all the time delays are completed)
5. But no new withdrawals can be proven against games of type 0

So proveWithdrawal logic I think would be unchanged since it just requires the game type to be the currently respected type, but we'd only update respectedGameTypeUpdatedAt when changing the respected game type if we were choosing to invalidate all withdrawals. And in checkWithdrawal we remove the check that the game type is the current respected game type - it must have been the respected game type when it was proven or proveWithdrawal would have reverted so as long as the game was created after the respectedGameTypeUpdatedAt we are ok to allow the withdrawal.

Someone more familiar with this code should definitely double check that logic, but that's the kind of thing I was hoping we'd be able to do.