Skip to content

fix(dash-spv): prevent filter sync crash on new block after full sync#816

Merged
xdustinface merged 1 commit into
devfrom
fix/filter-lookahead-empty-segment-panic
Jun 22, 2026
Merged

fix(dash-spv): prevent filter sync crash on new block after full sync#816
xdustinface merged 1 commit into
devfrom
fix/filter-lookahead-empty-segment-panic

Conversation

@xdustinface

@xdustinface xdustinface commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

When a wallet boots already fully synced, start_download takes its early return before assigning processing_height, leaving it at the initial 0. The next block that extends the chain then drives try_create_lookahead_batches, which falls back to processing_height when no batches are active and creates a lookahead batch starting at height 0. Loading filters from height 0, which were never stored under checkpoint sync, reads an empty leading segment and panics in SegmentCache::get_items (debug) or returns sentinel filters as real data (release).

Set processing_height to the computed scan_start before the early-return check so the scan frontier is recorded on every path. The genesis case (committed_height == 0) keeps starting at the wallet birth height because scan_start already encodes that distinction.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed filter synchronization to properly track the scan frontier during initialization, ensuring correct processing of new filter headers.
  • Tests

    • Added validation for filter header processing from the correct frontier position.

When a wallet boots already fully synced, `start_download` takes its early return before assigning `processing_height`, leaving it at the initial 0. The next block that extends the chain then drives `try_create_lookahead_batches`, which falls back to `processing_height` when no batches are active and creates a lookahead batch starting at height 0. Loading filters from height 0, which were never stored under checkpoint sync, reads an empty leading segment and panics in `SegmentCache::get_items` (debug) or returns sentinel filters as real data (release).

Set `processing_height` to the computed `scan_start` before the early-return check so the scan frontier is recorded on every path. The genesis case (`committed_height == 0`) keeps starting at the wallet birth height because `scan_start` already encodes that distinction.
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e9a74587-4a5b-4691-9786-df7a48686a55

📥 Commits

Reviewing files that changed from the base of the PR and between ceee4a9 and ecea9ff.

📒 Files selected for processing (1)
  • dash-spv/src/sync/filters/manager.rs

📝 Walkthrough

Walkthrough

FiltersManager::start_download now assigns self.processing_height = scan_start before the early-return block, replacing the previous assignment that occurred after it. A new async regression test covers the already-synced boot path, asserting that a subsequent handle_new_filter_headers call produces lookahead batches starting from the frontier rather than height 0.

Changes

processing_height early-return fix

Layer / File(s) Summary
Move processing_height assignment before early-return and regression test
dash-spv/src/sync/filters/manager.rs
self.processing_height = scan_start is moved to before early-return conditions in start_download; the now-redundant later assignment is removed. A new async test boots the manager already synced, asserts processing_height advances, then calls handle_new_filter_headers for a new block and asserts no batch starts at height 0 and lookahead begins at the frontier.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 A frontier once lost at the start of the race,
Now planted before the return-early case.
The scan begins proper, no stale height of 0,
The lookahead leaps forward, as it's meant to do!
One small move of assignment, one hop for the chain. 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: preventing a crash that occurs when a new block arrives after full synchronization, which directly matches the core bug fix in the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/filter-lookahead-empty-segment-panic

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.01%. Comparing base (ceee4a9) to head (ecea9ff).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #816      +/-   ##
==========================================
+ Coverage   72.82%   73.01%   +0.18%     
==========================================
  Files         323      323              
  Lines       72000    72021      +21     
==========================================
+ Hits        52432    52583     +151     
+ Misses      19568    19438     -130     
Flag Coverage Δ
core 76.74% <ø> (ø)
ffi 47.34% <ø> (+1.33%) ⬆️
rpc 20.00% <ø> (ø)
spv 90.21% <100.00%> (+0.03%) ⬆️
wallet 71.80% <ø> (ø)
Files with missing lines Coverage Δ
dash-spv/src/sync/filters/manager.rs 97.67% <100.00%> (+0.17%) ⬆️

... and 23 files with indirect coverage changes

@github-actions github-actions Bot added the ready-for-review CodeRabbit has approved this PR label Jun 22, 2026
@xdustinface xdustinface requested a review from ZocoLini June 22, 2026 05:31
@xdustinface xdustinface merged commit 95a3c8f into dev Jun 22, 2026
40 checks passed
@xdustinface xdustinface deleted the fix/filter-lookahead-empty-segment-panic branch June 22, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review CodeRabbit has approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants