Add parameters of Ed25519 and support twisted Edwards curves #317
Conversation
querolita
changed the title
|
Most of the diff is recompiling the bindings to make CI happy |
|
The files that this PR is directly modifying are: |
crypto/elliptic-curve.unit-test.ts
Outdated
|
|
||
| // Twisted Edwards curve tests | ||
|
|
||
| const Ed25519 = createCurveTwisted(TwistedCurveParams.Ed25519); |
There was a problem hiding this comment.
What's your preferred convention? CurveTwisted or TwistedCurve? Asking because in affine I believe I have read both.
crypto/elliptic-curve.ts
Outdated
| if (z === 0n) { | ||
| // degenerate case | ||
| return twistedZero; | ||
| } else if (z === 1n && g.x === 0n && g.y === 1n) { |
There was a problem hiding this comment.
If I didn't treat this case separately, the following else if would place a false in the infinity parameter and then unit tests would complain because they used to find assertions of (0,1,false) != (0,1,true). But note that this is a branch that does not exist in the affine counterpart, and it is assuming that the "only" infinity point is exactly (0,1,1).
crypto/elliptic-curve.ts
Outdated
| const Field = createField(p); | ||
| const Scalar = createField(order); | ||
| const one = { ...generator, infinity: false }; | ||
| const Endo = undefined; // for Ed25519 |
There was a problem hiding this comment.
Not sure how to generalize this for now. When we use this interface for something different than edwards25519, maybe this parameter should change.
|
TODO: subgroup check |
| // x/z | ||
| let x = mod(g.x * zinv, p); | ||
| // y/z | ||
| let y = mod(g.y * zinv, p); |
There was a problem hiding this comment.
I would enforce that if x === 0n and y === 1n, then we have the point at infinity. It happens when y = z.
Any point in projective coordinates that is in the representive class of (0 : 1) should be treated the point at infinity.
There was a problem hiding this comment.
shouldn't that be already taken into account in the normalized case that z=1, then if x=0 and y=1 the point {0,1} would be returned
crypto/elliptic-curve.ts
Outdated
| return Endo; | ||
| }, | ||
|
|
||
| from(g: { x: bigint; y: bigint }): GroupTwisted { |
There was a problem hiding this comment.
Don't you want to enforce to have x and y given as canonical values? (i.e. smaller than the order of the base field?). I would be highly in favor of this.
There was a problem hiding this comment.
I guess no, and then perform the operations modulo the order and check that it holds (like, {x: 0, y: 1+p} is the identity instead of failing). But I am open to other alternatives, whatever is the standard.
crypto/elliptic-curve.unit-test.ts
Outdated
| assert( | ||
| isOnCurve(pointOrder8) && isInSubgroup(pointOrder8), | ||
| isOnCurve(pointOrder8) && !isInSubgroup(pointOrder8), |
crypto/elliptic-curve.unit-test.ts
Outdated
| 'point on curve not in subgroup' | ||
| ); | ||
| assert(equal(scale(pointOrder8, 8n), zero), 'point has order 8'); | ||
| assert( | ||
| isOnCurve(pointOrder4) && !isInSubgroup(pointOrder4), |
| 'point on curve not in subgroup' | ||
| ); | ||
| assert( | ||
| isOnCurve(pointOrder2) && !isInSubgroup(pointOrder2), |
This is related to o1-labs/o1js#1283