Merge rust-bitcoin/rust-bitcoin#2481: Improve base58 crate

af49841433 Hide base58::Error internals (Tobin C. Harding)
4f68e79da0 bitcoin: Stop using base58 errors (Tobin C. Harding)
669d5e8fc6 base58: Add InvalidCharacterError for decoding (Tobin C. Harding)
ec8609393b base58: Add error module (Tobin C. Harding)
42fabbab03 base58: Run the formatter (Tobin C. Harding)

Pull request description:

  Improve the error code in the new `base58` crate.

ACKs for top commit:
  apoelstra:
    ACK af49841433
  sanket1729:
    ACK af49841433

Tree-SHA512: c05479f02a9a58c7c98fd5987e760288562372e16cceeeb655f0a5385b4a8605945a3b6f7fcf473a7132a40f8dc90d204bc5e9e64fd2cc0bdc37dbcabb4ddc5c

base58/Cargo.toml

@@ -14,7 +14,7 @@ exclude = ["tests", "contrib"]
 
 [features]
 default = ["std"]
-std = ["hashes/std"]
+std = ["hashes/std", "internals/std"]
 
 [package.metadata.docs.rs]
 all-features = true

base58/src/error.rs

@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Error code for the `base58` crate.
+
+use core::fmt;
+
+use internals::write_err;
+
+/// An error occurred during base58 decoding (with checksum).
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum Error {
+    /// Invalid character while decoding.
+    Decode(InvalidCharacterError),
+    /// Checksum was not correct.
+    IncorrectChecksum(IncorrectChecksumError),
+    /// Checked data was too short.
+    TooShort(TooShortError),
+}
+
+internals::impl_from_infallible!(Error);
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use Error::*;
+
+        match *self {
+            Decode(ref e) => write_err!(f, "decode"; e),
+            IncorrectChecksum(ref e) => write_err!(f, "incorrect checksum"; e),
+            TooShort(ref e) => write_err!(f, "too short"; e),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use Error::*;
+
+        match *self {
+            Decode(ref e) => Some(e),
+            IncorrectChecksum(ref e) => Some(e),
+            TooShort(ref e) => Some(e),
+        }
+    }
+}
+
+impl From<InvalidCharacterError> for Error {
+    #[inline]
+    fn from(e: InvalidCharacterError) -> Self { Self::Decode(e) }
+}
+
+impl From<IncorrectChecksumError> for Error {
+    #[inline]
+    fn from(e: IncorrectChecksumError) -> Self { Self::IncorrectChecksum(e) }
+}
+
+impl From<TooShortError> for Error {
+    #[inline]
+    fn from(e: TooShortError) -> Self { Self::TooShort(e) }
+}
+
+/// Checksum was not correct.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct IncorrectChecksumError {
+    /// The incorrect checksum.
+    pub(super) incorrect: u32,
+    /// The expected checksum.
+    pub(super) expected: u32,
+}
+
+impl IncorrectChecksumError {
+    /// Returns the incorrect checksum along with the expected checksum.
+    pub fn incorrect_checksum(&self) -> (u32, u32) { (self.incorrect, self.expected) }
+}
+
+impl fmt::Display for IncorrectChecksumError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "base58 checksum {:#x} does not match expected {:#x}",
+            self.incorrect, self.expected
+        )
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for IncorrectChecksumError {}
+
+/// The decode base58 data was too short (require at least 4 bytes for checksum).
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct TooShortError {
+    /// The length of the decoded data.
+    pub(super) length: usize,
+}
+
+impl TooShortError {
+    /// Returns the invalid base58 string length (require at least 4 bytes for checksum).
+    pub fn invalid_base58_length(&self) -> usize { self.length }
+}
+
+impl fmt::Display for TooShortError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "base58 decoded data was not long enough, must be at least 4 byte: {}", self.length)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for TooShortError {}
+
+/// Found a invalid ASCII byte while decoding base58 string.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidCharacterError {
+    pub(super) invalid: u8,
+}
+
+impl InvalidCharacterError {
+    /// Returns the ASCII byte that is not a valid base58 character.
+    pub fn invalid_base58_character(&self) -> u8 { self.invalid }
+}
+
+impl fmt::Display for InvalidCharacterError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "invalid base58 character {:#x}", self.invalid)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidCharacterError {}

base58/src/lib.rs

@@ -24,14 +24,22 @@ extern crate std;
 
 static BASE58_CHARS: &[u8] = b"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
 
-use core::{fmt, iter, slice, str};
+pub mod error;
+
 #[cfg(not(feature = "std"))]
 pub use alloc::{string::String, vec::Vec};
+use core::{fmt, iter, slice, str};
 #[cfg(feature = "std")]
 pub use std::{string::String, vec::Vec};
 
 use hashes::{sha256d, Hash};
 
+use crate::error::{IncorrectChecksumError, TooShortError};
+
+#[rustfmt::skip]                // Keep public re-exports separate.
+#[doc(inline)]
+pub use self::error::{Error, InvalidCharacterError};
+
 #[rustfmt::skip]
 static BASE58_DIGITS: [Option<u8>; 128] = [
     None,     None,     None,     None,     None,     None,     None,     None,     // 0-7
@@ -53,19 +61,19 @@ static BASE58_DIGITS: [Option<u8>; 128] = [
 ];
 
 /// Decodes a base58-encoded string into a byte vector.
-pub fn decode(data: &str) -> Result<Vec<u8>, Error> {
+pub fn decode(data: &str) -> Result<Vec<u8>, InvalidCharacterError> {
     // 11/15 is just over log_256(58)
     let mut scratch = vec![0u8; 1 + data.len() * 11 / 15];
     // Build in base 256
     for d58 in data.bytes() {
         // Compute "X = X * 58 + next_digit" in base 256
         if d58 as usize >= BASE58_DIGITS.len() {
-            return Err(Error::BadByte(d58));
+            return Err(InvalidCharacterError { invalid: d58 });
         }
         let mut carry = match BASE58_DIGITS[d58 as usize] {
             Some(d58) => d58 as u32,
             None => {
-                return Err(Error::BadByte(d58));
+                return Err(InvalidCharacterError { invalid: d58 });
             }
         };
         for d256 in scratch.iter_mut().rev() {
@@ -87,7 +95,7 @@ pub fn decode(data: &str) -> Result<Vec<u8>, Error> {
 pub fn decode_check(data: &str) -> Result<Vec<u8>, Error> {
     let mut ret: Vec<u8> = decode(data)?;
     if ret.len() < 4 {
-        return Err(Error::TooShort(ret.len()));
+        return Err(TooShortError { length: ret.len() }.into());
     }
     let check_start = ret.len() - 4;
 
@@ -98,8 +106,8 @@ pub fn decode_check(data: &str) -> Result<Vec<u8>, Error> {
     let expected = u32::from_le_bytes(hash_check);
     let actual = u32::from_le_bytes(data_check);
 
-    if expected != actual {
+    if actual != expected {
-        return Err(Error::BadChecksum(expected, actual));
+        return Err(IncorrectChecksumError { incorrect: actual, expected }.into());
     }
 
     ret.truncate(check_start);
@@ -207,63 +215,6 @@ impl<T: Default + Copy> SmallVec<T> {
     }
 }
 
-/// An error that might occur during base58 decoding.
-#[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
-pub enum Error {
-    /// Invalid character encountered.
-    BadByte(u8),
-    /// Checksum was not correct (expected, actual).
-    BadChecksum(u32, u32),
-    /// The length (in bytes) of the object was not correct.
-    ///
-    /// Note that if the length is excessively long the provided length may be an estimate (and the
-    /// checksum step may be skipped).
-    InvalidLength(usize),
-    /// Extended Key version byte(s) were not recognized.
-    InvalidExtendedKeyVersion([u8; 4]),
-    /// Address version byte were not recognized.
-    InvalidAddressVersion(u8),
-    /// Checked data was less than 4 bytes.
-    TooShort(usize),
-}
-
-internals::impl_from_infallible!(Error);
-
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use Error::*;
-
-        match *self {
-            BadByte(b) => write!(f, "invalid base58 character {:#x}", b),
-            BadChecksum(exp, actual) =>
-                write!(f, "base58ck checksum {:#x} does not match expected {:#x}", actual, exp),
-            InvalidLength(ell) => write!(f, "length {} invalid for this base58 type", ell),
-            InvalidExtendedKeyVersion(ref v) =>
-                write!(f, "extended key version {:#04x?} is invalid for this base58 type", v),
-            InvalidAddressVersion(ref v) =>
-                write!(f, "address version {} is invalid for this base58 type", v),
-            TooShort(_) => write!(f, "base58ck data not even long enough for a checksum"),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use Error::*;
-
-        match self {
-            BadByte(_)
-            | BadChecksum(_, _)
-            | InvalidLength(_)
-            | InvalidExtendedKeyVersion(_)
-            | InvalidAddressVersion(_)
-            | TooShort(_) => None,
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use hex::test_hex_unwrap as hex;
@@ -317,7 +268,10 @@ mod tests {
             Some(hex!("00f8917303bfa8ef24f292e8fa1419b20460ba064d"))
         );
         // Non Base58 char.
-        assert_eq!(decode("¢").unwrap_err(), Error::BadByte(194));
+        assert_eq!(
+            decode("¢").unwrap_err(),
+            InvalidCharacterError { invalid: 194 }
+        );
     }
 
     #[test]
@@ -330,6 +284,6 @@ mod tests {
         // Check that empty slice passes roundtrip.
         assert_eq!(decode_check(&encode_check(&[])), Ok(vec![]));
         // Check that `len > 4` is enforced.
-        assert_eq!(decode_check(&encode(&[1, 2, 3])), Err(Error::TooShort(3)));
+        assert_eq!(decode_check(&encode(&[1, 2, 3])), Err(TooShortError { length: 3 }.into()));
     }
 }

bitcoin/src/address/error.rs

@@ -137,6 +137,12 @@ pub enum ParseError {
     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),
 }
 
 internals::impl_from_infallible!(ParseError);
@@ -151,6 +157,9 @@ impl fmt::Display for ParseError {
             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),
         }
     }
 }
@@ -166,6 +175,9 @@ impl std::error::Error for ParseError {
             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),
         }
     }
 }
@@ -190,6 +202,18 @@ impl From<UnknownHrpError> for ParseError {
     fn from(e: UnknownHrpError) -> Self { Self::UnknownHrp(e) }
 }
 
+impl From<LegacyAddressTooLongError> for ParseError {
+    fn from(e: LegacyAddressTooLongError) -> Self { Self::LegacyAddressTooLong(e) }
+}
+
+impl From<InvalidBase58PayloadLengthError> for ParseError {
+    fn from(e: InvalidBase58PayloadLengthError) -> Self { Self::InvalidBase58PayloadLength(e) }
+}
+
+impl From<InvalidLegacyPrefixError> for ParseError {
+    fn from(e: InvalidLegacyPrefixError) -> Self { Self::InvalidLegacyPrefix(e) }
+}
+
 /// Unknown HRP error.
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
@@ -203,3 +227,66 @@ impl fmt::Display for UnknownHrpError {
 impl std::error::Error for UnknownHrpError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
 }
+
+/// Decoded base58 data was an invalid length.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidBase58PayloadLengthError {
+    /// The base58 payload length we got after decoding address string.
+    pub(crate) length: usize,
+}
+
+impl InvalidBase58PayloadLengthError {
+    /// Returns the invalid payload length.
+    pub fn invalid_base58_payload_length(&self) -> usize { self.length }
+}
+
+impl fmt::Display for InvalidBase58PayloadLengthError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "decoded base58 data was an invalid length: {} (expected 21)", self.length)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidBase58PayloadLengthError {}
+
+/// Legacy base58 address was too long, max 50 characters.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct LegacyAddressTooLongError {
+    /// The length of the legacy address.
+    pub(crate) length: usize,
+}
+
+impl LegacyAddressTooLongError {
+    /// Returns the invalid legacy address length.
+    pub fn invalid_legcay_address_length(&self) -> usize { self.length }
+}
+
+impl fmt::Display for LegacyAddressTooLongError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "legacy address is too long: {} (max 50 characters)", self.length)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for LegacyAddressTooLongError {}
+
+/// Invalid legacy address prefix in decoded base58 data.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidLegacyPrefixError {
+    /// The invalid prefix byte.
+    pub(crate) invalid: u8,
+}
+
+impl InvalidLegacyPrefixError {
+    /// Returns the invalid prefix.
+    pub fn invalid_legacy_address_prefix(&self) -> u8 { self.invalid }
+}
+
+impl fmt::Display for InvalidLegacyPrefixError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "invalid legacy address prefix in decoded base58 data {}", self.invalid)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidLegacyPrefixError {}

bitcoin/src/address/mod.rs

@@ -53,7 +53,10 @@ use crate::taproot::TapNodeHash;
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[doc(inline)]
 pub use self::{
-    error::{FromScriptError, NetworkValidationError, ParseError, P2shError, UnknownAddressTypeError, UnknownHrpError},
+    error::{
+        FromScriptError, InvalidBase58PayloadLengthError, InvalidLegacyPrefixError, LegacyAddressTooLongError,
+        NetworkValidationError, ParseError, P2shError, UnknownAddressTypeError, UnknownHrpError
+    },
 };
 
 /// The different types of addresses.
@@ -732,11 +735,11 @@ impl FromStr for Address<NetworkUnchecked> {
         // If segwit decoding fails, assume its a legacy address.
 
         if s.len() > 50 {
-            return Err(ParseError::Base58(base58::Error::InvalidLength(s.len() * 11 / 15)));
+            return Err(LegacyAddressTooLongError { length: s.len() }.into());
         }
         let data = base58::decode_check(s)?;
         if data.len() != 21 {
-            return Err(ParseError::Base58(base58::Error::InvalidLength(data.len())));
+            return Err(InvalidBase58PayloadLengthError { length: s.len() }.into());
         }
 
         let (prefix, data) = data.split_first().expect("length checked above");
@@ -759,7 +762,7 @@ impl FromStr for Address<NetworkUnchecked> {
                 let hash = ScriptHash::from_byte_array(data);
                 AddressInner::P2sh { hash, network: NetworkKind::Test }
             }
-            x => return Err(ParseError::Base58(base58::Error::InvalidAddressVersion(x))),
+            invalid => return Err(InvalidLegacyPrefixError { invalid }.into()),
         };
 
         Ok(Address(inner, PhantomData))

bitcoin/src/bip32.rs

@@ -489,6 +489,8 @@ pub enum Error {
     Hex(hex::HexToArrayError),
     /// `PublicKey` hex should be 66 or 130 digits long.
     InvalidPublicKeyHexLength(usize),
+    /// Base58 decoded data was an invalid length.
+    InvalidBase58PayloadLength(InvalidBase58PayloadLengthError)
 }
 
 internals::impl_from_infallible!(Error);
@@ -512,6 +514,7 @@ impl fmt::Display for Error {
             Hex(ref e) => write_err!(f, "Hexadecimal decoding error"; e),
             InvalidPublicKeyHexLength(got) =>
                 write!(f, "PublicKey hex should be 66 or 130 digits long, got: {}", got),
+            InvalidBase58PayloadLength(ref e) => write_err!(f, "base58 payload"; e),
         }
     }
 }
@@ -525,6 +528,7 @@ impl std::error::Error for Error {
             Secp256k1(ref e) => Some(e),
             Base58(ref e) => Some(e),
             Hex(ref e) => Some(e),
+            InvalidBase58PayloadLength(ref e) => Some(e),
             CannotDeriveFromHardenedKey
             | InvalidChildNumber(_)
             | InvalidChildNumberFormat
@@ -544,6 +548,10 @@ impl From<base58::Error> for Error {
     fn from(err: base58::Error) -> Self { Error::Base58(err) }
 }
 
+impl From<InvalidBase58PayloadLengthError> for Error {
+    fn from(e: InvalidBase58PayloadLengthError) -> Error { Self::InvalidBase58PayloadLength(e) }
+}
+
 impl Xpriv {
     /// Construct a new master key from a seed value
     pub fn new_master(network: impl Into<NetworkKind>, seed: &[u8]) -> Result<Xpriv, Error> {
@@ -828,7 +836,7 @@ impl FromStr for Xpriv {
         let data = base58::decode_check(inp)?;
 
         if data.len() != 78 {
-            return Err(base58::Error::InvalidLength(data.len()).into());
+            return Err(InvalidBase58PayloadLengthError { length: data.len() }.into());
         }
 
         Xpriv::decode(&data)
@@ -848,7 +856,7 @@ impl FromStr for Xpub {
         let data = base58::decode_check(inp)?;
 
         if data.len() != 78 {
-            return Err(base58::Error::InvalidLength(data.len()).into());
+            return Err(InvalidBase58PayloadLengthError { length: data.len() }.into());
         }
 
         Xpub::decode(&data)
@@ -863,6 +871,27 @@ impl From<&Xpub> for XKeyIdentifier {
     fn from(key: &Xpub) -> XKeyIdentifier { key.identifier() }
 }
 
+/// Decoded base58 data was an invalid length.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidBase58PayloadLengthError {
+    /// The base58 payload length we got after decoding xpriv/xpub string.
+    pub(crate) length: usize,
+}
+
+impl InvalidBase58PayloadLengthError {
+    /// Returns the invalid payload length.
+    pub fn invalid_base58_payload_length(&self) -> usize { self.length }
+}
+
+impl fmt::Display for InvalidBase58PayloadLengthError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "decoded base58 xpriv/xpub data was an invalid length: {} (expected 78)", self.length)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidBase58PayloadLengthError {}
+
 #[cfg(test)]
 mod tests {
     use hex::test_hex_unwrap as hex;

bitcoin/src/crypto/key.rs

@@ -476,16 +476,16 @@ impl PrivateKey {
         let compressed = match data.len() {
             33 => false,
             34 => true,
-            _ => {
+            length => {
-                return Err(FromWifError::Base58(base58::Error::InvalidLength(data.len())));
+                return Err(InvalidBase58PayloadLengthError { length }.into());
             }
         };
 
         let network = match data[0] {
             128 => NetworkKind::Main,
             239 => NetworkKind::Test,
-            x => {
+            invalid => {
-                return Err(FromWifError::Base58(base58::Error::InvalidAddressVersion(x)));
+                return Err(InvalidAddressVersionError { invalid }.into());
             }
         };
 
@@ -930,8 +930,12 @@ impl From<secp256k1::Error> for FromSliceError {
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub enum FromWifError {
-    /// A base58 error.
+    /// A base58 decoding error.
     Base58(base58::Error),
+    /// Base58 decoded data was an invalid length.
+    InvalidBase58PayloadLength(InvalidBase58PayloadLengthError),
+    /// Base58 decoded data contained an invalid address version byte.
+    InvalidAddressVersion(InvalidAddressVersionError),
     /// A secp256k1 error.
     Secp256k1(secp256k1::Error),
 }
@@ -941,8 +945,11 @@ internals::impl_from_infallible!(FromWifError);
 impl fmt::Display for FromWifError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use FromWifError::*;
+
         match *self {
             Base58(ref e) => write_err!(f, "invalid base58"; e),
+            InvalidBase58PayloadLength(ref e) => write_err!(f, "decoded base58 data was an invalid length"; e),
+            InvalidAddressVersion(ref e) => write_err!(f, "decoded base58 data contained an invalid address version btye"; e),
             Secp256k1(ref e) => write_err!(f, "private key validation failed"; e),
         }
     }
@@ -952,9 +959,12 @@ impl fmt::Display for FromWifError {
 impl std::error::Error for FromWifError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         use FromWifError::*;
-        match self {
+
-            Base58(e) => Some(e),
+        match *self {
-            Secp256k1(e)=> Some(e),
+            Base58(ref e) => Some(e),
+            InvalidBase58PayloadLength(ref e) => Some(e),
+            InvalidAddressVersion(ref e) => Some(e),
+            Secp256k1(ref e)=> Some(e),
         }
     }
 }
@@ -967,6 +977,14 @@ impl From<secp256k1::Error> for FromWifError {
     fn from(e: secp256k1::Error) -> Self { Self::Secp256k1(e) }
 }
 
+impl From<InvalidBase58PayloadLengthError> for FromWifError {
+    fn from(e: InvalidBase58PayloadLengthError) -> FromWifError { Self::InvalidBase58PayloadLength(e) }
+}
+
+impl From<InvalidAddressVersionError> for FromWifError {
+    fn from(e: InvalidAddressVersionError) -> FromWifError { Self::InvalidAddressVersion(e) }
+}
+
 /// Error returned while constructing public key from string.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum ParsePublicKeyError {
@@ -1064,6 +1082,48 @@ impl std::error::Error for UncompressedPublicKeyError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
 }
 
+/// Decoded base58 data was an invalid length.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidBase58PayloadLengthError {
+    /// The base58 payload length we got after decoding WIF string.
+    pub(crate) length: usize,
+}
+
+impl InvalidBase58PayloadLengthError {
+    /// Returns the invalid payload length.
+    pub fn invalid_base58_payload_length(&self) -> usize { self.length }
+}
+
+impl fmt::Display for InvalidBase58PayloadLengthError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "decoded base58 data was an invalid length: {} (expected 33 or 34)", self.length)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidBase58PayloadLengthError {}
+
+/// Invalid address version in decoded base58 data.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidAddressVersionError {
+    /// The invalid version.
+    pub(crate) invalid: u8,
+}
+
+impl InvalidAddressVersionError {
+    /// Returns the invalid version.
+    pub fn invalid_address_version(&self) -> u8 { self.invalid }
+}
+
+impl fmt::Display for InvalidAddressVersionError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "invalid address version in decoded base58 data {}", self.invalid)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidAddressVersionError {}
+
 #[cfg(test)]
 mod tests {
     use super::*;

units/src/amount.rs

@@ -2500,7 +2500,7 @@ mod tests {
 
         use super::ParseAmountError as E;
 
-        assert_eq!(Amount::from_str("x BTC"), Err(E::from(InvalidCharacterError { invalid_char: 'x', position: 0 }).into()));
+        assert_eq!(Amount::from_str("x BTC"), Err(InvalidCharacterError { invalid_char: 'x', position: 0 }.into()));
         assert_eq!(
             Amount::from_str("xBTC"),
             Err(Unknown(UnknownDenominationError("xBTC".into())).into()),