From d99e4a5955f457bb4bb8606608dd64821b1902b4 Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Wed, 5 Jul 2023 21:39:06 -0500 Subject: [PATCH] Add CreateTxError and use as error type for TxBuilder::finish() --- crates/bdk/src/wallet/coin_selection.rs | 5 +- crates/bdk/src/wallet/mod.rs | 117 +++++++++++++++++++----- crates/bdk/src/wallet/tx_builder.rs | 15 ++- crates/bdk/tests/wallet.rs | 15 +-- 4 files changed, 116 insertions(+), 36 deletions(-) diff --git a/crates/bdk/src/wallet/coin_selection.rs b/crates/bdk/src/wallet/coin_selection.rs index e7927cabb3..83e00d72f1 100644 --- a/crates/bdk/src/wallet/coin_selection.rs +++ b/crates/bdk/src/wallet/coin_selection.rs @@ -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; @@ -89,7 +90,7 @@ //! //! // inspect, sign, broadcast, ... //! -//! # Ok::<(), bdk::Error>(()) +//! # Ok::<(), CreateTxError<<() as PersistBackend>::WriteError>>(()) //! ``` use crate::types::FeeRate; diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index f2f717d9fe..0b6d2a5205 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -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; @@ -166,7 +167,7 @@ impl Wallet { pub enum NewError

{ /// 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), } @@ -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) } } } @@ -200,6 +201,61 @@ pub enum InsertTxError { #[cfg(feature = "std")] impl std::error::Error for NewError

{} +#[derive(Debug)] +/// Error returned from [`TxBuilder::finish`] +pub enum CreateTxError

{ + /// 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

fmt::Display for CreateTxError

+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

From for CreateTxError

{ + fn from(err: descriptor::error::Error) -> Self { + CreateTxError::Descriptor(err) + } +} + +impl

From for CreateTxError

{ + fn from(err: PolicyError) -> Self { + CreateTxError::Policy(err) + } +} + +impl

From for CreateTxError

{ + fn from(err: Error) -> Self { + CreateTxError::Bdk(err) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for CreateTxError

{} + impl Wallet { /// Create a wallet from a `descriptor` (and an optional `change_descriptor`) and load related /// transaction data from `db`. @@ -596,6 +652,8 @@ impl Wallet { /// # 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(); @@ -607,7 +665,7 @@ impl Wallet { /// }; /// /// // sign and broadcast ... - /// # Ok::<(), bdk::Error>(()) + /// # Ok::<(), CreateTxError<<() as PersistBackend>::WriteError>>(()) /// ``` /// /// [`TxBuilder`]: crate::TxBuilder @@ -624,7 +682,7 @@ impl Wallet { &mut self, coin_selection: Cs, params: TxParams, - ) -> Result<(psbt::PartiallySignedTransaction, TransactionDetails), Error> + ) -> Result<(psbt::PartiallySignedTransaction, TransactionDetails), CreateTxError> where D: PersistBackend, { @@ -659,7 +717,7 @@ impl Wallet { && 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 { @@ -667,7 +725,7 @@ impl Wallet { && internal_policy.requires_path() && params.internal_policy_path.is_none() { - return Err(Error::SpendingPolicyRequired(KeychainKind::Internal)); + return Err(Error::SpendingPolicyRequired(KeychainKind::Internal).into()); }; } @@ -696,13 +754,14 @@ impl Wallet { 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, @@ -745,7 +804,7 @@ impl Wallet { // 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) { @@ -763,7 +822,8 @@ impl Wallet { (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)) @@ -772,7 +832,8 @@ impl Wallet { 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 @@ -793,7 +854,8 @@ impl Wallet { if *fee < previous_fee.absolute { return Err(Error::FeeTooLow { required: previous_fee.absolute, - }); + } + .into()); } } (FeeRate::from_sat_per_vb(0.0), *fee) @@ -804,7 +866,8 @@ impl Wallet { if *rate < required_feerate { return Err(Error::FeeRateTooLow { required: required_feerate, - }); + } + .into()); } } (*rate, 0) @@ -819,7 +882,7 @@ impl Wallet { }; 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 @@ -833,7 +896,7 @@ impl Wallet { && 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) { @@ -868,7 +931,8 @@ impl Wallet { { 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( @@ -892,7 +956,7 @@ impl Wallet { 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 } }; @@ -936,10 +1000,11 @@ impl Wallet { 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()); } } @@ -998,6 +1063,8 @@ impl Wallet { /// # 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(); @@ -1021,7 +1088,7 @@ impl Wallet { /// 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>::WriteError>>(()) /// ``` // TODO: support for merging multiple transactions while bumping the fees pub fn build_fee_bump( @@ -1168,6 +1235,8 @@ impl Wallet { /// # 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(); @@ -1178,7 +1247,7 @@ impl Wallet { /// }; /// let finalized = wallet.sign(&mut psbt, SignOptions::default())?; /// assert!(finalized, "we should have signed all the inputs"); - /// # Ok::<(), bdk::Error>(()) + /// # Ok::<(), CreateTxError<<() as PersistBackend>::WriteError>>(()) pub fn sign( &self, psbt: &mut psbt::PartiallySignedTransaction, diff --git a/crates/bdk/src/wallet/tx_builder.rs b/crates/bdk/src/wallet/tx_builder.rs index 165f01f25f..c7db7cbd8b 100644 --- a/crates/bdk/src/wallet/tx_builder.rs +++ b/crates/bdk/src/wallet/tx_builder.rs @@ -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 @@ -33,7 +35,7 @@ //! // Turn on RBF signaling //! .enable_rbf(); //! let (psbt, tx_details) = tx_builder.finish()?; -//! # Ok::<(), bdk::Error>(()) +//! # Ok::<(), CreateTxError<<() as PersistBackend>::WriteError>>(()) //! ``` use crate::collections::BTreeMap; @@ -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, @@ -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(); @@ -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>::WriteError>>(()) /// ``` /// /// At the moment [`coin_selection`] is an exception to the rule as it consumes `self`. @@ -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> where D: PersistBackend, { @@ -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(); @@ -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>::WriteError>>(()) /// ``` /// /// [`allow_shrinking`]: Self::allow_shrinking diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs index 282a74fcb9..cdfab78a47 100644 --- a/crates/bdk/tests/wallet.rs +++ b/crates/bdk/tests/wallet.rs @@ -3,7 +3,7 @@ use bdk::descriptor::calc_checksum; use bdk::signer::{SignOptions, SignerError}; use bdk::wallet::coin_selection::LargestFirstCoinSelection; use bdk::wallet::AddressIndex::*; -use bdk::wallet::{AddressIndex, AddressInfo, Balance, Wallet}; +use bdk::wallet::{AddressIndex, AddressInfo, Balance, CreateTxError, Wallet}; use bdk::Error; use bdk::FeeRate; use bdk::KeychainKind; @@ -3096,10 +3096,10 @@ fn test_spend_coinbase() { .current_height(confirmation_height); assert!(matches!( builder.finish(), - Err(Error::InsufficientFunds { + Err(CreateTxError::Bdk(Error::InsufficientFunds { needed: _, available: 0 - }) + })) )); // Still unspendable... @@ -3109,10 +3109,10 @@ fn test_spend_coinbase() { .current_height(not_yet_mature_time); assert_matches!( builder.finish(), - Err(Error::InsufficientFunds { + Err(CreateTxError::Bdk(Error::InsufficientFunds { needed: _, available: 0 - }) + })) ); wallet @@ -3148,7 +3148,10 @@ fn test_allow_dust_limit() { builder.add_recipient(addr.script_pubkey(), 0); - assert_matches!(builder.finish(), Err(Error::OutputBelowDustLimit(0))); + assert_matches!( + builder.finish(), + Err(CreateTxError::Bdk(Error::OutputBelowDustLimit(0))) + ); let mut builder = wallet.build_tx();