frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(wallet): remove TxBuilder::allow_shrinking function and TxBuilderContext

Browse Source
This commit is contained in:
Steve Myers 2024-05-16 21:47:56 -05:00
parent 2eea0f4e90
commit 096b8ef781
No known key found for this signature in database
GPG Key ID: 8105A46B22C2D051
3 changed files with 121 additions and 193 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
crates/wallet/src/wallet/mod.rs
View File

@ -59,7 +59,7 @@ pub use utils::IsDust;
use coin_selection::DefaultCoinSelectionAlgorithm;
use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
use tx_builder::{FeePolicy, TxBuilder, TxParams};
use utils::{check_nsequence_rbf, After, Older, SecpCtx};
use crate::descriptor::policy::BuildSatisfaction;
@ -1293,12 +1293,11 @@ impl Wallet {
/// ```
///
/// [`TxBuilder`]: crate::TxBuilder
pub fn build_tx(&mut self) -> TxBuilder<'_, DefaultCoinSelectionAlgorithm, CreateTx> {
pub fn build_tx(&mut self) -> TxBuilder<'_, DefaultCoinSelectionAlgorithm> {
TxBuilder {
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
params: TxParams::default(),
coin_selection: DefaultCoinSelectionAlgorithm::default(),
phantom: core::marker::PhantomData,
}
}
@ -1684,7 +1683,7 @@ impl Wallet {
pub fn build_fee_bump(
&mut self,
txid: Txid,
) -> Result<TxBuilder<'_, DefaultCoinSelectionAlgorithm, BumpFee>, BuildFeeBumpError> {
) -> Result<TxBuilder<'_, DefaultCoinSelectionAlgorithm>, BuildFeeBumpError> {
let graph = self.indexed_graph.graph();
let txout_index = &self.indexed_graph.index;
let chain_tip = self.chain.tip().block_id();
@ -1808,7 +1807,6 @@ impl Wallet {
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
params,
coin_selection: DefaultCoinSelectionAlgorithm::default(),
phantom: core::marker::PhantomData,
})
}

253
crates/wallet/src/wallet/tx_builder.rs
View File

@ -19,7 +19,6 @@
//! # use bdk_wallet::*;
//! # use bdk_wallet::wallet::ChangeSet;
//! # use bdk_wallet::wallet::error::CreateTxError;
//! # use bdk_wallet::wallet::tx_builder::CreateTx;
//! # use bdk_persist::PersistBackend;
//! # use anyhow::Error;
//! # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@ -43,31 +42,16 @@
use alloc::{boxed::Box, rc::Rc, string::String, vec::Vec};
use core::cell::RefCell;
use core::fmt;
use core::marker::PhantomData;
use bitcoin::psbt::{self, Psbt};
use bitcoin::script::PushBytes;
use bitcoin::{absolute, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};
use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
use super::coin_selection::CoinSelectionAlgorithm;
use super::{CreateTxError, Wallet};
use crate::collections::{BTreeMap, HashSet};
use crate::{KeychainKind, LocalOutput, Utxo, WeightedUtxo};
/// Context in which the [`TxBuilder`] is valid
pub trait TxBuilderContext: core::fmt::Debug + Default + Clone {}
/// Marker type to indicate the [`TxBuilder`] is being used to create a new transaction (as opposed
/// to bumping the fee of an existing one).
#[derive(Debug, Default, Clone)]
pub struct CreateTx;
impl TxBuilderContext for CreateTx {}
/// Marker type to indicate the [`TxBuilder`] is being used to bump the fee of an existing transaction.
#[derive(Debug, Default, Clone)]
pub struct BumpFee;
impl TxBuilderContext for BumpFee {}
/// A transaction builder
///
/// A `TxBuilder` is created by calling [`build_tx`] or [`build_fee_bump`] on a wallet. After
@ -123,11 +107,10 @@ impl TxBuilderContext for BumpFee {}
/// [`finish`]: Self::finish
/// [`coin_selection`]: Self::coin_selection
#[derive(Debug)]
pub struct TxBuilder<'a, Cs, Ctx> {
pub struct TxBuilder<'a, Cs> {
pub(crate) wallet: Rc<RefCell<&'a mut Wallet>>,
pub(crate) params: TxParams,
pub(crate) coin_selection: Cs,
pub(crate) phantom: PhantomData<Ctx>,
}
/// The parameters for transaction creation sans coin selection algorithm.
@ -175,19 +158,18 @@ impl Default for FeePolicy {
}
}
impl<'a, Cs: Clone, Ctx> Clone for TxBuilder<'a, Cs, Ctx> {
impl<'a, Cs: Clone> Clone for TxBuilder<'a, Cs> {
fn clone(&self) -> Self {
TxBuilder {
wallet: self.wallet.clone(),
params: self.params.clone(),
coin_selection: self.coin_selection.clone(),
phantom: PhantomData,
}
}
}
// methods supported by both contexts, for any CoinSelectionAlgorithm
impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
// Methods supported for any CoinSelectionAlgorithm.
impl<'a, Cs> TxBuilder<'a, Cs> {
/// Set a custom fee rate.
///
/// This method sets the mining fee paid by the transaction as a rate on its size.
@ -212,8 +194,8 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
/// Note that this is really a minimum absolute fee -- it's possible to
/// overshoot it slightly since adding a change output to drain the remaining
/// excess might not be viable.
pub fn fee_absolute(&mut self, fee_amount: u64) -> &mut Self {
self.params.fee_policy = Some(FeePolicy::FeeAmount(fee_amount));
pub fn fee_absolute(&mut self, fee_amount: Amount) -> &mut Self {
self.params.fee_policy = Some(FeePolicy::FeeAmount(fee_amount.to_sat()));
self
}
@ -553,18 +535,14 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
/// Choose the coin selection algorithm
///
/// Overrides the [`DefaultCoinSelectionAlgorithm`].
/// Overrides the [`CoinSelectionAlgorithm`].
///
/// Note that this function consumes the builder and returns it so it is usually best to put this as the first call on the builder.
pub fn coin_selection<P: CoinSelectionAlgorithm>(
self,
coin_selection: P,
) -> TxBuilder<'a, P, Ctx> {
pub fn coin_selection<P: CoinSelectionAlgorithm>(self, coin_selection: P) -> TxBuilder<'a, P> {
TxBuilder {
wallet: self.wallet,
params: self.params,
coin_selection,
phantom: PhantomData,
}
}
@ -612,9 +590,84 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
self.params.allow_dust = allow_dust;
self
}
/// Replace the recipients already added with a new list
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, Amount)>) -> &mut Self {
self.params.recipients = recipients
.into_iter()
.map(|(script, amount)| (script, amount.to_sat()))
.collect();
self
}
impl<'a, Cs: CoinSelectionAlgorithm, Ctx> TxBuilder<'a, Cs, Ctx> {
/// Add a recipient to the internal list
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: Amount) -> &mut Self {
self.params
.recipients
.push((script_pubkey, amount.to_sat()));
self
}
/// Add data as an output, using OP_RETURN
pub fn add_data<T: AsRef<PushBytes>>(&mut self, data: &T) -> &mut Self {
let script = ScriptBuf::new_op_return(data);
self.add_recipient(script, Amount::ZERO);
self
}
/// Sets the address to *drain* excess coins to.
///
/// Usually, when there are excess coins they are sent to a change address generated by the
/// wallet. This option replaces the usual change address with an arbitrary `script_pubkey` of
/// your choosing. Just as with a change output, if the drain output is not needed (the excess
/// coins are too small) it will not be included in the resulting transaction. The only
/// difference is that it is valid to use `drain_to` without setting any ordinary recipients
/// with [`add_recipient`] (but it is perfectly fine to add recipients as well).
///
/// If you choose not to set any recipients, you should provide the utxos that the
/// transaction should spend via [`add_utxos`].
///
/// # Example
///
/// `drain_to` is very useful for draining all the coins in a wallet with [`drain_wallet`] to a
/// single address.
///
/// ```
/// # use std::str::FromStr;
/// # use bitcoin::*;
/// # use bdk_wallet::*;
/// # use bdk_wallet::wallet::ChangeSet;
/// # use bdk_wallet::wallet::error::CreateTxError;
/// # use bdk_persist::PersistBackend;
/// # use anyhow::Error;
/// # let to_address =
/// Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt")
/// .unwrap()
/// .assume_checked();
/// # let mut wallet = doctest_wallet!();
/// let mut tx_builder = wallet.build_tx();
///
/// tx_builder
/// // Spend all outputs in this wallet.
/// .drain_wallet()
/// // Send the excess (which is all the coins minus the fee) to this address.
/// .drain_to(to_address.script_pubkey())
/// .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
/// .enable_rbf();
/// let psbt = tx_builder.finish()?;
/// # Ok::<(), anyhow::Error>(())
/// ```
///
/// [`add_recipient`]: Self::add_recipient
/// [`add_utxos`]: Self::add_utxos
/// [`drain_wallet`]: Self::drain_wallet
pub fn drain_to(&mut self, script_pubkey: ScriptBuf) -> &mut Self {
self.params.drain_to = Some(script_pubkey);
self
}
}
impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs> {
/// Finish building the transaction.
///
/// Returns a new [`Psbt`] per [`BIP174`].
@ -689,142 +742,6 @@ impl fmt::Display for AddForeignUtxoError {
#[cfg(feature = "std")]
impl std::error::Error for AddForeignUtxoError {}
#[derive(Debug)]
/// Error returned from [`TxBuilder::allow_shrinking`]
pub enum AllowShrinkingError {
/// Script/PubKey was not in the original transaction
MissingScriptPubKey(ScriptBuf),
}
impl fmt::Display for AllowShrinkingError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::MissingScriptPubKey(script_buf) => write!(
f,
"Script/PubKey was not in the original transaction: {}",
script_buf,
),
}
}
}
#[cfg(feature = "std")]
impl std::error::Error for AllowShrinkingError {}
impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
/// Replace the recipients already added with a new list
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, Amount)>) -> &mut Self {
self.params.recipients = recipients
.into_iter()
.map(|(script, amount)| (script, amount.to_sat()))
.collect();
self
}
/// Add a recipient to the internal list
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: Amount) -> &mut Self {
self.params
.recipients
.push((script_pubkey, amount.to_sat()));
self
}
/// Add data as an output, using OP_RETURN
pub fn add_data<T: AsRef<PushBytes>>(&mut self, data: &T) -> &mut Self {
let script = ScriptBuf::new_op_return(data);
self.add_recipient(script, Amount::ZERO);
self
}
/// Sets the address to *drain* excess coins to.
///
/// Usually, when there are excess coins they are sent to a change address generated by the
/// wallet. This option replaces the usual change address with an arbitrary `script_pubkey` of
/// your choosing. Just as with a change output, if the drain output is not needed (the excess
/// coins are too small) it will not be included in the resulting transaction. The only
/// difference is that it is valid to use `drain_to` without setting any ordinary recipients
/// with [`add_recipient`] (but it is perfectly fine to add recipients as well).
///
/// If you choose not to set any recipients, you should either provide the utxos that the
/// transaction should spend via [`add_utxos`], or set [`drain_wallet`] to spend all of them.
///
/// When bumping the fees of a transaction made with this option, you probably want to
/// use [`allow_shrinking`] to allow this output to be reduced to pay for the extra fees.
///
/// # Example
///
/// `drain_to` is very useful for draining all the coins in a wallet with [`drain_wallet`] to a
/// single address.
///
/// ```
/// # use std::str::FromStr;
/// # use bitcoin::*;
/// # use bdk_wallet::*;
/// # use bdk_wallet::wallet::ChangeSet;
/// # use bdk_wallet::wallet::error::CreateTxError;
/// # use bdk_wallet::wallet::tx_builder::CreateTx;
/// # use bdk_persist::PersistBackend;
/// # use anyhow::Error;
/// # let to_address =
/// Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt")
/// .unwrap()
/// .assume_checked();
/// # let mut wallet = doctest_wallet!();
/// let mut tx_builder = wallet.build_tx();
///
/// tx_builder
/// // Spend all outputs in this wallet.
/// .drain_wallet()
/// // Send the excess (which is all the coins minus the fee) to this address.
/// .drain_to(to_address.script_pubkey())
/// .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
/// .enable_rbf();
/// let psbt = tx_builder.finish()?;
/// # Ok::<(), anyhow::Error>(())
/// ```
///
/// [`allow_shrinking`]: Self::allow_shrinking
/// [`add_recipient`]: Self::add_recipient
/// [`add_utxos`]: Self::add_utxos
/// [`drain_wallet`]: Self::drain_wallet
pub fn drain_to(&mut self, script_pubkey: ScriptBuf) -> &mut Self {
self.params.drain_to = Some(script_pubkey);
self
}
}
// methods supported only by bump_fee
impl<'a> TxBuilder<'a, DefaultCoinSelectionAlgorithm, BumpFee> {
/// Explicitly tells the wallet that it is allowed to reduce the amount of the output matching this
/// `script_pubkey` in order to bump the transaction fee. Without specifying this the wallet
/// will attempt to find a change output to shrink instead.
///
/// **Note** that the output may shrink to below the dust limit and therefore be removed. If it is
/// preserved then it is currently not guaranteed to be in the same position as it was
/// originally.
///
/// Returns an `Err` if `script_pubkey` can't be found among the recipients of the
/// transaction we are bumping.
pub fn allow_shrinking(
&mut self,
script_pubkey: ScriptBuf,
) -> Result<&mut Self, AllowShrinkingError> {
match self
.params
.recipients
.iter()
.position(|(recipient_script, _)| *recipient_script == script_pubkey)
{
Some(position) => {
self.params.recipients.remove(position);
self.params.drain_to = Some(script_pubkey);
Ok(self)
}
None => Err(AllowShrinkingError::MissingScriptPubKey(script_pubkey)),
}
}
}
/// Ordering of the transaction's inputs and outputs
#[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
pub enum TxOrdering {

53
crates/wallet/tests/wallet.rs
View File

@ -779,7 +779,7 @@ fn test_create_tx_absolute_fee() {
builder
.drain_to(addr.script_pubkey())
.drain_wallet()
.fee_absolute(100);
.fee_absolute(Amount::from_sat(100));
let psbt = builder.finish().unwrap();
let fee = check_fee!(wallet, psbt);
@ -799,7 +799,7 @@ fn test_create_tx_absolute_zero_fee() {
builder
.drain_to(addr.script_pubkey())
.drain_wallet()
.fee_absolute(0);
.fee_absolute(Amount::ZERO);
let psbt = builder.finish().unwrap();
let fee = check_fee!(wallet, psbt);
@ -820,7 +820,7 @@ fn test_create_tx_absolute_high_fee() {
builder
.drain_to(addr.script_pubkey())
.drain_wallet()
.fee_absolute(60_000);
.fee_absolute(Amount::from_sat(60_000));
let _ = builder.finish().unwrap();
}
@ -1658,7 +1658,7 @@ fn test_bump_fee_low_abs() {
.unwrap();
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_absolute(10);
builder.fee_absolute(Amount::from_sat(10));
builder.finish().unwrap();
}
@ -1680,7 +1680,7 @@ fn test_bump_fee_zero_abs() {
.unwrap();
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_absolute(0);
builder.fee_absolute(Amount::ZERO);
builder.finish().unwrap();
}
@ -1742,7 +1742,7 @@ fn test_bump_fee_reduce_change() {
assert_fee_rate!(psbt, fee.unwrap_or(0), feerate, @add_signature);
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_absolute(200);
builder.fee_absolute(Amount::from_sat(200));
builder.enable_rbf();
let psbt = builder.finish().unwrap();
let sent_received =
@ -1807,8 +1807,12 @@ fn test_bump_fee_reduce_single_recipient() {
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder
.fee_rate(feerate)
.allow_shrinking(addr.script_pubkey())
.unwrap();
// remove original tx drain_to address and amount
.set_recipients(Vec::new())
// set back original drain_to address
.drain_to(addr.script_pubkey())
// drain wallet output amount will be re-calculated with new fee rate
.drain_wallet();
let psbt = builder.finish().unwrap();
let sent_received =
wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx"));
@ -1849,9 +1853,13 @@ fn test_bump_fee_absolute_reduce_single_recipient() {
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder
.allow_shrinking(addr.script_pubkey())
.unwrap()
.fee_absolute(300);
.fee_absolute(Amount::from_sat(300))
// remove original tx drain_to address and amount
.set_recipients(Vec::new())
// set back original drain_to address
.drain_to(addr.script_pubkey())
// drain wallet output amount will be re-calculated with new fee rate
.drain_wallet();
let psbt = builder.finish().unwrap();
let tx = &psbt.unsigned_tx;
let sent_received = wallet.sent_and_received(tx);
@ -1923,8 +1931,6 @@ fn test_bump_fee_drain_wallet() {
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder
.drain_wallet()
.allow_shrinking(addr.script_pubkey())
.unwrap()
.fee_rate(FeeRate::from_sat_per_vb_unchecked(5));
let psbt = builder.finish().unwrap();
let sent_received = wallet.sent_and_received(&psbt.extract_tx().expect("failed to extract tx"));
@ -1940,7 +1946,7 @@ fn test_bump_fee_remove_output_manually_selected_only() {
// them, and make sure that `bump_fee` doesn't try to add more. This fails because we've
// told the wallet it's not allowed to add more inputs AND it can't reduce the value of the
// existing output. In other words, bump_fee + manually_selected_only is always an error
// unless you've also set "allow_shrinking" OR there is a change output.
// unless there is a change output.
let init_tx = Transaction {
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
@ -2092,7 +2098,7 @@ fn test_bump_fee_absolute_add_input() {
.unwrap();
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_absolute(6_000);
builder.fee_absolute(Amount::from_sat(6_000));
let psbt = builder.finish().unwrap();
let sent_received =
wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx"));
@ -2157,8 +2163,8 @@ fn test_bump_fee_no_change_add_input_and_change() {
.insert_tx(tx, ConfirmationTime::Unconfirmed { last_seen: 0 })
.unwrap();
// now bump the fees without using `allow_shrinking`. the wallet should add an
// extra input and a change output, and leave the original output untouched
// Now bump the fees, the wallet should add an extra input and a change output, and leave
// the original output untouched.
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_rate(FeeRate::from_sat_per_vb_unchecked(50));
let psbt = builder.finish().unwrap();
@ -2369,7 +2375,10 @@ fn test_bump_fee_absolute_force_add_input() {
// the new fee_rate is low enough that just reducing the change would be fine, but we force
// the addition of an extra input with `add_utxo()`
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.add_utxo(incoming_op).unwrap().fee_absolute(250);
builder
.add_utxo(incoming_op)
.unwrap()
.fee_absolute(Amount::from_sat(250));
let psbt = builder.finish().unwrap();
let sent_received =
wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx"));
@ -2478,8 +2487,12 @@ fn test_bump_fee_unconfirmed_input() {
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder
.fee_rate(FeeRate::from_sat_per_vb_unchecked(15))
.allow_shrinking(addr.script_pubkey())
.unwrap();
// remove original tx drain_to address and amount
.set_recipients(Vec::new())
// set back original drain_to address
.drain_to(addr.script_pubkey())
// drain wallet output amount will be re-calculated with new fee rate
.drain_wallet();
builder.finish().unwrap();
}