From b3b04e069955c86b7c0c391154efa1cf2272665a Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Sat, 20 Jun 2026 19:04:27 -0600 Subject: [PATCH] elliptic-curve: simplify `BatchInvert` trait The previous `BatchInvert` trait was based on blanket impls bounded by `Field`, unfortunately `k256` needs to provide its own to handle normalization. The new version operates in-place requiring a temporary buffer, whereas the previous version return a value. This winds up making the generic implementation of something like batch normalization significantly simpler, since it can be in complete charge of the storage. It also means we don't need separate methods for arrays vs slices / `Vec`. I'm sure we discussed and considered that at one point in the past and I probably said no at the time, but this style of API makes the downstream code *significantly* simpler. Also, rather than blanket impls it has a generic provided implementation which can work everywhere except `k256`, which needs to do its own normalization. --- elliptic-curve/src/ops.rs | 178 ++++++--------------------- elliptic-curve/src/scalar/nonzero.rs | 46 +------ 2 files changed, 39 insertions(+), 185 deletions(-) diff --git a/elliptic-curve/src/ops.rs b/elliptic-curve/src/ops.rs index 1ff2860e5..e6f67cf76 100644 --- a/elliptic-curve/src/ops.rs +++ b/elliptic-curve/src/ops.rs @@ -4,154 +4,52 @@ pub use bigint::{Invert, Reduce}; pub use core::ops::{Add, AddAssign, Mul, MulAssign, Neg, Shr, ShrAssign, Sub, SubAssign}; use crate::CurveGroup; -use core::iter; use ff::Field; use group::Group; -use subtle::{Choice, CtOption}; +use subtle::Choice; -#[cfg(feature = "alloc")] -use alloc::{borrow::ToOwned, vec::Vec}; - -/// Perform a batched inversion on a sequence of field elements (i.e. base field elements or scalars) -/// at an amortized cost that should be practically as efficient as a single inversion. -pub trait BatchInvert { - /// The output of batch inversion. A container of field elements. - type Output; - - /// Invert a batch of field elements. - fn batch_invert(field_elements: FieldElements) -> >::Output; -} - -impl BatchInvert<[T; N]> for T -where - T: Field, -{ - type Output = CtOption<[Self; N]>; - - fn batch_invert(mut field_elements: [Self; N]) -> CtOption<[Self; N]> { - let mut field_elements_pad = [Self::default(); N]; - let inversion_succeeded = - invert_batch_internal(&mut field_elements, &mut field_elements_pad, invert); - - CtOption::new(field_elements, inversion_succeeded) - } -} - -#[cfg(feature = "alloc")] -impl<'this, T> BatchInvert<&'this mut [Self]> for T -where - T: Field, -{ - type Output = CtOption<&'this mut [Self]>; - - fn batch_invert(field_elements: &'this mut [Self]) -> CtOption<&'this mut [Self]> { - let mut field_elements_pad: Vec = vec![Self::default(); field_elements.len()]; - let inversion_succeeded = - invert_batch_internal(field_elements, &mut field_elements_pad, invert); - - CtOption::new(field_elements, inversion_succeeded) - } -} - -#[cfg(feature = "alloc")] -impl BatchInvert<&[Self]> for T -where - T: Field, -{ - type Output = CtOption>; - - fn batch_invert(field_elements: &[Self]) -> CtOption> { - let mut field_elements: Vec = field_elements.to_owned(); - let mut field_elements_pad: Vec = vec![Self::default(); field_elements.len()]; - let inversion_succeeded = - invert_batch_internal(&mut field_elements, &mut field_elements_pad, invert); - - CtOption::new(field_elements, inversion_succeeded) - } -} - -#[cfg(feature = "alloc")] -impl BatchInvert> for T -where - T: Field, -{ - type Output = CtOption>; - - fn batch_invert(mut field_elements: Vec) -> CtOption> { - let mut field_elements_pad: Vec = vec![Self::default(); field_elements.len()]; - let inversion_succeeded = - invert_batch_internal(&mut field_elements, &mut field_elements_pad, invert); - - CtOption::new(field_elements, inversion_succeeded) - } -} - -fn invert(scalar: T) -> (T, Choice) { - let scalar = scalar.invert(); - let choice = scalar.is_some(); - let scalar = scalar.unwrap_or(T::default()); - - (scalar, choice) -} - -/// Implements "Montgomery's trick", a trick for computing many modular inverses at once. +/// Perform a batched inversion on a slice of field elements (i.e. base field elements or scalars) +/// at an amortized cost that should be practically as efficient as a single inversion, writing +/// the field element inversions into `element`, and utilizing `scratch` as temporary storage. /// -/// "Montgomery's trick" works by reducing the problem of computing `n` inverses -/// to computing a single inversion, plus some storage and `O(n)` extra multiplications. -/// -/// See: https://iacr.org/archive/pkc2004/29470042/29470042.pdf section 2.2. -pub(crate) fn invert_batch_internal + MulAssign>( - field_elements: &mut [T], - field_elements_pad: &mut [T], - invert: fn(T) -> (T, Choice), -) -> Choice { - let batch_size = field_elements.len(); - if batch_size != field_elements_pad.len() { - return Choice::from(0); - } - if batch_size == 0 { - return Choice::from(1); - } - - let mut acc = field_elements[0]; - field_elements_pad[0] = acc; - - for (field_element, field_element_pad) in field_elements - .iter_mut() - .zip(field_elements_pad.iter_mut()) - .skip(1) - { - // $ a_n = a_{n-1}*x_n $ - acc *= *field_element; - *field_element_pad = acc; - } - - let (mut acc, choice) = invert(acc); +/// # Panics +/// If `elements` and `scratch` are not the same length. +pub trait BatchInvert: Field { + /// Invert a batch of field elements. + /// + /// Returns the falsy [`Choice`] in the event any of the elements is `0`. + fn batch_invert_in_place(elements: &mut [Self], scratch: &mut [Self]) -> Choice { + // Implements "Montgomery's trick", a trick for computing many modular inverses at once. + // + // "Montgomery's trick" works by reducing the problem of computing `n` inverses + // to computing a single inversion, plus some storage and `O(n)` extra multiplications. + // + // See: https://iacr.org/archive/pkc2004/29470042/29470042.pdf section 2.2. + assert_eq!(elements.len(), scratch.len()); + + let mut acc = Self::ONE; + let mut all_nonzero = Choice::from(1u8); + + for (tmp, e) in scratch.iter_mut().zip(elements.iter()) { + // $ a_n = a_{n-1}*x_n $ + *tmp = acc; + let is_zero = e.ct_eq(&Self::ZERO); + all_nonzero &= !is_zero; + acc = Self::conditional_select(&(acc * e), &acc, is_zero); + } - // Shift the iterator by one element back. The one we are skipping is served in `acc`. - let field_elements_pad = field_elements_pad - .iter() - .rev() - .skip(1) - .map(Some) - .chain(iter::once(None)); + // `acc` is the product of every nonzero element, so this can't fail. + acc = acc.invert().unwrap_or(Self::ONE); - for (field_element, field_element_pad) in - field_elements.iter_mut().rev().zip(field_elements_pad) - { - if let Some(field_element_pad) = field_element_pad { - // Store in a temporary so we can overwrite `field_element`. - // $ a_{n-1} = {a_n}^{-1}*x_n $ - let tmp = acc * *field_element; - // $ {x_n}^{-1} = a_{n}^{-1}*a_{n-1} $ - *field_element = acc * *field_element_pad; - acc = tmp; - } else { - *field_element = acc; + for (e, tmp) in elements.iter_mut().zip(scratch.iter()).rev() { + let is_zero = e.ct_eq(&Self::ZERO); + let new_acc = Self::conditional_select(&(acc * *e), &acc, is_zero); + *e = Self::conditional_select(&(acc * *tmp), e, is_zero); + acc = new_acc; } - } - choice + all_nonzero + } } /// Linear combination. diff --git a/elliptic-curve/src/scalar/nonzero.rs b/elliptic-curve/src/scalar/nonzero.rs index d61065be8..d313d9927 100644 --- a/elliptic-curve/src/scalar/nonzero.rs +++ b/elliptic-curve/src/scalar/nonzero.rs @@ -2,7 +2,7 @@ use crate::{ CurveArithmetic, Error, FieldBytes, PrimeCurve, Scalar, ScalarValue, SecretKey, - ops::{self, BatchInvert, Invert, Reduce, ReduceNonZero}, + ops::{Invert, Reduce, ReduceNonZero}, point::NonIdentity, scalar::IsHigh, }; @@ -18,9 +18,6 @@ use rand_core::{CryptoRng, TryCryptoRng}; use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; use zeroize::Zeroize; -#[cfg(feature = "alloc")] -use alloc::vec::Vec; - #[cfg(feature = "serde")] use serdect::serde::{Deserialize, Serialize, de, ser}; @@ -104,47 +101,6 @@ where } } -impl BatchInvert<[Self; N]> for NonZeroScalar -where - C: CurveArithmetic + PrimeCurve, -{ - type Output = [Self; N]; - - fn batch_invert(mut field_elements: [Self; N]) -> [Self; N] { - let mut field_elements_pad = [Self { - scalar: Scalar::::ONE, - }; N]; - ops::invert_batch_internal(&mut field_elements, &mut field_elements_pad, |scalar| { - (scalar.invert(), Choice::from(1)) - }); - - field_elements - } -} - -#[cfg(feature = "alloc")] -impl BatchInvert> for NonZeroScalar -where - C: CurveArithmetic + PrimeCurve, -{ - type Output = Vec; - - fn batch_invert(mut field_elements: Vec) -> Vec { - let mut field_elements_pad: Vec = vec![ - Self { - scalar: Scalar::::ONE, - }; - field_elements.len() - ]; - - ops::invert_batch_internal(&mut field_elements, &mut field_elements_pad, |scalar| { - (scalar.invert(), Choice::from(1)) - }); - - field_elements - } -} - impl ConditionallySelectable for NonZeroScalar where C: CurveArithmetic,