From 8636ba7503a744d71c963b6149faf73e4e33af4c Mon Sep 17 00:00:00 2001 From: Arowolokehinde Date: Tue, 6 Oct 2026 14:31:40 +0100 Subject: [PATCH] fix: preserve detail from upstream address parse errors --- bdk-ffi/src/error.rs | 90 ++++++++++++++++++---------- bdk-ffi/src/tests/error.rs | 116 +++++++++++++++++++++++++++++++++++-- 2 files changed, 172 insertions(+), 34 deletions(-) diff --git a/bdk-ffi/src/error.rs b/bdk-ffi/src/error.rs index 8911852f..d0330a13 100644 --- a/bdk-ffi/src/error.rs +++ b/bdk-ffi/src/error.rs @@ -29,6 +29,7 @@ use bdk_wallet::tx_builder::AddUtxoError; use bdk_wallet::LoadError as BdkLoadError; use bdk_wallet::LoadWithPersistError as BdkLoadWithPersistError; use bdk_wallet::{chain, CreateWithPersistError as BdkCreateWithPersistError}; +use std::fmt::Write as _; use std::convert::TryInto; @@ -70,11 +71,11 @@ pub enum AddForeignUtxoError { #[derive(Debug, thiserror::Error, uniffi::Error)] #[uniffi::export(Debug, Display)] pub enum AddressParseError { - #[error("base58 address encoding error")] - Base58, + #[error("base58 address encoding error: {error_message}")] + Base58 { error_message: String }, - #[error("bech32 address encoding error")] - Bech32, + #[error("bech32 address encoding error: {error_message}")] + Bech32 { error_message: String }, #[error("witness version conversion/parsing error: {error_message}")] WitnessVersion { error_message: String }, @@ -82,24 +83,24 @@ pub enum AddressParseError { #[error("witness program error: {error_message}")] WitnessProgram { error_message: String }, - #[error("tried to parse an unknown hrp")] - UnknownHrp, + #[error("{error_message}")] + UnknownHrp { error_message: String }, - #[error("legacy address base58 string")] - LegacyAddressTooLong, + #[error("{error_message}")] + LegacyAddressTooLong { error_message: String }, - #[error("legacy address base58 data")] - InvalidBase58PayloadLength, + #[error("{error_message}")] + InvalidBase58PayloadLength { error_message: String }, - #[error("segwit address bech32 string")] - InvalidLegacyPrefix, + #[error("{error_message}")] + InvalidLegacyPrefix { error_message: String }, - #[error("validation error")] - NetworkValidation, + #[error("{error_message}")] + NetworkValidation { error_message: String }, // This error is required because the bdk::bitcoin::address::ParseError is non-exhaustive - #[error("other address parse error")] - OtherAddressParseErr, + #[error("other address parse error: {error_message}")] + OtherAddressParseErr { error_message: String }, } #[derive(Debug, thiserror::Error, uniffi::Error)] @@ -883,6 +884,21 @@ pub enum TxidParseError { // error conversions // ------------------------------------------------------------------------ +/// Build the complete message for an error, including its `source()` chain. +/// +/// `source()` does not cross the FFI boundary, and under `std` rust-bitcoin's +/// `write_err!` macro omits the source from `Display`, so `to_string()` on such +/// an error returns only the outermost label. +fn full_error_message(error: &dyn std::error::Error) -> String { + let mut message = error.to_string(); + let mut source = error.source(); + while let Some(inner) = source { + let _ = write!(message, ": {}", inner); + source = inner.source(); + } + message +} + impl From for CreateTxError { fn from(error: AddForeignUtxoError) -> Self { match error { @@ -965,22 +981,38 @@ impl From for ElectrumError { impl From for AddressParseError { fn from(error: BdkParseError) -> Self { match error { - BdkParseError::Base58(_) => AddressParseError::Base58, - BdkParseError::Bech32(_) => AddressParseError::Bech32, - BdkParseError::WitnessVersion(e) => AddressParseError::WitnessVersion { - error_message: e.to_string(), + BdkParseError::Base58(ref e) => AddressParseError::Base58 { + error_message: full_error_message(e), }, - BdkParseError::WitnessProgram(e) => AddressParseError::WitnessProgram { - error_message: e.to_string(), + BdkParseError::Bech32(ref e) => AddressParseError::Bech32 { + error_message: full_error_message(e), + }, + BdkParseError::WitnessVersion(ref e) => AddressParseError::WitnessVersion { + error_message: full_error_message(e), + }, + BdkParseError::WitnessProgram(ref e) => AddressParseError::WitnessProgram { + error_message: full_error_message(e), + }, + ParseError::UnknownHrp(ref e) => AddressParseError::UnknownHrp { + error_message: full_error_message(e), }, - ParseError::UnknownHrp(_) => AddressParseError::UnknownHrp, - ParseError::LegacyAddressTooLong(_) => AddressParseError::LegacyAddressTooLong, - ParseError::InvalidBase58PayloadLength(_) => { - AddressParseError::InvalidBase58PayloadLength + ParseError::LegacyAddressTooLong(ref e) => AddressParseError::LegacyAddressTooLong { + error_message: full_error_message(e), + }, + ParseError::InvalidBase58PayloadLength(ref e) => { + AddressParseError::InvalidBase58PayloadLength { + error_message: full_error_message(e), + } } - ParseError::InvalidLegacyPrefix(_) => AddressParseError::InvalidLegacyPrefix, - ParseError::NetworkValidation(_) => AddressParseError::NetworkValidation, - _ => AddressParseError::OtherAddressParseErr, + ParseError::InvalidLegacyPrefix(ref e) => AddressParseError::InvalidLegacyPrefix { + error_message: full_error_message(e), + }, + ParseError::NetworkValidation(ref e) => AddressParseError::NetworkValidation { + error_message: full_error_message(e), + }, + _ => AddressParseError::OtherAddressParseErr { + error_message: full_error_message(&error), + }, } } } diff --git a/bdk-ffi/src/tests/error.rs b/bdk-ffi/src/tests/error.rs index 42fd8e4b..a1aa4325 100644 --- a/bdk-ffi/src/tests/error.rs +++ b/bdk-ffi/src/tests/error.rs @@ -1,15 +1,121 @@ use crate::bitcoin::Psbt; use crate::error::{ - Bip32Error, Bip39Error, CannotConnectError, DescriptorError, DescriptorKeyError, ElectrumError, - EsploraError, ExtractTxError, PsbtError, PsbtParseError, RequestBuilderError, SignerError, - TransactionError, TxidParseError, + AddressParseError, Bip32Error, Bip39Error, CannotConnectError, DescriptorError, + DescriptorKeyError, ElectrumError, EsploraError, ExtractTxError, PsbtError, PsbtParseError, + RequestBuilderError, SignerError, TransactionError, TxidParseError, }; +use assert_matches::assert_matches; +use bdk_wallet::bitcoin::address::NetworkUnchecked; use bdk_wallet::bitcoin::{ - absolute, transaction, Amount as BdkAmount, Psbt as BdkPsbt, ScriptBuf as BdkScriptBuf, - Transaction as BdkTransaction, TxOut as BdkTxOut, + absolute, transaction, Address as BdkAddress, Amount as BdkAmount, Network as BdkNetwork, + Psbt as BdkPsbt, ScriptBuf as BdkScriptBuf, Transaction as BdkTransaction, TxOut as BdkTxOut, }; +#[test] +fn test_error_address_parse() { + let cases = vec![ + ( + AddressParseError::Base58 { + error_message: "decode: invalid base58 character 0x30".to_string(), + }, + "base58 address encoding error: decode: invalid base58 character 0x30", + ), + ( + AddressParseError::Bech32 { + error_message: "decoding segwit address failed: invalid checksum: the checksum \ + residue is not valid for the data" + .to_string(), + }, + "bech32 address encoding error: decoding segwit address failed: invalid checksum: \ + the checksum residue is not valid for the data", + ), + ( + AddressParseError::WitnessVersion { + error_message: "invalid witness script version: 17".to_string(), + }, + "witness version conversion/parsing error: invalid witness script version: 17", + ), + ( + AddressParseError::WitnessProgram { + error_message: "a v0 witness program must be either 20 or 32 bytes: length=21" + .to_string(), + }, + "witness program error: a v0 witness program must be either 20 or 32 bytes: length=21", + ), + ( + AddressParseError::UnknownHrp { + error_message: "unknown hrp: xyz".to_string(), + }, + "unknown hrp: xyz", + ), + ( + AddressParseError::LegacyAddressTooLong { + error_message: "legacy address is too long: 60 (max 50 characters)".to_string(), + }, + "legacy address is too long: 60 (max 50 characters)", + ), + ( + AddressParseError::InvalidBase58PayloadLength { + error_message: "decoded base58 data was an invalid length: 20 (expected 21)" + .to_string(), + }, + "decoded base58 data was an invalid length: 20 (expected 21)", + ), + ( + AddressParseError::InvalidLegacyPrefix { + error_message: "invalid legacy address prefix in decoded base58 data 8".to_string(), + }, + "invalid legacy address prefix in decoded base58 data 8", + ), + ( + AddressParseError::NetworkValidation { + error_message: + "address bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4 is not valid on testnet" + .to_string(), + }, + "address bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4 is not valid on testnet", + ), + ( + AddressParseError::OtherAddressParseErr { + error_message: "something new upstream".to_string(), + }, + "other address parse error: something new upstream", + ), + ]; + + for (error, expected_message) in cases { + assert_eq!(error.to_string(), expected_message); + } +} + +#[test] +fn test_address_parse_error_preserves_upstream_detail() { + let error: AddressParseError = "1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN3" + .parse::>() + .unwrap_err() + .into(); + + assert_matches!( + error, + AddressParseError::Base58 { error_message } if error_message.contains("checksum") + ); + + let address: BdkAddress = "bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4" + .parse() + .unwrap(); + let error: AddressParseError = address + .require_network(BdkNetwork::Testnet) + .unwrap_err() + .into(); + + assert_matches!( + error, + AddressParseError::NetworkValidation { error_message } + if error_message.contains("is not valid on testnet") + ); +} + #[test] fn test_error_bip32() { let cases = vec![