Skip to content

fix: announce tip to new peers when synced - #490

Merged
xdustinface merged 1 commit into
v0.42-devfrom
fix/announce-tip-to-new-peers
Apr 10, 2026
Merged

fix: announce tip to new peers when synced#490
xdustinface merged 1 commit into
v0.42-devfrom
fix/announce-tip-to-new-peers

Conversation

@xdustinface

@xdustinfacexdustinface commented Mar 3, 2026

Copy link
Copy Markdown
Collaborator

When already synced, send a GetHeaders with our tip locator to each
newly connected peer. Without this, Dash Core never learns our best-known
header and falls back to inv announcements instead of pushing unsolicited
header messages.

Based on:

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced peer connection handling for block header synchronization, enabling more efficient tracking of header announcements across peer reconnections.
  • Bug Fixes

    • Prevents duplicate header requests from being sent to peers that have already received announcements after reconnection.
  • Performance

    • Improved synchronization efficiency by optimizing how the system manages peer-to-peer header announcements in synced state.

@coderabbitai

coderabbitaiBot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

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: 54541de3-1848-4ee4-9ff6-33bc8abf6238

📥 Commits

Reviewing files that changed from the base of the PR and between 47c94b7 and 329b303.

📒 Files selected for processing (3)
  • dash-spv/src/network/mod.rs
  • dash-spv/src/sync/block_headers/manager.rs
  • dash-spv/src/sync/block_headers/sync_manager.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • dash-spv/src/network/mod.rs
  • dash-spv/src/sync/block_headers/sync_manager.rs
  • dash-spv/src/sync/block_headers/manager.rs

📝 Walkthrough

Walkthrough

These changes implement peer-targeted block header announcements in synced state. A new network method enables requesting headers from specific peers, while the sync manager now tracks announced peers and handles peer connection events to request tips from newly connected peers only once per peer.

Changes

Cohort / File(s)Summary
Network Request Routing
dash-spv/src/network/mod.rs
Added request_block_headers_from_peer() public method to RequestSender for sending header requests to a specific peer address instead of broadcasting.
Sync Manager Peer Announcement
dash-spv/src/sync/block_headers/manager.rs
Added announced_peers hash set to track peers receiving post-sync header announcements. Modified progress bumping to occur only on matched headers. Enhanced test coverage with create_synced_manager() helper and tests validating peer announcement behavior (single request per peer, reconnect handling, disconnection cleanup).
Event-Driven Peer Handling
dash-spv/src/sync/block_headers/sync_manager.rs
Extended handle_network_event() to match on PeerConnected and PeerDisconnected events. On peer connection in synced state, conditionally requests headers for the current tip from the peer and records it in announced_peers. On disconnection, removes the peer to allow re-announcement on reconnect. Updated clear_in_flight_state() to reset announced_peers alongside other state.

Sequence Diagram

sequenceDiagram
participant Peer as Connected Peer
participant SyncMgr as BlockHeadersManager
participant Network as RequestSender
participant Pipeline as Headers Pipeline
Peer->>SyncMgr: NetworkEvent::PeerConnected
activate SyncMgr
alt Synced State & Pipeline Initialized & Peer Not Announced
SyncMgr->>SyncMgr: Check announced_peers
SyncMgr->>Network: request_block_headers_from_peer(tip_hash, address)
activate Network
Network->>Peer: GetHeaders (tip_hash → stop_hash)
deactivate Network
SyncMgr->>SyncMgr: Add peer to announced_peers
else Peer Already Announced or Not Synced
SyncMgr->>SyncMgr: Skip announcement
end
deactivate SyncMgr
Peer->>SyncMgr: NetworkEvent::PeerDisconnected
activate SyncMgr
SyncMgr->>SyncMgr: Remove peer from announced_peers
deactivate SyncMgr
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A whisker-twitch of joy!
New peers connect, and headers flow,
One question per peer, never twice—
We sync our tips with rabbity finesse,
And when they hop away, we're ready once more! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'fix: announce tip to new peers when synced' directly and clearly summarizes the main change: announcing the tip to newly connected peers when the node is already synced. This accurately captures the core objective of the PR.
Docstring Coverage✅ PassedDocstring 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/announce-tip-to-new-peers

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.

@github-actionsgithub-actionsBot added the merge-conflict The PR conflicts with the target branch. label Mar 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

@xdustinface
xdustinfaceforce-pushed the fix/announce-tip-to-new-peers branch from 527f938 to 47c94b7CompareApril 6, 2026 10:11
@github-actionsgithub-actionsBot removed the merge-conflict The PR conflicts with the target branch. label Apr 6, 2026
@xdustinface
xdustinface marked this pull request as ready for review April 6, 2026 10:12

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dash-spv/src/network/manager.rs`:
- Around line 1089-1099: The helper send_to_specific_peer is unused; replace the
duplicate inline logic in the request processor (where you call
pool.get_peer(...) and then call send_message_to_peer(...)) with a single call
to send_to_specific_peer(addr, message). Ensure you await the call and propagate
its NetworkResult (or map errors consistently), using the same addr and
NetworkMessage values, so you can then remove the dead send_to_specific_peer
implementation if you prefer deletion instead of reuse.
In `@dash-spv/src/network/tests.rs`:
- Around line 57-61: The test destructures NetworkRequest::SendMessageToPeer in
the wrong order: the enum is defined as SendMessageToPeer(NetworkMessage,
SocketAddr) so the pattern should extract (recv_msg, recv_addr) not (recv_addr,
recv_msg); update the pattern in the test (the match against
NetworkRequest::SendMessageToPeer) to bind the first element to recv_msg and the
second to recv_addr, then adjust the assertions so recv_addr is compared to addr
and recv_msg is matched against NetworkMessage::Verack.
In `@dash-spv/src/sync/block_headers/manager.rs`:
- Around line 367-493: The test module formatting was changed by cargo-fmt; run
rustfmt to apply consistent formatting: run `cargo fmt --all` (or use your
editor’s Rust formatting) and re-stage the modified files so the test functions
like test_peer_connected_when_synced_sends_getheaders,
test_peer_connected_when_synced_skips_already_announced,
test_peer_disconnected_removes_from_announced,
test_peer_connected_when_not_synced_does_nothing,
test_empty_headers_after_tip_announcement_is_harmless, and
test_peer_connected_skipped_during_active_catchup reflect the formatter’s
changes; ensure no logic was altered and commit the formatted files before
pushing.
- Around line 384-389: The match arm is destructuring
NetworkRequest::SendMessageToPeer in the wrong order: the enum is defined as
SendMessageToPeer(NetworkMessage, SocketAddr) so change the pattern in the match
on `request` to `NetworkRequest::SendMessageToPeer(_, target_addr)` and then
assert_eq!(target_addr, addr) (update any variable names accordingly) so the
SocketAddr is compared to `addr` in the block where this match exists in
manager.rs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 09f2b58a-fbb8-46af-b593-db00752248c5

📥 Commits

Reviewing files that changed from the base of the PR and between dda1db7 and 47c94b7.

📒 Files selected for processing (5)
  • dash-spv/src/network/manager.rs
  • dash-spv/src/network/mod.rs
  • dash-spv/src/network/tests.rs
  • dash-spv/src/sync/block_headers/manager.rs
  • dash-spv/src/sync/block_headers/sync_manager.rs

Comment threaddash-spv/src/network/manager.rs Outdated
Comment threaddash-spv/src/network/tests.rs Outdated
Comment threaddash-spv/src/sync/block_headers/manager.rs Outdated
Comment threaddash-spv/src/sync/block_headers/manager.rs Outdated
When already synced, send a `GetHeaders` with our tip locator to each
newly connected peer. Without this, Dash Core never learns our best-known
header and falls back to inv announcements instead of pushing unsolicited
header messages.
@xdustinface
xdustinfaceforce-pushed the fix/announce-tip-to-new-peers branch from 47c94b7 to 329b303CompareApril 6, 2026 10:58
@codecov

codecovBot commented Apr 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.79518% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.80%. Comparing base (dda1db7) to head (329b303).
⚠️ Report is 7 commits behind head on v0.42-dev.

Files with missing linesPatch %Lines
dash-spv/src/sync/block_headers/manager.rs98.55%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v0.42-dev #490 +/- ##
=============================================
+ Coverage 67.57% 67.80% +0.22% 
=============================================
Files 318 318 Lines 67830 67911 +81 =============================================
+ Hits 45835 46044 +209 + Misses 21995 21867 -128 
FlagCoverage Δ
core75.21% <ø> (ø)
ffi38.42% <ø> (+0.86%)⬆️
rpc19.92% <ø> (ø)
spv85.12% <98.79%> (+0.35%)⬆️
wallet67.57% <ø> (ø)
Files with missing linesCoverage Δ
dash-spv/src/network/mod.rs76.00% <100.00%> (+3.27%)⬆️
dash-spv/src/sync/block_headers/sync_manager.rs85.00% <100.00%> (+0.38%)⬆️
dash-spv/src/sync/block_headers/manager.rs88.16% <98.55%> (+3.63%)⬆️

... and 18 files with indirect coverage changes

@github-actionsgithub-actionsBot added the ready-for-review CodeRabbit has approved this PR label Apr 6, 2026
@xdustinface

Copy link
Copy Markdown
CollaboratorAuthor

@CodeRabbit review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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
xdustinface merged commit a7808ca into v0.42-devApr 10, 2026
39 of 40 checks passed
@xdustinface
xdustinface deleted the fix/announce-tip-to-new-peers branch April 10, 2026 12:32
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
xdustinface added a commit to xdustinface/rust-dashcore that referenced this pull request Apr 11, 2026
When already synced, send a `GetHeaders` with our tip locator to each
newly connected peer. Without this, Dash Core never learns our best-known
header and falls back to inv announcements instead of pushing unsolicited
header messages.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-reviewCodeRabbit has approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@xdustinface@ZocoLini