Skip to content

Comments

op-challenger: call closeGame() for games with no bonds to claim#18787

Merged
ajsutton merged 6 commits intodevelopfrom
devin/1768421268-close-game-for-permissioned
Jan 15, 2026
Merged

op-challenger: call closeGame() for games with no bonds to claim#18787
ajsutton merged 6 commits intodevelopfrom
devin/1768421268-close-game-for-permissioned

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Jan 14, 2026

Summary

Fixes an issue where permissioned games don't update the AnchorStateRegistry because the challenger has no bonds to claim. When chains switch from permissioned to permissionless, the anchor state could be too old and can't be proven in time in cannon.

Changes:

  • Add CloseGameTx() method to FaultDisputeGameContract interface and all implementations
  • Older contract versions (080, 0180, 111, 120, 131) return ErrCloseGameNotSupported
  • Add GetBondDistributionMode() and CloseGameTx() to OptimisticZKDisputeGameContract interface
  • Call closeGame() once per game when no claimant has credit
  • Skip closeGame() when running in selective claim resolution mode (--selective-claim-resolution)

Updates since last revision

Addressed PR review comments:

  • Added selective field to Claimer to respect --selective-claim-resolution CLI option
  • Added tests for GetBondDistributionMode() and CloseGameTx() in optimisticzkdisputegame_test.go
  • Added tests for CloseGameTx() in faultdisputegame_test.go, including verification that older versions return ErrCloseGameNotSupported
  • Used faultTypes.NormalDistributionMode constant instead of magic number in test
  • Refactored ClaimBonds to only attempt closeGame once per game (not once per address)
  • Extracted claimBonds helper method that only iterates through addresses (claimer.go:75-84)
  • closeGame call remains in ClaimBonds method (claimer.go:68-70), not in the helper

Review & Testing Checklist for Human

  • Review claimBond return value semantics in claimer.go:88-119: Returns true when the game should NOT be closed (credit exists, credit locked, or game in progress). This is counterintuitive - verify all return paths are correct.
  • Review the claimBonds helper in claimer.go:75-84: Verify it only iterates addresses and returns whether any credit was found.
  • Verify closeGame call location in claimer.go:68-70: Confirm closeGame is called from ClaimBonds, not from the helper method.
  • Verify on-chain contract methods: Confirm the ZK and fault dispute game contracts actually have bondDistributionMode() and closeGame() methods on-chain.
  • Test on a permissioned chain: Deploy to a testnet with permissioned games and verify that closeGame() is called and the AnchorStateRegistry is updated.

Notes

  • The error handling silently ignores ErrSimulationFailed and ErrCloseGameNotSupported - this is intentional to gracefully handle games that aren't ready to close or use older contract versions.
  • Unit tests were added but no integration testing was performed.

Link to Devin run: https://app.devin.ai/sessions/969d461e83b040838c350e63f2e6b0d4
Requested by: Kelvin Fichter (@smartcontracts)

Last update: 2026-01-15 01:12 ET

This fixes an issue where permissioned games don't update the AnchorStateRegistry
because the challenger has no bonds to claim. When chains switch from permissioned
to permissionless, the anchor state could be too old and can't be proven in time.

Changes:
- Add CloseGameTx() method to FaultDisputeGameContract interface and all implementations
- Older contract versions (080, 0180, 111, 131) return ErrCloseGameNotSupported
- Modify Claimer.claimBond() to call closeGame() when:
  - Game is finalized (not in progress)
  - No credit to claim
  - bondDistributionMode is UNDECIDED
- Add tests for the new closeGame() behavior

Co-Authored-By: Kelvin Fichter <kelvinfichter@gmail.com>
@devin-ai-integration devin-ai-integration bot requested review from a team as code owners January 14, 2026 20:18
@devin-ai-integration devin-ai-integration bot requested a review from Inphi January 14, 2026 20:18
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@codecov
Copy link

codecov bot commented Jan 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.63%. Comparing base (b62556d) to head (a9a1dba).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18787      +/-   ##
===========================================
+ Coverage    73.93%   77.63%   +3.69%     
===========================================
  Files          190      135      -55     
  Lines        11318     7288    -4030     
===========================================
- Hits          8368     5658    -2710     
+ Misses        2804     1630    -1174     
+ Partials       146        0     -146     
Flag Coverage Δ
cannon-go-tests-64 ?
contracts-bedrock-tests 77.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 55 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Skip closeGame() when in selective claim resolution mode
- Add GetBondDistributionMode and CloseGameTx to OptimisticZKDisputeGameContract interface
- Add tests for GetBondDistributionMode and CloseGameTx in optimisticdisputegame_test.go
- Add tests for CloseGameTx in faultdisputegame_test.go including older versions returning ErrCloseGameNotSupported
- Pass SelectiveClaimResolution config to Claimer via service.go

Co-Authored-By: Kelvin Fichter <kelvinfichter@gmail.com>
Co-Authored-By: Kelvin Fichter <kelvinfichter@gmail.com>
Refactored ClaimBonds to:
- Create contract once per game instead of once per claimant
- Track if any claimant had credit > 0 for a game
- Only call closeGame once per game if no claimant had credit

This avoids wasting resources checking bond distribution mode for every address.

Co-Authored-By: Kelvin Fichter <kelvinfichter@gmail.com>
devin-ai-integration bot and others added 2 commits January 15, 2026 01:03
Co-Authored-By: Kelvin Fichter <kelvinfichter@gmail.com>
… addresses

Co-Authored-By: Kelvin Fichter <kelvinfichter@gmail.com>
@ajsutton ajsutton added this pull request to the merge queue Jan 15, 2026
Merged via the queue into develop with commit 3b496bc Jan 15, 2026
91 checks passed
@ajsutton ajsutton deleted the devin/1768421268-close-game-for-permissioned branch January 15, 2026 03:31
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.

1 participant