Skip to content
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

Draft
wants to merge 1 commit into
base: curveaffine
Choose a base branch
from

Conversation

str4d
Copy link
Member

@str4d str4d commented Jul 30, 2023

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.

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.
Comment on lines +17 to +29
/// 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;
Copy link
Member Author

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).

Copy link

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?

Copy link
Member Author

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?

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 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?

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.

Copy link

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

Copy link
Contributor

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)

Copy link
Contributor

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)

Copy link
Member Author

@str4d str4d Jul 31, 2023

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).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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)

@tarcieri Can you point me to these? I couldn't find them in the public API for the elliptic-curve crate.

Copy link
Contributor

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

Copy link

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 {
Copy link
Member Author

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:

Suggested change
pub trait TwistedEdwardsPoint: CurveAffine + Default + ConditionallySelectable {
pub trait TwistedEdwardsCurve: CurveAffine + Default + ConditionallySelectable {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants