From f2ec5822eee7f04d8397ae3dd9b3c20c8a310be8 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 31 Mar 2024 16:51:31 +0000 Subject: [PATCH 1/8] primitives: introduce FieldVec type This will allow `Polynomial` to work without an allocator (at least for "small" checksums. Here a "small" checksum is one of length at most 6 (which covers bech32 and bech32m). The descriptor checksum (8 characters), codex32 (13 characters) and "long codex32" (15 characters) will not work with no-alloc. I would like to fix this but it results in large types, especially a large InvalidResidueError type (see last commit of this PR), so probably it will need to be feature-gated or something. For now we just punt on it. This PR introduces the `primitives::correction` module but only puts a single constant into it for now. --- src/primitives/correction.rs | 35 ++++ src/primitives/fieldvec.rs | 366 +++++++++++++++++++++++++++++++++++ src/primitives/mod.rs | 2 + 3 files changed, 403 insertions(+) create mode 100644 src/primitives/correction.rs create mode 100644 src/primitives/fieldvec.rs diff --git a/src/primitives/correction.rs b/src/primitives/correction.rs new file mode 100644 index 00000000..2d551831 --- /dev/null +++ b/src/primitives/correction.rs @@ -0,0 +1,35 @@ +// 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. +//! + +/// **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; diff --git a/src/primitives/fieldvec.rs b/src/primitives/fieldvec.rs new file mode 100644 index 00000000..98876ec3 --- /dev/null +++ b/src/primitives/fieldvec.rs @@ -0,0 +1,366 @@ +// SPDX-License-Identifier: MIT + +//! Field Element Vector +//! +//! Provides a nostd-compatible vector for storing field elements. This has +//! an ad-hoc API and some limitations and should *not* be exposed in the +//! public API. +//! +//! Its primary purpose is to be a backing for the `Polynomial` type. The +//! idea is that `FieldVec` will act like a vector of arbitrary objects, +//! but manage alloc/no-alloc weirdness, while `Polynomial` defines all +//! the arithmetic operations without worrying about these things. +//! +//! This is very similar to the `ArrayVec` type from the `arrayvec` crate, +//! with two major differences: +//! +//! * In the case that an allocator is available, switches to being unbounded. +//! * It is specialized to field elements, and provides a number of utility +//! functions and constructors specifically for that case. +//! +//! Because it stores field elements, and fields always have a zero element, +//! we can avoid working with uninitialized memory by setting "undefined" +//! values to zero. There is theoretically a performance cost here, but +//! given that our arrays are limited in size to low tens of elements, it +//! is unlikely for this to be measurable. +//! +//! The purpose of this vector is to be a backing for the various (reduced) +//! polynomials we encounter when processing BCH codes. These polynomials +//! have degree <= the degree of the generator polynomial, whose degree +//! in turn is a small integer (6 for bech32, 8 for descriptors, and 13 +//! or 15 for codex32, as examples). +//! +//! An example of a reduced polynomial is the residue computed when +//! validating checksums. Typically, validating a BCH checksum just means +//! computing this residue, comparing it to a target value, and throwing +//! it away. However, we may want to keep the value in two cases: +//! +//! 1. When doing error correction, the residue value encodes the location +//! and values of the errors (assuming there are not too many). +//! 2. When distinguishing between bech32 and bech32m, which differ only +//! in their target residues, we may want to know the computed residue +//! so we can do a manual comparison against both values. +//! +//! Despite these arrays being very small for all checksums we are aware +//! of being practically used, in principle they can be any size, and we +//! don't want to limit our users artificially. We cannot have arbitrary +//! sized objects without an allocator, so we split the difference by +//! using a fixed-size array, and when the user tries to go beyond this, +//! panicking if an allocator is unavailable. +//! +//! Users of this type should take care not to expose this panic to users. +//! This shouldn't be too hard, because this type is internal to the library +//! which has two use cases: +//! +//! 1. Distinguishing bech32 and bech32m residues (within the limit). +//! 2. Doing error correction (should have a small top-level API and easy +//! to early-detect things outside the limit and return an error). +//! + +#[cfg(all(feature = "alloc", not(feature = "std")))] +use alloc::vec::Vec; +use core::{iter, ops, slice}; + +use super::Field; +use crate::primitives::correction::NO_ALLOC_MAX_LENGTH; + +/// A vector of field elements. +/// +/// Parameterized by the field type `F` which can be anything, but for most methods +/// to be enabled needs `Default` and `Clone`. (Both are implied by `Field`.) +#[derive(PartialEq, Eq, Clone, Debug, Hash)] +pub struct FieldVec { + inner_a: [F; NO_ALLOC_MAX_LENGTH], + len: usize, + #[cfg(feature = "alloc")] + inner_v: Vec, +} + +impl FieldVec { + /// Determines whether the residue is representable, given the current + /// compilation context. + /// + /// For small enough residues (which includes, in particular, bech32 and + /// bech32m), will always return true. Otherwise, returns true iff the + /// **alloc** feature is turned on. + /// + /// If you just want to panic when this is false, use `assert_has_data`. + #[inline] + pub fn has_data(&self) -> bool { self.len <= NO_ALLOC_MAX_LENGTH || cfg!(feature = "alloc") } + + /// Panics if [`Self::has_data`] is false, with an informative panic message. + #[inline] + pub fn assert_has_data(&self) { + assert!( + self.has_data(), + "checksums of {} characters (more than {}) require the `alloc` feature of `bech32` to be enabled", + self.len, + NO_ALLOC_MAX_LENGTH, + ); + } + + /// Number of stored field elements + #[inline] + pub fn len(&self) -> usize { self.len } + + /// Whether the vector is empty + #[inline] + pub fn is_empty(&self) -> bool { self.len == 0 } + + /// Returns an immutable iterator over the elements in the vector. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false. + pub fn iter(&self) -> slice::Iter { + if self.len > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return self.inner_v[..self.len].iter(); + } + self.inner_a[..self.len].iter() + } + + /// Returns a mutable iterator over the elements in the vector. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false. + pub fn iter_mut(&mut self) -> slice::IterMut { + if self.len > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return self.inner_v[..self.len].iter_mut(); + } + self.inner_a[..self.len].iter_mut() + } +} + +impl FieldVec { + /// Constructor from the powers of an element, from 0 upward. + /// + /// If the **alloc** feature is disabled and `n` exceeds the maximum size for + /// a no-alloc vector, this method will return a "dead" vector which will + /// panic if it is used. Users should use [`Self::has_data`] to determine + /// whether this is the case. + #[inline] + pub fn from_powers(elem: F, n: usize) -> Self { + iter::successors(Some(F::ONE), |gen| Some(elem.clone() * gen)).take(n + 1).collect() + } + + /// Multiply the elements of two vectors together, pointwise. + /// + /// # Panics + /// + /// Panics if the vectors are different lengths, or if [`Self::has_data`] is + /// false for either vector. + #[inline] + pub fn mul_assign_pointwise(&mut self, other: &Self) { + assert_eq!(self.len, other.len, "cannot add vectors of different lengths"); + for (i, fe) in self.iter_mut().enumerate() { + *fe *= &other[i]; + } + } + + /// Multiply the elements of two vectors together, pointwise. + /// + /// # Panics + /// + /// Panics if the vectors are different lengths, or if [`Self::has_data`] is + /// false for either vector. + #[inline] + pub fn mul_pointwise(mut self, other: &Self) -> Self { + self.mul_assign_pointwise(other); + self + } + + #[inline] + /// Lifts a vector of field elements to a vector of elements in an extension + /// field, via the inclusion map. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false. + pub fn lift>(&self) -> FieldVec { + self.iter().cloned().map(E::from).collect() + } +} + +impl iter::FromIterator for FieldVec { + /// Constructor from an iterator of elements. + /// + /// If the **alloc** feature is disabled and `n` exceeds the maximum size for + /// a no-alloc vector, this method will return a "dead" vector which will + /// panic if it is used. Users should use [`Self::has_data`] to determine + /// whether this is the case. + fn from_iter(iter: I) -> Self + where + I: IntoIterator, + { + let mut iter = iter.into_iter(); + // This goofy map construction is needed because we cannot use the + // `[F::default(); N]` syntax without adding a `Copy` bound to `F`. + // After Rust 1.63 we will be able to use array::from_fn. + let mut inner_a = [(); NO_ALLOC_MAX_LENGTH].map(|_| F::default()); + let mut len = 0; + for elem in iter.by_ref().take(NO_ALLOC_MAX_LENGTH) { + inner_a[len] = elem; + len += 1; + } + #[allow(unused_variables)] + if let Some(next) = iter.next() { + #[cfg(feature = "alloc")] + { + let mut inner_v = inner_a.to_vec(); + inner_v.push(next); + inner_v.extend(iter); + Self { inner_a, len: inner_v.len(), inner_v } + } + #[cfg(not(feature = "alloc"))] + { + // Create a dead FieldVec that will fail Self::has_data. + // It is still useful to be able to construct these, in + // order to populate the InvalidResidueError type. + // Accessors on that type must check its validity before + // using the vector. + Self { len: inner_a.len() + 1 + iter.count(), inner_a } + } + } else { + Self { + inner_a, + len, + #[cfg(feature = "alloc")] + inner_v: Vec::default(), + } + } + } +} + +impl<'a, F> IntoIterator for &'a FieldVec { + type Item = &'a F; + type IntoIter = slice::Iter<'a, F>; + #[inline] + fn into_iter(self) -> Self::IntoIter { self.iter() } +} + +impl<'a, F> IntoIterator for &'a mut FieldVec { + type Item = &'a mut F; + type IntoIter = slice::IterMut<'a, F>; + #[inline] + fn into_iter(self) -> Self::IntoIter { self.iter_mut() } +} + +impl ops::Index for FieldVec { + type Output = F; + fn index(&self, index: usize) -> &F { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &self.inner_v[..self.len][index]; + } + &self.inner_a[..self.len][index] + } +} + +impl ops::Index> for FieldVec { + type Output = [F]; + fn index(&self, index: ops::Range) -> &[F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &self.inner_v[..self.len][index]; + } + &self.inner_a[..self.len][index] + } +} + +impl ops::Index> for FieldVec { + type Output = [F]; + fn index(&self, index: ops::RangeFrom) -> &[F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &self.inner_v[..self.len][index]; + } + &self.inner_a[..self.len][index] + } +} + +impl ops::Index> for FieldVec { + type Output = [F]; + fn index(&self, index: ops::RangeTo) -> &[F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &self.inner_v[..self.len][index]; + } + &self.inner_a[..self.len][index] + } +} + +impl ops::Index for FieldVec { + type Output = [F]; + fn index(&self, index: ops::RangeFull) -> &[F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &self.inner_v[..self.len][index]; + } + &self.inner_a[..self.len][index] + } +} + +impl ops::IndexMut for FieldVec { + fn index_mut(&mut self, index: usize) -> &mut F { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &mut self.inner_v[..self.len][index]; + } + &mut self.inner_a[..self.len][index] + } +} + +impl ops::IndexMut> for FieldVec { + fn index_mut(&mut self, index: ops::Range) -> &mut [F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &mut self.inner_v[..self.len][index]; + } + &mut self.inner_a[..self.len][index] + } +} + +impl ops::IndexMut> for FieldVec { + fn index_mut(&mut self, index: ops::RangeFrom) -> &mut [F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &mut self.inner_v[..self.len][index]; + } + &mut self.inner_a[..self.len][index] + } +} + +impl ops::IndexMut> for FieldVec { + fn index_mut(&mut self, index: ops::RangeTo) -> &mut [F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &mut self.inner_v[..self.len][index]; + } + &mut self.inner_a[..self.len][index] + } +} + +impl ops::IndexMut for FieldVec { + fn index_mut(&mut self, index: ops::RangeFull) -> &mut [F] { + if self.len() > NO_ALLOC_MAX_LENGTH { + self.assert_has_data(); + #[cfg(feature = "alloc")] + return &mut self.inner_v[..self.len][index]; + } + &mut self.inner_a[..self.len][index] + } +} diff --git a/src/primitives/mod.rs b/src/primitives/mod.rs index fcc82023..026dc809 100644 --- a/src/primitives/mod.rs +++ b/src/primitives/mod.rs @@ -3,9 +3,11 @@ //! Provides the internal nuts and bolts that enable bech32 encoding/decoding. pub mod checksum; +pub mod correction; pub mod decode; pub mod encode; mod field; +mod fieldvec; pub mod gf32; pub mod gf32_ext; pub mod hrp; From 690fc7df1cbffceeabb00516d80fd555556bd334 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 18 Sep 2024 15:41:18 +0000 Subject: [PATCH 2/8] polynomial: use FieldVec in Polynomial We can now use `PrintImpl` even without an allocator. This is of limited use of course, but it paves the ground for use to do error correction without an allocator, which is interesting. --- src/primitives/checksum.rs | 33 ++-- src/primitives/fieldvec.rs | 28 ++++ src/primitives/mod.rs | 3 +- src/primitives/polynomial.rs | 302 ++++++++++++++++++++++------------- 4 files changed, 225 insertions(+), 141 deletions(-) diff --git a/src/primitives/checksum.rs b/src/primitives/checksum.rs index 50bebb75..7b3729e4 100644 --- a/src/primitives/checksum.rs +++ b/src/primitives/checksum.rs @@ -4,20 +4,14 @@ //! //! [BCH]: -#[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. /// @@ -169,10 +163,9 @@ 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, target: &'a [Fe32], bit_len: usize, hex_width: usize, @@ -180,7 +173,6 @@ pub struct PrintImpl<'a, ExtField = Fe1024> { phantom: PhantomData, } -#[cfg(feature = "alloc")] impl<'a, ExtField> PrintImpl<'a, ExtField> { /// Constructor for an object to print an impl-block for the [`Checksum`] trait. /// @@ -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, @@ -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, { 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::(); 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 ")?; @@ -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)?; diff --git a/src/primitives/fieldvec.rs b/src/primitives/fieldvec.rs index 98876ec3..3122df87 100644 --- a/src/primitives/fieldvec.rs +++ b/src/primitives/fieldvec.rs @@ -186,6 +186,34 @@ impl FieldVec { } } +impl FieldVec { + /// Pushes an item onto the end of the vector. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false, or if it would be false after the push. + pub fn push(&mut self, item: F) { + self.len += 1; + self.assert_has_data(); + + #[cfg(not(feature = "alloc"))] + { + self.inner_a[self.len - 1] = item; + } + + #[cfg(feature = "alloc")] + if self.len < NO_ALLOC_MAX_LENGTH + 1 { + self.inner_a[self.len - 1] = item; + } else { + if self.len == NO_ALLOC_MAX_LENGTH + 1 { + let inner_a = core::mem::take(&mut self.inner_a); + self.inner_v = inner_a.into(); + } + self.inner_v.push(item); + } + } +} + impl iter::FromIterator for FieldVec { /// Constructor from an iterator of elements. /// diff --git a/src/primitives/mod.rs b/src/primitives/mod.rs index 026dc809..a4d803fa 100644 --- a/src/primitives/mod.rs +++ b/src/primitives/mod.rs @@ -12,14 +12,13 @@ pub mod gf32; pub mod gf32_ext; pub mod hrp; pub mod iter; -#[cfg(feature = "alloc")] mod polynomial; pub mod segwit; use checksum::{Checksum, PackedNull}; use field::impl_ops_for_fe; pub use field::{Bech32Field, ExtensionField, Field}; -#[cfg(feature = "alloc")] +use fieldvec::FieldVec; use polynomial::Polynomial; use crate::{Fe1024, Fe32}; diff --git a/src/primitives/polynomial.rs b/src/primitives/polynomial.rs index 3ef65b4f..9f041433 100644 --- a/src/primitives/polynomial.rs +++ b/src/primitives/polynomial.rs @@ -2,27 +2,33 @@ //! Polynomials over Finite Fields -#[cfg(all(feature = "alloc", not(feature = "std"), not(test)))] -use alloc::vec::Vec; use core::{iter, ops}; -use super::Field; +use super::{ExtensionField, Field, FieldVec}; /// A polynomial over some field. #[derive(PartialEq, Eq, Clone, Debug, Hash)] pub struct Polynomial { - inner: Vec, -} - -impl Polynomial { - /// Constructor for a polynomial from a vector of coefficients. - /// - /// These coefficients are in "little endian" order. That is, the ith - /// coefficient is the one multiplied by x^i. - pub fn new(f: Vec) -> Self { Self { inner: f } } + /// The coefficients of the polynomial, in "little-endian" order. + /// That is the constant term is at index 0. + inner: FieldVec, } impl Polynomial { + /// Provides access to the underlying [`FieldVec`]. + pub fn into_inner(self) -> FieldVec { self.inner } + + /// Provides access to the underlying [`FieldVec`]. + pub fn as_inner(&self) -> &FieldVec { &self.inner } + + /// Constructs a polynomial from a slice of field elements, prepending + /// a 1 value to produce a monic polynomial. + pub fn with_monic_leading_term(coeffs: &[F]) -> Self { + let mut inner: FieldVec<_> = coeffs.iter().rev().cloned().collect(); + inner.push(F::ONE); + Polynomial { inner } + } + /// The degree of the polynomial. /// /// For constants it will return zero, including for the constant zero. @@ -44,63 +50,40 @@ impl Polynomial { } F::ZERO } -} -impl Polynomial { - /// Finds all roots of the polynomial in the given field, in - /// no particular order. + /// Whether 0 is a root of the polynomial. Equivalently, whether `x` is a + /// factor of the polynomial. + pub fn zero_is_root(&self) -> bool { self.inner.is_empty() || self.leading_term() == F::ZERO } + + /// An iterator over the roots of the residue, when interpreted as a polynomial. /// - /// Does not consider multiplicity; it assumes there are no - /// repeated roots. (FIXME we really ought to do so, and - /// definitely should before exposing this function in the - /// public API.) + /// Takes a base element `base`. The roots of the residue will be yielded as + /// nonnegative integers between 0 and 1 less than the order of the base, + /// inclusive. If `base` is a primitive element of the extension field, then + /// all distinct roots (in the extension field) will be found. /// - /// If the polynomial does not split, then the returned vector - /// will have length strictly less than [`Self::degree`]. If - /// the polynomial _does_ split then the length will be equal. + /// Iterates via Chien search, which is a form of brute force. Internally it + /// will do as many iterations as the multiplicative order of `base`, regardless + /// of how many roots are actually found. /// - /// For constants, will return vec![0] for the constant 0 and the - /// empty vector for any other constant. Probably the caller wants - /// to check if they have a constant and special-case this. - pub fn find_distinct_roots(&self) -> Vec { - // Implements Chien search - - let mut ret = Vec::with_capacity(self.degree()); - // Check if zero is a root - if self.inner.is_empty() || self.leading_term() == F::ZERO { - ret.push(F::ZERO); - } - // Special-case constants, which have 0 as a root iff they are the constant 0. - if self.degree() == 1 { - return ret; - // from here on out we know self.inner[0] won't panic - } - - // Vector of [1, gen, gen^2, ...] up to the degree d. - debug_assert_eq!(F::GENERATOR.multiplicative_order(), F::MULTIPLICATIVE_ORDER); - let gen_power = iter::successors(Some(F::ONE), |gen| Some(F::GENERATOR * gen)) - .take(self.degree() + 1) - .collect::>(); - - // We special-cased 0 above. So now we can check every nonzero element - // to see if it is a root. We brute-force this using Chein's algorithm, - // which exploits the fact that we can go from f(alpha^i) to f(alpha^{i+1}) - // pretty efficiently. So iterate through all the powers of the generator - // in this way. - let mut cand = F::ONE; - let mut eval = self.clone(); - for _ in 0..F::MULTIPLICATIVE_ORDER { - if eval.inner.iter().sum::() == F::ZERO { - ret.push(cand.clone()); - } + /// Only roots which are a power of `base` are returned, so if `base` is *not* + /// a primitive element then not all roots may be returned. + /// + /// This will **not** return 0 as a root under any circumstances. To check + /// whether zero is a root of the polynomial, run [`Self::zero_is_root`]. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false. + pub fn find_nonzero_distinct_roots>(&self, base: E) -> RootIter { + self.inner.assert_has_data(); - for (i, gen_power) in gen_power.iter().enumerate() { - eval.inner[i] *= gen_power; - } - cand *= F::GENERATOR; + RootIter { + idx: 0, + max_idx: base.multiplicative_order(), + base_powers: FieldVec::from_powers(base, self.inner.len() - 1), + polynomial: self.inner.lift(), } - - ret } /// Given a BCH generator polynomial, find an element alpha that maximizes the @@ -132,11 +115,14 @@ impl Polynomial { /// order, and therefore may return different values on consecutive runs. If /// there is a particular "elegant" value you are looking for, it may be /// worthwhile to run the function multiple times. - pub fn bch_generator_primitive_element(&self) -> (F, usize, ops::RangeInclusive) { - let roots = self.find_distinct_roots(); - debug_assert!(roots.len() <= self.degree(),); + pub fn bch_generator_primitive_element>( + &self, + ) -> (E, usize, ops::RangeInclusive) { + let roots: FieldVec = self.find_nonzero_distinct_roots(E::GENERATOR).collect(); + debug_assert!(roots.len() <= self.degree()); + // debug_assert!(roots.is_sorted()); // nightly only API assert_eq!( - self.degree(), + self.degree() + usize::from(self.zero_is_root()), roots.len(), "Found {} roots ({:?}) for a polynomial of degree {}; polynomial appears not to split.", roots.len(), @@ -144,63 +130,122 @@ impl Polynomial { self.degree(), ); - // Brute-force (worst case n^3 in the length of the polynomial) the longest + // Brute-force (worst case n^3*log(n) in the length of the polynomial) the longest // geometric series within the set of roots. The common ratio between these // roots will be our primitive element. // // We also learn the length of the series and the first root in the series. - let mut max_length = 0; - let mut max_start = F::ZERO; - let mut max_ratio = F::ZERO; - for r1 in &roots { - for r2 in &roots { - if r1 == r2 { + + let mut max_length = 0; // length of the max-length geometric series + let mut max_start = 0; // i such that the max-length series starts with gen^i + let mut max_ratio = 0; // i such that the ratio between terms is gen^i + + for i in 0..roots.len() { + for j in 0..roots.len() { + if i == j { continue; } - let ratio = r2.clone() / r1; + let r1 = roots[i]; + let mut r2 = roots[j]; - let mut len = 1; - let mut elem = r1.clone(); - while roots.contains(&(elem.clone() * &ratio)) { + let ratio = (E::MULTIPLICATIVE_ORDER + r2 - r1) % E::MULTIPLICATIVE_ORDER; + // To avoid needing alloc, we binary-search the slice rather than + // putting the roots into a HashSet or something so we can O(1) + // search them. In practice this doesn't matter because we have + // such a small number of roots (it may actually be faster than + // using a hashset) and because the root-finding takes such a + // long time that noboby can use this method in a loop anyway. + let mut len = 2; + while let Ok(k) = roots[..].binary_search(&((r2 + ratio) % E::MULTIPLICATIVE_ORDER)) + { len += 1; - elem *= ∶ + r2 = roots[k]; } + if len > max_length { max_length = len; - max_start = r2.clone(); + max_start = roots[i]; max_ratio = ratio; } } } - // We have the primitive element (max_ratio) and the first element in the - // series with that ratio (max_start). To get the actual exponents of the - // series, we need i such that max_start = max_ratio^i. + let prim_elem = E::GENERATOR.powi(max_ratio as i64); + let code_len = prim_elem.multiplicative_order(); + // We have the primitive element (prim_elem) and the first element in the + // series with that ratio (GENERATOR ** max_start). But we want to know + // an exponent i such that GENERATOR ** max_start = prim_elem ** i. // // It may occur that no such i exists, if the entire series is in a coset - // of the group generated by max_ratio. In *theory* this means that we + // of the group generated by prim_elem. In *theory* this means that we // should go back and find the second-longest geometric series and try // that, because for a real-life BCH code this situation indicates that // something is wrong and we should just panic. - let code_len = max_ratio.multiplicative_order(); - + let initial_elem = E::GENERATOR.powi(max_start as i64); let mut min_index = None; - let mut base = F::ONE; + let mut base = E::ONE; for i in 0..code_len { - base *= &max_ratio; - if base == max_start { + if base == initial_elem { min_index = Some(i); } + base *= &prim_elem; } - let min_index = match min_index { Some(idx) => idx, - None => panic!("Found geometric series within roots starting from {} (ratio {} length {}), but this series does not consist of powers of any generator.", max_start, max_ratio, max_length), + None => panic!("Found geometric series within roots starting from {} (ratio {} length {}), but the series does not consist of powers of the ratio.", initial_elem, prim_elem, max_length), }; // We write `a..=b - 1` instead of `a..b` because RangeInclusive is actually // a different type than Range, so the two syntaxes are not equivalent here. - (max_ratio, code_len, min_index..=min_index + max_length - 1) + (prim_elem, code_len, min_index..=min_index + max_length - 1) + } +} + +impl iter::FromIterator for Polynomial { + #[inline] + fn from_iter(iter: I) -> Self + where + I: IntoIterator, + { + Polynomial { inner: FieldVec::from_iter(iter) } + } +} + +impl From> for Polynomial { + fn from(inner: FieldVec) -> Self { Self { inner } } +} + +/// An iterator over the roots of a polynomial. +/// +/// This iterator is constructed by the [`Polynomial::find_nonzero_distinct_roots`] +/// method, which takes a field element as a base. The roots of the +/// polynomial are yielded as exponents of the base. See the documentation +/// of that method for more information. +pub struct RootIter { + idx: usize, + max_idx: usize, + base_powers: FieldVec, + polynomial: FieldVec, +} + +impl Iterator for RootIter { + type Item = usize; + fn next(&mut self) -> Option { + // A zero-length polynomial has no nonzero roots. Special-case this + // so that we can freely index the first coefficient of the polynomial. + if self.polynomial.is_empty() { + return None; + } + + while self.idx < self.max_idx { + let sum = self.polynomial.iter().sum::(); + self.polynomial.mul_assign_pointwise(&self.base_powers); + self.idx += 1; + if sum == F::ZERO { + return Some(self.idx - 1); + } + } + None } } @@ -210,33 +255,19 @@ mod tests { use crate::{Fe1024, Fe32}; #[test] + #[cfg(feature = "alloc")] fn roots() { - let bip93_poly = Polynomial::::new( - [ - Fe32::S, - Fe32::S, - Fe32::C, - Fe32::M, - Fe32::L, - Fe32::E, - Fe32::E, - Fe32::E, - Fe32::Q, - Fe32::G, - Fe32::_3, - Fe32::M, - Fe32::E, - Fe32::P, - ] - .iter() - .copied() - .map(Fe1024::from) - .collect(), - ); + let bip93_poly: Polynomial = { + use Fe32 as F; + [F::S, F::S, F::C, F::M, F::L, F::E, F::E, F::E, F::Q, F::G, F::_3, F::M, F::E, F::P] + } + .iter() + .copied() + .collect(); assert_eq!(bip93_poly.degree(), 13); - let (elem, order, root_indices) = bip93_poly.bch_generator_primitive_element(); + let (elem, order, root_indices) = bip93_poly.bch_generator_primitive_element::(); // Basically, only the order and the length of the `root_indices` range are // guaranteed to be consistent across runs. There will be two possible ranges, // a "low" one (in this case 9..16) and a "high one" (77..84) whose generator @@ -264,6 +295,11 @@ mod tests { Fe1024::new([Fe32::G, Fe32::G]).powi(77), ); // ...and these ones are actual unit tests. + // + // (In the actual implementation, which is now deterministic, only one of + // these if branches will ever be taken (I think the first). But all of them + // are algebraically valid and we reserve the right to change which one we + // return.) if elem == Fe1024::new([Fe32::_9, Fe32::_9]) { assert_eq!(root_indices, 9..=16); } else if elem == Fe1024::new([Fe32::Q, Fe32::G]) { @@ -276,4 +312,40 @@ mod tests { panic!("Unexpected generator {}", elem); } } + + #[test] + #[cfg(not(feature = "alloc"))] + fn roots() { + // Exactly the same test as above, but for bech32 + let bech32_poly: Polynomial = { + use Fe32 as F; + [F::J, F::A, F::_4, F::_5, F::K, F::A, F::P] + } + .iter() + .copied() + .collect(); + + assert_eq!(bech32_poly.degree(), 6); + + let (elem, order, root_indices) = bech32_poly.bch_generator_primitive_element::(); + // As above, only the order and the length of the `root_indices` range are + // guaranteed to be consistent across runs. There will be two possible ranges, + // a "low" one (in this case 24..27) and a "high one" (997..1000) whose generator + // is the multiplicative inverse of the small one. These correspond to the + // fact that for any order-1023 element, e^x = (e^-1)^(1023 - x). + assert_eq!(order, 1023); + // This assertion just illustrate the above comment... + assert_eq!( + Fe1024::new([Fe32::P, Fe32::X]).multiplicative_inverse(), + Fe1024::new([Fe32::_7, Fe32::F]), + ); + // ...and these ones are actual unit tests. + if elem == Fe1024::new([Fe32::P, Fe32::X]) { + assert_eq!(root_indices, 24..=26); + } else if elem == Fe1024::new([Fe32::_7, Fe32::F]) { + assert_eq!(root_indices, 997..=999); + } else { + panic!("Unexpected generator {}", elem); + } + } } From b76adbec0c55d55b5d0abcf551928217b3d6cd60 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 21 Sep 2024 17:04:16 +0000 Subject: [PATCH 3/8] polynomial: add some extra functionality enabled by FieldVec --- src/lib.rs | 2 + src/primitives/fieldvec.rs | 37 +++++++++++++- src/primitives/polynomial.rs | 97 +++++++++++++++++++++++++++++++++++- 3 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2cc18a99..561e6e74 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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; diff --git a/src/primitives/fieldvec.rs b/src/primitives/fieldvec.rs index 3122df87..83ee902c 100644 --- a/src/primitives/fieldvec.rs +++ b/src/primitives/fieldvec.rs @@ -59,7 +59,7 @@ #[cfg(all(feature = "alloc", not(feature = "std")))] use alloc::vec::Vec; -use core::{iter, ops, slice}; +use core::{iter, mem, ops, slice}; use super::Field; use crate::primitives::correction::NO_ALLOC_MAX_LENGTH; @@ -206,12 +206,45 @@ impl FieldVec { self.inner_a[self.len - 1] = item; } else { if self.len == NO_ALLOC_MAX_LENGTH + 1 { - let inner_a = core::mem::take(&mut self.inner_a); + let inner_a = mem::take(&mut self.inner_a); self.inner_v = inner_a.into(); } self.inner_v.push(item); } } + + /// Pops an item off the end of the vector. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false. + pub fn pop(&mut self) -> Option { + self.assert_has_data(); + if self.len == 0 { + return None; + } + + self.len -= 1; + #[cfg(not(feature = "alloc"))] + { + Some(mem::take(&mut self.inner_a[self.len])) + } + + #[cfg(feature = "alloc")] + if self.len < NO_ALLOC_MAX_LENGTH { + Some(mem::take(&mut self.inner_a[self.len])) + } else { + use core::convert::TryFrom; + + let ret = self.inner_v.pop(); + let inner_v = mem::take(&mut self.inner_v); + match <[F; NO_ALLOC_MAX_LENGTH]>::try_from(inner_v) { + Ok(arr) => self.inner_a = arr, + Err(vec) => self.inner_v = vec, + } + ret + } + } } impl iter::FromIterator for FieldVec { diff --git a/src/primitives/polynomial.rs b/src/primitives/polynomial.rs index 9f041433..9ffd594e 100644 --- a/src/primitives/polynomial.rs +++ b/src/primitives/polynomial.rs @@ -2,7 +2,7 @@ //! Polynomials over Finite Fields -use core::{iter, ops}; +use core::{fmt, iter, ops, slice}; use super::{ExtensionField, Field, FieldVec}; @@ -15,6 +15,13 @@ pub struct Polynomial { } impl Polynomial { + /// Determines whether the residue is representable, given the current + /// compilation context. + pub fn has_data(&self) -> bool { self.inner.has_data() } + + /// Panics if [`Self::has_data`] is false, with an informative panic message. + pub fn assert_has_data(&self) { self.inner.assert_has_data() } + /// Provides access to the underlying [`FieldVec`]. pub fn into_inner(self) -> FieldVec { self.inner } @@ -39,6 +46,18 @@ impl Polynomial { degree_without_leading_zeros - leading_zeros } + /// An iterator over the coefficients of the polynomial. + /// + /// Yields value in "little endian" order; that is, the constant term is returned first. + /// + /// # Panics + /// + /// Panics if [`Self::has_data`] is false. + pub fn iter(&self) -> slice::Iter { + self.assert_has_data(); + self.inner.iter() + } + /// The leading term of the polynomial. /// /// For the constant 0 polynomial, will return 0. @@ -55,7 +74,15 @@ impl Polynomial { /// factor of the polynomial. pub fn zero_is_root(&self) -> bool { self.inner.is_empty() || self.leading_term() == F::ZERO } - /// An iterator over the roots of the residue, when interpreted as a polynomial. + /// Helper function to add leading 0 terms until the polynomial has a specified + /// length. + fn zero_pad_up_to(&mut self, len: usize) { + while self.inner.len() < len { + self.inner.push(F::default()); + } + } + + /// An iterator over the roots of the polynomial. /// /// Takes a base element `base`. The roots of the residue will be yielded as /// nonnegative integers between 0 and 1 less than the order of the base, @@ -201,6 +228,19 @@ impl Polynomial { } } +impl fmt::Display for Polynomial { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if self.has_data() { + for fe in self.iter() { + write!(f, "{}", fe)?; + } + Ok(()) + } else { + f.write_str("") + } + } +} + impl iter::FromIterator for Polynomial { #[inline] fn from_iter(iter: I) -> Self @@ -215,6 +255,59 @@ impl From> for Polynomial { fn from(inner: FieldVec) -> Self { Self { inner } } } +impl ops::Add<&Polynomial> for Polynomial { + type Output = Polynomial; + + fn add(mut self, other: &Polynomial) -> Polynomial { + self += other; + self + } +} + +impl ops::Add> for Polynomial { + type Output = Polynomial; + fn add(self, other: Polynomial) -> Polynomial { self + &other } +} + +impl ops::Sub<&Polynomial> for Polynomial { + type Output = Polynomial; + fn sub(mut self, other: &Polynomial) -> Polynomial { + self -= other; + self + } +} + +impl ops::Sub> for Polynomial { + type Output = Polynomial; + fn sub(self, other: Polynomial) -> Polynomial { self - &other } +} + +impl ops::AddAssign<&Polynomial> for Polynomial { + fn add_assign(&mut self, other: &Self) { + self.zero_pad_up_to(other.inner.len()); + for i in 0..other.inner.len() { + self.inner[i] += &other.inner[i]; + } + } +} + +impl ops::AddAssign for Polynomial { + fn add_assign(&mut self, other: Polynomial) { *self += &other; } +} + +impl ops::SubAssign<&Polynomial> for Polynomial { + fn sub_assign(&mut self, other: &Polynomial) { + self.zero_pad_up_to(other.inner.len()); + for i in 0..other.inner.len() { + self.inner[i] -= &other.inner[i]; + } + } +} + +impl ops::SubAssign for Polynomial { + fn sub_assign(&mut self, other: Polynomial) { *self -= &other; } +} + /// An iterator over the roots of a polynomial. /// /// This iterator is constructed by the [`Polynomial::find_nonzero_distinct_roots`] From 4c6010eb2565c4020daf325f13376ec06b35fce6 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 15 Sep 2024 18:20:58 +0000 Subject: [PATCH 4/8] 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. --- src/lib.rs | 12 ++++--- src/primitives/decode.rs | 66 ++++++++++++++++++++++++++++++----- src/primitives/fieldvec.rs | 11 +++++- src/primitives/polynomial.rs | 8 +++++ tests/bip_173_test_vectors.rs | 14 ++++---- tests/bip_350_test_vectors.rs | 14 ++++---- 6 files changed, 97 insertions(+), 28 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 561e6e74..731be5f5 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 7752ffda..cd79cdb0 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::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: 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] @@ -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 83ee902c..7aaabc52 100644 --- a/src/primitives/fieldvec.rs +++ b/src/primitives/fieldvec.rs @@ -59,7 +59,7 @@ #[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::Field; use crate::primitives::correction::NO_ALLOC_MAX_LENGTH; @@ -297,6 +297,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/src/primitives/polynomial.rs b/src/primitives/polynomial.rs index 9ffd594e..ee619953 100644 --- a/src/primitives/polynomial.rs +++ b/src/primitives/polynomial.rs @@ -4,7 +4,9 @@ use core::{fmt, iter, ops, slice}; +use super::checksum::PackedFe32; use super::{ExtensionField, Field, FieldVec}; +use crate::Fe32; /// A polynomial over some field. #[derive(PartialEq, Eq, Clone, Debug, Hash)] @@ -14,6 +16,12 @@ pub struct Polynomial { inner: FieldVec, } +impl Polynomial { + pub fn from_residue(residue: R) -> Self { + (0..R::WIDTH).rev().map(|i| Fe32(residue.unpack(i))).collect() + } +} + impl Polynomial { /// Determines whether the residue is representable, given the current /// compilation context. diff --git a/tests/bip_173_test_vectors.rs b/tests/bip_173_test_vectors.rs index 74868f1d..85e97154 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 546beaab..6e310d9b 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(..))); } )* } From d08da463ea1be435a66a9fdaa04c5ae4e2a87a4b Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 19 Sep 2024 14:05:31 +0000 Subject: [PATCH 5/8] correction: introduce CorrectableError trait This is just a helper trait to describe all the errors in the library from which we can get an "invalid residue". The residues contain all the information needed for error correction. --- src/lib.rs | 1 + src/primitives/correction.rs | 71 ++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 731be5f5..64969b4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -166,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, diff --git a/src/primitives/correction.rs b/src/primitives/correction.rs index 2d551831..7918ac8a 100644 --- a/src/primitives/correction.rs +++ b/src/primitives/correction.rs @@ -6,6 +6,12 @@ //! 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. /// @@ -33,3 +39,68 @@ // 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, + } + } +} From 0e3d954ffe8e8112baebc28c854fff78facd3557 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 21 Sep 2024 18:16:29 +0000 Subject: [PATCH 6/8] polynomial: make Eq/PartialEq take leading zeros into account Two polynomials are equal even if they have a different number of leading zeros. Putting this into its own commit rather than folding it into one of the FieldVec commits, because this bug was present even before the FieldVec stuff. Also remove the `Hash` impl because it's unneeded and we would be expected to keep it in sync with Eq. --- src/primitives/polynomial.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/primitives/polynomial.rs b/src/primitives/polynomial.rs index ee619953..0c60f584 100644 --- a/src/primitives/polynomial.rs +++ b/src/primitives/polynomial.rs @@ -9,19 +9,26 @@ use super::{ExtensionField, Field, FieldVec}; use crate::Fe32; /// A polynomial over some field. -#[derive(PartialEq, Eq, Clone, Debug, Hash)] +#[derive(Clone, Debug)] pub struct Polynomial { /// The coefficients of the polynomial, in "little-endian" order. /// That is the constant term is at index 0. inner: FieldVec, } +impl PartialEq for Polynomial { + fn eq(&self, other: &Self) -> bool { + self.inner[..self.degree()] == other.inner[..other.degree()] + } +} + +impl Eq for Polynomial {} + impl Polynomial { pub fn from_residue(residue: R) -> Self { (0..R::WIDTH).rev().map(|i| Fe32(residue.unpack(i))).collect() } } - impl Polynomial { /// Determines whether the residue is representable, given the current /// compilation context. From dfc29ad26bde9657862ded1b1176bfc64416d10f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 22 Sep 2024 13:32:45 +0000 Subject: [PATCH 7/8] fieldvec: add a bunch of unit tests --- src/primitives/decode.rs | 9 +++ src/primitives/fieldvec.rs | 122 +++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/src/primitives/decode.rs b/src/primitives/decode.rs index cd79cdb0..0cb78b4a 100644 --- a/src/primitives/decode.rs +++ b/src/primitives/decode.rs @@ -1262,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);* $(;)?) => { $( diff --git a/src/primitives/fieldvec.rs b/src/primitives/fieldvec.rs index 7aaabc52..566f6afa 100644 --- a/src/primitives/fieldvec.rs +++ b/src/primitives/fieldvec.rs @@ -434,3 +434,125 @@ impl ops::IndexMut for FieldVec { &mut self.inner_a[..self.len][index] } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Fe1024, Fe32}; + + #[test] + fn push_pop() { + let mut x: FieldVec<_> = (0..NO_ALLOC_MAX_LENGTH).collect(); + let x_1: FieldVec<_> = (0..NO_ALLOC_MAX_LENGTH - 1).collect(); + + assert_eq!(x.len(), NO_ALLOC_MAX_LENGTH); + assert!(!x.is_empty()); + + assert_eq!(x.pop(), Some(NO_ALLOC_MAX_LENGTH - 1)); + assert_eq!(x, x_1); + x.push(NO_ALLOC_MAX_LENGTH - 1); + + let mut y: FieldVec<_> = None.into_iter().collect(); + for i in 0..NO_ALLOC_MAX_LENGTH { + y.push(i); + assert_eq!(y[i], i); + y[i] = i + 1; + assert_eq!(y[i], i + 1); + y[i] -= 1; + } + assert_eq!(x, y); + + for i in (0..NO_ALLOC_MAX_LENGTH).rev() { + assert_eq!(y.pop(), Some(i)); + } + assert_eq!(y.len(), 0); + assert!(y.is_empty()); + } + + #[test] + fn iter_slice() { + let mut x: FieldVec<_> = (0..NO_ALLOC_MAX_LENGTH).collect(); + assert!(x.iter().copied().eq(0..NO_ALLOC_MAX_LENGTH)); + assert!(x[..].iter().copied().eq(0..NO_ALLOC_MAX_LENGTH)); + assert!(x[0..].iter().copied().eq(0..NO_ALLOC_MAX_LENGTH)); + assert!(x[..NO_ALLOC_MAX_LENGTH].iter().copied().eq(0..NO_ALLOC_MAX_LENGTH)); + assert!(x[1..].iter().copied().eq(1..NO_ALLOC_MAX_LENGTH)); + assert!(x[..NO_ALLOC_MAX_LENGTH - 1].iter().copied().eq(0..NO_ALLOC_MAX_LENGTH - 1)); + assert!(x[1..NO_ALLOC_MAX_LENGTH - 1].iter().copied().eq(1..NO_ALLOC_MAX_LENGTH - 1)); + + // mutable slicing + x[..].reverse(); + assert!(x.iter().copied().eq((0..NO_ALLOC_MAX_LENGTH).rev())); + x[..NO_ALLOC_MAX_LENGTH].reverse(); + assert!(x.iter().copied().eq(0..NO_ALLOC_MAX_LENGTH)); + x[0..].reverse(); + assert!(x.iter().copied().eq((0..NO_ALLOC_MAX_LENGTH).rev())); + x[0..NO_ALLOC_MAX_LENGTH].reverse(); + assert!(x.iter().copied().eq(0..NO_ALLOC_MAX_LENGTH)); + + for elem in x.iter_mut() { + *elem += 1; + } + assert!(x.iter().copied().eq(1..NO_ALLOC_MAX_LENGTH + 1)); + } + + #[test] + fn field_ops() { + let qs: FieldVec<_> = FieldVec::from_powers(Fe32::Q, NO_ALLOC_MAX_LENGTH - 1); + let ps: FieldVec<_> = FieldVec::from_powers(Fe32::P, NO_ALLOC_MAX_LENGTH - 1); + let pzr: FieldVec<_> = FieldVec::from_powers(Fe32::Z, 3); + + assert_eq!(qs.len(), NO_ALLOC_MAX_LENGTH); + assert_eq!(ps.len(), NO_ALLOC_MAX_LENGTH); + assert_eq!(pzr.len(), 4); + + let pzr = pzr.lift::(); // should work and be a no-op + + // This is somewhat weird behavior but mathematically reasonable. The + // `from_powers` constructor shouldn't ever be called with 0 as a base. + // If you need a particular different output from this call, feel free + // to change this test....but think twice about what you're doing. + assert!(qs.iter().copied().eq(Some(Fe32::P) + .into_iter() + .chain(iter::repeat(Fe32::Q).take(NO_ALLOC_MAX_LENGTH - 1)))); + // These checks though are correct and unambiguous. + assert!(ps.iter().copied().eq(iter::repeat(Fe32::P).take(NO_ALLOC_MAX_LENGTH))); + assert_eq!(pzr.iter().copied().collect::>(), [Fe32::P, Fe32::Z, Fe32::Y, Fe32::G,]); + + let pow2 = pzr.clone().mul_pointwise(&pzr); + assert_eq!(pow2.iter().copied().collect::>(), [Fe32::P, Fe32::Y, Fe32::S, Fe32::J,]); + + let lifted = pzr.lift::(); + assert_eq!( + lifted.iter().copied().collect::>(), + [ + Fe1024::from(Fe32::P), + Fe1024::from(Fe32::Z), + Fe1024::from(Fe32::Y), + Fe1024::from(Fe32::G), + ] + ); + } + + #[test] + fn construct_too_far() { + let x: FieldVec<_> = (0..NO_ALLOC_MAX_LENGTH + 1).collect(); + let y: FieldVec<_> = FieldVec::from_powers(Fe32::Q, NO_ALLOC_MAX_LENGTH); + assert_eq!(x.len(), NO_ALLOC_MAX_LENGTH + 1); + assert_eq!(y.len(), NO_ALLOC_MAX_LENGTH + 1); + } + + #[test] + #[cfg_attr(not(feature = "alloc"), should_panic)] + fn access_too_far() { + let x: FieldVec<_> = (0..NO_ALLOC_MAX_LENGTH + 1).collect(); + let _ = x[0]; + } + + #[test] + #[cfg_attr(not(feature = "alloc"), should_panic)] + fn push_too_far() { + let mut x: FieldVec<_> = (0..NO_ALLOC_MAX_LENGTH).collect(); + x.push(100); + } +} From c209ce8b12954102a7b0219a44ac09f7f0f7587e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 22 Sep 2024 14:50:03 +0000 Subject: [PATCH 8/8] polynomial: expand unit tests --- src/primitives/polynomial.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/primitives/polynomial.rs b/src/primitives/polynomial.rs index 0c60f584..04860020 100644 --- a/src/primitives/polynomial.rs +++ b/src/primitives/polynomial.rs @@ -359,22 +359,25 @@ impl Iterator for RootIter { #[cfg(test)] mod tests { + use Fe32 as F; + use super::*; use crate::{Fe1024, Fe32}; #[test] #[cfg(feature = "alloc")] fn roots() { - let bip93_poly: Polynomial = { - use Fe32 as F; - [F::S, F::S, F::C, F::M, F::L, F::E, F::E, F::E, F::Q, F::G, F::_3, F::M, F::E, F::P] - } - .iter() - .copied() - .collect(); + #[rustfmt::skip] + let mut bip93_poly = Polynomial::with_monic_leading_term( + &[F::E, F::M, F::_3, F::G, F::Q, F::E, F::E, F::E, F::L, F::M, F::C, F::S, F::S] + ); + assert_eq!(bip93_poly.leading_term(), F::P); + assert!(!bip93_poly.zero_is_root()); assert_eq!(bip93_poly.degree(), 13); + bip93_poly.zero_pad_up_to(1000); // should have no visible effect + let (elem, order, root_indices) = bip93_poly.bch_generator_primitive_element::(); // Basically, only the order and the length of the `root_indices` range are // guaranteed to be consistent across runs. There will be two possible ranges, @@ -422,18 +425,15 @@ mod tests { } #[test] - #[cfg(not(feature = "alloc"))] - fn roots() { + fn roots_bech32() { // Exactly the same test as above, but for bech32 - let bech32_poly: Polynomial = { - use Fe32 as F; - [F::J, F::A, F::_4, F::_5, F::K, F::A, F::P] - } - .iter() - .copied() - .collect(); + let bech32_poly = + Polynomial::with_monic_leading_term(&[F::A, F::K, F::_5, F::_4, F::A, F::J]); - assert_eq!(bech32_poly.degree(), 6); + assert_eq!( + bech32_poly.iter().copied().collect::>(), + [F::J, F::A, F::_4, F::_5, F::K, F::A, F::P], + ); let (elem, order, root_indices) = bech32_poly.bch_generator_primitive_element::(); // As above, only the order and the length of the `root_indices` range are