Skip to content

Commit 4c6010e

Browse files
committed
primitives: introduce InvalidResidueError
We are going to want to extend the ChecksumError::InvalidResidue error variant to allow error correction, and in order to do so, we need to know the actual residue computed from a string, not just that the residue failed to match the target. Uses the `Polynomial` type; see the previous commits for some caveats about this type when alloc is disabled. (Prior to this PR, you just couldn't use Polynomial at all without alloc.) As an initial application of the new error, avoid re-validating a bech32 address to handle the distinction between bech32m and bech32. Instead, if bech32m validation fails, check if the "bad" residue actually matches the bech32 target. If so, accept. On my system this reduces the `bech32_parse_address` benchmark from 610ns to 455ns, more than a 33% speedup.
1 parent b76adbe commit 4c6010e

File tree

6 files changed

+97
-28
lines changed

6 files changed

+97
-28
lines changed

src/lib.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -218,13 +218,15 @@ const BUF_LENGTH: usize = 10;
218218
pub fn decode(s: &str) -> Result<(Hrp, Vec<u8>), DecodeError> {
219219
let unchecked = UncheckedHrpstring::new(s)?;
220220

221-
if let Err(e) = unchecked.validate_checksum::<Bech32m>() {
222-
if !unchecked.has_valid_checksum::<Bech32>() {
221+
match unchecked.validate_checksum::<Bech32m>() {
222+
Ok(_) => {}
223+
Err(ChecksumError::InvalidResidue(ref res_err)) if res_err.matches_bech32_checksum() => {}
224+
Err(e) => {
223225
return Err(DecodeError::Checksum(e));
224226
}
225-
};
226-
// One of the checksums was valid, Ck is only for length and since
227-
// they are both the same we can use either here.
227+
}
228+
// One of the checksums was valid. `Bech32m` is only used for its
229+
// length and since it is the same as `Bech32` we can use either here.
228230
let checked = unchecked.remove_checksum::<Bech32m>();
229231

230232
Ok((checked.hrp(), checked.byte_iter().collect()))

src/primitives/decode.rs

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,12 @@
7676
use core::{fmt, iter, slice, str};
7777

7878
use crate::error::write_err;
79-
use crate::primitives::checksum::{self, Checksum};
79+
use crate::primitives::checksum::{self, Checksum, PackedFe32};
8080
use crate::primitives::gf32::Fe32;
8181
use crate::primitives::hrp::{self, Hrp};
8282
use crate::primitives::iter::{Fe32IterExt, FesToBytes};
8383
use crate::primitives::segwit::{self, WitnessLengthError, VERSION_0};
84+
use crate::primitives::Polynomial;
8485
use crate::{Bech32, Bech32m};
8586

8687
/// Separator between the hrp and payload (as defined by BIP-173).
@@ -277,8 +278,9 @@ impl<'s> UncheckedHrpstring<'s> {
277278
checksum_eng.input_fe(fe);
278279
}
279280

280-
if checksum_eng.residue() != &Ck::TARGET_RESIDUE {
281-
return Err(InvalidResidue);
281+
let residue = *checksum_eng.residue();
282+
if residue != Ck::TARGET_RESIDUE {
283+
return Err(InvalidResidue(InvalidResidueError::new(residue, Ck::TARGET_RESIDUE)));
282284
}
283285

284286
Ok(())
@@ -952,7 +954,7 @@ pub enum ChecksumError {
952954
/// String exceeds maximum allowed length.
953955
CodeLength(CodeLengthError),
954956
/// The checksum residue is not valid for the data.
955-
InvalidResidue,
957+
InvalidResidue(InvalidResidueError),
956958
/// The checksummed string is not a valid length.
957959
InvalidLength,
958960
}
@@ -963,7 +965,7 @@ impl fmt::Display for ChecksumError {
963965

964966
match *self {
965967
CodeLength(ref e) => write_err!(f, "string exceeds maximum allowed length"; e),
966-
InvalidResidue => write!(f, "the checksum residue is not valid for the data"),
968+
InvalidResidue(ref e) => write_err!(f, "checksum failed"; e),
967969
InvalidLength => write!(f, "the checksummed string is not a valid length"),
968970
}
969971
}
@@ -976,11 +978,56 @@ impl std::error::Error for ChecksumError {
976978

977979
match *self {
978980
CodeLength(ref e) => Some(e),
979-
InvalidResidue | InvalidLength => None,
981+
InvalidResidue(ref e) => Some(e),
982+
InvalidLength => None,
980983
}
981984
}
982985
}
983986

987+
/// Residue mismatch validating the checksum. That is, "the checksum failed".
988+
#[derive(Debug, Clone, PartialEq, Eq)]
989+
pub struct InvalidResidueError {
990+
actual: Polynomial<Fe32>,
991+
target: Polynomial<Fe32>,
992+
}
993+
994+
impl fmt::Display for InvalidResidueError {
995+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
996+
if self.actual.has_data() {
997+
write!(f, "residue {} did not match target {}", self.actual, self.target)
998+
} else {
999+
f.write_str("residue mismatch")
1000+
}
1001+
}
1002+
}
1003+
1004+
impl InvalidResidueError {
1005+
/// Constructs a new "invalid residue" error.
1006+
fn new<F: PackedFe32>(residue: F, target_residue: F) -> Self {
1007+
Self {
1008+
actual: Polynomial::from_residue(residue),
1009+
target: Polynomial::from_residue(target_residue),
1010+
}
1011+
}
1012+
1013+
/// Whether this "invalid residue" error actually represents a valid residue
1014+
/// for the bech32 checksum.
1015+
///
1016+
/// This method could in principle be made generic over the intended checksum,
1017+
/// but it is not clear what the purpose would be (checking bech32 vs bech32m
1018+
/// is a special case), and the method would necessarily panic if called with
1019+
/// too large a checksum without an allocator. We would like to better understand
1020+
/// the usecase for this before exposing such a footgun.
1021+
pub fn matches_bech32_checksum(&self) -> bool {
1022+
self.actual == Polynomial::from_residue(Bech32::TARGET_RESIDUE)
1023+
}
1024+
}
1025+
1026+
#[cfg(feature = "std")]
1027+
impl std::error::Error for InvalidResidueError {
1028+
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
1029+
}
1030+
9841031
/// Encoding HRP and data into a bech32 string exceeds the checksum code length.
9851032
#[derive(Debug, Clone, PartialEq, Eq)]
9861033
#[non_exhaustive]
@@ -1065,6 +1112,9 @@ impl std::error::Error for PaddingError {
10651112

10661113
#[cfg(test)]
10671114
mod tests {
1115+
#[cfg(all(feature = "alloc", not(feature = "std")))]
1116+
use alloc::vec::Vec;
1117+
10681118
use super::*;
10691119

10701120
#[test]
@@ -1117,7 +1167,7 @@ mod tests {
11171167
.expect("string parses correctly")
11181168
.validate_checksum::<Bech32>()
11191169
.unwrap_err();
1120-
assert_eq!(err, InvalidResidue);
1170+
assert!(matches!(err, InvalidResidue(..)));
11211171
}
11221172

11231173
#[test]
@@ -1178,7 +1228,7 @@ mod tests {
11781228
.unwrap()
11791229
.validate_checksum::<Bech32m>()
11801230
.unwrap_err();
1181-
assert_eq!(err, InvalidResidue);
1231+
assert!(matches!(err, InvalidResidue(..)));
11821232
}
11831233

11841234
#[test]

src/primitives/fieldvec.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
6060
#[cfg(all(feature = "alloc", not(feature = "std")))]
6161
use alloc::vec::Vec;
62-
use core::{iter, mem, ops, slice};
62+
use core::{fmt, iter, mem, ops, slice};
6363

6464
use super::Field;
6565
use crate::primitives::correction::NO_ALLOC_MAX_LENGTH;
@@ -297,6 +297,15 @@ impl<F: Clone + Default> iter::FromIterator<F> for FieldVec<F> {
297297
}
298298
}
299299

300+
impl<F: fmt::Display> fmt::Display for FieldVec<F> {
301+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
302+
for fe in self.iter() {
303+
fe.fmt(f)?;
304+
}
305+
Ok(())
306+
}
307+
}
308+
300309
impl<'a, F> IntoIterator for &'a FieldVec<F> {
301310
type Item = &'a F;
302311
type IntoIter = slice::Iter<'a, F>;

src/primitives/polynomial.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
55
use core::{fmt, iter, ops, slice};
66

7+
use super::checksum::PackedFe32;
78
use super::{ExtensionField, Field, FieldVec};
9+
use crate::Fe32;
810

911
/// A polynomial over some field.
1012
#[derive(PartialEq, Eq, Clone, Debug, Hash)]
@@ -14,6 +16,12 @@ pub struct Polynomial<F> {
1416
inner: FieldVec<F>,
1517
}
1618

19+
impl Polynomial<Fe32> {
20+
pub fn from_residue<R: PackedFe32>(residue: R) -> Self {
21+
(0..R::WIDTH).rev().map(|i| Fe32(residue.unpack(i))).collect()
22+
}
23+
}
24+
1725
impl<F: Field> Polynomial<F> {
1826
/// Determines whether the residue is representable, given the current
1927
/// compilation context.

tests/bip_173_test_vectors.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@ fn bip_173_checksum_calculated_with_uppercase_form() {
1616
// BIP-173 states reason for error should be: "checksum calculated with uppercase form of HRP".
1717
let s = "A1G7SGD8";
1818

19-
assert_eq!(
19+
assert!(matches!(
2020
CheckedHrpstring::new::<Bech32>(s).unwrap_err(),
21-
CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue)
22-
);
21+
CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue(..))
22+
));
2323

24-
assert_eq!(
24+
assert!(matches!(
2525
SegwitHrpstring::new(s).unwrap_err(),
26-
SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue)
27-
);
26+
SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue(..))
27+
));
2828
}
2929

3030
macro_rules! check_valid_bech32 {
@@ -35,7 +35,7 @@ macro_rules! check_valid_bech32 {
3535
let p = UncheckedHrpstring::new($valid_bech32).unwrap();
3636
p.validate_checksum::<Bech32>().expect("valid bech32");
3737
// Valid bech32 strings are by definition invalid bech32m.
38-
assert_eq!(p.validate_checksum::<Bech32m>().unwrap_err(), ChecksumError::InvalidResidue);
38+
assert!(matches!(p.validate_checksum::<Bech32m>(), Err(ChecksumError::InvalidResidue(..))));
3939
}
4040
)*
4141
}

tests/bip_350_test_vectors.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ fn bip_350_checksum_calculated_with_uppercase_form() {
1515
// BIP-350 states reason for error should be: "checksum calculated with uppercase form of HRP".
1616
let s = "M1VUXWEZ";
1717

18-
assert_eq!(
18+
assert!(matches!(
1919
CheckedHrpstring::new::<Bech32m>(s).unwrap_err(),
20-
CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue)
21-
);
20+
CheckedHrpstringError::Checksum(ChecksumError::InvalidResidue(..))
21+
));
2222

23-
assert_eq!(
23+
assert!(matches!(
2424
SegwitHrpstring::new(s).unwrap_err(),
25-
SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue)
26-
);
25+
SegwitHrpstringError::Checksum(ChecksumError::InvalidResidue(..))
26+
));
2727
}
2828

2929
macro_rules! check_valid_bech32m {
@@ -34,7 +34,7 @@ macro_rules! check_valid_bech32m {
3434
let p = UncheckedHrpstring::new($valid_bech32m).unwrap();
3535
p.validate_checksum::<Bech32m>().expect("valid bech32m");
3636
// Valid bech32m strings are by definition invalid bech32.
37-
assert_eq!(p.validate_checksum::<Bech32>().unwrap_err(), ChecksumError::InvalidResidue);
37+
assert!(matches!(p.validate_checksum::<Bech32>().unwrap_err(), ChecksumError::InvalidResidue(..)));
3838
}
3939
)*
4040
}

0 commit comments

Comments
 (0)