Skip to content

Commit

Permalink
Fix checksum error type
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tcharding committed Oct 22, 2023
1 parent 7207926 commit 8eb7954
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 30 deletions.
37 changes: 13 additions & 24 deletions src/primitives/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -186,7 +186,7 @@ impl<'s> UncheckedHrpstring<'s> {
}

if checksum_eng.residue() != &Ck::TARGET_RESIDUE {
return Err(InvalidChecksum);
return Err(InvalidResidue);
}

Ok(())
Expand Down Expand Up @@ -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.
Expand All @@ -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"),
}
Expand All @@ -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,
}
}
}
Expand All @@ -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 residue is not valid for the data.
InvalidResidue,
/// The checksum is not a valid length.
InvalidChecksumLength,
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 checksum is not a valid length"),
}
}
}
Expand All @@ -773,7 +762,7 @@ impl std::error::Error for ChecksumError {
use ChecksumError::*;

match *self {
InvalidChecksum | InvalidChecksumLength => None,
InvalidResidue | InvalidLength => None,
}
}
}
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
6 changes: 3 additions & 3 deletions tests/bip_173_test_vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand All @@ -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);
}
)*
}
Expand Down
6 changes: 3 additions & 3 deletions tests/bip_350_test_vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand All @@ -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);
}
)*
}
Expand Down

0 comments on commit 8eb7954

Please sign in to comment.