Skip to content

fix(dash-spv): stop waiting on a work-block list request no peer answers - #1272

Closed
PastaPastaPasta wants to merge 1 commit into
devfrom
pasta/spv-work-block-request-deadline
Closed

PastaPastaPasta wants to merge 1 commit into
devfrom
pasta/spv-work-block-request-deadline

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

The masternode manager asks for the masternode list at the work block of each new quorum. While any MnListDiff request is wanted, it starts no other masternode update (has_pending_requests), neither tip updates nor the catch-up in tick. The pipeline drops a wanted target only when its diff arrives, and the network broker keeps re-sending an unanswered request without end.

Some requests are never answered: Dash Core drops a getmnlistdiff it can't build without replying (src/net_processing.cpp:5621-5626). One such work-block request stops masternode-list updates until the process restarts.

This change:

  • records when each work-block list request was first queued, at the two places that queue them (the QRInfo handler and the tip-update work-block fetch);
  • makes tick drop work-block requests that have waited longer than WORK_BLOCK_LIST_DEADLINE (100 s, the length of the QRInfo retry ladder). For each one it logs a warning and calls request_answered, so the broker stops re-sending it. If nothing else is wanted, the pipeline completes as it does when the last diff arrives.

The quorum whose list was dropped stays unverified. The next QRInfo collects the requests for every unverified quorum again, so the list is asked for again then. Tip-update requests have no deadline and behave as before.

This is groundwork for the #1262 fix, which adds more work-block list requests.

Changelog

  • dash-spv stops waiting on a work-block masternode list request that no peer answers after 100 s, so masternode updates continue

Tests

  • an_unanswered_work_block_list_stops_blocking_updates (fails on dev): a tip update asks for a work-block list that is never answered. Nothing expires 1 s before the deadline. After it, tick answers the request, completes the pipeline and emits MasternodeStateUpdated, and the next new tip is requested.
  • only_work_block_requests_expire (pipeline): with a tip-update and a work-block request queued together, only the work-block one expires, and only after the deadline.
  • test_qrinfo_timeout_schedule now also checks that the deadline equals the QRInfo retry ladder.
  • The tests use tokio::time::pause/advance.
  • Locally: fmt, workspace clippy -D warnings (all features and targets), dash-spv clippy for x86_64-pc-windows-gnu, and all dash-spv tests, including the dashd regtest integration tests (dashd_sync, dashd_masternode).

🤖 Generated with Claude Code

PR Hygiene · ec3636e

  • Bots — coderabbitai not yet — /skip-bots proceeds without the ones not yet reported
  • Self-review — post /self-reviewed
  • Build running
  • Approvals
    • dash-spv (dash-spv/src/sync/masternodes/pipeline.rs, dash-spv/src/sync/masternodes/sync_manager.rs) — QuantumExplorer or ZocoLini or xdustinface

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

The masternode manager asks for the masternode lists at the work blocks of
new quorums, and while any MnListDiff request is wanted it starts no other
masternode update. A peer that cannot build the requested diff never
answers, and the broker re-sends the request without end, so masternode
updates stop for good.

Work-block list requests now record when they were first queued. Once one
has waited WORK_BLOCK_LIST_DEADLINE (100 s, the length of the QRInfo retry
ladder), tick drops it, tells the network to stop re-sending it and
completes the pipeline if nothing else is wanted. The quorum stays
unverified, and the next QRInfo asks for its list again. Tip-update
requests keep waiting as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between af5f267 and ec3636e.

Files selected for processing: 2
  • dash-spv/src/sync/masternodes/pipeline.rs
  • dash-spv/src/sync/masternodes/sync_manager.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Nov 1, 2026
@codecov

codecov Bot commented Nov 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.71%. Comparing base (af5f267) to head (ec3636e).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1272      +/-   ##
==========================================
- Coverage   80.73%   80.71%   -0.03%     
==========================================
  Files         291      291              
  Lines       77783    77880      +97     
==========================================
+ Hits        62801    62863      +62     
- Misses      14982    15017      +35     
Flag Coverage Δ
core 78.30% <ø> (ø)
ffi 58.20% <ø> (ø)
rpc 52.05% <ø> (ø)
spv 93.69% <100.00%> (-0.14%) ⬇️
wallet 82.89% <ø> (ø)
Files with missing lines Coverage Δ
dash-spv/src/sync/masternodes/pipeline.rs 100.00% <100.00%> (ø)
dash-spv/src/sync/masternodes/sync_manager.rs 99.36% <100.00%> (+0.09%) ⬆️

... and 6 files with indirect coverage changes

@xdustinface

Copy link
Copy Markdown
Collaborator

This duplicates #1258

@PastaPastaPasta

Copy link
Copy Markdown
Member Author

Thanks, you're right. Closing as a duplicate of #1258, which covers this with the wider fix.


🤖 Posted autonomously by Claude on behalf of pasta.

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

Labels

waiting-bots Waiting for the review bots to report on this head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants