Skip to content

fix: gate FilterHeadersSyncComplete on block header sync completion#631

Merged
xdustinface merged 1 commit into
v0.42-devfrom
fix/filter-headers-sync-ordering
Apr 6, 2026
Merged

fix: gate FilterHeadersSyncComplete on block header sync completion#631
xdustinface merged 1 commit into
v0.42-devfrom
fix/filter-headers-sync-ordering

Conversation

@xdustinface
Copy link
Copy Markdown
Collaborator

@xdustinface xdustinface commented Apr 6, 2026

Without this gate, FilterHeadersSyncComplete could fire before BlockHeaderSyncComplete when filter headers caught up to incrementally stored block headers. This caused downstream sync phases (filters, blocks) to start before all headers were available, leading to sync stalls.

Add block_headers_synced flag to FilterHeadersManager that is set only when BlockHeaderSyncComplete is received. Extract try_complete_sync helper to centralize the completion predicate across all four emission points.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Refined synchronization completion logic to enforce proper sequencing of operations, ensuring all prerequisites are satisfied before sync completion is confirmed.
    • Added validation to prevent incomplete operations from being incorrectly marked as finished.
  • Tests

    • Extended test coverage for synchronization completion with new tests validating edge cases, dependency satisfaction, and proper state transitions.

Without this gate, `FilterHeadersSyncComplete` could fire before
`BlockHeaderSyncComplete` when filter headers caught up to incrementally
stored block headers. This caused downstream sync phases (filters,
blocks) to start before all headers were available, leading to sync
stalls.

Add `block_headers_synced` flag to `FilterHeadersManager` that is set
only when `BlockHeaderSyncComplete` is received. Extract
`try_complete_sync` helper to centralize the completion predicate across
all four emission points.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 6, 2026

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: e84b2e47-349b-483a-9f32-63684dc2cd7c

📥 Commits

Reviewing files that changed from the base of the PR and between dda1db7 and 26ecb2d.

📒 Files selected for processing (2)
  • dash-spv/src/sync/filter_headers/manager.rs
  • dash-spv/src/sync/filter_headers/sync_manager.rs

📝 Walkthrough

Walkthrough

Added a block_headers_synced boolean field to FilterHeadersManager and introduced try_complete_sync() method to gate filter headers sync completion on both block headers sync completion and target height progress. Refactored sync completion logic to use centralized method instead of inline checks.

Changes

Cohort / File(s) Summary
Synchronization Gate Logic
dash-spv/src/sync/filter_headers/manager.rs, dash-spv/src/sync/filter_headers/sync_manager.rs
Added block_headers_synced field to track block header sync completion. Introduced try_complete_sync() method that only emits FilterHeadersSyncComplete when both block_headers_synced is true and progress reaches target height. Refactored both files to delegate sync completion decisions to this centralized method and properly manage the flag based on BlockHeaderSyncComplete events.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops in sync with golden code,
Block headers pave the wavy road,
Filter frames now wait their turn,
Till both flags in hearts do burn!
One true gate, no race today,
We hop together, synchronized way!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 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: adding a gate to FilterHeadersSyncComplete that depends on block header sync completion, which matches the core fix in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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-headers-sync-ordering

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
Copy link
Copy Markdown

codecov Bot commented Apr 6, 2026

Codecov Report

❌ Patch coverage is 97.70115% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.76%. Comparing base (dda1db7) to head (26ecb2d).
⚠️ Report is 2 commits behind head on v0.42-dev.

Files with missing lines Patch % Lines
dash-spv/src/sync/filter_headers/manager.rs 97.67% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           v0.42-dev     #631      +/-   ##
=============================================
+ Coverage      67.57%   67.76%   +0.18%     
=============================================
  Files            318      318              
  Lines          67830    67896      +66     
=============================================
+ Hits           45835    46007     +172     
+ Misses         21995    21889     -106     
Flag Coverage Δ
core 75.21% <ø> (ø)
ffi 38.42% <ø> (+0.86%) ⬆️
rpc 19.92% <ø> (ø)
spv 84.94% <97.70%> (+0.18%) ⬆️
wallet 67.57% <ø> (ø)
Files with missing lines Coverage Δ
dash-spv/src/sync/filter_headers/sync_manager.rs 100.00% <100.00%> (ø)
dash-spv/src/sync/filter_headers/manager.rs 90.70% <97.67%> (+11.20%) ⬆️

... and 15 files with indirect coverage changes

@github-actions github-actions Bot added the ready-for-review CodeRabbit has approved this PR label Apr 6, 2026
@xdustinface xdustinface requested a review from ZocoLini April 6, 2026 09:02
@xdustinface xdustinface merged commit 1e75084 into v0.42-dev Apr 6, 2026
40 checks passed
@xdustinface xdustinface deleted the fix/filter-headers-sync-ordering branch April 6, 2026 23:53
shumkov added a commit that referenced this pull request Apr 11, 2026
wallet-run had been rebased onto newer v0.42-dev than wallet2; this
merge brings in the upstream fixes, plus resolves the mirror-commit
overlap on key-wallet. No branch-specific logic — every wallet-run
key-wallet commit has an identical mirror on wallet2.

v0.42-dev upstream bits:
  - fix: announce tip to new peers when synced (#490)
  - fix: subscribe to SPV event monitors before startup (#636)
  - refactor: unify logging on tracing (#635)
  - chore: cleanup unused dependencies (#633)
  - fix: process broadcast transactions via dispatch_local (#626)
  - fix: gate FilterHeadersSyncComplete on block header sync (#631)
  - refactor: use String for TransactionRecord::label (#624)

# Conflicts:
#	key-wallet/src/managed_account/transaction_record.rs
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