From 3d994f7bdb84cd3bb324fdb34947ed083464e35e Mon Sep 17 00:00:00 2001 From: "Jamil Lambert, PhD" Date: Thu, 17 Oct 2024 20:01:28 +0100 Subject: [PATCH 1/2] Decode an address string based on prefix When a decoding error occurs for a bech32 address string the error is discarded and the same address string is attempted to be decoded as base58. This then incorrectly returns a base58 error. Check the string prefix and decode as bech32 or base58 and return the relevant error. If the prefix is unknown return an `UnknownHrpError`. --- bitcoin/src/address/mod.rs | 92 +++++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 37 deletions(-) diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index d05d0cdf3..5188f62f6 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -799,47 +799,21 @@ impl Address { }; Address(inner, PhantomData) } -} -impl From
for ScriptBuf { - fn from(a: Address) -> Self { a.script_pubkey() } -} + /// Parse a bech32 Address string + pub fn from_bech32_str(s: &str) -> Result, ParseError> { + let (hrp, witness_version, data) = bech32::segwit::decode(s)?; + let version = WitnessVersion::try_from(witness_version.to_u8())?; + let program = WitnessProgram::new(version, &data) + .expect("bech32 guarantees valid program length for witness"); -// Alternate formatting `{:#}` is used to return uppercase version of bech32 addresses which should -// be used in QR codes, see [`Address::to_qr_uri`]. -impl fmt::Display for Address { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, fmt) } -} - -impl fmt::Debug for Address { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if V::IS_CHECKED { - fmt::Display::fmt(&self.0, f) - } else { - write!(f, "Address(")?; - fmt::Display::fmt(&self.0, f)?; - write!(f, ")") - } + let hrp = KnownHrp::from_hrp(hrp)?; + let inner = AddressInner::Segwit { program, hrp }; + Ok(Address(inner, PhantomData)) } -} - -/// Address can be parsed only with `NetworkUnchecked`. -impl FromStr for Address { - type Err = ParseError; - - fn from_str(s: &str) -> Result, ParseError> { - if let Ok((hrp, witness_version, data)) = bech32::segwit::decode(s) { - let version = WitnessVersion::try_from(witness_version.to_u8())?; - let program = WitnessProgram::new(version, &data) - .expect("bech32 guarantees valid program length for witness"); - - let hrp = KnownHrp::from_hrp(hrp)?; - let inner = AddressInner::Segwit { program, hrp }; - return Ok(Address(inner, PhantomData)); - } - - // If segwit decoding fails, assume its a legacy address. + /// Parse a base58 Address string + pub fn from_base58_str(s: &str) -> Result, ParseError> { if s.len() > 50 { return Err(LegacyAddressTooLongError { length: s.len() }.into()); } @@ -875,6 +849,50 @@ impl FromStr for Address { } } +impl From
for ScriptBuf { + fn from(a: Address) -> Self { a.script_pubkey() } +} + +// Alternate formatting `{:#}` is used to return uppercase version of bech32 addresses which should +// be used in QR codes, see [`Address::to_qr_uri`]. +impl fmt::Display for Address { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, fmt) } +} + +impl fmt::Debug for Address { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if V::IS_CHECKED { + fmt::Display::fmt(&self.0, f) + } else { + write!(f, "Address(")?; + fmt::Display::fmt(&self.0, f)?; + write!(f, ")") + } + } +} + +/// Address can be parsed only with `NetworkUnchecked`. +/// +/// Only segwit bech32 addresses prefixed with `bc`, `bcrt` or `tb` and legacy base58 addresses +/// prefixed with `1`, `2, `3`, `m` or `n` are supported. +impl FromStr for Address { + type Err = ParseError; + + fn from_str(s: &str) -> Result, ParseError> { + if ["bc1", "bcrt1", "tb1"].iter().any(|&prefix| s.to_lowercase().starts_with(prefix)) { + Address::from_bech32_str(s) + } else if ["1", "2", "3", "m", "n"].iter().any(|&prefix| s.starts_with(prefix)) { + Address::from_base58_str(s) + } else { + let hrp = match s.rfind('1') { + Some(pos) => &s[..pos], + None => s, + }; + Err(UnknownHrpError(hrp.to_owned()).into()) + } + } +} + /// Convert a byte array of a pubkey hash into a segwit redeem hash fn segwit_redeem_hash(pubkey_hash: PubkeyHash) -> hash160::Hash { let mut sha_engine = hash160::Hash::engine(); From 9c2ac46902ae2e6f2513ee125ea5c89953ac89a2 Mon Sep 17 00:00:00 2001 From: "Jamil Lambert, PhD" Date: Mon, 21 Oct 2024 20:55:42 +0100 Subject: [PATCH 2/2] Split up ParseError ParseError is too general and the functions returning it do not have an error path for all variants. Split out the Bech32 and Base58 related errors into their own enums. --- bitcoin/src/address/error.rs | 180 ++++++++++++++++++++++++----------- bitcoin/src/address/mod.rs | 24 +++-- 2 files changed, 145 insertions(+), 59 deletions(-) diff --git a/bitcoin/src/address/error.rs b/bitcoin/src/address/error.rs index de85ea3d0..58d0cd1e5 100644 --- a/bitcoin/src/address/error.rs +++ b/bitcoin/src/address/error.rs @@ -76,22 +76,10 @@ impl std::error::Error for UnknownAddressTypeError { #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub enum ParseError { - /// Base58 error. - Base58(base58::Error), + /// Base58 legacy decoding error. + Base58(Base58Error), /// Bech32 segwit decoding error. - Bech32(bech32::segwit::DecodeError), - /// A witness version conversion/parsing error. - WitnessVersion(witness_version::TryFromError), - /// A witness program error. - WitnessProgram(witness_program::Error), - /// Tried to parse an unknown HRP. - UnknownHrp(UnknownHrpError), - /// Legacy address is too long. - LegacyAddressTooLong(LegacyAddressTooLongError), - /// Invalid base58 payload data length for legacy address. - InvalidBase58PayloadLength(InvalidBase58PayloadLengthError), - /// Invalid legacy address prefix in base58 data payload. - InvalidLegacyPrefix(InvalidLegacyPrefixError), + Bech32(Bech32Error), /// Address's network differs from required one. NetworkValidation(NetworkValidationError), } @@ -104,13 +92,7 @@ impl fmt::Display for ParseError { match *self { Base58(ref e) => write_err!(f, "base58 error"; e), - Bech32(ref e) => write_err!(f, "bech32 segwit decoding error"; e), - WitnessVersion(ref e) => write_err!(f, "witness version conversion/parsing error"; e), - WitnessProgram(ref e) => write_err!(f, "witness program error"; e), - UnknownHrp(ref e) => write_err!(f, "tried to parse an unknown hrp"; e), - LegacyAddressTooLong(ref e) => write_err!(f, "legacy address base58 string"; e), - InvalidBase58PayloadLength(ref e) => write_err!(f, "legacy address base58 data"; e), - InvalidLegacyPrefix(ref e) => write_err!(f, "legacy address base58 prefix"; e), + Bech32(ref e) => write_err!(f, "bech32 error"; e), NetworkValidation(ref e) => write_err!(f, "validation error"; e), } } @@ -124,47 +106,21 @@ impl std::error::Error for ParseError { match *self { Base58(ref e) => Some(e), Bech32(ref e) => Some(e), - WitnessVersion(ref e) => Some(e), - WitnessProgram(ref e) => Some(e), - UnknownHrp(ref e) => Some(e), - LegacyAddressTooLong(ref e) => Some(e), - InvalidBase58PayloadLength(ref e) => Some(e), - InvalidLegacyPrefix(ref e) => Some(e), NetworkValidation(ref e) => Some(e), } } } -impl From for ParseError { - fn from(e: base58::Error) -> Self { Self::Base58(e) } +impl From for ParseError { + fn from(e: Base58Error) -> Self { Self::Base58(e) } } -impl From for ParseError { - fn from(e: bech32::segwit::DecodeError) -> Self { Self::Bech32(e) } -} - -impl From for ParseError { - fn from(e: witness_version::TryFromError) -> Self { Self::WitnessVersion(e) } -} - -impl From for ParseError { - fn from(e: witness_program::Error) -> Self { Self::WitnessProgram(e) } +impl From for ParseError { + fn from(e: Bech32Error) -> Self { Self::Bech32(e) } } impl From for ParseError { - fn from(e: UnknownHrpError) -> Self { Self::UnknownHrp(e) } -} - -impl From for ParseError { - fn from(e: LegacyAddressTooLongError) -> Self { Self::LegacyAddressTooLong(e) } -} - -impl From for ParseError { - fn from(e: InvalidBase58PayloadLengthError) -> Self { Self::InvalidBase58PayloadLength(e) } -} - -impl From for ParseError { - fn from(e: InvalidLegacyPrefixError) -> Self { Self::InvalidLegacyPrefix(e) } + fn from(e: UnknownHrpError) -> ParseError { Self::Bech32(e.into()) } } impl From for ParseError { @@ -205,6 +161,124 @@ impl fmt::Display for NetworkValidationError { #[cfg(feature = "std")] impl std::error::Error for NetworkValidationError {} +/// Bech32 related error. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum Bech32Error { + /// Parse segwit Bech32 error. + ParseBech32(bech32::segwit::DecodeError), + /// A witness version conversion/parsing error. + WitnessVersion(witness_version::TryFromError), + /// A witness program error. + WitnessProgram(witness_program::Error), + /// Tried to parse an unknown HRP. + UnknownHrp(UnknownHrpError), +} + +internals::impl_from_infallible!(Bech32Error); + +impl fmt::Display for Bech32Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use Bech32Error::*; + + match *self { + ParseBech32(ref e) => write_err!(f, "segwit parsing error"; e), + WitnessVersion(ref e) => write_err!(f, "witness version conversion/parsing error"; e), + WitnessProgram(ref e) => write_err!(f, "witness program error"; e), + UnknownHrp(ref e) => write_err!(f, "unknown hrp error"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for Bech32Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use Bech32Error::*; + + match *self { + ParseBech32(ref e) => Some(e), + WitnessVersion(ref e) => Some(e), + WitnessProgram(ref e) => Some(e), + UnknownHrp(ref e) => Some(e), + } + } +} + +impl From for Bech32Error { + fn from(e: bech32::segwit::DecodeError) -> Self { Self::ParseBech32(e) } +} + +impl From for Bech32Error { + fn from(e: witness_version::TryFromError) -> Self { Self::WitnessVersion(e) } +} + +impl From for Bech32Error { + fn from(e: witness_program::Error) -> Self { Self::WitnessProgram(e) } +} + +impl From for Bech32Error { + fn from(e: UnknownHrpError) -> Self { Self::UnknownHrp(e) } +} + +/// Base58 related error. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum Base58Error { + /// Parse legacy Base58 error. + ParseBase58(base58::Error), + /// Legacy address is too long. + LegacyAddressTooLong(LegacyAddressTooLongError), + /// Invalid base58 payload data length for legacy address. + InvalidBase58PayloadLength(InvalidBase58PayloadLengthError), + /// Invalid legacy address prefix in base58 data payload. + InvalidLegacyPrefix(InvalidLegacyPrefixError), +} + +internals::impl_from_infallible!(Base58Error); + +impl fmt::Display for Base58Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use Base58Error::*; + + match *self { + ParseBase58(ref e) => write_err!(f, "legacy parsing error"; e), + LegacyAddressTooLong(ref e) => write_err!(f, "legacy address length error"; e), + InvalidBase58PayloadLength(ref e) => write_err!(f, "legacy payload length error"; e), + InvalidLegacyPrefix(ref e) => write_err!(f, "legacy prefix error"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for Base58Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use Base58Error::*; + + match *self { + ParseBase58(ref e) => Some(e), + LegacyAddressTooLong(ref e) => Some(e), + InvalidBase58PayloadLength(ref e) => Some(e), + InvalidLegacyPrefix(ref e) => Some(e), + } + } +} + +impl From for Base58Error { + fn from(e: base58::Error) -> Self { Self::ParseBase58(e) } +} + +impl From for Base58Error { + fn from(e: LegacyAddressTooLongError) -> Self { Self::LegacyAddressTooLong(e) } +} + +impl From for Base58Error { + fn from(e: InvalidBase58PayloadLengthError) -> Self { Self::InvalidBase58PayloadLength(e) } +} + +impl From for Base58Error { + fn from(e: InvalidLegacyPrefixError) -> Self { Self::InvalidLegacyPrefix(e) } +} + /// Decoded base58 data was an invalid length. #[derive(Debug, Clone, PartialEq, Eq)] pub struct InvalidBase58PayloadLengthError { diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index 5188f62f6..87744113b 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -59,8 +59,9 @@ use crate::taproot::TapNodeHash; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] pub use self::error::{ - FromScriptError, InvalidBase58PayloadLengthError, InvalidLegacyPrefixError, LegacyAddressTooLongError, - NetworkValidationError, ParseError, UnknownAddressTypeError, UnknownHrpError, + Base58Error, Bech32Error, FromScriptError, InvalidBase58PayloadLengthError, + InvalidLegacyPrefixError, LegacyAddressTooLongError, NetworkValidationError, + ParseError, UnknownAddressTypeError, UnknownHrpError }; /// The different types of addresses. @@ -801,7 +802,7 @@ impl Address { } /// Parse a bech32 Address string - pub fn from_bech32_str(s: &str) -> Result, ParseError> { + pub fn from_bech32_str(s: &str) -> Result, Bech32Error> { let (hrp, witness_version, data) = bech32::segwit::decode(s)?; let version = WitnessVersion::try_from(witness_version.to_u8())?; let program = WitnessProgram::new(version, &data) @@ -813,7 +814,7 @@ impl Address { } /// Parse a base58 Address string - pub fn from_base58_str(s: &str) -> Result, ParseError> { + pub fn from_base58_str(s: &str) -> Result, Base58Error> { if s.len() > 50 { return Err(LegacyAddressTooLongError { length: s.len() }.into()); } @@ -875,14 +876,25 @@ impl fmt::Debug for Address { /// /// Only segwit bech32 addresses prefixed with `bc`, `bcrt` or `tb` and legacy base58 addresses /// prefixed with `1`, `2, `3`, `m` or `n` are supported. +/// +/// # Errors +/// +/// - [`ParseError::Bech32`] if the segwit address begins with a `bc`, `bcrt` or `tb` and is not a +/// valid bech32 address. +/// +/// - [`ParseError::Base58`] if the legacy address begins with a `1`, `2`, `3`, `m` or `n` and is +/// not a valid base58 address. +/// +/// - [`UnknownHrpError`] if the address does not begin with one of the above segwit or +/// legacy prifixes. impl FromStr for Address { type Err = ParseError; fn from_str(s: &str) -> Result, ParseError> { if ["bc1", "bcrt1", "tb1"].iter().any(|&prefix| s.to_lowercase().starts_with(prefix)) { - Address::from_bech32_str(s) + Ok(Address::from_bech32_str(s)?) } else if ["1", "2", "3", "m", "n"].iter().any(|&prefix| s.starts_with(prefix)) { - Address::from_base58_str(s) + Ok(Address::from_base58_str(s)?) } else { let hrp = match s.rfind('1') { Some(pos) => &s[..pos],