bitcoin: Stop using base58 errors

We are currently using the `base58::Error` type to create errors in
`bitcoin`, these are bitcoin errors not `base58` errors.

Note that we add what looks like duplicate
`InvalidBase58PayloadLengthError` types but they are different because
of the expected length. This could have been a field but I elected not
to do so for two reasons:

1. We will need to do so anyways if we crate smash more
2. The `crypto::key` one can have one of two values 33 or 34.

With this applied we can remove the now unused error variants from
`base58::Error`.
This commit is contained in:
Tobin C. Harding 2024-02-15 16:26:55 +11:00
parent 669d5e8fc6
commit 4f68e79da0
No known key found for this signature in database
GPG Key ID: 40BF9E4C269D6607
5 changed files with 193 additions and 31 deletions

View File

@ -14,15 +14,6 @@ pub enum Error {
Decode(InvalidCharacterError),
/// 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),
}
@ -37,11 +28,6 @@ impl fmt::Display for Error {
Decode(ref e) => write_err!(f, "decode"; e),
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"),
}
}
@ -55,9 +41,6 @@ impl std::error::Error for Error {
match self {
Decode(ref e) => Some(e),
BadChecksum(_, _)
| InvalidLength(_)
| InvalidExtendedKeyVersion(_)
| InvalidAddressVersion(_)
| TooShort(_) => None,
}
}

View File

@ -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 {}

View File

@ -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))

View File

@ -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;

View File

@ -476,16 +476,16 @@ impl PrivateKey {
let compressed = match data.len() {
33 => false,
34 => true,
_ => {
return Err(FromWifError::Base58(base58::Error::InvalidLength(data.len())));
length => {
return Err(InvalidBase58PayloadLengthError { length }.into());
}
};
let network = match data[0] {
128 => NetworkKind::Main,
239 => NetworkKind::Test,
x => {
return Err(FromWifError::Base58(base58::Error::InvalidAddressVersion(x)));
invalid => {
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),
Secp256k1(e)=> Some(e),
match *self {
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::*;