ethereum-optimism / ethereum-optimism/optimism
contracts-bedrock: MIG-SASR-RGT accepts super game types the migration validator never validates
- Dominant language
- Go
- Stars
- 6.5k
- Forks
- 4k
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 134
Description
**Claude:** Filed on behalf of @ajsutton, from review feedback by @wwared on ethereum-optimism/optimism#22471.
## Problem
`OPContractsManagerMigrationValidator.MIG-SASR-RGT` accepts any game type that satisfies `GameTypes.isSuperGame`:
https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrationValidator.sol#L128-L132
`isSuperGame` returns true for `SUPER_CANNON`, `SUPER_PERMISSIONED`, `SUPER_ASTERISC_KONA`, `SUPER_CANNON_KONA` and `ZK_DISPUTE_GAME`, but the migration validator only requires and validates `SUPER_PERMISSIONED` and `SUPER_CANNON_KONA` (`MIG-DGF-10` / `MIG-DGF-20`), and it requires `SUPER_CANNON` to be unregistered (`MIG-DGF-60`).
So a migrated chain can respect a game type that has no registered implementation and still pass migration validation. The respected game type gates withdrawal finalization, so the result is a chain that validates clean while withdrawals cannot finalize.
`SUPER_CANNON` is the clearest case: `MIG-DGF-60` asserts it is *not* registered, yet `MIG-SASR-RGT` accepts it as the respected type.
## Why it is filed separately
ethereum-optimism/optimism#22471 added the equivalent assertion to the standard validator (`ASR-RGT`), which now restricts the respected type to the set the selected branch validates. That PR deliberately did not touch the migration validator: the two validators now disagree, and closing the gap needs a design decision plus its own semver bump and tests.
## The naive fix is not correct
Restricting the check to exactly `SUPER_PERMISSIONED` and `SUPER_CANNON_KONA` would reject a ZK-respecting migrated chain. `OPContractsManagerMigrationValidator.sol` has no `ZK_DISPUTE_GAME` references at all, and nothing in the migration path asserts ZK is absent, so a migrated chain can legitimately register and respect a ZK game. That is the mirror of the bug @wwared caught in `ASR-RGT` during review.
Pick one:
1. Allow ZK under the same two conditions the standard validator uses — the `ZK_DISPUTE_GAME` dev feature is enabled *and* the factory registers an implementation. See `isRespectedGameTypeValidated` in `OPContractsManagerStandardValidator.sol` for the reference implementation.
2. Decide ZK is out of scope post-migration and additionally assert `gameImpls(ZK_DISPUTE_GAME) == 0`, alongside the existing `MIG-DGF-60` treatment of `SUPER_CANNON`.
Option 1 keeps the two validators consistent and is what I would expect to want.
## Work required
- Tighten the `MIG-SASR-RGT` condition per the decision above.
- Bump the `OPContractsManagerMigrationValidator` semver and regenerate `snapshots/semver-lock.json`.
- Add coverage for the `SUPER_CANNON` case. Current coverage at `packages/contracts-bedrock/test/L1/opcm/OPContractsManagerMigrationValidator.t.sol:753` only exercises a non-super type, so the gap is untested today.
## Related
- ethereum-optimism/optimism#22471 — added `ASR-RGT` to the standard validator; review thread that raised this.
- ethereum-optimism/optimism#21529 — ZK dev feature removal, relevant to whichever option is chosen.
Contributor guide
Research direction
Start with MIG-SASR-RGT and MIG-DGF-10, MIG-DGF-20, and MIG-DGF-60 in packages/contracts-bedrock/src/L1/opcm/OPContractsManagerMigrationValidator.sol, then compare isRespectedGameTypeValidated in OPContractsManagerStandardValidator.sol. Review the test around packages/contracts-bedrock/test/L1/opcm/OPContractsManagerMigrationValidator.t.sol:753 and choose the documented ZK behavior. Done includes the semver bump, regenerated snapshots/semver-lock.json, and coverage for SUPER_CANNON.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100