Skip to content

Commit

Permalink
Add CreateTxError and use as error type for TxBuilder::finish()
Browse files Browse the repository at this point in the history
  • Loading branch information
@notmandatory
notmandatory committed Jul 6, 2023
1 parent 81c7613 commit d99e4a5
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 36 deletions.
5 changes: 3 additions & 2 deletions crates/bdk/src/wallet/coin_selection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
//! ```
//! # use std::str::FromStr;
//! # use bitcoin::*;
//! # use bdk::wallet::{self, coin_selection::*};
//! # use bdk::wallet::{self, ChangeSet, coin_selection::*, CreateTxError};
//! # use bdk_chain::PersistBackend;
//! # use bdk::*;
//! # use bdk::wallet::coin_selection::decide_change;
//! # const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4;
Expand Down Expand Up @@ -89,7 +90,7 @@
//!
//! // inspect, sign, broadcast, ...
//!
//! # Ok::<(), bdk::Error>(())
//! # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
//! ```

use crate::types::FeeRate;
Expand Down
117 changes: 93 additions & 24 deletions crates/bdk/src/wallet/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,17 @@ pub mod hardwaresigner;

pub use utils::IsDust;

use crate::descriptor;
#[allow(deprecated)]
use coin_selection::DefaultCoinSelectionAlgorithm;
use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
use utils::{check_nsequence_rbf, After, Older, SecpCtx};

use crate::descriptor::policy::BuildSatisfaction;
use crate::descriptor::policy::{BuildSatisfaction, PolicyError};
use crate::descriptor::{
calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta,
ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorError,
DescriptorMeta, ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
};
use crate::error::{Error, MiniscriptPsbtError};
use crate::psbt::PsbtUtils;
Expand Down Expand Up @@ -166,7 +167,7 @@ impl Wallet {
pub enum NewError<P> {
/// There was problem with the descriptors passed in
Descriptor(crate::descriptor::DescriptorError),
/// We were unable to load the wallet's data from the persistance backend
/// We were unable to load the wallet's data from the persistence backend
Persist(P),
}

Expand All @@ -178,7 +179,7 @@ where
match self {
NewError::Descriptor(e) => e.fmt(f),
NewError::Persist(e) => {
write!(f, "failed to load wallet from persistance backend: {}", e)
write!(f, "failed to load wallet from persistence backend: {}", e)
}
}
}
Expand All @@ -200,6 +201,61 @@ pub enum InsertTxError {
#[cfg(feature = "std")]
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for NewError<P> {}

#[derive(Debug)]
/// Error returned from [`TxBuilder::finish`]
pub enum CreateTxError<P> {
/// There was a problem with the descriptors passed in
Descriptor(DescriptorError),
/// We were unable to write wallet data to the persistence backend
Persist(P),
/// There was a problem while extracting and manipulating policies
Policy(PolicyError),
/// TODO: replace this with specific error types
Bdk(Error),
}

#[cfg(feature = "std")]
impl<P> fmt::Display for CreateTxError<P>
where
P: fmt::Display,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Descriptor(e) => e.fmt(f),
Self::Persist(e) => {
write!(
f,
"failed to write wallet data to persistence backend: {}",
e
)
}
Self::Bdk(e) => e.fmt(f),
Self::Policy(e) => e.fmt(f),
}
}
}

impl<P> From<descriptor::error::Error> for CreateTxError<P> {
fn from(err: descriptor::error::Error) -> Self {
CreateTxError::Descriptor(err)
}
}

impl<P> From<PolicyError> for CreateTxError<P> {
fn from(err: PolicyError) -> Self {
CreateTxError::Policy(err)
}
}

impl<P> From<Error> for CreateTxError<P> {
fn from(err: Error) -> Self {
CreateTxError::Bdk(err)
}
}

#[cfg(feature = "std")]
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for CreateTxError<P> {}

impl<D> Wallet<D> {
/// Create a wallet from a `descriptor` (and an optional `change_descriptor`) and load related
/// transaction data from `db`.
Expand Down Expand Up @@ -596,6 +652,8 @@ impl<D> Wallet<D> {
/// # use std::str::FromStr;
/// # use bitcoin::*;
/// # use bdk::*;
/// # use bdk::wallet::{ChangeSet,CreateTxError};
/// # use bdk_chain::PersistBackend;
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
/// # let mut wallet = doctest_wallet!();
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
Expand All @@ -607,7 +665,7 @@ impl<D> Wallet<D> {
/// };
///
/// // sign and broadcast ...
/// # Ok::<(), bdk::Error>(())
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
/// ```
///
/// [`TxBuilder`]: crate::TxBuilder
Expand All @@ -624,7 +682,7 @@ impl<D> Wallet<D> {
&mut self,
coin_selection: Cs,
params: TxParams,
) -> Result<(psbt::PartiallySignedTransaction, TransactionDetails), Error>
) -> Result<(psbt::PartiallySignedTransaction, TransactionDetails), CreateTxError<D::WriteError>>
where
D: PersistBackend<ChangeSet>,
{
Expand Down Expand Up @@ -659,15 +717,15 @@ impl<D> Wallet<D> {
&& external_policy.requires_path()
&& params.external_policy_path.is_none()
{
return Err(Error::SpendingPolicyRequired(KeychainKind::External));
return Err(Error::SpendingPolicyRequired(KeychainKind::External).into());
};
// Same for the internal_policy path, if present
if let Some(internal_policy) = &internal_policy {
if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeForbidden
&& internal_policy.requires_path()
&& params.internal_policy_path.is_none()
{
return Err(Error::SpendingPolicyRequired(KeychainKind::Internal));
return Err(Error::SpendingPolicyRequired(KeychainKind::Internal).into());
};
}

Expand Down Expand Up @@ -696,13 +754,14 @@ impl<D> Wallet<D> {

let version = match params.version {
Some(tx_builder::Version(0)) => {
return Err(Error::Generic("Invalid version `0`".into()))
return Err(Error::Generic("Invalid version `0`".into()).into())
}
Some(tx_builder::Version(1)) if requirements.csv.is_some() => {
return Err(Error::Generic(
"TxBuilder requested version `1`, but at least `2` is needed to use OP_CSV"
.into(),
))
)
.into())
}
Some(tx_builder::Version(x)) => x,
None if requirements.csv.is_some() => 2,
Expand Down Expand Up @@ -745,7 +804,7 @@ impl<D> Wallet<D> {
// Specific nLockTime required and it's compatible with the constraints
Some(x) if requirements.timelock.unwrap().is_same_unit(x) && x >= requirements.timelock.unwrap() => x,
// Invalid nLockTime required
Some(x) => return Err(Error::Generic(format!("TxBuilder requested timelock of `{:?}`, but at least `{:?}` is required to spend from this script", x, requirements.timelock.unwrap())))
Some(x) => return Err(Error::Generic(format!("TxBuilder requested timelock of `{:?}`, but at least `{:?}` is required to spend from this script", x, requirements.timelock.unwrap())).into())
};

let n_sequence = match (params.rbf, requirements.csv) {
Expand All @@ -763,7 +822,8 @@ impl<D> Wallet<D> {
(Some(tx_builder::RbfValue::Value(rbf)), _) if !rbf.is_rbf() => {
return Err(Error::Generic(
"Cannot enable RBF with a nSequence >= 0xFFFFFFFE".into(),
))
)
.into())
}
// RBF with a specific value requested, but the value is incompatible with CSV
(Some(tx_builder::RbfValue::Value(rbf)), Some(csv))
Expand All @@ -772,7 +832,8 @@ impl<D> Wallet<D> {
return Err(Error::Generic(format!(
"Cannot enable RBF with nSequence `{:?}` given a required OP_CSV of `{:?}`",
rbf, csv
)))
))
.into())
}

// RBF enabled with the default value with CSV also enabled. CSV takes precedence
Expand All @@ -793,7 +854,8 @@ impl<D> Wallet<D> {
if *fee < previous_fee.absolute {
return Err(Error::FeeTooLow {
required: previous_fee.absolute,
});
}
.into());
}
}
(FeeRate::from_sat_per_vb(0.0), *fee)
Expand All @@ -804,7 +866,8 @@ impl<D> Wallet<D> {
if *rate < required_feerate {
return Err(Error::FeeRateTooLow {
required: required_feerate,
});
}
.into());
}
}
(*rate, 0)
Expand All @@ -819,7 +882,7 @@ impl<D> Wallet<D> {
};

if params.manually_selected_only && params.utxos.is_empty() {
return Err(Error::NoUtxosSelected);
return Err(Error::NoUtxosSelected.into());
}

// we keep it as a float while we accumulate it, and only round it at the end
Expand All @@ -833,7 +896,7 @@ impl<D> Wallet<D> {
&& value.is_dust(script_pubkey)
&& !script_pubkey.is_provably_unspendable()
{
return Err(Error::OutputBelowDustLimit(index));
return Err(Error::OutputBelowDustLimit(index).into());
}

if self.is_mine(script_pubkey) {
Expand Down Expand Up @@ -868,7 +931,8 @@ impl<D> Wallet<D> {
{
return Err(Error::Generic(
"The `change_policy` can be set only if the wallet has a change_descriptor".into(),
));
)
.into());
}

let (required_utxos, optional_utxos) = self.preselect_utxos(
Expand All @@ -892,7 +956,7 @@ impl<D> Wallet<D> {
self.indexed_graph.index.mark_used(&change_keychain, index);
self.persist
.stage(ChangeSet::from(IndexedAdditions::from(index_additions)));
self.persist.commit().expect("TODO");
self.persist.commit().map_err(CreateTxError::Persist)?;
spk
}
};
Expand Down Expand Up @@ -936,10 +1000,11 @@ impl<D> Wallet<D> {
return Err(Error::InsufficientFunds {
needed: *dust_threshold,
available: remaining_amount.saturating_sub(*change_fee),
});
}
.into());
}
} else {
return Err(Error::NoRecipients);
return Err(Error::NoRecipients.into());
}
}

Expand Down Expand Up @@ -998,6 +1063,8 @@ impl<D> Wallet<D> {
/// # use std::str::FromStr;
/// # use bitcoin::*;
/// # use bdk::*;
/// # use bdk::wallet::{ChangeSet, CreateTxError};
/// # use bdk_chain::PersistBackend;
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
/// # let mut wallet = doctest_wallet!();
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
Expand All @@ -1021,7 +1088,7 @@ impl<D> Wallet<D> {
/// let _ = wallet.sign(&mut psbt, SignOptions::default())?;
/// let fee_bumped_tx = psbt.extract_tx();
/// // broadcast fee_bumped_tx to replace original
/// # Ok::<(), bdk::Error>(())
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
/// ```
// TODO: support for merging multiple transactions while bumping the fees
pub fn build_fee_bump(
Expand Down Expand Up @@ -1168,6 +1235,8 @@ impl<D> Wallet<D> {
/// # use std::str::FromStr;
/// # use bitcoin::*;
/// # use bdk::*;
/// # use bdk::wallet::{ChangeSet, CreateTxError};
/// # use bdk_chain::PersistBackend;
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
/// # let mut wallet = doctest_wallet!();
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
Expand All @@ -1178,7 +1247,7 @@ impl<D> Wallet<D> {
/// };
/// let finalized = wallet.sign(&mut psbt, SignOptions::default())?;
/// assert!(finalized, "we should have signed all the inputs");
/// # Ok::<(), bdk::Error>(())
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
pub fn sign(
&self,
psbt: &mut psbt::PartiallySignedTransaction,
Expand Down
15 changes: 11 additions & 4 deletions crates/bdk/src/wallet/tx_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
//! # use std::str::FromStr;
//! # use bitcoin::*;
//! # use bdk::*;
//! # use bdk::wallet::{ChangeSet, CreateTxError};
//! # use bdk::wallet::tx_builder::CreateTx;
//! # use bdk_chain::PersistBackend;
//! # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
//! # let mut wallet = doctest_wallet!();
//! // create a TxBuilder from a wallet
Expand All @@ -33,7 +35,7 @@
//! // Turn on RBF signaling
//! .enable_rbf();
//! let (psbt, tx_details) = tx_builder.finish()?;
//! # Ok::<(), bdk::Error>(())
//! # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
//! ```

use crate::collections::BTreeMap;
Expand All @@ -48,6 +50,7 @@ use bitcoin::{LockTime, OutPoint, Script, Sequence, Transaction};

use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
use super::ChangeSet;
use crate::wallet::CreateTxError;
use crate::{
types::{FeeRate, KeychainKind, LocalUtxo, WeightedUtxo},
TransactionDetails,
Expand Down Expand Up @@ -81,6 +84,8 @@ impl TxBuilderContext for BumpFee {}
/// # use bdk::wallet::tx_builder::*;
/// # use bitcoin::*;
/// # use core::str::FromStr;
/// # use bdk::wallet::{ChangeSet, CreateTxError};
/// # use bdk_chain::PersistBackend;
/// # let mut wallet = doctest_wallet!();
/// # let addr1 = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
/// # let addr2 = addr1.clone();
Expand All @@ -105,7 +110,7 @@ impl TxBuilderContext for BumpFee {}
/// };
///
/// assert_eq!(psbt1.unsigned_tx.output[..2], psbt2.unsigned_tx.output[..2]);
/// # Ok::<(), bdk::Error>(())
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
/// ```
///
/// At the moment [`coin_selection`] is an exception to the rule as it consumes `self`.
Expand Down Expand Up @@ -527,7 +532,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
/// Returns the [`BIP174`] "PSBT" and summary details about the transaction.
///
/// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
pub fn finish(self) -> Result<(Psbt, TransactionDetails), Error>
pub fn finish(self) -> Result<(Psbt, TransactionDetails), CreateTxError<D::WriteError>>
where
D: PersistBackend<ChangeSet>,
{
Expand Down Expand Up @@ -625,7 +630,9 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
/// # use std::str::FromStr;
/// # use bitcoin::*;
/// # use bdk::*;
/// # use bdk::wallet::{ChangeSet, CreateTxError};
/// # use bdk::wallet::tx_builder::CreateTx;
/// # use bdk_chain::PersistBackend;
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
/// # let mut wallet = doctest_wallet!();
/// let mut tx_builder = wallet.build_tx();
Expand All @@ -638,7 +645,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
/// .fee_rate(FeeRate::from_sat_per_vb(5.0))
/// .enable_rbf();
/// let (psbt, tx_details) = tx_builder.finish()?;
/// # Ok::<(), bdk::Error>(())
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
/// ```
///
/// [`allow_shrinking`]: Self::allow_shrinking
Expand Down

0 comments on commit d99e4a5

Please sign in to comment.