Skip to content

fix(eth,continuity): lock-free ETH repair with shared generations; tip reads fail over; HTTP deadlines - #1352

Open
DylanVerstraete wants to merge 2 commits into
fix/prover-liveness-bfrom
fix/prover-liveness-c
Open

DylanVerstraete wants to merge 2 commits into
fix/prover-liveness-bfrom
fix/prover-liveness-c

Conversation

@DylanVerstraete

Copy link
Copy Markdown
Contributor

Third PR of the prover liveness audit, findings 3 and 4. Stacked on #1351#1350; retarget to usc-dev as those merge.

Finding 3: one stalled repair blocked every caller

ReconnectingEthRpcProvider::reconnect held the client write lock across eth::Client::reconnect, i.e. across the primary dial, every fallback dial and their chain-id reads. Every operation takes the read lock first, so one stalled repair blocked proof requests, backfill and health probes for that chain, and concurrent failures queued N separate repairs. The HTTP transport was alloy's default reqwest client, which has no timeout at all.

  • Repairs dial on a clone of the client, outside the lock and under a 30 s deadline, then swap it in atomically. Readers are never queued behind network I/O.
  • Connection generations: a caller that failed against generation N only dials if the live generation is still N; otherwise someone already repaired and it simply retries. Repairs are serialised on a mutex, so N simultaneous failures cost one dial.
  • Every attempt of every operation runs under a call deadline (300 s default; with_timeouts() to override).
  • The HTTP transport now has connect (10 s) and request (60 s) timeouts.

Finding 4: a healthy fallback could not keep proofs flowing

eth::Client::get_last_block asked only the primary, and every proof validates its heights against that tip, so a healthy fallback plus a warm Merkle cache still could not serve when the primary failed. Startup also failed outright when any fallback was unreachable.

  • get_last_block walks [primary, fallbacks…] the way block fetches already do. A lagging fallback reports a lower tip, which can only make the confirmation check stricter, never accept an unconfirmed block.
  • An unreachable fallback at startup is logged and skipped. The configured URL list is retained and reconnect() re-dials from it, so the backup is picked up on the next repair instead of being forgotten. A fallback on the wrong chain remains fatal (misconfiguration).

Tests

proof-gen-api-server/tests/liveness_eth_rpc.rs (wiremock on loopback):

  • tip read served by the fallback when the primary's eth_blockNumber errors
  • an unrelated caller gets its answer in < 2 s while another caller's repair dial hangs 10 s
  • two concurrent failing callers → exactly one dial per retry round (2 eth_chainId, not 4), generation ends at 2
  • dead fallback skipped at startup; wrong-chain fallback rejected

cargo test -p eth -p continuity -p proof-gen-api-server green (existing 21 eth, 81 + 27 prover); clippy -D warnings on eth, continuity, proof-gen-api-server, archiver; fmt.

The archiver's own dial helper (#1348) already tolerated a dead fallback by retrying primary-only; with this change new_with_fallbacks itself does, so that retry path becomes a no-op safety net.

…p reads fail over; HTTP deadlines

Prover liveness audit, findings 3 and 4.

Finding 3: ReconnectingEthRpcProvider::reconnect held the client write
lock across eth::Client::reconnect, i.e. across the primary dial, every
fallback dial and their chain-id reads. Every operation takes the read
lock first, so one stalled repair blocked proof requests, backfill and
health probes for that chain, and concurrent failures queued N separate
repairs. The HTTP transport was alloy's default reqwest client, which has
no timeout at all.
- Repairs dial on a clone of the client, outside the lock and under a
  30 s deadline, then swap it in atomically. Readers are never queued
  behind network I/O.
- Connection generations: a caller that failed against generation N
  only dials if the live generation is still N; otherwise someone already
  repaired and it retries on the new client. Repairs are serialised on a
  mutex so N simultaneous failures cost one dial.
- Every attempt of every operation runs under a call deadline (300 s
  default, with_timeouts() to override).
- The HTTP transport is built with connect (10 s) and request (60 s)
  timeouts.

Finding 4: eth::Client::get_last_block only asked the primary, and every
proof validates heights against that tip, so a healthy fallback and a
warm cache could not keep proofs flowing when the primary failed. Startup
also failed outright when any fallback was unreachable.
- get_last_block walks [primary, fallbacks...] like block fetches do; a
  lagging fallback can only make the confirmation check stricter.
- An unreachable fallback at startup is logged and skipped; the configured
  URL list is kept and reconnect() re-dials from it, so the backup is
  picked up once it is back. A fallback on the wrong chain stays fatal.

Tests (proof-gen-api-server/tests/liveness_eth_rpc.rs, wiremock): tip
served by fallback when the primary fails; an unrelated caller is not
blocked behind another caller's 10 s repair dial; two concurrent failing
callers produce exactly one dial per retry round (2, not 4); a dead
fallback is skipped at startup while a wrong-chain fallback is rejected.
@cursor

cursor Bot commented Sep 11, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes reconnect concurrency, timeouts, and tip-read routing on the proof-critical ETH RPC path; behavior is more resilient but mis-tuned deadlines could cause false failures on very slow RPCs.

Overview
Improves source-chain RPC liveness so proof generation is not wedged by hung repairs or a dead primary when backups are healthy.

ReconnectingEthRpcProvider no longer holds the client write lock across reconnect I/O: it dials on a clone under a deadline, swaps the client atomically, and tracks connection generations so concurrent failures share one repair instead of N parallel dials. Each RPC attempt is wrapped in a call timeout (defaults 300s / 30s dial; overridable via with_timeouts()).

eth::Client adds HTTP connect/request timeouts, reconnect_with_deadline, retains configured fallback URLs for later repair, skips unreachable fallbacks at startup (wrong chain still fatal), and makes get_last_block walk primary then fallbacks like block fetches.

Adds wiremock liveness tests for tip failover, non-blocking repair, shared repair dials, and fallback startup policy.

Reviewed by Cursor Bugbot for commit cd6af7a. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c73b79c. Configure here.

Comment thread common/continuity/src/rpc.rs
…under their own bound

Bugbot follow-up on #1352: the repair wrapped Client::reconnect (primary
plus every fallback, in order) in one dial deadline, so a hung fallback
could exhaust it after the primary was already up and the successful
primary connection was discarded. reconnect_with_deadline(primary) now
bounds the primary dial alone; each fallback is re-dialled under a 10 s
bound of its own and keeps its previous provider on failure. The primary's
outcome alone decides whether the repair succeeded.
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