From 8eb7954da33cbb9c5e620a049cff12cb1e1ff94b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 23 Oct 2023 06:02:38 +1100 Subject: [PATCH] Fix checksum error type 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. --- src/primitives/decode.rs | 37 ++++++++++++----------------------- tests/bip_173_test_vectors.rs | 6 +++--- tests/bip_350_test_vectors.rs | 6 +++--- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/primitives/decode.rs b/src/primitives/decode.rs index 5b5cb53f3..bff223e1e 100644 --- a/src/primitives/decode.rs +++ b/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::::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,10 +739,10 @@ 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 residue is not valid for the data. + InvalidResidue, /// The checksum is not a valid length. - InvalidChecksumLength, + InvalidLength, } impl fmt::Display for ChecksumError { @@ -761,8 +750,8 @@ impl fmt::Display for ChecksumError { 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 checksum 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::() .unwrap_err(); - assert_eq!(err, InvalidChecksumLength); + assert_eq!(err, InvalidLength); let err = UncheckedHrpstring::new("A1G7SGD8") .expect("string parses correctly") .validate_checksum::() .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::().unwrap_err(); - assert_eq!(err, InvalidChecksumLength); + assert_eq!(err, InvalidLength); } let err = UncheckedHrpstring::new("M1VUXWEZ") .unwrap() .validate_checksum::() .unwrap_err(); - assert_eq!(err, InvalidChecksum); + assert_eq!(err, InvalidResidue); } #[test] diff --git a/tests/bip_173_test_vectors.rs b/tests/bip_173_test_vectors.rs index 217169548..74868f1d7 100644 --- a/tests/bip_173_test_vectors.rs +++ b/tests/bip_173_test_vectors.rs @@ -18,12 +18,12 @@ fn bip_173_checksum_calculated_with_uppercase_form() { assert_eq!( CheckedHrpstring::new::(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::().expect("valid bech32"); // Valid bech32 strings are by definition invalid bech32m. - assert_eq!(p.validate_checksum::().unwrap_err(), ChecksumError::InvalidChecksum); + assert_eq!(p.validate_checksum::().unwrap_err(), ChecksumError::InvalidResidue); } )* } diff --git a/tests/bip_350_test_vectors.rs b/tests/bip_350_test_vectors.rs index 6fbdaa836..546beaab7 100644 --- a/tests/bip_350_test_vectors.rs +++ b/tests/bip_350_test_vectors.rs @@ -17,12 +17,12 @@ fn bip_350_checksum_calculated_with_uppercase_form() { assert_eq!( CheckedHrpstring::new::(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::().expect("valid bech32m"); // Valid bech32m strings are by definition invalid bech32. - assert_eq!(p.validate_checksum::().unwrap_err(), ChecksumError::InvalidChecksum); + assert_eq!(p.validate_checksum::().unwrap_err(), ChecksumError::InvalidResidue); } )* }