Skip to content

fix(fault-proof): robustness follow-ups to #865 + cost-estimator parity#901

Open
fakedev9999 wants to merge 7 commits into
mainfrom
pr-894-followups
Open

fix(fault-proof): robustness follow-ups to #865 + cost-estimator parity#901
fakedev9999 wants to merge 7 commits into
mainfrom
pr-894-followups

Conversation

@fakedev9999
Copy link
Copy Markdown
Member

Supersedes #894.

#894 is a cross-repo PR from celo-org:seolaoh/fault-proof-robustness-followups with maintainerCanModify=false, and CI on that PR is failing mostly because fork/cross-repo checks do not receive the required env/secrets (L1_RPC empty, checkout ref issues for tests/elf/Fault Proof (sync)/cost-estimator). This PR re-opens the same set of changes from a same-repo branch so CI can run with full secrets.

What's preserved

All four of @seolaoh's original #894 commits are included unchanged:

  • feat(scripts): add --no-safe-head-split to cost-estimator
  • fix(fault-proof): pre-flight on-chain status check before prove/resolve/claim
  • fix(fault-proof): reset creation guard when tracked game is pruned
  • fix(fault-proof): warn-log L1 head regression in sync_state

What's added

One follow-up commit on top — small parity/scope fixes from review of #894:

  • Scopes --no-safe-head-split to cost-estimator only. Moved out of shared HostExecutorArgs (also used by multi and gen-sp1-test-artifacts) into a cost-estimator-specific CostEstimatorArgs wrapper, so unrelated host binaries no longer advertise a flag they ignore.
  • Adds challenger-side resolve/claim latest-state pre-flight parity. Mirrors the proposer's pre-flight eth_call check before submitting resolve() / claimCredit(), with fail-open semantics (warn on RPC error, proceed). Comments adapted for challenger semantics: cached candidate flags can be stale by submission time (another actor's tx may have landed at latest), distinct from the proposer's pinned-snapshot lag.

Verification

  • cargo fmt --check
  • cargo check -p op-succinct-fp --lib
  • cargo check -p op-succinct-scripts --bin cost-estimator
  • cargo test -p op-succinct-scripts --lib — 10/10 pass
  • cost-estimator --help | rg "no-safe-head-split" — flag present
  • multi --help | rg "no-safe-head-split" — exit 1 (no match)
  • gen-sp1-test-artifacts --help | rg "no-safe-head-split" — exit 1 (no match)

Credit

Original PR by @seolaoh: #894. Leaving #894 open.

seolaoh and others added 5 commits May 4, 2026 17:54
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>
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>
…ve/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>
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>
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.
@fakedev9999 fakedev9999 marked this pull request as ready for review May 7, 2026 11:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Metric Value
Batch Start 17,649,926
Batch End 17,649,931
Witness Generation (seconds) 0
Execution Duration (seconds) 45
Total Instruction Count 634,161,040
Oracle Verify Cycles 64,287,455
Derivation Cycles 486,367,816
Block Execution Cycles 6,538,433
Blob Verification Cycles 30,712,219
Total SP1 Gas 904,886,237
Number of Blocks 5
Number of Transactions 5
Ethereum Gas Used 230,790
Cycles per Block 126,832,208
Cycles per Transaction 126,832,208
Transactions per Block 1
Gas Used per Block 46,158
Gas Used per Transaction 46,158
BN Pair Cycles 0
BN Add Cycles 0
BN Mul Cycles 0
KZG Eval Cycles 0
EC Recover Cycles 0
P256 Verify Cycles 0

# Conflicts:
#	scripts/utils/bin/cost_estimator.rs
Copy link
Copy Markdown
Contributor

@seolaoh seolaoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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.

2 participants