feat(rpc): implement morph_getTransactionHashesByReference#106
Open
feat(rpc): implement morph_getTransactionHashesByReference#106
Conversation
…age layer - 4-table schema: ReferenceIndex, BlockReferenceIndex, IndexedBlocks, IndexMeta - chain identity validation on DB open (chain_id, genesis_hash, schema_version) - BackfillState (NotStarted/InProgress/Complete) + is_ready AtomicBool - writer: write_block / delete_block (three-table atomic) - reader: prefix cursor query with is_ready + lag threshold guards - backfill: jade_first_block binary search + batched backfill with crash recovery - reconcile: canonical hash check (offline reorg detection) + suffix gap fill - metrics helpers for lag, progress, state, readiness - 17 unit tests all passing
Implements the RPC namespace, ExEx, and node integration that back the new
reference index storage layer added in the previous commit.
- morph-rpc: new morph_ namespace with getTransactionHashesByReference
- returns -32000 "reference index initializing" before Task A completes
- returns -32000 "reference index is behind" when lag exceeds threshold
- validates limit/offset bounds with -32602
- morph-node: ReferenceIndexControl (watch-channel-coordinated) wires the
startup indexing task (Task A) with the ExEx (Task B)
- Task A: maybe_reset_jade_sentinel -> backfill -> reconcile ->
is_ready=true -> startup FinishedHeight
- Task B: drains notifications from node launch, gates writes on is_ready,
gap-fills from main DB on first is_ready notification, processes
ChainCommitted/Reverted/Reorged with three-table atomic writes, sends
FinishedHeight(BlockNumHash)
- MorphAddOns: optional reference_index control; when present spawns Task A
via task_executor and registers the morph_ RPC handler
- bin/morph-reth: opens reference index DB under <datadir>/morph/reference_index,
installs the ExEx, and injects the control into MorphAddOns
Three node-level integration tests verify the reference index storage layer: - finds single morph_tx by reference after backfill - paginates results across multiple blocks correctly - returns empty for unrelated reference keys Tests run backfill + reconcile directly against the node's provider (no ExEx required), which is sufficient to validate the query path.
Finding 4 (Critical): NoHash → WithHash for all historical block reads
- backfill.rs, reconcile.rs, exex/reference_index.rs fill_gap
- reth docs say TransactionVariant::NoHash produces invalid tx hashes;
write_block stores tx_hash in index keys, so NoHash caused silent data
corruption in all historical indexing paths
Finding 3 (Moderate): reconcile check_start clamped to indexed_from
- sentinel path (jade not yet active): backfill sets indexed_to=head but
writes no IndexedBlocks, so reconcile's scan would see None != canonical
and trigger spurious fork_height at check_start
- fix: take max(indexed_from, indexed_to - depth) as check_start
Finding 1 (Critical): Task A startup failure upgraded to fatal
- was: error! log and continue, leaving RPC stuck at "initializing" forever
- now: panic propagated into spawn_critical → node shutdown on failure
Finding 5 (Moderate): first-ready gap fill is now delete-then-write
- fill_gap renamed to fill_gap_idempotent, deletes stale entries first
- also handles ChainReverted notification as first is_ready notification
Finding 8 (Nit): RPC method marked blocking for MDBX I/O
- #[method(name = ..., blocking)] so jsonrpsee dispatches on blocking pool
Also: clarify write_block doc — not idempotent, caller must delete_block first
Finding F2 (High): InProgress resume clamped to jade_first_block_number
- If crash happens between InProgress write and first batch commit,
indexed_to is still 0; resume now uses max(indexed_to+1, jade_first)
instead of plain indexed_to+1 to avoid re-indexing pre-Jade blocks
Finding F3 (High): RPC offset/limit use alloy_serde::quantity::opt
- geth sends hex-encoded quantities ("0x0", "0x64") on the wire;
plain u64 serde deserializer rejected them silently
- matches spec constraint #2: geth compatibility first
Finding F5 (Medium): paired snapshot validation implemented
- ReferenceIndexDb::validate_paired_snapshot(main_block_hash_fn)
checks snapshot_block_hash/number against the main DB provider
and returns descriptive errors on mismatch or ahead-of-DB cases
- called from run_startup_indexing before backfill starts, after
provider is available (matching spec timing requirement)
…arent
Scenario:
- reconcile finishes with indexed_to = H
- is_ready=false drain window: ExEx drains ChainCommitted{H+1}, {H+2}
but writes nothing
- is_ready=true, first_ready=true
- first notification is ChainReverted { old: [H+2] }
- revert_start = H+2, parent = H+1
Old code:
first_ready branch only filled when revert_start <= indexed_to (false here),
so nothing happened. handle_notification then ran delete_block(H+2) as a
no-op and set indexed_to = parent = H+1 -- but H+1 had never been written.
Result: indexed_to is ahead of what is actually indexed, future reconcile
sees phantom hash mismatch on H+1.
Fix:
In first_ready ChainReverted branch, compute parent = revert_start - 1.
If parent > indexed_to, drain window committed canonical blocks
(indexed_to+1..=parent) that will SURVIVE this revert; backfill them now
via fill_gap_idempotent. handle_notification then rolls back to parent
consistently.
parent <= indexed_to case (revert overlaps already-indexed range) is
already correct without special handling.
…inishedHeight Finding 1 (High): reconcile treated indexed_hash=None as hash mismatch The sentinel path (Jade not yet activated) sets indexed_to=head but writes no IndexedBlocks entries. reconcile then scanned [indexed_from, indexed_to], found None != canonical_hash, and falsely triggered reorg rebuild for pre-Jade blocks. Fix: skip blocks where IndexedBlocks has no entry (None means "never indexed", not "hash mismatch"). Note: indexed_from.max(depth_start) from an earlier round correctly narrows the scan range but did NOT prevent the None comparison since the range still included indexed_to itself in the sentinel case. Finding 2 (Medium): startup FinishedHeight could regress past live commits tokio::select! is non-deterministic: if a ChainCommitted notification is processed before the startup watch channel fires, ExExEvent::FinishedHeight would be sent at H+k then regressed to H (the startup indexed_to). Fix: introduce send_finished_height_monotonic() that only sends when the new height strictly exceeds last_finished; both the startup watch arm and ChainCommitted/ChainReorged share this helper. ChainReorged is the one permitted exception (reth ExEx docs allow height to go down on reorgs).
…nal error scrub Finding 1 (Critical): InProgress + sentinel crash-recovery skips Jade history If node crashes between writing InProgress+SENTINEL (txn 1) and writing Complete (txn 2), the next restart had BackfillState::InProgress and jade_first_block_number=SENTINEL. The previous fix for InProgress resume only avoided starting from block 1, but the sentinel fast-path inside the InProgress branch still blindly marked Complete without re-resolving Jade. Fix: when InProgress+SENTINEL, re-run resolve_jade_first_block against the current head. If Jade is now active, persist the real jade_first_block_number and continue backfill from there. Only take the immediate-complete shortcut when Jade is still not active on the current head. Finding 2 (Moderate): to_rpc_error leaked internal Database/Provider/Other error strings verbatim to RPC callers. The spec only documents two state-gating responses; anything else is an internal failure. Fix: map Database/Provider/Other to a fixed -32603 "internal reference index error" message; log the full error internally with tracing::error!.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new ChangesReference Index Feature & Integration
Sequence Diagram(s)sequenceDiagram
participant NodeBin as Node Binary
participant RefDB as ReferenceIndexDB
participant NodeBuilder as Node Builder / AddOns
participant Startup as Startup Indexing Task
participant ExEx as ExEx Task
participant Chain as Blockchain Provider
NodeBin->>RefDB: open(chain_id, genesis_hash)
RefDB->>RefDB: validate_or_init_chain_identity()
NodeBin->>NodeBuilder: with_reference_index(control)
NodeBuilder->>Startup: spawn_blocking(run_startup_indexing)
Startup->>Chain: read headers / resolve Jade first block
Chain-->>Startup: headers / jade_block
Startup->>RefDB: run_backfill(batch writes)
RefDB->>RefDB: write indexed blocks & meta
Startup->>RefDB: run_startup_reconcile()
RefDB->>Chain: verify paired snapshots / detect reorg
RefDB->>RefDB: fill gaps / update indexed_to
Startup->>RefDB: mark_startup_finished() -> signal
ExEx->>RefDB: await readiness signal
ExEx->>Chain: receive block notifications
ExEx->>RefDB: apply transactional updates per notification
sequenceDiagram
participant Client as JSON-RPC Client
participant Rpc as Morph RPC Handler
participant Reader as ReferenceIndexReader
participant RefDB as ReferenceIndexDB
Client->>Rpc: morph_getTransactionHashesByReference(args)
Rpc->>Reader: build ReferenceQuery + canonical_tip
Reader->>RefDB: is_ready()?
alt not ready
RefDB-->>Reader: not ready
Reader-->>Rpc: IndexNotInitialized error
Rpc-->>Client: JSON-RPC error
else ready
Reader->>RefDB: indexed_to (compute lag)
alt lag > threshold
Reader-->>Rpc: IndexBehind error
Rpc-->>Client: JSON-RPC error
else
Reader->>RefDB: open read tx, seek reference key
Reader->>RefDB: iterate, apply offset/limit
Reader-->>Rpc: Vec<ReferenceTransactionResult>
Rpc-->>Client: JSON-RPC result
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/node/src/exex/reference_index.rs`:
- Around line 95-104: The startup branch forwards a potentially stale
FinishedHeight (startup_rx) that can be rendered non-canonical by an intervening
ChainReorged; before calling send_finished_height_monotonic(&ctx.events, block,
&mut last_finished) re-validate the startup payload against the current index
state and canonical hash (e.g. compare block.number and block.hash to
control.db.indexed_to() and the chain's canonical hash) and skip/dismiss the
startup payload if any notification (ChainReorged or other index mutations) has
already advanced or changed the canonical head; apply the same guard to the
other occurrence around send_finished_height_monotonic noted in the review
(lines referenced 359-370) to avoid re-advancing pruning to a non-canonical
block.
In `@crates/reference-index/src/db.rs`:
- Around line 169-181: The code currently allows reopening when
IndexMeta::SchemaVersion is missing (stored_schema == None), which lets a
pre-schema DB be opened as if current; update the reopen logic to reject a
missing schema version when other index metadata exists: after retrieving
stored_schema via
tx.get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into()), if
stored_schema is None then check for presence of other meta keys (e.g.
tx.get::<crate::tables::IndexMeta>(IndexMetaKey::ChainId.into()) and
tx.get(...IndexMetaKey::GenesisHash.into())); if any of those return Some(_)
then return an error (use ReferenceIndexError::SchemaMismatch or a new
MissingSchemaVersion variant) instead of falling through, otherwise allow
initialization as before. Ensure you still compare Some(v) != SCHEMA_VERSION as
in the existing match.
In `@crates/reference-index/src/reconcile.rs`:
- Around line 84-90: The rollback path in reconcile.rs currently deletes
BlockReferenceIndex and IndexedBlocks via delete_block(&tx, number) and updates
indexed_to, but it does not remove associated ReferenceIndex rows, leaving
reverted tx hashes still queryable by reader.query(); modify the rollback helper
to also delete matching ReferenceIndex entries within the same transaction
(extend delete_block or add a delete_reference_index(&tx, number) call so
ReferenceIndex rows for that block are removed before tx.commit()), and add a
reorg regression test that performs an offline reorg and asserts that
reader.query() no longer returns reverted transaction hashes.
🪄 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: bdde8bdd-87f9-4290-b548-ddd0771e0c77
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
Cargo.tomlbin/morph-reth/Cargo.tomlbin/morph-reth/src/main.rscrates/node/Cargo.tomlcrates/node/src/add_ons.rscrates/node/src/exex/mod.rscrates/node/src/exex/reference_index.rscrates/node/src/lib.rscrates/node/tests/it/main.rscrates/node/tests/it/reference_index.rscrates/reference-index/Cargo.tomlcrates/reference-index/src/backfill.rscrates/reference-index/src/db.rscrates/reference-index/src/lib.rscrates/reference-index/src/reader.rscrates/reference-index/src/reconcile.rscrates/reference-index/src/tables.rscrates/reference-index/src/types.rscrates/reference-index/src/writer.rscrates/rpc/Cargo.tomlcrates/rpc/src/lib.rscrates/rpc/src/morph/handler.rscrates/rpc/src/morph/mod.rscrates/rpc/src/morph/rpc.rs
Comment on lines
+95
to
+104
| // Forward the startup FinishedHeight when Task A finishes. | ||
| changed = startup_rx.changed() => { | ||
| if changed.is_ok() && let Some(block) = *startup_rx.borrow_and_update() { | ||
| debug!( | ||
| target: "morph::reference_index", | ||
| block_number = block.number, | ||
| "startup complete; forwarding initial FinishedHeight" | ||
| ); | ||
| send_finished_height_monotonic(&ctx.events, block, &mut last_finished)?; | ||
| } |
There was a problem hiding this comment.
Guard the startup FinishedHeight against intervening reorgs.
A shorter ChainReorged can be handled before startup_rx.changed() wins the select!. After that, this branch can still forward the stale startup (number, hash) because send_finished_height_monotonic only compares heights. That re-advances the pruning marker to a non-canonical block after the reorg.
At minimum, re-check that the startup payload still matches control.db.indexed_to() and the current canonical hash before sending it, or drop the startup payload once any notification has already mutated the index.
Also applies to: 359-370
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/node/src/exex/reference_index.rs` around lines 95 - 104, The startup
branch forwards a potentially stale FinishedHeight (startup_rx) that can be
rendered non-canonical by an intervening ChainReorged; before calling
send_finished_height_monotonic(&ctx.events, block, &mut last_finished)
re-validate the startup payload against the current index state and canonical
hash (e.g. compare block.number and block.hash to control.db.indexed_to() and
the chain's canonical hash) and skip/dismiss the startup payload if any
notification (ChainReorged or other index mutations) has already advanced or
changed the canonical head; apply the same guard to the other occurrence around
send_finished_height_monotonic noted in the review (lines referenced 359-370) to
avoid re-advancing pruning to a non-canonical block.
Comment on lines
+169
to
+181
| let stored_schema = tx | ||
| .get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into())? | ||
| .map(decode_u32) | ||
| .transpose()?; | ||
| match stored_schema { | ||
| Some(v) if v != SCHEMA_VERSION => { | ||
| return Err(ReferenceIndexError::SchemaMismatch { | ||
| expected: SCHEMA_VERSION, | ||
| actual: v, | ||
| }); | ||
| } | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
Reject missing schema_version on reopen.
If ChainId/GenesisHash exist but SchemaVersion is absent, this branch falls through and opens the DB as current schema. That bypasses the compatibility check and lets a pre-schema or partially initialized index be read with the new layout.
Suggested fix
let stored_schema = tx
.get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into())?
.map(decode_u32)
.transpose()?;
match stored_schema {
- Some(v) if v != SCHEMA_VERSION => {
+ Some(v) if v != SCHEMA_VERSION => {
return Err(ReferenceIndexError::SchemaMismatch {
expected: SCHEMA_VERSION,
actual: v,
});
}
- _ => {}
+ Some(_) => {}
+ None => {
+ return Err(ReferenceIndexError::Other(eyre::eyre!(
+ "reference index schema_version missing"
+ )));
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let stored_schema = tx | |
| .get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into())? | |
| .map(decode_u32) | |
| .transpose()?; | |
| match stored_schema { | |
| Some(v) if v != SCHEMA_VERSION => { | |
| return Err(ReferenceIndexError::SchemaMismatch { | |
| expected: SCHEMA_VERSION, | |
| actual: v, | |
| }); | |
| } | |
| _ => {} | |
| } | |
| let stored_schema = tx | |
| .get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into())? | |
| .map(decode_u32) | |
| .transpose()?; | |
| match stored_schema { | |
| Some(v) if v != SCHEMA_VERSION => { | |
| return Err(ReferenceIndexError::SchemaMismatch { | |
| expected: SCHEMA_VERSION, | |
| actual: v, | |
| }); | |
| } | |
| Some(_) => {} | |
| None => { | |
| return Err(ReferenceIndexError::Other(eyre::eyre!( | |
| "reference index schema_version missing" | |
| ))); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/reference-index/src/db.rs` around lines 169 - 181, The code currently
allows reopening when IndexMeta::SchemaVersion is missing (stored_schema ==
None), which lets a pre-schema DB be opened as if current; update the reopen
logic to reject a missing schema version when other index metadata exists: after
retrieving stored_schema via
tx.get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into()), if
stored_schema is None then check for presence of other meta keys (e.g.
tx.get::<crate::tables::IndexMeta>(IndexMetaKey::ChainId.into()) and
tx.get(...IndexMetaKey::GenesisHash.into())); if any of those return Some(_)
then return an error (use ReferenceIndexError::SchemaMismatch or a new
MissingSchemaVersion variant) instead of falling through, otherwise allow
initialization as before. Ensure you still compare Some(v) != SCHEMA_VERSION as
in the existing match.
Comment on lines
+84
to
+90
| let tx = db.tx_mut()?; | ||
| for number in fh..=indexed_to { | ||
| delete_block(&tx, number)?; | ||
| } | ||
| let new_indexed_to = fh.saturating_sub(1); | ||
| update_indexed_to(&tx, new_indexed_to)?; | ||
| tx.commit()?; |
There was a problem hiding this comment.
Rollback must also purge ReferenceIndex rows.
This path rewinds indexed_to, but reader.query() reads from ReferenceIndex, and the provided crates/reference-index/src/writer.rs:72-109 snippet shows delete_block() only removes BlockReferenceIndex plus IndexedBlocks. After an offline reorg, reverted tx hashes can therefore stay queryable even though the block was rolled back. Please make the rollback helper delete the matching ReferenceIndex entries in the same transaction and add a reorg regression test for that path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/reference-index/src/reconcile.rs` around lines 84 - 90, The rollback
path in reconcile.rs currently deletes BlockReferenceIndex and IndexedBlocks via
delete_block(&tx, number) and updates indexed_to, but it does not remove
associated ReferenceIndex rows, leaving reverted tx hashes still queryable by
reader.query(); modify the rollback helper to also delete matching
ReferenceIndex entries within the same transaction (extend delete_block or add a
delete_reference_index(&tx, number) call so ReferenceIndex rows for that block
are removed before tx.commit()), and add a reorg regression test that performs
an offline reorg and asserts that reader.query() no longer returns reverted
transaction hashes.
SnapshotBlockNumber/SnapshotBlockHash metadata keys and validate_paired_snapshot() had no write path — ops tar-compress the datadir directly so these fields were always empty and the check was always a no-op. chain_id + genesis_hash at open() plus reconcile's canonical hash check already cover the mismatch cases that matter.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
crates/node/src/exex/reference_index.rs (1)
95-104:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRe-validate the startup
FinishedHeightbefore forwarding it.This is still racy with shorter reorgs. If a
ChainReorgedis handled before this watch arm runs,send_finished_height_monotoniccan re-send the stale startup(number, hash)and move pruning back onto a non-canonical block.Suggested fix
changed = startup_rx.changed() => { if changed.is_ok() && let Some(block) = *startup_rx.borrow_and_update() { + let current_number = control.db.indexed_to()?; + let current_hash = ctx.components.provider().block_hash(current_number)?; + if current_number != block.number || current_hash != Some(block.hash) { + debug!( + target: "morph::reference_index", + startup_number = block.number, + current_number, + "dropping stale startup FinishedHeight" + ); + continue; + } debug!( target: "morph::reference_index", block_number = block.number, "startup complete; forwarding initial FinishedHeight" );🤖 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/exex/reference_index.rs` around lines 95 - 104, Re-validate the startup FinishedHeight after the startup_rx.changed() branch but before calling send_finished_height_monotonic: after reading block from startup_rx.borrow_and_update(), check the current chain tip or recent reorg flag (the same check used when handling ChainReorged events) to ensure the (number, hash) is still canonical; if it is no longer canonical skip forwarding and do not update last_finished. Locate the watch arm using startup_rx, the call to send_finished_height_monotonic(&ctx.events, block, &mut last_finished), and the ChainReorged handling logic to reuse the canonical-validation check so the startup message cannot re-send a stale FinishedHeight.crates/reference-index/src/db.rs (1)
167-178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject reopen when
schema_versionis missing.If
ChainId/GenesisHashare already persisted butSchemaVersionis absent, this still falls through as success. That lets a pre-schema or partially initialized index be read with the current layout.Suggested fix
match stored_schema { Some(v) if v != SCHEMA_VERSION => { return Err(ReferenceIndexError::SchemaMismatch { expected: SCHEMA_VERSION, actual: v, }); } - _ => {} + Some(_) => {} + None => { + return Err(ReferenceIndexError::Other(eyre::eyre!( + "reference index schema_version missing" + ))); + } }🤖 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/db.rs` around lines 167 - 178, The code currently lets stored_schema == None pass, allowing a partially-initialized index to be treated as valid; update the match on stored_schema (the result of tx.get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into())) so that None is treated as an error rather than success: either return Err(ReferenceIndexError::SchemaMissing) if you prefer a dedicated error variant, or return Err(ReferenceIndexError::SchemaMismatch { expected: SCHEMA_VERSION, actual: 0 }) for backwards compatibility; make this change in the stored_schema match branch (and add the new ReferenceIndexError::SchemaMissing variant if you choose that route).
🤖 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/reference-index/src/writer.rs`:
- Around line 151-160: The function set_jade_first_block_number in writer.rs is
not formatted to project rustfmt standards; run rustfmt (cargo fmt) on the file
(or the repo) to reformat this function and surrounding code so the Rustfmt CI
job passes, ensuring code around IndexMetaKey::JadeFirstBlockNumber,
encode_u64(block_number), and the tx.put::<IndexMeta>(...) call is adjusted to
the project's style rules.
---
Duplicate comments:
In `@crates/node/src/exex/reference_index.rs`:
- Around line 95-104: Re-validate the startup FinishedHeight after the
startup_rx.changed() branch but before calling send_finished_height_monotonic:
after reading block from startup_rx.borrow_and_update(), check the current chain
tip or recent reorg flag (the same check used when handling ChainReorged events)
to ensure the (number, hash) is still canonical; if it is no longer canonical
skip forwarding and do not update last_finished. Locate the watch arm using
startup_rx, the call to send_finished_height_monotonic(&ctx.events, block, &mut
last_finished), and the ChainReorged handling logic to reuse the
canonical-validation check so the startup message cannot re-send a stale
FinishedHeight.
In `@crates/reference-index/src/db.rs`:
- Around line 167-178: The code currently lets stored_schema == None pass,
allowing a partially-initialized index to be treated as valid; update the match
on stored_schema (the result of
tx.get::<crate::tables::IndexMeta>(IndexMetaKey::SchemaVersion.into())) so that
None is treated as an error rather than success: either return
Err(ReferenceIndexError::SchemaMissing) if you prefer a dedicated error variant,
or return Err(ReferenceIndexError::SchemaMismatch { expected: SCHEMA_VERSION,
actual: 0 }) for backwards compatibility; make this change in the stored_schema
match branch (and add the new ReferenceIndexError::SchemaMissing variant if you
choose that route).
🪄 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: f60dc856-8bd6-4037-8652-db2b6fbb3c99
📒 Files selected for processing (3)
crates/node/src/exex/reference_index.rscrates/reference-index/src/db.rscrates/reference-index/src/writer.rs
Comment on lines
+151
to
+160
| pub fn set_jade_first_block_number<Tx: DbTxMut>( | ||
| tx: &Tx, | ||
| block_number: u64, | ||
| ) -> Result<(), ReferenceIndexError> { | ||
| tx.put::<IndexMeta>( | ||
| IndexMetaKey::JadeFirstBlockNumber.into(), | ||
| encode_u64(block_number), | ||
| )?; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Run cargo fmt on this file before merge.
The Rustfmt job is failing on writer.rs, so this block still needs formatting cleanup to unblock CI.
🧰 Tools
🪛 GitHub Actions: Lint / 0_Rustfmt.txt
[error] 159-159: cargo fmt --all -- --check: formatting differences detected. Run 'cargo fmt' to format crates/reference-index/src/writer.rs.
🪛 GitHub Actions: Lint / Rustfmt
[error] 159-159: Command failed: cargo fmt --all -- --check. Formatting issues detected in writer.rs (diff around line 159).
🤖 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/writer.rs` around lines 151 - 160, The function
set_jade_first_block_number in writer.rs is not formatted to project rustfmt
standards; run rustfmt (cargo fmt) on the file (or the repo) to reformat this
function and surrounding code so the Rustfmt CI job passes, ensuring code around
IndexMetaKey::JadeFirstBlockNumber, encode_u64(block_number), and the
tx.put::<IndexMeta>(...) call is adjusted to the project's style rules.
- ReferenceIndexDb: drop highest_indexed_block / highest_block_reference_index (no callers in the workspace) - ReferenceIndexDb: remove unused DbCursorRO import exposed by the deletion - ReferenceIndexReader: drop db() accessor (no callers in the workspace)
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.
Summary
morph-reference-indexcrate: persistent MDBX index (4 tables) for querying MorphTx transactions by their 32-bytereferencekeymorph_getTransactionHashesByReferenceRPC with geth-compatible response shape and hex-quantity paginationDesign
See
docs/superpowers/specs/2026-04-29-morph-rpc-extensions-design.mdfor the full spec (7 rounds of adversarial review + multiple GPT reviews before implementation).Storage: independent MDBX instance at
<datadir>/morph/reference_index/, four tables:ReferenceIndex— primary query index:(reference, blockNumber, txIndex, txHash) → blockTimestampBlockReferenceIndex— reverse index for atomic block deletion:(blockNumber, txIndex, txHash) → referenceIndexedBlocks— canonical hash per indexed block, for offline reorg detectionIndexMeta— chain identity, backfill state, jade_first_block_number, snapshot metadataStartup (Task A): paired-snapshot validation → jade sentinel re-resolution → batched backfill from
jade_first_block_number(256 blocks/batch, 10ms sleep, atomic last-batch) → startup reconcile (canonical hash check + suffix gap fill) →is_ready = true→ initialFinishedHeightLive (Task B / ExEx): registered via
install_exex, drains from node launch to avoid backpressure, gates writes onis_ready, gap-fills on first ready notification, handlesChainCommitted/ChainReverted/ChainReorgedatomicallyRPC:
-32000 "reference index initializing"while not ready;-32000 "reference index is behind"whencanonical_tip - indexed_to > lag_threshold(default 16); accepts geth hex-quantityoffset/limit;#[method(blocking)]for MDBX readsCorrectness fixes addressed during adversarial review (3 Codex rounds)
TransactionVariant::WithHash(notNoHash) for all historical block reads — reth docs sayNoHashproduces invalid tx hashes; index keys embedtx_hashreconcilescans only fromindexed_fromas lower bound, and skipsIndexedBlocks = None(never-indexed blocks are not a mismatch)InProgress + SENTINELcrash recovery re-resolves Jade against current head before marking completeChainRevertedwithparent > indexed_to: fills the surviving canonical gap before the revertfill_gap_idempotent(delete-then-write) replaces plain append for all gap-fill pathssend_finished_height_monotonickeepsFinishedHeightnon-regressing across startup watch vs live notification racepanic!intospawn_critical→ node shutdown (not silenterror!)snapshot_block_hashvalidated against main DB provider after node startalloy_serde::quantity::optforoffset/limitto accept geth hex wire format-32603without leaking storage details to RPC clientsTest plan
cargo nextest run -p morph-reference-index— 17 unit tests (key encoding, DB open/chain identity, writer idempotency, reader lag/initializing guards)cargo nextest run -p morph-node --features test-utils -E 'test(reference_index)'— 3 integration tests: single tx lookup, pagination across blocks, empty result for unrelated referencecargo clippy -p morph-reference-index -p morph-rpc -p morph-node --all-targets -- -D warningscargo fmt --all -- --checkSummary by CodeRabbit