fix: avoid extra GetHeaders after post-sync header processing#486
Merged
xdustinface merged 1 commit intov0.42-devfrom Mar 4, 2026
Merged
fix: avoid extra GetHeaders after post-sync header processing#486xdustinface merged 1 commit intov0.42-devfrom
GetHeaders after post-sync header processing#486xdustinface merged 1 commit intov0.42-devfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThese changes add tip-complete state tracking and gating in block header sync to avoid unsolicited post‑sync requests, introduce a duplicate‑header guard, new pipeline helpers ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Manager as BlockHeadersManager
participant Pipeline as HeadersPipeline
participant Network as NetworkRequest
Client->>Manager: receive_headers(headers)
activate Manager
Manager->>Pipeline: receive_headers(headers)
activate Pipeline
alt Duplicate header (matches segment tip)
Pipeline-->>Manager: None (ignored)
else New headers
Pipeline->>Pipeline: buffer/process headers
Pipeline-->>Manager: Some(events)
end
deactivate Pipeline
Manager->>Manager: tip_was_complete = Pipeline.is_tip_complete()
alt Not syncing AND tip_was_complete
Manager->>Manager: mark_tip_complete()
Note over Manager: prepare state for next unsolicited header
else Syncing OR tip not complete
Manager->>Network: send_request(GetHeaders)
activate Network
Network-->>Manager: Request sent
deactivate Network
end
deactivate Manager
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/sync/block_headers/pipeline.rs`:
- Around line 195-201: The current guard in the pipeline that checks
headers[0].block_hash() against self.segments' current_tip_hash and returns
Ok(None) drops valid trailing headers when the batch overlaps; change it to skip
only the duplicate prefix: iterate over headers from the front,
advancing/trimming while header.block_hash() matches any s.current_tip_hash in
self.segments, then continue processing the remaining headers; if trimming
consumes the entire batch, then return Ok(None). Update the logic around the
existing first_hash / self.segments.iter().any(...) check in pipeline.rs so it
trims the duplicate prefix (using headers slicing/drain) instead of rejecting
the whole headers vector.
In `@dash/src/test_utils/block.rs`:
- Around line 47-57: The dummy_chain helper currently hardcodes consensus bits
(0x2100ffff) when constructing Header.bits; change dummy_chain to accept a bits
parameter (e.g., bits: u32 or CompactTarget) or retrieve the value from the
test/network params and pass it into the Header construction so Header { ...,
bits: CompactTarget::from_consensus(bits), ... } instead of the literal; update
all callers of dummy_chain to provide the appropriate bits (or wrap a
convenience overload that uses the network default) and ensure
CompactTarget::from_consensus is used consistently.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dash-spv/src/sync/block_headers/manager.rsdash-spv/src/sync/block_headers/pipeline.rsdash/src/test_utils/block.rs
When synced, `handle_headers_pipeline` always calls `send_pending()` after receiving each new block header, triggering a follow-up `GetHeaders` that always returned empty. This causes lot of unnecessary round-trips and leaves the tip segment in a pending state that rejected the next actual header as "unrequested headers". Use `tip_was_complete` to distinguish unsolicited headers from active catch-up responses. Only skip `send_pending()` and mark the tip complete for the unsolicited case, so multi-batch catch-up still works correctly.
xdustinface
force-pushed
the
fix/avoid-tip-header-requests
branch
from
ee275b7 to
7acd86f
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dash-spv/src/sync/block_headers/manager.rs (1)
313-317: Consider extracting duplicated test helper.
create_test_request_senderis duplicated in bothpipeline.rs(lines 348-352) and here. Consider extracting to a shared test utilities module to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dash-spv/src/sync/block_headers/manager.rs` around lines 313 - 317, The helper function create_test_request_sender is duplicated between manager.rs and pipeline.rs; extract it into a shared test utilities module (e.g., tests::utils or a new crate-level tests_helpers module) and update both locations to import and call that single helper; ensure the new module returns (RequestSender, tokio::sync::mpsc::UnboundedReceiver<NetworkRequest>) and constructs the channel with unbounded_channel() so RequestSender::new(tx) behavior is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@dash-spv/src/sync/block_headers/manager.rs`:
- Around line 313-317: The helper function create_test_request_sender is
duplicated between manager.rs and pipeline.rs; extract it into a shared test
utilities module (e.g., tests::utils or a new crate-level tests_helpers module)
and update both locations to import and call that single helper; ensure the new
module returns (RequestSender,
tokio::sync::mpsc::UnboundedReceiver<NetworkRequest>) and constructs the channel
with unbounded_channel() so RequestSender::new(tx) behavior is preserved.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dash-spv/src/sync/block_headers/manager.rsdash-spv/src/sync/block_headers/pipeline.rsdash/src/test_utils/block.rs
ZocoLini
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When synced,
handle_headers_pipelinealways callssend_pending()after receiving each new block header, triggering a follow-upGetHeadersthat always returned empty. This causes lot of unnecessary round-trips and leaves the tip segment in a pending state that rejected the next actual header as "unrequested headers".Use
tip_was_completeto distinguish unsolicited headers from active catch-up responses. Only skipsend_pending()and mark the tip complete for the unsolicited case, so multi-batch catch-up still works correctly.Summary by CodeRabbit
New Features
Bug Fixes
Tests