diff --git a/key-wallet/src/transaction_checking/transaction_router/mod.rs b/key-wallet/src/transaction_checking/transaction_router/mod.rs index 51f35acbe..0b2ebc0d9 100644 --- a/key-wallet/src/transaction_checking/transaction_router/mod.rs +++ b/key-wallet/src/transaction_checking/transaction_router/mod.rs @@ -88,7 +88,9 @@ impl TransactionRouter { /// label, never a precondition for discovery, so both shapes must consult the full set of /// fund-bearing accounts. An account only matches when a scriptPubKey or spent UTXO actually /// belongs to it, so checking extra accounts never produces false positives. - fn fund_bearing_account_types() -> Vec { + /// Visible to unit tests so routing assertions can compare against the + /// production list rather than re-listing the five types by hand. + pub(crate) fn fund_bearing_account_types() -> Vec { vec![ AccountTypeToCheck::StandardBIP44, AccountTypeToCheck::StandardBIP32, @@ -156,14 +158,17 @@ impl TransactionRouter { ]); accounts } - TransactionType::AssetUnlock => { - vec![AccountTypeToCheck::StandardBIP44, AccountTypeToCheck::StandardBIP32] + // Credit-side mirror of the AssetLock debit fix (#867 / #900): a coinbase + // (mining reward / masternode payout) or asset unlock (Platform credit + // withdrawal) can pay any user-chosen address, including CoinJoin and + // DashPay. Only the account types returned here are consulted for + // ownership, so omitting those accounts dropped the coin after the + // block was already downloaded. Discovery is membership-based like + // Dash Core's `IsMine`, so consulting the full fund-bearing set never + // yields false positives. + TransactionType::AssetUnlock | TransactionType::Coinbase => { + Self::fund_bearing_account_types() } - TransactionType::Coinbase => vec![ - // Check all account types for unknown special transactions - AccountTypeToCheck::StandardBIP44, - AccountTypeToCheck::StandardBIP32, - ], TransactionType::Ignored => vec![], } } diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs b/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs index 6371710b1..1ff4205c3 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs @@ -21,10 +21,13 @@ fn test_asset_unlock_routing() { let tx_type = TransactionType::AssetUnlock; let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - // Asset unlock only goes to standard accounts - assert_eq!(accounts.len(), 2); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + // Asset unlock withdrawals can pay any fund-bearing address (destination is + // user-chosen), so routing must cover the full fund-bearing set. + assert_eq!( + accounts, + TransactionRouter::fund_bearing_account_types(), + "AssetUnlock should route to all fund-bearing account types" + ); // Should NOT check identity accounts - those are for locks only assert!(!accounts.contains(&AccountTypeToCheck::IdentityRegistration)); @@ -62,9 +65,11 @@ fn test_asset_unlock_classification() { // Verify routing for AssetUnlock let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - assert_eq!(accounts.len(), 2, "AssetUnlock should route to exactly 2 account types"); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + assert_eq!( + accounts, + TransactionRouter::fund_bearing_account_types(), + "AssetUnlock should route to all fund-bearing account types" + ); } #[tokio::test] diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs b/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs index 0ed906a49..0d59d675c 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs @@ -273,13 +273,15 @@ fn test_coinbase_routing() { let tx_type = TransactionType::Coinbase; let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - // Coinbase should route to standard accounts - assert_eq!(accounts.len(), 2, "Coinbase should route to exactly 2 account types"); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + // Coinbase can pay any fund-bearing address (mining reward / masternode + // payout is user-chosen), so routing must cover the full fund-bearing set. + assert_eq!( + accounts, + TransactionRouter::fund_bearing_account_types(), + "Coinbase should route to all fund-bearing account types" + ); - // Should NOT route to special account types - assert!(!accounts.contains(&AccountTypeToCheck::CoinJoin)); + // Should NOT route to non-fund-bearing special account types assert!(!accounts.contains(&AccountTypeToCheck::IdentityRegistration)); assert!(!accounts.contains(&AccountTypeToCheck::ProviderOwnerKeys)); } diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index 7ea653924..15717d5b0 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -257,6 +257,32 @@ mod tests { use dashcore::{Address, BlockHash, TxIn, Txid}; use dashcore_hashes::Hash; + /// Wallet with a single CoinJoin account and one derived external address. + /// Shared fixture for credit/debit regressions that target CoinJoin ownership. + fn wallet_with_coinjoin_address() -> (Wallet, ManagedWalletInfo, Address) { + let network = Network::Testnet; + let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::None) + .expect("Should create wallet"); + 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); + let coinjoin_xpub = + wallet.accounts.coinjoin_accounts.get(&0).expect("coinjoin account").account_xpub; + let coinjoin_address = managed_wallet + .first_coinjoin_managed_account_mut() + .expect("managed coinjoin") + .next_address(Some(&coinjoin_xpub), true) + .expect("coinjoin address"); + (wallet, managed_wallet, coinjoin_address) + } + /// Test wallet checker with unrelated transaction #[tokio::test] async fn test_wallet_checker_unrelated_transaction() { @@ -744,6 +770,180 @@ mod tests { ); } + /// Regression: a coinbase that pays a CoinJoin address must credit that account. + /// + /// `check_core_transaction` only consults the account types returned by + /// `TransactionRouter::get_relevant_account_types`. Before the fix, the + /// `Coinbase` arm returned only StandardBIP44/BIP32, so a mining reward or + /// masternode payout to a CoinJoin (or DashPay) address was never matched: + /// the block was still downloaded (filters query all scripts), but the + /// output was dropped purely by the account-type narrowing, undercounting + /// the balance (dashpay/rust-dashcore#900). Discovery is membership-based + /// like Dash Core's `IsMine`, so consulting the full fund-bearing set cannot + /// yield a false positive. + #[tokio::test] + async fn test_coinbase_paying_coinjoin_address_is_credited() { + use crate::transaction_checking::transaction_router::TransactionRouter; + + let (mut wallet, mut managed_wallet, coinjoin_address) = wallet_with_coinjoin_address(); + + let reward = 5_000_000_000u64; + let coinbase_tx = Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::all_zeros(), + vout: 0xffffffff, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: reward, + script_pubkey: coinjoin_address.script_pubkey(), + }], + special_transaction_payload: None, + }; + assert_eq!( + TransactionRouter::classify_transaction(&coinbase_tx), + TransactionType::Coinbase, + "tx must classify as Coinbase so it routes through the Coinbase arm" + ); + + let block_height = 100_000; + let context = TransactionContext::InBlock(BlockInfo::new( + block_height, + BlockHash::from_slice(&[9u8; 32]).expect("Should create block hash"), + 1_650_000_200, + )); + let result = managed_wallet + .check_core_transaction(&coinbase_tx, context, &mut wallet, true, true) + .await; + managed_wallet.update_last_processed_height(block_height); + + assert!(result.is_relevant, "coinbase paying a CoinJoin address must be relevant"); + assert_eq!(result.total_received, reward, "coinbase must credit the CoinJoin output value"); + + let coinjoin_account = + managed_wallet.first_coinjoin_managed_account().expect("coinjoin account"); + assert!( + coinjoin_account.transactions().contains_key(&coinbase_tx.txid()), + "coinbase must be recorded on the CoinJoin account" + ); + assert_eq!(coinjoin_account.utxos.len(), 1, "coinbase must create a CoinJoin UTXO"); + let utxo = coinjoin_account.utxos.values().next().expect("CoinJoin UTXO"); + assert!(utxo.is_coinbase, "credited UTXO must be marked coinbase"); + + // Aggregate wallet balance — the actual regression is a lost credit. + // Before the fix, routing never consulted CoinJoin, so the reward was + // dropped and immature balance stayed 0. + assert_eq!( + managed_wallet.balance.immature(), + reward, + "immature balance must credit the CoinJoin coinbase reward" + ); + assert_eq!( + managed_wallet.balance.total(), + reward, + "total balance must include the immature CoinJoin coinbase" + ); + } + + /// Sibling credit-side regression for AssetUnlock (Platform credit withdrawal). + /// + /// Same membership-based routing gap as the coinbase case above: before the + /// fix, `AssetUnlock` only consulted StandardBIP44/BIP32, so a withdrawal to + /// a CoinJoin address was never credited (dashpay/rust-dashcore#900). + #[tokio::test] + async fn test_asset_unlock_paying_coinjoin_address_is_credited() { + use crate::transaction_checking::transaction_router::TransactionRouter; + use dashcore::blockdata::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; + use dashcore::blockdata::transaction::special_transaction::asset_unlock::request_info::AssetUnlockRequestInfo; + use dashcore::blockdata::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBasePayload; + use dashcore::blockdata::transaction::special_transaction::TransactionPayload; + use dashcore::bls_sig_utils::BLSSignature; + + let (mut wallet, mut managed_wallet, coinjoin_address) = wallet_with_coinjoin_address(); + + let unlock_value = 100_000_000u64; + let asset_unlock_tx = Transaction { + version: 3, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::from_byte_array([1u8; 32]), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: unlock_value, + script_pubkey: coinjoin_address.script_pubkey(), + }], + special_transaction_payload: Some(TransactionPayload::AssetUnlockPayloadType( + AssetUnlockPayload { + base: AssetUnlockBasePayload { + version: 1, + index: 42, + fee: 1000, + }, + request_info: AssetUnlockRequestInfo { + request_height: 500_000, + quorum_hash: [5u8; 32].into(), + }, + quorum_sig: BLSSignature::from([6u8; 96]), + }, + )), + }; + assert_eq!( + TransactionRouter::classify_transaction(&asset_unlock_tx), + TransactionType::AssetUnlock, + "tx must classify as AssetUnlock so it routes through the AssetUnlock arm" + ); + + // Use InBlock (not chainlocked) so the full record is retained under the + // default `keep-finalized-transactions=OFF` feature; the load-bearing + // assertions are UTXO creation and confirmed balance credit. + let context = TransactionContext::InBlock(BlockInfo::new( + 500_100, + BlockHash::from_slice(&[10u8; 32]).expect("Should create block hash"), + 1_650_000_300, + )); + let result = managed_wallet + .check_core_transaction(&asset_unlock_tx, context, &mut wallet, true, true) + .await; + managed_wallet.update_last_processed_height(500_100); + + assert!(result.is_relevant, "asset unlock paying a CoinJoin address must be relevant"); + assert_eq!( + result.total_received, unlock_value, + "asset unlock must credit the CoinJoin output value" + ); + + let coinjoin_account = + managed_wallet.first_coinjoin_managed_account().expect("coinjoin account"); + assert!( + coinjoin_account.transactions().contains_key(&asset_unlock_tx.txid()), + "asset unlock must be recorded on the CoinJoin account" + ); + assert_eq!(coinjoin_account.utxos.len(), 1, "asset unlock must create a CoinJoin UTXO"); + + assert_eq!( + managed_wallet.balance.confirmed(), + unlock_value, + "confirmed balance must credit the CoinJoin asset-unlock withdrawal" + ); + assert_eq!( + managed_wallet.balance.total(), + unlock_value, + "total balance must include the CoinJoin asset-unlock credit" + ); + } + /// Test the full coinbase maturity flow - immature to mature transition #[tokio::test] async fn test_wallet_checker_immature_transaction_flow() {