Skip to content

Conversation

carloskiki
Copy link

@carloskiki carloskiki commented Apr 27, 2025

The driving reason for implementing this is to eventually implement the hash2curve traits for elligator2.

See also: #612, #492.

Main Concern for Ed25519

The CurveArithmetic trait requires an AffinePoint for which this crate only has CompressedEdwardsY. The problem is that CurveArithmetic requires an Infallible conversion from AffinePoint (CompressedEdwardsY) to ProjectivePoint (EdwardsPoint).

Currently, this implementation skips the fallible implementation of CompressedEdwardsY -> EdwardsPoint and creates an invalid EdwardsPoint if CompressedEdwardsY is invalid (we probably don't want this).

I can think of 2 other solutions:

  • We introduce a new pub AffineEdwardsPoint { x: FieldElement, y: FieldElement } (with private internals) to implement Curve & CurveArithmetic. I believe this would not be a breaking change.
  • We make the internals of CompressedEdwardsY private (Breaking change), and introduce a type alias for the the wire representation of CompressedEdwardsY e.g.:
    type EdwardsYBytes = [u8; 32];
    Then the creation of CompressedEdwardsY would require validation and would be fallible.
    See also: Should CompressedEdwardsY guarantee validity? #745.

Main Concern for Curve25519:

It does not seem like there is a public ProjectivePoint for curve25519. One solution would be to make the current one public.

Note

Only Ed25519 has the traits implemented (hence why this is a draft), but I would like feedback on this implementation, as I feel the implementation for Curve25519 will have similar issues. The code contains QUESTIONs that explain the issues faced.

Both Ed25519 and Curve25519 will be housed in the crate curve25519, since this is where the arithmetic is implemented for both curves.

@tarcieri
Copy link
Contributor

@carloskiki there will hopefully be a v0.14 release of elliptic-curve out soon along with corresponding v0.14 releases of ff/group. It would be good to target those (at least the prereleases currently available), though that may need other upgrades elsewhere in the project.

The CurveArithmetic trait requires an AffinePoint for which this crate only has CompressedEdwardsY

CompressedEdwardsY is an encoding which must first be decompressed before you have an affine representation. It's more analogous to the SEC1 encoding which is the main one supported by elliptic-curve currently.

We introduce a new pub AffineEdwardsPoint { x: FieldElement, y: FieldElement } (with private internals) to implement Curve & CurveArithmetic. I believe this would not be a breaking change.

This would probably make the most sense as a way to support the current elliptic-curve API. It's already computed inside of e.g. EdwardsPoint::compress, though it's less helpful in the point decompression case.

Perhaps the elliptic-curve API could be better generalized so as not to expose an AffinePoint/ProjectivePoint distinction, which isn't particularly helpful with Curve25519, and instead just having a single type for elements of the curve group. In the https://github.com/RustCrypto/elliptic-curves crates there could potentially instead be a Point enum that picks the appropriate formula based on the given variant which would also simplify the API to end users, but might require a somewhat difficult refactoring. We're making breaking changes now though, so it's something to consider.

tarcieri added a commit that referenced this pull request Jun 6, 2025
Based on discussions about `elliptic-curve` trait impls in #746, and
observing a similar type in `ed448-goldilocks` which inspired this one
(not to mention in all of the @RustCrypto elliptic curve crates), adds
an `AffinePoint` type with `x` and `y` coordinates.

For now, the type is kept out of the public API, and used as an
implementation detail for point compression. However, it's been written
with the intent of eventually stabilizing and exposing it. It's been
marked `pub` so unused functionality doesn't automatically trigger dead
code lints.

Further work could include refactoring point decompression to first
produce an `AffinePoint` and then convert to extended twisted Edwards
coordinates (i.e. `EdwardsPoint`), which is more or less what the
existing `step_1` and `step_2` functions do (`step_1` technically
produces projective coordinates, but `Z` is always set to `ONE`).
tarcieri added a commit that referenced this pull request Jun 6, 2025
Based on discussions about `elliptic-curve` trait impls in #746, and
observing a similar type in `ed448-goldilocks` which inspired this one
(not to mention in all of the @RustCrypto elliptic curve crates), adds
an `AffinePoint` type with `x` and `y` coordinates.

For now, the type is kept out of the public API, and used as an
implementation detail for point compression. However, it's been written
with the intent of eventually stabilizing and exposing it. It's been
marked `pub` so unused functionality doesn't automatically trigger dead
code lints.

Further work could include refactoring point decompression to first
produce an `AffinePoint` and then convert to extended twisted Edwards
coordinates (i.e. `EdwardsPoint`), which is more or less what the
existing `step_1` and `step_2` functions do (`step_1` technically
produces projective coordinates, but `Z` is always set to `ONE`).
tarcieri added a commit that referenced this pull request Jun 6, 2025
Based on discussions about `elliptic-curve` trait impls in #746, and
observing a similar type in `ed448-goldilocks` which inspired this one
(not to mention in all of the @RustCrypto elliptic curve crates), adds
an `AffinePoint` type with `x` and `y` coordinates.

For now, the type is kept out of the public API, and used as an
implementation detail for point compression. However, it's been written
with the intent of eventually stabilizing and exposing it. It's been
marked `pub` so unused functionality doesn't automatically trigger dead
code lints.

Further work could include refactoring point decompression to first
produce an `AffinePoint` and then convert to extended twisted Edwards
coordinates (i.e. `EdwardsPoint`), which is more or less what the
existing `step_1` and `step_2` functions do (`step_1` technically
produces projective coordinates, but `Z` is always set to `ONE`).
rozbb pushed a commit that referenced this pull request Jun 8, 2025
* curve: extract `AffinePoint` type

Based on discussions about `elliptic-curve` trait impls in #746, and
observing a similar type in `ed448-goldilocks` which inspired this one
(not to mention in all of the @RustCrypto elliptic curve crates), adds
an `AffinePoint` type with `x` and `y` coordinates.

For now, the type is kept out of the public API, and used as an
implementation detail for point compression. However, it's been written
with the intent of eventually stabilizing and exposing it. It's been
marked `pub` so unused functionality doesn't automatically trigger dead
code lints.

Further work could include refactoring point decompression to first
produce an `AffinePoint` and then convert to extended twisted Edwards
coordinates (i.e. `EdwardsPoint`), which is more or less what the
existing `step_1` and `step_2` functions do (`step_1` technically
produces projective coordinates, but `Z` is always set to `ONE`).

* Update curve25519-dalek/src/edwards.rs

/// QUESTION: I don't know where to put this singleton. Maybe in the crate's root?
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)]
pub struct Ed25519;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ed25519 is the name of the EdDSA variant.

RFC7748 calls the Edwards form of the curve "edwards25519"

@carloskiki
Copy link
Author

I opened RustCrypto/traits#1900 to see if there would be interest in using a single Point type instead of the affine and projective variants. Since elliptic-curve v0.14.0 is already in release candidate phase this may have to wait to v0.15.0 if we decide this is a good idea. I will be drafting some traits to see how it would work soon.

In the mean time we may want to wait to implement these traits, especially for curve25519, since none of the required mathematical operations for montgomery::ProjectivePoint are implemented. I hardly see a use in implementing those other than to satisfy the trait bounds.

@carloskiki carloskiki marked this pull request as draft June 11, 2025 01:13

use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::affine::AffinePoint, EdwardsPoint, Scalar};

/// QUESTION: I don't know where to put this singleton. Maybe in the crate's root?
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably put it in edwards.rs

@tarcieri
Copy link
Contributor

@carloskiki I think it's fine to impl them just for EdwardsPoint and RistrettoPoint for now

rozbb pushed a commit that referenced this pull request Jul 7, 2025
* curve: extract `AffinePoint` type

Based on discussions about `elliptic-curve` trait impls in #746, and
observing a similar type in `ed448-goldilocks` which inspired this one
(not to mention in all of the @RustCrypto elliptic curve crates), adds
an `AffinePoint` type with `x` and `y` coordinates.

For now, the type is kept out of the public API, and used as an
implementation detail for point compression. However, it's been written
with the intent of eventually stabilizing and exposing it. It's been
marked `pub` so unused functionality doesn't automatically trigger dead
code lints.

Further work could include refactoring point decompression to first
produce an `AffinePoint` and then convert to extended twisted Edwards
coordinates (i.e. `EdwardsPoint`), which is more or less what the
existing `step_1` and `step_2` functions do (`step_1` technically
produces projective coordinates, but `Z` is always set to `ONE`).

* Update curve25519-dalek/src/edwards.rs
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.

2 participants