Skip to content

Commit a8601e9

Browse files
committed
Fix finalized game close handling
1 parent 480d681 commit a8601e9

9 files changed

Lines changed: 528 additions & 65 deletions

File tree

fault-proof/src/backup.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ mod tests {
143143
proposal_status: ProposalStatus::Unchallenged,
144144
deadline: 0,
145145
should_attempt_to_resolve: false,
146+
should_attempt_to_close_game: false,
146147
should_attempt_to_claim_bond: false,
147148
aggregation_vkey: B256::ZERO,
148149
range_vkey_commitment: B256::ZERO,

fault-proof/src/challenger.rs

Lines changed: 135 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{
1616
contract::{
1717
AnchorStateRegistry::AnchorStateRegistryInstance,
1818
DisputeGameFactory::DisputeGameFactoryInstance, GameStatus, OPSuccinctFaultDisputeGame,
19-
ProposalStatus,
19+
ProposalStatus, BOND_DISTRIBUTION_MODE_UNDECIDED, MAX_CLOSE_GAME_FAILURES,
2020
},
2121
is_parent_challenger_wins, is_parent_resolved,
2222
prometheus::ChallengerGauge,
@@ -36,6 +36,7 @@ where
3636
anchor_state_registry: AnchorStateRegistryInstance<P>,
3737
factory: DisputeGameFactoryInstance<P>,
3838
challenger_bond: OnceLock<U256>,
39+
close_game_failures: Arc<Mutex<HashMap<Address, u8>>>,
3940
state: Arc<Mutex<ChallengerState>>,
4041
}
4142

@@ -61,6 +62,7 @@ where
6162
anchor_state_registry,
6263
factory,
6364
challenger_bond: OnceLock::new(),
65+
close_game_failures: Arc::new(Mutex::new(HashMap::new())),
6466
state: Arc::new(Mutex::new(ChallengerState {
6567
cursor: U256::ZERO,
6668
games: HashMap::new(),
@@ -105,6 +107,10 @@ where
105107
tracing::warn!("Failed to handle game resolution: {:?}", e);
106108
}
107109

110+
if let Err(e) = self.handle_game_closing().await {
111+
tracing::warn!("Failed to handle game closing: {:?}", e);
112+
}
113+
108114
if let Err(e) = self.handle_bond_claiming().await {
109115
tracing::warn!("Failed to handle bond claiming: {:?}", e);
110116
}
@@ -180,9 +186,10 @@ where
180186
/// wins.
181187
/// - Games are marked for resolution if the parent is resolved, the game is over, and it's
182188
/// own game.
183-
/// - Games are marked for bond claim if they are finalized and there is credit to claim.
184-
/// - Games are evicted once finalized with no remaining credit or whenever resolves as
185-
/// defender wins.
189+
/// - Finalized UNDECIDED challenger-won games are marked for close.
190+
/// - Closed challenger-won games are marked for bond claim if there is credit to claim.
191+
/// - Closed challenger-won games with no remaining credit are evicted, as are games that
192+
/// resolve as defender wins.
186193
pub async fn sync_state(&self) -> Result<()> {
187194
// 1. Load new games.
188195
let mut next_index = {
@@ -228,6 +235,7 @@ where
228235
proposal_status: ProposalStatus,
229236
should_attempt_to_challenge: bool,
230237
should_attempt_to_resolve: bool,
238+
should_attempt_to_close_game: bool,
231239
should_attempt_to_claim_bond: bool,
232240
},
233241
Remove(U256),
@@ -287,15 +295,45 @@ where
287295
proposal_status,
288296
should_attempt_to_challenge,
289297
should_attempt_to_resolve,
298+
should_attempt_to_close_game: false,
290299
should_attempt_to_claim_bond: false,
291300
});
292301
}
293302
GameStatus::CHALLENGER_WINS => {
294303
let is_finalized =
295304
self.anchor_state_registry.isGameFinalized(game.address).call().await?;
296-
let credit = contract.credit(signer_address).call().await?;
297305

298-
if is_finalized && credit == U256::ZERO {
306+
if !is_finalized {
307+
actions.push(GameSyncAction::Update {
308+
index: game.index,
309+
status,
310+
proposal_status,
311+
should_attempt_to_challenge: false,
312+
should_attempt_to_resolve: false,
313+
should_attempt_to_close_game: false,
314+
should_attempt_to_claim_bond: false,
315+
});
316+
continue;
317+
}
318+
319+
let bond_distribution_mode = contract.bondDistributionMode().call().await?;
320+
if bond_distribution_mode == BOND_DISTRIBUTION_MODE_UNDECIDED {
321+
actions.push(GameSyncAction::Update {
322+
index: game.index,
323+
status,
324+
proposal_status,
325+
should_attempt_to_challenge: false,
326+
should_attempt_to_resolve: false,
327+
should_attempt_to_close_game: true,
328+
should_attempt_to_claim_bond: false,
329+
});
330+
continue;
331+
}
332+
333+
self.close_game_failures.lock().await.remove(&game.address);
334+
335+
let credit = contract.credit(signer_address).call().await?;
336+
if credit == U256::ZERO {
299337
actions.push(GameSyncAction::Remove(game.index));
300338
} else {
301339
actions.push(GameSyncAction::Update {
@@ -304,7 +342,8 @@ where
304342
proposal_status,
305343
should_attempt_to_challenge: false,
306344
should_attempt_to_resolve: false,
307-
should_attempt_to_claim_bond: is_finalized && credit > U256::ZERO,
345+
should_attempt_to_close_game: false,
346+
should_attempt_to_claim_bond: true,
308347
});
309348
}
310349
}
@@ -324,13 +363,15 @@ where
324363
proposal_status,
325364
should_attempt_to_challenge,
326365
should_attempt_to_resolve,
366+
should_attempt_to_close_game,
327367
should_attempt_to_claim_bond,
328368
} => {
329369
if let Some(game) = state.games.get_mut(&index) {
330370
game.status = status;
331371
game.proposal_status = proposal_status;
332372
game.should_attempt_to_challenge = should_attempt_to_challenge;
333373
game.should_attempt_to_resolve = should_attempt_to_resolve;
374+
game.should_attempt_to_close_game = should_attempt_to_close_game;
334375
game.should_attempt_to_claim_bond = should_attempt_to_claim_bond;
335376
}
336377
}
@@ -385,6 +426,7 @@ where
385426
proposal_status: claim_data.status,
386427
should_attempt_to_challenge: false,
387428
should_attempt_to_resolve: false,
429+
should_attempt_to_close_game: false,
388430
should_attempt_to_claim_bond: false,
389431
},
390432
);
@@ -598,6 +640,91 @@ where
598640
Ok(())
599641
}
600642

643+
/// Closes finalized challenger-won games before any bond-claim or eviction decision.
644+
#[tracing::instrument(skip(self), level = "info", name = "[[Closing Challenger Games]]")]
645+
async fn handle_game_closing(&self) -> Result<()> {
646+
let candidates = {
647+
let state = self.state.lock().await;
648+
state
649+
.games
650+
.values()
651+
.filter(|game| game.should_attempt_to_close_game)
652+
.cloned()
653+
.collect::<Vec<_>>()
654+
};
655+
656+
for game in candidates {
657+
let failure_count =
658+
self.close_game_failures.lock().await.get(&game.address).copied().unwrap_or(0);
659+
if failure_count >= MAX_CLOSE_GAME_FAILURES {
660+
tracing::error!(
661+
game_index = %game.index,
662+
game_address = ?game.address,
663+
failures = failure_count,
664+
"Skipping game close after repeated failures; restart resets this in-memory guard"
665+
);
666+
continue;
667+
}
668+
669+
if let Err(error) = self.submit_close_game_transaction(&game).await {
670+
let mut failures = self.close_game_failures.lock().await;
671+
let count = failures.entry(game.address).or_default();
672+
*count = count.saturating_add(1);
673+
ChallengerGauge::GameCloseError.increment(1.0);
674+
675+
if error.is_revert() {
676+
tracing::error!(
677+
game_index = %game.index,
678+
game_address = ?game.address,
679+
failures = *count,
680+
?error,
681+
"Close game tx included but reverted on-chain"
682+
);
683+
} else {
684+
tracing::warn!(
685+
game_index = %game.index,
686+
game_address = ?game.address,
687+
failures = *count,
688+
?error,
689+
"Close game tx unconfirmed (may be on-chain), will verify next cycle"
690+
);
691+
}
692+
continue;
693+
}
694+
695+
self.close_game_failures.lock().await.remove(&game.address);
696+
}
697+
698+
Ok(())
699+
}
700+
701+
async fn submit_close_game_transaction(&self, game: &Game) -> Result<()> {
702+
let contract = OPSuccinctFaultDisputeGame::new(game.address, self.l1_provider.clone());
703+
let transaction_request = contract.closeGame().gas(200_000).into_transaction_request();
704+
let receipt = self
705+
.signer
706+
.send_transaction_request_with_timeout(
707+
self.config.l1_rpc.clone(),
708+
transaction_request,
709+
self.config.tx_confirmation_timeout,
710+
)
711+
.await?;
712+
713+
if !receipt.status() {
714+
bail!("{TX_REVERTED_PREFIX} {receipt:?}");
715+
}
716+
717+
tracing::info!(
718+
game_index = %game.index,
719+
game_address = ?game.address,
720+
l2_block_end = %game.l2_block_number,
721+
tx_hash = ?receipt.transaction_hash,
722+
"Game closed successfully"
723+
);
724+
725+
Ok(())
726+
}
727+
601728
/// Claims bonds from games flagged for claiming.
602729
#[tracing::instrument(skip(self), level = "info", name = "[[Claiming Challenger Bonds]]")]
603730
pub async fn handle_bond_claiming(&self) -> Result<()> {
@@ -702,6 +829,7 @@ pub struct Game {
702829
pub proposal_status: ProposalStatus,
703830
pub should_attempt_to_challenge: bool,
704831
pub should_attempt_to_resolve: bool,
832+
pub should_attempt_to_close_game: bool,
705833
pub should_attempt_to_claim_bond: bool,
706834
}
707835

fault-proof/src/contract.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
use alloy_sol_macro::sol;
22
use serde::{Deserialize, Serialize};
33

4+
/// `BondDistributionMode.UNDECIDED` in the Optimism dispute-game contracts.
5+
pub const BOND_DISTRIBUTION_MODE_UNDECIDED: u8 = 0;
6+
7+
/// Maximum consecutive close-game failures before automatic retries pause until restart.
8+
pub const MAX_CLOSE_GAME_FAILURES: u8 = 5;
9+
410
sol! {
511
type GameType is uint32;
612
type Claim is bytes32;
@@ -124,6 +130,12 @@ sol! {
124130
/// @notice Claim the credit belonging to the recipient address.
125131
function claimCredit(address _recipient) external;
126132

133+
/// @notice Closes out the game and determines the bond distribution mode.
134+
function closeGame() external;
135+
136+
/// @notice Returns the bond distribution mode.
137+
function bondDistributionMode() external view returns (uint8);
138+
127139
/// @notice Returns the credit balance of a given recipient.
128140
function credit(address _recipient) external view returns (uint256 credit_);
129141
}

fault-proof/src/prometheus.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ pub enum ProposerGauge {
8787
message = "Total number of game resolution errors encountered by the proposer"
8888
)]
8989
GameResolutionError,
90+
#[strum(
91+
serialize = "op_succinct_fp_game_close_error",
92+
message = "Total number of game close errors encountered by the proposer"
93+
)]
94+
GameCloseError,
9095
#[strum(
9196
serialize = "op_succinct_fp_bond_claiming_error",
9297
message = "Total number of bond claiming errors encountered by the proposer"
@@ -187,6 +192,11 @@ pub enum ChallengerGauge {
187192
message = "Total number of game resolution errors encountered by the challenger"
188193
)]
189194
GameResolutionError,
195+
#[strum(
196+
serialize = "op_succinct_fp_challenger_game_close_error",
197+
message = "Total number of game close errors encountered by the challenger"
198+
)]
199+
GameCloseError,
190200
#[strum(
191201
serialize = "op_succinct_fp_challenger_bond_claiming_error",
192202
message = "Total number of bond claiming errors encountered by the challenger"

0 commit comments

Comments
 (0)