Merge #814: Improve display error formatting

9019793 Improve display error formatting (Yuki Kishimoto)

Pull request description:

  ### Description

  Closes #555

  ### Checklists

  #### All Submissions:

  * [X] I've signed all my commits
  * [X] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [X] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [X] I'm linking the issue being fixed by this PR

ACKs for top commit:
  notmandatory:
    ACK 9019793

Tree-SHA512: 44f0c7e0374e255bf99cbbfb01f2af76746e52afe67ace1595d2830bd60be99f3c8726e6ad9744878978257796a79b3933e7ef21c30a5e4b99a16aaf9ac7ff64

src/blockchain/compact_filters/mod.rs

@@ -580,7 +580,27 @@ pub enum CompactFiltersError {
 
 impl fmt::Display for CompactFiltersError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::InvalidResponse => write!(f, "A peer sent an invalid or unexpected response"),
+            Self::InvalidHeaders => write!(f, "Invalid headers"),
+            Self::InvalidFilterHeader => write!(f, "Invalid filter header"),
+            Self::InvalidFilter => write!(f, "Invalid filters"),
+            Self::MissingBlock => write!(f, "The peer is missing a block in the valid chain"),
+            Self::BlockHashNotFound => write!(f, "Block hash not found"),
+            Self::DataCorruption => write!(
+                f,
+                "The data stored in the block filters storage are corrupted"
+            ),
+            Self::NotConnected => write!(f, "A peer is not connected"),
+            Self::Timeout => write!(f, "A peer took too long to reply to one of our messages"),
+            Self::PeerBloomDisabled => write!(f, "Peer doesn't advertise the BLOOM service flag"),
+            Self::NoPeers => write!(f, "No peers have been specified"),
+            Self::Db(err) => write!(f, "Internal database error: {}", err),
+            Self::Io(err) => write!(f, "Internal I/O error: {}", err),
+            Self::Bip158(err) => write!(f, "Invalid BIP158 filter: {}", err),
+            Self::Time(err) => write!(f, "Invalid system time: {}", err),
+            Self::Global(err) => write!(f, "Generic error: {}", err),
+        }
     }
 }
 

src/descriptor/error.rs

@@ -53,7 +53,26 @@ impl From<crate::keys::KeyError> for Error {
 
 impl std::fmt::Display for Error {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::InvalidHdKeyPath => write!(f, "Invalid HD key path"),
+            Self::InvalidDescriptorChecksum => {
+                write!(f, "The provided descriptor doesn't match its checksum")
+            }
+            Self::HardenedDerivationXpub => write!(
+                f,
+                "The descriptor contains hardened derivation steps on public extended keys"
+            ),
+            Self::Key(err) => write!(f, "Key error: {}", err),
+            Self::Policy(err) => write!(f, "Policy error: {}", err),
+            Self::InvalidDescriptorCharacter(char) => {
+                write!(f, "Invalid descriptor character: {}", char)
+            }
+            Self::Bip32(err) => write!(f, "BIP32 error: {}", err),
+            Self::Base58(err) => write!(f, "Base58 error: {}", err),
+            Self::Pk(err) => write!(f, "Key-related error: {}", err),
+            Self::Miniscript(err) => write!(f, "Miniscript error: {}", err),
+            Self::Hex(err) => write!(f, "Hex decoding error: {}", err),
+        }
     }
 }
 

src/descriptor/policy.rs

@@ -512,7 +512,14 @@ pub enum PolicyError {
 
 impl fmt::Display for PolicyError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::NotEnoughItemsSelected(err) => write!(f, "Not enought items selected: {}", err),
+            Self::IndexOutOfRange(index) => write!(f, "Index out of range: {}", index),
+            Self::AddOnLeaf => write!(f, "Add on leaf"),
+            Self::AddOnPartialComplete => write!(f, "Add on partial complete"),
+            Self::MixedTimelockUnits => write!(f, "Mixed timelock units"),
+            Self::IncompatibleConditions => write!(f, "Incompatible conditions"),
+        }
     }
 }
 

src/error.rs

@@ -107,7 +107,7 @@ pub enum Error {
     MiniscriptPsbt(MiniscriptPsbtError),
     /// BIP32 error
     Bip32(bitcoin::util::bip32::Error),
-    /// An ECDSA error
+    /// A secp256k1 error
     Secp256k1(bitcoin::secp256k1::Error),
     /// Error serializing or deserializing JSON data
     Json(serde_json::Error),
@@ -157,6 +157,18 @@ pub enum MiniscriptPsbtError {
     OutputUpdate(miniscript::psbt::OutputUpdateError),
 }
 
+impl fmt::Display for MiniscriptPsbtError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Conversion(err) => write!(f, "Conversion error: {}", err),
+            Self::UtxoUpdate(err) => write!(f, "UTXO update error: {}", err),
+            Self::OutputUpdate(err) => write!(f, "Output update error: {}", err),
+        }
+    }
+}
+
+impl std::error::Error for MiniscriptPsbtError {}
+
 /// Represents the last failed [`crate::blockchain::WalletSync`] sync attempt in which we were short
 /// on cached `scriptPubKey`s.
 #[derive(Debug)]
@@ -169,7 +181,93 @@ pub struct MissingCachedScripts {
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::InvalidU32Bytes(_) => write!(
+                f,
+                "Wrong number of bytes found when trying to convert to u32"
+            ),
+            Self::Generic(err) => write!(f, "Generic error: {}", err),
+            Self::ScriptDoesntHaveAddressForm => write!(f, "Script doesn't have address form"),
+            Self::NoRecipients => write!(f, "Cannot build tx without recipients"),
+            Self::NoUtxosSelected => write!(f, "No UTXO selected"),
+            Self::OutputBelowDustLimit(limit) => {
+                write!(f, "Output below the dust limit: {}", limit)
+            }
+            Self::InsufficientFunds { needed, available } => write!(
+                f,
+                "Insufficient funds: {} sat available of {} sat needed",
+                available, needed
+            ),
+            Self::BnBTotalTriesExceeded => {
+                write!(f, "Branch and bound coin selection: total tries exceeded")
+            }
+            Self::BnBNoExactMatch => write!(f, "Branch and bound coin selection: not exact match"),
+            Self::UnknownUtxo => write!(f, "UTXO not found in the internal database"),
+            Self::TransactionNotFound => {
+                write!(f, "Transaction not found in the internal database")
+            }
+            Self::TransactionConfirmed => write!(f, "Transaction already confirmed"),
+            Self::IrreplaceableTransaction => write!(f, "Transaction can't be replaced"),
+            Self::FeeRateTooLow { required } => write!(
+                f,
+                "Fee rate too low: required {} sat/vbyte",
+                required.as_sat_per_vb()
+            ),
+            Self::FeeTooLow { required } => write!(f, "Fee to low: required {} sat", required),
+            Self::FeeRateUnavailable => write!(f, "Fee rate unavailable"),
+            Self::MissingKeyOrigin(err) => write!(f, "Missing key origin: {}", err),
+            Self::Key(err) => write!(f, "Key error: {}", err),
+            Self::ChecksumMismatch => write!(f, "Descriptor checksum mismatch"),
+            Self::SpendingPolicyRequired(keychain_kind) => {
+                write!(f, "Spending policy required: {:?}", keychain_kind)
+            }
+            Self::InvalidPolicyPathError(err) => write!(f, "Invalid policy path: {}", err),
+            Self::Signer(err) => write!(f, "Signer error: {}", err),
+            Self::InvalidNetwork { requested, found } => write!(
+                f,
+                "Invalid network: requested {} but found {}",
+                requested, found
+            ),
+            #[cfg(feature = "verify")]
+            Self::Verification(err) => write!(f, "Transaction verification error: {}", err),
+            Self::InvalidProgressValue(progress) => {
+                write!(f, "Invalid progress value: {}", progress)
+            }
+            Self::ProgressUpdateError => write!(
+                f,
+                "Progress update error (maybe the channel has been closed)"
+            ),
+            Self::InvalidOutpoint(outpoint) => write!(
+                f,
+                "Requested outpoint doesn't exist in the tx: {}",
+                outpoint
+            ),
+            Self::Descriptor(err) => write!(f, "Descriptor error: {}", err),
+            Self::Encode(err) => write!(f, "Encoding 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::Secp256k1(err) => write!(f, "Secp256k1 error: {}", err),
+            Self::Json(err) => write!(f, "Serialize/Deserialize JSON error: {}", err),
+            Self::Hex(err) => write!(f, "Hex decoding error: {}", err),
+            Self::Psbt(err) => write!(f, "PSBT error: {}", err),
+            Self::PsbtParse(err) => write!(f, "Impossible to parse PSBT: {}", err),
+            Self::MissingCachedScripts(missing_cached_scripts) => {
+                write!(f, "Missing cached scripts: {:?}", missing_cached_scripts)
+            }
+            #[cfg(feature = "electrum")]
+            Self::Electrum(err) => write!(f, "Electrum client error: {}", err),
+            #[cfg(feature = "esplora")]
+            Self::Esplora(err) => write!(f, "Esplora client error: {}", err),
+            #[cfg(feature = "compact_filters")]
+            Self::CompactFilters(err) => write!(f, "Compact filters client error: {}", err),
+            #[cfg(feature = "key-value-db")]
+            Self::Sled(err) => write!(f, "Sled database error: {}", err),
+            #[cfg(feature = "rpc")]
+            Self::Rpc(err) => write!(f, "RPC client error: {}", err),
+            #[cfg(feature = "sqlite")]
+            Self::Rusqlite(err) => write!(f, "SQLite error: {}", err),
+        }
     }
 }
 

src/keys/mod.rs

@@ -935,7 +935,14 @@ impl_error!(bitcoin::util::bip32::Error, Bip32, KeyError);
 
 impl std::fmt::Display for KeyError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::InvalidScriptContext => write!(f, "Invalid script context"),
+            Self::InvalidNetwork => write!(f, "Invalid network"),
+            Self::InvalidChecksum => write!(f, "Invalid checksum"),
+            Self::Message(err) => write!(f, "{}", err),
+            Self::Bip32(err) => write!(f, "BIP32 error: {}", err),
+            Self::Miniscript(err) => write!(f, "Miniscript error: {}", err),
+        }
     }
 }
 

src/wallet/signer.rs

@@ -180,7 +180,22 @@ impl From<sighash::Error> for SignerError {
 
 impl fmt::Display for SignerError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::MissingKey => write!(f, "Missing private key"),
+            Self::InvalidKey => write!(f, "The private key in use has the right fingerprint but derives differently than expected"),
+            Self::UserCanceled => write!(f, "The user canceled the operation"),
+            Self::InputIndexOutOfRange => write!(f, "Input index out of range"),
+            Self::MissingNonWitnessUtxo => write!(f, "Missing non-witness UTXO"),
+            Self::InvalidNonWitnessUtxo => write!(f, "Invalid non-witness UTXO"),
+            Self::MissingWitnessUtxo => write!(f, "Missing witness UTXO"),
+            Self::MissingWitnessScript => write!(f, "Missing witness script"),
+            Self::MissingHdKeypath => write!(f, "Missing fingerprint and derivation path"),
+            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),
+            #[cfg(feature = "hardware-signer")]
+            Self::HWIError(err) => write!(f, "Error while signing using hardware wallets: {}", err),
+        }
     }
 }
 

src/wallet/verify.rs

@@ -91,7 +91,12 @@ pub enum VerifyError {
 
 impl fmt::Display for VerifyError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            Self::MissingInputTx(txid) => write!(f, "The transaction being spent is not available in the database or the blockchain client: {}", txid),
+            Self::InvalidInput(outpoint) => write!(f, "The transaction being spent doesn't have the requested output: {}", outpoint),
+            Self::Consensus(err) => write!(f, "Consensus error: {:?}", err),
+            Self::Global(err) => write!(f, "Generic error: {}", err),
+        }
     }
 }