diff --git a/src/cli.rs b/src/cli.rs index bf50d961..06b74fcb 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -406,7 +406,7 @@ where .map(|i| parse_outpoint(i)) .collect::, _>>() .map_err(|s| Error::Generic(s.to_string()))?; - tx_builder = tx_builder.utxos(utxos); + tx_builder = tx_builder.utxos(utxos).manually_selected_only(); } if let Some(unspendable) = sub_matches.values_of("unspendable") { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index ac350d45..25482ead 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -349,17 +349,14 @@ where )); } - let (available_utxos, use_all_utxos) = self.get_available_utxos( + let (must_use_utxos, may_use_utxos) = self.get_must_may_use_utxos( builder.change_policy, - &builder.utxos, &builder.unspendable, + &builder.utxos, builder.send_all, + builder.manually_selected_only, )?; - let (must_use_utxos, may_use_utxos) = match use_all_utxos { - true => (available_utxos, vec![]), - false => (vec![], available_utxos), - }; let coin_selection::CoinSelectionResult { txin, selected_amount, @@ -596,30 +593,29 @@ where }) .collect::, _>>()?; - let builder_extra_utxos = builder.utxos.as_ref().map(|utxos| { - utxos - .iter() - .filter(|utxo| { - !original_txin - .iter() - .any(|txin| &&txin.previous_output == utxo) - }) - .cloned() - .collect() - }); - let (available_utxos, use_all_utxos) = self.get_available_utxos( - builder.change_policy, - &builder_extra_utxos, - &builder.unspendable, - false, - )?; - let available_utxos = - rbf::filter_available(self.database.borrow().deref(), available_utxos.into_iter())?; + let builder_extra_utxos = builder + .utxos + .iter() + .filter(|utxo| { + !original_txin + .iter() + .any(|txin| &&txin.previous_output == utxo) + }) + .cloned() + .collect::>(); - let (mut must_use_utxos, may_use_utxos) = match use_all_utxos { - true => (available_utxos, vec![]), - false => (vec![], available_utxos), - }; + let (must_use_utxos, may_use_utxos) = self.get_must_may_use_utxos( + builder.change_policy, + &builder.unspendable, + &builder_extra_utxos[..], + false, // when doing bump_fee `send_all` does not mean use all available utxos + builder.manually_selected_only, + )?; + + let mut must_use_utxos = + rbf::filter_available(self.database.borrow().deref(), must_use_utxos.into_iter())?; + let may_use_utxos = + rbf::filter_available(self.database.borrow().deref(), may_use_utxos.into_iter())?; must_use_utxos.append(&mut original_utxos); let amount_needed = tx.output.iter().fold(0, |acc, out| acc + out.value); @@ -965,13 +961,7 @@ where Ok(()) } - fn get_available_utxos( - &self, - change_policy: tx_builder::ChangeSpendPolicy, - utxo: &Option>, - unspendable: &HashSet, - send_all: bool, - ) -> Result<(Vec<(UTXO, usize)>, bool), Error> { + fn get_available_utxos(&self) -> Result, Error> { let external_weight = self .get_descriptor_for_script_type(ScriptType::External) .0 @@ -990,33 +980,57 @@ where (utxo, weight) }; - match utxo { - // with manual coin selection we always want to spend all the selected utxos, no matter - // what (even if they are marked as unspendable) - Some(raw_utxos) => { - let full_utxos = raw_utxos - .iter() - .map(|u| { - Ok(add_weight( - self.database - .borrow() - .get_utxo(&u)? - .ok_or(Error::UnknownUTXO)?, - )) - }) - .collect::, Error>>()?; + let utxos = self.list_unspent()?.into_iter().map(add_weight).collect(); - Ok((full_utxos, true)) - } - // otherwise limit ourselves to the spendable utxos for the selected policy, and the `send_all` setting - None => { - let utxos = self.list_unspent()?.into_iter().filter(|u| { - change_policy.is_satisfied_by(u) && !unspendable.contains(&u.outpoint) - }); + Ok(utxos) + } - Ok((utxos.map(add_weight).collect(), send_all)) - } + /// Given the options returns the list of utxos that must be used to form the + /// transaction and any further that may be used if needed. + #[allow(clippy::type_complexity)] + fn get_must_may_use_utxos( + &self, + change_policy: tx_builder::ChangeSpendPolicy, + unspendable: &HashSet, + manually_selected: &[OutPoint], + must_use_all_available: bool, + manual_only: bool, + ) -> Result<(Vec<(UTXO, usize)>, Vec<(UTXO, usize)>), Error> { + // must_spend <- manually selected utxos + // may_spend <- all other available utxos + let mut may_spend = self.get_available_utxos()?; + let mut must_spend = { + let must_spend_idx = manually_selected + .iter() + .map(|manually_selected| { + may_spend + .iter() + .position(|available| available.0.outpoint == *manually_selected) + .ok_or(Error::UnknownUTXO) + }) + .collect::, _>>()?; + + must_spend_idx + .into_iter() + .map(|i| may_spend.remove(i)) + .collect() + }; + + // NOTE: we are intentionally ignoring `unspendable` here. i.e manual + // selection overrides unspendable. + if manual_only { + return Ok((must_spend, vec![])); } + + may_spend.retain(|u| { + change_policy.is_satisfied_by(&u.0) && !unspendable.contains(&u.0.outpoint) + }); + + if must_use_all_available { + must_spend.append(&mut may_spend); + } + + Ok((must_spend, may_spend)) } fn complete_transaction>( @@ -1988,6 +2002,56 @@ mod test { assert!(psbt.inputs[0].witness_utxo.is_some()); } + #[test] + fn test_create_tx_add_utxo() { + let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); + let small_output_txid = wallet.database.borrow_mut().received_tx( + testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 1)), + Some(100), + ); + + let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); + let (psbt, details) = wallet + .create_tx( + TxBuilder::with_recipients(vec![(addr.script_pubkey(), 30_000)]).add_utxo( + OutPoint { + txid: small_output_txid, + vout: 0, + }, + ), + ) + .unwrap(); + + assert_eq!( + psbt.global.unsigned_tx.input.len(), + 2, + "should add an additional input since 25_000 < 30_000" + ); + assert_eq!(details.sent, 75_000, "total should be sum of both inputs"); + } + + #[test] + #[should_panic(expected = "InsufficientFunds")] + fn test_create_tx_manually_selected_insufficient() { + let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); + let small_output_txid = wallet.database.borrow_mut().received_tx( + testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 1)), + Some(100), + ); + + let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); + wallet + .create_tx( + TxBuilder::with_recipients(vec![(addr.script_pubkey(), 30_000)]) + .add_utxo(OutPoint { + txid: small_output_txid, + vout: 0, + }) + .manually_selected_only(), + ) + .unwrap(); + } + #[test] #[should_panic(expected = "IrreplaceableTransaction")] fn test_bump_fee_irreplaceable_tx() { @@ -2330,6 +2394,7 @@ mod test { txid: incoming_txid, vout: 0, }]) + .manually_selected_only() .send_all() .enable_rbf(), ) @@ -2506,6 +2571,7 @@ mod test { txid: incoming_txid, vout: 0, }) + .manually_selected_only() .enable_rbf(), ) .unwrap(); diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index f72bf48d..17f97dd9 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -63,8 +63,9 @@ pub struct TxBuilder> { pub(crate) send_all: bool, pub(crate) fee_policy: Option, pub(crate) policy_path: Option>>, - pub(crate) utxos: Option>, + pub(crate) utxos: Vec, pub(crate) unspendable: HashSet, + pub(crate) manually_selected_only: bool, pub(crate) sighash: Option, pub(crate) ordering: TxOrdering, pub(crate) locktime: Option, @@ -102,6 +103,7 @@ where policy_path: Default::default(), utxos: Default::default(), unspendable: Default::default(), + manually_selected_only: Default::default(), sighash: Default::default(), ordering: Default::default(), locktime: Default::default(), @@ -141,11 +143,19 @@ impl> TxBuilder { self } - /// Send all the selected utxos to a single output + /// Send all inputs to a single output. + /// + /// The semantics of `send_all` depend on whether you are using [`create_tx`] or [`bump_fee`]. + /// In `create_tx` it (by default) **selects all the wallets inputs** and sends them to a single + /// output. In `bump_fee` it means to send the original inputs and any additional manually + /// selected intputs to a single output. /// /// Adding more than one recipients with this option enabled will result in an error. /// /// The value associated with the only recipient is irrelevant and will be replaced by the wallet. + /// + /// [`bump_fee`]: crate::wallet::Wallet::bump_fee + /// [`create_tx`]: crate::wallet::Wallet::create_tx pub fn send_all(mut self) -> Self { self.send_all = true; self @@ -179,7 +189,7 @@ impl> TxBuilder { /// These have priority over the "unspendable" utxos, meaning that if a utxo is present both in /// the "utxos" and the "unspendable" list, it will be spent. pub fn utxos(mut self, utxos: Vec) -> Self { - self.utxos = Some(utxos); + self.utxos = utxos; self } @@ -188,7 +198,19 @@ impl> TxBuilder { /// These have priority over the "unspendable" utxos, meaning that if a utxo is present both in /// the "utxos" and the "unspendable" list, it will be spent. pub fn add_utxo(mut self, utxo: OutPoint) -> Self { - self.utxos.get_or_insert(vec![]).push(utxo); + self.utxos.push(utxo); + self + } + + /// Only spend utxos added by [`add_utxo`] and [`utxos`]. + /// + /// The wallet will **not** add additional utxos to the transaction even if they are needed to + /// make the transaction valid. + /// + /// [`add_utxo`]: Self::add_utxo + /// [`utxos`]: Self::utxos + pub fn manually_selected_only(mut self) -> Self { + self.manually_selected_only = true; self } @@ -310,6 +332,7 @@ impl> TxBuilder { policy_path: self.policy_path, utxos: self.utxos, unspendable: self.unspendable, + manually_selected_only: self.manually_selected_only, sighash: self.sighash, ordering: self.ordering, locktime: self.locktime,