fix: initialize everything correctly in FiltersManager::new

[xdustinface]

Read from FilterStorage and FilterHeaderStorage on construction and initialize properly so progress is accurate from the start.

Summary by CodeRabbit

• Bug Fixes

  • Filter manager initialization now properly handles and reports errors instead of silently ignoring failures.

• Improvements

  • Sync progress is automatically restored from persistent storage on startup, allowing the application to resume filtering from its previous state.

[coderabbitai]

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 46 seconds.

⌛ 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: 8e073771-50ff-410d-bac8-6260f0c13abb

📥 Commits

Reviewing files that changed from the base of the PR and between 347628c and 2445967.

📒 Files selected for processing (1)

• dash-spv/src/sync/filters/manager.rs

📝 Walkthrough

Walkthrough

The changes make FiltersManager::new fallible by returning a SyncResult<Self>, enabling it to read and restore persisted filter state during initialization. The lifecycle is updated to propagate errors from this async construction via .await?.

Changes

Cohort / File(s)	Summary
Client Lifecycle Error Propagation dash-spv/src/client/lifecycle.rs	Updated DashSpvClient::new to propagate FiltersManager construction errors via .await? instead of .await.
FiltersManager Initialization with State Restoration dash-spv/src/sync/filters/manager.rs	Constructor now returns SyncResult<Self>, reads persisted state from filter, block-header, and filter-header storages during initialization to restore stored_height, target_height, and filter_header_tip_height into FiltersProgress. Tests updated to handle fallible constructor; new test verifies state restoration from populated storage.

Sequence Diagram

sequenceDiagram
    participant Client as DashSpvClient::new
    participant FM as FiltersManager::new
    participant FS as FilterStorage
    participant HS as HeaderStorage
    participant FHS as FilterHeaderStorage
    participant FP as FiltersProgress

    Client->>FM: new(...)
    FM->>FS: filter_tip_height()
    FS-->>FM: stored_height
    FM->>HS: get_tip()
    HS-->>FM: target_height
    FM->>FHS: get_filter_tip_height()
    FHS-->>FM: filter_header_tip_height
    FM->>FP: Initialize with restored heights
    FP-->>FM: FiltersProgress
    FM-->>Client: Ok(FiltersManager)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A constructor learns to remember,
Reading whispers from storage's chamber,
Errors now flow, not silently hide,
Progress restored from each vault inside,
The manager awakens, restored and aware! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: fixing initialization in FiltersManager::new. It aligns with the PR's core objective of reading from storage and initializing progress correctly.
Docstring Coverage	✅ Passed	Docstring coverage is 80.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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/filters-manager-initialization

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

dash-spv/src/sync/filters/manager.rs (1)

85-113: Initialize next_batch_to_store with stored height for clearer semantics.

The field is currently initialized to 0 but is immediately overwritten in the sync method (line 204) with (stored_filters_tip + 1).max(header_start_height). Initializing it to the correct value in new() aligns with the field's semantic purpose and avoids redundant assignment.

Suggest initializing based on stored_height:

💡 Suggested initialization

         Ok(Self {
             progress: initial_progress,
             header_storage,
             filter_header_storage,
             filter_storage,
             wallet,
             filter_pipeline: FiltersPipeline::new(),
             pending_batches: BTreeSet::new(),
-            next_batch_to_store: 0,
+            next_batch_to_store: stored_height.saturating_add(1),
             // Multi-batch processing
             active_batches: BTreeMap::new(),
             processing_height: 0,
             blocks_remaining: BTreeMap::new(),
             filters_matched: HashSet::new(),
         })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dash-spv/src/sync/filters/manager.rs` around lines 85 - 113, In the new()
constructor for the Filters manager, initialize next_batch_to_store using the
computed stored_height instead of 0 so it reflects the real starting batch—set
next_batch_to_store to (stored_height + 1).max(header_start_height) (same logic
used later in sync) to avoid redundant reassignment; update the initialization
of next_batch_to_store in the struct construction and remove or simplify the
duplicate assignment in the sync method (which currently computes
(stored_filters_tip + 1).max(header_start_height)), referencing the
next_batch_to_store field, new() function, stored_height, and
header_start_height to locate where to apply the change.

🤖 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/filters/manager.rs`:
- Around line 85-113: In the new() constructor for the Filters manager,
initialize next_batch_to_store using the computed stored_height instead of 0 so
it reflects the real starting batch—set next_batch_to_store to (stored_height +
1).max(header_start_height) (same logic used later in sync) to avoid redundant
reassignment; update the initialization of next_batch_to_store in the struct
construction and remove or simplify the duplicate assignment in the sync method
(which currently computes (stored_filters_tip + 1).max(header_start_height)),
referencing the next_batch_to_store field, new() function, stored_height, and
header_start_height to locate where to apply the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3ddba4a8-7c09-4df3-af26-957e68b6880c

📥 Commits

Reviewing files that changed from the base of the PR and between 5004b52 and 347628c.

📒 Files selected for processing (2)

• dash-spv/src/client/lifecycle.rs
• dash-spv/src/sync/filters/manager.rs