feat: general extensions and documentation #58
Conversation
| use super::*; | ||
|
|
||
| // TODO: An empty struct like this and G2Curve is a code smell. We should probably |
There was a problem hiding this comment.
This was a design @thor314 had taken I believe from artworks? (correct me if I am wrong). I am okay thinking about a single struct too.
There was a problem hiding this comment.
You could at least do a single struct like
struct CurveGroup<F: FiniteField> {
_phantom: F,
}This way you can just use one struct and define it over whatever field you'd like.
I think there's probably something even nicer we could do to better utilize the Curve as we have as of now.
| } | ||
| } | ||
|
|
||
| // TODO: This should likely use a `Self::ScalarField` instead of `u32`. |
There was a problem hiding this comment.
not sure I see why? Scaler mult should work for any number (point can be doubled any amount of times), in the field or not. if we constrain the type here what does it give us? It seems like it would make the interface harder. I am down to turn it into the scalar field under the hood here, but that seems like an optimization
There was a problem hiding this comment.
My thought is that we should define a ScalarField for the curve anyway because we will eventually need access to this. It's a bit odd to multiply by a number that isn't in the scalar field of a curve, it's not really very intuitive to think about it that way to me at least since you should reduce mod SCALAR_FIELD_ORDER prior to computing it.
There was a problem hiding this comment.
As far as i understand all elliptic curve points should be able to be multiplied by an arbitrarily large number. (under the hood this is just point doubling and point addition). No it's clear that it might be redundant to do so (especially if it is bigger than the scaler field order like you point out). But i think the interface should take any number and just abstractly mod the scaler order under the hood before point doubling. That just feels like a more flexible interface to me. But also i don't mind changing it since this is for learning.
This PR will close #55