refactor(wallet)!: Use SignerError as error type for Wallet::sign() and Wallet::finalize_psbt()

refactor(wallet)!: Add SignerError::MiniscriptPsbt enum variant

Author: notmandatory

crates/bdk/src/wallet/error.rs

@@ -14,7 +14,7 @@
 use crate::descriptor::policy::PolicyError;
 use crate::descriptor::DescriptorError;
 use crate::wallet::coin_selection;
-use crate::{descriptor, wallet, FeeRate, KeychainKind};
+use crate::{descriptor, FeeRate, KeychainKind};
 use alloc::string::String;
 use bitcoin::{absolute, psbt, OutPoint, ScriptBuf, Sequence, Txid};
 use core::fmt;
@@ -291,29 +291,6 @@ impl fmt::Display for BuildFeeBumpError {
 #[cfg(feature = "std")]
 impl std::error::Error for BuildFeeBumpError {}
 
-#[derive(Debug)]
-/// Error returned from [`Wallet::sign`]
-///
-/// [`Wallet::sign`]: wallet::Wallet::sign
-pub enum SignError {
-    /// Signing error
-    Signer(wallet::signer::SignerError),
-    /// Miniscript PSBT error
-    MiniscriptPsbt(MiniscriptPsbtError),
-}
-
-impl fmt::Display for SignError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::Signer(err) => write!(f, "Signer error: {}", err),
-            Self::MiniscriptPsbt(err) => write!(f, "Miniscript PSBT error: {}", err),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for SignError {}
-
 #[derive(Debug)]
 /// Error returned from [`TxBuilder::add_utxo`] and [`TxBuilder::add_utxos`]
 ///
@@ -340,10 +317,9 @@ impl fmt::Display for AddUtxoError {
 impl std::error::Error for AddUtxoError {}
 
 #[derive(Debug)]
-/// Error returned from [`TxBuilder::add_utxo`] and [`TxBuilder::add_utxos`]
+/// Error returned from [`TxBuilder::add_foreign_utxo`].
 ///
-/// [`TxBuilder::add_utxo`]: wallet::tx_builder::TxBuilder::add_utxo
-/// [`TxBuilder::add_utxos`]: wallet::tx_builder::TxBuilder::add_utxos
+/// [`TxBuilder::add_foreign_utxo`]: wallet::tx_builder::TxBuilder::add_foreign_utxo
 pub enum AddForeignUtxoError {
     /// Foreign utxo outpoint txid does not match PSBT input txid
     InvalidTxid {

crates/bdk/src/wallet/mod.rs

@@ -74,7 +74,7 @@ use crate::psbt::PsbtUtils;
 use crate::signer::SignerError;
 use crate::types::*;
 use crate::wallet::coin_selection::Excess::{Change, NoChange};
-use crate::wallet::error::{BuildFeeBumpError, CreateTxError, MiniscriptPsbtError, SignError};
+use crate::wallet::error::{BuildFeeBumpError, CreateTxError, MiniscriptPsbtError};
 
 const COINBASE_MATURITY: u32 = 100;
 
@@ -1430,7 +1430,7 @@ impl<D> Wallet<D> {
     /// # use bitcoin::*;
     /// # use bdk::*;
     /// # use bdk::wallet::ChangeSet;
-    /// # use bdk::wallet::error::{CreateTxError, SignError};
+    /// # use bdk::wallet::error::CreateTxError;
     /// # use bdk_chain::PersistBackend;
     /// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
     /// # let mut wallet = doctest_wallet!();
@@ -1440,18 +1440,18 @@ impl<D> Wallet<D> {
     ///     builder.add_recipient(to_address.script_pubkey(), 50_000);
     ///     builder.finish()?
     /// };
-    /// let  finalized = wallet.sign(&mut psbt, SignOptions::default())?;
+    /// let finalized = wallet.sign(&mut psbt, SignOptions::default())?;
     /// assert!(finalized, "we should have signed all the inputs");
     /// # Ok::<(),anyhow::Error>(())
     pub fn sign(
         &self,
         psbt: &mut psbt::PartiallySignedTransaction,
         sign_options: SignOptions,
-    ) -> Result<bool, SignError> {
+    ) -> Result<bool, SignerError> {
         // This adds all the PSBT metadata for the inputs, which will help us later figure out how
         // to derive our keys
         self.update_psbt_with_descriptor(psbt)
-            .map_err(SignError::MiniscriptPsbt)?;
+            .map_err(SignerError::MiniscriptPsbt)?;
 
         // If we aren't allowed to use `witness_utxo`, ensure that every input (except p2tr and finalized ones)
         // has the `non_witness_utxo`
@@ -1463,7 +1463,7 @@ impl<D> Wallet<D> {
                 .filter(|i| i.tap_internal_key.is_none() && i.tap_merkle_root.is_none())
                 .any(|i| i.non_witness_utxo.is_none())
         {
-            return Err(SignError::Signer(SignerError::MissingNonWitnessUtxo));
+            return Err(SignerError::MissingNonWitnessUtxo);
         }
 
         // If the user hasn't explicitly opted-in, refuse to sign the transaction unless every input
@@ -1476,7 +1476,7 @@ impl<D> Wallet<D> {
                     || i.sighash_type == Some(TapSighashType::Default.into())
             })
         {
-            return Err(SignError::Signer(SignerError::NonStandardSighash));
+            return Err(SignerError::NonStandardSighash);
         }
 
         for signer in self
@@ -1485,9 +1485,7 @@ impl<D> Wallet<D> {
             .iter()
             .chain(self.change_signers.signers().iter())
         {
-            signer
-                .sign_transaction(psbt, &sign_options, &self.secp)
-                .map_err(SignError::Signer)?;
+            signer.sign_transaction(psbt, &sign_options, &self.secp)?;
         }
 
         // attempt to finalize
@@ -1531,7 +1529,7 @@ impl<D> Wallet<D> {
         &self,
         psbt: &mut psbt::PartiallySignedTransaction,
         sign_options: SignOptions,
-    ) -> Result<bool, SignError> {
+    ) -> Result<bool, SignerError> {
         let chain_tip = self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default();
 
         let tx = &psbt.unsigned_tx;
@@ -1541,7 +1539,7 @@ impl<D> Wallet<D> {
             let psbt_input = &psbt
                 .inputs
                 .get(n)
-                .ok_or(SignError::Signer(SignerError::InputIndexOutOfRange))?;
+                .ok_or(SignerError::InputIndexOutOfRange)?;
             if psbt_input.final_script_sig.is_some() || psbt_input.final_script_witness.is_some() {
                 continue;
             }

crates/bdk/src/wallet/signer.rs

@@ -103,6 +103,7 @@ use miniscript::{Legacy, Segwitv0, SigType, Tap, ToPublicKey};
 use super::utils::SecpCtx;
 use crate::descriptor::{DescriptorMeta, XKeyUtils};
 use crate::psbt::PsbtUtils;
+use crate::wallet::error::MiniscriptPsbtError;
 
 /// Identifier of a signer in the `SignersContainers`. Used as a key to find the right signer among
 /// multiple of them
@@ -159,6 +160,8 @@ pub enum SignerError {
     InvalidSighash,
     /// Error while computing the hash to sign
     SighashError(sighash::Error),
+    /// Miniscript PSBT error
+    MiniscriptPsbt(MiniscriptPsbtError),
     /// Error while signing using hardware wallets
     #[cfg(feature = "hardware-signer")]
     HWIError(hwi::error::Error),
@@ -192,6 +195,7 @@ impl fmt::Display for SignerError {
             Self::NonStandardSighash => write!(f, "The psbt contains a non standard sighash"),
             Self::InvalidSighash => write!(f, "Invalid SIGHASH for the signing context in use"),
             Self::SighashError(err) => write!(f, "Error while computing the hash to sign: {}", err),
+            Self::MiniscriptPsbt(err) => write!(f, "Miniscript PSBT error: {}", err),
             #[cfg(feature = "hardware-signer")]
             Self::HWIError(err) => write!(f, "Error while signing using hardware wallets: {}", err),
         }

crates/bdk/src/wallet/tx_builder.rs

@@ -551,7 +551,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
 
     /// Finish building the transaction.
     ///
-    /// Returns the [`BIP174`] "PSBT" and summary details about 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<D::WriteError>>

crates/bdk/tests/wallet.rs

@@ -3,7 +3,7 @@ use bdk::descriptor::calc_checksum;
 use bdk::psbt::PsbtUtils;
 use bdk::signer::{SignOptions, SignerError};
 use bdk::wallet::coin_selection::{self, LargestFirstCoinSelection};
-use bdk::wallet::error::{AddForeignUtxoError, CreateTxError, SignError};
+use bdk::wallet::error::{AddForeignUtxoError, CreateTxError};
 use bdk::wallet::AddressIndex::*;
 use bdk::wallet::{AddressIndex, AddressInfo, Balance, Wallet};
 use bdk::{FeeRate, KeychainKind};
@@ -2428,7 +2428,7 @@ fn test_sign_nonstandard_sighash() {
     );
     assert_matches!(
         result,
-        Err(SignError::Signer(SignerError::NonStandardSighash)),
+        Err(SignerError::NonStandardSighash),
         "Signing failed with the wrong error type"
     );
 
@@ -2845,7 +2845,7 @@ fn test_taproot_sign_missing_witness_utxo() {
     );
     assert_matches!(
         result,
-        Err(SignError::Signer(SignerError::MissingWitnessUtxo)),
+        Err(SignerError::MissingWitnessUtxo),
         "Signing should have failed with the correct error because the witness_utxo is missing"
     );
 
@@ -3186,7 +3186,7 @@ fn test_taproot_sign_non_default_sighash() {
     );
     assert_matches!(
         result,
-        Err(SignError::Signer(SignerError::NonStandardSighash)),
+        Err(SignerError::NonStandardSighash),
         "Signing failed with the wrong error type"
     );
 
@@ -3204,7 +3204,7 @@ fn test_taproot_sign_non_default_sighash() {
     );
     assert_matches!(
         result,
-        Err(SignError::Signer(SignerError::MissingWitnessUtxo)),
+        Err(SignerError::MissingWitnessUtxo),
         "Signing failed with the wrong error type"
     );
 

crates/chain/src/tx_graph.rs

@@ -57,6 +57,7 @@ use crate::{
 use alloc::collections::vec_deque::VecDeque;
 use alloc::vec::Vec;
 use bitcoin::{OutPoint, Script, Transaction, TxOut, Txid};
+use core::fmt::{self, Formatter};
 use core::{
     convert::Infallible,
     ops::{Deref, RangeInclusive},
@@ -145,6 +146,26 @@ pub enum CalculateFeeError {
     NegativeFee(i64),
 }
 
+impl fmt::Display for CalculateFeeError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        match self {
+            CalculateFeeError::MissingTxOut(outpoints) => write!(
+                f,
+                "missing `TxOut` for one or more of the inputs of the tx: {:?}",
+                outpoints
+            ),
+            CalculateFeeError::NegativeFee(fee) => write!(
+                f,
+                "transaction is invalid according to the graph and has negative fee: {}",
+                fee
+            ),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for CalculateFeeError {}
+
 impl<A> TxGraph<A> {
     /// Iterate over all tx outputs known by [`TxGraph`].
     ///