Skip to content

Commit

Permalink
Update QuadExtField::sqrt for better performance
Browse files Browse the repository at this point in the history
Currently, the QuadExtField uses an expensive runtime inverse of two
calculation defined as `P::BaseField::one().double().inverse()`.

With the proposed change, we compute the BigInt `(p+1)/2` that is ~15%
cheaper than the previous method.

Alternatively, we could require a compile-time constant provided by the
user that represents `1/2`. However, having a constant requirement to
satisfy a single use-case isn't ideal.

Closes #210
  • Loading branch information
vlopes11 committed Nov 24, 2021
1 parent 88178d8 commit 5f16308
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
### Improvements

- [\#339](https://github.com/arkworks-rs/algebra/pull/339) (ark-ff) Remove duplicated code from `test_field` module and replace its usage with `ark-test-curves` crate.
- [\#352](https://github.com/arkworks-rs/algebra/pull/352) (ark-ff) Update `QuadExtField::sqrt` for better performance.

### Bugfixes

Expand Down
21 changes: 15 additions & 6 deletions ff/src/fields/models/quadratic_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ use ark_std::rand::{
};

use crate::{
biginteger::BigInteger,
bytes::{FromBytes, ToBytes},
fields::{Field, LegendreSymbol, PrimeField, SquareRootField},
fields::{Field, FpParameters, LegendreSymbol, PrimeField, SquareRootField},
ToConstraintField, UniformRand,
};

Expand Down Expand Up @@ -331,6 +332,7 @@ impl<P: QuadExtParameters> Field for QuadExtField<P> {
impl<'a, P: QuadExtParameters> SquareRootField for QuadExtField<P>
where
P::BaseField: SquareRootField,
P::BaseField: From<<<P as QuadExtParameters>::BasePrimeField as PrimeField>::BigInt>,
{
fn legendre(&self) -> LegendreSymbol {
// The LegendreSymbol in a field of order q for an element x can be
Expand All @@ -355,11 +357,18 @@ where
// Try computing the square root
// Check at the end of the algorithm if it was a square root
let alpha = self.norm();
// TODO: Precompute this
let two_inv = P::BaseField::one()
.double()
.inverse()
.expect("Two should always have an inverse");

// Compute `(p+1)/2` as `1/2`.
// This is cheaper than `P::BaseField::one().double().inverse()`
let mut two_inv = <<P as QuadExtParameters>::BasePrimeField as PrimeField>::Params::MODULUS;

// TODO use from_limbs API
// https://github.com/arkworks-rs/algebra/issues/351
two_inv.add_nocarry(&1u64.into());
two_inv.div2();

let two_inv = P::BaseField::from(two_inv);

alpha.sqrt().and_then(|alpha| {
let mut delta = (alpha + &self.c0) * &two_inv;
if delta.legendre().is_qnr() {
Expand Down

0 comments on commit 5f16308

Please sign in to comment.