perf(reference-index): bound reorg to finalized, simplify read gate, bounded RPC wait (#143 #144 #145)#146
Merged
panos-xyz merged 4 commits intoJul 20, 2026
Conversation
The reference-index read path gated every query through a 7-variant ReferenceIndexPhase state machine plus a `generation` seqlock. Most of that machinery was not load-bearing: read correctness already comes from (1) the MDBX MVCC read-transaction snapshot, (2) the `(indexed_to, indexed_hash) == tip` comparison, and (3) the RPC-layer before/after `chain_info()` bracketing. - Delete the `generation` seqlock: the spin-for-even loop and the end-of-read re-check. The re-check was over-conservative — it discarded a fully-correct result (single snapshot, == tip) as `IndexUnavailable` merely because the runtime flipped `phase` mid-read. - Take `phase` out of the read path. It stays a pure metrics/log field (the runtime still sets it and the gauge still exports it). The one read-gate bit that remains — "Unavailable → tell clients not to retry" — is now a single `unavailable: AtomicBool` instead of a 7-way match. Deferred/Backfill/Repairing now fall through to the tip comparison, which returns `IndexBehind` on its own (Repairing is transient, so `IndexBehind`/retry is more correct than the old `IndexUnavailable`). - Move the pre-Jade check to the RPC layer via `is_pre_jade`: pre-Jade is a complete, terminal empty answer (`Ok([])`), not `IndexBehind`, so the query never enters the reader. query_at is now: check unavailable → open tx → read indexed_to/hash → compare == tip → iterate → return.
The index runtime is asynchronous (PR #141 decoupled it from block execution), so for a few milliseconds after a new block becomes the canonical tip the durable cursor still lags it. The common "submit a MorphTx, then immediately query its reference" pattern lands in that window and gets `IndexBehind`, forcing clients into a retry loop — a problem that gets worse as block time shrinks (#123). Instead of returning `IndexBehind` immediately, the RPC handler now waits a bounded amount (default 100 ms, polled every 5 ms) for the runtime to catch up to the canonical tip, then serves real data. The handler is a `blocking` RPC method, so the sleep only occupies a blocking-pool thread, never the async reactor, and is strictly bounded by the timeout. - Re-reads the canonical tip each poll so a tip that advances mid-wait is retried against the fresh head rather than spuriously failing. - Fails fast (no wait) when the runtime is `Deferred` (rapid sync) or `PreJade` (one-time Jade-activation boundary): neither catches up on the few-ms timescale the wait targets. - Adds metrics: rpc_waits_total, rpc_wait_timeouts_total, and rpc_wait_duration_seconds, recorded via ReferenceIndexHandle. Response shape and wire semantics are unchanged; only the timing of the `IndexBehind` decision moves.
…lock (#143) Precise incremental reorg rollback relies on the `IndexedBlocks` breadcrumb table (`block_number -> block_hash`). To support a reorg that could theoretically rewind to the Jade start, the reconcile path required retaining every breadcrumb across `jade_first ..= indexed_to`, so the table grew linearly and unbounded with the chain. A fixed depth cap is the wrong fix: any block not yet finalized on L1 can reorg, and reorg depth is a function of block-production speed and the L1 batch-submission interval, not a constant. The correct hard boundary is the L1 finalized block, which by definition can never be reorged. - `reconcile_cursor` now floors the rewind at `max(finalized, jade_first)` instead of `jade_first`; without finality yet it falls back to `jade_first` (unchanged behavior). A divergence that reaches below finalized is impossible on a healthy chain, so it surfaces as `ManualRebuildRequired` rather than scanning to Jade. - `commit_canonical_batch` prunes `IndexedBlocks` breadcrumbs below finalized, but only once the cursor tip has reached the pruning floor, so a deep backfill still below finalized keeps its rows (and the tip's own row) intact. Only the reorg breadcrumbs are pruned; the ReferenceIndex / BlockReferenceIndex query data is never touched, so pruned history stays fully queryable. - `CanonicalChain` gains `finalized_block_number`, forwarding to `BlockIdReader::finalized_block_number` (morph sets finalized via the engine API `newSafeL2Block`); the existing `BlockReaderIdExt` blanket bound already provides it. Accepted trade-off: while L1 submission stalls the retained window grows to `tip - finalized`, bounded by L1 liveness rather than an artificial constant. In steady state finalized tracks close to the tip, so the table stays small.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bound finalized breadcrumb pruning, preserve Jade snapshot bracketing, and make RPC catch-up waits deterministic and accurately observed.
panos-xyz
added a commit
that referenced
this pull request
Jul 22, 2026
…#141) * refactor: migrate engine validation to reth main hooks * fix(node): clear withdraw expectation on L1 index pre-check failure validate_payload already registered an expectation in convert_payload_to_block; clearing it on the early L1-index reject path matches the inner-failure cleanup and avoids a stale cache entry until LRU eviction. * chore: fix rustfmt and bump crossbeam-epoch for RUSTSEC-2026-0204 Format the L1-index cleanup path, upgrade crossbeam-epoch to 0.9.20, and drop advisory ignores that no longer match after the reth main bump. * refactor(reference-index): replace ExEx with canonical cursor runtime * fix(reference-index): allow no-op body pruning * fix(reference-index): bound rapid sync deferral * chore(deps): upgrade reth to v2.4.0 * perf(reference-index): bound reorg to finalized, simplify read gate, bounded RPC wait (#143 #144 #145) (#146) * refactor(reference-index): simplify reader read gate (#145) The reference-index read path gated every query through a 7-variant ReferenceIndexPhase state machine plus a `generation` seqlock. Most of that machinery was not load-bearing: read correctness already comes from (1) the MDBX MVCC read-transaction snapshot, (2) the `(indexed_to, indexed_hash) == tip` comparison, and (3) the RPC-layer before/after `chain_info()` bracketing. - Delete the `generation` seqlock: the spin-for-even loop and the end-of-read re-check. The re-check was over-conservative — it discarded a fully-correct result (single snapshot, == tip) as `IndexUnavailable` merely because the runtime flipped `phase` mid-read. - Take `phase` out of the read path. It stays a pure metrics/log field (the runtime still sets it and the gauge still exports it). The one read-gate bit that remains — "Unavailable → tell clients not to retry" — is now a single `unavailable: AtomicBool` instead of a 7-way match. Deferred/Backfill/Repairing now fall through to the tip comparison, which returns `IndexBehind` on its own (Repairing is transient, so `IndexBehind`/retry is more correct than the old `IndexUnavailable`). - Move the pre-Jade check to the RPC layer via `is_pre_jade`: pre-Jade is a complete, terminal empty answer (`Ok([])`), not `IndexBehind`, so the query never enters the reader. query_at is now: check unavailable → open tx → read indexed_to/hash → compare == tip → iterate → return. * feat(reference-index): bounded server-side wait on IndexBehind (#144) The index runtime is asynchronous (PR #141 decoupled it from block execution), so for a few milliseconds after a new block becomes the canonical tip the durable cursor still lags it. The common "submit a MorphTx, then immediately query its reference" pattern lands in that window and gets `IndexBehind`, forcing clients into a retry loop — a problem that gets worse as block time shrinks (#123). Instead of returning `IndexBehind` immediately, the RPC handler now waits a bounded amount (default 100 ms, polled every 5 ms) for the runtime to catch up to the canonical tip, then serves real data. The handler is a `blocking` RPC method, so the sleep only occupies a blocking-pool thread, never the async reactor, and is strictly bounded by the timeout. - Re-reads the canonical tip each poll so a tip that advances mid-wait is retried against the fresh head rather than spuriously failing. - Fails fast (no wait) when the runtime is `Deferred` (rapid sync) or `PreJade` (one-time Jade-activation boundary): neither catches up on the few-ms timescale the wait targets. - Adds metrics: rpc_waits_total, rpc_wait_timeouts_total, and rpc_wait_duration_seconds, recorded via ReferenceIndexHandle. Response shape and wire semantics are unchanged; only the timing of the `IndexBehind` decision moves. * perf(reference-index): bound reorg rewind floor to the L1 finalized block (#143) Precise incremental reorg rollback relies on the `IndexedBlocks` breadcrumb table (`block_number -> block_hash`). To support a reorg that could theoretically rewind to the Jade start, the reconcile path required retaining every breadcrumb across `jade_first ..= indexed_to`, so the table grew linearly and unbounded with the chain. A fixed depth cap is the wrong fix: any block not yet finalized on L1 can reorg, and reorg depth is a function of block-production speed and the L1 batch-submission interval, not a constant. The correct hard boundary is the L1 finalized block, which by definition can never be reorged. - `reconcile_cursor` now floors the rewind at `max(finalized, jade_first)` instead of `jade_first`; without finality yet it falls back to `jade_first` (unchanged behavior). A divergence that reaches below finalized is impossible on a healthy chain, so it surfaces as `ManualRebuildRequired` rather than scanning to Jade. - `commit_canonical_batch` prunes `IndexedBlocks` breadcrumbs below finalized, but only once the cursor tip has reached the pruning floor, so a deep backfill still below finalized keeps its rows (and the tip's own row) intact. Only the reorg breadcrumbs are pruned; the ReferenceIndex / BlockReferenceIndex query data is never touched, so pruned history stays fully queryable. - `CanonicalChain` gains `finalized_block_number`, forwarding to `BlockIdReader::finalized_block_number` (morph sets finalized via the engine API `newSafeL2Block`); the existing `BlockReaderIdExt` blanket bound already provides it. Accepted trade-off: while L1 submission stalls the retained window grows to `tip - finalized`, bounded by L1 liveness rather than an artificial constant. In steady state finalized tracks close to the tip, so the table stays small. * fix(reference-index): harden optimized read path Bound finalized breadcrumb pruning, preserve Jade snapshot bracketing, and make RPC catch-up waits deterministic and accurately observed. * refactor(reference-index): trim metrics to the operationally load-bearing set The subsystem exported 14 metrics, several redundant or derivable. Reduce to the 5 an operator actually acts on, with no in-repo dashboard consumers to break: - phase readiness lives here (Live == serving) - lag_blocks how far the index trails the canonical head - reorgs_total canonical suffix repairs - failures_total failed reconciliation turns - rpc_wait_timeouts_total bounded RPC waits that still returned IndexBehind Dropped: - rpc_ready strictly derivable from phase == Live - target_block duplicates the node's canonical-head metric (and == indexed_block + lag_blocks) - indexed_block absolute cursor; lag_blocks is the actionable delta - indexed_blocks_total throughput counter, low signal for this index - committed_batches_total only meaningful ratio'd against indexed_blocks_total - lagged_notifications_total niche; correlates with lag_blocks - batch_duration_seconds histogram; perf detail, not an operating signal - rpc_waits_total / rpc_wait_duration_seconds the timeout counter is the actionable RPC-wait signal on its own observe_rpc_wait now takes only `timed_out` and counts just the timeout; the RpcWaitGuard no longer tracks a start instant. * fix(rpc): extend reference index wait timeout
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of #141 (base branch
codex/reth-main-reference-index); GitHub will retarget this tomainonce #141 merges. Three independent reference-index optimizations, one commit each.#145 — simplify reader read gate (
1bc23b4)Drops machinery that was not load-bearing for correctness. Read correctness already comes from (1) the MDBX MVCC read-transaction snapshot, (2) the
(indexed_to, indexed_hash) == tipcomparison, and (3) the RPC-layer before/afterchain_info()bracketing.generationseqlock (spin-for-even loop + over-conservative end-of-read re-check).phaseout of the read path — it stays a pure metrics/log field; the only remaining read gate is a singleunavailable: AtomicBool.is_pre_jade(pre-Jade is a terminal empty answer, notIndexBehind).Closes #145.
#144 — bounded server-side wait on
IndexBehind(c7dfc53)The index runtime is asynchronous (#141), so for a few ms after a new block becomes the canonical tip the cursor lags it, and the common "submit a MorphTx then immediately query its reference" pattern hits
IndexBehindand retries.blockingRPC handler now waits a bounded amount (default 100 ms, 5 ms poll) for the runtime to catch up, re-reading the tip each poll, then serves real data.Deferred(rapid sync) andPreJade(one-time Jade-activation boundary).rpc_waits_total,rpc_wait_timeouts_total,rpc_wait_duration_secondsmetrics.Closes #144.
#143 — bound reorg rewind floor to the L1 finalized block (
c9f48fe)The
IndexedBlocksbreadcrumb table grew linearly and unbounded because the reorg rewind floor wasjade_first. A fixed depth cap is the wrong fix (any pre-finalized block can reorg; depth depends on block time and L1 batch interval). The true hard boundary is the L1 finalized block.reconcile_cursorfloors the rewind atmax(finalized, jade_first)(falls back tojade_firstwhen finality is unknown).commit_canonical_batchprunesIndexedBlocksbreadcrumbs below finalized, only once the cursor tip has reached the floor (deep sub-finalized backfill keeps its rows). Query data is never touched.CanonicalChaingainsfinalized_block_number, forwarding toBlockIdReader::finalized_block_number.Closes #143.
Testing
cargo test -p morph-reference-index --lib: 38 passedcargo test -p morph-rpc --lib: 84 passedcargo clippy -p morph-reference-index -p morph-rpc --lib --tests: clean(The crate's doctest target fails for a pre-existing, security-related reason unrelated to this change; verified via
--lib.)