refactor(wallet)!: Add SignError and use as error type for Wallet::si…

…gn() and Wallet::finalize_psbt()
notmandatory · Oct 19, 2023 · 9dd9cd0 · 9dd9cd0
1 parent 17118a5
commit 9dd9cd0
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 34 deletions.
diff --git a/crates/bdk/src/error.rs b/crates/bdk/src/error.rs
@@ -32,20 +32,14 @@ pub enum Error {
     Key(crate::keys::KeyError),
     /// Descriptor checksum mismatch
     ChecksumMismatch,
-    /// Signing error
-    Signer(crate::wallet::signer::SignerError),
     /// Requested outpoint doesn't exist in the tx (vout greater than available outputs)
     InvalidOutpoint(OutPoint),
     /// Error related to the parsing and usage of descriptors
     Descriptor(crate::descriptor::error::Error),
     /// Miniscript error
     Miniscript(miniscript::Error),
-    /// Miniscript PSBT error
-    MiniscriptPsbt(MiniscriptPsbtError),
     /// BIP32 error
     Bip32(bitcoin::bip32::Error),
-    /// Partially signed bitcoin transaction error
-    Psbt(bitcoin::psbt::Error),
 }
 
 /// Errors returned by miniscript when updating inconsistent PSBTs
@@ -81,17 +75,14 @@ impl fmt::Display for Error {
             Self::FeeRateUnavailable => write!(f, "Fee rate unavailable"),
             Self::Key(err) => write!(f, "Key error: {}", err),
             Self::ChecksumMismatch => write!(f, "Descriptor checksum mismatch"),
-            Self::Signer(err) => write!(f, "Signer error: {}", err),
             Self::InvalidOutpoint(outpoint) => write!(
                 f,
                 "Requested outpoint doesn't exist in the tx: {}",
                 outpoint
             ),
             Self::Descriptor(err) => write!(f, "Descriptor error: {}", err),
             Self::Miniscript(err) => write!(f, "Miniscript error: {}", err),
-            Self::MiniscriptPsbt(err) => write!(f, "Miniscript PSBT error: {}", err),
             Self::Bip32(err) => write!(f, "BIP32 error: {}", err),
-            Self::Psbt(err) => write!(f, "PSBT error: {}", err),
         }
     }
 }
@@ -113,7 +104,6 @@ macro_rules! impl_error {
 }
 
 impl_error!(descriptor::error::Error, Descriptor);
-impl_error!(wallet::signer::SignerError, Signer);
 
 impl From<crate::keys::KeyError> for Error {
     fn from(key_error: crate::keys::KeyError) -> Error {
@@ -127,9 +117,7 @@ impl From<crate::keys::KeyError> for Error {
 }
 
 impl_error!(miniscript::Error, Miniscript);
-impl_error!(MiniscriptPsbtError, MiniscriptPsbt);
 impl_error!(bitcoin::bip32::Error, Bip32);
-impl_error!(bitcoin::psbt::Error, Psbt);
 
 #[derive(Debug)]
 /// Error returned from [`TxBuilder::finish`]
@@ -394,3 +382,27 @@ 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),
+}
+
+#[cfg(feature = "std")]
+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 {}
diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs
@@ -67,7 +67,7 @@ use crate::descriptor::{
     calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta,
     ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
 };
-use crate::error::{BuildFeeBumpError, CreateTxError, Error, MiniscriptPsbtError};
+use crate::error::{BuildFeeBumpError, CreateTxError, Error, MiniscriptPsbtError, SignError};
 use crate::psbt::PsbtUtils;
 use crate::signer::SignerError;
 use crate::types::*;
@@ -1430,27 +1430,28 @@ impl<D> Wallet<D> {
     /// # use bitcoin::*;
     /// # use bdk::*;
     /// # use bdk::wallet::ChangeSet;
-    /// # use bdk::error::CreateTxError;
+    /// # use bdk::error::{CreateTxError, SignError};
     /// # use bdk_chain::PersistBackend;
     /// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
     /// # let mut wallet = doctest_wallet!();
     /// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
     /// let mut psbt = {
     ///     let mut builder = wallet.build_tx();
     ///     builder.add_recipient(to_address.script_pubkey(), 50_000);
-    ///     builder.finish().expect("psbt")
+    ///     builder.finish()?
     /// };
     /// let  finalized = wallet.sign(&mut psbt, SignOptions::default())?;
     /// assert!(finalized, "we should have signed all the inputs");
-    /// # Ok::<(), bdk::Error>(())
+    /// # Ok::<(),anyhow::Error>(())
     pub fn sign(
         &self,
         psbt: &mut psbt::PartiallySignedTransaction,
         sign_options: SignOptions,
-    ) -> Result<bool, Error> {
+    ) -> Result<bool, SignError> {
         // 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)?;
+        self.update_psbt_with_descriptor(psbt)
+            .map_err(SignError::MiniscriptPsbt)?;
 
         // If we aren't allowed to use `witness_utxo`, ensure that every input (except p2tr and finalized ones)
         // has the `non_witness_utxo`
@@ -1462,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(Error::Signer(signer::SignerError::MissingNonWitnessUtxo));
+            return Err(SignError::Signer(SignerError::MissingNonWitnessUtxo));
         }
 
         // If the user hasn't explicitly opted-in, refuse to sign the transaction unless every input
@@ -1475,7 +1476,7 @@ impl<D> Wallet<D> {
                     || i.sighash_type == Some(TapSighashType::Default.into())
             })
         {
-            return Err(Error::Signer(signer::SignerError::NonStandardSighash));
+            return Err(SignError::Signer(SignerError::NonStandardSighash));
         }
 
         for signer in self
@@ -1484,7 +1485,9 @@ impl<D> Wallet<D> {
             .iter()
             .chain(self.change_signers.signers().iter())
         {
-            signer.sign_transaction(psbt, &sign_options, &self.secp)?;
+            signer
+                .sign_transaction(psbt, &sign_options, &self.secp)
+                .map_err(SignError::Signer)?;
         }
 
         // attempt to finalize
@@ -1528,7 +1531,7 @@ impl<D> Wallet<D> {
         &self,
         psbt: &mut psbt::PartiallySignedTransaction,
         sign_options: SignOptions,
-    ) -> Result<bool, Error> {
+    ) -> Result<bool, SignError> {
         let chain_tip = self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default();
 
         let tx = &psbt.unsigned_tx;
@@ -1538,7 +1541,7 @@ impl<D> Wallet<D> {
             let psbt_input = &psbt
                 .inputs
                 .get(n)
-                .ok_or(Error::Signer(SignerError::InputIndexOutOfRange))?;
+                .ok_or(SignError::Signer(SignerError::InputIndexOutOfRange))?;
             if psbt_input.final_script_sig.is_some() || psbt_input.final_script_witness.is_some() {
                 continue;
             }

diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs
@@ -1,6 +1,6 @@
 use assert_matches::assert_matches;
 use bdk::descriptor::calc_checksum;
-use bdk::error::CreateTxError;
+use bdk::error::{CreateTxError, SignError};
 use bdk::psbt::PsbtUtils;
 use bdk::signer::{SignOptions, SignerError};
 use bdk::wallet::coin_selection::{self, LargestFirstCoinSelection};
@@ -2432,7 +2432,7 @@ fn test_sign_nonstandard_sighash() {
     );
     assert_matches!(
         result,
-        Err(bdk::Error::Signer(SignerError::NonStandardSighash)),
+        Err(SignError::Signer(SignerError::NonStandardSighash)),
         "Signing failed with the wrong error type"
     );
 
@@ -2849,7 +2849,7 @@ fn test_taproot_sign_missing_witness_utxo() {
     );
     assert_matches!(
         result,
-        Err(bdk::Error::Signer(SignerError::MissingWitnessUtxo)),
+        Err(SignError::Signer(SignerError::MissingWitnessUtxo)),
         "Signing should have failed with the correct error because the witness_utxo is missing"
     );
 
@@ -3190,7 +3190,7 @@ fn test_taproot_sign_non_default_sighash() {
     );
     assert_matches!(
         result,
-        Err(bdk::Error::Signer(SignerError::NonStandardSighash)),
+        Err(SignError::Signer(SignerError::NonStandardSighash)),
         "Signing failed with the wrong error type"
     );
 
@@ -3208,7 +3208,7 @@ fn test_taproot_sign_non_default_sighash() {
     );
     assert_matches!(
         result,
-        Err(bdk::Error::Signer(SignerError::MissingWitnessUtxo)),
+        Err(SignError::Signer(SignerError::MissingWitnessUtxo)),
         "Signing failed with the wrong error type"
     );
 

diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs
@@ -27,9 +27,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         Network::Testnet,
     )?;
 
-    let address = wallet
-        .try_get_address(AddressIndex::New)
-        .expect("new address");
+    let address = wallet.try_get_address(AddressIndex::New)?;
     println!("Generated Address: {}", address);
 
     let balance = wallet.get_balance();

diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs
@@ -26,9 +26,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         Network::Testnet,
     )?;
 
-    let address = wallet
-        .try_get_address(AddressIndex::New)
-        .expect("new address");
+    let address = wallet.try_get_address(AddressIndex::New)?;
     println!("Generated Address: {}", address);
 
     let balance = wallet.get_balance();