Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions key-wallet/src/managed_account/managed_core_funds_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
});
Comment thread
xdustinface marked this conversation as resolved.

let txid = tx.txid();
let mut utxos_changed = false;
Expand Down Expand Up @@ -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(),
Expand Down
80 changes: 80 additions & 0 deletions key-wallet/src/transaction_checking/wallet_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
123 changes: 82 additions & 41 deletions key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;

Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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<CreditOutputFunding> {
amounts
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<TransactionPayload>,
/// Reservation set of the funding account, captured by `set_funding`. The
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
26 changes: 26 additions & 0 deletions key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![
Expand Down
Loading