group crate refactoring #230
Conversation
Sized is always part of the prelude, and binding on std causes compilation issues for no-std crates.
The type Curve*::Engine::Fr is equivalent to Curve*::Scalar, making Engine a redundant associated type.
Group represents a cryptographic group with a large prime-order subgroup and a small cofactor. PrimeGroup further constrains the group to have a cofactor of one.
The GroupOps trait represents the group operation (addition), and the combination of the group operation with group inversion (subtraction). Group inversion (negation) is constrained directly on the Group trait.
For prime-order groups, this may be Self.
Codecov Report
@@ Coverage Diff @@
## master #230 +/- ##
==========================================
+ Coverage 65.25% 65.37% +0.11%
==========================================
Files 106 106
Lines 14973 14968 -5
==========================================
+ Hits 9771 9785 +14
+ Misses 5202 5183 -19
Continue to review full report at Codecov.
|
The Scalar associated type is moved from CurveProjective to Group.
|
The first commit in this PR is also in #227; I'll rebase whichever PR is not merged first. |
|
|
|
We want to be able to apply FFTs either to elements of the group or to the scalars themselves, which requires us to reason about a shared "scalar" field and group-like arithmetic in either case. |
| /// Returns the additive identity. | ||
| fn zero() -> Self; | ||
| /// Returns the additive identity, also known as the "neutral element". | ||
| fn identity() -> Self; |
There was a problem hiding this comment.
I actually prefer zero for the additive identity. So I would prefer to revert all of the s/zero/identity/ changes. I find zero more natural especially if we're implementing Add and Neg. additive_identity is too long, and identity is ambiguous.
| /// Returns a fixed generator of unknown exponent. | ||
| fn one() -> Self; | ||
| /// Returns a fixed generator of the prime-order subgroup. | ||
| fn generator() -> Self::Subgroup; |
There was a problem hiding this comment.
This on the other hand, is much better.
| /// Determines if this point represents the point at infinity; the | ||
| /// additive identity. |
There was a problem hiding this comment.
| /// Determines if this point represents the point at infinity; the | |
| /// additive identity. | |
| /// Determines if this point represents the additive identity, or zero point. |
It's not the point at infinity for Edwards curves. (Edwards curves over the rationals, or incomplete Edwards curves, do have points at infinity that are not the additive identity. This is a hobby horse but I am right :-p )
There was a problem hiding this comment.
That's an old doc-comment; it should match the Group::identity doc-comment, which is currently:
/// Determines if this point is the identity.
The new
Grouptrait represents a cryptographic group with a large prime-order subgroup and a small cofactor. ThePrimeGrouptrait further constrains the group to have a cofactor of one.CurveProjectivenow primarily contains EC-specific functionality.Part of #161.