Skip to content

Remove TxBuilder allow_shrinking() and unneeded context param #1386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
notmandatory merged 1 commit into from
May 23, 2024
Merged

Remove TxBuilder allow_shrinking() and unneeded context param #1386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 3 additions & 5 deletions crates/wallet/src/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
})
}

Expand Down
243 changes: 80 additions & 163 deletions crates/wallet/src/wallet/tx_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -612,106 +590,7 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
self.params.allow_dust = allow_dust;
self
}
}

impl<'a, Cs: CoinSelectionAlgorithm, Ctx> TxBuilder<'a, Cs, Ctx> {
/// Finish building the transaction.
///
/// Returns a new [`Psbt`] per [`BIP174`].
///
/// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
pub fn finish(self) -> Result<Psbt, CreateTxError> {
self.wallet
.borrow_mut()
.create_tx(self.coin_selection, self.params)
}
}

#[derive(Debug)]
/// Error returned from [`TxBuilder::add_utxo`] and [`TxBuilder::add_utxos`]
pub enum AddUtxoError {
/// Happens when trying to spend an UTXO that is not in the internal database
UnknownUtxo(OutPoint),
}

impl fmt::Display for AddUtxoError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::UnknownUtxo(outpoint) => write!(
f,
"UTXO not found in the internal database for txid: {} with vout: {}",
outpoint.txid, outpoint.vout
),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for AddUtxoError {}

#[derive(Debug)]
/// Error returned from [`TxBuilder::add_foreign_utxo`].
pub enum AddForeignUtxoError {
/// Foreign utxo outpoint txid does not match PSBT input txid
InvalidTxid {
/// PSBT input txid
input_txid: Txid,
/// Foreign UTXO outpoint
foreign_utxo: OutPoint,
},
/// Requested outpoint doesn't exist in the tx (vout greater than available outputs)
InvalidOutpoint(OutPoint),
/// Foreign utxo missing witness_utxo or non_witness_utxo
MissingUtxo,
}

impl fmt::Display for AddForeignUtxoError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::InvalidTxid {
input_txid,
foreign_utxo,
} => write!(
f,
"Foreign UTXO outpoint txid: {} does not match PSBT input txid: {}",
foreign_utxo.txid, input_txid,
),
Self::InvalidOutpoint(outpoint) => write!(
f,
"Requested outpoint doesn't exist for txid: {} with vout: {}",
outpoint.txid, outpoint.vout,
),
Self::MissingUtxo => write!(f, "Foreign utxo missing witness_utxo or non_witness_utxo"),
}
}
}

#[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
Expand Down Expand Up @@ -745,11 +624,8 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
/// 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.
/// If you choose not to set any recipients, you should provide the utxos that the
/// transaction should spend via [`add_utxos`].
///
/// # Example
///
Expand All @@ -762,7 +638,6 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
/// # 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 =
Expand All @@ -783,7 +658,6 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
/// # Ok::<(), anyhow::Error>(())
/// ```
///
/// [`allow_shrinking`]: Self::allow_shrinking
/// [`add_recipient`]: Self::add_recipient
/// [`add_utxos`]: Self::add_utxos
/// [`drain_wallet`]: Self::drain_wallet
Expand All @@ -793,38 +667,81 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
}
}

// 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.
impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs> {
/// Finish building the transaction.
///
/// **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 a new [`Psbt`] per [`BIP174`].
///
/// 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)),
/// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
pub fn finish(self) -> Result<Psbt, CreateTxError> {
self.wallet
.borrow_mut()
.create_tx(self.coin_selection, self.params)
}
}

#[derive(Debug)]
/// Error returned from [`TxBuilder::add_utxo`] and [`TxBuilder::add_utxos`]
pub enum AddUtxoError {
/// Happens when trying to spend an UTXO that is not in the internal database
UnknownUtxo(OutPoint),
}

impl fmt::Display for AddUtxoError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::UnknownUtxo(outpoint) => write!(
f,
"UTXO not found in the internal database for txid: {} with vout: {}",
outpoint.txid, outpoint.vout
),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for AddUtxoError {}

#[derive(Debug)]
/// Error returned from [`TxBuilder::add_foreign_utxo`].
pub enum AddForeignUtxoError {
/// Foreign utxo outpoint txid does not match PSBT input txid
InvalidTxid {
/// PSBT input txid
input_txid: Txid,
/// Foreign UTXO outpoint
foreign_utxo: OutPoint,
},
/// Requested outpoint doesn't exist in the tx (vout greater than available outputs)
InvalidOutpoint(OutPoint),
/// Foreign utxo missing witness_utxo or non_witness_utxo
MissingUtxo,
}

impl fmt::Display for AddForeignUtxoError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::InvalidTxid {
input_txid,
foreign_utxo,
} => write!(
f,
"Foreign UTXO outpoint txid: {} does not match PSBT input txid: {}",
foreign_utxo.txid, input_txid,
),
Self::InvalidOutpoint(outpoint) => write!(
f,
"Requested outpoint doesn't exist for txid: {} with vout: {}",
outpoint.txid, outpoint.vout,
),
Self::MissingUtxo => write!(f, "Foreign utxo missing witness_utxo or non_witness_utxo"),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for AddForeignUtxoError {}

/// Ordering of the transaction's inputs and outputs
#[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
pub enum TxOrdering {
Expand Down
Loading