Implement TryFrom various integer types

Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid
value encountered while converting to a `u8`. This is buggy because we
cast to the `u8` to stash the value so we loose valuable bits.

Implement `TryFrom` by doing:
- Add an `Error::Overflow` variant.
- Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls
- Add a new `TryFromError` and nest it in `Error::TryFrom`
- Implement `TryFrom<T> for u5` impls for all standard integer types.

Inspired by issue rust-bitcoin#60 and the stdlib macros of the same name.

Author: tcharding

src/error.rs

@@ -0,0 +1,31 @@
+// Written by the Rust Bitcoin developers.
+// SPDX-License-Identifier: CC0-1.0
+
+// TODO: We can get this from `bitcoin-internals` crate once that is released.
+
+//! # Error
+//!
+//! Error handling macros and helpers.
+//!
+
+/// Formats error.
+///
+/// If `std` feature is OFF appends error source (delimited by `: `). We do this because
+/// `e.source()` is only available in std builds, without this macro the error source is lost for
+/// no-std builds.
+#[macro_export]
+macro_rules! write_err {
+    ($writer:expr, $string:literal $(, $args:expr)*; $source:expr) => {
+        {
+            #[cfg(feature = "std")]
+            {
+                let _ = &$source;   // Prevents clippy warnings.
+                write!($writer, $string $(, $args)*)
+            }
+            #[cfg(not(feature = "std"))]
+            {
+                write!($writer, concat!($string, ": {}") $(, $args)*, $source)
+            }
+        }
+    }
+}

src/lib.rs

@@ -74,6 +74,8 @@ use core::{fmt, mem};
 #[cfg(feature = "std")]
 use std::borrow::Cow;
 
+mod error;
+
 /// Integer in the range `0..32`
 #[derive(PartialEq, Eq, Debug, Copy, Clone, Default, PartialOrd, Ord, Hash)]
 #[allow(non_camel_case_types)]
@@ -91,18 +93,56 @@ impl From<u5> for u8 {
     fn from(v: u5) -> u8 { v.0 }
 }
 
-impl TryFrom<u8> for u5 {
-    type Error = Error;
-
-    /// Errors if `value` is out of range.
-    fn try_from(value: u8) -> Result<Self, Self::Error> {
-        if value > 31 {
-            Err(Error::InvalidData(value))
-        } else {
-            Ok(u5(value))
-        }
+macro_rules! impl_try_from_upper_bounded {
+    ($($ty:ident)+) => {
+        $(
+            impl TryFrom<$ty> for u5 {
+                type Error = TryFromIntError;
+
+                /// Tries to create the target number type from a source number type.
+                ///
+                /// # Errors
+                ///
+                /// Returns an error if `value` overflows a `u5`.
+                fn try_from(value: $ty) -> Result<Self, Self::Error> {
+                    if value > 31 {
+                        Err(TryFromIntError::PosOverflow)
+                    } else {
+                        let x = u8::try_from(value).expect("within range");
+                        Ok(u5(x))
+                    }
+                }
+            }
+        )+
     }
 }
+macro_rules! impl_try_from_both_bounded {
+    ($($ty:ident)+) => {
+        $(
+            impl TryFrom<$ty> for u5 {
+                type Error = TryFromIntError;
+
+                /// Tries to create the target number type from a source number type.
+                ///
+                /// # Errors
+                ///
+                /// Returns an error if `value` is outside of the range of a `u5`.
+                fn try_from(value: $ty) -> Result<Self, Self::Error> {
+                    if value < 0 {
+                        Err(TryFromIntError::NegOverflow)
+                    } else if value > 31 {
+                        Err(TryFromIntError::PosOverflow)
+                    } else {
+                        let x = u8::try_from(value).expect("within range");
+                        Ok(u5(x))
+                    }
+                }
+            }
+        )+
+    }
+}
+impl_try_from_upper_bounded!(u8 u16 u32 u64 u128);
+impl_try_from_both_bounded!(i8 i16 i32 i64 i128);
 
 impl AsRef<u8> for u5 {
     fn as_ref(&self) -> &u8 { &self.0 }
@@ -340,7 +380,10 @@ impl<T: AsRef<[u8]>> CheckBase32<Vec<u5>> for T {
     type Err = Error;
 
     fn check_base32(self) -> Result<Vec<u5>, Self::Err> {
-        self.as_ref().iter().map(|x| u5::try_from(*x)).collect::<Result<Vec<u5>, Error>>()
+        self.as_ref()
+            .iter()
+            .map(|x| u5::try_from(*x).map_err(Error::TryFrom))
+            .collect::<Result<Vec<u5>, Error>>()
     }
 }
 
@@ -669,12 +712,14 @@ pub enum Error {
     InvalidLength,
     /// Some part of the string contains an invalid character.
     InvalidChar(char),
-    /// Some part of the data has an invalid value.
-    InvalidData(u8),
     /// The bit conversion failed due to a padding issue.
     InvalidPadding,
     /// The whole string must be of one case.
     MixedCase,
+    /// Attempted to convert a value which overflows a `u5`.
+    Overflow,
+    /// Conversion to u5 failed.
+    TryFrom(TryFromIntError),
 }
 
 impl fmt::Display for Error {
@@ -686,9 +731,10 @@ impl fmt::Display for Error {
             InvalidChecksum => write!(f, "invalid checksum"),
             InvalidLength => write!(f, "invalid length"),
             InvalidChar(n) => write!(f, "invalid character (code={})", n),
-            InvalidData(n) => write!(f, "invalid data point ({})", n),
             InvalidPadding => write!(f, "invalid padding"),
             MixedCase => write!(f, "mixed-case strings not allowed"),
+            TryFrom(ref e) => write_err!(f, "conversion to u5 failed"; e),
+            Overflow => write!(f, "attempted to convert a value which overflows a u5"),
         }
     }
 }
@@ -699,12 +745,54 @@ impl std::error::Error for Error {
         use Error::*;
 
         match *self {
+            TryFrom(ref e) => Some(e),
             MissingSeparator | InvalidChecksum | InvalidLength | InvalidChar(_)
-            | InvalidData(_) | InvalidPadding | MixedCase => None,
+            | InvalidPadding | MixedCase | Overflow => None,
         }
     }
 }
 
+impl From<TryFromIntError> for Error {
+    fn from(e: TryFromIntError) -> Self { Error::TryFrom(e) }
+}
+
+/// Error return when TryFrom<T> fails for T -> u5 conversion.
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
+pub enum TryFromIntError {
+    /// Attempted to convert a negative value to a `u5`.
+    NegOverflow,
+    /// Attempted to convert a value which overflows a `u5`.
+    PosOverflow,
+}
+
+impl fmt::Display for TryFromIntError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use TryFromIntError::*;
+
+        match *self {
+            NegOverflow => write!(f, "attempted to convert a negative value to a u5"),
+            PosOverflow => write!(f, "attempted to convert a value which overflows a u5"),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for TryFromIntError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use TryFromIntError::*;
+
+        match *self {
+            NegOverflow | PosOverflow => None,
+        }
+    }
+}
+
+// impl From<convert::Error> for Error {
+//     fn from(e: convert::Error) -> Self {
+//         Error::InvalidData(e)
+//     }
+// }
+
 /// Convert between bit sizes
 ///
 /// # Errors
@@ -738,7 +826,7 @@ where
         let v: u32 = u32::from(Into::<u8>::into(*value));
         if (v >> from) != 0 {
             // Input value exceeds `from` bit size
-            return Err(Error::InvalidData(v as u8));
+            return Err(Error::Overflow);
         }
         acc = (acc << from) | v;
         bits += from;
@@ -885,7 +973,7 @@ mod tests {
         // Set of [data, from_bits, to_bits, pad, expected error]
         let tests: Vec<(Vec<u8>, u32, u32, bool, Error)> = vec![
             (vec![0xff], 8, 5, false, Error::InvalidPadding),
-            (vec![0x02], 1, 1, true, Error::InvalidData(0x02)),
+            (vec![0x02], 1, 1, true, Error::Overflow),
         ];
         for t in tests {
             let (data, from_bits, to_bits, pad, expected_error) = t;
@@ -918,7 +1006,7 @@ mod tests {
         assert!([0u8, 1, 2, 30, 31, 255].check_base32().is_err());
 
         assert!([1u8, 2, 3, 4].check_base32().is_ok());
-        assert_eq!([30u8, 31, 35, 20].check_base32(), Err(Error::InvalidData(35)));
+        assert_eq!([30u8, 31, 35, 20].check_base32(), Err(Error::TryFrom(TryFromIntError::PosOverflow)))
     }
 
     #[test]
@@ -1028,4 +1116,13 @@ mod tests {
 
         assert_eq!(encoded_str, "hrp1qqqq40atq3");
     }
+
+    #[test]
+    fn try_from_err() {
+        assert!(u5::try_from(32_u8).is_err());
+        assert!(u5::try_from(32_u16).is_err());
+        assert!(u5::try_from(32_u32).is_err());
+        assert!(u5::try_from(32_u64).is_err());
+        assert!(u5::try_from(32_u128).is_err());
+    }
 }