refactor(reference-index): replace ExEx with canonical cursor runtime#141
Merged
Conversation
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.
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.
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe reference index is migrated from ExEx startup integration to a background runtime with canonical-chain synchronization, phase-aware querying, bounded RPC catch-up, stricter metadata validation, and end-to-end RPC tests. ChangesReference index runtime migration
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
panos-xyz
force-pushed
the
codex/reth-main-reference-index
branch
from
July 14, 2026 10:19
fc7af55 to
c38f45f
Compare
panos-xyz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/node/src/validator.rs`:
- Around line 313-320: Ensure the validation flow consumes the withdraw-root
expectation for every failure path, including errors from
validate_next_l1_msg_index_for_block and inner validate_payload. Use block_hash
with post_execution_validator.take_withdraw_trie_root_expectation, preserving
successful validation and avoiding duplicate cleanup on any single path.
In `@crates/reference-index/src/runtime.rs`:
- Around line 263-304: Validate the jade_first_block boundary from both the
stored and resolve_jade_first_block paths before using it: block n must have
timestamp >= jade_timestamp and block n - 1 must have timestamp <
jade_timestamp. Revalidate the same boundary immediately before
commit_canonical_batch, and reject or retry when either condition fails so an
inconsistent concurrent-reorg boundary is never persisted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8758adb6-0f39-4f44-894e-cf4db309beca
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
Cargo.tomlbin/morph-reth/Cargo.tomlbin/morph-reth/src/main.rscrates/chainspec/src/hardfork.rscrates/consensus/src/validation.rscrates/engine-api/src/builder.rscrates/engine-tree-ext/Cargo.tomlcrates/engine-tree-ext/src/gate.rscrates/engine-tree-ext/src/lib.rscrates/engine-tree-ext/src/payload_validator.rscrates/engine-tree-ext/src/trie_updates.rscrates/engine-tree-ext/tests/jade_boundary.rscrates/evm/Cargo.tomlcrates/evm/src/block/mod.rscrates/evm/src/context.rscrates/evm/src/evm.rscrates/node/Cargo.tomlcrates/node/build.rscrates/node/src/add_ons.rscrates/node/src/args.rscrates/node/src/exex/mod.rscrates/node/src/exex/reference_index.rscrates/node/src/lib.rscrates/node/src/node.rscrates/node/src/test_utils.rscrates/node/src/validator.rscrates/node/tests/it/engine.rscrates/node/tests/it/helpers.rscrates/node/tests/it/reference_index.rscrates/payload/builder/src/builder.rscrates/payload/types/src/attributes.rscrates/payload/types/src/lib.rscrates/reference-index/Cargo.tomlcrates/reference-index/src/backfill.rscrates/reference-index/src/db.rscrates/reference-index/src/lib.rscrates/reference-index/src/metrics.rscrates/reference-index/src/reader.rscrates/reference-index/src/reconcile.rscrates/reference-index/src/runtime.rscrates/reference-index/src/source.rscrates/reference-index/src/tables.rscrates/reference-index/src/types.rscrates/reference-index/src/writer.rscrates/revm/src/evm.rscrates/revm/src/precompiles.rscrates/rpc/Cargo.tomlcrates/rpc/src/eth/call.rscrates/rpc/src/eth/mod.rscrates/rpc/src/morph/handler.rscrates/rpc/src/morph/rpc.rs
💤 Files with no reviewable changes (10)
- crates/engine-tree-ext/src/lib.rs
- crates/engine-tree-ext/src/gate.rs
- crates/node/src/lib.rs
- crates/engine-tree-ext/Cargo.toml
- crates/engine-tree-ext/src/trie_updates.rs
- crates/engine-tree-ext/src/payload_validator.rs
- crates/node/src/exex/reference_index.rs
- crates/node/src/exex/mod.rs
- bin/morph-reth/Cargo.toml
- crates/engine-tree-ext/tests/jade_boundary.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/node/src/validator.rs`:
- Around line 313-320: Ensure the validation flow consumes the withdraw-root
expectation for every failure path, including errors from
validate_next_l1_msg_index_for_block and inner validate_payload. Use block_hash
with post_execution_validator.take_withdraw_trie_root_expectation, preserving
successful validation and avoiding duplicate cleanup on any single path.
In `@crates/reference-index/src/runtime.rs`:
- Around line 263-304: Validate the jade_first_block boundary from both the
stored and resolve_jade_first_block paths before using it: block n must have
timestamp >= jade_timestamp and block n - 1 must have timestamp <
jade_timestamp. Revalidate the same boundary immediately before
commit_canonical_batch, and reject or retry when either condition fails so an
inconsistent concurrent-reorg boundary is never persisted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8758adb6-0f39-4f44-894e-cf4db309beca
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
Cargo.tomlbin/morph-reth/Cargo.tomlbin/morph-reth/src/main.rscrates/chainspec/src/hardfork.rscrates/consensus/src/validation.rscrates/engine-api/src/builder.rscrates/engine-tree-ext/Cargo.tomlcrates/engine-tree-ext/src/gate.rscrates/engine-tree-ext/src/lib.rscrates/engine-tree-ext/src/payload_validator.rscrates/engine-tree-ext/src/trie_updates.rscrates/engine-tree-ext/tests/jade_boundary.rscrates/evm/Cargo.tomlcrates/evm/src/block/mod.rscrates/evm/src/context.rscrates/evm/src/evm.rscrates/node/Cargo.tomlcrates/node/build.rscrates/node/src/add_ons.rscrates/node/src/args.rscrates/node/src/exex/mod.rscrates/node/src/exex/reference_index.rscrates/node/src/lib.rscrates/node/src/node.rscrates/node/src/test_utils.rscrates/node/src/validator.rscrates/node/tests/it/engine.rscrates/node/tests/it/helpers.rscrates/node/tests/it/reference_index.rscrates/payload/builder/src/builder.rscrates/payload/types/src/attributes.rscrates/payload/types/src/lib.rscrates/reference-index/Cargo.tomlcrates/reference-index/src/backfill.rscrates/reference-index/src/db.rscrates/reference-index/src/lib.rscrates/reference-index/src/metrics.rscrates/reference-index/src/reader.rscrates/reference-index/src/reconcile.rscrates/reference-index/src/runtime.rscrates/reference-index/src/source.rscrates/reference-index/src/tables.rscrates/reference-index/src/types.rscrates/reference-index/src/writer.rscrates/revm/src/evm.rscrates/revm/src/precompiles.rscrates/rpc/Cargo.tomlcrates/rpc/src/eth/call.rscrates/rpc/src/eth/mod.rscrates/rpc/src/morph/handler.rscrates/rpc/src/morph/rpc.rs
💤 Files with no reviewable changes (10)
- crates/engine-tree-ext/src/lib.rs
- crates/engine-tree-ext/src/gate.rs
- crates/node/src/lib.rs
- crates/engine-tree-ext/Cargo.toml
- crates/engine-tree-ext/src/trie_updates.rs
- crates/engine-tree-ext/src/payload_validator.rs
- crates/node/src/exex/reference_index.rs
- crates/node/src/exex/mod.rs
- bin/morph-reth/Cargo.toml
- crates/engine-tree-ext/tests/jade_boundary.rs
🛑 Comments failed to post (2)
crates/node/src/validator.rs (1)
313-320: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Consume withdraw-root expectations on every validation failure.
The precheck at Line 314 and
validate_block’s inner error path return without removing the shared expectation. A stale entry can later be applied to the same block hash and falsely reject a retry or block-input validation.Proposed cleanup
let block_hash = payload.block.hash(); - self.validate_next_l1_msg_index_for_block(payload.block.as_ref(), &ctx)?; + if let Err(err) = + self.validate_next_l1_msg_index_for_block(payload.block.as_ref(), &ctx) + { + self.post_execution_validator + .take_withdraw_trie_root_expectation(block_hash); + return Err(err); + } fn validate_block( &mut self, block: SealedBlock<morph_primitives::Block>, ctx: TreeCtx<'_, MorphPrimitives>, ) -> reth_engine_tree::tree::ValidationOutcome<MorphPrimitives> { + let block_hash = block.hash(); self.validate_next_l1_msg_index_for_block(&block, &ctx)?; - let output = self.inner.validate_block(block, ctx)?; - self.validate_withdraw_trie_root(output) + match self.inner.validate_block(block, ctx) { + Ok(output) => self.validate_withdraw_trie_root(output), + Err(err) => { + self.post_execution_validator + .take_withdraw_trie_root_expectation(block_hash); + Err(err) + } + } }Also applies to: 325-332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/node/src/validator.rs` around lines 313 - 320, Ensure the validation flow consumes the withdraw-root expectation for every failure path, including errors from validate_next_l1_msg_index_for_block and inner validate_payload. Use block_hash with post_execution_validator.take_withdraw_trie_root_expectation, preserving successful validation and avoiding duplicate cleanup on any single path.crates/reference-index/src/runtime.rs (1)
263-304: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the Jade boundary before trusting or persisting it.
resolve_jade_first_blockuses separate provider reads, so a concurrent reorg can produce a canonical but incorrect boundary. Once persisted, later turns skip resolution and can permanently omit Jade blocks or index pre-Jade blocks.Before using either the stored or resolved value, verify that block
nhastimestamp >= jade_timestampand blockn - 1hastimestamp < jade_timestamp; revalidate immediately before committing the first batch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/reference-index/src/runtime.rs` around lines 263 - 304, Validate the jade_first_block boundary from both the stored and resolve_jade_first_block paths before using it: block n must have timestamp >= jade_timestamp and block n - 1 must have timestamp < jade_timestamp. Revalidate the same boundary immediately before commit_canonical_batch, and reject or retry when either condition fails so an inconsistent concurrent-reorg boundary is never persisted.
![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.
…dex/reth-main-reference-index # Conflicts: # Cargo.toml
panos-xyz
force-pushed
the
codex/reth-main-reference-index
branch
from
July 16, 2026 03:27
e7f897c to
4a0d756
Compare
6 tasks
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.
This was referenced Jul 20, 2026
Closed
…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.
…ring 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.
chengwenxi
approved these changes
Jul 21, 2026
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
2 participants
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.
Important
This is a stacked PR on top of #140. PR #140 owns the Reth main upgrade and
engine-tree-extremoval; this PR should be reviewed only for thereference-index changes. After #140 merges, this PR can be retargeted to
main.Summary
Reference-index behavior
(block number, block hash)cursor are the source of truth; notifications are wake-up/rate signals only<datadir>/morph/reference_index; there is no automatic migration or deletion of the unreleased ExEx schemaValidation
cargo fmt --all -- --checkcargo check --workspace --all-featurescargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-features --no-fail-fastScope
This PR intentionally excludes the local benchmark harness, contracts, benchmark report, and snapshot scripts that are being developed separately.
Summary by CodeRabbit
New Features
Bug Fixes