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

Introduce CurveAffine trait #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce CurveAffine trait #48

wants to merge 1 commit into from

Conversation

str4d
Copy link
Member

@str4d str4d commented Jul 29, 2023

This unifies the methods previously exposed by the PrimeCurveAffine and CofactorCurveAffine traits. The prime-order and cofactor traits are now all marker traits, and their affine-specific traits are automatically derived.

@str4d
Copy link
Member Author

str4d commented Jul 29, 2023

I tried to do this years ago, but back then rustc couldn't handle it. Apparently as of at least 1.56 it can; I'm able to use these updated traits in:

@tarcieri it would be great if you could test this PR with the elliptic-curve crate and the downstream RustCrypto curve crates.

Comment on lines +135 to +136
/// Scalars modulo the order of this group's scalar field.
///
/// This associated type is temporary, and will be removed once downstream users have
/// migrated to using `Curve` as the primary generic bound.
type Scalar: PrimeField;
Copy link
Member Author

@str4d str4d Jul 29, 2023

Choose a reason for hiding this comment

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

I want to remove this associated type (and instead use <Self::Curve as Group>::Scalar, but for annoying historical type-checking reasons we used (the equivalent of) C: PrimeCurveAffine as the main generic parameter in https://github.com/zcash/halo2. I want to change it to use C: PrimeCurve instead (or G: PrimeGroup where I can), but before I can do that I need to kill the CurveExt trait (zcash/pasta_curves#41). So this associated type will stay until that is complete (though I hope to complete 41 in the same release cycle as this PR).

On that note, it would be good to know if any other downstream dependencies do the same thing.

Comment on lines +141 to +139
/// Returns the additive identity.
fn identity() -> Self;
Copy link
Member Author

Choose a reason for hiding this comment

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

If we end up landing #42 in its current form, then we'd do the obvious thing and replace these methods with CurveAffine: Identity.

This unifies the methods previously exposed by the `PrimeCurveAffine`
and `CofactorCurveAffine` traits. The prime-order and cofactor traits
are now all marker traits, and their affine-specific traits are
automatically derived.
@dragan2234
Copy link

hey there @str4d is there any progress on this PR? I'm asking because the privacy-scaling-explorations/halo2curves#38 is dependent on this, and I would like to test pedersen commitment circuit in proof system based on bls12-381 curve operations. Motivation is here: https://hackmd.io/l2XvogKoQOCH748T1rdWZw

@CPerezz
Copy link

CPerezz commented Sep 11, 2023

hey there @str4d is there any progress on this PR? I'm asking because the privacy-scaling-explorations/halo2curves#38 is dependent on this, and I would like to test pedersen commitment circuit in proof system based on bls12-381 curve operations. Motivation is here: https://hackmd.io/l2XvogKoQOCH748T1rdWZw

We're waiting for this and all the bumps in all the child libs (pasta, bls etc..). I have the visibility changes ready. And ready to be upstreamed (in case we can avoid vendoring).

But for now, our CurveEndo trait relies on pasta_curves::CurveExt which means we cannot cleanly implement this. (I'll try to impl CurveExt in our vendored dep temporarily while this is not fixed) See https://github.com/privacy-scaling-explorations/bls12_381.

huitseeker added a commit to lurk-lab/arecibo that referenced this pull request Dec 23, 2023
- the DlogGroup trait is now group-crate aware, and requires traits in those terms,
- the requirements will be further streamlined when zkcrypto/group#48 merges
- simplified declarations boilerplate in halo2curves & pasta macros
- removed boilerplate macro duplication for grumpkin_msm.
huitseeker added a commit to lurk-lab/arecibo that referenced this pull request Dec 23, 2023
- the DlogGroup trait is now group-crate aware, and requires traits in those terms,
- the requirements will be further streamlined when zkcrypto/group#48 merges
- simplified declarations boilerplate in halo2curves & pasta macros
- removed boilerplate macro duplication for grumpkin_msm.
huitseeker added a commit to lurk-lab/arecibo that referenced this pull request Dec 23, 2023
- the DlogGroup trait is now group-crate aware, and requires traits in those terms,
- the requirements will be further streamlined when zkcrypto/group#48 merges
- simplified declarations boilerplate in halo2curves & pasta macros
- removed boilerplate macro duplication for grumpkin_msm.
huitseeker added a commit to lurk-lab/arecibo that referenced this pull request Dec 23, 2023
- the DlogGroup trait is now group-crate aware, and requires traits in those terms,
- the requirements will be further streamlined when zkcrypto/group#48 merges
- simplified declarations boilerplate in halo2curves & pasta macros
- removed boilerplate macro duplication for grumpkin_msm.
github-merge-queue bot pushed a commit to lurk-lab/arecibo that referenced this pull request Dec 25, 2023
* refactor: Refactor trait imports in provider/traits.rs

- Refactored `provider/traits.rs` to remove local definitions of helper traits.
- Incorporated `GroupOps`, `GroupOpsOwned`, and `ScalarMulOwned` from `group` module into `provider/traits.rs` to maintain functionality.

* refactor: Refactor `DlogGroup` trait and optimize batch operations

- the DlogGroup trait is now group-crate aware, and requires traits in those terms,
- the requirements will be further streamlined when zkcrypto/group#48 merges
- simplified declarations boilerplate in halo2curves & pasta macros
- removed boilerplate macro duplication for grumpkin_msm.

* fix: adjust macro invocations for wasm32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants