Merge rust-bitcoin/rust-bitcoin#3533: base58: Close all errors

c92290278e base58: Close all errors (Tobin C. Harding)

Pull request description:

  Currently we have a bunch of public errors in the `base58` crate. Only two are returned by public functions `decode()` and `decode_check()` (`Error` and `InvalidCharacterError` respectively).

  - Close the two public errors by adding private inner errors.
  - Add getters on the public errors to get the error data.
  - Make all other errors private.
  - Call `impl_from_infallible` for _all_ error types.

  Done as part of #3261

ACKs for top commit:
  apoelstra:
    ACK c92290278eaa4b4f062928c6bc6c2984e1ab7ba6; successfully ran local tests

Tree-SHA512: e2ecef89691b41dfcc8ea3ef02c3719c45c72dbb252c76eadd97d03e32518ac7e00a2623d753de947d607ac3edf785f1ffdef8178a4f17ac3e7fd26ee01031ab

base58/src/error.rs

@@ -8,8 +8,10 @@ use internals::write_err;
 
 /// An error occurred during base58 decoding (with checksum).
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
-pub enum Error {
+pub struct Error(pub(super) ErrorInner);
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(super) enum ErrorInner {
     /// Invalid character while decoding.
     Decode(InvalidCharacterError),
     /// Checksum was not correct.
@@ -19,12 +21,39 @@ pub enum Error {
 }
 
 internals::impl_from_infallible!(Error);
+internals::impl_from_infallible!(ErrorInner);
+
+impl Error {
+    /// Returns the invalid base58 ssscharacter, if encountered.
+    pub fn invalid_character(&self) -> Option<u8> {
+        match self.0 {
+            ErrorInner::Decode(ref e) => Some(e.invalid_character()),
+            _ => None,
+        }
+    }
+
+    /// Returns the incorrect checksum along with the expected checksum, if encountered.
+    pub fn incorrect_checksum(&self) -> Option<(u32, u32)> {
+        match self.0 {
+            ErrorInner::IncorrectChecksum(ref e) => Some((e.incorrect, e.expected)),
+            _ => None,
+        }
+    }
+
+    /// Returns the invalid base58 string length (require at least 4 bytes for checksum), if encountered.
+    pub fn invalid_length(&self) -> Option<usize> {
+        match self.0 {
+            ErrorInner::TooShort(ref e) => Some(e.length),
+            _ => None,
+        }
+    }
+}
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use Error::*;
+        use ErrorInner::*;
 
-        match *self {
+        match self.0 {
             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),
@@ -35,9 +64,9 @@ impl fmt::Display for Error {
 #[cfg(feature = "std")]
 impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use Error::*;
+        use ErrorInner::*;
 
-        match *self {
+        match self.0 {
             Decode(ref e) => Some(e),
             IncorrectChecksum(ref e) => Some(e),
             TooShort(ref e) => Some(e),
@@ -46,33 +75,27 @@ impl std::error::Error for Error {
 }
 
 impl From<InvalidCharacterError> for Error {
-    #[inline]
-    fn from(e: InvalidCharacterError) -> Self { Self::Decode(e) }
+    fn from(e: InvalidCharacterError) -> Self { Self(ErrorInner::Decode(e)) }
 }
 
 impl From<IncorrectChecksumError> for Error {
-    #[inline]
-    fn from(e: IncorrectChecksumError) -> Self { Self::IncorrectChecksum(e) }
+    fn from(e: IncorrectChecksumError) -> Self { Self(ErrorInner::IncorrectChecksum(e)) }
 }
 
 impl From<TooShortError> for Error {
-    #[inline]
-    fn from(e: TooShortError) -> Self { Self::TooShort(e) }
+    fn from(e: TooShortError) -> Self { Self(ErrorInner::TooShort(e)) }
 }
 
 /// Checksum was not correct.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct IncorrectChecksumError {
+pub(super) 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) }
-}
+internals::impl_from_infallible!(IncorrectChecksumError);
 
 impl fmt::Display for IncorrectChecksumError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -89,15 +112,12 @@ 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 {
+pub(super) 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 }
-}
+internals::impl_from_infallible!(TooShortError);
 
 impl fmt::Display for TooShortError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -114,18 +134,26 @@ impl std::error::Error for TooShortError {}
 
 /// Found a invalid ASCII byte while decoding base58 string.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct InvalidCharacterError {
+pub struct InvalidCharacterError(pub(super) InvalidCharacterErrorInner);
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(super) struct InvalidCharacterErrorInner {
     pub(super) invalid: u8,
 }
 
+internals::impl_from_infallible!(InvalidCharacterError);
+internals::impl_from_infallible!(InvalidCharacterErrorInner);
+
 impl InvalidCharacterError {
-    /// Returns the ASCII byte that is not a valid base58 character.
-    pub fn invalid_base58_character(&self) -> u8 { self.invalid }
+    pub(super) fn new(invalid: u8) -> Self { Self(InvalidCharacterErrorInner{ invalid }) }
+
+    /// Returns the invalid base58 character.
+    pub fn invalid_character(&self) -> u8 { self.0.invalid }
 }
 
 impl fmt::Display for InvalidCharacterError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "invalid base58 character {:#x}", self.invalid)
+        write!(f, "invalid base58 character {:#x}", self.0.invalid)
     }
 }
 

base58/src/lib.rs

@@ -76,12 +76,12 @@ pub fn decode(data: &str) -> Result<Vec<u8>, InvalidCharacterError> {
     for d58 in data.bytes() {
         // Compute "X = X * 58 + next_digit" in base 256
         if usize::from(d58) >= BASE58_DIGITS.len() {
-            return Err(InvalidCharacterError { invalid: d58 });
+            return Err(InvalidCharacterError::new(d58));
         }
         let mut carry = match BASE58_DIGITS[usize::from(d58)] {
             Some(d58) => u32::from(d58),
             None => {
-                return Err(InvalidCharacterError { invalid: d58 });
+                return Err(InvalidCharacterError::new(d58));
             }
         };
         if scratch.is_empty() {
@@ -304,7 +304,7 @@ mod tests {
             Some(hex!("00f8917303bfa8ef24f292e8fa1419b20460ba064d"))
         );
         // Non Base58 char.
-        assert_eq!(decode("¢").unwrap_err(), InvalidCharacterError { invalid: 194 });
+        assert_eq!(decode("¢").unwrap_err(), InvalidCharacterError::new(194));
     }
 
     #[test]