From dc319041332bc06a3a2b5e543954103d04495672 Mon Sep 17 00:00:00 2001 From: fakedev9999 Date: Mon, 30 Mar 2026 00:32:49 -0700 Subject: [PATCH 1/5] fix(fault-proof): skip proving for games where gameOver() is true The proposer wastes prover-network gas by generating proofs for games that are already over (expired or already proven by someone else). The on-chain prove() call reverts with GameOver() in both cases. Add gameOver() checks in two places: - should_skip_proving(): before starting proof generation, to avoid wasting compute on games that are already over - prove_game(): after proof generation completes but before submitting the on-chain tx, to catch games that expired during the minutes-to- hours proof generation window Addresses succinctlabs/cloud-ops#75 References celo-org/op-succinct#91 --- fault-proof/src/proposer.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/fault-proof/src/proposer.rs b/fault-proof/src/proposer.rs index ae197ef83..6fbce1157 100644 --- a/fault-proof/src/proposer.rs +++ b/fault-proof/src/proposer.rs @@ -1134,6 +1134,11 @@ where let agg_proof = self.prover.generate_agg_proof(sp1_stdin).await?; + // Final guard: check if the game ended during proof generation (minutes to hours). + if game.gameOver().call().await? { + bail!("Game is over (expired or already proven), aborting proof submission"); + } + let transaction_request = game.prove(agg_proof.bytes().into()).into_transaction_request(); let receipt = self .signer @@ -2165,7 +2170,13 @@ where } } - // Check deadline if provided + // Check on-chain gameOver() — covers both expiry and already-proven cases. + let contract = OPSuccinctFaultDisputeGame::new(game_address, self.l1_provider.clone()); + if contract.gameOver().call().await? { + tracing::info!(?game_address, "Game is over (expired or already proven), skipping"); + return Ok(true); + } + if let Some(deadline) = deadline { let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH)?.as_secs(); From c44dbc3b28714af30383ad94203b306890924b97 Mon Sep 17 00:00:00 2001 From: fakedev9999 Date: Mon, 30 Mar 2026 02:06:19 -0700 Subject: [PATCH 2/5] refactor: order checks cheapest-first, add warn log before bail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move gameOver() RPC call after the local deadline check in should_skip_proving() — avoids unnecessary RPC for games already caught by the cheaper cached-deadline check - Add tracing::warn before bail in prove_game() so operators can see when hours of proof work are discarded - Update doc comment to reflect new check order --- fault-proof/src/proposer.rs | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/fault-proof/src/proposer.rs b/fault-proof/src/proposer.rs index 6fbce1157..51cadcbde 100644 --- a/fault-proof/src/proposer.rs +++ b/fault-proof/src/proposer.rs @@ -1134,8 +1134,11 @@ where let agg_proof = self.prover.generate_agg_proof(sp1_stdin).await?; - // Final guard: check if the game ended during proof generation (minutes to hours). if game.gameOver().call().await? { + tracing::warn!( + ?game_address, + "Game ended during proof generation, aborting submission" + ); bail!("Game is over (expired or already proven), aborting proof submission"); } @@ -2137,22 +2140,14 @@ where Ok(true) } - /// Check if proving should be skipped for any reason. - /// - /// Returns `Ok(true)` if proving should be skipped: - /// - Game not found in cache - /// - Game not owned (vkeys don't match) - /// - Deadline has passed - /// - /// Returns `Ok(false)` if proving should proceed. - /// Logs a warning if the deadline is approaching. + /// Check if proving should be skipped. Checks are ordered cheapest-first: + /// cache lookup → deadline (local) → gameOver() (on-chain RPC). async fn should_skip_proving( &self, game_address: Address, deadline: Option, is_defense: bool, ) -> Result { - // Check ownership - only prove games we own { let state = self.state.read().await; let game = state.games.values().find(|g| g.address == game_address); @@ -2170,13 +2165,6 @@ where } } - // Check on-chain gameOver() — covers both expiry and already-proven cases. - let contract = OPSuccinctFaultDisputeGame::new(game_address, self.l1_provider.clone()); - if contract.gameOver().call().await? { - tracing::info!(?game_address, "Game is over (expired or already proven), skipping"); - return Ok(true); - } - if let Some(deadline) = deadline { let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH)?.as_secs(); @@ -2213,6 +2201,14 @@ where } } + // On-chain check for cases the deadline check can't catch (e.g., already proven by another + // party). + let contract = OPSuccinctFaultDisputeGame::new(game_address, self.l1_provider.clone()); + if contract.gameOver().call().await? { + tracing::info!(?game_address, "Game is over (expired or already proven), skipping"); + return Ok(true); + } + Ok(false) } From 57e98448c3a27022a28bec5406323cc50227c923 Mon Sep 17 00:00:00 2001 From: fakedev9999 Date: Mon, 30 Mar 2026 02:07:18 -0700 Subject: [PATCH 3/5] fix: handle transient RPC failure in prove_game gameOver check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't discard hours of proof work on a transient RPC hiccup. If the gameOver() call fails, proceed with submission — the on-chain prove() will revert if the game is truly over (cheap gas vs. wasted proof). --- fault-proof/src/proposer.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/fault-proof/src/proposer.rs b/fault-proof/src/proposer.rs index 51cadcbde..df9f29eff 100644 --- a/fault-proof/src/proposer.rs +++ b/fault-proof/src/proposer.rs @@ -1134,12 +1134,24 @@ where let agg_proof = self.prover.generate_agg_proof(sp1_stdin).await?; - if game.gameOver().call().await? { - tracing::warn!( - ?game_address, - "Game ended during proof generation, aborting submission" - ); - bail!("Game is over (expired or already proven), aborting proof submission"); + // Best-effort check: don't discard an expensive proof on a transient RPC failure. + // If the game is truly over, the on-chain prove() will revert (cheap gas). + match game.gameOver().call().await { + Ok(true) => { + tracing::warn!( + ?game_address, + "Game ended during proof generation, aborting submission" + ); + bail!("Game is over (expired or already proven), aborting proof submission"); + } + Ok(false) => {} + Err(e) => { + tracing::warn!( + ?game_address, + error = ?e, + "Failed to check gameOver(), proceeding with submission" + ); + } } let transaction_request = game.prove(agg_proof.bytes().into()).into_transaction_request(); From 942b745b8da673494fa138d39cd7fc39322a97f9 Mon Sep 17 00:00:00 2001 From: fakedev9999 Date: Mon, 30 Mar 2026 04:26:50 -0700 Subject: [PATCH 4/5] fix: evict challenged+expired games and their subtrees from cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Games that are Challenged + expired (deadline < now) are guaranteed to resolve as CHALLENGER_WINS. This cascades to all descendants via the contract's resolve() logic (parent CHALLENGER_WINS → child CHALLENGER_WINS). Without eviction, these dead games stay IN_PROGRESS in cache forever (nobody calls resolve), causing the proposer to repeatedly attempt proving → revert → retry. RemoveSubtree is safe because: - Challenged + expired = CHALLENGER_WINS regardless of parent state - All descendants also guaranteed CHALLENGER_WINS (parent cascade) - sync_games reads fresh on-chain claim_data, so if a prove() tx landed, status would be ChallengedAndValidProofProvided (not evicted) --- fault-proof/src/proposer.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fault-proof/src/proposer.rs b/fault-proof/src/proposer.rs index df9f29eff..bf2823b68 100644 --- a/fault-proof/src/proposer.rs +++ b/fault-proof/src/proposer.rs @@ -787,6 +787,13 @@ where match status { GameStatus::IN_PROGRESS => { + // Challenged + expired = guaranteed CHALLENGER_WINS, cascades to + // descendants. + if claim_data.status == ProposalStatus::Challenged && now_ts >= deadline { + actions.push(GameSyncAction::RemoveSubtree(index)); + continue; + } + let game_type = contract.gameType().call().await?; let parent_resolved = is_parent_resolved(parent_index, self.factory.as_ref()).await?; From 4ce238dbeddc81702749d6074979f2ff5eead1f3 Mon Sep 17 00:00:00 2001 From: fakedev9999 Date: Mon, 6 Apr 2026 04:39:22 -0700 Subject: [PATCH 5/5] test(fault-proof): update sync tests for challenged+expired eviction Align test expectations with the new RemoveSubtree behavior for Challenged + expired games: - test_mixed_states_multiple_branches: first sync now evicts games 3+4 immediately (previously required a second sync after on-chain resolution). Added a stability check confirming a second sync after on-chain resolution is a no-op. - test_in_progress_proposal_status_multi_branch: game 1 (Challenged + expired) is now evicted during sync. Assert absence instead of inspecting stale cache entry. Challenged status coverage for should_attempt_to_resolve is already in test_in_progress_games_resolution_marking. --- fault-proof/tests/sync.rs | 55 ++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/fault-proof/tests/sync.rs b/fault-proof/tests/sync.rs index 074f1616f..7d3aba425 100644 --- a/fault-proof/tests/sync.rs +++ b/fault-proof/tests/sync.rs @@ -1105,33 +1105,13 @@ mod proposer_sync { tracing::info!("✓ Game {game_address} resolved"); } - // Step 4: Sync state + // Step 4: Sync state — game 3 is Challenged + expired, so it and its child (game 4) + // are evicted immediately by the RemoveSubtree path. proposer.sync_state().await?; - // Verify: All games are cached let snapshot = proposer.state_snapshot().await; - snapshot.assert_game_len(6); + snapshot.assert_game_len(4); // 0, 1, 2, 5 (games 3, 4 evicted) - // Verify: Canonical head is game 5 (highest L2 block among valid games) - snapshot.assert_canonical_head(Some(5), 6, starting_l2_block); - - // Step 5: Resolve game 3, 4 and 5 - for game_address in game_addresses.iter().skip(3) { - env.resolve_game(*game_address).await?; - tracing::info!("✓ Game {game_address} resolved"); - } - - // Step 6: Sync state - proposer.sync_state().await?; - - // Verify: Games 0, 1, 2, 5 retained; games 3, 4 removed - let snapshot = proposer.state_snapshot().await; - snapshot.assert_game_len(4); // 0, 1, 2, 5 - - // Canonical head should be game 5 (highest block among reachable games) - snapshot.assert_canonical_head(Some(5), 6, starting_l2_block); - - // Verify specific games are present let game_indices: Vec = snapshot.games.iter().map(|(idx, _)| *idx).collect(); assert!(game_indices.contains(&U256::from(0)), "Game 0 should be retained"); assert!(game_indices.contains(&U256::from(1)), "Game 1 should be retained"); @@ -1139,13 +1119,28 @@ mod proposer_sync { assert!(game_indices.contains(&U256::from(5)), "Game 5 should be retained"); assert!( !game_indices.contains(&U256::from(3)), - "Game 3 should be removed (CHALLENGER_WINS)" + "Game 3 should be evicted (Challenged + expired)" ); assert!( !game_indices.contains(&U256::from(4)), - "Game 4 should be removed (child of CHALLENGER_WINS)" + "Game 4 should be evicted (child of Challenged + expired)" ); + snapshot.assert_canonical_head(Some(5), 6, starting_l2_block); + + // Step 5: Resolve remaining on-chain games (3, 4, 5) and re-sync. + // Games 3 and 4 were already evicted; this confirms a second sync is stable. + for game_address in game_addresses.iter().skip(3) { + env.resolve_game(*game_address).await?; + tracing::info!("✓ Game {game_address} resolved"); + } + + proposer.sync_state().await?; + + let snapshot = proposer.state_snapshot().await; + snapshot.assert_game_len(4); // still 0, 1, 2, 5 + snapshot.assert_canonical_head(Some(5), 6, starting_l2_block); + Ok(()) } @@ -1330,18 +1325,20 @@ mod proposer_sync { env.warp_time(MAX_CHALLENGE_DURATION + MAX_PROVE_DURATION + 1).await?; env.resolve_game(game_addresses[0]).await?; - // Sync and inspect per-branch flags. + // Sync — game 1 is Challenged + expired, so it is evicted. Game 2 (proved) is retained. proposer.sync_state().await?; - let game_1 = proposer.get_game(U256::from(1)).await.unwrap(); - assert_eq!(game_1.proposal_status, ProposalStatus::Challenged); - assert!(!game_1.should_attempt_to_resolve, "Challenged game should not auto-resolve"); + assert!( + proposer.get_game(U256::from(1)).await.is_none(), + "Game 1 should be evicted (Challenged + expired)" + ); let game_2 = proposer.get_game(U256::from(2)).await.unwrap(); assert_eq!(game_2.proposal_status, ProposalStatus::UnchallengedAndValidProofProvided); assert!(game_2.should_attempt_to_resolve, "Proof-provided game should attempt to resolve"); let snapshot = proposer.state_snapshot().await; + snapshot.assert_game_len(2); // only root (0) and game 2 remain snapshot.assert_canonical_head(Some(2), 3, starting_l2_block); Ok(())