fix: consolidate duplicate FFISyncProgress destroy functions

[xdustinface]

dash_spv_ffi_sync_progress_destroy didn't cleanup the nested progress pointers which leads to leaks on cleanup. This PR merges dash_spv_ffi_sync_progress_destroy into dash_spv_ffi_manager_sync_progress_destroy.

Summary by CodeRabbit

• Refactor

  • Renamed the synchronization progress cleanup function for consistent public API naming.
  • Consolidated duplicate manager-specific cleanup into the single, renamed public cleanup function, slightly reducing the documented API surface.

• Documentation

  • Updated API docs and examples to reference the renamed cleanup function for correct usage.

[coderabbitai]

📝 Walkthrough

Walkthrough

Public FFI destructor for FFISyncProgress was renamed from dash_spv_ffi_manager_sync_progress_destroy to dash_spv_ffi_sync_progress_destroy, moved into the types module, and usages/docs/header updated; the new implementation also frees nested progress pointers before dropping the parent.

Changes

Cohort / File(s)	Summary
API Doc & Header dash-spv-ffi/FFI_API.md, dash-spv-ffi/include/dash_spv_ffi.h	Updated documentation and C header to remove the manager-specific destroy entry and reference dash_spv_ffi_sync_progress_destroy. Total public function count adjusted.
Types (destructor impl) dash-spv-ffi/src/types.rs	Renamed/moved public extern "C" function to dash_spv_ffi_sync_progress_destroy; implementation now frees nested progress pointers (headers, filter_headers, filters, blocks, masternodes, chainlocks, instantsend) before dropping the outer FFISyncProgress.
Client & Call Sites dash-spv-ffi/src/client.rs, dash-spv-ffi/src/callbacks.rs, dash-spv-ffi/tests/test_types.rs	Removed the old manager-specific destructor from client; updated imports and calls to use dash_spv_ffi_sync_progress_destroy in callbacks, client docs, and tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through docs and header lines,
A single name now brightly shines.
Nested bits I gently freed,
Neat and tidy — done with speed.
Hooray, the progress destructor aligns!

🚥 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 describes the main change: consolidating duplicate FFISyncProgress destroy functions, which is the primary objective of this PR.
Docstring Coverage	✅ Passed	Docstring 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 (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/consolidate-progress-destroy

---

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]

Caution

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

⚠️ Outside diff range comments (1)

dash-spv-ffi/src/client.rs (1)

408-423: ⚠️ Potential issue | 🟡 Minor

dash_spv_ffi_client_get_sync_progress is missing a destroy hint in its doc comment.

dash_spv_ffi_client_get_manager_sync_progress (Line 428) was updated in this PR to reference dash_spv_ffi_sync_progress_destroy, but the sibling function dash_spv_ffi_client_get_sync_progress returns the same *mut FFISyncProgress type without mentioning how to free it.

📝 Suggested doc update

 /// Get the current sync progress snapshot.
 ///
+/// Use `dash_spv_ffi_sync_progress_destroy` to free the returned struct.
+///
 /// # Safety
 /// - `client` must be a valid, non-null pointer.

The same one-liner should also be added to the corresponding declaration in dash-spv-ffi/include/dash_spv_ffi.h (around line 511–514).

🤖 Prompt for AI Agents

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

In `@dash-spv-ffi/src/client.rs` around lines 408 - 423, Add the missing "destroy"
hint to the doc comment for dash_spv_ffi_client_get_sync_progress stating that
the returned *mut FFISyncProgress must be freed with
dash_spv_ffi_sync_progress_destroy (mirror the comment added to
dash_spv_ffi_client_get_manager_sync_progress); also update the corresponding
declaration comment in the C header (dash_spv_ffi.h) for the client function to
reference dash_spv_ffi_sync_progress_destroy so both Rust export and C header
consistently document how to free the FFISyncProgress pointer.

🤖 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-ffi/src/client.rs`:
- Around line 408-423: Add the missing "destroy" hint to the doc comment for
dash_spv_ffi_client_get_sync_progress stating that the returned *mut
FFISyncProgress must be freed with dash_spv_ffi_sync_progress_destroy (mirror
the comment added to dash_spv_ffi_client_get_manager_sync_progress); also update
the corresponding declaration comment in the C header (dash_spv_ffi.h) for the
client function to reference dash_spv_ffi_sync_progress_destroy so both Rust
export and C header consistently document how to free the FFISyncProgress
pointer.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26e7bc4 and 979e43b.

📒 Files selected for processing (6)

• dash-spv-ffi/FFI_API.md
• dash-spv-ffi/include/dash_spv_ffi.h
• dash-spv-ffi/src/callbacks.rs
• dash-spv-ffi/src/client.rs
• dash-spv-ffi/src/types.rs
• dash-spv-ffi/tests/test_types.rs

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (2)

dash-spv-ffi/FFI_API.md (1)

476-489: ⚠️ Potential issue | 🟡 Minor

dash_spv_ffi_client_get_sync_progress detailed section is missing the free-function reference.

The dash_spv_ffi_client_get_manager_sync_progress entry (line 467) was updated to mention dash_spv_ffi_sync_progress_destroy, but the dash_spv_ffi_client_get_sync_progress entry (lines 476-489) has no equivalent note. Both functions return an owned *mut FFISyncProgress that requires the same cleanup call.

📝 Suggested doc update

 **Description:**
-Get the current sync progress snapshot.  # Safety - `client` must be a valid, non-null pointer.
+Get the current sync progress snapshot.  Use `dash_spv_ffi_sync_progress_destroy` to free the returned struct.  # Safety - `client` must be a valid, non-null pointer.

🤖 Prompt for AI Agents

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

In `@dash-spv-ffi/FFI_API.md` around lines 476 - 489, Update the documentation for
dash_spv_ffi_client_get_sync_progress to match the manager variant: state that
the returned pointer is owned and must be freed with
dash_spv_ffi_sync_progress_destroy; specifically, add a Safety/Notes sentence
near the function signature (as done for
dash_spv_ffi_client_get_manager_sync_progress) indicating callers must call
dash_spv_ffi_sync_progress_destroy on the returned *mut FFISyncProgress to avoid
leaks and ensure consistent docs for the functions
dash_spv_ffi_client_get_sync_progress and
dash_spv_ffi_client_get_manager_sync_progress.

dash-spv-ffi/src/client.rs (1)

408-423: ⚠️ Potential issue | 🟡 Minor

dash_spv_ffi_client_get_sync_progress is missing the free-function doc.

get_manager_sync_progress (line 428) was updated to document dash_spv_ffi_sync_progress_destroy, but get_sync_progress returns the same heap-allocated *mut FFISyncProgress with identical nested pointer allocation and has no such hint. Without it, callers have no indication they must call the destroy function.

📝 Suggested doc update

 /// Get the current sync progress snapshot.
 ///
+/// Use `dash_spv_ffi_sync_progress_destroy` to free the returned struct.
+///
 /// # Safety
 /// - `client` must be a valid, non-null pointer.

🤖 Prompt for AI Agents

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

In `@dash-spv-ffi/src/client.rs` around lines 408 - 423,
dash_spv_ffi_client_get_sync_progress returns a heap-allocated *mut
FFISyncProgress but its docstring does not mention the required free function;
update the doc comment on dash_spv_ffi_client_get_sync_progress (like
get_manager_sync_progress) to state that the returned FFISyncProgress must be
freed with dash_spv_ffi_sync_progress_destroy and mention ownership transfer
semantics for FFISyncProgress::from so callers know to call the destroy function
to avoid leaks.

🤖 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-ffi/FFI_API.md`:
- Around line 476-489: Update the documentation for
dash_spv_ffi_client_get_sync_progress to match the manager variant: state that
the returned pointer is owned and must be freed with
dash_spv_ffi_sync_progress_destroy; specifically, add a Safety/Notes sentence
near the function signature (as done for
dash_spv_ffi_client_get_manager_sync_progress) indicating callers must call
dash_spv_ffi_sync_progress_destroy on the returned *mut FFISyncProgress to avoid
leaks and ensure consistent docs for the functions
dash_spv_ffi_client_get_sync_progress and
dash_spv_ffi_client_get_manager_sync_progress.

In `@dash-spv-ffi/src/client.rs`:
- Around line 408-423: dash_spv_ffi_client_get_sync_progress returns a
heap-allocated *mut FFISyncProgress but its docstring does not mention the
required free function; update the doc comment on
dash_spv_ffi_client_get_sync_progress (like get_manager_sync_progress) to state
that the returned FFISyncProgress must be freed with
dash_spv_ffi_sync_progress_destroy and mention ownership transfer semantics for
FFISyncProgress::from so callers know to call the destroy function to avoid
leaks.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 979e43b and 24ee468.

📒 Files selected for processing (6)

• dash-spv-ffi/FFI_API.md
• dash-spv-ffi/include/dash_spv_ffi.h
• dash-spv-ffi/src/callbacks.rs
• dash-spv-ffi/src/client.rs
• dash-spv-ffi/src/types.rs
• dash-spv-ffi/tests/test_types.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• dash-spv-ffi/include/dash_spv_ffi.h