Skip to content

Commit c877c1a

Browse files
feat: various notification / statistics and performance improvements (#157)
* Fix missing statistics updates for filter matches and relevant transactions This commit fixes two statistics tracking issues that were accidentally omitted during the wallet integration refactoring: 1. filters_matched statistic was not being incremented when filter matches were detected in sequential sync (sequential/mod.rs:1390-1398) 2. blocks_with_relevant_transactions statistic was not being incremented when the wallet found relevant transactions in blocks (block_processor.rs:238-242) Changes: - Added stats field to SequentialSyncManager struct to enable statistics updates during filter matching - Updated SequentialSyncManager::new() to accept stats parameter - Added filters_matched increment when filter match is detected - Added blocks_with_relevant_transactions increment when wallet processes relevant transactions - Updated DashSpvClient to pass stats when creating SequentialSyncManager Root cause: These statistics were tracked in old commented-out code (marked with "TODO: Re-implement with wallet integration") but were never re-added to the new simplified wallet integration code path. Result: Statistics now correctly show non-zero values for "Filters Matched" and "Blocks w/ Relevant Txs" in sync status logs, providing accurate visibility into sync progress. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Add TransactionDetected event emission in block processor - Emit individual TransactionDetected events for each relevant transaction found in blocks - Include transaction details: txid, confirmed status, block height, and amount - Remove excessive debug logging from FFI layer This enables real-time transaction notifications to Swift layer for wallet balance updates. * Fix account balance not being updated after transaction processing Root cause: When processing transactions, UTXOs were correctly added/removed from accounts, but the account's balance field was never recalculated. This caused wallet-level balance to be correct (calculated by summing account UTXOs) while individual account balances remained at zero. Changes: - Add balance recalculation after UTXO updates in wallet_checker.rs - Calculate confirmed/unconfirmed/locked balance from account's UTXO set - Call account.update_balance() with recalculated values Impact: - managed_account_get_balance() FFI now returns correct values - Multi-account wallets will now show correct per-account balances - Removes need for Swift workaround of using wallet-level balance Fixes account balance query returning zero despite correct UTXO state. * Add support for querying all addresses with (0, 0) range Allow address_pool_get_addresses_in_range to return all addresses when called with start_index=0 and end_index=0. This provides a clean way to retrieve all generated addresses without hardcoding a limit. Implementation: - Special case: when start=0 and end=0, iterate from 0 to highest_generated - Normal case: continue with existing range validation and iteration - Returns all addresses that exist in the pool, including gap limit buffer This avoids hardcoding arbitrary limits (like 10000) in client code. * Add FFI transaction list support for managed accounts Implement transaction retrieval functionality in key-wallet-ffi to expose transaction data from managed accounts to client applications. - Add FFITransactionRecord struct with transaction details (txid, amount, height, etc.) - Implement managed_account_get_transactions() to retrieve transaction array - Implement managed_account_free_transactions() for proper memory cleanup - Update C header with new FFI structures and functions This enables client applications to display wallet transaction history with complete transaction information including confirmations, fees, and transaction types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * perf(dash-spv): optimize header sync and add parallel CFHeaders flow control - Add CachedHeader wrapper to eliminate redundant X11 hash computations during header sync (4-6x reduction in hash operations per header) - Implement parallel CFHeaders synchronization with flow control, including request buffering, sequential processing, retry logic, and timeout handling - Optimize storage operations by accepting precomputed hashes in store_headers_internal to avoid redundant hash computations - Reduce disk storage index save frequency to every 10k entries instead of every periodic save to minimize expensive cloning and serialization - Use cached hashes throughout validation and storage pipeline in both headers.rs and headers_with_reorg.rs - Add configuration options for CFHeaders flow control with sensible defaults: max_concurrent_cfheaders_requests_parallel (50), cfheaders_request_timeout (30s), max_cfheaders_retries (3), enable_cfheaders_flow_control (true) - Remove unused mutable variable in block_processor.rs These optimizations significantly reduce CPU usage during header sync by avoiding expensive X11 hash recomputation and enable faster filter header sync through parallel requests with proper flow control. * feat(dash-spv): enhance masternode state persistence and synchronization - Implemented storage of masternode state after sync completion to allow phase manager to detect sync status. - Updated masternode synchronization logic to use the latest persisted state height for requesting masternode list updates, ensuring accurate base height for ChainLock validation. - Added logging for potential issues during state persistence and retrieval, improving observability of the synchronization process. These changes improve the reliability of masternode synchronization and enhance the overall flow control in the dash-spv module. * docs: update FFI API documentation * fix: replace Layout::array unwrap with proper error handling in managed_account Replace the panic-prone unwrap() call on Layout::array with proper error handling. Now returns false on layout computation failure instead of panicking at the FFI boundary, improving robustness when handling edge cases like integer overflow. * fix: suppress deprecated GenericArray warnings in BIP38 code Add #[allow(deprecated)] attributes to aes_encrypt and aes_decrypt functions to suppress CI clippy errors about generic-array 0.14.x deprecation. The aes crate still requires the old version, making an upgrade infeasible at this time. * fix(key-wallet): correct state update flag in transaction routing tests Fixed two test cases that incorrectly passed Some(&wallet) when they should have passed None to prevent state updates. The tests were checking that state is not modified, but were actually causing updates to occur. - test_transaction_routing_to_bip32_account: line 167 - test_transaction_affects_multiple_accounts: line 411 All 434 tests now pass. * fix(dash-spv): add missing CFHeaders flow control fields to test config Add max_concurrent_cfheaders_requests_parallel, enable_cfheaders_flow_control, cfheaders_request_timeout_secs, and max_cfheaders_retries fields to the ClientConfig initialization in network tests to fix compilation error. * fix(key-wallet-ffi): handle Layout::array overflow safely in deallocator Replace unwrap() call in managed_account_free_transactions with proper Result matching. This mirrors the allocation path and prevents potential panics in FFI context when layout calculation would overflow. - Match on Layout::array Result instead of unwrap - Return early if Err to avoid deallocation with invalid layout - Ensures FFI deallocator is safe and never panics * refactor: add explicit up-to-date check before computing start_height in CFHeaders request Skip redundant CFHeaders requests when filter_tip >= stop_height by checking the up-to-date condition before computing start_height. This avoids unnecessary height calculations and network requests when the filter is already synced to or past the target height. * fix(spv): correct TransactionDetected net_amount via wallet transaction_effect; include affected addresses\n\nfeat(wallet-manager): add WalletInterface::transaction_effect and implement in WalletManager using TransactionRouter and per-account checks\n\ntest(spv): add tests for wallet-provided net, fallback behavior, and negative net with duplicates * fix: resolve type_complexity clippy warning in block_processor_test Factor out complex type Arc<Mutex<BTreeMap<Txid, (i64, Vec<String>)>>> into a type alias TransactionEffectsMap for improved code clarity and to satisfy clippy type_complexity lint. --------- Co-authored-by: Claude <[email protected]>
1 parent da5f9ec commit c877c1a

File tree

21 files changed

+1528
-131
lines changed

21 files changed

+1528
-131
lines changed

dash-spv/src/client/block_processor.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,39 @@ impl<W: WalletInterface + Send + Sync + 'static, S: StorageManager + Send + Sync
234234
block_hash,
235235
height
236236
);
237+
238+
// Update statistics for blocks with relevant transactions
239+
{
240+
let mut stats = self.stats.write().await;
241+
stats.blocks_with_relevant_transactions += 1;
242+
}
243+
244+
// Emit TransactionDetected events for each relevant transaction
245+
for txid in &txids {
246+
if let Some(tx) = block.txdata.iter().find(|t| &t.txid() == txid) {
247+
// Ask the wallet for the precise effect of this transaction
248+
let effect = wallet.transaction_effect(tx, self.network).await;
249+
if let Some((net_amount, affected_addresses)) = effect {
250+
tracing::info!("📤 Emitting TransactionDetected event for {}", txid);
251+
let _ = self.event_tx.send(SpvEvent::TransactionDetected {
252+
txid: txid.to_string(),
253+
confirmed: true,
254+
block_height: Some(height),
255+
amount: net_amount,
256+
addresses: affected_addresses,
257+
});
258+
} else {
259+
// Fallback: emit event with zero and no addresses if wallet could not compute
260+
let _ = self.event_tx.send(SpvEvent::TransactionDetected {
261+
txid: txid.to_string(),
262+
confirmed: true,
263+
block_height: Some(height),
264+
amount: 0,
265+
addresses: Vec::new(),
266+
});
267+
}
268+
}
269+
}
237270
}
238271
drop(wallet); // Release lock
239272

dash-spv/src/client/block_processor_test.rs

Lines changed: 215 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,31 @@ mod tests {
1212
use std::sync::Arc;
1313
use tokio::sync::{mpsc, oneshot, Mutex, RwLock};
1414

15+
// Type alias for transaction effects map
16+
type TransactionEffectsMap =
17+
Arc<Mutex<std::collections::BTreeMap<dashcore::Txid, (i64, Vec<String>)>>>;
18+
1519
// Mock WalletInterface implementation for testing
1620
struct MockWallet {
1721
processed_blocks: Arc<Mutex<Vec<(dashcore::BlockHash, u32)>>>,
1822
processed_transactions: Arc<Mutex<Vec<dashcore::Txid>>>,
23+
// Map txid -> (net_amount, addresses)
24+
effects: TransactionEffectsMap,
1925
}
2026

2127
impl MockWallet {
2228
fn new() -> Self {
2329
Self {
2430
processed_blocks: Arc::new(Mutex::new(Vec::new())),
2531
processed_transactions: Arc::new(Mutex::new(Vec::new())),
32+
effects: Arc::new(Mutex::new(std::collections::BTreeMap::new())),
2633
}
2734
}
35+
36+
async fn set_effect(&self, txid: dashcore::Txid, net: i64, addresses: Vec<String>) {
37+
let mut map = self.effects.lock().await;
38+
map.insert(txid, (net, addresses));
39+
}
2840
}
2941

3042
#[async_trait::async_trait]
@@ -64,6 +76,15 @@ mod tests {
6476
async fn describe(&self, _network: Network) -> String {
6577
"MockWallet (test implementation)".to_string()
6678
}
79+
80+
async fn transaction_effect(
81+
&self,
82+
tx: &Transaction,
83+
_network: Network,
84+
) -> Option<(i64, Vec<String>)> {
85+
let map = self.effects.lock().await;
86+
map.get(&tx.txid()).cloned()
87+
}
6788
}
6889

6990
fn create_test_block(network: Network) -> Block {
@@ -110,6 +131,15 @@ mod tests {
110131

111132
// Send block processing task
112133
let (response_tx, _response_rx) = oneshot::channel();
134+
135+
// Prime wallet with an effect for the coinbase tx in the genesis block
136+
let txid = block.txdata[0].txid();
137+
{
138+
let wallet_guard = wallet.read().await;
139+
wallet_guard
140+
.set_effect(txid, 1234, vec!["XyTestAddr1".to_string(), "XyTestAddr2".to_string()])
141+
.await;
142+
}
113143
task_tx
114144
.send(BlockProcessingTask::ProcessBlock {
115145
block: Box::new(block.clone()),
@@ -120,22 +150,47 @@ mod tests {
120150
// Process the block in a separate task
121151
let processor_handle = tokio::spawn(async move { processor.run().await });
122152

123-
// Wait for event
153+
// Wait for events; capture the TransactionDetected for our tx
154+
let mut saw_tx_event = false;
124155
tokio::time::timeout(std::time::Duration::from_millis(100), async {
125156
while let Some(event) = event_rx.recv().await {
126-
if let SpvEvent::BlockProcessed {
127-
hash,
128-
..
129-
} = event
130-
{
131-
assert_eq!(hash.to_string(), block_hash.to_string());
132-
break;
157+
match event {
158+
SpvEvent::TransactionDetected {
159+
txid: tid,
160+
amount,
161+
addresses,
162+
confirmed,
163+
block_height,
164+
} => {
165+
// Should use wallet-provided values
166+
assert_eq!(tid, txid.to_string());
167+
assert_eq!(amount, 1234);
168+
assert_eq!(
169+
addresses,
170+
vec!["XyTestAddr1".to_string(), "XyTestAddr2".to_string()]
171+
);
172+
assert!(confirmed);
173+
assert_eq!(block_height, Some(0));
174+
saw_tx_event = true;
175+
}
176+
SpvEvent::BlockProcessed {
177+
hash,
178+
..
179+
} => {
180+
assert_eq!(hash.to_string(), block_hash.to_string());
181+
if saw_tx_event {
182+
break;
183+
}
184+
}
185+
_ => {}
133186
}
134187
}
135188
})
136189
.await
137190
.expect("Should receive block processed event");
138191

192+
assert!(saw_tx_event, "Should emit TransactionDetected with wallet-provided effect");
193+
139194
// Verify wallet was called
140195
{
141196
let wallet = wallet.read().await;
@@ -292,6 +347,158 @@ mod tests {
292347
let _ = processor_handle.await;
293348
}
294349

350+
#[tokio::test]
351+
async fn test_transaction_detected_fallback_when_no_wallet_effect() {
352+
let (processor, task_tx, mut event_rx, _wallet, storage) = setup_processor().await;
353+
354+
// Create a test block
355+
let block = create_test_block(Network::Dash);
356+
let block_hash = block.block_hash();
357+
let txid = block.txdata[0].txid();
358+
359+
// Store header so height lookup succeeds
360+
{
361+
let mut storage = storage.lock().await;
362+
storage.store_headers(&[block.header]).await.unwrap();
363+
}
364+
365+
// Send block processing task without priming any effect (transaction_effect will return None)
366+
let (response_tx, _response_rx) = oneshot::channel();
367+
task_tx
368+
.send(BlockProcessingTask::ProcessBlock {
369+
block: Box::new(block.clone()),
370+
response_tx,
371+
})
372+
.unwrap();
373+
374+
// Process
375+
let processor_handle = tokio::spawn(async move { processor.run().await });
376+
377+
let mut saw_tx_event = false;
378+
tokio::time::timeout(std::time::Duration::from_millis(100), async {
379+
while let Some(event) = event_rx.recv().await {
380+
match event {
381+
SpvEvent::TransactionDetected {
382+
txid: tid,
383+
amount,
384+
addresses,
385+
confirmed,
386+
block_height,
387+
} => {
388+
assert_eq!(tid, txid.to_string());
389+
assert_eq!(
390+
amount, 0,
391+
"fallback amount should be 0 when no effect available"
392+
);
393+
assert!(addresses.is_empty(), "fallback addresses should be empty");
394+
assert!(confirmed);
395+
assert_eq!(block_height, Some(0));
396+
saw_tx_event = true;
397+
}
398+
SpvEvent::BlockProcessed {
399+
hash,
400+
..
401+
} => {
402+
assert_eq!(hash.to_string(), block_hash.to_string());
403+
if saw_tx_event {
404+
break;
405+
}
406+
}
407+
_ => {}
408+
}
409+
}
410+
})
411+
.await
412+
.expect("Should receive events");
413+
414+
assert!(saw_tx_event, "Should emit TransactionDetected with fallback values");
415+
416+
// Shutdown
417+
drop(task_tx);
418+
let _ = processor_handle.await;
419+
}
420+
421+
#[tokio::test]
422+
async fn test_transaction_detected_negative_amount_and_duplicate_addresses() {
423+
let (processor, task_tx, mut event_rx, wallet, storage) = setup_processor().await;
424+
425+
// Create a test block
426+
let block = create_test_block(Network::Dash);
427+
let block_hash = block.block_hash();
428+
let txid = block.txdata[0].txid();
429+
430+
// Store header so height lookup succeeds
431+
{
432+
let mut storage = storage.lock().await;
433+
storage.store_headers(&[block.header]).await.unwrap();
434+
}
435+
436+
// Prime wallet with negative amount and duplicate addresses
437+
{
438+
let wallet_guard = wallet.read().await;
439+
wallet_guard
440+
.set_effect(
441+
txid,
442+
-500,
443+
vec!["DupAddr".to_string(), "DupAddr".to_string(), "UniqueAddr".to_string()],
444+
)
445+
.await;
446+
}
447+
448+
// Send block processing task
449+
let (response_tx, _response_rx) = oneshot::channel();
450+
task_tx
451+
.send(BlockProcessingTask::ProcessBlock {
452+
block: Box::new(block.clone()),
453+
response_tx,
454+
})
455+
.unwrap();
456+
457+
// Process
458+
let processor_handle = tokio::spawn(async move { processor.run().await });
459+
460+
let mut saw_tx_event = false;
461+
tokio::time::timeout(std::time::Duration::from_millis(100), async {
462+
while let Some(event) = event_rx.recv().await {
463+
match event {
464+
SpvEvent::TransactionDetected {
465+
txid: tid,
466+
amount,
467+
addresses,
468+
confirmed,
469+
block_height,
470+
} => {
471+
assert_eq!(tid, txid.to_string());
472+
assert_eq!(amount, -500);
473+
// BlockProcessor uses wallet-provided addresses as-is (no dedup here)
474+
assert_eq!(addresses, vec!["DupAddr", "DupAddr", "UniqueAddr"]);
475+
assert!(confirmed);
476+
assert_eq!(block_height, Some(0));
477+
saw_tx_event = true;
478+
}
479+
SpvEvent::BlockProcessed {
480+
hash,
481+
..
482+
} => {
483+
assert_eq!(hash.to_string(), block_hash.to_string());
484+
if saw_tx_event {
485+
break;
486+
}
487+
}
488+
_ => {}
489+
}
490+
}
491+
})
492+
.await
493+
.expect("Should receive events");
494+
495+
assert!(saw_tx_event, "Should emit TransactionDetected with negative net and duplicates");
496+
497+
// Shutdown
498+
drop(task_tx);
499+
let _ = processor_handle.await;
500+
}
501+
295502
#[tokio::test]
296503
async fn test_process_mempool_transaction() {
297504
let (processor, task_tx, _event_rx, wallet, _storage) = setup_processor().await;

dash-spv/src/client/config.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,19 @@ pub struct ClientConfig {
163163
/// Rate limit for CF header requests per second (default: 10.0).
164164
pub cfheaders_request_rate_limit: Option<f64>,
165165

166+
// CFHeaders flow control configuration
167+
/// Maximum concurrent CFHeaders requests for parallel sync (default: 50).
168+
pub max_concurrent_cfheaders_requests_parallel: usize,
169+
170+
/// Enable flow control for CFHeaders requests (default: true).
171+
pub enable_cfheaders_flow_control: bool,
172+
173+
/// Timeout for CFHeaders requests in seconds (default: 30).
174+
pub cfheaders_request_timeout_secs: u64,
175+
176+
/// Maximum retry attempts for failed CFHeaders batches (default: 3).
177+
pub max_cfheaders_retries: u32,
178+
166179
/// Rate limit for filter requests per second (default: 50.0).
167180
pub filters_request_rate_limit: Option<f64>,
168181

@@ -238,6 +251,11 @@ impl Default for ClientConfig {
238251
blocks_request_rate_limit: None,
239252
start_from_height: None,
240253
wallet_creation_time: None,
254+
// CFHeaders flow control defaults
255+
max_concurrent_cfheaders_requests_parallel: 50,
256+
enable_cfheaders_flow_control: true,
257+
cfheaders_request_timeout_secs: 30,
258+
max_cfheaders_retries: 3,
241259
// QRInfo defaults (simplified per plan)
242260
qr_info_extra_share: false, // Matches DMLviewer.patch default
243261
qr_info_timeout: Duration::from_secs(30),

dash-spv/src/client/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,7 @@ impl<
311311
received_filter_heights,
312312
wallet.clone(),
313313
state.clone(),
314+
stats.clone(),
314315
)
315316
.map_err(SpvError::Sync)?;
316317

dash-spv/src/network/tests.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ mod multi_peer_tests {
146146
enable_filter_flow_control: true,
147147
filter_request_delay_ms: 0,
148148
max_concurrent_filter_requests: 50,
149+
max_concurrent_cfheaders_requests_parallel: 50,
150+
enable_cfheaders_flow_control: true,
151+
cfheaders_request_timeout_secs: 30,
152+
max_cfheaders_retries: 3,
149153
enable_cfheader_gap_restart: true,
150154
cfheader_gap_check_interval_secs: 15,
151155
cfheader_gap_restart_cooldown_secs: 30,

0 commit comments

Comments
 (0)