Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve codebase with some clippy suggestions #867

Merged
merged 14 commits into from
Nov 20, 2024
Merged
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,
)))
Pratyush marked this conversation as resolved.
Show resolved Hide resolved
}

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);
tcoratger marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -652,15 +652,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
Loading