Skip to content

Commit

Permalink
fix(wallet): remove TxBuilder allow_shrinking function and unneeded c…
Browse files Browse the repository at this point in the history 
…ontext param
  • Loading branch information
@notmandatory
notmandatory committed Mar 25, 2024
1 parent 80e190b commit b51fa22
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 159 deletions.
8 changes: 3 additions & 5 deletions crates/bdk/src/wallet/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub use utils::IsDust;
#[allow(deprecated)]
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;
Expand Down Expand Up @@ -1252,12 +1252,11 @@ impl<D> Wallet<D> {
/// ```
///
/// [`TxBuilder`]: crate::TxBuilder
pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, CreateTx> {
pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm> {
TxBuilder {
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
params: TxParams::default(),
coin_selection: DefaultCoinSelectionAlgorithm::default(),
phantom: core::marker::PhantomData,
}
}

Expand Down Expand Up @@ -1662,7 +1661,7 @@ impl<D> Wallet<D> {
pub fn build_fee_bump(
&mut self,
txid: Txid,
) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, BumpFee>, BuildFeeBumpError> {
) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm>, BuildFeeBumpError> {
let graph = self.indexed_graph.graph();
let txout_index = &self.indexed_graph.index;
let chain_tip = self.chain.tip().block_id();
Expand Down Expand Up @@ -1786,7 +1785,6 @@ impl<D> Wallet<D> {
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
params,
coin_selection: DefaultCoinSelectionAlgorithm::default(),
phantom: core::marker::PhantomData,
})
}

Expand Down
220 changes: 78 additions & 142 deletions crates/bdk/src/wallet/tx_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,13 @@
use alloc::{boxed::Box, rc::Rc, string::String, vec::Vec};
use core::cell::RefCell;
use core::fmt;
use core::marker::PhantomData;

use bdk_chain::PersistBackend;
use bitcoin::psbt::{self, PartiallySignedTransaction as Psbt};
use bitcoin::script::PushBytes;
use bitcoin::{absolute, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};

use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
use super::coin_selection::CoinSelectionAlgorithm;
use super::{ChangeSet, CreateTxError, Wallet};
use crate::collections::{BTreeMap, HashSet};
use crate::{KeychainKind, LocalOutput, Utxo, WeightedUtxo};
Expand Down Expand Up @@ -124,11 +123,10 @@ impl TxBuilderContext for BumpFee {}
/// [`finish`]: Self::finish
/// [`coin_selection`]: Self::coin_selection
#[derive(Debug)]
pub struct TxBuilder<'a, D, Cs, Ctx> {
pub struct TxBuilder<'a, D, Cs> {
pub(crate) wallet: Rc<RefCell<&'a mut Wallet<D>>>,
pub(crate) params: TxParams,
pub(crate) coin_selection: Cs,
pub(crate) phantom: PhantomData<Ctx>,
}

/// The parameters for transaction creation sans coin selection algorithm.
Expand Down Expand Up @@ -176,19 +174,18 @@ impl Default for FeePolicy {
}
}

impl<'a, D, Cs: Clone, Ctx> Clone for TxBuilder<'a, D, Cs, Ctx> {
impl<'a, D, Cs: Clone> Clone for TxBuilder<'a, D, 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, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
// methods supported for any CoinSelectionAlgorithm
impl<'a, D, Cs> TxBuilder<'a, D, Cs> {
/// Set a custom fee rate.
///
/// This method sets the mining fee paid by the transaction as a rate on its size.
Expand Down Expand Up @@ -561,12 +558,11 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
pub fn coin_selection<P: CoinSelectionAlgorithm>(
self,
coin_selection: P,
) -> TxBuilder<'a, D, P, Ctx> {
) -> TxBuilder<'a, D, P> {
TxBuilder {
wallet: self.wallet,
params: self.params,
coin_selection,
phantom: PhantomData,
}
}

Expand Down Expand Up @@ -614,9 +610,80 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, 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, u64)>) -> &mut Self {
self.params.recipients = recipients;
self
}

/// Add a recipient to the internal list
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: u64) -> &mut Self {
self.params.recipients.push((script_pubkey, amount));
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, 0u64);
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::*;
/// # use bdk::wallet::ChangeSet;
/// # use bdk::wallet::error::CreateTxError;
/// # use bdk::wallet::tx_builder::CreateTx;
/// # use bdk_chain::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, D, Cs: CoinSelectionAlgorithm, Ctx> TxBuilder<'a, D, Cs, Ctx> {
impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs> {
/// Finish building the transaction.
///
/// Returns a new [`Psbt`] per [`BIP174`].
Expand Down Expand Up @@ -694,137 +761,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, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
/// Replace the recipients already added with a new list
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, u64)>) -> &mut Self {
self.params.recipients = recipients;
self
}

/// Add a recipient to the internal list
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: u64) -> &mut Self {
self.params.recipients.push((script_pubkey, amount));
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, 0u64);
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::*;
/// # use bdk::wallet::ChangeSet;
/// # use bdk::wallet::error::CreateTxError;
/// # use bdk::wallet::tx_builder::CreateTx;
/// # use bdk_chain::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, D> TxBuilder<'a, D, 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 {
Expand Down
Loading

0 comments on commit b51fa22

Please sign in to comment.