Skip to content

Conversation

@tarcieri
Copy link
Contributor

@tarcieri tarcieri commented 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).

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 tarcieri force-pushed the curve/edwards-affine branch from 44d86ab to cdf999e Compare June 6, 2025 22:13
@rozbb rozbb merged commit 8c53a8f into main Jun 8, 2025
46 checks passed
@rozbb rozbb deleted the curve/edwards-affine branch June 9, 2025 04:25
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
@kayabaNerve
Copy link
Contributor

I was considering a PR to support deserialization of uncompressed points, as I'm currently paying a notable performance penalty reading points from a binary (generated at compile-time) for no reason when I found this PR. As a note, I'd love for a public AffinePoint I can load an x/y coordinate into and convert into a EdwardsPoint.

@tarcieri
Copy link
Contributor Author

@kayabaNerve you're fine with it still checking the coordinates are a valid solution to the curve equation, right? (it would still avoid the sqrt decompression requires)

I would be okay with a public checked constructor like that, when and if we do decide to make AffinePoint public.

@kayabaNerve
Copy link
Contributor

Of course. The few multiplications there are <5% of the sqrt though 😅

@tarcieri
Copy link
Contributor Author

@kayabaNerve want to open a PR for discussion to export AffinePoint as pub and add a public constructor which takes the coordinates?

@kayabaNerve
Copy link
Contributor

Working on this after #787. It presumably will have to build upon that, unless we define the coordinates as [u8; 32]...

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.

4 participants