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

Consider promoting Group::{identity, generator} to associated constants #32

Open
tarcieri opened this issue Jul 31, 2022 · 2 comments
Open
Milestone

Comments

@tarcieri
Copy link
Contributor

Similar suggestion to zkcrypto/ff#87

Using associated constants instead of static methods allows using values in const contexts, like defining other constants or const fn.

@kayabaNerve
Copy link

kayabaNerve commented Aug 17, 2022

I am against making generator a constant. There's a variety of reasons to use alternate generators, and their generation frequently falls under non-const functions. The ability to re-declare a curve around a static is appreciated, albeit niche.

@str4d
Copy link
Member

str4d commented Dec 6, 2022

I think the generator side of this definitely needs some experimentation, because while there are logical reasons for choosing a particular generator as standard, they aren't as natural as the singular choice of Field::ONE.

Regarding Group::identity, I've tried changing it to Group::IDENTITY and am able to migrate both bls12_381 and pasta_curves to it. However, we would also want to change PrimeCurveAffine::identity and CofactorCurveAffine::identity as well, and here I run into a problem: BLS12-381 has no affine representation for the identity, with the "point at infinity" taking its place, and bls12_381::{G1Affine, G2Affine} mark whether they are the point at infinity with a subtle::Choice, which has no const constructor. Even if we only deployed the Group::IDENTITY change, there may be implementations of the group traits that similarly use non-const types within their G: Group type.

I'm therefore not going to block group 0.13 on this issue. We should take some time to investigate this issue, with the goal of having it resolved ahead of group 0.14 (which will include other changes to the group traits that are being refactored out of the pasta_curves::arithmetic::CurveExt etc. traits, as well as #30).

@str4d str4d added this to the 0.14 milestone Dec 7, 2022
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

No branches or pull requests

3 participants