Merge #150: Fix checksum error type

eeb434f Fix checksum error type (Tobin C. Harding)

Pull request description:

  We have a couple of checksum error problems;

  - Unused checksum related variants in `CharError`
  - Unuseful error message (duplicated string "invalid checksum")

  Fix both at the same time by removing the unused variants from `CharError` and by renaming the `InvalidChecksum` variant to `InvalidResidue` and improving the `Display` impl appropriately.

ACKs for top commit:
  apoelstra:
    ACK eeb434f

Tree-SHA512: 6d0cd477951c2e9e838f3466f2114a0dc9a575c16085eea2173d971c23f8058d342c9b44b5410286a31c9e9ad4619a0a5241d1d540d0540044cdb018a195c4a8

Author: apoelstra

src/primitives/decode.rs

@@ -174,7 +174,7 @@ impl<'s> UncheckedHrpstring<'s> {
         }
 
         if self.data.len() < Ck::CHECKSUM_LENGTH {
-            return Err(InvalidChecksumLength);
+            return Err(InvalidLength);
         }
 
         let mut checksum_eng = checksum::Engine::<Ck>::new();
@@ -186,7 +186,7 @@ impl<'s> UncheckedHrpstring<'s> {
         }
 
         if checksum_eng.residue() != &Ck::TARGET_RESIDUE {
-            return Err(InvalidChecksum);
+            return Err(InvalidResidue);
         }
 
         Ok(())
@@ -705,10 +705,6 @@ pub enum CharError {
     MissingSeparator,
     /// No characters after the separator.
     NothingAfterSeparator,
-    /// The checksum does not match the rest of the data.
-    InvalidChecksum,
-    /// The checksum is not a valid length.
-    InvalidChecksumLength,
     /// Some part of the string contains an invalid character.
     InvalidChar(char),
     /// The whole string must be of one case.
@@ -722,8 +718,6 @@ impl fmt::Display for CharError {
         match *self {
             MissingSeparator => write!(f, "missing human-readable separator, \"{}\"", SEP),
             NothingAfterSeparator => write!(f, "invalid data - no characters after the separator"),
-            InvalidChecksum => write!(f, "invalid checksum"),
-            InvalidChecksumLength => write!(f, "the checksum is not a valid length"),
             InvalidChar(n) => write!(f, "invalid character (code={})", n),
             MixedCase => write!(f, "mixed-case strings not allowed"),
         }
@@ -736,12 +730,7 @@ impl std::error::Error for CharError {
         use CharError::*;
 
         match *self {
-            MissingSeparator
-            | NothingAfterSeparator
-            | InvalidChecksum
-            | InvalidChecksumLength
-            | InvalidChar(_)
-            | MixedCase => None,
+            MissingSeparator | NothingAfterSeparator | InvalidChar(_) | MixedCase => None,
         }
     }
 }
@@ -750,19 +739,19 @@ impl std::error::Error for CharError {
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub enum ChecksumError {
-    /// The checksum does not match the rest of the data.
-    InvalidChecksum,
-    /// The checksum is not a valid length.
-    InvalidChecksumLength,
+    /// The checksum residue is not valid for the data.
+    InvalidResidue,
+    /// The checksummed string is not a valid length.
+    InvalidLength,
 }
 
 impl fmt::Display for ChecksumError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use ChecksumError::*;
 
         match *self {
-            InvalidChecksum => write!(f, "invalid checksum"),
-            InvalidChecksumLength => write!(f, "the checksum is not a valid length"),
+            InvalidResidue => write!(f, "the checksum residue is not valid for the data"),
+            InvalidLength => write!(f, "the checksummed string is not a valid length"),
         }
     }
 }
@@ -773,7 +762,7 @@ impl std::error::Error for ChecksumError {
         use ChecksumError::*;
 
         match *self {
-            InvalidChecksum | InvalidChecksumLength => None,
+            InvalidResidue | InvalidLength => None,
         }
     }
 }
@@ -858,13 +847,13 @@ mod tests {
             .expect("string parses correctly")
             .validate_checksum::<Bech32>()
             .unwrap_err();
-        assert_eq!(err, InvalidChecksumLength);
+        assert_eq!(err, InvalidLength);
 
         let err = UncheckedHrpstring::new("A1G7SGD8")
             .expect("string parses correctly")
             .validate_checksum::<Bech32>()
             .unwrap_err();
-        assert_eq!(err, InvalidChecksum);
+        assert_eq!(err, InvalidResidue);
     }
 
     #[test]
@@ -918,14 +907,14 @@ mod tests {
         for s in invalid {
             let err =
                 UncheckedHrpstring::new(s).unwrap().validate_checksum::<Bech32m>().unwrap_err();
-            assert_eq!(err, InvalidChecksumLength);
+            assert_eq!(err, InvalidLength);
         }
 
         let err = UncheckedHrpstring::new("M1VUXWEZ")
             .unwrap()
             .validate_checksum::<Bech32m>()
             .unwrap_err();
-        assert_eq!(err, InvalidChecksum);
+        assert_eq!(err, InvalidResidue);
     }
 
     #[test]

tests/bip_173_test_vectors.rs

@@ -18,12 +18,12 @@ fn bip_173_checksum_calculated_with_uppercase_form() {
 
     assert_eq!(
         CheckedHrpstring::new::<Bech32>(s).unwrap_err(),
-        CheckedHrpstringError::Checksum(ChecksumError::InvalidChecksum)
+        CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue)
     );
 
     assert_eq!(
         SegwitHrpstring::new(s).unwrap_err(),
-        SegwitHrpstringError::Checksum(ChecksumError::InvalidChecksum)
+        SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue)
     );
 }
 
@@ -35,7 +35,7 @@ macro_rules! check_valid_bech32 {
                 let p = UncheckedHrpstring::new($valid_bech32).unwrap();
                 p.validate_checksum::<Bech32>().expect("valid bech32");
                 // Valid bech32 strings are by definition invalid bech32m.
-                assert_eq!(p.validate_checksum::<Bech32m>().unwrap_err(), ChecksumError::InvalidChecksum);
+                assert_eq!(p.validate_checksum::<Bech32m>().unwrap_err(), ChecksumError::InvalidResidue);
             }
         )*
     }

tests/bip_350_test_vectors.rs

@@ -17,12 +17,12 @@ fn bip_350_checksum_calculated_with_uppercase_form() {
 
     assert_eq!(
         CheckedHrpstring::new::<Bech32m>(s).unwrap_err(),
-        CheckedHrpstringError::Checksum(ChecksumError::InvalidChecksum)
+        CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue)
     );
 
     assert_eq!(
         SegwitHrpstring::new(s).unwrap_err(),
-        SegwitHrpstringError::Checksum(ChecksumError::InvalidChecksum)
+        SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue)
     );
 }
 
@@ -34,7 +34,7 @@ macro_rules! check_valid_bech32m {
                 let p = UncheckedHrpstring::new($valid_bech32m).unwrap();
                 p.validate_checksum::<Bech32m>().expect("valid bech32m");
                 // Valid bech32m strings are by definition invalid bech32.
-                assert_eq!(p.validate_checksum::<Bech32>().unwrap_err(), ChecksumError::InvalidChecksum);
+                assert_eq!(p.validate_checksum::<Bech32>().unwrap_err(), ChecksumError::InvalidResidue);
             }
         )*
     }