From f8184aa08980dc6f7afc0664f572c95cfc3b5a30 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 1 Apr 2026 19:11:19 +1100 Subject: [PATCH 1/4] 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` - Return `TransactionRecord` from `record_transaction` and propagate via `TransactionCheckResult` - Refactor `OnTransactionReceivedCallback` to pass `*const FFITransactionRecord` --- dash-spv-ffi/src/bin/ffi_cli.rs | 21 +- dash-spv-ffi/src/callbacks.rs | 99 +++- dash-spv-ffi/tests/dashd_sync/callbacks.rs | 21 +- dash-spv/tests/dashd_sync/helpers.rs | 2 +- key-wallet-ffi/src/managed_account.rs | 225 ++++++++- key-wallet-ffi/src/types.rs | 449 ++++++++++++------ key-wallet-manager/src/event_tests.rs | 42 +- key-wallet-manager/src/events.rs | 26 +- key-wallet-manager/src/lib.rs | 21 +- key-wallet-manager/src/test_helpers.rs | 2 +- key-wallet/src/managed_account/mod.rs | 4 +- .../transaction_checking/account_checker.rs | 4 + .../transaction_checking/wallet_checker.rs | 5 +- 13 files changed, 660 insertions(+), 261 deletions(-) diff --git a/dash-spv-ffi/src/bin/ffi_cli.rs b/dash-spv-ffi/src/bin/ffi_cli.rs index 30c6efe88..786bdf0d9 100644 --- a/dash-spv-ffi/src/bin/ffi_cli.rs +++ b/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 ); } diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index cbff4419e..4627b1102 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/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,85 @@ 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 = - 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 = 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 = record + .output_details + .iter() + .map(|d| FFIOutputDetail { + index: d.index, + role: FFIOutputRole::from(d.role), + }) + .collect(); + + 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: record + .label + .as_ref() + .map(|l| CString::new(l.as_str()).unwrap().into_raw()) + .unwrap_or(std::ptr::null_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 { diff --git a/dash-spv-ffi/tests/dashd_sync/callbacks.rs b/dash-spv-ffi/tests/dashd_sync/callbacks.rs index 56a8125b0..b89bc0b6e 100644 --- a/dash-spv-ffi/tests/dashd_sync/callbacks.rs +++ b/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. @@ -343,29 +344,21 @@ 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_txids.lock().unwrap_or_else(|e| e.into_inner()).push(r.txid); + tracker.received_amounts.lock().unwrap_or_else(|e| e.into_inner()).push(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( diff --git a/dash-spv/tests/dashd_sync/helpers.rs b/dash-spv/tests/dashd_sync/helpers.rs index a9eb92481..869c34f94 100644 --- a/dash-spv/tests/dashd_sync/helpers.rs +++ b/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, } diff --git a/key-wallet-ffi/src/managed_account.rs b/key-wallet-ffi/src/managed_account.rs index b55138969..495430a4b 100644 --- a/key-wallet-ffi/src/managed_account.rs +++ b/key-wallet-ffi/src/managed_account.rs @@ -4,14 +4,17 @@ //! ManagedAccount instances from the key-wallet crate. FFIManagedCoreAccount is a //! simple wrapper around `Arc` without additional fields. -use std::os::raw::c_uint; +use std::os::raw::{c_char, c_uint}; use std::sync::Arc; use dashcore::hashes::Hash; use crate::address_pool::{FFIAddressPool, FFIAddressPoolType}; use crate::error::{FFIError, FFIErrorCode}; -use crate::types::{FFIAccountType, FFITransactionContext}; +use crate::types::{ + FFIAccountType, FFIInputDetail, FFIOutputDetail, FFIOutputRole, FFITransactionContext, + FFITransactionDirection, FFITransactionType, +}; use crate::wallet_manager::FFIWalletManager; use crate::FFINetwork; use key_wallet::account::account_collection::{DashpayAccountKey, PlatformPaymentAccountKey}; @@ -660,6 +663,8 @@ pub unsafe extern "C" fn managed_core_account_get_utxo_count( } /// FFI-compatible transaction record +/// +/// Heap-allocated fields must be freed with `managed_core_account_free_transactions`. #[repr(C)] pub struct FFITransactionRecord { /// Transaction ID (32 bytes) @@ -668,8 +673,26 @@ pub struct FFITransactionRecord { pub net_amount: i64, /// Transaction context (mempool, instant-send, in-block, chain-locked + block info) pub context: FFITransactionContext, + /// Classified transaction type + pub transaction_type: FFITransactionType, + /// Direction of the transaction relative to the wallet + pub direction: FFITransactionDirection, /// Fee if known, 0 if unknown pub fee: u64, + /// Input details array + pub input_details: *mut FFIInputDetail, + /// Number of input details + pub input_details_count: usize, + /// Output details array + pub output_details: *mut FFIOutputDetail, + /// Number of output details + pub output_details_count: usize, + /// Consensus-serialized transaction bytes + pub tx_data: *mut u8, + /// Length of `tx_data` + pub tx_len: usize, + /// Optional label (null if not set) + pub label: *mut c_char, } /// Get all transactions from a managed account @@ -717,17 +740,65 @@ pub unsafe extern "C" fn managed_core_account_get_transactions( for (i, (_txid, record)) in transactions.iter().enumerate() { let ffi_record = &mut *ptr.add(i); - // Copy txid ffi_record.txid = record.txid.to_byte_array(); - - // Copy net amount ffi_record.net_amount = record.net_amount; - - // Copy transaction context ffi_record.context = FFITransactionContext::from(record.context); - - // Copy fee (0 if unknown) + ffi_record.transaction_type = FFITransactionType::from(record.transaction_type); + ffi_record.direction = FFITransactionDirection::from(record.direction); ffi_record.fee = record.fee.unwrap_or(0); + + // Serialize transaction bytes + let tx_slice = dashcore::consensus::serialize(&record.transaction).into_boxed_slice(); + ffi_record.tx_len = tx_slice.len(); + ffi_record.tx_data = if tx_slice.is_empty() { + std::ptr::null_mut() + } else { + Box::into_raw(tx_slice) as *mut u8 + }; + + // Input details + let input_slice: Box<[FFIInputDetail]> = record + .input_details + .iter() + .map(|d| { + let addr = std::ffi::CString::new(d.address.to_string()).unwrap_or_default(); + FFIInputDetail { + index: d.index, + value: d.value, + address: addr.into_raw(), + } + }) + .collect::>() + .into_boxed_slice(); + ffi_record.input_details_count = input_slice.len(); + ffi_record.input_details = if input_slice.is_empty() { + std::ptr::null_mut() + } else { + Box::into_raw(input_slice) as *mut FFIInputDetail + }; + + // Label + ffi_record.label = match &record.label { + Some(label) => std::ffi::CString::new(label.as_str()).unwrap_or_default().into_raw(), + None => std::ptr::null_mut(), + }; + + // Output details + let output_slice: Box<[FFIOutputDetail]> = record + .output_details + .iter() + .map(|d| FFIOutputDetail { + index: d.index, + role: FFIOutputRole::from(d.role), + }) + .collect::>() + .into_boxed_slice(); + ffi_record.output_details_count = output_slice.len(); + ffi_record.output_details = if output_slice.is_empty() { + std::ptr::null_mut() + } else { + Box::into_raw(output_slice) as *mut FFIOutputDetail + }; } *transactions_out = ptr; @@ -747,13 +818,50 @@ pub unsafe extern "C" fn managed_core_account_free_transactions( transactions: *mut FFITransactionRecord, count: usize, ) { - if !transactions.is_null() && count > 0 { - let layout = match std::alloc::Layout::array::(count) { - Ok(layout) => layout, - Err(_) => return, - }; - std::alloc::dealloc(transactions as *mut u8, layout); + if transactions.is_null() || count == 0 { + return; } + + for i in 0..count { + let record = &*transactions.add(i); + + // Free input detail addresses first, then the array + if !record.input_details.is_null() && record.input_details_count > 0 { + let slice = + std::slice::from_raw_parts_mut(record.input_details, record.input_details_count); + for detail in slice.iter() { + if !detail.address.is_null() { + drop(std::ffi::CString::from_raw(detail.address)); + } + } + drop(Box::from_raw(slice as *mut [FFIInputDetail])); + } + + // Free output details + if !record.output_details.is_null() && record.output_details_count > 0 { + drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut( + record.output_details, + record.output_details_count, + ))); + } + + // Free tx data + if !record.tx_data.is_null() && record.tx_len > 0 { + drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(record.tx_data, record.tx_len))); + } + + // Free label + if !record.label.is_null() { + drop(std::ffi::CString::from_raw(record.label)); + } + } + + // Free the main array + let layout = match std::alloc::Layout::array::(count) { + Ok(layout) => layout, + Err(_) => return, + }; + std::alloc::dealloc(transactions as *mut u8, layout); } /// Free a managed account handle @@ -1339,7 +1447,11 @@ pub unsafe extern "C" fn managed_platform_account_result_free_error( mod tests { use super::*; use crate::address_pool::address_pool_free; - use crate::types::{FFIAccountCreationOptionType, FFIWalletAccountCreationOptions}; + use crate::types::{ + FFIAccountCreationOptionType, FFIBlockInfo, FFIInputDetail, FFIOutputDetail, FFIOutputRole, + FFITransactionContext, FFITransactionContextType, FFITransactionDirection, + FFITransactionType, FFIWalletAccountCreationOptions, + }; use crate::wallet_manager::{ wallet_manager_add_wallet_from_mnemonic_with_options, wallet_manager_create, wallet_manager_free, wallet_manager_free_wallet_ids, wallet_manager_get_wallet_ids, @@ -1876,4 +1988,85 @@ mod tests { address_pool_free(ptr::null_mut()); } } + + #[test] + fn test_free_transactions_null_safety() { + unsafe { + managed_core_account_free_transactions(std::ptr::null_mut(), 0); + managed_core_account_free_transactions(std::ptr::null_mut(), 5); + } + } + + #[test] + fn test_ffi_transaction_record_roundtrip() { + unsafe { + let count = 2; + let layout = std::alloc::Layout::array::(count).unwrap(); + let records = std::alloc::alloc(layout) as *mut FFITransactionRecord; + assert!(!records.is_null()); + + // First record: with sub-allocations + let r0 = &mut *records.add(0); + r0.txid = [0xaa; 32]; + r0.net_amount = 50000; + r0.context = FFITransactionContext { + context_type: FFITransactionContextType::Mempool, + block_info: FFIBlockInfo::empty(), + }; + r0.transaction_type = FFITransactionType::Standard; + r0.direction = FFITransactionDirection::Incoming; + r0.fee = 226; + + // Create input details + let addr = std::ffi::CString::new("XtestAddress123").unwrap(); + let input_slice = vec![FFIInputDetail { + index: 0, + value: 100000, + address: addr.into_raw(), + }] + .into_boxed_slice(); + r0.input_details_count = input_slice.len(); + r0.input_details = Box::into_raw(input_slice) as *mut FFIInputDetail; + + // Create output details + let output_slice = vec![FFIOutputDetail { + index: 0, + role: FFIOutputRole::Received, + }] + .into_boxed_slice(); + r0.output_details_count = output_slice.len(); + r0.output_details = Box::into_raw(output_slice) as *mut FFIOutputDetail; + + // Create tx data + let tx_slice = vec![0u8; 10].into_boxed_slice(); + r0.tx_len = tx_slice.len(); + r0.tx_data = Box::into_raw(tx_slice) as *mut u8; + + // Create label + let label = std::ffi::CString::new("Payment for coffee").unwrap(); + r0.label = label.into_raw(); + + // Second record: empty sub-arrays + let r1 = &mut *records.add(1); + r1.txid = [0xbb; 32]; + r1.net_amount = -10000; + r1.context = FFITransactionContext { + context_type: FFITransactionContextType::Mempool, + block_info: FFIBlockInfo::empty(), + }; + r1.transaction_type = FFITransactionType::Standard; + r1.direction = FFITransactionDirection::Outgoing; + r1.fee = 0; + r1.input_details = std::ptr::null_mut(); + r1.input_details_count = 0; + r1.output_details = std::ptr::null_mut(); + r1.output_details_count = 0; + r1.tx_data = std::ptr::null_mut(); + r1.tx_len = 0; + r1.label = std::ptr::null_mut(); + + // Free should not crash + managed_core_account_free_transactions(records, count); + } + } } diff --git a/key-wallet-ffi/src/types.rs b/key-wallet-ffi/src/types.rs index a0f81d188..53e65b37a 100644 --- a/key-wallet-ffi/src/types.rs +++ b/key-wallet-ffi/src/types.rs @@ -1,6 +1,8 @@ //! Common types for FFI interface use dashcore::hashes::Hash; +use key_wallet::managed_account::transaction_record::{OutputRole, TransactionDirection}; +use key_wallet::transaction_checking::transaction_router::TransactionType; use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; use key_wallet::{Network, Wallet}; use std::os::raw::c_char; @@ -432,147 +434,6 @@ impl FFIAccountType { } } -#[cfg(test)] -mod tests { - use super::*; - - fn valid_block_info() -> FFIBlockInfo { - FFIBlockInfo { - height: 1000, - block_hash: [0xab; 32], - timestamp: 1700000000, - } - } - - #[test] - #[should_panic(expected = "DashpayReceivingFunds cannot be converted to AccountType")] - fn test_dashpay_receiving_funds_to_account_type_panics() { - // This should panic because we cannot construct a DashPay account without identity IDs - let _ = FFIAccountType::DashpayReceivingFunds.to_account_type(0); - } - - #[test] - #[should_panic(expected = "DashpayExternalAccount cannot be converted to AccountType")] - fn test_dashpay_external_account_to_account_type_panics() { - // This should panic because we cannot construct a DashPay account without identity IDs - let _ = FFIAccountType::DashpayExternalAccount.to_account_type(0); - } - - #[test] - #[should_panic(expected = "PlatformPayment cannot be converted to AccountType")] - fn test_platform_payment_to_account_type_panics() { - // This should panic because we cannot construct a Platform Payment account without indices - let _ = FFIAccountType::PlatformPayment.to_account_type(0); - } - - #[test] - #[should_panic(expected = "Cannot convert AccountType::DashpayReceivingFunds")] - fn test_dashpay_receiving_funds_from_account_type_panics() { - // This should panic because we cannot represent identity IDs in the FFI tuple - let account_type = key_wallet::AccountType::DashpayReceivingFunds { - index: 0, - user_identity_id: [1u8; 32], - friend_identity_id: [2u8; 32], - }; - let _ = FFIAccountType::from_account_type(&account_type); - } - - #[test] - #[should_panic(expected = "Cannot convert AccountType::DashpayExternalAccount")] - fn test_dashpay_external_account_from_account_type_panics() { - // This should panic because we cannot represent identity IDs in the FFI tuple - let account_type = key_wallet::AccountType::DashpayExternalAccount { - index: 0, - user_identity_id: [1u8; 32], - friend_identity_id: [2u8; 32], - }; - let _ = FFIAccountType::from_account_type(&account_type); - } - - #[test] - fn test_non_dashpay_conversions_work() { - // Verify that non-DashPay types still convert correctly - let standard_bip44 = FFIAccountType::StandardBIP44.to_account_type(5); - assert!(matches!( - standard_bip44, - key_wallet::AccountType::Standard { - index: 5, - .. - } - )); - - let coinjoin = FFIAccountType::CoinJoin.to_account_type(3); - assert!(matches!( - coinjoin, - key_wallet::AccountType::CoinJoin { - index: 3 - } - )); - - // Test reverse conversion - let (ffi_type, index, _) = FFIAccountType::from_account_type(&standard_bip44); - assert_eq!(ffi_type, FFIAccountType::StandardBIP44); - assert_eq!(index, 5); - } - - #[test] - fn transaction_context_from_ffi_mempool_with_empty_block_info() { - let result = transaction_context_from_ffi( - FFITransactionContextType::Mempool, - &FFIBlockInfo::empty(), - ); - assert!(matches!(result, Some(TransactionContext::Mempool))); - } - - #[test] - fn transaction_context_from_ffi_instant_send_with_empty_block_info() { - let result = transaction_context_from_ffi( - FFITransactionContextType::InstantSend, - &FFIBlockInfo::empty(), - ); - assert!(matches!(result, Some(TransactionContext::InstantSend))); - } - - #[test] - fn transaction_context_from_ffi_in_block_with_empty_block_info() { - let result = transaction_context_from_ffi( - FFITransactionContextType::InBlock, - &FFIBlockInfo::empty(), - ); - assert!(result.is_none()); - } - - #[test] - fn transaction_context_from_ffi_in_chain_locked_block_with_empty_block_info() { - let result = transaction_context_from_ffi( - FFITransactionContextType::InChainLockedBlock, - &FFIBlockInfo::empty(), - ); - assert!(result.is_none()); - } - - #[test] - fn transaction_context_from_ffi_in_block_with_valid_block_info() { - let block_info = valid_block_info(); - let result = transaction_context_from_ffi(FFITransactionContextType::InBlock, &block_info); - let ctx = result.expect("should return Some for InBlock with valid block info"); - assert!(matches!(ctx, TransactionContext::InBlock(info) if info.height() == 1000)); - } - - #[test] - fn transaction_context_from_ffi_in_chain_locked_block_with_valid_block_info() { - let block_info = valid_block_info(); - let result = transaction_context_from_ffi( - FFITransactionContextType::InChainLockedBlock, - &block_info, - ); - let ctx = result.expect("should return Some for InChainLockedBlock with valid block info"); - assert!( - matches!(ctx, TransactionContext::InChainLockedBlock(info) if info.height() == 1000) - ); - } -} - /// Address type enumeration #[repr(C)] #[derive(Debug, Clone, Copy)] @@ -921,3 +782,309 @@ impl From for FFITransactionContext { } } } + +/// FFI-compatible transaction direction +#[repr(C)] +#[derive(Debug, Clone, Copy)] +pub enum FFITransactionDirection { + Incoming = 0, + Outgoing = 1, + Internal = 2, + CoinJoin = 3, +} + +impl From for FFITransactionDirection { + fn from(dir: TransactionDirection) -> Self { + match dir { + TransactionDirection::Incoming => Self::Incoming, + TransactionDirection::Outgoing => Self::Outgoing, + TransactionDirection::Internal => Self::Internal, + TransactionDirection::CoinJoin => Self::CoinJoin, + } + } +} + +/// FFI-compatible transaction type classification +#[repr(C)] +#[derive(Debug, Clone, Copy)] +pub enum FFITransactionType { + Standard = 0, + CoinJoin = 1, + ProviderRegistration = 2, + ProviderUpdateRegistrar = 3, + ProviderUpdateService = 4, + ProviderUpdateRevocation = 5, + AssetLock = 6, + AssetUnlock = 7, + Coinbase = 8, + Ignored = 9, +} + +impl From for FFITransactionType { + fn from(tt: TransactionType) -> Self { + match tt { + TransactionType::Standard => Self::Standard, + TransactionType::CoinJoin => Self::CoinJoin, + TransactionType::ProviderRegistration => Self::ProviderRegistration, + TransactionType::ProviderUpdateRegistrar => Self::ProviderUpdateRegistrar, + TransactionType::ProviderUpdateService => Self::ProviderUpdateService, + TransactionType::ProviderUpdateRevocation => Self::ProviderUpdateRevocation, + TransactionType::AssetLock => Self::AssetLock, + TransactionType::AssetUnlock => Self::AssetUnlock, + TransactionType::Coinbase => Self::Coinbase, + TransactionType::Ignored => Self::Ignored, + } + } +} + +/// FFI-compatible output role +#[repr(C)] +#[derive(Debug, Clone, Copy)] +pub enum FFIOutputRole { + Received = 0, + Change = 1, + Sent = 2, + Unspendable = 3, +} + +impl From for FFIOutputRole { + fn from(role: OutputRole) -> Self { + match role { + OutputRole::Received => Self::Received, + OutputRole::Change => Self::Change, + OutputRole::Sent => Self::Sent, + OutputRole::Unspendable => Self::Unspendable, + } + } +} + +/// FFI-compatible input detail +#[repr(C)] +pub struct FFIInputDetail { + pub index: u32, + pub value: u64, + pub address: *mut std::os::raw::c_char, +} + +/// FFI-compatible output detail +#[repr(C)] +pub struct FFIOutputDetail { + pub index: u32, + pub role: FFIOutputRole, +} + +#[cfg(test)] +mod tests { + use super::*; + use key_wallet::transaction_checking::BlockInfo; + + fn valid_block_info() -> FFIBlockInfo { + FFIBlockInfo { + height: 1000, + block_hash: [0xab; 32], + timestamp: 1700000000, + } + } + + #[test] + #[should_panic(expected = "DashpayReceivingFunds cannot be converted to AccountType")] + fn test_dashpay_receiving_funds_to_account_type_panics() { + // This should panic because we cannot construct a DashPay account without identity IDs + let _ = FFIAccountType::DashpayReceivingFunds.to_account_type(0); + } + + #[test] + #[should_panic(expected = "DashpayExternalAccount cannot be converted to AccountType")] + fn test_dashpay_external_account_to_account_type_panics() { + // This should panic because we cannot construct a DashPay account without identity IDs + let _ = FFIAccountType::DashpayExternalAccount.to_account_type(0); + } + + #[test] + #[should_panic(expected = "PlatformPayment cannot be converted to AccountType")] + fn test_platform_payment_to_account_type_panics() { + // This should panic because we cannot construct a Platform Payment account without indices + let _ = FFIAccountType::PlatformPayment.to_account_type(0); + } + + #[test] + #[should_panic(expected = "Cannot convert AccountType::DashpayReceivingFunds")] + fn test_dashpay_receiving_funds_from_account_type_panics() { + // This should panic because we cannot represent identity IDs in the FFI tuple + let account_type = key_wallet::AccountType::DashpayReceivingFunds { + index: 0, + user_identity_id: [1u8; 32], + friend_identity_id: [2u8; 32], + }; + let _ = FFIAccountType::from_account_type(&account_type); + } + + #[test] + #[should_panic(expected = "Cannot convert AccountType::DashpayExternalAccount")] + fn test_dashpay_external_account_from_account_type_panics() { + // This should panic because we cannot represent identity IDs in the FFI tuple + let account_type = key_wallet::AccountType::DashpayExternalAccount { + index: 0, + user_identity_id: [1u8; 32], + friend_identity_id: [2u8; 32], + }; + let _ = FFIAccountType::from_account_type(&account_type); + } + + #[test] + fn test_non_dashpay_conversions_work() { + // Verify that non-DashPay types still convert correctly + let standard_bip44 = FFIAccountType::StandardBIP44.to_account_type(5); + assert!(matches!( + standard_bip44, + key_wallet::AccountType::Standard { + index: 5, + .. + } + )); + + let coinjoin = FFIAccountType::CoinJoin.to_account_type(3); + assert!(matches!( + coinjoin, + key_wallet::AccountType::CoinJoin { + index: 3 + } + )); + + // Test reverse conversion + let (ffi_type, index, _) = FFIAccountType::from_account_type(&standard_bip44); + assert_eq!(ffi_type, FFIAccountType::StandardBIP44); + assert_eq!(index, 5); + } + + #[test] + fn transaction_context_from_ffi_mempool_with_empty_block_info() { + let result = transaction_context_from_ffi( + FFITransactionContextType::Mempool, + &FFIBlockInfo::empty(), + ); + assert!(matches!(result, Some(TransactionContext::Mempool))); + } + + #[test] + fn transaction_context_from_ffi_instant_send_with_empty_block_info() { + let result = transaction_context_from_ffi( + FFITransactionContextType::InstantSend, + &FFIBlockInfo::empty(), + ); + assert!(matches!(result, Some(TransactionContext::InstantSend))); + } + + #[test] + fn transaction_context_from_ffi_in_block_with_empty_block_info() { + let result = transaction_context_from_ffi( + FFITransactionContextType::InBlock, + &FFIBlockInfo::empty(), + ); + assert!(result.is_none()); + } + + #[test] + fn transaction_context_from_ffi_in_chain_locked_block_with_empty_block_info() { + let result = transaction_context_from_ffi( + FFITransactionContextType::InChainLockedBlock, + &FFIBlockInfo::empty(), + ); + assert!(result.is_none()); + } + + #[test] + fn transaction_context_from_ffi_in_block_with_valid_block_info() { + let block_info = valid_block_info(); + let result = transaction_context_from_ffi(FFITransactionContextType::InBlock, &block_info); + let ctx = result.expect("should return Some for InBlock with valid block info"); + assert!(matches!(ctx, TransactionContext::InBlock(info) if info.height() == 1000)); + } + + #[test] + fn transaction_context_from_ffi_in_chain_locked_block_with_valid_block_info() { + let block_info = valid_block_info(); + let result = transaction_context_from_ffi( + FFITransactionContextType::InChainLockedBlock, + &block_info, + ); + let ctx = result.expect("should return Some for InChainLockedBlock with valid block info"); + assert!( + matches!(ctx, TransactionContext::InChainLockedBlock(info) if info.height() == 1000) + ); + } + + #[test] + fn test_ffi_transaction_direction_from() { + assert!(matches!( + FFITransactionDirection::from(TransactionDirection::Incoming), + FFITransactionDirection::Incoming + )); + assert!(matches!( + FFITransactionDirection::from(TransactionDirection::Outgoing), + FFITransactionDirection::Outgoing + )); + assert!(matches!( + FFITransactionDirection::from(TransactionDirection::Internal), + FFITransactionDirection::Internal + )); + assert!(matches!( + FFITransactionDirection::from(TransactionDirection::CoinJoin), + FFITransactionDirection::CoinJoin + )); + } + + #[test] + fn test_ffi_transaction_type_from() { + assert!(matches!( + FFITransactionType::from(TransactionType::Standard), + FFITransactionType::Standard + )); + assert!(matches!( + FFITransactionType::from(TransactionType::CoinJoin), + FFITransactionType::CoinJoin + )); + assert!(matches!( + FFITransactionType::from(TransactionType::ProviderRegistration), + FFITransactionType::ProviderRegistration + )); + assert!(matches!( + FFITransactionType::from(TransactionType::AssetLock), + FFITransactionType::AssetLock + )); + assert!(matches!( + FFITransactionType::from(TransactionType::Coinbase), + FFITransactionType::Coinbase + )); + assert!(matches!( + FFITransactionType::from(TransactionType::Ignored), + FFITransactionType::Ignored + )); + } + + #[test] + fn test_ffi_transaction_context_from_in_block() { + let hash = dashcore::BlockHash::from_byte_array([0xab; 32]); + let block_info = BlockInfo::new(1000, hash, 1700000000); + let ctx = FFITransactionContext::from(TransactionContext::InBlock(block_info)); + assert!(matches!(ctx.context_type, FFITransactionContextType::InBlock)); + assert_eq!(ctx.block_info.height, 1000); + assert_eq!(ctx.block_info.block_hash, [0xab; 32]); + assert_eq!(ctx.block_info.timestamp, 1700000000); + } + + #[test] + fn test_ffi_transaction_context_from_mempool() { + let ctx = FFITransactionContext::from(TransactionContext::Mempool); + assert!(matches!(ctx.context_type, FFITransactionContextType::Mempool)); + assert_eq!(ctx.block_info.block_hash, [0u8; 32]); + } + + #[test] + fn test_ffi_output_role_from() { + assert!(matches!(FFIOutputRole::from(OutputRole::Received), FFIOutputRole::Received)); + assert!(matches!(FFIOutputRole::from(OutputRole::Change), FFIOutputRole::Change)); + assert!(matches!(FFIOutputRole::from(OutputRole::Sent), FFIOutputRole::Sent)); + assert!(matches!(FFIOutputRole::from(OutputRole::Unspendable), FFIOutputRole::Unspendable)); + } +} diff --git a/key-wallet-manager/src/event_tests.rs b/key-wallet-manager/src/event_tests.rs index ba5fa886b..9b1a4308a 100644 --- a/key-wallet-manager/src/event_tests.rs +++ b/key-wallet-manager/src/event_tests.rs @@ -20,16 +20,14 @@ async fn test_mempool_to_confirmed_event_flow() { let event = assert_single_event(&mut rx); match event { WalletEvent::TransactionReceived { - txid: ev_txid, wallet_id: ev_wid, - status, - amount, + record, .. } => { - assert_eq!(status, TransactionContext::Mempool); - assert_eq!(ev_txid, tx.txid()); + assert_eq!(record.context, TransactionContext::Mempool); + assert_eq!(record.txid, tx.txid()); assert_eq!(ev_wid, wallet_id); - assert_eq!(amount, TX_AMOUNT as i64); + assert_eq!(record.net_amount, TX_AMOUNT as i64); } other => panic!("expected TransactionReceived, got {:?}", other), } @@ -455,21 +453,23 @@ async fn test_process_block_emits_events() { match event { WalletEvent::TransactionReceived { - status, account_index, - addresses, + record, .. } => { assert!( matches!( - status, - TransactionContext::InBlock(info) if info.height() == 1000 - ), + record.context, + TransactionContext::InBlock(info) if info.height() == 1000 + ), "expected InBlock at height 1000, got {:?}", - status + record.context ); assert_eq!(*account_index, 0); - assert!(!addresses.is_empty(), "expected non-empty addresses"); + assert!( + !record.input_details.is_empty() || !record.output_details.is_empty(), + "expected non-empty details" + ); } _ => unreachable!(), } @@ -549,11 +549,9 @@ async fn test_mempool_to_block_to_chainlocked_event_flow() { let event = assert_single_event(&mut rx); assert!( matches!( - event, - WalletEvent::TransactionReceived { - status: TransactionContext::Mempool, - .. - } + &event, + WalletEvent::TransactionReceived { record, .. } + if record.context == TransactionContext::Mempool ), "expected TransactionReceived(Mempool), got {:?}", event @@ -605,11 +603,9 @@ async fn test_chainlocked_block_event_flow() { let event = assert_single_event(&mut rx); assert!( matches!( - event, - WalletEvent::TransactionReceived { - status: TransactionContext::InChainLockedBlock(info), - .. - } if info.height() == 2000 + &event, + WalletEvent::TransactionReceived { record, .. } + if matches!(record.context, TransactionContext::InChainLockedBlock(info) if info.height() == 2000) ), "expected TransactionReceived(InChainLockedBlock at 2000), got {:?}", event diff --git a/key-wallet-manager/src/events.rs b/key-wallet-manager/src/events.rs index 7fc63ceab..77999f29e 100644 --- a/key-wallet-manager/src/events.rs +++ b/key-wallet-manager/src/events.rs @@ -3,10 +3,12 @@ //! These events are emitted by the WalletManager when significant wallet //! operations occur, allowing consumers to receive push-based notifications. -use crate::WalletId; -use dashcore::{Address, Amount, SignedAmount, Txid}; +use dashcore::{Amount, SignedAmount, Txid}; +use key_wallet::managed_account::transaction_record::TransactionRecord; use key_wallet::transaction_checking::TransactionContext; +use crate::WalletId; + /// Events emitted by the wallet manager. /// /// Each event represents a meaningful wallet state change that consumers @@ -17,16 +19,10 @@ pub enum WalletEvent { TransactionReceived { /// ID of the affected wallet. wallet_id: WalletId, - /// Context at the time the transaction was first seen. - status: TransactionContext, /// Account index within the wallet. account_index: u32, - /// Transaction ID. - txid: Txid, - /// Net amount change (positive for incoming, negative for outgoing). - amount: i64, - /// Addresses involved in the transaction. - addresses: Vec
, + /// The full transaction record with all details. + record: Box, }, /// The confirmation status of a previously seen transaction has changed. TransactionStatusChanged { @@ -57,16 +53,14 @@ impl WalletEvent { pub fn description(&self) -> String { match self { WalletEvent::TransactionReceived { - txid, - amount, - status, + record, .. } => { format!( "TransactionReceived(txid={}, amount={}, status={})", - txid, - SignedAmount::from_sat(*amount), - status + record.txid, + SignedAmount::from_sat(record.net_amount), + record.context ) } WalletEvent::TransactionStatusChanged { diff --git a/key-wallet-manager/src/lib.rs b/key-wallet-manager/src/lib.rs index 493acb58d..f1412de89 100644 --- a/key-wallet-manager/src/lib.rs +++ b/key-wallet-manager/src/lib.rs @@ -506,28 +506,11 @@ impl WalletManager { } if check_result.is_new_transaction { - // First time seeing this transaction — emit TransactionReceived - for account_match in &check_result.affected_accounts { - let Some(account_index) = - account_match.account_type_match.account_index() - else { - continue; - }; - let amount = account_match.received as i64 - account_match.sent as i64; - let addresses: Vec
= account_match - .account_type_match - .all_involved_addresses() - .into_iter() - .map(|info| info.address) - .collect(); - + for (account_index, record) in check_result.new_records { let event = WalletEvent::TransactionReceived { wallet_id, - status: context, account_index, - txid: tx.txid(), - amount, - addresses, + record: Box::new(record), }; let _ = self.event_sender.send(event); } diff --git a/key-wallet-manager/src/test_helpers.rs b/key-wallet-manager/src/test_helpers.rs index eaa0fd3b5..6b3a8dcae 100644 --- a/key-wallet-manager/src/test_helpers.rs +++ b/key-wallet-manager/src/test_helpers.rs @@ -81,7 +81,7 @@ pub(crate) async fn assert_lifecycle_flow(contexts: &[TransactionContext], input if i == 0 { assert!( - matches!(event, WalletEvent::TransactionReceived { wallet_id: wid, status, .. } if wid == wallet_id && status == *ctx), + matches!(&event, WalletEvent::TransactionReceived { wallet_id: wid, record, .. } if *wid == wallet_id && record.context == *ctx), "context[{}]: expected TransactionReceived with wallet_id and status {:?}, got {:?}", i, ctx, diff --git a/key-wallet/src/managed_account/mod.rs b/key-wallet/src/managed_account/mod.rs index a56d746a2..e6d5d117a 100644 --- a/key-wallet/src/managed_account/mod.rs +++ b/key-wallet/src/managed_account/mod.rs @@ -441,7 +441,7 @@ impl ManagedCoreAccount { account_match: &AccountMatch, context: TransactionContext, transaction_type: TransactionType, - ) { + ) -> TransactionRecord { let net_amount = account_match.received as i64 - account_match.sent as i64; let receive_addrs: HashSet<_> = account_match @@ -532,9 +532,11 @@ impl ManagedCoreAccount { net_amount, ); + let record = tx_record.clone(); self.transactions.insert(tx.txid(), tx_record); self.update_utxos(tx, account_match, context); + record } /// Mark all UTXOs belonging to a transaction as InstantSend-locked. diff --git a/key-wallet/src/transaction_checking/account_checker.rs b/key-wallet/src/transaction_checking/account_checker.rs index c5e0b8212..a9750f1aa 100644 --- a/key-wallet/src/transaction_checking/account_checker.rs +++ b/key-wallet/src/transaction_checking/account_checker.rs @@ -9,6 +9,7 @@ use super::transaction_router::AccountTypeToCheck; use crate::account::{ManagedAccountCollection, ManagedCoreAccount}; use crate::managed_account::address_pool::{AddressInfo, PublicKeyType}; use crate::managed_account::managed_account_type::ManagedAccountType; +use crate::managed_account::transaction_record::TransactionRecord; use crate::Address; use dashcore::address::Payload; use dashcore::blockdata::transaction::Transaction; @@ -45,6 +46,8 @@ pub struct TransactionCheckResult { pub total_received_for_credit_conversion: u64, /// New addresses generated during gap limit maintenance pub new_addresses: Vec
, + /// Transaction records created for new transactions, paired with their account index + pub new_records: Vec<(u32, TransactionRecord)>, } /// Enum representing the type of Core account that matched with embedded data @@ -372,6 +375,7 @@ impl ManagedAccountCollection { total_sent: 0, total_received_for_credit_conversion: 0, new_addresses: Vec::new(), + new_records: Vec::new(), }; for account_type in account_types { diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index 7f58d2413..e00c8f647 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -124,7 +124,10 @@ impl WalletTransactionChecker for ManagedWalletInfo { }; if is_new { - account.record_transaction(tx, &account_match, context, tx_type); + let record = account.record_transaction(tx, &account_match, context, tx_type); + if let Some(account_index) = account_match.account_type_match.account_index() { + result.new_records.push((account_index, record)); + } result.state_modified = true; } else if account.confirm_transaction(tx, &account_match, context, tx_type) { result.state_modified = true; From 76f8a09683232acb130751f4c71b4b7d43857ddd Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 1 Apr 2026 22:04:01 +1100 Subject: [PATCH 2/4] fix: prevent `label` CString memory leak in transaction callback Addresses CodeRabbit review comment on PR #614 https://github.com/dashpay/rust-dashcore/pull/614#discussion_r3021281445 --- dash-spv-ffi/src/callbacks.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index 4627b1102..b24184683 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -730,6 +730,11 @@ impl FFIWalletEventCallbacks { }) .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, @@ -755,11 +760,9 @@ impl FFIWalletEventCallbacks { tx_bytes.as_ptr() as *mut _ }, tx_len: tx_bytes.len(), - label: record - .label + label: c_label .as_ref() - .map(|l| CString::new(l.as_str()).unwrap().into_raw()) - .unwrap_or(std::ptr::null_mut()), + .map_or(std::ptr::null_mut(), |l| l.as_ptr() as *mut _), }; cb( From 06490f95a5edb8d6f8045acc9fcb2f87112f14fc Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 1 Apr 2026 22:05:49 +1100 Subject: [PATCH 3/4] refactor: combine received txid/amount into single mutex in test tracker Addresses CodeRabbit review comment on PR #614 https://github.com/dashpay/rust-dashcore/pull/614#discussion_r3021281455 --- dash-spv-ffi/src/callbacks.rs | 6 +-- dash-spv-ffi/tests/dashd_sync/callbacks.rs | 12 +++--- .../tests/dashd_sync/tests_callback.rs | 37 +++++++------------ .../tests/dashd_sync/tests_transaction.rs | 6 +-- 4 files changed, 26 insertions(+), 35 deletions(-) diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index b24184683..8aefa815f 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -730,10 +730,8 @@ impl FFIWalletEventCallbacks { }) .collect(); - let c_label = record - .label - .as_ref() - .map(|l| CString::new(l.as_str()).unwrap_or_default()); + 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(), diff --git a/dash-spv-ffi/tests/dashd_sync/callbacks.rs b/dash-spv-ffi/tests/dashd_sync/callbacks.rs index b89bc0b6e..4e4c67ab0 100644 --- a/dash-spv-ffi/tests/dashd_sync/callbacks.rs +++ b/dash-spv-ffi/tests/dashd_sync/callbacks.rs @@ -49,9 +49,8 @@ pub(super) struct CallbackTracker { pub(super) connected_peers: Mutex>, pub(super) errors: Mutex>, - // Transaction data from on_transaction_received - pub(super) received_txids: Mutex>, - pub(super) received_amounts: Mutex>, + // Transaction data from on_transaction_received (txid, net_amount) + pub(super) received_transactions: Mutex>, // Balance data from on_balance_updated pub(super) last_spendable: AtomicU64, @@ -353,8 +352,11 @@ extern "C" fn on_transaction_received( }; if !record.is_null() { let r = unsafe { &*record }; - tracker.received_txids.lock().unwrap_or_else(|e| e.into_inner()).push(r.txid); - tracker.received_amounts.lock().unwrap_or_else(|e| e.into_inner()).push(r.net_amount); + tracker + .received_transactions + .lock() + .unwrap_or_else(|e| e.into_inner()) + .push((r.txid, r.net_amount)); } tracker.transaction_received_count.fetch_add(1, Ordering::SeqCst); let wallet_str = unsafe { cstr_or_unknown(wallet_id) }; diff --git a/dash-spv-ffi/tests/dashd_sync/tests_callback.rs b/dash-spv-ffi/tests/dashd_sync/tests_callback.rs index 67881b591..fad54eedd 100644 --- a/dash-spv-ffi/tests/dashd_sync/tests_callback.rs +++ b/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,21 @@ fn test_callbacks_post_sync_transactions_and_disconnect() { tx_received_after ); - // Verify the sent txid appears in the callback data + // Verify the sent txid and amount appear paired in the callback data let sent_txid_bytes = *txid.as_byte_array(); - let received_txids = tracker.received_txids.lock().unwrap(); + let received_txs = tracker.received_transactions.lock().unwrap(); assert!( - received_txids.contains(&sent_txid_bytes), - "sent txid should appear in received_txids callback data" + received_txs.iter().any(|&(txid, _)| txid == sent_txid_bytes), + "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(); assert!( - received_amounts.contains(&100_000_000), - "1 DASH (100_000_000 sat) should appear in received_amounts: {:?}", - *received_amounts + received_txs + .iter() + .any(|&(txid, amount)| txid == sent_txid_bytes && amount == 100_000_000), + "sent txid should be paired with 1 DASH (100_000_000 sat): {:?}", + *received_txs ); - drop(received_amounts); + drop(received_txs); let balance_updated_after = tracker.balance_updated_count.load(Ordering::SeqCst); tracing::info!( diff --git a/dash-spv-ffi/tests/dashd_sync/tests_transaction.rs b/dash-spv-ffi/tests/dashd_sync/tests_transaction.rs index df897865c..b97c5d4a4 100644 --- a/dash-spv-ffi/tests/dashd_sync/tests_transaction.rs +++ b/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!( From 416d91e0b543dcaefb2614fc200d5cfcc069e295 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 1 Apr 2026 22:40:12 +1100 Subject: [PATCH 4/4] 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. --- .../tests/dashd_sync/tests_callback.rs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/dash-spv-ffi/tests/dashd_sync/tests_callback.rs b/dash-spv-ffi/tests/dashd_sync/tests_callback.rs index fad54eedd..e7396e2fd 100644 --- a/dash-spv-ffi/tests/dashd_sync/tests_callback.rs +++ b/dash-spv-ffi/tests/dashd_sync/tests_callback.rs @@ -301,19 +301,25 @@ fn test_callbacks_post_sync_transactions_and_disconnect() { tx_received_after ); - // Verify the sent txid and amount appear paired 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_txs = tracker.received_transactions.lock().unwrap(); + let sent_entry = received_txs.iter().find(|&&(id, _)| id == sent_txid_bytes); assert!( - received_txs.iter().any(|&(txid, _)| txid == sent_txid_bytes), + sent_entry.is_some(), "sent txid should appear in received transaction callback data" ); + 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_txs - .iter() - .any(|&(txid, amount)| txid == sent_txid_bytes && amount == 100_000_000), - "sent txid should be paired with 1 DASH (100_000_000 sat): {:?}", - *received_txs + net_amount < 0 && net_amount > -100_000, + "internal transfer net_amount should equal -fee (small negative), got: {}", + net_amount ); drop(received_txs);