fix(challenger): call setAnchorState on resolved DEFENDER_WINS games by leopoldjoy · Pull Request #2372 · base/base

leopoldjoy · 2026-04-24T15:03:08Z

Summary

The AnchorStateRegistry was never being updated by any off-chain component. In the multiproof system (AggregateVerifier), claimCredit() does not call closeGame() — unlike FaultDisputeGame where claimCredit() triggers closeGame() which calls setAnchorState(). This meant the anchor root was never advancing after proposals finalized.

Root Cause

FaultDisputeGame.claimCredit() → calls closeGame() → calls setAnchorState() ✅
AggregateVerifier.claimCredit() → does NOT call closeGame() ❌
Zero references to closeGame, close_game, setAnchorState, or set_anchor_state exist in any .rs file in the repo

Fix

Integrates a best-effort setAnchorState() call into the BondManager lifecycle. After each successful advance_game() tick, if the game is past NeedsResolve and resolved as DEFENDER_WINS, we send a permissionless setAnchorState(game) tx to the ASR contract. The call is self-validating on-chain (checks finalization, airgap delay, DEFENDER_WINS status, and staleness) so premature calls revert harmlessly and are retried on the next tick.

Changes

anchor_state_registry.rs — Add setAnchorState(address) to IAnchorStateRegistry sol! bindings and encode_set_anchor_state_calldata() helper
lib.rs (contracts) — Re-export encode_set_anchor_state_calldata
bond.rs — Add anchor_state_registry_address to BondManager, anchor_update_complete flag to TrackedGame, and try_anchor_update() method called after each advance_game() success
cli.rs — Add --anchor-state-registry-address / BASE_CHALLENGER_ANCHOR_STATE_REGISTRY_ADDRESS CLI arg
config.rs — Wire through anchor_state_registry_address
service.rs — Pass config to BondManager::new()
metrics.rs — Add anchor_update_tx_outcome_total counter (success/error/skipped)
driver.rs (tests) — Update default_bond_manager() for new parameter

Testing

6 new unit tests covering all anchor update code paths:

anchor_update_skipped_when_no_asr_configured
anchor_update_skipped_for_needs_resolve_phase
anchor_update_skipped_for_challenger_wins
anchor_update_sends_tx_for_defender_wins
anchor_update_retries_on_failure
anchor_update_not_retried_after_success

All 39 tests pass, clippy clean with -D warnings.

Future Work

Consider adding closeGame() to AggregateVerifier.claimCredit() in a contract upgrade for defense-in-depth (mirrors the FaultDisputeGame pattern).

The AnchorStateRegistry was never being updated by any off-chain component. In the multiproof system (AggregateVerifier), claimCredit() does not call closeGame() — unlike FaultDisputeGame where claimCredit() triggers closeGame() which calls setAnchorState(). This meant the anchor root was never advancing after proposals finalized. This fix integrates a best-effort setAnchorState() call into the BondManager lifecycle. After each successful advance_game() tick, if the game is past NeedsResolve and resolved as DEFENDER_WINS, we send a permissionless setAnchorState(game) tx to the ASR contract. The call is self-validating on-chain (checks finalization, airgap delay, DEFENDER_WINS status, and staleness) so premature calls revert harmlessly and are retried on the next tick. Changes: - Add setAnchorState(address) to IAnchorStateRegistry sol! bindings and encode_set_anchor_state_calldata() helper - Add anchor_state_registry_address config to BondManager, CLI, and config - Add try_anchor_update() method called after each advance_game() success - Add anchor_update_tx_outcome_total metric (success/error/skipped) - Add 6 unit tests covering all anchor update code paths

cb-heimdall · 2026-04-24T15:03:11Z

🟡 Heimdall Review Status

Requirement Status More Info

Reviews

🟡 0/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 0

Global minimum 0

Max 1

1

1 if commit is unverified 0

Sum 1

vercel · 2026-04-24T15:03:14Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
base	Ignored	Preview	Apr 24, 2026 6:33pm

Address review feedback: - Cache the on-chain game status on TrackedGame after the first successful RPC read, avoiding redundant status() calls on every poll tick during the airgap delay period (status is immutable after resolution). - Warn at startup when anchor-state-registry-address is configured but bond-claim-addresses is empty, since the anchor updates require the bond manager to function.

… try_resolve Address review feedback: - Call try_anchor_update on the Ok(Some(_)) path before the game is removed from tracking. This gives a last-chance anchor update attempt when the bond lifecycle completes, preventing permanent loss of the update if the airgap delay is longer than the DelayedWETH delay. - Cache the game status in try_resolve when the game is already resolved (status immutable), so try_anchor_update on the same tick can skip the redundant status() RPC round-trip.

Relax the NeedsResolve phase guard in try_anchor_update so that games with a cached status (known to be resolved) can still attempt the anchor update even when the phase hasn't advanced past NeedsResolve. This fixes an edge case where a DEFENDER_WINS game is marked NotClaimable (bond recipient not in our claim set after resolution) and removed before the anchor update is attempted — the phase guard was blocking the last-chance try_anchor_update call because set_phase was never reached.

Cache the game status after we successfully submit a resolve() tx, not just on the 'already resolved' path. Without this, self-resolved DEFENDER_WINS games that are NotClaimable would miss their anchor update because cached_status was None and the NeedsResolve phase guard would trigger.

…f config Remove the --anchor-state-registry-address CLI parameter and all associated config plumbing. The AnchorStateRegistry address is now read from each game contract via anchorStateRegistry() and cached per-game on TrackedGame, eliminating the risk of misconfiguration.

github-actions · 2026-04-24T18:25:46Z

Review Summary

The implementation is well-structured and follows established crate patterns. The setAnchorState() integration is cleanly layered into the existing BondManager lifecycle with good status caching to avoid redundant RPC calls, proper metrics, and solid test coverage for the new try_anchor_update paths.

Outstanding concern: anchor update lost on game removal

The most important open issue (flagged in prior inline comments): when the on-chain airgap delay is longer than the DelayedWETH withdrawal delay, the bond lifecycle completes and the game is removed from self.tracked before setAnchorState becomes callable. The "last-chance" try_anchor_update at line 600 will revert, and the game is then permanently removed at line 617 with no future retries.

This is a correctness issue for the stated goal of advancing the anchor root. Consider either:

Keeping DEFENDER_WINS games with anchor_update_complete == false in tracked after the bond lifecycle completes (with a TTL/max-retry to bound growth), or
Documenting this as intentionally best-effort, relying on other actors to call the permissionless setAnchorState.

Notes on prior review comments

Several earlier inline comments (status not cached on success resolve path, NotClaimable games missing anchor update, if let Ok swallowing errors) appear to have been addressed in the current revision — the code now caches status on both the "already resolved" and "success resolve" paths (lines 702-737), and uses match with debug! logging on the error branch.

Minor observation

The PR description references CLI/config/service changes (--anchor-state-registry-address, ChallengerConfig wiring) that are not present in the diff. The actual implementation reads the ASR address dynamically from the game contract, which is a cleaner approach. The description should be updated to match the implementation.