fix(dash-spv): extend the CFHeaders queue from the tick, not only on a header event - #964
Draft
romchornyi wants to merge 1 commit into
Draft
fix(dash-spv): extend the CFHeaders queue from the tick, not only on a header event#964romchornyi wants to merge 1 commit into
romchornyi wants to merge 1 commit into
Conversation
…a header event `handle_new_headers` is the only thing that ever extends the filter-header queue, and it runs solely off `BlockHeaderSyncComplete` and `BlockHeadersStored`. Block-header storage can advance without either reaching this manager: a segment completing out of order promotes a run of buffered headers, and on a long scan that is how the last stretch of the chain lands. When that happens the manager keeps the target it was last told about, its queue drains, and nothing re-arms it. Filter headers then stop for good while block headers, ChainLocks and inv announcements carry on — so the client looks alive while sync is frozen, which is what makes this hard to spot from the outside. Observed on a mainnet restore: the queue was last extended to height 2_398_000 at 19:12:58, block headers reached 2_523_515 at 19:36:48, and filter headers never moved again — filters and blocks stuck at 95% with `last_activity` climbing past twenty minutes. The tick now re-reads the tip from storage and calls `handle_new_headers` when it has moved past what this manager knows. Same shape as promoting finished header segments from the tick (#960): trust the tick, not the message. The regression test drives exactly that sequence — storage advances with no event delivered — and fails without this change.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #964 +/- ##
=======================================
Coverage 76.89% 76.90%
=======================================
Files 329 329
Lines 82332 82372 +40
=======================================
+ Hits 63312 63347 +35
- Misses 19020 19025 +5
|
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.
Issue being fixed or feature implemented
Filter-header sync can stop permanently mid-scan and never resume, while the client keeps looking healthy.
handle_new_headersis the only thing that ever extends the CFHeaders queue, and it runs solely offBlockHeaderSyncCompleteandBlockHeadersStored. Block-header storage can advance without either reaching this manager — a segment completing out of order promotes a run of buffered headers, and on a long scan that is how the last stretch of the chain lands. When that happens the manager keeps the target it was last told about, its queue drains, and nothing re-arms it.Block headers, ChainLocks and inv announcements carry on normally, so from the outside the client looks alive while sync is frozen.
Observed on a mainnet restore:
Segment 38: complete)Filters and blocks sat at 95% with
last_activityclimbing past twenty minutes. The wallet's transactions stop arriving at that point, because the persisted watermark cannot advance past the filter frontier.Same shape as #960 (promote finished header segments from the tick, not only on a message), in the neighbouring pipeline.
What was done?
tick()re-reads the block-header tip from storage and callshandle_new_headerswhen it has moved past what this manager was last told. Addedstored_block_header_tip()on the manager for that read — the storage handle is private to the sibling module, and this keeps it that way rather than widening the field.No change to the event paths; this is purely an additional way in.
How Has This Been Tested?
test_tick_extends_when_storage_tip_advanced_without_an_eventdrives exactly the failing sequence: a manager mid-sync that was last told the tip was 1000, storage advanced past that with no event delivered, then onetick.The test is falsifiable and was checked both ways:
sync_manager.rsstashed — fails withtick must pick up a tip that advanced without an eventFull
dash-spvlib suite: 556 passed, 0 failed, 2 ignored.Not yet verified end-to-end on mainnet. The mechanism is covered by the unit test, and an app-level run over a patched local checkout completed normally, but that run was on testnet — the network where the stall was originally observed has not been re-run against this build. Keeping this as a draft for that reason.
Breaking Changes
None. Additive: one new private-to-module accessor and an extra branch in
tick.Checklist
For repository code-owners and collaborators only