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.

Sign up for GitHub

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

Jump to bottom

Miscellaneous Changes: Serialization, additional helper methods, reorg #235

Closed
wants to merge 17 commits into from
Closed

Miscellaneous Changes: Serialization, additional helper methods, reorg #235

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
54ccc1f
Merge celo/zexe w/ arkworks
jon-chuang Dec 21, 2020
1689686
Merge branch 'master' into jonch/celo-merge
jon-chuang Mar 14, 2021
4328859
hotfix
jon-chuang Mar 15, 2021
96a6d75
remove cuda
jon-chuang Mar 17, 2021
b3ffa79
remove downstream batched
jon-chuang Mar 17, 2021
44edd01
remove glv infra
jon-chuang Mar 17, 2021
a956c20
remove batch ops
jon-chuang Mar 17, 2021
5219495
fmt
jon-chuang Mar 17, 2021
3945a5c
remove bw6 asm
jon-chuang Mar 17, 2021
308452e
remove remnants
jon-chuang Mar 17, 2021
bad1cf2
remove remnants
jon-chuang Mar 17, 2021
a478be0
remove remnants
jon-chuang Mar 17, 2021
f980cdf
fix serialization tests (compressed default)
jon-chuang Mar 17, 2021
71d2917
Apply suggestions from code review
jon-chuang Mar 17, 2021
e1e3350
stash
jon-chuang Mar 17, 2021
8c910a1
Merge branch 'jonch/celo-org-serial-misc' of https://github.com/arkwo…
jon-chuang Mar 17, 2021
5d26eff
Merge branch 'master' into jonch/celo-org-serial-misc
Pratyush Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ec/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ ark-ff = { path = "../ff", default-features = false }
derivative = { version = "2", features = ["use_core"] }
num-traits = { version = "0.2", default-features = false }
rayon = { version = "1", optional = true }
serde_json = { version = "1.0.58", optional = true }
paste = "0.1"
jon-chuang marked this conversation as resolved.
Show resolved Hide resolved
zeroize = { version = "1", default-features = false, features = ["zeroize_derive"] }

[dev-dependencies]
rand_xorshift = "0.2"
Comment on lines +24 to +25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use ark_std::rand::XorShiftRng directly, no need to import this.

Suggested change
[dev-dependencies]
rand_xorshift = "0.2"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I'll have to change this...


[features]
default = []
std = [ "ark-std/std", "ark-ff/std", "ark-serialize/std" ]
Expand Down
2 changes: 2 additions & 0 deletions ec/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ pub trait ProjectiveCurve:
self = res;
self
}

fn get_x(&mut self) -> &mut Self::BaseField;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is well defined for projective curve representations, as the same curve point can have different projective coordinates, which means that the X coordinate can differ.

(Also no code in this PR uses it, but I guess your later PRs will?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yes, I can remove it for now, it will be used only for a generic script to generate parameters.

The purpose is to mutate x, in the use case for the purpose of applying an endomorphism, for which the particular Z coordinate does not matter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, maybe it would be better to add xyz_endo_in_place/xyz_endo methods in the appropriate location (in the ModelParameters trait, perhaps?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly, but the method already defined later on would apply multiplication by a constant endo param (P::glv_endomorphism_in_place(&mut p.x);).

This more generic method allows you to do whatever you want with x. I don't think this is an issue in itself if you know what you're doing... If you're worried about clutter, well, unfortunately I can't think of a workaround.

Further, as far as the curves we have are concerned, get_x and get_y for the ProjectiveCurve trait seem fairly reasonable...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we can discuss it further in the PR for which it is required.

Copy link
Member

@Pratyush Pratyush Mar 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I meant that instead one would do something like P::glv_endomorphism_in_place(&mut p);, which fully encapsulates the details of the endomorphism; the signature would be something like

trait SWModelParameters {
	/// Existing stuff
	
	fn glv_endomorphism_in_place(p: &mut GroupProjective<Self>) {
		// do whatever
	}
}

This takes the point as a whole, instead of the coordinate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yeah, we can discuss it in the GLV PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking, the plan is to remove it from this PR right?

}

/// Affine representation of an elliptic curve point guaranteed to be
Expand Down
56 changes: 9 additions & 47 deletions ec/src/models/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,53 +8,15 @@ pub mod mnt6;
pub mod short_weierstrass_jacobian;
pub mod twisted_edwards_extended;

pub use {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad this code was moved to the relevant locations!

short_weierstrass_jacobian::SWModelParameters,
twisted_edwards_extended::{MontgomeryModelParameters, TEModelParameters},
};

pub trait ModelParameters: Send + Sync + 'static {
type BaseField: Field + SquareRootField;
type ScalarField: PrimeField + SquareRootField + Into<<Self::ScalarField as PrimeField>::BigInt>;
}

pub trait SWModelParameters: ModelParameters {
const COEFF_A: Self::BaseField;
const COEFF_B: Self::BaseField;
const COFACTOR: &'static [u64];
const COFACTOR_INV: Self::ScalarField;
const AFFINE_GENERATOR_COEFFS: (Self::BaseField, Self::BaseField);

#[inline(always)]
fn mul_by_a(elem: &Self::BaseField) -> Self::BaseField {
let mut copy = *elem;
copy *= &Self::COEFF_A;
copy
}

#[inline(always)]
fn add_b(elem: &Self::BaseField) -> Self::BaseField {
let mut copy = *elem;
copy += &Self::COEFF_B;
copy
}
}

pub trait TEModelParameters: ModelParameters {
const COEFF_A: Self::BaseField;
const COEFF_D: Self::BaseField;
const COFACTOR: &'static [u64];
const COFACTOR_INV: Self::ScalarField;
const AFFINE_GENERATOR_COEFFS: (Self::BaseField, Self::BaseField);

type MontgomeryModelParameters: MontgomeryModelParameters<BaseField = Self::BaseField>;

#[inline(always)]
fn mul_by_a(elem: &Self::BaseField) -> Self::BaseField {
let mut copy = *elem;
copy *= &Self::COEFF_A;
copy
}
}

pub trait MontgomeryModelParameters: ModelParameters {
const COEFF_A: Self::BaseField;
const COEFF_B: Self::BaseField;

type TEModelParameters: TEModelParameters<BaseField = Self::BaseField>;
type ScalarField: PrimeField
+ SquareRootField
+ From<<Self::ScalarField as PrimeField>::BigInt>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this From? Its a breaking change, so if so it will need to be noted in changelog

+ Into<<Self::ScalarField as PrimeField>::BigInt>;
}
68 changes: 57 additions & 11 deletions ec/src/models/short_weierstrass_jacobian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use ark_ff::{
ToConstraintField, UniformRand,
};

use crate::{models::SWModelParameters as Parameters, AffineCurve, ProjectiveCurve};
use crate::{AffineCurve, ModelParameters, ProjectiveCurve};

use num_traits::{One, Zero};
use zeroize::Zeroize;
Expand All @@ -29,6 +29,30 @@ use ark_std::rand::{
#[cfg(feature = "parallel")]
use rayon::prelude::*;

pub trait SWModelParameters: ModelParameters + Sized {
const COEFF_A: Self::BaseField;
const COEFF_B: Self::BaseField;
const COFACTOR: &'static [u64];
const COFACTOR_INV: Self::ScalarField;
const AFFINE_GENERATOR_COEFFS: (Self::BaseField, Self::BaseField);

#[inline(always)]
fn mul_by_a(elem: &Self::BaseField) -> Self::BaseField {
let mut copy = *elem;
copy *= &Self::COEFF_A;
copy
}

#[inline(always)]
fn add_b(elem: &Self::BaseField) -> Self::BaseField {
let mut copy = *elem;
copy += &Self::COEFF_B;
copy
}
}

use SWModelParameters as Parameters;

#[derive(Derivative)]
#[derivative(
Copy(bound = "P: Parameters"),
Expand Down Expand Up @@ -202,7 +226,7 @@ impl<P: Parameters> AffineCurve for GroupAffine<P> {
}

#[inline]
fn mul<S: Into<<Self::ScalarField as PrimeField>::BigInt>>(&self, by: S) -> GroupProjective<P> {
fn mul<S: Into<<Self::ScalarField as PrimeField>::BigInt>>(&self, by: S) -> Self::Projective {
let bits = BitIteratorBE::new(by.into());
self.mul_bits(bits)
}
Expand Down Expand Up @@ -390,6 +414,11 @@ impl<P: Parameters> ProjectiveCurve for GroupProjective<P> {
type ScalarField = P::ScalarField;
type Affine = GroupAffine<P>;

#[inline(always)]
fn get_x(&mut self) -> &mut Self::BaseField {
&mut self.x
}

#[inline]
fn prime_subgroup_generator() -> Self {
GroupAffine::prime_subgroup_generator().into()
Expand Down Expand Up @@ -560,6 +589,18 @@ impl<P: Parameters> ProjectiveCurve for GroupProjective<P> {
self.z -= &hh;
}
}

fn mul<S: AsRef<[u64]>>(mut self, other: S) -> Self {
let mut res = Self::zero();
for b in BitIteratorBE::without_leading_zeros(other.as_ref()) {
res.double_in_place();
if b {
res += self;
}
}
self = res;
self
jon-chuang marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl<P: Parameters> Neg for GroupProjective<P> {
Expand Down Expand Up @@ -782,34 +823,39 @@ impl<P: Parameters> CanonicalSerialize for GroupProjective<P> {
impl<P: Parameters> CanonicalDeserialize for GroupAffine<P> {
#[allow(unused_qualifications)]
fn deserialize<R: Read>(reader: R) -> Result<Self, SerializationError> {
let p = Self::deserialize_unchecked(reader)?;
if !p.is_in_correct_subgroup_assuming_on_curve() {
return Err(SerializationError::InvalidData);
}
Ok(p)
}

#[allow(unused_qualifications)]
fn deserialize_unchecked<R: Read>(reader: R) -> Result<Self, SerializationError> {
let (x, flags): (P::BaseField, SWFlags) =
CanonicalDeserializeWithFlags::deserialize_with_flags(reader)?;
if flags.is_infinity() {
Ok(Self::zero())
} else {
let p = GroupAffine::<P>::get_point_from_x(x, flags.is_positive().unwrap())
.ok_or(SerializationError::InvalidData)?;
if !p.is_in_correct_subgroup_assuming_on_curve() {
return Err(SerializationError::InvalidData);
}
Ok(p)
}
}

#[allow(unused_qualifications)]
fn deserialize_uncompressed<R: Read>(
reader: R,
) -> Result<Self, ark_serialize::SerializationError> {
let p = Self::deserialize_unchecked(reader)?;

fn deserialize_uncompressed<R: Read>(reader: R) -> Result<Self, SerializationError> {
let p = Self::deserialize_uncompressed_unchecked(reader)?;
if !p.is_in_correct_subgroup_assuming_on_curve() {
return Err(SerializationError::InvalidData);
}
Ok(p)
}

#[allow(unused_qualifications)]
fn deserialize_unchecked<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
fn deserialize_uncompressed_unchecked<R: Read>(
mut reader: R,
) -> Result<Self, SerializationError> {
let x: P::BaseField = CanonicalDeserialize::deserialize(&mut reader)?;
let (y, flags): (P::BaseField, SWFlags) =
CanonicalDeserializeWithFlags::deserialize_with_flags(&mut reader)?;
Expand Down
60 changes: 47 additions & 13 deletions ec/src/models/twisted_edwards_extended.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use crate::{
models::{MontgomeryModelParameters as MontgomeryParameters, TEModelParameters as Parameters},
AffineCurve, ProjectiveCurve,
};
use crate::{AffineCurve, ModelParameters, ProjectiveCurve};
use ark_serialize::{
CanonicalDeserialize, CanonicalDeserializeWithFlags, CanonicalSerialize,
CanonicalSerializeWithFlags, EdwardsFlags, SerializationError,
Expand All @@ -23,12 +20,38 @@ use zeroize::Zeroize;
use ark_ff::{
bytes::{FromBytes, ToBytes},
fields::{BitIteratorBE, Field, PrimeField, SquareRootField},
ToConstraintField, UniformRand,
impl_additive_ops_from_ref, ToConstraintField, UniformRand,
};

#[cfg(feature = "parallel")]
use rayon::prelude::*;

pub trait MontgomeryModelParameters: ModelParameters {
const COEFF_A: Self::BaseField;
const COEFF_B: Self::BaseField;

type TEModelParameters: TEModelParameters<BaseField = Self::BaseField>;
}

pub trait TEModelParameters: ModelParameters + Sized {
const COEFF_A: Self::BaseField;
const COEFF_D: Self::BaseField;
const COFACTOR: &'static [u64];
const COFACTOR_INV: Self::ScalarField;
const AFFINE_GENERATOR_COEFFS: (Self::BaseField, Self::BaseField);

type MontgomeryModelParameters: MontgomeryModelParameters<BaseField = Self::BaseField>;

#[inline(always)]
fn mul_by_a(elem: &Self::BaseField) -> Self::BaseField {
let mut copy = *elem;
copy *= &Self::COEFF_A;
copy
}
}

use {MontgomeryModelParameters as MontgomeryParameters, TEModelParameters as Parameters};

#[derive(Derivative)]
#[derivative(
Copy(bound = "P: Parameters"),
Expand Down Expand Up @@ -180,7 +203,7 @@ impl<P: Parameters> Neg for GroupAffine<P> {
}
}

ark_ff::impl_additive_ops_from_ref!(GroupAffine, Parameters);
impl_additive_ops_from_ref!(GroupAffine, Parameters);

impl<'a, P: Parameters> Add<&'a Self> for GroupAffine<P> {
type Output = Self;
Expand Down Expand Up @@ -433,6 +456,11 @@ impl<P: Parameters> ProjectiveCurve for GroupProjective<P> {
type ScalarField = P::ScalarField;
type Affine = GroupAffine<P>;

#[inline(always)]
fn get_x(&mut self) -> &mut Self::BaseField {
&mut self.x
}

fn prime_subgroup_generator() -> Self {
GroupAffine::prime_subgroup_generator().into()
}
Expand Down Expand Up @@ -708,7 +736,6 @@ impl<P: MontgomeryParameters> MontgomeryGroupAffine<P> {
}
}
}

impl<P: Parameters> CanonicalSerialize for GroupAffine<P> {
#[allow(unused_qualifications)]
#[inline]
Expand Down Expand Up @@ -773,24 +800,29 @@ impl<P: Parameters> CanonicalSerialize for GroupProjective<P> {

impl<P: Parameters> CanonicalDeserialize for GroupAffine<P> {
#[allow(unused_qualifications)]
fn deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
fn deserialize<R: Read>(reader: R) -> Result<Self, SerializationError> {
let p = Self::deserialize_unchecked(reader)?;
if !p.is_in_correct_subgroup_assuming_on_curve() {
return Err(SerializationError::InvalidData);
}
Ok(p)
}
#[allow(unused_qualifications)]
fn deserialize_unchecked<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
let (x, flags): (P::BaseField, EdwardsFlags) =
CanonicalDeserializeWithFlags::deserialize_with_flags(&mut reader)?;
if x == P::BaseField::zero() {
Ok(Self::zero())
} else {
let p = GroupAffine::<P>::get_point_from_x(x, flags.is_positive())
.ok_or(SerializationError::InvalidData)?;
if !p.is_in_correct_subgroup_assuming_on_curve() {
return Err(SerializationError::InvalidData);
}
Ok(p)
}
}

#[allow(unused_qualifications)]
fn deserialize_uncompressed<R: Read>(reader: R) -> Result<Self, SerializationError> {
let p = Self::deserialize_unchecked(reader)?;
let p = Self::deserialize_uncompressed_unchecked(reader)?;

if !p.is_in_correct_subgroup_assuming_on_curve() {
return Err(SerializationError::InvalidData);
Expand All @@ -799,7 +831,9 @@ impl<P: Parameters> CanonicalDeserialize for GroupAffine<P> {
}

#[allow(unused_qualifications)]
fn deserialize_unchecked<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
fn deserialize_uncompressed_unchecked<R: Read>(
mut reader: R,
) -> Result<Self, SerializationError> {
let x: P::BaseField = CanonicalDeserialize::deserialize(&mut reader)?;
let y: P::BaseField = CanonicalDeserialize::deserialize(&mut reader)?;

Expand Down
5 changes: 3 additions & 2 deletions ff-asm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![recursion_limit = "128"]

use proc_macro::TokenStream;
use quote::quote;
use syn::{
parse::{Parse, ParseStream},
Expr, Item, ItemFn,
Expand Down Expand Up @@ -38,9 +39,9 @@ pub fn unroll_for_loops(_meta: TokenStream, input: TokenStream) -> TokenStream {
block: Box::new(new_block),
..item_fn
});
quote::quote! ( #new_item ).into()
quote! ( #new_item ).into()
} else {
quote::quote! ( #item ).into()
quote! ( #item ).into()
}
}

Expand Down
Loading