refactor(reference-index): replace ExEx with canonical cursor runtime#141
refactor(reference-index): replace ExEx with canonical cursor runtime#141panos-xyz wants to merge 9 commits into
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: 22 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 ignored due to path filters (1)
📒 Files selected for processing (25)
📝 WalkthroughWalkthroughThe PR migrates Morph’s reference index from ExEx startup wiring to a background runtime, adds phase-aware canonical-chain synchronization and RPC reads, replaces the removed engine-tree extension with Morph-specific validation, and updates REth, payload, EVM, and workspace APIs. ChangesReference index runtime and RPC
Engine, payload, and dependency migration
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
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
e7f897c to
4a0d756
Compare
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.
1 participant
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.