Skip to content

Commit

Permalink
Merge #204: Introduce FieldVec and improve Polynomial and `Invali…
Browse files Browse the repository at this point in the history
…dResidueError`

c209ce8 polynomial: expand unit tests (Andrew Poelstra)
dfc29ad fieldvec: add a bunch of unit tests (Andrew Poelstra)
0e3d954 polynomial: make Eq/PartialEq take leading zeros into account (Andrew Poelstra)
d08da46 correction: introduce CorrectableError trait (Andrew Poelstra)
4c6010e primitives: introduce InvalidResidueError (Andrew Poelstra)
b76adbe polynomial: add some extra functionality enabled by FieldVec (Andrew Poelstra)
690fc7d polynomial: use FieldVec in Polynomial (Andrew Poelstra)
f2ec582 primitives: introduce FieldVec type (Andrew Poelstra)

Pull request description:

  This introduces a new internal type `FieldVec` which is basically a backing array for checksum residues with some fiddly logic to work in a noalloc context.

  Unlike ArrayVec or other similar types, this one's semantics are "an unbounded vector if you have an allocator, a bounded vector otherwise". If you go outside of the bounds without an allocator the code will panic, but our use of it ensures that this is never exposed to a user.

  It also assumes that it's holding a `Default` type which lets us avoid unsafety (and dismisses the potential performance cost because this array is never expected to have more than 10 or 20 elements, even when it is "unbounded").

  Using this backing, it improves `Polynomial` and removes the alloc bound from it; using this, it puts `Polynomial` into a new `InvalidResidueError` type, which will allow users to extract the necessary information from a checksum error to run the error correction algorithm. (The resulting change to an error type makes this an API-breaking change, though I don't think that anything else here is API-breaking.)

  The next PR will actually implement correction :).

ACKs for top commit:
  tcharding:
    ACK c209ce8
  clarkmoody:
    ACK c209ce8

Tree-SHA512: 16a23bd88101be549c9d50f6b32f669688dfb1efbd6e842c1c3db2203d1775e647f0909c4a65191fd332cd07778c0431ae7b4635909a00f7c2d86135fae042e2
  • Loading branch information
apoelstra committed Sep 30, 2024
2 parents 86a5bdc + c209ce8 commit 3f98190
Show file tree
Hide file tree
Showing 9 changed files with 1,061 additions and 167 deletions.
15 changes: 10 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
// Coding conventions
#![deny(missing_docs)]
#![allow(clippy::suspicious_arithmetic_impl)] // this lint is literally always wrong
#![allow(clippy::suspicious_op_assign_impl)] // ...and "always wrong" loves company

#[cfg(bench)]
extern crate test;
Expand Down Expand Up @@ -164,6 +166,7 @@ use crate::primitives::decode::{ChecksumError, UncheckedHrpstring, UncheckedHrps
#[doc(inline)]
pub use {
crate::primitives::checksum::Checksum,
crate::primitives::correction::CorrectableError,
crate::primitives::gf32::Fe32,
crate::primitives::gf32_ext::{Fe1024, Fe32768},
crate::primitives::hrp::Hrp,
Expand Down Expand Up @@ -216,13 +219,15 @@ const BUF_LENGTH: usize = 10;
pub fn decode(s: &str) -> Result<(Hrp, Vec<u8>), DecodeError> {
let unchecked = UncheckedHrpstring::new(s)?;

if let Err(e) = unchecked.validate_checksum::<Bech32m>() {
if !unchecked.has_valid_checksum::<Bech32>() {
match unchecked.validate_checksum::<Bech32m>() {
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::<Bech32m>();

Ok((checked.hrp(), checked.byte_iter().collect()))
Expand Down
33 changes: 9 additions & 24 deletions src/primitives/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,14 @@
//!
//! [BCH]: <https://en.wikipedia.org/wiki/BCH_code>

#[cfg(all(feature = "alloc", not(feature = "std"), not(test)))]
#[cfg(all(feature = "alloc", not(feature = "std")))]
use alloc::vec::Vec;
#[cfg(feature = "alloc")]
use core::fmt;
#[cfg(feature = "alloc")]
use core::marker::PhantomData;
use core::{mem, ops};
use core::{fmt, mem, ops};

#[cfg(feature = "alloc")]
use super::Polynomial;
use crate::primitives::hrp::Hrp;
#[cfg(feature = "alloc")]
use crate::Fe1024;
use crate::Fe32;
use crate::{Fe1024, Fe32};

/// Trait defining a particular checksum.
///
Expand Down Expand Up @@ -169,18 +163,16 @@ pub trait Checksum {
/// to compute the values yourself. The reason is that the specific values
/// used depend on the representation of extension fields, which may differ
/// between implementations (and between specifications) of your BCH code.
#[cfg(feature = "alloc")]
pub struct PrintImpl<'a, ExtField = Fe1024> {
name: &'a str,
generator: &'a [Fe32],
generator: Polynomial<Fe32>,
target: &'a [Fe32],
bit_len: usize,
hex_width: usize,
midstate_repr: &'static str,
phantom: PhantomData<ExtField>,
}

#[cfg(feature = "alloc")]
impl<'a, ExtField> PrintImpl<'a, ExtField> {
/// Constructor for an object to print an impl-block for the [`Checksum`] trait.
///
Expand Down Expand Up @@ -225,7 +217,7 @@ impl<'a, ExtField> PrintImpl<'a, ExtField> {
// End sanity checks.
PrintImpl {
name,
generator,
generator: Polynomial::with_monic_leading_term(generator),
target,
bit_len,
hex_width,
Expand All @@ -235,23 +227,16 @@ impl<'a, ExtField> PrintImpl<'a, ExtField> {
}
}

#[cfg(feature = "alloc")]
impl<'a, ExtField> fmt::Display for PrintImpl<'a, ExtField>
where
ExtField: super::Bech32Field + super::ExtensionField<BaseField = Fe32>,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
// Generator polynomial as a polynomial over GF1024
let gen_poly = {
let mut v = Vec::with_capacity(self.generator.len() + 1);
v.push(ExtField::ONE);
v.extend(self.generator.iter().cloned().map(ExtField::from));
Polynomial::new(v)
};
let (gen, length, exponents) = gen_poly.bch_generator_primitive_element();
let (gen, length, exponents) = self.generator.bch_generator_primitive_element::<ExtField>();

write!(f, "// Code block generated by Checksum::print_impl polynomial ")?;
for fe in self.generator {
for fe in self.generator.as_inner() {
write!(f, "{}", fe)?;
}
write!(f, " target ")?;
Expand All @@ -278,9 +263,9 @@ where
)?;
f.write_str("\n")?;
writeln!(f, " const CODE_LENGTH: usize = {};", length)?;
writeln!(f, " const CHECKSUM_LENGTH: usize = {};", gen_poly.degree())?;
writeln!(f, " const CHECKSUM_LENGTH: usize = {};", self.generator.degree())?;
writeln!(f, " const GENERATOR_SH: [{}; 5] = [", self.midstate_repr)?;
let mut gen5 = self.generator.to_vec();
let mut gen5 = self.generator.clone().into_inner();
for _ in 0..5 {
let gen_packed = u128::pack(gen5.iter().copied().map(From::from));
writeln!(f, " 0x{:0width$x},", gen_packed, width = self.hex_width)?;
Expand Down
106 changes: 106 additions & 0 deletions src/primitives/correction.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// SPDX-License-Identifier: MIT

//! Error Correction
//!
//! Implements the Berlekamp-Massey algorithm to locate errors, with Forney's
//! equation to identify the error values, in a BCH-encoded string.
//!

use crate::primitives::decode::{
CheckedHrpstringError, ChecksumError, InvalidResidueError, SegwitHrpstringError,
};
#[cfg(feature = "alloc")]
use crate::DecodeError;

/// **One more than** the maximum length (in characters) of a checksum which
/// can be error-corrected without an allocator.
///
/// When the **alloc** feature is enabled, this constant is practically irrelevant.
/// When the feature is disabled, it represents a length beyond which this library
/// does not support error correction.
///
/// If you need this value to be increased, please file an issue describing your
/// usecase. Bear in mind that an increased value will increase memory usage for
/// all users, and the focus of this library is the Bitcoin ecosystem, so we may
/// not be able to accept your request.
// This constant is also used when comparing bech32 residues against the
// bech32/bech32m targets, which should work with no-alloc. Therefore this
// constant must be > 6 (the length of the bech32(m) checksum).
//
// Otherwise it basically represents a tradeoff between stack usage and the
// size of error types, vs functionality in a no-alloc setting. The value
// of 7 covers bech32 and bech32m. To get the descriptor checksum we need a
// value and the descriptor checksum. To also get codex32 it should be >13,
// and for "long codex32" >15 ... but consider that no-alloc contexts are
// likely to be underpowered and will struggle to do correction on these
// big codes anyway.
//
// Perhaps we will want to add a feature gate, off by default, that boosts
// this to 16, or maybe even higher. But we will wait for implementors who
// complain.
pub const NO_ALLOC_MAX_LENGTH: usize = 7;

/// Trait describing an error for which an error correction algorithm is applicable.
///
/// Essentially, this trait represents an error which contains an [`InvalidResidueError`]
/// variant.
pub trait CorrectableError {
/// Given a decoding error, if this is a "checksum failed" error, extract
/// that specific error type.
///
/// There are many ways in which decoding a checksummed string might fail.
/// If the string was well-formed in all respects except that the final
/// checksum characters appear to be wrong, it is possible to run an
/// error correction algorithm to attempt to extract errors.
///
/// In all other cases we do not have enough information to do correction.
///
/// This is the function that implementors should implement.
fn residue_error(&self) -> Option<&InvalidResidueError>;
}

impl CorrectableError for InvalidResidueError {
fn residue_error(&self) -> Option<&InvalidResidueError> { Some(self) }
}

impl CorrectableError for ChecksumError {
fn residue_error(&self) -> Option<&InvalidResidueError> {
match self {
ChecksumError::InvalidResidue(ref e) => Some(e),
_ => None,
}
}
}

impl CorrectableError for SegwitHrpstringError {
fn residue_error(&self) -> Option<&InvalidResidueError> {
match self {
SegwitHrpstringError::Checksum(ref e) => e.residue_error(),
_ => None,
}
}
}

impl CorrectableError for CheckedHrpstringError {
fn residue_error(&self) -> Option<&InvalidResidueError> {
match self {
CheckedHrpstringError::Checksum(ref e) => e.residue_error(),
_ => None,
}
}
}

#[cfg(feature = "alloc")]
impl CorrectableError for crate::segwit::DecodeError {
fn residue_error(&self) -> Option<&InvalidResidueError> { self.0.residue_error() }
}

#[cfg(feature = "alloc")]
impl CorrectableError for DecodeError {
fn residue_error(&self) -> Option<&InvalidResidueError> {
match self {
DecodeError::Checksum(ref e) => e.residue_error(),
_ => None,
}
}
}
75 changes: 67 additions & 8 deletions src/primitives/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::Polynomial;
use crate::{Bech32, Bech32m};

/// Separator between the hrp and payload (as defined by BIP-173).
Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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,
}
Expand All @@ -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"),
}
}
Expand All @@ -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<Fe32>,
target: Polynomial<Fe32>,
}

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<F: PackedFe32>(residue: F, target_residue: F) -> Self {
Self {
actual: Polynomial::from_residue(residue),
target: Polynomial::from_residue(target_residue),
}
}

/// 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 == Polynomial::from_residue(Bech32::TARGET_RESIDUE)
}
}

#[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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -1117,7 +1167,7 @@ mod tests {
.expect("string parses correctly")
.validate_checksum::<Bech32>()
.unwrap_err();
assert_eq!(err, InvalidResidue);
assert!(matches!(err, InvalidResidue(..)));
}

#[test]
Expand Down Expand Up @@ -1178,7 +1228,7 @@ mod tests {
.unwrap()
.validate_checksum::<Bech32m>()
.unwrap_err();
assert_eq!(err, InvalidResidue);
assert!(matches!(err, InvalidResidue(..)));
}

#[test]
Expand Down Expand Up @@ -1212,6 +1262,15 @@ mod tests {
}
}

#[test]
#[allow(clippy::assertions_on_constants)]
fn constant_sanity() {
assert!(
crate::primitives::correction::NO_ALLOC_MAX_LENGTH > 6,
"The NO_ALLOC_MAX_LENGTH constant must be > 6. See its documentation for why.",
);
}

macro_rules! check_invalid_segwit_addresses {
($($test_name:ident, $reason:literal, $address:literal);* $(;)?) => {
$(
Expand Down
Loading

0 comments on commit 3f98190

Please sign in to comment.