From 6b3ba4436d674f36f5c1a27d21fc65002fa0c805 Mon Sep 17 00:00:00 2001 From: bfoss765 Date: Sat, 11 Jul 2026 06:40:14 -0400 Subject: [PATCH 1/2] fix(key-wallet): cover CoinJoin/DashPay accounts for AssetLock routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TransactionRouter::get_relevant_account_types(AssetLock)` returned only the BIP44/BIP32 standard accounts plus the identity/asset-lock accounts, omitting the CoinJoin and DashPay fund-bearing accounts. Only `check_core_transaction` marks a UTXO spent, and it scopes spend detection to the account types this router returns. So an asset lock funded from a CoinJoin or DashPay UTXO never had its input debited: the spent UTXO kept counting toward the balance indefinitely while the (BIP44) change output was still credited, inflating the reported balance by exactly the spent amount — on both relay and rescan. Field evidence (Android SDK wallet, dashpay/dash-wallet#1507): outpoint-diff reconciliation against the dashj oracle showed the SDK balance high by exactly the value of asset-lock-spent CoinJoin inputs that were never debited. Fix: the AssetLock arm now consults the full `fund_bearing_account_types()` set (BIP44, BIP32, CoinJoin, DashpayReceivingFunds, DashpayExternalAccount) before the identity/asset-lock accounts. Discovery is membership-based like Dash Core's `IsMine`, so consulting extra accounts never yields false positives. Adds an end-to-end regression test that funds a CoinJoin UTXO, spends it via an asset-lock transaction through `check_core_transaction`, and asserts the CoinJoin UTXO is debited and removed (fails on the old routing: total_sent 0). Also extends the AssetLock routing unit test. Refs: dashpay/platform#4073, dashpay/platform#4074, dashpay/dash-wallet#1507 Co-Authored-By: Claude Fable 5 --- .../transaction_router/mod.rs | 32 ++-- .../transaction_router/tests/routing.rs | 8 +- .../transaction_checking/wallet_checker.rs | 154 ++++++++++++++++++ 3 files changed, 183 insertions(+), 11 deletions(-) diff --git a/key-wallet/src/transaction_checking/transaction_router/mod.rs b/key-wallet/src/transaction_checking/transaction_router/mod.rs index b7d74b290..51f35acbe 100644 --- a/key-wallet/src/transaction_checking/transaction_router/mod.rs +++ b/key-wallet/src/transaction_checking/transaction_router/mod.rs @@ -134,16 +134,28 @@ impl TransactionRouter { AccountTypeToCheck::StandardBIP32, AccountTypeToCheck::CoinJoin, ], - TransactionType::AssetLock => vec![ - AccountTypeToCheck::StandardBIP44, - AccountTypeToCheck::StandardBIP32, - AccountTypeToCheck::IdentityRegistration, - AccountTypeToCheck::IdentityTopUp, - AccountTypeToCheck::IdentityTopUpNotBound, - AccountTypeToCheck::IdentityInvitation, - AccountTypeToCheck::AssetLockAddressTopUp, - AccountTypeToCheck::AssetLockShieldedAddressTopUp, - ], + TransactionType::AssetLock => { + // An asset lock can be funded from any fund-bearing account, and only + // `check_core_transaction` debits a spent UTXO — scoped to the accounts + // returned here. Omitting CoinJoin / DashPay meant asset locks funded from + // those accounts never had their inputs debited, so the spent UTXOs kept + // counting toward the balance indefinitely while the (BIP44) change output + // was still credited — the balance ended up high by exactly the + // CoinJoin/DashPay amount spent, both on relay and on rescan + // (dashpay/platform#4073, dashpay/platform#4074, dashpay/dash-wallet#1507). + // Discovery is membership-based like Dash Core's `IsMine`, so consulting the + // full fund-bearing set never yields false positives. + let mut accounts = Self::fund_bearing_account_types(); + accounts.extend([ + AccountTypeToCheck::IdentityRegistration, + AccountTypeToCheck::IdentityTopUp, + AccountTypeToCheck::IdentityTopUpNotBound, + AccountTypeToCheck::IdentityInvitation, + AccountTypeToCheck::AssetLockAddressTopUp, + AccountTypeToCheck::AssetLockShieldedAddressTopUp, + ]); + accounts + } TransactionType::AssetUnlock => { vec![AccountTypeToCheck::StandardBIP44, AccountTypeToCheck::StandardBIP32] } diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs b/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs index 76568c45f..65d2d487b 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs @@ -432,9 +432,15 @@ fn test_asset_lock_transaction_routing() { let tx_type = TransactionType::AssetLock; let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - // Should check standard accounts and all identity accounts + // Should check all fund-bearing accounts (an asset lock can be funded from any of them) + // and all identity accounts. assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + // Fund-bearing accounts an asset lock can spend from. Their absence let asset-lock + // spends of CoinJoin / DashPay UTXOs go undebited (dashpay/platform#4073, #4074). + assert!(accounts.contains(&AccountTypeToCheck::CoinJoin)); + assert!(accounts.contains(&AccountTypeToCheck::DashpayReceivingFunds)); + assert!(accounts.contains(&AccountTypeToCheck::DashpayExternalAccount)); assert!(accounts.contains(&AccountTypeToCheck::IdentityRegistration)); assert!(accounts.contains(&AccountTypeToCheck::IdentityTopUp)); assert!(accounts.contains(&AccountTypeToCheck::IdentityTopUpNotBound)); diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index 65469b561..b6dc3a508 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -565,6 +565,160 @@ mod tests { assert_eq!(record.net_amount, -(funding_value as i64)); } + /// Regression: an asset-lock transaction that spends a CoinJoin UTXO must + /// debit that UTXO. + /// + /// `check_core_transaction` only marks a UTXO spent for the account types + /// returned by `TransactionRouter::get_relevant_account_types`. Before the + /// fix, the `AssetLock` arm omitted `CoinJoin` (and the DashPay accounts), + /// so an asset lock funded from a CoinJoin UTXO never had that input + /// debited: the spent UTXO kept counting toward the balance while the BIP44 + /// change output was still credited, inflating the balance by exactly the + /// spent amount — on both relay and rescan (dashpay/platform#4073, #4074, + /// dashpay/dash-wallet#1507). + #[tokio::test] + async fn test_asset_lock_spending_coinjoin_utxo_is_debited() { + use crate::account::AccountType; + use crate::managed_account::managed_account_type::ManagedAccountType; + use dashcore::blockdata::transaction::special_transaction::asset_lock::AssetLockPayload; + use dashcore::blockdata::transaction::special_transaction::TransactionPayload; + + let network = Network::Testnet; + + // Wallet with a BIP44 account (asset-lock change lands here) and a + // CoinJoin account (funds the asset lock). + let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::None) + .expect("Should create wallet"); + wallet + .add_account( + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }, + None, + ) + .expect("Should add BIP44 account"); + wallet + .add_account( + AccountType::CoinJoin { + index: 0, + }, + None, + ) + .expect("Should add CoinJoin account"); + + let mut managed_wallet = + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); + + // Derive a CoinJoin external address and a BIP44 change address. + let coinjoin_xpub = + wallet.accounts.coinjoin_accounts.get(&0).expect("coinjoin account").account_xpub; + let coinjoin_address = { + let managed_account = + managed_wallet.first_coinjoin_managed_account_mut().expect("managed coinjoin"); + if let ManagedAccountType::CoinJoin { + external_addresses, + .. + } = managed_account.managed_account_type_mut() + { + external_addresses + .next_unused(&KeySource::Public(coinjoin_xpub), true) + .expect("coinjoin address") + } else { + panic!("Expected CoinJoin account type"); + } + }; + let bip44_xpub = + wallet.accounts.standard_bip44_accounts.get(&0).expect("bip44 account").account_xpub; + let change_address = managed_wallet + .first_bip44_managed_account_mut() + .expect("managed bip44") + .next_change_address(Some(&bip44_xpub), true) + .expect("bip44 change address"); + + // Fund the CoinJoin address, creating a CoinJoin UTXO. + let funding_value = 100_000_000u64; + let funding_tx = Transaction::dummy(&coinjoin_address, 0..1, &[funding_value]); + let funding_context = TransactionContext::InChainLockedBlock(BlockInfo::new( + 1, + BlockHash::from_slice(&[7u8; 32]).expect("Should create block hash"), + 1_650_000_000, + )); + let funding_result = managed_wallet + .check_core_transaction(&funding_tx, funding_context, &mut wallet, true, true) + .await; + assert!(funding_result.is_relevant, "CoinJoin funding must be relevant"); + assert_eq!(funding_result.total_received, funding_value); + assert_eq!( + managed_wallet.first_coinjoin_managed_account().expect("coinjoin account").utxos.len(), + 1, + "CoinJoin funding must create a UTXO" + ); + + // Build an asset-lock tx spending the CoinJoin UTXO, with BIP44 change. + let credit_value = 40_000_000u64; + let change_value = funding_value - credit_value - 1_000; // small fee + let asset_lock_tx = Transaction { + version: 3, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: funding_tx.txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: change_value, + script_pubkey: change_address.script_pubkey(), + }], + special_transaction_payload: Some(TransactionPayload::AssetLockPayloadType( + AssetLockPayload { + version: 1, + credit_outputs: vec![TxOut { + value: credit_value, + script_pubkey: change_address.script_pubkey(), + }], + }, + )), + }; + assert_eq!( + TransactionRouter::classify_transaction(&asset_lock_tx), + TransactionType::AssetLock, + "tx must classify as AssetLock so it routes through the AssetLock arm" + ); + + let spend_context = TransactionContext::InChainLockedBlock(BlockInfo::new( + 2, + BlockHash::from_slice(&[8u8; 32]).expect("Should create block hash"), + 1_650_000_100, + )); + let spend_result = managed_wallet + .check_core_transaction(&asset_lock_tx, spend_context, &mut wallet, true, true) + .await; + + assert!(spend_result.is_relevant, "asset lock spending our UTXO must be relevant"); + // The load-bearing assertion: the CoinJoin input must be debited. + assert_eq!( + spend_result.total_sent, funding_value, + "asset lock must debit the CoinJoin UTXO it spends" + ); + assert_eq!( + spend_result.total_received, change_value, + "BIP44 change output must be credited" + ); + assert!( + managed_wallet + .first_coinjoin_managed_account() + .expect("coinjoin account") + .utxos + .is_empty(), + "spent CoinJoin UTXO must be removed" + ); + } + /// Test the full coinbase maturity flow - immature to mature transition #[tokio::test] async fn test_wallet_checker_immature_transaction_flow() { From 34756ff45d8ae1e3dab0302d0c52b337ceb77671 Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Sat, 11 Jul 2026 14:34:12 -0400 Subject: [PATCH 2/2] test(key-wallet): assert aggregate wallet balance in the asset-lock CoinJoin-debit test Addresses CodeRabbit's nitpick on dashpay/rust-dashcore#867. The regression the test guards against is balance inflation, but it previously asserted only the per-transaction totals and the CoinJoin UTXO removal. Add confirmed/spendable/ total balance assertions after the spend: a non-debited CoinJoin coin would report funding_value + change_value; a correct debit leaves only the confirmed BIP44 change (the asset-lock credit output is locked into Platform, never a spendable wallet UTXO). Existing assertions preserved. Co-Authored-By: Claude Fable 5 --- .../transaction_checking/wallet_checker.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index b6dc3a508..356ce45da 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -717,6 +717,32 @@ mod tests { .is_empty(), "spent CoinJoin UTXO must be removed" ); + + // Aggregate wallet balance — the actual regression is balance inflation. + // `check_core_transaction` was called with `update_balance = true`, so + // the cached balance now reflects the spend. If the spent CoinJoin coin + // were NOT debited, the wallet would still count its `funding_value` + // (100_000_000) on top of the new BIP44 change, reporting + // `funding_value + change_value`. After a correct debit only the + // confirmed BIP44 change UTXO remains (the asset-lock credit output is + // locked into Platform, never counted as a spendable wallet UTXO — see + // the `total_received == change_value` assertion above). + assert_eq!( + managed_wallet.balance.confirmed(), + change_value, + "confirmed balance must be only the BIP44 change; the spent CoinJoin \ + coin must not inflate it" + ); + assert_eq!( + managed_wallet.balance.spendable(), + change_value, + "spendable balance must fall from funding_value to change_value after the spend" + ); + assert_eq!( + managed_wallet.balance.total(), + change_value, + "total balance must not double-count the spent CoinJoin UTXO" + ); } /// Test the full coinbase maturity flow - immature to mature transition