diff --git a/dash-spv-ffi/FFI_API.md b/dash-spv-ffi/FFI_API.md index 2fa1c0796..1023f3f1c 100644 --- a/dash-spv-ffi/FFI_API.md +++ b/dash-spv-ffi/FFI_API.md @@ -4,7 +4,7 @@ This document provides a comprehensive reference for all FFI (Foreign Function I **Auto-generated**: This documentation is automatically generated from the source code. Do not edit manually. -**Total Functions**: 47 +**Total Functions**: 49 ## Table of Contents @@ -93,12 +93,14 @@ Functions: 2 ### Event Callbacks -Functions: 4 +Functions: 6 | Function | Description | Module | |----------|-------------|--------| +| `dash_spv_ffi_client_clear_client_error_callback` | Clear the client error callback | client | | `dash_spv_ffi_client_clear_network_event_callbacks` | Clear network event callbacks | client | | `dash_spv_ffi_client_clear_progress_callback` | Clear progress callback | client | +| `dash_spv_ffi_client_set_client_error_callback` | Set a callback for fatal client errors (start failure, sync thread crash) | client | | `dash_spv_ffi_client_set_network_event_callbacks` | Set network event callbacks for push-based event notifications | client | | `dash_spv_ffi_client_set_progress_callback` | Set progress callback for sync progress updates | client | @@ -592,6 +594,22 @@ This function is unsafe because: - The caller must ensure all pointers are valid ### Event Callbacks - Detailed +#### `dash_spv_ffi_client_clear_client_error_callback` + +```c +dash_spv_ffi_client_clear_client_error_callback(client: *mut FFIDashSpvClient,) -> i32 +``` + +**Description:** +Clear the client error callback. # Safety - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. + +**Safety:** +- `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. + +**Module:** `client` + +--- + #### `dash_spv_ffi_client_clear_network_event_callbacks` ```c @@ -624,6 +642,22 @@ Clear progress callback. # Safety - `client` must be a valid, non-null pointer --- +#### `dash_spv_ffi_client_set_client_error_callback` + +```c +dash_spv_ffi_client_set_client_error_callback(client: *mut FFIDashSpvClient, callback: FFIClientErrorCallback,) -> i32 +``` + +**Description:** +Set a callback for fatal client errors (start failure, sync thread crash). # Safety - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. - The `callback` struct and its `user_data` must remain valid until the callback is cleared. - The callback must be thread-safe as it may be called from a background thread. + +**Safety:** +- `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. - The `callback` struct and its `user_data` must remain valid until the callback is cleared. - The callback must be thread-safe as it may be called from a background thread. + +**Module:** `client` + +--- + #### `dash_spv_ffi_client_set_network_event_callbacks` ```c diff --git a/dash-spv-ffi/include/dash_spv_ffi.h b/dash-spv-ffi/include/dash_spv_ffi.h index 70daf7472..7ba662c7f 100644 --- a/dash-spv-ffi/include/dash_spv_ffi.h +++ b/dash-spv-ffi/include/dash_spv_ffi.h @@ -433,6 +433,23 @@ typedef struct FFIProgressCallback { void *user_data; } FFIProgressCallback; +/** + * Callback for fatal client errors (e.g. start failure, monitor thread crash). + * + * The `error` string pointer is borrowed and only valid for the duration + * of the callback. Callers must copy the string if they need to retain it + * after the callback returns. + */ +typedef void (*OnClientErrorCallback)(const char *error, void *user_data); + +/** + * Client error callback configuration. + */ +typedef struct FFIClientErrorCallback { + OnClientErrorCallback on_error; + void *user_data; +} FFIClientErrorCallback; + /** * FFIResult type for error handling */ @@ -675,6 +692,27 @@ int32_t dash_spv_ffi_client_set_progress_callback(struct FFIDashSpvClient *clien */ int32_t dash_spv_ffi_client_clear_progress_callback(struct FFIDashSpvClient *client) ; +/** + * Set a callback for fatal client errors (start failure, sync thread crash). + * + * # Safety + * - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. + * - The `callback` struct and its `user_data` must remain valid until the callback is cleared. + * - The callback must be thread-safe as it may be called from a background thread. + */ + +int32_t dash_spv_ffi_client_set_client_error_callback(struct FFIDashSpvClient *client, + struct FFIClientErrorCallback callback) +; + +/** + * Clear the client error callback. + * + * # Safety + * - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. + */ + int32_t dash_spv_ffi_client_clear_client_error_callback(struct FFIDashSpvClient *client) ; + struct FFIClientConfig *dash_spv_ffi_config_new(FFINetwork network) ; struct FFIClientConfig *dash_spv_ffi_config_mainnet(void) ; diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index e81a07a77..97ce9028e 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -575,6 +575,48 @@ impl Default for FFIWalletEventCallbacks { } } +// ============================================================================ +// FFIClientErrorCallback - Fatal client-level errors +// ============================================================================ + +/// Callback for fatal client errors (e.g. start failure, monitor thread crash). +/// +/// The `error` string pointer is borrowed and only valid for the duration +/// of the callback. Callers must copy the string if they need to retain it +/// after the callback returns. +pub type OnClientErrorCallback = + Option; + +/// Client error callback configuration. +#[repr(C)] +#[derive(Clone)] +pub struct FFIClientErrorCallback { + pub on_error: OnClientErrorCallback, + pub user_data: *mut c_void, +} + +unsafe impl Send for FFIClientErrorCallback {} +unsafe impl Sync for FFIClientErrorCallback {} + +impl Default for FFIClientErrorCallback { + fn default() -> Self { + Self { + on_error: None, + user_data: std::ptr::null_mut(), + } + } +} + +impl FFIClientErrorCallback { + /// Dispatch a client error to the callback. + pub fn dispatch(&self, error: &str) { + if let Some(cb) = self.on_error { + let c_error = CString::new(error).unwrap_or_default(); + cb(c_error.as_ptr(), self.user_data); + } + } +} + impl FFIWalletEventCallbacks { /// Dispatch a WalletEvent to the appropriate callback. pub fn dispatch(&self, event: &key_wallet_manager::WalletEvent) { diff --git a/dash-spv-ffi/src/client.rs b/dash-spv-ffi/src/client.rs index 92162fe28..1562b9ec8 100644 --- a/dash-spv-ffi/src/client.rs +++ b/dash-spv-ffi/src/client.rs @@ -1,7 +1,7 @@ use crate::{ - null_check, set_last_error, FFIClientConfig, FFIErrorCode, FFINetworkEventCallbacks, - FFIProgressCallback, FFISyncEventCallbacks, FFISyncProgress, FFIWalletEventCallbacks, - FFIWalletManager, + null_check, set_last_error, FFIClientConfig, FFIClientErrorCallback, FFIErrorCode, + FFINetworkEventCallbacks, FFIProgressCallback, FFISyncEventCallbacks, FFISyncProgress, + FFIWalletEventCallbacks, FFIWalletManager, }; // Import wallet types from key-wallet-ffi use key_wallet_ffi::FFIWalletManager as KeyWalletFFIWalletManager; @@ -120,6 +120,7 @@ pub struct FFIDashSpvClient { network_event_callbacks: Arc>>, wallet_event_callbacks: Arc>>, progress_callback: Arc>>, + client_error_callback: Arc>>, } /// Create a new SPV client and return an opaque pointer. @@ -179,6 +180,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_new( network_event_callbacks: Arc::new(Mutex::new(None)), wallet_event_callbacks: Arc::new(Mutex::new(None)), progress_callback: Arc::new(Mutex::new(None)), + client_error_callback: Arc::new(Mutex::new(None)), }; Box::into_raw(Box::new(ffi_client)) } @@ -351,13 +353,17 @@ pub unsafe extern "C" fn dash_spv_ffi_client_run(client: *mut FFIDashSpvClient) )); } - // Spawn the sync monitoring task + let error_callback = client.client_error_callback.clone(); let spv_client = client.inner.clone(); tasks.push(client.runtime.spawn(async move { tracing::debug!("Sync task: starting run"); if let Err(e) = spv_client.run(shutdown_token).await { tracing::error!("Sync task: error: {}", e); + let cb = error_callback.lock().unwrap().clone(); + if let Some(ref cb) = cb { + cb.dispatch(&e.to_string()); + } } tracing::debug!("Sync task: exiting"); @@ -709,3 +715,38 @@ pub unsafe extern "C" fn dash_spv_ffi_client_clear_progress_callback( FFIErrorCode::Success as i32 } + +/// Set a callback for fatal client errors (start failure, sync thread crash). +/// +/// # Safety +/// - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. +/// - The `callback` struct and its `user_data` must remain valid until the callback is cleared. +/// - The callback must be thread-safe as it may be called from a background thread. +#[no_mangle] +pub unsafe extern "C" fn dash_spv_ffi_client_set_client_error_callback( + client: *mut FFIDashSpvClient, + callback: FFIClientErrorCallback, +) -> i32 { + null_check!(client); + + let client = &(*client); + *client.client_error_callback.lock().unwrap() = Some(callback); + + FFIErrorCode::Success as i32 +} + +/// Clear the client error callback. +/// +/// # Safety +/// - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. +#[no_mangle] +pub unsafe extern "C" fn dash_spv_ffi_client_clear_client_error_callback( + client: *mut FFIDashSpvClient, +) -> i32 { + null_check!(client); + + let client = &(*client); + *client.client_error_callback.lock().unwrap() = None; + + FFIErrorCode::Success as i32 +} diff --git a/dash-spv-ffi/tests/unit/test_client_lifecycle.rs b/dash-spv-ffi/tests/unit/test_client_lifecycle.rs index 6bda25e8f..ceef717ec 100644 --- a/dash-spv-ffi/tests/unit/test_client_lifecycle.rs +++ b/dash-spv-ffi/tests/unit/test_client_lifecycle.rs @@ -9,6 +9,8 @@ mod tests { use key_wallet_ffi::FFINetwork; use serial_test::serial; use std::ffi::CString; + use std::sync::mpsc; + use std::sync::{Arc as StdArc, Mutex as StdMutex}; use std::thread; use std::time::Duration; use tempfile::TempDir; @@ -209,6 +211,124 @@ mod tests { } } + #[test] + #[serial] + fn test_client_error_callback_fires_on_start_failure() { + let (tx, rx) = mpsc::channel::(); + let tx_ptr = Box::into_raw(Box::new(tx)); + + extern "C" fn on_error( + error: *const std::os::raw::c_char, + user_data: *mut std::os::raw::c_void, + ) { + let tx = unsafe { &*(user_data as *const mpsc::Sender) }; + let error_str = unsafe { std::ffi::CStr::from_ptr(error) }.to_str().unwrap().to_owned(); + let _ = tx.send(error_str); + } + + unsafe { + let (config, _temp_dir) = create_test_config_with_dir(); + let client = dash_spv_ffi_client_new(config); + assert!(!client.is_null()); + + let callback = FFIClientErrorCallback { + on_error: Some(on_error), + user_data: tx_ptr as *mut std::os::raw::c_void, + }; + let result = dash_spv_ffi_client_set_client_error_callback(client, callback); + assert_eq!(result, FFIErrorCode::Success as i32); + + // Call run() twice — the second run's sync thread will call + // start() on the already-running client, triggering "already running" + let run_result = dash_spv_ffi_client_run(client); + assert_eq!(run_result, FFIErrorCode::Success as i32); + + // Brief wait for the first run's sync thread to complete start() + thread::sleep(Duration::from_millis(200)); + + let _run_result2 = dash_spv_ffi_client_run(client); + + // Wait for the error callback to fire (with timeout) + let error_msg = rx + .recv_timeout(Duration::from_secs(5)) + .expect("Error callback should have been called on start failure"); + assert!( + error_msg.contains("already running"), + "Expected 'already running' error, got: {}", + error_msg + ); + + dash_spv_ffi_client_stop(client); + + // Free the sender only after stop has joined all threads, + // so no background thread can call on_error with a dangling user_data. + drop(Box::from_raw(tx_ptr)); + + dash_spv_ffi_client_destroy(client); + dash_spv_ffi_config_destroy(config); + } + } + + #[test] + #[serial] + fn test_client_error_callback_dispatch() { + let error_store: StdArc>> = StdArc::new(StdMutex::new(None)); + let error_store_raw = StdArc::into_raw(error_store.clone()); + + extern "C" fn on_error( + error: *const std::os::raw::c_char, + user_data: *mut std::os::raw::c_void, + ) { + assert!(!error.is_null()); + let store = unsafe { StdArc::from_raw(user_data as *const StdMutex>) }; + let error_str = unsafe { std::ffi::CStr::from_ptr(error) }.to_str().unwrap().to_owned(); + *store.lock().unwrap() = Some(error_str); + let _ = StdArc::into_raw(store); + } + + let callback = FFIClientErrorCallback { + on_error: Some(on_error), + user_data: error_store_raw as *mut std::os::raw::c_void, + }; + + callback.dispatch("test error message"); + + let received = error_store.lock().unwrap(); + assert_eq!(received.as_deref(), Some("test error message")); + drop(received); + + unsafe { drop(StdArc::from_raw(error_store_raw)) }; + } + + #[test] + #[serial] + fn test_client_error_callback_null_client() { + unsafe { + let callback = FFIClientErrorCallback { + on_error: None, + user_data: std::ptr::null_mut(), + }; + + assert_eq!( + dash_spv_ffi_client_set_client_error_callback(std::ptr::null_mut(), callback), + FFIErrorCode::NullPointer as i32 + ); + + assert_eq!( + dash_spv_ffi_client_clear_client_error_callback(std::ptr::null_mut()), + FFIErrorCode::NullPointer as i32 + ); + } + } + + #[test] + #[serial] + fn test_client_error_callback_no_callback_set() { + // Dispatch with no callback set should not panic + let callback = FFIClientErrorCallback::default(); + callback.dispatch("should not panic"); + } + #[test] #[serial] fn test_client_repeated_creation_destruction() { diff --git a/dash-spv/src/client/sync_coordinator.rs b/dash-spv/src/client/sync_coordinator.rs index 42ac541a0..da1fe444d 100644 --- a/dash-spv/src/client/sync_coordinator.rs +++ b/dash-spv/src/client/sync_coordinator.rs @@ -30,32 +30,48 @@ impl DashSpvClient { - tracing::info!("Sync progress:{}", *progress_updates.borrow()); + let error = tokio::select! { + result = progress_updates.changed() => { + match result { + Ok(()) => { + tracing::info!("Sync progress: {}", *progress_updates.borrow()); + None + } + Err(_) => { + tracing::warn!("Progress channel closed."); + break None + } + } } _ = sync_coordinator_tick_interval.tick() => { - // Tick the sync coordinator to aggregate progress - if let Err(e) = self.sync_coordinator.lock().await.tick().await { - tracing::warn!("Sync coordinator tick error: {}", e); - } + self.sync_coordinator.lock().await.tick().await.err().map(Into::into) } _ = token.cancelled() => { tracing::debug!("DashSpvClient run loop cancelled"); - break + break None } + }; + + if error.is_some() { + break error; } - } + }; + + // Always stop the client + let stop_result = self.stop().await; - self.stop().await + match error { + Some(e) => Err(e), + None => stop_result, + } } } diff --git a/dash-spv/src/sync/sync_coordinator.rs b/dash-spv/src/sync/sync_coordinator.rs index aa653fdde..ad074d2ec 100644 --- a/dash-spv/src/sync/sync_coordinator.rs +++ b/dash-spv/src/sync/sync_coordinator.rs @@ -236,9 +236,11 @@ where } Ok(Err(e)) => { tracing::error!("Manager task failed: {}", e); + return Err(e); } Err(e) => { tracing::error!("Manager task panicked: {}", e); + return Err(SyncError::InvalidState(format!("Manager task panicked: {}", e))); } } }