diff --git a/src/lib.rs b/src/lib.rs index 561e6e74f..731be5f5a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -218,13 +218,15 @@ const BUF_LENGTH: usize = 10; pub fn decode(s: &str) -> Result<(Hrp, Vec), DecodeError> { let unchecked = UncheckedHrpstring::new(s)?; - if let Err(e) = unchecked.validate_checksum::() { - if !unchecked.has_valid_checksum::() { + match unchecked.validate_checksum::() { + Ok(_) => {} + Err(ChecksumError::InvalidResidue(ref res_err)) if res_err.matches_bech32_checksum() => {} + Err(e) => { return Err(DecodeError::Checksum(e)); } - }; - // One of the checksums was valid, Ck is only for length and since - // they are both the same we can use either here. + } + // One of the checksums was valid. `Bech32m` is only used for its + // length and since it is the same as `Bech32` we can use either here. let checked = unchecked.remove_checksum::(); Ok((checked.hrp(), checked.byte_iter().collect())) diff --git a/src/primitives/decode.rs b/src/primitives/decode.rs index 7752ffdaf..f463f35f6 100644 --- a/src/primitives/decode.rs +++ b/src/primitives/decode.rs @@ -76,11 +76,12 @@ use core::{fmt, iter, slice, str}; use crate::error::write_err; -use crate::primitives::checksum::{self, Checksum}; +use crate::primitives::checksum::{self, Checksum, PackedFe32}; use crate::primitives::gf32::Fe32; use crate::primitives::hrp::{self, Hrp}; use crate::primitives::iter::{Fe32IterExt, FesToBytes}; use crate::primitives::segwit::{self, WitnessLengthError, VERSION_0}; +use crate::primitives::{FieldVec, Polynomial}; use crate::{Bech32, Bech32m}; /// Separator between the hrp and payload (as defined by BIP-173). @@ -277,8 +278,9 @@ impl<'s> UncheckedHrpstring<'s> { checksum_eng.input_fe(fe); } - if checksum_eng.residue() != &Ck::TARGET_RESIDUE { - return Err(InvalidResidue); + let residue = *checksum_eng.residue(); + if residue != Ck::TARGET_RESIDUE { + return Err(InvalidResidue(InvalidResidueError::new(residue, Ck::TARGET_RESIDUE))); } Ok(()) @@ -952,7 +954,7 @@ pub enum ChecksumError { /// String exceeds maximum allowed length. CodeLength(CodeLengthError), /// The checksum residue is not valid for the data. - InvalidResidue, + InvalidResidue(InvalidResidueError), /// The checksummed string is not a valid length. InvalidLength, } @@ -963,7 +965,7 @@ impl fmt::Display for ChecksumError { match *self { CodeLength(ref e) => write_err!(f, "string exceeds maximum allowed length"; e), - InvalidResidue => write!(f, "the checksum residue is not valid for the data"), + InvalidResidue(ref e) => write_err!(f, "checksum failed"; e), InvalidLength => write!(f, "the checksummed string is not a valid length"), } } @@ -976,11 +978,56 @@ impl std::error::Error for ChecksumError { match *self { CodeLength(ref e) => Some(e), - InvalidResidue | InvalidLength => None, + InvalidResidue(ref e) => Some(e), + InvalidLength => None, } } } +/// Residue mismatch validating the checksum. That is, "the checksum failed". +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InvalidResidueError { + actual: Polynomial, + target: Polynomial, +} + +impl fmt::Display for InvalidResidueError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if self.actual.has_data() { + write!(f, "residue {} did not match target {}", self.actual, self.target) + } else { + f.write_str("residue mismatch") + } + } +} + +impl InvalidResidueError { + /// Constructs a new "invalid residue" error. + fn new(residue: F, target_residue: F) -> Self { + Self { + actual: FieldVec::from_residue(residue).into(), + target: FieldVec::from_residue(target_residue).into(), + } + } + + /// Whether this "invalid residue" error actually represents a valid residue + /// for the bech32 checksum. + /// + /// This method could in principle be made generic over the intended checksum, + /// but it is not clear what the purpose would be (checking bech32 vs bech32m + /// is a special case), and the method would necessarily panic if called with + /// too large a checksum without an allocator. We would like to better understand + /// the usecase for this before exposing such a footgun. + pub fn matches_bech32_checksum(&self) -> bool { + self.actual == FieldVec::from_residue(Bech32::TARGET_RESIDUE).into() + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InvalidResidueError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + /// Encoding HRP and data into a bech32 string exceeds the checksum code length. #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] @@ -1065,6 +1112,9 @@ impl std::error::Error for PaddingError { #[cfg(test)] mod tests { + #[cfg(all(feature = "alloc", not(feature = "std")))] + use alloc::vec::Vec; + use super::*; #[test] @@ -1117,7 +1167,7 @@ mod tests { .expect("string parses correctly") .validate_checksum::() .unwrap_err(); - assert_eq!(err, InvalidResidue); + assert!(matches!(err, InvalidResidue(..))); } #[test] @@ -1178,7 +1228,7 @@ mod tests { .unwrap() .validate_checksum::() .unwrap_err(); - assert_eq!(err, InvalidResidue); + assert!(matches!(err, InvalidResidue(..))); } #[test] diff --git a/src/primitives/fieldvec.rs b/src/primitives/fieldvec.rs index f884019b4..72a3dda3a 100644 --- a/src/primitives/fieldvec.rs +++ b/src/primitives/fieldvec.rs @@ -59,10 +59,12 @@ #[cfg(all(feature = "alloc", not(feature = "std")))] use alloc::vec::Vec; -use core::{iter, mem, ops, slice}; +use core::{fmt, iter, mem, ops, slice}; +use super::checksum::PackedFe32; use super::Field; use crate::primitives::correction::NO_ALLOC_MAX_LENGTH; +use crate::Fe32; /// A vector of field elements. /// @@ -77,6 +79,12 @@ pub struct FieldVec { inner_v: Vec, } +impl FieldVec { + pub fn from_residue(residue: R) -> Self { + (0..R::WIDTH).map(|i| Fe32(residue.unpack(i))).collect() + } +} + impl FieldVec { /// Determines whether the residue is representable, given the current /// compilation context. @@ -278,6 +286,15 @@ impl iter::FromIterator for FieldVec { } } +impl fmt::Display for FieldVec { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + for fe in self.iter() { + fe.fmt(f)?; + } + Ok(()) + } +} + impl<'a, F> IntoIterator for &'a FieldVec { type Item = &'a F; type IntoIter = slice::Iter<'a, F>; diff --git a/tests/bip_173_test_vectors.rs b/tests/bip_173_test_vectors.rs index 74868f1d7..85e971549 100644 --- a/tests/bip_173_test_vectors.rs +++ b/tests/bip_173_test_vectors.rs @@ -16,15 +16,15 @@ fn bip_173_checksum_calculated_with_uppercase_form() { // BIP-173 states reason for error should be: "checksum calculated with uppercase form of HRP". let s = "A1G7SGD8"; - assert_eq!( + assert!(matches!( CheckedHrpstring::new::(s).unwrap_err(), - CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue) - ); + CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue(..)) + )); - assert_eq!( + assert!(matches!( SegwitHrpstring::new(s).unwrap_err(), - SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue) - ); + SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue(..)) + )); } macro_rules! check_valid_bech32 { @@ -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::InvalidResidue); + assert!(matches!(p.validate_checksum::(), Err(ChecksumError::InvalidResidue(..)))); } )* } diff --git a/tests/bip_350_test_vectors.rs b/tests/bip_350_test_vectors.rs index 546beaab7..6e310d9b7 100644 --- a/tests/bip_350_test_vectors.rs +++ b/tests/bip_350_test_vectors.rs @@ -15,15 +15,15 @@ fn bip_350_checksum_calculated_with_uppercase_form() { // BIP-350 states reason for error should be: "checksum calculated with uppercase form of HRP". let s = "M1VUXWEZ"; - assert_eq!( + assert!(matches!( CheckedHrpstring::new::(s).unwrap_err(), - CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue) - ); + CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue(..)) + )); - assert_eq!( + assert!(matches!( SegwitHrpstring::new(s).unwrap_err(), - SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue) - ); + SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue(..)) + )); } macro_rules! check_valid_bech32m { @@ -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::InvalidResidue); + assert!(matches!(p.validate_checksum::().unwrap_err(), ChecksumError::InvalidResidue(..))); } )* }