refactor: split FiltersProgress.current_height into committed/stored

[xdustinface]

Split ambiguous current_height in FiltersProgress into two fields:

• committed_height: fully processed by wallet
• stored_height: downloaded to filter storage

Moves committed_height tracking from FiltersManager struct into FiltersProgress and update the relevant FFI code.

Based on:

• fix: separate filter scan height from synced height #442

Summary by CodeRabbit

• Refactor

  • Filter synchronization now tracks separate committed and stored heights instead of a single current height, improving clarity of sync status.
  • Progress output and logs now show both committed and stored heights for clearer visibility into processing versus stored tip.

• Tests

  • Sync progress tests updated to reflect the new committed/stored height representation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

This change replaces a single current_height with two explicit fields—committed_height and stored_height—and updates FFI types, core filters progress, manager/coordinator logic, CLI output, and tests to use the new fields and methods.

Changes

Cohort / File(s)	Summary
FFI Public Interface dash-spv-ffi/include/dash_spv_ffi.h, dash-spv-ffi/src/types.rs	Replaced current_height with committed_height and stored_height in FFI structs and From conversions.
FFI CLI & Tests dash-spv-ffi/src/bin/ffi_cli.rs, dash-spv-ffi/tests/test_types.rs	Updated CLI progress output and tests to print/validate committed_height and stored_height; tests adapt to new API (update_committed_height / update_stored_height).
Filters Progress Core dash-spv/src/sync/filters/progress.rs	Replaced private current_height with committed_height and stored_height; added getters and mutation methods; updated Display and ProgressPercentage to use committed_height/stored_height.
Filters Manager & Sync Logic dash-spv/src/sync/filters/manager.rs, dash-spv/src/sync/filters/sync_manager.rs, dash-spv/src/sync/sync_coordinator.rs	Removed committed_height field from manager; replaced direct field uses with progress.committed_height()/progress.stored_height(); switched calls from update_current_height to update_committed_height/update_stored_height; updated related logs and logic checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble at heights, now split in two,
Committed steps and stored views anew,
One carrot for progress, one clover for store,
Together they hop the sync path more! 🌿🧭

🚥 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 accurately summarizes the main refactoring change: splitting the ambiguous current_height field into two distinct fields (committed_height and stored_height) across the FiltersProgress struct and related code.
Docstring Coverage	✅ Passed	Docstring coverage is 82.14% 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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/filters-current-height

---

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.}

[xdustinface]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

144-161: ⚠️ Potential issue | 🔴 Critical

Bug: Format arguments are in the wrong order — percentage and stored_height are swapped.

The format string has ({:.1}%) followed by stored:{}, but the arguments supply self.stored_height (u32) for the percentage slot and pct (f64) for the stored slot. This will output something like Syncing 100/1000 (500%) stored:85.3 instead of the intended Syncing 100/1000 (85.3%) stored:500.

🐛 Proposed fix: swap the argument order

         write!(
             f,
             "{:?} {}/{} ({:.1}%) stored:{}, downloaded: {}, processed: {}, matched: {}, last_activity: {}s",
             self.state,
             self.committed_height,
             self.target_height,
-            self.stored_height,
             pct,
+            self.stored_height,
             self.downloaded,
             self.processed,
             self.matched,
             self.last_activity.elapsed().as_secs(),
         )

🤖 Prompt for AI Agents

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

In `@dash-spv/src/sync/filters/progress.rs` around lines 144 - 161, The Display
impl for FiltersProgress has the percentage and stored_height arguments swapped;
in fn fmt(&self, f: &mut fmt::Formatter<'_>) change the order of the arguments
passed to write! so that pct (from self.percentage() * 100.0) fills the ({:.1}%)
placeholder and self.stored_height fills the stored:{} placeholder—i.e., pass
self.state, self.committed_height, self.target_height, pct, self.stored_height,
self.downloaded, self.processed, self.matched,
self.last_activity.elapsed().as_secs() to write!.

🧹 Nitpick comments (1)

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

93-103: Consider adding monotonic guards to update_committed_height and update_stored_height.

update_target_height (line 107-112) enforces monotonic increase, but these new setters allow arbitrary values. The manager code in manager.rs line 500 manually checks end > self.progress.committed_height() before calling update_committed_height, suggesting the invariant exists but isn't enforced at the progress level. This is low-risk since callers currently handle it, but centralizing the guard would be more defensive.

🤖 Prompt for AI Agents

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

In `@dash-spv/src/sync/filters/progress.rs` around lines 93 - 103, Add monotonic
guards inside update_committed_height and update_stored_height so they only
change state when the new height is greater than the current value (mirror the
behavior of update_target_height). Specifically, in update_committed_height(&mut
self, height: u32) and update_stored_height(&mut self, height: u32) check the
existing self.committed_height and self.stored_height respectively and return
early if the incoming height is <= current; only assign the field and call
bump_last_activity() when the height strictly increases. This centralizes the
invariant and avoids relying on callers (e.g., manager.rs) to enforce
monotonicity.

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@dash-spv/src/sync/filters/progress.rs`:
- Around line 144-161: The Display impl for FiltersProgress has the percentage
and stored_height arguments swapped; in fn fmt(&self, f: &mut
fmt::Formatter<'_>) change the order of the arguments passed to write! so that
pct (from self.percentage() * 100.0) fills the ({:.1}%) placeholder and
self.stored_height fills the stored:{} placeholder—i.e., pass self.state,
self.committed_height, self.target_height, pct, self.stored_height,
self.downloaded, self.processed, self.matched,
self.last_activity.elapsed().as_secs() to write!.

---

Nitpick comments:
In `@dash-spv/src/sync/filters/progress.rs`:
- Around line 93-103: Add monotonic guards inside update_committed_height and
update_stored_height so they only change state when the new height is greater
than the current value (mirror the behavior of update_target_height).
Specifically, in update_committed_height(&mut self, height: u32) and
update_stored_height(&mut self, height: u32) check the existing
self.committed_height and self.stored_height respectively and return early if
the incoming height is <= current; only assign the field and call
bump_last_activity() when the height strictly increases. This centralizes the
invariant and avoids relying on callers (e.g., manager.rs) to enforce
monotonicity.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/filters-current-height

---

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.}