diff --git a/key-wallet/src/managed_account/managed_core_funds_account.rs b/key-wallet/src/managed_account/managed_core_funds_account.rs index eb4f9dd98..474b725aa 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -179,11 +179,20 @@ impl ManagedCoreFundsAccount { .map(|info| info.address.clone()) .collect(); - // Detect a self-send: this account owns at least one input being - // spent. `account_match.sent` is computed by matching inputs against - // this account's UTXO set, so a non-zero value means we owned at - // least one of the spent outpoints. - let has_owned_input = account_match.sent > 0; + // Detect a trusted self-send, mirroring Bitcoin Core's + // `CWalletTx::IsTrusted`: every input must spend one of our own + // UTXOs that is itself final (confirmed, InstantSend-locked, or + // trusted). Parent trust already carries the recursion, so one + // level of lookup is transitive over the whole ancestry. The + // spent parents are still present in `self.utxos` here because + // they are only removed after the insert loop below. An unknown + // or non-final parent denies trust, so funds that the network + // may still drop never surface as confirmed. + let all_inputs_final_and_ours = tx.input.iter().all(|input| { + self.utxos.get(&input.previous_output).is_some_and(|parent| { + parent.is_confirmed || parent.is_instantlocked || parent.is_trusted + }) + }); let txid = tx.txid(); let mut utxos_changed = false; @@ -215,12 +224,13 @@ impl ManagedCoreFundsAccount { } // Flag outputs from a "trusted" mempool transaction we created — - // one that spends at least one of our own UTXOs and pays this - // output back to one of our internal (change) addresses. Such - // an output is just our previously-tracked funds returning, so - // `update_balance` credits it to the confirmed bucket even + // one whose inputs all spend our own final UTXOs and which pays + // this output back to one of our internal (change) addresses. + // Such an output is just our previously-tracked funds returning, + // so `update_balance` credits it to the confirmed bucket even // before the parent transaction settles. - let is_trusted_output = has_owned_input && change_addrs.contains(&addr); + let is_trusted_output = + all_inputs_final_and_ours && change_addrs.contains(&addr); let txout = dashcore::TxOut { value: output.value, script_pubkey: output.script_pubkey.clone(), diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index b62def243..209930457 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -1814,6 +1814,86 @@ mod tests { assert_eq!(ctx.managed_wallet.balance.spendable(), change_amount); } + /// Sibling of `test_self_send_change_in_mempool_lands_in_confirmed_balance`: + /// a self-send change output is only trusted when the spent parent is + /// itself final. `Utxo::is_trusted` mirrors Bitcoin Core's + /// `CWalletTx::IsTrusted`, which is recursive: a 0-conf output is trusted + /// only if every parent resolves to confirmed, InstantSend-locked, or + /// trusted. Change spending an unconfirmed external parent must therefore + /// stay in the unconfirmed bucket, otherwise non-final funds surface as + /// confirmed/spendable and downstream asset locks get built on them. + #[tokio::test] + async fn test_self_send_change_with_unconfirmed_parent_is_not_trusted() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + // Unconfirmed external funding UTXO: the parent stays in the mempool. + let funding_value = 1_000_000u64; + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[funding_value]); + ctx.check_transaction(&funding_tx, TransactionContext::Mempool).await; + assert_eq!(ctx.managed_wallet.balance.confirmed(), 0); + assert_eq!(ctx.managed_wallet.balance.unconfirmed(), funding_value); + + let change_address = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + + // Spend the still-unconfirmed funding UTXO: some out, the rest back + // to ourselves as change, broadcast into the mempool. + let send_amount = 600_000u64; + let fee = 1_000u64; + let change_amount = funding_value - send_amount - fee; + let spend_tx = Transaction { + version: 2, + 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: send_amount, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change_address.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + ctx.check_transaction(&spend_tx, TransactionContext::Mempool).await; + + let change_outpoint = OutPoint { + txid: spend_tx.txid(), + vout: 1, + }; + let change_utxo = + ctx.bip44_account().utxos.get(&change_outpoint).expect("change UTXO recorded"); + + assert!( + !change_utxo.is_trusted, + "change spending an unconfirmed parent must not be trusted" + ); + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 0, + "non-final funds must not be counted as confirmed" + ); + assert_eq!(ctx.managed_wallet.balance.unconfirmed(), change_amount); + } + /// Sibling of `test_self_send_change_in_mempool_lands_in_confirmed_balance`: /// when the wallet receives a mempool payment but does not own any of the /// inputs, an output that happens to land on one of our addresses must diff --git a/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs index 05753d336..4542ace06 100644 --- a/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs @@ -213,6 +213,7 @@ impl ManagedWalletInfo { credit_outputs, ))) .set_funding(funds_acc, acc) + .require_final_inputs() .build_signed(wallet, |addr| funds_acc.address_derivation_path(&addr)) .await?; @@ -283,6 +284,7 @@ impl ManagedWalletInfo { credit_outputs, ))) .set_funding(funds_acc, &acc) + .require_final_inputs() .build_signed(signer, |addr| funds_acc.address_derivation_path(&addr)) .await?; @@ -346,8 +348,9 @@ mod tests { use super::*; use crate::signer::{ExtendedPubKeySigner, SignerMethod}; use crate::wallet::initialization::WalletAccountCreationOptions; - use crate::Network; - use dashcore::ScriptBuf; + use crate::{Network, Utxo}; + use dashcore::{OutPoint, ScriptBuf, Txid}; + use dashcore_hashes::Hash; fn test_credit_outputs(amounts: &[u64]) -> Vec { amounts @@ -374,6 +377,40 @@ mod tests { (wallet, info) } + /// Fund account 0 with a UTXO at a fresh receive address and return its + /// outpoint. + fn insert_funded_utxo( + info: &mut ManagedWalletInfo, + wallet: &Wallet, + txid_byte: u8, + value: u64, + is_confirmed: bool, + ) -> OutPoint { + let account_xpub = wallet.get_bip44_account(0).unwrap().account_xpub; + let account = info.accounts.standard_bip44_accounts.get_mut(&0).unwrap(); + let funding_address = account.next_receive_address(Some(&account_xpub), true).unwrap(); + let outpoint = OutPoint { + txid: Txid::from_byte_array([txid_byte; 32]), + vout: 0, + }; + let utxo = Utxo { + outpoint, + txout: TxOut { + value, + script_pubkey: funding_address.script_pubkey(), + }, + address: funding_address, + height: 1000, + is_coinbase: false, + is_confirmed, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }; + account.utxos.insert(outpoint, utxo); + outpoint + } + // -- Error type tests -- #[test] @@ -422,6 +459,46 @@ mod tests { ); } + /// An account whose only funds are an unconfirmed mempool UTXO must not + /// produce an asset lock: a non-final input is not InstantSend-eligible + /// per DIP-0010, so the funding transaction could never receive the lock + /// Platform requires. + #[tokio::test] + async fn test_rejects_non_final_funding() { + let (wallet, mut info) = test_wallet_and_info(); + insert_funded_utxo(&mut info, &wallet, 0x11, 1_000_000, false); + info.update_last_processed_height(1100); + + let result = info.build_asset_lock(&wallet, 0, test_credit_outputs(&[200_000]), 1000).await; + assert!( + matches!(result, Err(AssetLockError::Builder(_))), + "asset lock must not be built on unconfirmed funds, got: {:?}", + result.err() + ); + } + + /// With a mix of confirmed and unconfirmed funds, coin selection must only + /// spend the confirmed UTXO, even though the unconfirmed one is larger. + #[tokio::test] + async fn test_selects_only_final_funding() { + let (wallet, mut info) = test_wallet_and_info(); + let confirmed_outpoint = insert_funded_utxo(&mut info, &wallet, 0x22, 1_000_000, true); + insert_funded_utxo(&mut info, &wallet, 0x33, 5_000_000, false); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock(&wallet, 0, test_credit_outputs(&[200_000]), 1000) + .await + .expect("confirmed funds should cover the asset lock"); + assert!(!result.transaction.input.is_empty()); + for txin in &result.transaction.input { + assert_eq!( + txin.previous_output, confirmed_outpoint, + "asset lock spent a non-final input" + ); + } + } + // -- Signer-variant tests -- /// Signer implementation backed by a real [`RootExtendedPrivKey`]. Models @@ -599,10 +676,6 @@ mod tests { #[tokio::test] async fn test_signer_happy_path_end_to_end() { - use crate::Utxo; - use dashcore::{OutPoint, TxOut, Txid}; - use dashcore_hashes::Hash; - let (wallet, mut info) = test_wallet_and_info(); let root = match &wallet.wallet_type { crate::wallet::WalletType::Mnemonic { @@ -612,41 +685,9 @@ mod tests { _ => unreachable!(), }; - // Generate a receive address on account 0 and fund it with a - // real UTXO at that address — coin selection needs a confirmed, - // spendable output the signer can sign for. - let account_xpub = wallet.get_bip44_account(0).unwrap().account_xpub; - let funding_address = info - .accounts - .standard_bip44_accounts - .get_mut(&0) - .unwrap() - .next_receive_address(Some(&account_xpub), true) - .unwrap(); - - let utxo = Utxo { - outpoint: OutPoint { - txid: Txid::from_byte_array([0x11; 32]), - vout: 0, - }, - txout: TxOut { - value: 1_000_000, - script_pubkey: funding_address.script_pubkey(), - }, - address: funding_address, - height: 1000, - is_coinbase: false, - is_confirmed: true, - is_instantlocked: false, - is_locked: false, - is_trusted: false, - }; - info.accounts - .standard_bip44_accounts - .get_mut(&0) - .unwrap() - .utxos - .insert(utxo.outpoint, utxo); + // Coin selection needs a confirmed, spendable output the signer can + // sign for. + insert_funded_utxo(&mut info, &wallet, 0x11, 1_000_000, true); info.update_last_processed_height(1100); let signer = InMemorySigner { diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 65d628e85..2ed56efb3 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -56,6 +56,7 @@ pub struct TransactionBuilder { fee_rate: FeeRate, current_height: u32, selection_strategy: SelectionStrategy, + require_final_inputs: bool, /// Special transaction payload for Dash-specific transactions special_payload: Option, /// Reservation set of the funding account, captured by `set_funding`. The @@ -80,11 +81,23 @@ impl TransactionBuilder { fee_rate: FeeRate::normal(), current_height: 0, selection_strategy: SelectionStrategy::BranchAndBound, + require_final_inputs: false, special_payload: None, reservations: None, } } + /// Restrict coin selection to final inputs: confirmed or + /// InstantSend-locked UTXOs. Per DIP-0010 only such inputs are + /// InstantSend-eligible, so transactions that must receive an + /// InstantSend lock themselves (e.g. asset locks funding Platform + /// credits) must never spend other mempool outputs, including our + /// own trusted change. + pub fn require_final_inputs(mut self) -> Self { + self.require_final_inputs = true; + self + } + pub fn set_current_height(mut self, current_height: u32) -> Self { self.current_height = current_height; self @@ -301,6 +314,10 @@ impl TransactionBuilder { _ => self.outputs.iter().map(|o| o.value).sum(), }; + if self.require_final_inputs { + self.inputs.retain(|utxo| utxo.is_confirmed || utxo.is_instantlocked); + } + let selection = CoinSelector::new(self.selection_strategy) .select_coins_with_size( self.inputs.iter(), diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs index 1a9cb8c4c..029ab4782 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs @@ -175,6 +175,32 @@ mod tests { assert!(change_output.is_some(), "Should have change output"); } + /// Ordinary spends deliberately admit unconfirmed inputs: only builders + /// that opt in via `require_final_inputs` (e.g. asset locks) restrict + /// selection to confirmed or InstantSend-locked UTXOs. + #[test] + fn test_ordinary_spend_admits_unconfirmed_inputs() { + let utxos = vec![Utxo::dummy(0, 300000, 100, false, false)]; + let recipient_address = Address::from_str("yTb47qEBpNmgXvYYsHEN4nh8yJwa5iC4Cs") + .unwrap() + .require_network(Network::Testnet) + .unwrap(); + let change_address = Address::from_str("yXfXh3jFYHHxnJZVsXnPcktCENqPaAhcX1") + .unwrap() + .require_network(Network::Testnet) + .unwrap(); + + let (tx, _fee) = TransactionBuilder::new() + .set_fee_rate(FeeRate::normal()) + .set_current_height(200) + .set_change_address(change_address) + .add_output(&recipient_address, 150000) + .add_inputs(utxos) + .build_unsigned() + .expect("ordinary spend of an unconfirmed UTXO must succeed"); + assert!(!tx.input.is_empty()); + } + #[test] fn test_sweep_builder_drains_to_single_output() { let utxos = vec![