Skip to content

Commit 6b64339

Browse files
fakedev9999seolaohclaude
committed
fix(fault-proof): robustness follow-ups to #865 + cost-estimator parity (#901)
* fix(fault-proof): warn-log L1 head regression in sync_state Distinguish the two skip cases: log at WARN when confirmed_number moves backwards (load-balanced RPC backend regression or deep L1 reorg) so operators can detect unhealthy backends, and keep DEBUG for the normal equal case where L1 simply hasn't ticked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(fault-proof): reset creation guard when tracked game is pruned When sync_games prunes future games (above pinned_latest_index) due to abnormal cache states like backup restore into a shorter chain or deep L1 reorg, the duplicate-creation guard could point at a game that no longer exists on chain. Without resetting, should_create_game blocks indefinitely because canonical_head_l2_block cannot advance through an orphaned game. Reset is gated on the guarded address being among the entries this prune actually removes (evaluated before the removal loop). Checking "absent from post-prune cache" would over-clear in the case where the just-created game has not yet been added to the cache and an unrelated prune fires, allowing should_create_game to re-submit a duplicate at the same L2 block before the cache catches up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(fault-proof): pre-flight on-chain status check before prove/resolve/claim With sync_l1_confirmations > 0, the pinned cache lags behind the chain tip by sync_l1_confirmations × block_time, so a recently confirmed prove(), resolve(), or claimCredit() tx may not yet be reflected in should_attempt_* flags. Without a pre-flight check, the proposer would re-submit duplicate transactions that revert on chain — wasting gas for resolve/claim, and re-running expensive proof generation for prove. Each path now does one eth_call at `latest` before submission: - resolve_games: skip if GameStatus != IN_PROGRESS - claim_bonds: skip if credit(signer) == 0 - should_skip_proving: skip if ProposalStatus is *ValidProofProvided or Resolved (single check covers both already-proven and timeout default-loss cases since Resolved is set whenever GameStatus moves out of IN_PROGRESS) On RPC failure the check logs a warn and proceeds, so transient backend issues don't block legitimate work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(scripts): add --no-safe-head-split to cost-estimator When SafeDB is active, cost-estimator splits the requested range at every span batch boundary via split_range_based_on_safe_heads, producing one zkVM execution per span batch regardless of --batch-size. That mirrors a hypothetical "split each proposal at span batch boundaries" workload, not what the proposer actually does (RANGE_SPLIT_COUNT-driven arithmetic split, default 1 = single execution per proposal interval). The new --no-safe-head-split flag forces split_range_basic so the range is partitioned solely by --batch-size, giving a closer estimate of the per-segment cost the proposer sees on the prover network. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: add PR 894 follow-up parity Scope --no-safe-head-split to cost-estimator only (out of shared HostExecutorArgs), and add challenger-side resolve/claim latest-state pre-flight parity to the proposer changes from #894. --------- Co-authored-by: seolaoh <osa8361@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 106e2ea commit 6b64339

3 files changed

Lines changed: 197 additions & 7 deletions

File tree

‎fault-proof/src/challenger.rs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,32 @@ where
545545
};
546546

547547
for game in candidates {
548+
// Pre-flight on-chain status check at `latest`. The cached `should_attempt_to_resolve`
549+
// flag is captured at sync time and can be stale by submission — between sync and
550+
// this loop, another actor's `resolve()` may have landed (or this loop already
551+
// resolved an earlier candidate that affected this one). Re-checking at `latest`
552+
// avoids submitting a resolution that would only revert on chain.
553+
let contract = OPSuccinctFaultDisputeGame::new(game.address, self.l1_provider.clone());
554+
match contract.status().call().await {
555+
Ok(status) if status != GameStatus::IN_PROGRESS => {
556+
tracing::info!(
557+
game_index = %game.index,
558+
game_address = ?game.address,
559+
?status,
560+
"Skipping resolve: game already resolved on chain"
561+
);
562+
continue;
563+
}
564+
Err(e) => {
565+
tracing::warn!(
566+
game_address = ?game.address,
567+
error = ?e,
568+
"Pre-flight status check failed, proceeding with resolve"
569+
);
570+
}
571+
_ => {}
572+
}
573+
548574
if let Err(error) = self.submit_resolution_transaction(&game).await {
549575
if error.is_revert() {
550576
tracing::error!(
@@ -611,7 +637,33 @@ where
611637
.collect::<Vec<_>>()
612638
};
613639

640+
let signer_address = self.signer.address();
614641
for game in candidates {
642+
// Pre-flight on-chain credit check at `latest`. The cached
643+
// `should_attempt_to_claim_bond` flag is captured at sync time and can be stale by
644+
// submission — a recently confirmed `claimCredit()` (e.g., from a prior cycle or
645+
// another actor) is already reflected at `latest`. Re-checking avoids submitting a
646+
// claim that would only revert on chain.
647+
let contract = OPSuccinctFaultDisputeGame::new(game.address, self.l1_provider.clone());
648+
match contract.credit(signer_address).call().await {
649+
Ok(credit) if credit == U256::ZERO => {
650+
tracing::info!(
651+
game_index = %game.index,
652+
game_address = ?game.address,
653+
"Skipping claim: bond already claimed on chain"
654+
);
655+
continue;
656+
}
657+
Err(e) => {
658+
tracing::warn!(
659+
game_address = ?game.address,
660+
error = ?e,
661+
"Pre-flight credit check failed, proceeding with claim"
662+
);
663+
}
664+
_ => {}
665+
}
666+
615667
if let Err(error) = self.submit_bond_claim_transaction(&game).await {
616668
if error.is_revert() {
617669
tracing::error!(

‎fault-proof/src/proposer.rs‎

Lines changed: 125 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -654,13 +654,26 @@ where
654654
latest_block.header.number.saturating_sub(self.config.sync_l1_confirmations);
655655

656656
// If L1 hasn't advanced past the last synced block, all on-chain state is identical.
657+
//
658+
// `confirmed_number < prev` indicates backend regression from a load-balanced RPC, or a
659+
// deep L1 reorg past `sync_l1_confirmations`. This case should be logged at WARN so
660+
// operators can detect unhealthy backends or L1 reorg; the equal case stays at DEBUG since
661+
// it's the normal "L1 hasn't ticked" path.
657662
let prev = self.last_synced_l1_block.load(Ordering::Relaxed);
658663
if confirmed_number > 0 && confirmed_number <= prev {
659-
tracing::debug!(
660-
confirmed_number,
661-
last_synced = prev,
662-
"L1 head unchanged, skipping sync"
663-
);
664+
if confirmed_number < prev {
665+
tracing::warn!(
666+
confirmed_number,
667+
last_synced = prev,
668+
"L1 confirmed head moved backwards (backend regression or deep reorg), skipping sync"
669+
);
670+
} else {
671+
tracing::debug!(
672+
confirmed_number,
673+
last_synced = prev,
674+
"L1 head unchanged, skipping sync"
675+
);
676+
}
664677
return Ok(());
665678
}
666679

@@ -734,6 +747,20 @@ where
734747
.copied()
735748
.collect();
736749
if !future_games.is_empty() {
750+
// Determine if the duplicate-creation guard's tracked game is among the
751+
// entries this prune is about to remove. Must be evaluated BEFORE the
752+
// removal loop while state.games still holds them. Checking "absent from
753+
// post-prune cache" instead would over-clear the guard when the just-
754+
// created game has not yet been added to the cache (e.g., right after
755+
// creation, or after a backup restore that prunes unrelated entries),
756+
// allowing should_create_game to re-submit a duplicate at the same L2
757+
// block before the cache catches up.
758+
let guarded_addr = *self.last_created_game_address.lock().await;
759+
let guard_in_pruned = guarded_addr != Address::ZERO &&
760+
future_games.iter().any(|idx| {
761+
state.games.get(idx).is_some_and(|g| g.address == guarded_addr)
762+
});
763+
737764
for idx in &future_games {
738765
state.games.remove(idx);
739766
}
@@ -747,6 +774,14 @@ where
747774
if should_clear_anchor {
748775
state.anchor_game = None;
749776
}
777+
if guard_in_pruned {
778+
self.last_created_game_l2_block.store(0, Ordering::Relaxed);
779+
*self.last_created_game_address.lock().await = Address::ZERO;
780+
tracing::warn!(
781+
?guarded_addr,
782+
"Reset creation guard: tracked game was among pruned entries"
783+
);
784+
}
750785
}
751786
}
752787

@@ -1378,6 +1413,31 @@ where
13781413
};
13791414

13801415
for game in candidates {
1416+
// Pre-flight on-chain status check at `latest`. The cached `should_attempt_to_resolve`
1417+
// is derived from the pinned (lagged) snapshot, so a recently confirmed `resolve()` tx
1418+
// may not yet be reflected. Querying at `latest` avoids re-submitting a resolution
1419+
// that would only revert on chain.
1420+
let contract = OPSuccinctFaultDisputeGame::new(game.address, self.l1_provider.clone());
1421+
match contract.status().call().await {
1422+
Ok(status) if status != GameStatus::IN_PROGRESS => {
1423+
tracing::info!(
1424+
game_index = %game.index,
1425+
game_address = ?game.address,
1426+
?status,
1427+
"Skipping resolve: game already resolved on chain"
1428+
);
1429+
continue;
1430+
}
1431+
Err(e) => {
1432+
tracing::warn!(
1433+
game_address = ?game.address,
1434+
error = ?e,
1435+
"Pre-flight status check failed, proceeding with resolve"
1436+
);
1437+
}
1438+
_ => {}
1439+
}
1440+
13811441
if let Err(error) = self.submit_resolution_transaction(&game).await {
13821442
if error.is_revert() {
13831443
tracing::error!(
@@ -1420,7 +1480,33 @@ where
14201480
.collect::<Vec<_>>()
14211481
};
14221482

1483+
let signer_address = self.signer.address();
14231484
for game in candidates {
1485+
// Pre-flight on-chain credit check at `latest`. The cached
1486+
// `should_attempt_to_claim_bond` is derived from the pinned (lagged)
1487+
// snapshot, so a recently confirmed `claimCredit()` tx may not yet be
1488+
// reflected. Querying at `latest` avoids re-submitting a claim that
1489+
// would only revert on chain.
1490+
let contract = OPSuccinctFaultDisputeGame::new(game.address, self.l1_provider.clone());
1491+
match contract.credit(signer_address).call().await {
1492+
Ok(credit) if credit == U256::ZERO => {
1493+
tracing::info!(
1494+
game_index = %game.index,
1495+
game_address = ?game.address,
1496+
"Skipping claim: bond already claimed on chain"
1497+
);
1498+
continue;
1499+
}
1500+
Err(e) => {
1501+
tracing::warn!(
1502+
game_address = ?game.address,
1503+
error = ?e,
1504+
"Pre-flight credit check failed, proceeding with claim"
1505+
);
1506+
}
1507+
_ => {}
1508+
}
1509+
14241510
if let Err(error) = self.submit_bond_claim_transaction(&game).await {
14251511
if error.is_revert() {
14261512
tracing::error!(
@@ -2336,6 +2422,7 @@ where
23362422
/// Returns `Ok(true)` if proving should be skipped:
23372423
/// - Game not found in cache
23382424
/// - Game not owned (vkeys don't match)
2425+
/// - Game is already proven or resolved on chain (pre-flight check at `latest`)
23392426
/// - Deadline has passed
23402427
///
23412428
/// Returns `Ok(false)` if proving should proceed.
@@ -2364,6 +2451,39 @@ where
23642451
}
23652452
}
23662453

2454+
// Pre-flight on-chain status check at `latest`. The cached `proposal_status` is read
2455+
// from the pinned (lagged) block, so a recently confirmed prove() or resolve() tx may
2456+
// not yet be reflected. Querying at `latest` avoids expensive proof regeneration that
2457+
// would only revert on submission. Skip when:
2458+
// - ProposalStatus is *ValidProofProvided (proof already submitted), or
2459+
// - ProposalStatus is Resolved (game concluded — set whenever GameStatus moves out of
2460+
// IN_PROGRESS, including timeout default-loss).
2461+
let contract = OPSuccinctFaultDisputeGame::new(game_address, self.l1_provider.clone());
2462+
match contract.claimData().call().await {
2463+
Ok(claim_data) => {
2464+
if matches!(
2465+
claim_data.status,
2466+
ProposalStatus::UnchallengedAndValidProofProvided |
2467+
ProposalStatus::ChallengedAndValidProofProvided |
2468+
ProposalStatus::Resolved
2469+
) {
2470+
tracing::info!(
2471+
?game_address,
2472+
proposal_status = ?claim_data.status,
2473+
"Skipping proving: game already proven or resolved on chain"
2474+
);
2475+
return Ok(true);
2476+
}
2477+
}
2478+
Err(e) => {
2479+
tracing::warn!(
2480+
?game_address,
2481+
error = ?e,
2482+
"Pre-flight proposal status check failed, proceeding with proving"
2483+
);
2484+
}
2485+
}
2486+
23672487
// Check deadline if provided
23682488
if let Some(deadline) = deadline {
23692489
let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH)?.as_secs();

‎scripts/utils/bin/cost_estimator.rs‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,22 @@ use op_succinct_host_utils::{
1616
};
1717
use op_succinct_proof_utils::{get_range_elf_embedded, initialize_host};
1818
use op_succinct_scripts::HostExecutorArgs;
19+
20+
// Cost-estimator-specific CLI args. Wraps `HostExecutorArgs` and adds the estimator-only
21+
// `--no-safe-head-split` flag so unrelated host binaries (e.g. `multi`,
22+
// `gen-sp1-test-artifacts`) don't advertise a flag they ignore.
23+
#[derive(Debug, Clone, Parser)]
24+
#[command(about = "Estimate OP Succinct execution costs over an L2 block range")]
25+
struct CostEstimatorArgs {
26+
#[command(flatten)]
27+
host: HostExecutorArgs,
28+
/// Bypass span-batch-aligned splitting even when SafeDB is active. Forces the basic
29+
/// fixed-size splitter so the range is partitioned solely by `--batch-size`. Useful for
30+
/// estimating per-segment cost as the proposer sees it (one zkVM execution per
31+
/// `RANGE_SPLIT_COUNT` segment) rather than per span batch.
32+
#[arg(long)]
33+
no_safe_head_split: bool,
34+
}
1935
use rayon::iter::{IntoParallelIterator, ParallelIterator};
2036
use sp1_sdk::{
2137
blocking::{CpuProver, Prover},
@@ -227,7 +243,9 @@ fn aggregate_execution_stats(
227243

228244
#[tokio::main]
229245
async fn main() -> Result<()> {
230-
let args = HostExecutorArgs::parse();
246+
let args = CostEstimatorArgs::parse();
247+
let no_safe_head_split = args.no_safe_head_split;
248+
let args = args.host;
231249

232250
dotenv::from_path(&args.env_file).ok();
233251
utils::setup_logger();
@@ -261,7 +279,7 @@ async fn main() -> Result<()> {
261279
// splitting algorithm. Otherwise, we use the simple range splitting algorithm.
262280
let safe_db_activated = data_fetcher.is_safe_db_activated().await?;
263281

264-
let split_ranges = if safe_db_activated {
282+
let split_ranges = if safe_db_activated && !no_safe_head_split {
265283
split_range_based_on_safe_heads(
266284
&data_fetcher,
267285
l2_start_block,

0 commit comments

Comments
 (0)