refactor(dash-spv): DashSPVClient stores its own cancelation token simplifying consumers API

[ZocoLini]

Summary by CodeRabbit

• New Features

  • Client now manages internal cancellation coordination automatically.

• Bug Fixes

  • Improved shutdown reliability by using explicit stop() method instead of external token cancellation.

• Refactor

  • Simplified client API: run() method no longer requires a cancellation token parameter. Use stop() to trigger graceful shutdown.

[coderabbitai]

📝 Walkthrough

Walkthrough

DashSpvClient refactors shutdown coordination by moving from an external CancellationToken parameter in run() to an internal cancel_token field managed by the client. Callers now invoke stop() to trigger shutdown. The FFI layer, all examples, CLI entrypoint, test infrastructure, and integration tests are updated to use the new API.

Changes

Client shutdown refactoring from external to internal coordination

Layer / File(s)	Summary
Internal cancellation token and run() API update dash-spv/src/client/core.rs, dash-spv/src/client/lifecycle.rs, dash-spv/src/client/sync_coordinator.rs	DashSpvClient gains a cancel_token field initialized in the constructor, Clone impl shares the token across clones, and run() signature changes from run(token: CancellationToken) to run(), updating the select! loop to listen to self.cancel_token.cancelled() instead of the provided token.
FFI layer shutdown refactoring dash-spv-ffi/src/client.rs	FFIDashSpvClient removes shutdown_token field, the stop_client_internal helper is deleted, and dash_spv_ffi_client_stop, dash_spv_ffi_client_run, and dash_spv_ffi_client_destroy are updated to call client.stop().await and run() without token passing.
Example and documentation updates dash-spv/examples/filter_sync.rs, dash-spv/examples/simple_sync.rs, dash-spv/examples/spv_with_wallet.rs, dash-spv/src/lib.rs	All examples and lib.rs documentation remove CancellationToken imports and token construction, calling client.run().await? directly.
CLI Ctrl-C handler and run() integration dash-spv/src/main.rs	The CLI replaces token-based shutdown with a spawned Ctrl-C handler that calls client.stop() when interrupted, and changes the client run invocation from client.run(shutdown_token) to client.run().await?.
Test setup infrastructure updates dash-spv/tests/dashd_masternode/setup.rs, dash-spv/tests/dashd_sync/setup.rs	ClientHandle in both test setups removes the cancel_token field, stop() method calls client.stop().await instead of canceling a token, and spawned run tasks call run().await without token arguments.
Integration test updates dash-spv/tests/peer_test.rs, dash-spv/tests/wallet_integration_test.rs	Both test files remove CancellationToken imports, spawn client run tasks without token arguments, and invoke client.stop().await to trigger shutdown instead of cancel.cancel().

Possibly related issues

• #577 — This PR directly addresses the cancellation-token refactoring: it removes the external CancellationToken parameter from DashSpvClient::run() and internalizes cancellation control via cancel_token, shifting the shutdown API from token cancellation to explicit stop() calls.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 A refactored run, no token's needed now,
Clients manage their own cancellation, take a bow!
No more passing tokens to and fro,
Just call stop() when ready to go. 🛑

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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: DashSpvClient now stores its own cancellation token internally, eliminating the need for callers to manage and pass tokens explicitly.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/spv-cancel-token

---

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

[codecov]

Codecov Report

❌ Patch coverage is 62.50000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.69%. Comparing base (bbf0a9c) to head (0d67abe).
⚠️ Report is 1 commits behind head on v0.42-dev.

Files with missing lines	Patch %	Lines
dash-spv/src/main.rs	0.00%	6 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           v0.42-dev     #771      +/-   ##
=============================================
+ Coverage      72.62%   72.69%   +0.06%     
=============================================
  Files            320      320              
  Lines          70254    70325      +71     
=============================================
+ Hits           51022    51122     +100     
+ Misses         19232    19203      -29     

Flag	Coverage Δ
core	76.30% <ø> (ø)
ffi	49.15% <100.00%> (+0.73%)	⬆️
rpc	20.00% <ø> (ø)
spv	89.94% <40.00%> (-0.03%)	⬇️
wallet	71.27% <ø> (-0.01%)	⬇️

Files with missing lines	Coverage Δ
dash-spv-ffi/src/client.rs	57.73% <100.00%> (-0.19%)	⬇️
dash-spv/src/client/core.rs	47.82% <100.00%> (+1.15%)	⬆️
dash-spv/src/client/lifecycle.rs	78.46% <100.00%> (+0.62%)	⬆️
dash-spv/src/client/sync_coordinator.rs	76.34% <100.00%> (ø)
dash-spv/src/main.rs	0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dash-spv/src/client/sync_coordinator.rs`:
- Around line 118-121: The run loop is waiting on self.cancel_token.cancelled()
but stop() only flips running later, allowing extra tick() work during shutdown;
update stop() to call self.cancel_token.cancel() immediately (before waiting on
running or doing teardown) so the run() branch listening on cancel_token will
wake and exit promptly, and keep existing running flag/teardown logic intact;
locate methods named run(), stop(), and the field cancel_token and ensure stop()
invokes the token's cancel() (or equivalent) prior to the rest of the shutdown
sequence.

In `@dash-spv/tests/peer_test.rs`:
- Around line 65-66: The test currently swallows shutdown failures by ignoring
the results of client.stop() and the run-task handle.await; change the test to
assert both succeed instead of discarding them—await client.stop().await and
assert it returns Ok (or unwrap) and await handle.await and assert the task
returned Ok (or unwrap) so shutdown errors are propagated; apply the same change
for the other occurrences around client.stop() and handle.await in the file.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7b997428-6b50-42a1-9fb3-6fe28a2844fe

📥 Commits

Reviewing files that changed from the base of the PR and between cfb01fa and 0d67abe.

📒 Files selected for processing (13)

• dash-spv-ffi/src/client.rs
• dash-spv/examples/filter_sync.rs
• dash-spv/examples/simple_sync.rs
• dash-spv/examples/spv_with_wallet.rs
• dash-spv/src/client/core.rs
• dash-spv/src/client/lifecycle.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/lib.rs
• dash-spv/src/main.rs
• dash-spv/tests/dashd_masternode/setup.rs
• dash-spv/tests/dashd_sync/setup.rs
• dash-spv/tests/peer_test.rs
• dash-spv/tests/wallet_integration_test.rs

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wire stop() to the internal token to prevent shutdown races.

At Line 118, run() waits on self.cancel_token.cancelled(), but in this layer stop() never cancels that token and only flips running at the end of teardown. This allows extra tick() work while shutdown is already in progress.

Suggested fix

diff --git a/dash-spv/src/client/lifecycle.rs b/dash-spv/src/client/lifecycle.rs
@@
     pub async fn stop(&self) -> Result<()> {
         // Check if already stopped
         {
             let running = self.running.read().await;
             if !*running {
                 return Ok(());
             }
         }
+
+        // Wake run loop immediately.
+        self.cancel_token.cancel();
+
+        // Prevent further coordinator ticks during shutdown.
+        {
+            let mut running = self.running.write().await;
+            *running = false;
+        }
@@
-        // Mark as stopped
-        let mut running = self.running.write().await;
-        *running = false;
-
         Ok(())
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dash-spv/src/client/sync_coordinator.rs` around lines 118 - 121, The run loop
is waiting on self.cancel_token.cancelled() but stop() only flips running later,
allowing extra tick() work during shutdown; update stop() to call
self.cancel_token.cancel() immediately (before waiting on running or doing
teardown) so the run() branch listening on cancel_token will wake and exit
promptly, and keep existing running flag/teardown logic intact; locate methods
named run(), stop(), and the field cancel_token and ensure stop() invokes the
token's cancel() (or equivalent) prior to the rest of the shutdown sequence.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don't swallow shutdown failures in these tests.

These assertions now cover the refactored stop path, but both client.stop() and the run-task result are ignored. That can turn shutdown regressions into false positives.

Proposed fix

-    let _ = client.stop().await;
-    let _ = handle.await;
+    client.stop().await.expect("failed to stop client");
+    handle.await.expect("run task panicked").expect("run task returned error");

Also applies to: 100-101, 133-134

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dash-spv/tests/peer_test.rs` around lines 65 - 66, The test currently
swallows shutdown failures by ignoring the results of client.stop() and the
run-task handle.await; change the test to assert both succeed instead of
discarding them—await client.stop().await and assert it returns Ok (or unwrap)
and await handle.await and assert the task returned Ok (or unwrap) so shutdown
errors are propagated; apply the same change for the other occurrences around
client.stop() and handle.await in the file.

[xdustinface]

CodeRabbit review should be addressed. Its legit the token is not canceled anymore now it only work because of the running variable which is a weird thing to begin with that we have both of them. We should probably consolidate this into a watch::<bool> or so? Which can be a follow up, but we should still cancel the token in here or i guess you can also just make this a "drop all the cancelation token" PR and get rid of it.