Conversation
…umber mappings in KV backend, while keeping recovery fast on long-running chains. Fix inconsistent Ethereum block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a block number ↔ Ethereum hash mapping persistence and repair mechanism. It adds a new backend method Changes
Sequence DiagramsequenceDiagram
participant SW as Sync Worker
participant MC as Mapping<br/>Cursor
participant MDB as Mapping DB
participant RPC as RPC Handler
participant Storage
SW->>SW: sync_one_block<br/>(determine canonical)
SW->>MDB: write_hashes<br/>(with Write mode)
MDB->>Storage: persist mapping
SW->>SW: detect reorg
SW->>MDB: repair_canonical<br/>_number_mappings_batch
MDB->>MC: canonical_number_repair_cursor()
MC-->>MDB: block_hint
MDB->>Storage: find_latest_indexed<br/>_canonical_block
Storage-->>MDB: (block_num, hash)
MDB->>MDB: set_block_hash_by_number<br/>(repair stale mapping)
MDB->>MC: set_canonical_number<br/>_repair_cursor
MC->>Storage: persist cursor
RPC->>MDB: resolve_canonical<br/>_substrate_hash_by_number
MDB->>Storage: check mapping
alt mapping stale/missing
MDB->>Storage: read Ethereum block
MDB->>MDB: set_block_hash_by_number<br/>(repair)
end
MDB-->>RPC: canonical_hash
RPC-->>RPC: block_info_by_number
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@client/db/src/kv/mod.rs`:
- Around line 153-160: Run rustfmt (cargo fmt) to fix formatting in the block
that computes fallback and calls
self.mapping.set_latest_canonical_indexed_block; ensure the let fallback line
and the if block are formatted according to rustfmt (no extra indentation or
misplaced braces) so the block using block_number, INDEXED_RECOVERY_SCAN_LIMIT,
and self.mapping.set_latest_canonical_indexed_block(...) matches project style
and passes CI.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client/db/src/kv/mod.rs`:
- Around line 133-143: The current min_block_hint can be greater than the scan
start (start_block) causing an empty recovery scan; clamp or drop the hint
before calling find_latest_indexed_canonical_block. After computing
min_block_hint (from mapping.latest_canonical_indexed_block_number and
best_number) ensure you compare it to start_block and if min_block_hint >
start_block set the hint to None (or clamp it to start_block) so
find_latest_indexed_canonical_block receives a valid lower-bound; adjust the
code paths around min_block_hint, latest_canonical_indexed_block_number, and the
call to find_latest_indexed_canonical_block accordingly.
… blocks and auto-repair stale entries
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/mapping-sync/src/kv/mod.rs (1)
368-406:⚠️ Potential issue | 🟠 MajorAvoid repairing number mappings for non-canonical blocks when
is_new_bestis import-time only.
is_new_bestcan be true for a block that was best at import but is no longer canonical by sync time. In that case,repair_canonical_number_mapping_for_hashcan overwrite canonical mappings with a non-canonical hash, undoing the earlier skip logic. Consider gating the “latest canonical indexed” update and repair logic on a canonical check at sync time.🛠️ Proposed fix
if is_new_best { - let block_number: u64 = (*operating_header.number()).unique_saturated_into(); - frontier_backend - .mapping() - .set_latest_canonical_indexed_block(block_number)?; + let block_number: u64 = (*operating_header.number()).unique_saturated_into(); + let is_canonical_now = client + .hash(block_number.unique_saturated_into()) + .map_err(|e| format!("{e:?}"))? + == Some(hash); + if !is_canonical_now { + log::debug!( + target: "mapping-sync", + "Skipping canonical remap for non-canonical block #{} ({:?})", + operating_header.number(), + hash, + ); + } else { + frontier_backend + .mapping() + .set_latest_canonical_indexed_block(block_number)?; - let mut reorg_remapped = 0u64; - if repair_canonical_number_mapping_for_hash( - client, - storage_override.as_ref(), - frontier_backend, - hash, - )? - .is_some() - { - reorg_remapped = reorg_remapped.saturating_add(1); - } - if let Some(info) = reorg_info.as_ref() { - for enacted_hash in &info.enacted { - if repair_canonical_number_mapping_for_hash( - client, - storage_override.as_ref(), - frontier_backend, - *enacted_hash, - )? - .is_some() - { - reorg_remapped = reorg_remapped.saturating_add(1); - } - } - } - log::debug!( - target: "mapping-sync", - "Reorg canonical remap touched {reorg_remapped} blocks at new best {:?}", - hash, - ); + let mut reorg_remapped = 0u64; + if repair_canonical_number_mapping_for_hash( + client, + storage_override.as_ref(), + frontier_backend, + hash, + )? + .is_some() + { + reorg_remapped = reorg_remapped.saturating_add(1); + } + if let Some(info) = reorg_info.as_ref() { + for enacted_hash in &info.enacted { + if repair_canonical_number_mapping_for_hash( + client, + storage_override.as_ref(), + frontier_backend, + *enacted_hash, + )? + .is_some() + { + reorg_remapped = reorg_remapped.saturating_add(1); + } + } + } + log::debug!( + target: "mapping-sync", + "Reorg canonical remap touched {reorg_remapped} blocks at new best {:?}", + hash, + ); + } }
…ilize latest-index pointer
…ired are retried (or persist unresolved ranges), instead of always advancing past them
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ts-tests/tests/test-gas.ts (2)
241-259:⚠️ Potential issue | 🟡 MinorUse
letinstead ofvarto avoid variable redeclaration.Static analysis correctly flags that
iis redeclared at line 259. Usingvarhoists the variable to function scope, causing unintended shadowing. Useletfor proper block scoping.Proposed fix
- for (var i = 0; i < callsPerBlock; i++) { + for (let i = 0; i < callsPerBlock; i++) {- for (var i = 0; i < transfersPerBlock + extraTransfers; i++) { + for (let i = 0; i < transfersPerBlock + extraTransfers; i++) {
362-380:⚠️ Potential issue | 🟡 MinorUse
letinstead ofvarto avoid variable redeclaration.Same issue as in the previous test block - static analysis flags
iredeclaration at line 380.Proposed fix
- for (var i = 0; i < callsPerBlock; i++) { + for (let i = 0; i < callsPerBlock; i++) {- for (var i = 0; i < transfersPerBlock + extraTransfers; i++) { + for (let i = 0; i < transfersPerBlock + extraTransfers; i++) {
🧹 Nitpick comments (1)
client/mapping-sync/src/kv/mod.rs (1)
208-275: Consider adding early termination whenfirst_unresolvedis set.The current implementation continues iterating through the entire batch even after encountering the first unresolved block, but subsequent unresolved blocks won't update
first_unresolved(due toget_or_insert). While this maximizes repair in a single pass, it means the cursor will be set to retry from the first unresolved block, potentially re-processing already-repaired blocks in the next batch.This trade-off is acceptable for gradual recovery, but consider whether early termination on consecutive unresolved blocks would be more efficient for scenarios where a large range is unindexed.
librelois
added a commit
to moonbeam-foundation/frontier
that referenced
this pull request
Feb 12, 2026
polkadot-evm#1818) * Fix inconsistent Eth block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains. Fix inconsistent Ethereum block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains. * rustfmt * repair number mappings and stabilize latest-indexed recovery * Fix KV mapping sync to write block-number mappings only for canonical blocks and auto-repair stale entries * rustfmt * clippy lints * Guard is_new_best remap logic with sync-time canonical check * Harden canonical mapping sync against stale import-time best and stabilize latest-index pointer * improve sync and add tests * Fix pointer initialization & adjust reorg test scenario * Change repair cursor behavior so blocks that cannot currently be repaired are retried (or persist unresolved ranges), instead of always advancing past them * clippy lint * prettier * fix test gas * ts-tests: wait for chain head block in `createAndFinalizeBlock` to reduce CI flakiness * ts-tests: harden pov-size and newHeads tests against CI timing races * improve tests
librelois
added a commit
to moonbeam-foundation/frontier
that referenced
this pull request
Feb 12, 2026
polkadot-evm#1818) * Fix inconsistent Eth block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains. Fix inconsistent Ethereum block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains. * rustfmt * repair number mappings and stabilize latest-indexed recovery * Fix KV mapping sync to write block-number mappings only for canonical blocks and auto-repair stale entries * rustfmt * clippy lints * Guard is_new_best remap logic with sync-time canonical check * Harden canonical mapping sync against stale import-time best and stabilize latest-index pointer * improve sync and add tests * Fix pointer initialization & adjust reorg test scenario * Change repair cursor behavior so blocks that cannot currently be repaired are retried (or persist unresolved ranges), instead of always advancing past them * clippy lint * prettier * fix test gas * ts-tests: wait for chain head block in `createAndFinalizeBlock` to reduce CI flakiness * ts-tests: harden pov-size and newHeads tests against CI timing races * improve tests
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.
Goal of the Changes
Fix inconsistent
latestEthereum RPC behavior in KV-backed Frontier by ensuring block-number mappings stay canonical and recoverable during indexing lag/reorg windows, so APIs likeeth_getBlockByNumber("latest"),eth_blockNumber,eth_coinbase,eth_getLogs, and filter polling remain aligned.Rollout notes
Existing gaps are repaired by background batch plus on-read repair; recovery may be gradual on large affected ranges.
What Reviewers Need to Know
eth_getBlockByNumber("latest")now resolves viabackend.latest_block_hash()and canonical number->hash repair logic instead of relying on client best-head assumptions.eth_blockNumberandeth_coinbasenow use the same indexed head source to avoid returning data from blocks not yet visible tolatest.last_pollfrom that same capped height, preventing skipped logs when best head is ahead of indexed head.LATEST_CANONICAL_INDEXED_BLOCK) and repair cursor (CANONICAL_NUMBER_REPAIR_CURSOR).NumberMappingWrite::{Write, Skip}to avoid corrupting number mappings with non-canonical blocks.set_block_hash_by_numberto backend trait (default no-op; KV implements mutation, SQL keeps no-op) to support RPC-side self-healing.Testing
ts-tests/tests/test-latest-block-consistency.tscovering startup, reorg, lag, and multi-RPC consistency scenarios.ts-tests/tests/test-filter-api.tswith a regression test for log filter polling under indexing lag.