Skip to content

fix(dash-spv): handle buffered sync events during WaitingForConnections#768

Open
xdustinface wants to merge 3 commits into
v0.42-devfrom
fix/sync-event-waiting-for-connections
Open

fix(dash-spv): handle buffered sync events during WaitingForConnections#768
xdustinface wants to merge 3 commits into
v0.42-devfrom
fix/sync-event-waiting-for-connections

Conversation

@xdustinface
Copy link
Copy Markdown
Collaborator

Three independent fixes for a class of races where sync events buffered in the broadcast channel get delivered to a manager while it sits in SyncState::WaitingForConnections (between stop_sync and the next start_sync).

  • MasternodesManager::send_qrinfo_for_tip no longer mutates state before the network send. On Err("No connected peers") the manager stays in WaitingForConnections instead of getting stranded in Syncing with qrinfo_in_flight = None (which tick cannot recover).
  • ChainLockManager::handle_sync_event drops buffered MasternodeStateUpdated events while in WaitingForConnections. Prevents a peerless Synced transition. The event re-fires after reconnect via verify_and_complete / complete_incremental_pipeline.
  • InstantSendManager::handle_sync_event drops events the same way. Avoids running validate_pending against state that on_disconnect already cleared.

…til after send

A `BlockHeaderSyncComplete` event delivered while `MasternodesManager` is in `WaitingForConnections` reaches `send_qrinfo_for_tip` with no peers connected. The previous order set state to `Syncing` before `requests.request_qr_info(...)?` propagated `Err("No connected peers")`, leaving the manager in `Syncing` with `qrinfo_in_flight = None`. `tick` only retries when `qrinfo_in_flight.is_some()` so the manager was stuck until the next external event.

Reorder so the state mutation only happens after the send and the in-flight bookkeeping have committed. On send failure the manager remains in `WaitingForConnections` and the next `start_sync` (or buffered `BlockHeaderSyncComplete` after reconnect) cleanly retries.
…connections

`MasternodeStateUpdated` is a one-shot trigger that flips `masternode_ready` and forces `Synced`. A buffered event consumed while the manager is in `WaitingForConnections` (between `stop_sync` and the next `start_sync`) would force a peerless `Synced`, contradicting the network state.

Gate `handle_sync_event` on `WaitingForConnections`. `on_disconnect` already resets `masternode_ready`, and `MasternodesManager` re-emits `MasternodeStateUpdated` from both `verify_and_complete` and `complete_incremental_pipeline` on the next sync cycle after reconnect, so dropping the buffered event loses nothing.
…r connections

A buffered `MasternodeStateUpdated` consumed during `WaitingForConnections` would run `validate_pending` against state that `on_disconnect` already cleared. The existing `set_state(Synced)` was already gated on `Syncing | WaitForEvents` so state transitions were not affected, but the validation work itself is meaningless while peerless.

Gate `handle_sync_event` on `WaitingForConnections`. `MasternodesManager` re-emits `MasternodeStateUpdated` on the next sync cycle after reconnect.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Warning

Rate limit exceeded

@xdustinface has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 43 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 156184ba-147c-4f76-bf34-c144fe450f47

📥 Commits

Reviewing files that changed from the base of the PR and between 3281616 and fc3543d.

📒 Files selected for processing (5)
  • dash-spv/src/sync/chainlock/manager.rs
  • dash-spv/src/sync/chainlock/sync_manager.rs
  • dash-spv/src/sync/instantsend/manager.rs
  • dash-spv/src/sync/instantsend/sync_manager.rs
  • dash-spv/src/sync/masternodes/manager.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sync-event-waiting-for-connections

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 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.65%. Comparing base (5297d61) to head (fc3543d).
⚠️ Report is 5 commits behind head on v0.42-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##           v0.42-dev     #768      +/-   ##
=============================================
+ Coverage      72.26%   72.65%   +0.39%     
=============================================
  Files            320      320              
  Lines          70275    70292      +17     
=============================================
+ Hits           50785    51072     +287     
+ Misses         19490    19220     -270     
Flag Coverage Δ
core 76.30% <ø> (ø)
ffi 48.41% <ø> (+2.31%) ⬆️
rpc 20.00% <ø> (ø)
spv 90.07% <100.00%> (+0.31%) ⬆️
wallet 71.27% <ø> (+0.01%) ⬆️
Files with missing lines Coverage Δ
dash-spv/src/sync/chainlock/manager.rs 93.49% <100.00%> (+0.23%) ⬆️
dash-spv/src/sync/chainlock/sync_manager.rs 87.09% <100.00%> (ø)
dash-spv/src/sync/instantsend/manager.rs 93.20% <100.00%> (+0.34%) ⬆️
dash-spv/src/sync/instantsend/sync_manager.rs 88.46% <ø> (ø)
dash-spv/src/sync/masternodes/manager.rs 93.38% <100.00%> (+0.17%) ⬆️

... and 23 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant