zkcrypto / group

Elliptic curve group traits and utilities.
Other
91 stars 32 forks source link

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

Open tarcieri opened 2 years ago

tarcieri commented 2 years ago

Similar suggestion to https://github.com/zkcrypto/ff/issues/87

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

kayabaNerve commented 2 years ago

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 commented 1 year ago

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).