feat: expose full TransactionRecord through FFI and wallet events (#614)

* feat: expose full `TransactionRecord` through FFI and wallet events

- Add FFI types for transaction direction, type, output role, and input/output details
- Enrich `FFITransactionRecord` with classification, details, label and serialized tx bytes
- Simplify `WalletEvent::TransactionReceived` to carry `Box<TransactionRecord>`
- Return `TransactionRecord` from `record_transaction` and propagate via `TransactionCheckResult`
- Refactor `OnTransactionReceivedCallback` to pass `*const FFITransactionRecord`

* fix: prevent `label` CString memory leak in transaction callback

Addresses CodeRabbit review comment on PR #614
#614 (comment)

* refactor: combine received txid/amount into single mutex in test tracker

Addresses CodeRabbit review comment on PR #614
#614 (comment)

* fix: correct `net_amount` assertion in FFI callback integration test

The test sends 1 DASH from the same wallet (same mnemonic) that the SPV
client tracks, making it an internal transfer where both inputs and
outputs belong to the wallet. The `net_amount` therefore equals
approximately `-fee`, not +100_000_000. Replace the strict amount pairing
assertion with a txid presence check and non-zero net_amount verification.

Author: xdustinface

dash-spv-ffi/src/bin/ffi_cli.rs

@@ -5,6 +5,7 @@ use std::ptr;
 use clap::{Arg, ArgAction, Command};
 
 use dash_spv_ffi::*;
+use key_wallet_ffi::managed_account::FFITransactionRecord;
 use key_wallet_ffi::types::FFITransactionContext;
 use key_wallet_ffi::wallet_manager::wallet_manager_add_wallet_from_mnemonic;
 use key_wallet_ffi::{FFIError, FFINetwork};
@@ -157,24 +158,28 @@ extern "C" fn on_peers_updated(connected_count: u32, best_height: u32, _user_dat
 
 extern "C" fn on_transaction_received(
     wallet_id: *const c_char,
-    status: FFITransactionContext,
     account_index: u32,
-    txid: *const [u8; 32],
-    amount: i64,
-    addresses: *const c_char,
+    record: *const FFITransactionRecord,
     _user_data: *mut c_void,
 ) {
     let wallet_str = ffi_string_to_rust(wallet_id);
-    let addr_str = ffi_string_to_rust(addresses);
     let wallet_short = if wallet_str.len() > 8 {
         &wallet_str[..8]
     } else {
         &wallet_str
     };
-    let txid_hex = unsafe { hex::encode(*txid) };
+    if record.is_null() {
+        println!(
+            "[Wallet] TX received: wallet={}..., account={}, record=null",
+            wallet_short, account_index
+        );
+        return;
+    }
+    let r = unsafe { &*record };
+    let txid_hex = hex::encode(r.txid);
     println!(
-        "[Wallet] TX received: wallet={}..., txid={}, account={}, amount={} duffs, status={:?}, addresses={}",
-        wallet_short, txid_hex, account_index, amount, status, addr_str
+        "[Wallet] TX received: wallet={}..., txid={}, account={}, amount={} duffs, tx_size={}",
+        wallet_short, txid_hex, account_index, r.net_amount, r.tx_len
     );
 }
 

dash-spv-ffi/src/callbacks.rs

@@ -11,7 +11,11 @@ use dash_spv::network::NetworkEvent;
 use dash_spv::sync::{SyncEvent, SyncProgress};
 use dash_spv::EventHandler;
 use dashcore::hashes::Hash;
-use key_wallet_ffi::types::FFITransactionContext;
+use key_wallet_ffi::managed_account::FFITransactionRecord;
+use key_wallet_ffi::types::{
+    FFIInputDetail, FFIOutputDetail, FFIOutputRole, FFITransactionContext, FFITransactionDirection,
+    FFITransactionType,
+};
 use key_wallet_manager::WalletEvent;
 use std::ffi::CString;
 use std::os::raw::{c_char, c_void};
@@ -530,17 +534,15 @@ impl FFINetworkEventCallbacks {
 
 /// Callback for WalletEvent::TransactionReceived
 ///
-/// The `wallet_id`, `addresses` string pointers and the `txid` hash pointer
-/// are borrowed and only valid for the duration of the callback. Callers must
-/// copy any data they need to retain after the callback returns.
+/// The `record` pointer is borrowed and only valid for the duration of the
+/// callback. Callers must copy any data they need to retain after the callback
+/// returns. The record contains all transaction details including serialized
+/// transaction bytes, input/output details, and classification metadata.
 pub type OnTransactionReceivedCallback = Option<
     extern "C" fn(
         wallet_id: *const c_char,
-        status: FFITransactionContext,
         account_index: u32,
-        txid: *const [u8; 32],
-        amount: i64,
-        addresses: *const c_char,
+        record: *const FFITransactionRecord,
         user_data: *mut c_void,
     ),
 >;
@@ -696,28 +698,86 @@ impl FFIWalletEventCallbacks {
         match event {
             WalletEvent::TransactionReceived {
                 wallet_id,
-                status,
                 account_index,
-                txid,
-                amount,
-                addresses,
+                record,
             } => {
                 if let Some(cb) = self.on_transaction_received {
                     let wallet_id_hex = hex::encode(wallet_id);
                     let c_wallet_id = CString::new(wallet_id_hex).unwrap_or_default();
-                    let txid_bytes = txid.as_byte_array();
-                    let addresses_str: Vec<String> =
-                        addresses.iter().map(|a| a.to_string()).collect();
-                    let c_addresses = CString::new(addresses_str.join(",")).unwrap_or_default();
+
+                    let tx_bytes =
+                        dashcore::consensus::serialize(&record.transaction).into_boxed_slice();
+
+                    let input_details: Vec<FFIInputDetail> = record
+                        .input_details
+                        .iter()
+                        .map(|d| {
+                            let addr = CString::new(d.address.to_string()).unwrap_or_default();
+                            FFIInputDetail {
+                                index: d.index,
+                                value: d.value,
+                                address: addr.into_raw(),
+                            }
+                        })
+                        .collect();
+
+                    let output_details: Vec<FFIOutputDetail> = record
+                        .output_details
+                        .iter()
+                        .map(|d| FFIOutputDetail {
+                            index: d.index,
+                            role: FFIOutputRole::from(d.role),
+                        })
+                        .collect();
+
+                    let c_label =
+                        record.label.as_ref().map(|l| CString::new(l.as_str()).unwrap_or_default());
+
+                    let ffi_record = FFITransactionRecord {
+                        txid: record.txid.to_byte_array(),
+                        net_amount: record.net_amount,
+                        context: FFITransactionContext::from(record.context),
+                        transaction_type: FFITransactionType::from(record.transaction_type),
+                        direction: FFITransactionDirection::from(record.direction),
+                        fee: record.fee.unwrap_or(0),
+                        input_details: if input_details.is_empty() {
+                            std::ptr::null_mut()
+                        } else {
+                            input_details.as_ptr() as *mut _
+                        },
+                        input_details_count: input_details.len(),
+                        output_details: if output_details.is_empty() {
+                            std::ptr::null_mut()
+                        } else {
+                            output_details.as_ptr() as *mut _
+                        },
+                        output_details_count: output_details.len(),
+                        tx_data: if tx_bytes.is_empty() {
+                            std::ptr::null_mut()
+                        } else {
+                            tx_bytes.as_ptr() as *mut _
+                        },
+                        tx_len: tx_bytes.len(),
+                        label: c_label
+                            .as_ref()
+                            .map_or(std::ptr::null_mut(), |l| l.as_ptr() as *mut _),
+                    };
+
                     cb(
                         c_wallet_id.as_ptr(),
-                        FFITransactionContext::from(*status),
                         *account_index,
-                        txid_bytes as *const [u8; 32],
-                        *amount,
-                        c_addresses.as_ptr(),
+                        &ffi_record as *const FFITransactionRecord,
                         self.user_data,
                     );
+
+                    // Free the CString addresses from input details
+                    for detail in input_details {
+                        if !detail.address.is_null() {
+                            unsafe {
+                                drop(CString::from_raw(detail.address));
+                            }
+                        }
+                    }
                 }
             }
             WalletEvent::TransactionStatusChanged {

dash-spv-ffi/tests/dashd_sync/callbacks.rs

@@ -7,6 +7,7 @@ use std::sync::{Arc, Mutex};
 use std::time::Duration;
 
 use dash_spv_ffi::*;
+use key_wallet_ffi::managed_account::FFITransactionRecord;
 use key_wallet_ffi::types::FFITransactionContext;
 
 /// Tracks callback invocations for verification.
@@ -48,9 +49,8 @@ pub(super) struct CallbackTracker {
     pub(super) connected_peers: Mutex<Vec<String>>,
     pub(super) errors: Mutex<Vec<String>>,
 
-    // Transaction data from on_transaction_received
-    pub(super) received_txids: Mutex<Vec<[u8; 32]>>,
-    pub(super) received_amounts: Mutex<Vec<i64>>,
+    // Transaction data from on_transaction_received (txid, net_amount)
+    pub(super) received_transactions: Mutex<Vec<([u8; 32], i64)>>,
 
     // Balance data from on_balance_updated
     pub(super) last_spendable: AtomicU64,
@@ -343,29 +343,24 @@ extern "C" fn on_peers_updated(connected_count: u32, best_height: u32, user_data
 
 extern "C" fn on_transaction_received(
     wallet_id: *const c_char,
-    _status: FFITransactionContext,
     account_index: u32,
-    txid: *const [u8; 32],
-    amount: i64,
-    _addresses: *const c_char,
+    record: *const FFITransactionRecord,
     user_data: *mut c_void,
 ) {
     let Some(tracker) = (unsafe { tracker_from(user_data) }) else {
         return;
     };
-    if !txid.is_null() {
-        let txid_bytes = unsafe { *txid };
-        tracker.received_txids.lock().unwrap_or_else(|e| e.into_inner()).push(txid_bytes);
+    if !record.is_null() {
+        let r = unsafe { &*record };
+        tracker
+            .received_transactions
+            .lock()
+            .unwrap_or_else(|e| e.into_inner())
+            .push((r.txid, r.net_amount));
     }
-    tracker.received_amounts.lock().unwrap_or_else(|e| e.into_inner()).push(amount);
     tracker.transaction_received_count.fetch_add(1, Ordering::SeqCst);
     let wallet_str = unsafe { cstr_or_unknown(wallet_id) };
-    tracing::info!(
-        "on_transaction_received: wallet={}, account={}, amount={}",
-        wallet_str,
-        account_index,
-        amount
-    );
+    tracing::info!("on_transaction_received: wallet={}, account={}", wallet_str, account_index,);
 }
 
 extern "C" fn on_transaction_status_changed(

dash-spv-ffi/tests/dashd_sync/tests_callback.rs

@@ -212,20 +212,13 @@ fn test_all_callbacks_during_sync() {
         );
 
         // Validate transaction data from initial sync
-        let received_txids = tracker.received_txids.lock().unwrap();
-        assert!(!received_txids.is_empty(), "should have received transaction txids during sync");
-        drop(received_txids);
-
-        let received_amounts = tracker.received_amounts.lock().unwrap();
-        assert!(
-            !received_amounts.is_empty(),
-            "should have received transaction amounts during sync"
-        );
+        let received_txs = tracker.received_transactions.lock().unwrap();
+        assert!(!received_txs.is_empty(), "should have received transactions during sync");
         assert!(
-            received_amounts.iter().any(|&a| a != 0),
+            received_txs.iter().any(|&(_, amount)| amount != 0),
             "at least one received transaction amount should be non-zero"
         );
-        drop(received_amounts);
+        drop(received_txs);
 
         // Masternodes are disabled in test config, so these should not fire
         let masternode_updated = tracker.masternode_state_updated_count.load(Ordering::SeqCst);
@@ -308,23 +301,27 @@ fn test_callbacks_post_sync_transactions_and_disconnect() {
             tx_received_after
         );
 
-        // Verify the sent txid appears in the callback data
+        // Verify the sent txid appears in the callback data with a non-zero
+        // net_amount.  The SPV wallet and dashd share the same mnemonic so the
+        // transaction is an internal transfer (wallet owns both inputs and
+        // outputs); net_amount therefore equals approximately -fee, not the
+        // nominal send amount.
         let sent_txid_bytes = *txid.as_byte_array();
-        let received_txids = tracker.received_txids.lock().unwrap();
+        let received_txs = tracker.received_transactions.lock().unwrap();
+        let sent_entry = received_txs.iter().find(|&&(id, _)| id == sent_txid_bytes);
         assert!(
-            received_txids.contains(&sent_txid_bytes),
-            "sent txid should appear in received_txids callback data"
+            sent_entry.is_some(),
+            "sent txid should appear in received transaction callback data"
         );
-        drop(received_txids);
-
-        // Verify 1 DASH (100_000_000 satoshis) appears in received amounts
-        let received_amounts = tracker.received_amounts.lock().unwrap();
+        let &(_, net_amount) = sent_entry.unwrap();
+        // Internal transfer: net_amount = received - sent = (send_amount + change) - input = -fee.
+        // The fee must be negative, non-zero, and small (< 0.001 DASH).
         assert!(
-            received_amounts.contains(&100_000_000),
-            "1 DASH (100_000_000 sat) should appear in received_amounts: {:?}",
-            *received_amounts
+            net_amount < 0 && net_amount > -100_000,
+            "internal transfer net_amount should equal -fee (small negative), got: {}",
+            net_amount
         );
-        drop(received_amounts);
+        drop(received_txs);
 
         let balance_updated_after = tracker.balance_updated_count.load(Ordering::SeqCst);
         tracing::info!(

dash-spv-ffi/tests/dashd_sync/tests_transaction.rs

@@ -74,14 +74,14 @@ fn test_ffi_sync_then_generate_blocks() {
         );
 
         // Verify the transaction was received via wallet callback
-        let received_txids = ctx.tracker().received_txids.lock().unwrap();
+        let received_txs = ctx.tracker().received_transactions.lock().unwrap();
         let txid_bytes = *txid.as_byte_array();
         assert!(
-            received_txids.contains(&txid_bytes),
+            received_txs.iter().any(|&(txid, _)| txid == txid_bytes),
             "Wallet callback should have received txid {}",
             txid
         );
-        drop(received_txids);
+        drop(received_txs);
 
         // Verify via wallet query as well
         assert!(

dash-spv/tests/dashd_sync/helpers.rs

@@ -140,7 +140,7 @@ pub(super) async fn wait_for_mempool_tx(
             _ = &mut timeout => return None,
             result = receiver.recv() => {
                 match result {
-                    Ok(WalletEvent::TransactionReceived { txid, status: TransactionContext::Mempool, .. }) => return Some(txid),
+                    Ok(WalletEvent::TransactionReceived { ref record, .. }) if record.context == TransactionContext::Mempool => return Some(record.txid),
                     Ok(_) => continue,
                     Err(_) => return None,
                 }