feat: Introduce DashSpvClientInterface

[xdustinface]

This introduces a new interface for command based access to DashSpvClient data from within different threads without the need of any arcs/locks but instead via async channels.

This was implemented as a more general alternative for #200 so for now it only contains DashSpvClientInterface:: get_quorum_by_height.

Example:

let (command_sender, command_receiver) = tokio::sync::mpsc::unbounded_channel();
let shutdown_token = CancellationToken::new();

let client_task = tokio::spawn(async move {
    tracing::info!("Run SPV client...");
    client.run(command_receiver, shutdown_token)
});

let client_interface = DashSpvClientInterface::new(command_sender);
let quorum_hash = QuorumHash::from_str("4acfa5c6d92071d206da5b767039d42f24e7ab1a694a5b8014cddc088311e448").unwrap();
let result = client_interface.get_quorum_by_height(0, LLMQType::Llmqtype25_67, quorum_hash).await;
println!("get_quorum_by_height result: {:?}", result);

tokio::join!(client_task);

Summary by CodeRabbit

• New Features

  • Added command-driven interface for asynchronous SPV client operations.
  • Introduced async quorum lookup capability with improved error reporting.
  • Implemented cancellation token–based shutdown mechanism for better lifecycle management.

• Bug Fixes

  • Enhanced error handling for channel and quorum lookup failures with explicit error codes.

• API Changes

  • Updated client lifecycle methods to support command-driven workflows.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This change introduces a command-driven architecture for the Dash SPV client, replacing signal-based shutdown with cancellation tokens and channel-based commands. A new DashSpvClientInterface handles async command dispatch, the monitor_network method is updated to multiplex commands and cancellation, and error types are extended to support channel and quorum lookup failures.

Changes

Cohort / File(s)	Change Summary
Client lifecycle and command architecture dash-spv/src/client/sync_coordinator.rs	Updated monitor_network signature to accept command receiver and cancellation token; added run() method to coordinate lifecycle; added handle_command() to process command messages. Control flow now uses tokio::select! to multiplex commands, network messages, and cancellation signals.
Command-driven interface dash-spv/src/client/interface.rs, dash-spv/src/client/mod.rs	New module interface.rs introducing DashSpvClientInterface struct with async command dispatch, DashSpvClientCommand enum for typed messaging, and get_quorum_by_height() method for async quorum queries via oneshot channels. Module re-exported in client/mod.rs.
Query layer updates dash-spv/src/client/queries.rs	Changed get_quorum_at_height signature from Option-returning with raw types to Result-returning with LLMQType and QuorumHash types; now returns cloned entries and explicit SpvError::QuorumLookupError on failure.
Error handling dash-spv/src/error.rs, dash-spv-ffi/src/error.rs	Added ChannelFailure(String, String) and QuorumLookupError(String) variants to SpvError enum in dash-spv; mapped new variants in FFI error conversion (ChannelFailure → RuntimeError, QuorumLookupError → ValidationError).
Main runtime updates dash-spv/src/main.rs, dash-spv-ffi/src/client.rs	Replaced tokio::select! await on monitor_network() and Ctrl-C with calls to new client.run(command_receiver, shutdown_token) method; introduced CancellationToken and unbounded command channels for lifecycle management.
Example updates dash-spv/examples/simple_sync.rs, dash-spv/examples/filter_sync.rs, dash-spv/examples/spv_with_wallet.rs	Consistent refactoring across all three examples: replaced tokio::select! + monitor_network() + manual stop() flow with unified client.run(command_receiver, shutdown_token) call; removed tokio::signal usage in favor of CancellationToken.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Client as DashSpvClient
    participant Monitor as monitor_network<br/>(select loop)
    participant Cmd as Command Handler
    participant Net as Network/Messages
    
    Note over App,Net: Old Flow (signal-based)
    App->>Client: sync_to_tip()
    Client->>Monitor: monitor_network()
    rect rgb(200, 220, 255)
        Monitor->>Net: Poll messages
        Net-->>Monitor: Process
    end
    App->>+App: Ctrl-C received
    App->>Client: stop()
    Client-->>Monitor: Abort
    
    Note over App,Net: New Flow (command-driven)
    App->>App: Create shutdown_token<br/>& command_receiver
    App->>+Client: run(command_receiver,<br/>shutdown_token)
    Client->>Monitor: monitor_network(receiver, token)
    
    rect rgb(240, 240, 240)
        loop Select on multiplex
            alt Command available
                Monitor->>Cmd: Dequeue command
                Cmd->>Cmd: handle_command()
                rect rgb(220, 240, 220)
                    Cmd->>Monitor: Process & respond
                end
            else Cancellation signaled
                Monitor->>Monitor: Token cancelled
                Monitor-->>Client: Return
            else Network message
                Monitor->>Net: Process message
            end
        end
    end
    App->>App: Signal shutdown_token
    Monitor-->>Client: Exit loop
    Client-->>App: Return Result

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~40 minutes

Areas requiring extra attention:

• Command channel integration: Verify oneshot/mpsc channel error handling and unbounded channel semantics in DashSpvClientInterface.get_quorum_by_height() and handle_command() implementation.
• Cancellation token semantics: Ensure CancellationToken is properly propagated and that timeouts interact correctly with the new tokio::select! branches in monitor_network().
• Return type changes: Verify get_quorum_at_height() Result return and cloning behavior do not introduce performance regressions or unexpected error propagation paths.
• FFI boundary: Confirm new SpvError variants are correctly mapped and that the unbounded channel creation in dash_spv_ffi_client_sync_to_tip_with_progress() integrates safely with the FFI layer.
• Example consistency: All three examples follow the same pattern; spot-check one for correctness and assume others are consistent.

Poem

🐰 Commands now flow through channels bright,
With tokens to cancel and hold the night,
No more select! and Ctrl-C strife—
Just run and let cancellation guide thy life! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'feat: Introduce DashSpvClientInterface' accurately describes the main change: introducing a new interface for command-based access to DashSpvClient data. The title aligns with the primary feature addition and is specific, concise, and clearly communicates the key change.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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 feat/dash-spv-client-interface

---

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

[pauldelucia]

It works and generally LGTM. I'm able to access the MasternodeListEngine for quorum lookups in DET during and after sync.

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

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

889-895: Consider whether the unused command channel overhead is necessary in the FFI path.

The FFI synchronization path creates an unbounded channel but never uses the _command_sender. While this aligns with the new monitor_network API signature and prepares for future command support, it introduces unnecessary channel allocation overhead for a feature not yet used in the FFI layer.

Consider one of these approaches:

1. Document the intent: Add a comment explaining this prepares for future command-based FFI APIs
2. Defer the change: Keep the old FFI monitor_network signature until commands are actually needed
3. Accept the overhead: The unbounded channel allocation is minimal and acceptable for API consistency

Since this is a "Chill" review and the overhead is minimal, this is fine as-is if it simplifies the API surface.

dash-spv/src/main.rs (1)

633-633: Unused command sender creates unnecessary channel overhead.

The _command_sender is created but never used in the main CLI application. While this matches the new run() API signature and prepares for future command-based interactions, it allocates an unbounded channel that serves no purpose currently.

This is acceptable for API consistency, but consider documenting the intent:

+// Command channel for future interactive commands (sender currently unused)
 let (_command_sender, command_receiver) = tokio::sync::mpsc::unbounded_channel();

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e9ef04 and c586be9.

📒 Files selected for processing (11)

• dash-spv-ffi/src/client.rs (1 hunks)
• dash-spv-ffi/src/error.rs (2 hunks)
• dash-spv/examples/filter_sync.rs (2 hunks)
• dash-spv/examples/simple_sync.rs (2 hunks)
• dash-spv/examples/spv_with_wallet.rs (2 hunks)
• dash-spv/src/client/interface.rs (1 hunks)
• dash-spv/src/client/mod.rs (1 hunks)
• dash-spv/src/client/queries.rs (4 hunks)
• dash-spv/src/client/sync_coordinator.rs (3 hunks)
• dash-spv/src/error.rs (2 hunks)
• dash-spv/src/main.rs (2 hunks)

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.

📚 Learning: 2025-06-26T16:01:37.609Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.

Applied to files:

• dash-spv/src/main.rs
• dash-spv/examples/filter_sync.rs
• dash-spv/examples/spv_with_wallet.rs
• dash-spv-ffi/src/client.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/client/interface.rs

📚 Learning: 2025-02-25T06:19:32.230Z

Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list_entry/hash.rs:7-12
Timestamp: 2025-02-25T06:19:32.230Z
Learning: The `consensus_encode` method on `MasternodeListEntry` writing to a `Vec` buffer cannot fail, so using `.expect()` is appropriate rather than propagating the error with the `?` operator.

Applied to files:

• dash-spv/src/client/queries.rs

📚 Learning: 2025-06-26T15:54:02.509Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.

Applied to files:

• dash-spv/src/client/interface.rs

🧬 Code graph analysis (4)

dash-spv/examples/filter_sync.rs (1)

dash-spv/src/network/manager.rs (1)

• new (78-128)

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

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

• masternode_list_engine (202-206)

dash-spv/examples/spv_with_wallet.rs (2)

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

• new (56-60)

dash-spv/src/network/manager.rs (1)

• new (78-128)

dash-spv/src/client/interface.rs (3)

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

• dashcore (29-29)

key-wallet/src/derivation.rs (1)

• derive (88-90)

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

• new (81-85)

🔇 Additional comments (10)

dash-spv/src/client/queries.rs (3)

13-13: LGTM: Type safety improvements.

The introduction of LLMQType and QuorumHash semantic types improves type safety over raw u8 and &[u8; 32] types.

Also applies to: 17-17

---

97-128: LGTM: Comprehensive error handling.

The error handling provides detailed context for all failure cases (missing quorum, missing quorum type, missing masternode list) with appropriate logging and SpvError::QuorumLookupError messages.

---

75-94: The clone operation is appropriate given the API design and usage pattern.

The function's change from Option<&QualifiedQuorumEntry> to Result<QualifiedQuorumEntry> is well-justified:

1. API Design: Returning Result provides better error context than Option, which is essential when a quorum isn't found.
2. Ownership Requirements: The single call site (sync_coordinator.rs:611) sends the result through a channel, requiring owned data regardless. Cloning is unavoidable in this pattern.
3. Call Frequency: Not in a hot path—called once per message in an async handler, not in tight validation loops.

While QualifiedQuorumEntry contains variable-sized data (vectors for signers/valid_members) and large signatures, the performance impact is negligible given the non-critical call context. The design trade-off (owned data for cleaner async/error handling) is sound.

dash-spv/src/client/sync_coordinator.rs (4)

15-15: LGTM: Command-based interface integration.

The updated monitor_network signature properly integrates the command receiver and cancellation token for the new command-based architecture.

Also applies to: 23-24, 71-75

---

484-567: LGTM: Well-structured event loop multiplexing.

The tokio::select! block properly multiplexes command handling, network message processing, timeouts, and cancellation. The comprehensive error handling with categorization (Network, Storage, Validation errors) and continue-on-error semantics ensures resilience.

---

573-601: LGTM: Clean lifecycle management.

The run method provides clean lifecycle management by:

• Spawning the client monitoring task with the command receiver
• Spawning a separate shutdown signal handler
• Properly joining tasks and handling errors
• Ensuring stop() is called on completion

---

603-621: LGTM: Clean command handler implementation.

The handle_command method properly processes commands and returns results via the oneshot channel. The error context in the ChannelFailure includes all command parameters for debugging.

dash-spv/src/client/interface.rs (3)

8-14: LGTM: Clean error handling abstraction.

The type aliases and receive helper function provide a clean abstraction for mapping oneshot channel errors to SpvError::ChannelFailure with context.

---

16-48: LGTM: Well-designed command pattern.

The DashSpvClientCommand enum and its implementations provide a clean command pattern:

• Encapsulates request parameters and response channel
• send method with proper error handling
• Display implementation for logging (correctly excludes the sender)

---

50-78: LGTM: Clean async interface with intentional public field.

The DashSpvClientInterface provides a clean async interface for client queries. The command_sender field is marked public, which allows direct access if needed. If you prefer encapsulation, consider making it private and providing an accessor method, but the current design is acceptable for this use case.