feat(portal): verify dispute game l2ChainId for non-super games#20924
feat(portal): verify dispute game l2ChainId for non-super games#20924stevennevins wants to merge 4 commits into
Conversation
OptimismPortal2.proveWithdrawalTransaction relied on rootClaimByChainId to bind super-game roots to the portal's L2 chain ID. The non-super branch called rootClaim() directly with no chain-ID check, so a misconfigured FaultDisputeGame implementation registered for a different L2 could let a proposer prove a withdrawal against another chain's output root and finalize through this portal. Add a fail-closed check that the game's l2ChainId() matches systemConfig.l2ChainId() before reading rootClaim(). Adds OptimismPortal_UnknownChainId error and exposes l2ChainId() on the base IDisputeGame interface. Bumps semver to 5.6.2. Aligns the existing OptimismPortal2 test fixtures so the FaultDisputeGame implementation is deployed with the same chain ID as SystemConfig.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #20924 +/- ##
==========================================
+ Coverage 76.3% 82.0% +5.7%
==========================================
Files 185 130 -55
Lines 10698 6665 -4033
==========================================
- Hits 8166 5469 -2697
+ Misses 2388 1196 -1192
+ Partials 144 0 -144
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The new test_proveWithdrawalTransaction_wrongChainIdNonSuperGame_reverts only exercises the non-super branch of proveWithdrawalTransaction. On op-mainnet upgrade-fork runs with SUPER_ROOT_GAMES_MIGRATION enabled the respected game type is a super game, so the portal takes the rootClaimByChainId path and never reaches the new chain ID check. The test then sees no revert and fails. Skip when the respected game type is a super game.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
The previous fix skipped the test when respected game type was super, but that defeats the purpose — we want to exercise the non-super chain ID check regardless of which game type happens to be respected in the environment. Mock the game's gameType to CANNON so the portal takes the non-super branch, then assert the chain ID mismatch reverts.
|
An alternative approach would be to (and we did this for the non-super ZKDIsputeGame) require |
The check was originally written this way because FDG that existed before we added the rootClaimByChainId wouldn't have that selector to call. The Zk games don't have that same issue, so I think they need different solutions |
2 participants
Summary
OptimismPortal2.proveWithdrawalTransactionbinds super-game roots to the portal's L2 chain ID viarootClaimByChainId(systemConfig.l2ChainId()), but the non-super branch calledrootClaim()directly with no chain-ID check. A non-super dispute game implementation misconfigured (via OPCM, migration, or owner upgrade) for a different L2 could let a proposer prove a withdrawal against another chain's output root and finalize through this portal.Changes
OptimismPortal_UnknownChainId()whendisputeGameProxy.l2ChainId() != systemConfig.l2ChainId(). Super-game path is untouched.l2ChainId()to the baseIDisputeGameinterface (already implemented by all concrete game types).OptimismPortal2semver 5.6.1 → 5.6.2 and regenerate snapshots.test_proveWithdrawalTransaction_wrongChainIdNonSuperGame_reverts. Align existing portal/invariant test fixtures so the deployedFaultDisputeGameimpl uses the same L2 chain ID asSystemConfigfixes #20883