Skip to content

Commit

Permalink
chore: improve codebase with some clippy suggestions (#867)
Browse files Browse the repository at this point in the history
* chore: improve codebase with some clippy suggestions

* Update ec/src/hashing/tests.rs

* Update ec/src/hashing/tests.rs

* Update ec/src/hashing/tests.rs

* Update ff/src/biginteger/tests.rs

* Update ec/src/hashing/curve_maps/wb.rs

* fix comments

* Update ff/src/biginteger/tests.rs

Co-authored-by: Pratyush Mishra <[email protected]>

* Update ff/src/biginteger/tests.rs

Co-authored-by: Pratyush Mishra <[email protected]>

* Update ff/src/biginteger/tests.rs

Co-authored-by: Pratyush Mishra <[email protected]>

* Update ff/src/biginteger/tests.rs

Co-authored-by: Pratyush Mishra <[email protected]>

---------

Co-authored-by: Pratyush Mishra <[email protected]>
  • Loading branch information
tcoratger and Pratyush authored Nov 20, 2024
1 parent 1aa8ed5 commit 902ca31
Show file tree
Hide file tree
Showing 49 changed files with 272 additions and 252 deletions.
16 changes: 13 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,20 @@ authors = [ "arkworks contributors" ]
homepage = "https://arkworks.rs"
repository = "https://github.com/arkworks-rs/algebra"
categories = ["cryptography"]
include = ["Cargo.toml", "src", "README.md", "LICENSE-APACHE", "LICENSE-MIT", "doc/katex-header.html"]
include = [
"Cargo.toml",
"src",
"README.md",
"LICENSE-APACHE",
"LICENSE-MIT",
"doc/katex-header.html",
]
license = "MIT OR Apache-2.0"
edition = "2021"
rust-version = "1.75"

[workspace.metadata.docs.rs]
rustdoc-args = [ "--html-in-header katex-header.html" ]
rustdoc-args = ["--html-in-header katex-header.html"]

[workspace.metadata.release]
dependent-version = "fix"
Expand All @@ -56,7 +63,10 @@ arrayvec = { version = "0.7", default-features = false }
criterion = "0.5.0"
educe = "0.6.0"
digest = { version = "0.10", default-features = false }
hashbrown = { version = "0.15", default-features = false, features = ["inline-more", "allocator-api2"] }
hashbrown = { version = "0.15", default-features = false, features = [
"inline-more",
"allocator-api2",
] }
hex = "0.4"
itertools = { version = "0.13", default-features = false }
libtest-mimic = "0.8.1"
Expand Down
4 changes: 2 additions & 2 deletions ec/src/hashing/curve_maps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ pub mod elligator2;
pub mod swu;
pub mod wb;

//// parity method on the Field elements based on [\[1\]] Section 4.1
//// which is used by multiple curve maps including Elligator2 and SWU
/// parity method on the Field elements based on [\[1\]] Section 4.1
/// which is used by multiple curve maps including Elligator2 and SWU
/// - [\[1\]] <https://datatracker.ietf.org/doc/html/rfc9380/>
pub fn parity<F: Field>(element: &F) -> bool {
element
Expand Down
15 changes: 10 additions & 5 deletions ec/src/hashing/curve_maps/wb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type BaseField<MP> = <MP as CurveConfig>::BaseField;

/// [`IsogenyMap`] defines an isogeny between curves of
/// form `Phi(x, y) := (a(x), b(x)*y).
///
/// The `x` coordinate of the codomain point only depends on the
/// `x`-coordinate of the domain point, and the
/// `y`-coordinate of the codomain point is a multiple of the `y`-coordinate of the domain point.
Expand All @@ -39,7 +40,7 @@ pub struct IsogenyMap<
pub y_map_denominator: &'a [BaseField<Codomain>],
}

impl<'a, Domain, Codomain> IsogenyMap<'a, Domain, Codomain>
impl<Domain, Codomain> IsogenyMap<'_, Domain, Codomain>
where
Domain: SWCurveConfig,
Codomain: SWCurveConfig<BaseField = BaseField<Domain>>,
Expand All @@ -64,10 +65,14 @@ where
}
}

/// Trait defining the necessary parameters for the WB hash-to-curve method
/// for the curves of Weierstrass form of:
/// of y^2 = x^3 + a*x + b where b != 0 but `a` can be zero like BLS-381 curve.
/// From [\[WB2019\]]
/// Trait defining the necessary parameters for the WB hash-to-curve method.
///
/// This method is used for curves in Weierstrass form defined by:
///
/// `y^2 = x^3 + a*x + b` where `b != 0`, but `a` can be zero,
/// as seen in curves like BLS-381.
///
/// For more information, refer to [WB2019].
///
/// - [\[WB2019\]] <http://dx.doi.org/10.46586/tches.v2019.i4.154-179>
pub trait WBConfig: SWCurveConfig + Sized {
Expand Down
11 changes: 8 additions & 3 deletions ec/src/hashing/map_to_curve_hasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ pub trait MapToCurve<T: CurveGroup>: Sized {
fn map_to_curve(point: T::BaseField) -> Result<T::Affine, HashToCurveError>;
}

/// Helper struct that can be used to construct elements on the elliptic curve
/// from arbitrary messages, by first hashing the message onto a field element
/// and then mapping it to the elliptic curve defined over that field.
/// A helper struct used to construct elements on an elliptic curve
/// from arbitrary messages.
///
/// The process works in two stages:
///
/// 1. First, the message is hashed into a field element.
/// 2. Then, the resulting field element is mapped to the elliptic curve
/// defined over that field.
pub struct MapToCurveBasedHasher<T, H2F, M2C>
where
T: CurveGroup,
Expand Down
16 changes: 9 additions & 7 deletions ec/src/models/bw6/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub trait BW6Config: 'static + Eq + Sized {

// Computes the exponent of an element of the cyclotomic subgroup by the curve seed
fn exp_by_x(f: &Fp6<Self::Fp6Config>) -> Fp6<Self::Fp6Config> {
Self::cyclotomic_exp_signed(f, &Self::X, Self::X_IS_NEGATIVE)
Self::cyclotomic_exp_signed(f, Self::X, Self::X_IS_NEGATIVE)
}

fn final_exponentiation_hard_part(f: &Fp6<Self::Fp6Config>) -> Fp6<Self::Fp6Config> {
Expand All @@ -74,7 +74,9 @@ pub trait BW6Config: 'static + Eq + Sized {

fn final_exponentiation(f: MillerLoopOutput<BW6<Self>>) -> Option<PairingOutput<BW6<Self>>> {
let easy_part = BW6::<Self>::final_exponentiation_easy_part(f.0);
Some(Self::final_exponentiation_hard_part(&easy_part)).map(PairingOutput)
Some(PairingOutput(Self::final_exponentiation_hard_part(
&easy_part,
)))
}

fn multi_miller_loop(
Expand Down Expand Up @@ -219,7 +221,7 @@ impl<P: BW6Config> BW6<P> {
}

fn exp_by_x_minus_1_div_3(f: &Fp6<P::Fp6Config>) -> Fp6<P::Fp6Config> {
P::cyclotomic_exp_signed(f, &P::X_MINUS_1_DIV_3, P::X_IS_NEGATIVE)
P::cyclotomic_exp_signed(f, P::X_MINUS_1_DIV_3, P::X_IS_NEGATIVE)
}

// f^[(p^3-1)(p+1)]
Expand Down Expand Up @@ -285,9 +287,9 @@ impl<P: BW6Config> BW6<P> {
// d1 = (ht-hy)//2
let d1 = (P::H_T - P::H_Y) / 2;
// H = F**d1 * E
let h = P::cyclotomic_exp_signed(&f, &[d1 as u64], d1 < 0) * &e;
let h = P::cyclotomic_exp_signed(&f, [d1 as u64], d1 < 0) * &e;
// H = H**2 * H * B * G**d2
let h = h.square() * &h * &b * g.cyclotomic_exp(&[d2]);
let h = h.square() * &h * &b * g.cyclotomic_exp([d2]);
// return A * H
a * &h
} else {
Expand Down Expand Up @@ -325,9 +327,9 @@ impl<P: BW6Config> BW6<P> {
// d1 = (ht+hy)//2
let d1 = (P::H_T + P::H_Y) / 2;
// J = H**d1 * E
let j = P::cyclotomic_exp_signed(&h, &[d1 as u64], d1 < 0) * &e;
let j = P::cyclotomic_exp_signed(&h, [d1 as u64], d1 < 0) * &e;
// K = J**2 * J * B * I**d2
let k = j.square() * &j * &b * i.cyclotomic_exp(&[d2]);
let k = j.square() * &j * &b * i.cyclotomic_exp([d2]);
// return A * K
a * &k
}
Expand Down
1 change: 1 addition & 0 deletions ec/src/models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod twisted_edwards;

/// Elliptic curves can be represented via different "models" with varying
/// efficiency properties.
///
/// `CurveConfig` bundles together the types that are common
/// to all models of the given curve, namely the `BaseField` over which the
/// curve is defined, and the `ScalarField` defined by the appropriate
Expand Down
2 changes: 1 addition & 1 deletion ec/src/models/short_weierstrass/affine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl<P: SWCurveConfig> AffineRepr for Affine<P> {
type Group = Projective<P>;

fn xy(&self) -> Option<(Self::BaseField, Self::BaseField)> {
(!self.infinity).then(|| (self.x, self.y))
(!self.infinity).then_some((self.x, self.y))
}

#[inline]
Expand Down
5 changes: 3 additions & 2 deletions ec/src/models/short_weierstrass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ mod serialization_flags;
pub use serialization_flags::*;

/// Constants and convenience functions that collectively define the [Short Weierstrass model](https://www.hyperelliptic.org/EFD/g1p/auto-shortw.html)
/// of the curve. In this model, the curve equation is `y² = x³ + a * x + b`,
/// for constants `a` and `b`.
/// of the curve.
///
/// In this model, the curve equation is `y² = x³ + a * x + b`, for constants `a` and `b`.
pub trait SWCurveConfig: super::CurveConfig {
/// Coefficient `a` of the curve equation.
const COEFF_A: Self::BaseField;
Expand Down
2 changes: 1 addition & 1 deletion ec/src/models/twisted_edwards/affine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl<P: TECurveConfig> AffineRepr for Affine<P> {
type Group = Projective<P>;

fn xy(&self) -> Option<(Self::BaseField, Self::BaseField)> {
(!self.is_zero()).then(|| (self.x, self.y))
(!self.is_zero()).then_some((self.x, self.y))
}

fn generator() -> Self {
Expand Down
10 changes: 6 additions & 4 deletions ec/src/models/twisted_edwards/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ mod serialization_flags;
pub use serialization_flags::*;

/// Constants and convenience functions that collectively define the [Twisted Edwards model](https://www.hyperelliptic.org/EFD/g1p/auto-twisted.html)
/// of the curve. In this model, the curve equation is
/// `a * x² + y² = 1 + d * x² * y²`, for constants `a` and `d`.
/// of the curve.
///
/// In this model, the curve equation is `a * x² + y² = 1 + d * x² * y²`, for constants `a` and `d`.
pub trait TECurveConfig: super::CurveConfig {
/// Coefficient `a` of the curve equation.
const COEFF_A: Self::BaseField;
Expand Down Expand Up @@ -159,8 +160,9 @@ pub trait TECurveConfig: super::CurveConfig {
}

/// Constants and convenience functions that collectively define the [Montgomery model](https://www.hyperelliptic.org/EFD/g1p/auto-montgom.html)
/// of the curve. In this model, the curve equation is
/// `b * y² = x³ + a * x² + x`, for constants `a` and `b`.
/// of the curve.
///
/// In this model, the curve equation is `b * y² = x³ + a * x² + x`, for constants `a` and `b`.
pub trait MontCurveConfig: super::CurveConfig {
/// Coefficient `a` of the curve equation.
const COEFF_A: Self::BaseField;
Expand Down
9 changes: 6 additions & 3 deletions ec/src/scalar_mul/glv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ use num_traits::{One, Signed};

/// The GLV parameters for computing the endomorphism and scalar decomposition.
pub trait GLVConfig: Send + Sync + 'static + SWCurveConfig {
/// Constants that are used to calculate `phi(G) := lambda*G`.
/// Constant used to calculate `phi(G) := lambda*G`.
///
/// The coefficients of the endomorphism
const ENDO_COEFFS: &'static [Self::BaseField];

/// Constant used to calculate `phi(G) := lambda*G`.
///
/// The eigenvalue corresponding to the endomorphism.
const LAMBDA: Self::ScalarField;

Expand All @@ -30,7 +32,8 @@ pub trait GLVConfig: Send + Sync + 'static + SWCurveConfig {
let scalar: BigInt = k.into_bigint().into().into();

let coeff_bigints: [BigInt; 4] = Self::SCALAR_DECOMP_COEFFS.map(|x| {
BigInt::from_biguint(x.0.then_some(Sign::Plus).unwrap_or(Sign::Minus), x.1.into())
let sign = if x.0 { Sign::Plus } else { Sign::Minus };
BigInt::from_biguint(sign, x.1.into())
});

let [n11, n12, n21, n22] = coeff_bigints;
Expand Down
4 changes: 2 additions & 2 deletions ec/src/scalar_mul/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl<T: ScalarMul> BatchMulPreprocessing<T> {
) -> Self {
let window = Self::compute_window_size(num_scalars);
let in_window = 1 << window;
let outerc = (max_scalar_size + window - 1) / window;
let outerc = max_scalar_size.div_ceil(window);
let last_in_window = 1 << (max_scalar_size - (outerc - 1) * window);

let mut multiples_of_g = vec![vec![T::zero(); in_window]; outerc];
Expand Down Expand Up @@ -236,7 +236,7 @@ impl<T: ScalarMul> BatchMulPreprocessing<T> {
}

fn windowed_mul(&self, scalar: &T::ScalarField) -> T {
let outerc = (self.max_scalar_size + self.window - 1) / self.window;
let outerc = self.max_scalar_size.div_ceil(self.window);
let modulus_size = T::ScalarField::MODULUS_BIT_SIZE as usize;
let scalar_val = scalar.into_bigint().to_bits_le();

Expand Down
12 changes: 6 additions & 6 deletions ec/src/scalar_mul/variable_base/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ pub trait VariableBaseMSM: ScalarMul {

/// Streaming multi-scalar multiplication algorithm with hard-coded chunk
/// size.
fn msm_chunks<I: ?Sized, J>(bases_stream: &J, scalars_stream: &I) -> Self
fn msm_chunks<I, J>(bases_stream: &J, scalars_stream: &I) -> Self
where
I: Iterable,
I: Iterable + ?Sized,
I::Item: Borrow<Self::ScalarField>,
J: Iterable,
J::Item: Borrow<Self::MulBase>,
Expand All @@ -88,7 +88,7 @@ pub trait VariableBaseMSM: ScalarMul {
let mut bases = bases_init.skip(bases_stream.len() - scalars_stream.len());
let step: usize = 1 << 20;
let mut result = Self::zero();
for _ in 0..(scalars_stream.len() + step - 1) / step {
for _ in 0..scalars_stream.len().div_ceil(step) {
let bases_step = (&mut bases)
.take(step)
.map(|b| *b.borrow())
Expand Down Expand Up @@ -119,7 +119,7 @@ fn msm_bigint_wnaf<V: VariableBaseMSM>(
};

let num_bits = V::ScalarField::MODULUS_BIT_SIZE as usize;
let digits_count = (num_bits + c - 1) / c;
let digits_count = num_bits.div_ceil(c);
#[cfg(feature = "parallel")]
let scalar_digits = scalars
.into_par_iter()
Expand Down Expand Up @@ -281,9 +281,9 @@ fn make_digits(a: &impl BigInteger, w: usize, num_bits: usize) -> impl Iterator<
} else {
num_bits
};
let digits_count = (num_bits + w - 1) / w;
let digits_count = num_bits.div_ceil(w);

(0..digits_count).into_iter().map(move |i| {
(0..digits_count).map(move |i| {
// Construct a buffer of bits of the scalar, starting at `bit_offset`.
let bit_offset = i * w;
let u64_idx = bit_offset / 64;
Expand Down
4 changes: 2 additions & 2 deletions ff-asm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub fn x86_64_asm_mul(input: TokenStream) -> TokenStream {
} else {
panic!("The number of limbs must be a literal");
};
if num_limbs <= 6 && num_limbs <= 3 * MAX_REGS {
if num_limbs <= 6 {
let impl_block = generate_impl(num_limbs, true);

let inner_ts: Expr = syn::parse_str(&impl_block).unwrap();
Expand Down Expand Up @@ -110,7 +110,7 @@ pub fn x86_64_asm_square(input: TokenStream) -> TokenStream {
} else {
panic!("The number of limbs must be a literal");
};
if num_limbs <= 6 && num_limbs <= 3 * MAX_REGS {
if num_limbs <= 6 {
let impl_block = generate_impl(num_limbs, false);

let inner_ts: Expr = syn::parse_str(&impl_block).unwrap();
Expand Down
11 changes: 6 additions & 5 deletions ff-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ pub fn unroll_for_loops(args: TokenStream, input: TokenStream) -> TokenStream {

if let Item::Fn(item_fn) = item {
let new_block = {
let &ItemFn {
let ItemFn {
block: ref box_block,
..
} = &item_fn;
} = item_fn;
unroll::unroll_in_block(box_block, unroll_by)
};
let new_item = Item::Fn(ItemFn {
Expand Down Expand Up @@ -138,7 +138,8 @@ fn test_str_to_limbs() {
let number = 1i128 << i;
let signed_number = match sign {
Minus => -number,
Plus | _ => number,
Plus => number,
_ => number,
};
for base in [2, 8, 16, 10] {
let mut string = match base {
Expand All @@ -151,10 +152,10 @@ fn test_str_to_limbs() {
if sign == Minus {
string.insert(0, '-');
}
let (is_positive, limbs) = utils::str_to_limbs(&format!("{}", string));
let (is_positive, limbs) = utils::str_to_limbs(&string.to_string());
assert_eq!(
limbs[0],
format!("{}u64", signed_number.abs() as u64),
format!("{}u64", signed_number.unsigned_abs() as u64),
"{signed_number}, {i}"
);
if i > 63 {
Expand Down
3 changes: 1 addition & 2 deletions ff-macros/src/montgomery/mul.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ pub(super) fn mul_assign_impl(
let mut carry = 0u64;
fa::mac(scratch[#i], tmp, #modulus_0, &mut carry);
});
for j in 1..num_limbs {
let modulus_j = modulus_limbs[j];
for (j, modulus_j) in modulus_limbs.iter().enumerate().take(num_limbs).skip(1) {
let k = i + j;
body.extend(quote!(scratch[#k] = fa::mac_with_carry(scratch[#k], tmp, #modulus_j, &mut carry);));
}
Expand Down
2 changes: 1 addition & 1 deletion ff-macros/src/unroll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use syn::{

/// Routine to unroll for loops within a block
pub(crate) fn unroll_in_block(block: &Block, unroll_by: usize) -> Block {
let &Block {
let Block {
ref brace_token,
ref stmts,
} = block;
Expand Down
14 changes: 5 additions & 9 deletions ff/src/biginteger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,15 +651,11 @@ impl<const N: usize> TryFrom<BigUint> for BigInt<N> {
} else {
let mut limbs = [0u64; N];

bytes
.chunks(8)
.into_iter()
.enumerate()
.for_each(|(i, chunk)| {
let mut chunk_padded = [0u8; 8];
chunk_padded[..chunk.len()].copy_from_slice(chunk);
limbs[i] = u64::from_le_bytes(chunk_padded)
});
bytes.chunks(8).enumerate().for_each(|(i, chunk)| {
let mut chunk_padded = [0u8; 8];
chunk_padded[..chunk.len()].copy_from_slice(chunk);
limbs[i] = u64::from_le_bytes(chunk_padded)
});

Ok(Self(limbs))
}
Expand Down
Loading

0 comments on commit 902ca31

Please sign in to comment.