-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add extension traits for exposing coordinates of curve points #49
base: curveaffine
Are you sure you want to change the base?
Conversation
We use extension traits to limit how generic the generic code can be, forcing it to be aware of the specific curve model that the coordinates are associated with. Closes #30.
/// An affine elliptic curve point on a twisted Edwards curve | ||
/// $a \cdot x^2 + y^2 = 1 + d \cdot x^2 \cdot y^2$. | ||
pub trait TwistedEdwardsPoint: CurveAffine + Default + ConditionallySelectable { | ||
/// Field element type used in the curve equation. | ||
type Base: Copy + ConditionallySelectable; | ||
|
||
/// The parameter $a$ in the twisted Edwards curve equation. | ||
/// | ||
/// When $a = 1$, this reduces to an ordinary Edwards curve. | ||
const A: Self::Base; | ||
|
||
/// The parameter $d$ in the twisted Edwards curve equation. | ||
const D: Self::Base; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not yet sure whether this trait should include the model parts as well, or whether that should be via an extension trait of the Curve
trait. In general downstream users should use the Curve
trait as the primary downstream interface, which would mean applying a bound of the form:
fn foo<C: Curve>() where C::Affine: TwistedEdwardsPoint {}
However, if we're not going to expose model-related APIs on the Curve
type, then this is probably overall a simpler trait hierarchy, and a reasonable pathway to take (that we could always change track on later after we get some experience with it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: is there a specific reason that MontgomeryPoint
is necessarily an Affine
subtype? It also has a projective representation. Would it make sense to separate the notion of curve equation and coordinate representation?
From an API perspective, I think that'd mean writing trait MontgomeryPoint
as below, but without the methods. And then defining struct AffineMontgomeryPoint<Base> { u: Base, v: Base }
and impl<F> MontgomeryPoint<Base=F> for AffineMontgomeryPoint<F>
or something like that. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: is there a specific reason that
MontgomeryPoint
is necessarily anAffine
subtype? It also has a projective representation. Would it make sense to separate the notion of curve equation and coordinate representation?
This is somewhat an artifact of the trait approach we've taken in the group
crate: the Group
trait denotes both a cryptographic group, and elements of that group. So there is currently no single place where a curve-only trait could live.
What I was suggesting above was that we would effectively do this split, by way of having an extension trait of Curve
where the curve model lives, but that trait would also be where any efficient-representation methods would need to live. Except that the efficient representation is not explicitly any specific representation; it's "whatever representation is best for this curve implementation", meaning that there's nothing to expose there.
So if we wanted to expose the projective representation, it would likely instead be an extension trait to the extension trait, which starts making the trait hierarchy quite complex, and we'd need a way to convey and manage that complexity. I'm not against
From an API perspective, I think that'd mean writing
trait MontgomeryPoint
as below, but without the methods. And then definingstruct AffineMontgomeryPoint<Base> { u: Base, v: Base }
andimpl<F> MontgomeryPoint<Base=F> for AffineMontgomeryPoint<F>
or something like that. Does that make sense?
The core method that the current trait needs is from_base_coordinates
. That is necessary for safety: I want the coordinates struct to be guaranteed to represent a valid coordinate tuple, which requires having a validity-checked from-bare-coordinates constructor we can jump off to construct the coordinates struct.
So if we do split out a curve-only trait, we'd split this trait in the middle: have trait MontgomeryCurve: Curve
, and then trait AffineMontgomeryPoint: CurveAffine where Self::Curve: MontgomeryPoint
. Representing that trait bound in the type system is likely rather complex, but I'll have a go and see if it is workable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand. I'm all for keeping it simple. Do you think there's an easy way to have to two representations for an Edwards point? This has affine but curve25519-dalek needs XYZT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from_bare_coordinates
can convert to extended twisted Edwards coordinates by setting Z=1 and T=X*Y (and calling is_valid
on the resulting point)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is somewhat an artifact of the trait approach we've taken in the group crate: the Group trait denotes both a cryptographic group, and elements of that group. So there is currently no single place where a curve-only trait could live.
@str4d FWIW, in the elliptic-curve
crate we have ZSTs which represent curve types and act as trait carriers (namely for traits that define the various associated element types)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think there's an easy way to have to two representations for an Edwards point? This has affine but curve25519-dalek needs XYZT
@rozbb to clarify, it needs to expose the coordinates of XYZT? If not and it just needs to use such points, that is already the case before this PR, and the idea behind the difference between the Curve
and CurveAffine
traits:
CurveAffine
represents specifically affine coordinates that are both closer to the encoding, and can be a type used for mixed addition or scalar mul.Curve
represents the efficient type of a curve point for the given implementation; in the case of curve25519-dalek the XYZT representation.
If you want to expose the XYZT coordinates at all, then you can do that with concrete methods in curve25519-dalek
and don't need changes in group
. If you want to expose them generically, then we'd need to discuss in what context this needs to be used (which should probably go into #30, or if sufficiently different then a new issue):
- If the intent is to write generic code for the subset of efficient point impls that use XYZT coords, then extension traits on
Curve
that constrain to a given efficient model may be sufficient. - If the intent is to obtain XYZT coords for given points, regardless of what coords they currently are in, then we'd need to define a trait or struct space that forms a state space of compatible coordinate models, along with an extension trait that allows converting the efficient
Curve
type from whatever its model is to the target model (which might be a no-op, or might not).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main one (and also one of the main integration points with the group
crate, as it were): https://docs.rs/elliptic-curve/latest/elliptic_curve/trait.CurveArithmetic.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@str4d you're right, we don't need to expose our specific representation. Any equivalent one should do, at least for a first draft
|
||
/// An affine elliptic curve point on a twisted Edwards curve | ||
/// $a \cdot x^2 + y^2 = 1 + d \cdot x^2 \cdot y^2$. | ||
pub trait TwistedEdwardsPoint: CurveAffine + Default + ConditionallySelectable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, following the existing group
crate conventions (given that we use the one trait to represent both the curve and points on the curve), we should probably name this:
pub trait TwistedEdwardsPoint: CurveAffine + Default + ConditionallySelectable { | |
pub trait TwistedEdwardsCurve: CurveAffine + Default + ConditionallySelectable { |
We use extension traits to limit how generic the generic code can be, forcing it to be aware of the specific curve model that the coordinates are associated with.
Closes #30.