Change GroupAffine/GroupProjective to maintain invariants

[hdevalence]

Currently the GroupAffine and GroupProjective structs do not maintain either of these invariants:

• the point is on the curve;
• the point is in the prime-order subgroup;

The GroupAffine struct provides methods to check these, but GroupProjective does not. The API could be improved by:

• removing the invariant check methods;
• adding the invariant checks to the new() method and making it fallible;
• adding immutable field accessors fn x(&self) -> &F, fn y(&self) -> &F, etc;
• making the coordinate fields private.

This ensures that any users of the structures outside of the module subtree can't violate these invariants.

[daira]

Please consider using the zkcrypto/group and zkcrypto/ff crates (the latter probably without the "derive" feature), if Arkworks' group APIs are going to change. These crates are pretty stable now (despite the 0.9 version number) and used in production for Zcash, including in bellman (Zcash version), and bls12_381. Interoperability with them would be very useful, and they're quite minimal dependencies.

[hdevalence]

As a user rather than a maintainer of arkworks, I don't really have a stake in this, but I'm not sure it's fair to describe those crates as being minimal dependencies -- regardless of code size or number of transitive dependencies, a fundamental abstraction like a group or field trait is a significant dependency, affecting nearly every part of the codebase, and switching to it isn't trivial.

[burdges]

If this happens, it's worth retaining some new_unchecked interface for points not in the prime order subgroup: You can avoid doublings like ((..((c + a_1) + a_2)..) + a_n) - c with c independent. I've always seen this trick with DDH yielding independence of c, but I've heard people argue for also placingc in another coset, and maybe that's useful sometimes for reducing constraint count at the cost of fragility elsewhere. This seems orthogonal to everything else discussed here.

[daira]

@hdevalence That's why I only suggested switching to it in the case that Arkworks' APIs were going to change in order to make essentially the same PrimeGroup/CofactorGroup distinction that the group crate makes — which had been proposed.

[Pratyush]

Hmm I don't think we intend to have separate traits for PrimeGroup and CofactorGroup; the discussion in this issue is about changing APIs on certain generic structs (twisted_edwards_extended::Group{Affine,Projective} and short_weierstrass_jacobian::Group{Affine,Projective})