Skip to content

fix(dash-spv): fire catch-up QRInfo past Incremental mining window#743

Merged
xdustinface merged 1 commit into
v0.42-devfrom
fix/catchup-qrinfo
May 9, 2026
Merged

fix(dash-spv): fire catch-up QRInfo past Incremental mining window#743
xdustinface merged 1 commit into
v0.42-devfrom
fix/catchup-qrinfo

Conversation

@xdustinface
Copy link
Copy Markdown
Collaborator

@xdustinface xdustinface commented May 8, 2026

When a header batch lands during an in-flight Incremental MnListDiff, intermediate BlockHeadersStored events get dropped by the has_pending_requests guard, and the tick's current_height < block_header_tip_height check is false once Incremental catches up. If the cycle's mining window opens and closes inside that batch, the catch-up branch in next_pipeline_mode never fires and the cycle's rotated quorums stay unvalidated until a later block extends past the window.

Re-evaluate next_pipeline_mode at the advanced tip when Incremental completes and fire the QRInfo immediately if the gate picks QuorumValidation. Fixes a race in test_masternode_list_sync_with_quorum_rotation where dashd delivered headers in two batches straddling the third DKG cycle's mining window.

Summary by CodeRabbit

  • Bug Fixes
    • Improved masternodes synchronization reliability with better catch-up handling during pipeline transitions
    • Enhanced quorum validation retry logic to prevent missed synchronization windows
    • Strengthened edge-case test coverage for synchronization scenarios

When a header batch lands during an in-flight `Incremental` `MnListDiff`, intermediate `BlockHeadersStored` events get dropped by the `has_pending_requests` guard, and the tick's `current_height < block_header_tip_height` check is false once `Incremental` catches up. If the cycle's mining window opens and closes inside that batch, the catch-up branch in `next_pipeline_mode` never fires and the cycle's rotated quorums stay unvalidated until a later block extends past the window.

Re-evaluate `next_pipeline_mode` at the advanced tip when `Incremental` completes and fire the QRInfo immediately if the gate picks `QuorumValidation`. Fixes a race in `test_masternode_list_sync_with_quorum_rotation` where `dashd` delivered headers in two batches straddling the third DKG cycle's mining window.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

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: 25c79c7c-89e8-4528-bbda-63810e0ffda7

📥 Commits

Reviewing files that changed from the base of the PR and between 49c8c6d and 878f21f.

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

📝 Walkthrough

Walkthrough

This PR extends MasternodesManager::complete_pipeline to accept a RequestSender parameter and implements catch-up QRInfo dispatch logic. After an Incremental pipeline completes, the manager re-evaluates the cycle gate at the current tip; if QuorumValidation is selected, it dispatches a catch-up QRInfo request. Four call sites in sync_manager are updated to thread the RequestSender through all completion paths.

Changes

Incremental Pipeline Catch-up QRInfo Dispatch

Layer / File(s) Summary
Core Implementation
dash-spv/src/sync/masternodes/manager.rs
complete_pipeline signature updated to accept &RequestSender. Incremental case now re-evaluates cycle gate at current tip; if QuorumValidation is selected, dispatches catch-up QRInfo via send_qrinfo_for_tip, resetting qrinfo_retry_count and clearing pending state. On dispatch failure, logs warning and preserves cycle-attempt behavior.
Test Infrastructure
dash-spv/src/sync/masternodes/manager.rs
Test module imports extended with message/request types. New helper make_synced_incremental_manager(tip) constructs a manager in Synced state with pipeline_mode = Incremental, preloads dummy headers, and returns RequestSender/receiver pair for network request assertions.
Test Cases
dash-spv/src/sync/masternodes/manager.rs
Two tests verify catch-up behavior: (1) after Incremental completion, gate re-evaluation bumps rotation_cycles and enqueues GetQRInfo when gate selects QuorumValidation; (2) no QRInfo enqueued when gate selects Incremental.
Call Site Integration
dash-spv/src/sync/masternodes/sync_manager.rs
Four completion call paths updated to pass requests parameter: after QRInfo processing fast-path, after MnListDiff completion, when QRInfo times out with retries exhausted, and when MnListDiff completes during tick().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A whisker's twitch, a catch-up quest,
When pipelines shift and gates digest,
QRInfo flies if validation's chosen—
No stone-cold delay shall leave us frozen! 🐰✨

🚥 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 'fix(dash-spv): fire catch-up QRInfo past Incremental mining window' directly and specifically describes the main change: enabling catch-up QRInfo dispatch after Incremental pipeline completion when the mining window is missed.
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/catchup-qrinfo

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 May 8, 2026

Codecov Report

❌ Patch coverage is 96.87500% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.40%. Comparing base (49c8c6d) to head (878f21f).
⚠️ Report is 1 commits behind head on v0.42-dev.

Files with missing lines Patch % Lines
dash-spv/src/sync/masternodes/manager.rs 96.87% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           v0.42-dev     #743      +/-   ##
=============================================
+ Coverage      71.31%   71.40%   +0.09%     
=============================================
  Files            321      321              
  Lines          70339    70401      +62     
=============================================
+ Hits           50160    50273     +113     
+ Misses         20179    20128      -51     
Flag Coverage Δ
core 76.68% <ø> (ø)
ffi 44.97% <ø> (ø)
rpc 20.00% <ø> (ø)
spv 87.89% <96.87%> (+0.37%) ⬆️
wallet 69.85% <ø> (ø)
Files with missing lines Coverage Δ
dash-spv/src/sync/masternodes/sync_manager.rs 70.19% <ø> (+0.99%) ⬆️
dash-spv/src/sync/masternodes/manager.rs 82.51% <96.87%> (+14.58%) ⬆️

... and 8 files with indirect coverage changes

@xdustinface
Copy link
Copy Markdown
Collaborator Author

xdustinface commented May 8, 2026

@CodeRabbit review!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

@xdustinface Sure! I'll review the changes in this PR now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@xdustinface
Copy link
Copy Markdown
Collaborator Author

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions github-actions Bot added the ready-for-review CodeRabbit has approved this PR label May 8, 2026
@xdustinface xdustinface merged commit 34f574d into v0.42-dev May 9, 2026
64 of 65 checks passed
@xdustinface xdustinface deleted the fix/catchup-qrinfo branch May 9, 2026 00:32
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