refactor(reference-index): replace ExEx with canonical cursor runtime#141
refactor(reference-index): replace ExEx with canonical cursor runtime#141panos-xyz wants to merge 4 commits into
Conversation
|
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:
📝 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 |
panos-xyz
force-pushed
the
codex/reth-main-reference-index
branch
from
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
e7f897c to
4a0d756
Compare
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.