From 87e791677d40025fe1ba2c5ccaac449bb2ec803b Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 17:32:45 +0200 Subject: [PATCH 1/7] fix(spv): apply InstantSend locks to self-broadcast transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-broadcast transactions bypass the MempoolManager (fed directly to WalletManager via notify_wallet_after_broadcast). When the IS lock arrives from the network, the MempoolManager doesn't know about the tx, so it stores the lock as "pending" — never matched, never applied. Result: the tx stays unconfirmed and balance.spendable() returns 0. Fix: in SpvEventHandler::on_sync_event(InstantLockReceived), apply the IS lock directly on the WalletManager via process_instant_send_lock(). For MempoolManager-tracked txs this is a harmless no-op — the WalletManager deduplicates via its instant_send_locks HashSet. Co-Authored-By: Claude Opus 4.6 --- src/spv/manager.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/spv/manager.rs b/src/spv/manager.rs index d68aed31e..1e0300a11 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -144,6 +144,13 @@ pub(crate) struct SpvEventHandler { connection_status: Option>, reconcile_tx: Arc>>>, finality_tx: Arc>>>, + /// Wallet manager reference for applying InstantSend locks directly. + /// + /// Self-broadcast transactions bypass the MempoolManager and are fed to + /// the WalletManager via `notify_wallet_after_broadcast()`. When the IS + /// lock arrives later, the MempoolManager doesn't know about the tx and + /// cannot apply the lock. We apply it here directly on the WalletManager. + wallet: Arc>>, } impl EventHandler for SpvEventHandler { @@ -256,6 +263,26 @@ impl EventHandler for SpvEventHandler { | SyncEvent::SyncComplete { .. } ); + // Apply InstantSend locks directly on the WalletManager. + // + // Self-broadcast transactions bypass the MempoolManager (they are fed + // directly to WalletManager via notify_wallet_after_broadcast). When + // the IS lock arrives from the network, the MempoolManager doesn't + // know about the tx and stores it as a "pending IS lock" that is + // never matched. Applying the lock here ensures self-broadcast txs + // transition from unconfirmed to spendable. + // + // For MempoolManager-tracked txs this is a harmless no-op — the + // WalletManager deduplicates via its instant_send_locks HashSet. + if let SyncEvent::InstantLockReceived { instant_lock, .. } = event { + let txid = instant_lock.txid; + let wallet = Arc::clone(&self.wallet); + tokio::spawn(async move { + let mut wm = wallet.write().await; + wm.process_instant_send_lock(txid); + }); + } + // Forward finality-relevant events for asset lock proof construction. let finality_tx = self.finality_tx.lock().ok().and_then(|g| g.clone()); if let Some(ref ftx) = finality_tx { @@ -1379,6 +1406,7 @@ impl SpvManager { connection_status: self.connection_status_snapshot(), reconcile_tx: Arc::clone(&self.reconcile_tx), finality_tx: Arc::clone(&self.finality_tx), + wallet: Arc::clone(&self.wallet), }); DashSpvClient::new( From e995ed5e1a6c987fed06fb156c4395f5676bf27a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 17:36:23 +0200 Subject: [PATCH 2/7] docs: link IS lock workaround to upstream rust-dashcore#487 Both notify_wallet_after_broadcast and the EventHandler IS lock workaround exist because upstream broadcast doesn't call handle_tx on the MempoolManager. Added TODO linking them so they can be removed together when the upstream fix lands. Co-Authored-By: Claude Opus 4.6 --- src/spv/manager.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/spv/manager.rs b/src/spv/manager.rs index 1e0300a11..e2073044e 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -263,14 +263,21 @@ impl EventHandler for SpvEventHandler { | SyncEvent::SyncComplete { .. } ); + // TODO(workaround): Remove once dashpay/rust-dashcore#487 is fixed. + // // Apply InstantSend locks directly on the WalletManager. // // Self-broadcast transactions bypass the MempoolManager (they are fed - // directly to WalletManager via notify_wallet_after_broadcast). When - // the IS lock arrives from the network, the MempoolManager doesn't - // know about the tx and stores it as a "pending IS lock" that is - // never matched. Applying the lock here ensures self-broadcast txs - // transition from unconfirmed to spendable. + // directly to WalletManager via notify_wallet_after_broadcast — see + // the other workaround in spawn_request_handler). When the IS lock + // arrives from the network, the MempoolManager doesn't know about + // the tx and stores it as a "pending IS lock" that is never matched. + // Applying the lock here ensures self-broadcast txs transition from + // unconfirmed to spendable. + // + // Once upstream broadcast calls handle_tx() on the MempoolManager, + // both workarounds (notify_wallet_after_broadcast and this) can be + // removed — the normal MempoolManager pipeline will handle everything. // // For MempoolManager-tracked txs this is a harmless no-op — the // WalletManager deduplicates via its instant_send_locks HashSet. From c13831e80c0f573625543c9ec09ffea06b7d4a7e Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 18:07:55 +0200 Subject: [PATCH 3/7] fix(test): reduce cleanup sweep timeout from 10s to 1s With 14+ orphaned wallets from previous runs, the 10s per-wallet spendable balance wait added 2+ minutes to test startup. Most orphaned wallets have 0 spendable balance anyway (IS locks never arrived), so the wait is wasted. Co-Authored-By: Claude Opus 4.6 --- tests/backend-e2e/framework/cleanup.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/backend-e2e/framework/cleanup.rs b/tests/backend-e2e/framework/cleanup.rs index 586493169..70a6ecd1d 100644 --- a/tests/backend-e2e/framework/cleanup.rs +++ b/tests/backend-e2e/framework/cleanup.rs @@ -60,8 +60,10 @@ pub async fn cleanup_test_wallets( }; // Wait briefly for SPV to sync this wallet's balance. + // Keep this short — with many orphaned wallets the cumulative wait + // delays test startup significantly (14 wallets × 10s = 2+ min). let _ = - wait::wait_for_spendable_balance(app_context, hash, 1, Duration::from_secs(10)).await; + wait::wait_for_spendable_balance(app_context, hash, 1, Duration::from_secs(1)).await; let balance = { let wallet = wallet_arc.read().expect("wallet lock"); From 7251b9ee40b2ab0076e4f38a4e5d7d42ef7e58a2 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 18:13:18 +0200 Subject: [PATCH 4/7] fix(test): wait for SPV sync before checking framework wallet balance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The init sequence waited 180s for spendable balance BEFORE waiting for SPV to reach Running state. Wallet balances are only available after compact filter sync completes, so the balance check always timed out on the first attempt, wasting 3+ minutes per retry. Swapped the order: wait for SPV Running first (up to 300s), then check spendable balance (30s — should be near-instant after sync). Co-Authored-By: Claude Opus 4.6 --- tests/backend-e2e/framework/harness.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/backend-e2e/framework/harness.rs b/tests/backend-e2e/framework/harness.rs index c04db4f1b..15776a803 100644 --- a/tests/backend-e2e/framework/harness.rs +++ b/tests/backend-e2e/framework/harness.rs @@ -210,13 +210,24 @@ impl BackendTestContext { .await .expect("Framework wallet not picked up by SPV"); - // Wait for SPV to sync and funds to become spendable + // Wait for SPV to fully sync (including masternodes) so MempoolManager + // is active and bloom filter is built before any test broadcasts. + // This must come BEFORE the spendable balance check — wallet balances + // are only available after compact filter sync completes. + tracing::info!("Waiting for SPV to complete full sync (masternodes + mempool)..."); + wait::wait_for_spv_running(&app_context, Duration::from_secs(300)) + .await + .expect("SPV did not reach Running state within 300s"); + tracing::info!("SPV fully synced — mempool bloom filter active"); + + // Now check framework wallet balance — SPV has synced, so balances + // should be available immediately (no need for a long timeout). tracing::info!("Waiting for SPV to sync framework wallet spendable balance..."); match wait::wait_for_spendable_balance( &app_context, framework_wallet_hash, 1, // at least 1 duff spendable - Duration::from_secs(180), + Duration::from_secs(30), ) .await { @@ -249,14 +260,6 @@ impl BackendTestContext { } } - // Wait for SPV to fully sync (including masternodes) so MempoolManager - // is active and bloom filter is built before any test broadcasts. - tracing::info!("Waiting for SPV to complete full sync (masternodes + mempool)..."); - wait::wait_for_spv_running(&app_context, Duration::from_secs(120)) - .await - .expect("SPV did not reach Running state within 120s"); - tracing::info!("SPV fully synced — mempool bloom filter active"); - // Verify balance is above minimum threshold funding::verify_framework_funded(&app_context, framework_wallet_hash).await; From d6ce0ece0466dc07a0e8d2750b6ae763d1473bc9 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 18:16:21 +0200 Subject: [PATCH 5/7] fix(spv): mark spent UTXOs before releasing wallet lock on payment send_wallet_payment_via_spv() built and signed the transaction under the WalletManager write lock, then dropped the lock before broadcasting. Concurrent callers could select the same UTXOs, creating double-spend transactions that the network rejects (no IS lock issued for the conflicting tx). Now calls process_mempool_transaction() while still holding the write lock, so spent UTXOs are immediately marked and unavailable to concurrent callers. Co-Authored-By: Claude Opus 4.6 --- src/backend_task/core/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/backend_task/core/mod.rs b/src/backend_task/core/mod.rs index 28f29edd4..7f311ea3a 100644 --- a/src/backend_task/core/mod.rs +++ b/src/backend_task/core/mod.rs @@ -33,7 +33,7 @@ use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::transaction_builder: BuilderError, TransactionBuilder, }; use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use dash_sdk::dpp::key_wallet_manager::{WalletError, WalletId, WalletManager}; +use dash_sdk::dpp::key_wallet_manager::{WalletError, WalletId, WalletInterface, WalletManager}; use std::path::PathBuf; use std::str::FromStr; use std::sync::{Arc, RwLock}; @@ -593,7 +593,13 @@ impl AppContext { &parsed_recipients, &request, )?; - self.sign_spv_transaction(&mut wm, &wallet_id, unsigned)? + let signed = self.sign_spv_transaction(&mut wm, &wallet_id, unsigned)?; + + // Notify the wallet about the outgoing tx while still holding the + // write lock. This marks spent UTXOs immediately so concurrent + // callers don't select the same inputs (double-spend prevention). + let _ = wm.process_mempool_transaction(&signed, false).await; + signed }; self.spv_manager From c217928470fe792473f50963d1c59fb99e59d0a7 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 19:37:16 +0200 Subject: [PATCH 6/7] =?UTF-8?q?fix(test):=20add=20bloom=20filter=20propaga?= =?UTF-8?q?tion=20delay=20before=20A=E2=86=92B=20payment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tx_is_ours test sends from wallet A to wallet B, but B's bloom filter may not have propagated to peers yet. Peers don't relay the tx back through B's filter, so B never sees it. Adding a 2s delay after wallet creation gives the bloom filter time to reach peers. Co-Authored-By: Claude Opus 4.6 --- tests/backend-e2e/tx_is_ours.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/backend-e2e/tx_is_ours.rs b/tests/backend-e2e/tx_is_ours.rs index 1281b69d2..793237663 100644 --- a/tests/backend-e2e/tx_is_ours.rs +++ b/tests/backend-e2e/tx_is_ours.rs @@ -37,6 +37,11 @@ async fn test_spv_transactions_is_ours_flag() { .await .expect("Wallet A should have spendable funds"); + // Allow bloom filter to propagate to peers so B's addresses are + // monitored before we broadcast A→B. Without this, peers may not + // relay the tx back through B's filter. + tokio::time::sleep(Duration::from_secs(2)).await; + // Send from A to B let request = WalletPaymentRequest { recipients: vec![PaymentRecipient { From 41795286124f84b13c5166203cdec3cb7256b9ce Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Apr 2026 22:01:57 +0200 Subject: [PATCH 7/7] fix(test): delete empty orphaned wallets instead of accumulating them Orphaned test wallets with 0 total balance were skipped during cleanup and accumulated across runs (~10MB + 185 monitored addresses each). By run 6, ~30 orphaned wallets caused reconciliation to saturate all 12 CPU cores (987% CPU, 928s runtime). Now deletes wallets with 0 total balance via remove_wallet(). Only wallets with unconfirmed-but-unspendable funds are kept for future cleanup attempts. Co-Authored-By: Claude Opus 4.6 --- tests/backend-e2e/framework/cleanup.rs | 54 +++++++++++++++++++------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/tests/backend-e2e/framework/cleanup.rs b/tests/backend-e2e/framework/cleanup.rs index 70a6ecd1d..f7c911029 100644 --- a/tests/backend-e2e/framework/cleanup.rs +++ b/tests/backend-e2e/framework/cleanup.rs @@ -50,6 +50,9 @@ pub async fn cleanup_test_wallets( wallet_hashes.len() ); + let mut swept = 0u32; + let mut deleted = 0u32; + for hash in wallet_hashes { let wallet_arc = { let wallets = app_context.wallets().read().expect("wallets lock"); @@ -60,28 +63,44 @@ pub async fn cleanup_test_wallets( }; // Wait briefly for SPV to sync this wallet's balance. - // Keep this short — with many orphaned wallets the cumulative wait - // delays test startup significantly (14 wallets × 10s = 2+ min). let _ = wait::wait_for_spendable_balance(app_context, hash, 1, Duration::from_secs(1)).await; - let balance = { + let (spendable, total) = { let wallet = wallet_arc.read().expect("wallet lock"); - wallet.confirmed_balance_duffs() + ( + wallet.confirmed_balance_duffs(), + wallet.total_balance_duffs(), + ) }; - if balance == 0 { + // Delete wallets with no funds at all — they're fully spent orphans + // from previous runs. Without this, wallets accumulate across runs + // and degrade performance (~10 MB + 185 monitored addresses each). + if total == 0 { + if let Err(e) = app_context.remove_wallet(&hash) { + tracing::warn!( + "Cleanup: failed to delete empty wallet {:?}: {}", + &hash[..4], + e + ); + } else { + deleted += 1; + } continue; } - // TODO(CMT-032): Also withdraw Platform credits from test identities back to - // the framework wallet. Requires: enumerate identities owned by test wallets, - // call IdentityTask::WithdrawFromIdentity for each, wait for Core balance. + if spendable == 0 { + // Has unconfirmed funds but nothing spendable — skip sweep, + // will be cleaned up on a future run once funds confirm. + continue; + } + // Attempt to sweep spendable funds back to framework wallet let request = WalletPaymentRequest { recipients: vec![PaymentRecipient { address: framework_address.clone(), - amount_duffs: balance, + amount_duffs: spendable, }], subtract_fee_from_amount: true, memo: Some("E2E cleanup: sweep orphaned wallet".to_string()), @@ -94,12 +113,19 @@ pub async fn cleanup_test_wallets( }); match run_task(app_context, task).await { - Ok(_) => tracing::info!( - "Cleanup: returned {} duffs from orphaned wallet {:?}", - balance, - &hash[..4] - ), + Ok(_) => { + swept += 1; + tracing::info!( + "Cleanup: returned {} duffs from orphaned wallet {:?}", + spendable, + &hash[..4] + ); + } Err(e) => tracing::warn!("Cleanup: failed to sweep wallet {:?}: {}", &hash[..4], e), } } + + if swept > 0 || deleted > 0 { + tracing::info!("Cleanup complete: {swept} swept, {deleted} deleted (empty)"); + } }