From 9dd9cd0b1910ba3c070240c2c8b0dba097a070fa Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Mon, 16 Oct 2023 18:53:19 -0500 Subject: [PATCH] refactor(wallet)!: Add SignError and use as error type for Wallet::sign() and Wallet::finalize_psbt() --- crates/bdk/src/error.rs | 36 ++++++++++++------- crates/bdk/src/wallet/mod.rs | 25 +++++++------ crates/bdk/tests/wallet.rs | 10 +++--- .../wallet_esplora_async/src/main.rs | 4 +-- .../wallet_esplora_blocking/src/main.rs | 4 +-- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/crates/bdk/src/error.rs b/crates/bdk/src/error.rs index d73ceb7228..47b47b7644 100644 --- 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,7 +75,6 @@ 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: {}", @@ -89,9 +82,7 @@ impl fmt::Display for Error { ), 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 for Error { fn from(key_error: crate::keys::KeyError) -> Error { @@ -127,9 +117,7 @@ impl From 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 index fb207a8799..3c12152342 100644 --- 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,7 +1430,7 @@ impl Wallet { /// # 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!(); @@ -1438,19 +1438,20 @@ impl Wallet { /// 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 { + ) -> Result { // 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 Wallet { .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 Wallet { || 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 Wallet { .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 Wallet { &self, psbt: &mut psbt::PartiallySignedTransaction, sign_options: SignOptions, - ) -> Result { + ) -> Result { let chain_tip = self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(); let tx = &psbt.unsigned_tx; @@ -1538,7 +1541,7 @@ impl Wallet { 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 index 5aa44a55c9..7e56c1f21a 100644 --- 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 index 99e7f02831..a3bcfc9aaf 100644 --- 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> { 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 index 717f04fafb..6b8935c4cd 100644 --- 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> { 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();