Add Ed448 (Goldilocks) curve support by sridhar-panigrahi · Pull Request #61 · LFDT-Lockness/generic-ec

sridhar-panigrahi · 2026-03-23T21:30:33Z

Following up on #58 — updated based on reviewer feedback to use ed448 instead of curve448.

What changed

Renamed to Ed448, CURVE_NAME = "ed448", feature flag curve-ed448
Scalar encoding is now 57 bytes (RFC 8032) instead of 56
Zeroize derived on the curve struct, Point, and Scalar
FromUniformBytes now uses 84 bytes (k = 224 per RFC 9380)
Reduce<57> and Reduce<114> replacing the old Reduce<56>/Reduce<112>

All tests pass.

Adds Curve448 as a new curve backend using ed448-goldilocks-plus. Follows the same manual trait implementation approach as Ed25519 since Curve448 uses Edwards encoding rather than SEC1. - 57-byte compressed point encoding (CompressedEdwardsY) - 56-byte scalar encoding with Reduce<56> and Reduce<112> - Cofactor-4 torsion checking via CofactorGroup - All existing tests instantiated for the new curve Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com> Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

socket-security · 2026-03-23T21:30:57Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	ed448-goldilocks-plus@0.16.0

View full report

sridhar-panigrahi · 2026-03-23T21:41:30Z

@survived , please let me know you thoughts on this !

Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com> Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

survived · 2026-03-28T14:33:02Z

Hey @sridhar-panigrahi, sorry, it will take me some time to review the PR as I'm pressed by some other work. I should be able to get on this in a couple of weeks unless @maurges gets to it first!

sridhar-panigrahi · 2026-03-29T13:10:07Z

it's ok @survived , I'll try to find and work on something even more valuable on this repo .

sridhar-panigrahi · 2026-03-29T18:57:36Z

@maurges , please let me know your thoughts on this !

maurges · 2026-04-03T15:46:20Z

I left a couple of comments, but the big question is whether we want this curve to be curve448 (56 byte encoded scalar) or ed448 (57 byte encoded scalar). I'm tending towards the second, since the biggest usage of this curve is for signatures after all, but maybe curve448 is more general and is easy enough to adapt to signatures too. Or maybe it's possible to have both

survived · 2026-04-08T09:23:55Z

We def need ed448 not curve448, to align with existing ed25519

I still haven't had time to look at PR, but curve448 is in a twisted form in which there's no point addition defined. The only way around is by mapping points from curve448 to ed448, perform point addition, and map back. This is extremely inefficient, we should not do that.

survived · 2026-04-08T09:27:22Z

Sorry, it's my bad, I wrote curve448 in #58. I meant ed448

maurges · 2026-04-08T09:52:04Z

The only way around is by mapping points from curve448 to ed448, perform point addition, and map back

If I understand right, here internally everything is treated as ed448 values, and is only encoded as curve448

sridhar-panigrahi · 2026-04-10T17:01:51Z

Hey @survived and @maurges, I've updated the PR to address your feedback:

Switched from curve448 to ed448 as requested — renamed struct to Ed448, set CURVE_NAME = "ed448", and changed scalar encoding to 57 bytes (RFC 8032 format)
Added zeroize::Zeroize derive to the curve struct, Point, and Scalar
Used GenericArray::into() in to_bytes_compressed instead of manual copy
Fixed FromUniformBytes to use L = 84 bytes (k = 224, as you pointed out @maurges)
Also renamed the feature flag from curve448 to ed448 (and curve-curve448 to curve-ed448 in the top-level crate)

Let me know if there's anything else to adjust!

- Rename Curve448 to Ed448, CURVE_NAME = ed448 - Scalar encoding changed to 57 bytes (RFC 8032) - Derive Zeroize on Ed448, Point, and Scalar structs - Use GenericArray::into() in to_bytes_compressed - Fix FromUniformBytes to use 84 bytes (k=224 per RFC 9380) - Replace Reduce<56>/Reduce<112> with Reduce<57>/Reduce<114> - Fix Reduce<57> to use from_bytes_mod_order_wide - Fix Straus multiscalar for non-multiple-of-8 scalar byte lengths - Rename feature flags: curve448 to ed448, curve-curve448 to curve-ed448 Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

maurges

I marked three places with the usage of GenericArray::from_slice, but there are others, and all of them should be replaced

maurges · 2026-04-22T08:57:26Z

+        use group::GroupEncoding;
+        self.0.to_bytes().into()


Nitpick: it's inconsistent that above you use a fully qualified call to group::cofactor::CofactorGroup::is_torsion_free, but here you import instead.

My personal preference is to use fully-qualified calls. But importing traits does make the implementation look more similar to ed25519, which makes this review easier. So use your judgement

Used the inherent self.0.is_torsion_free() method directly — avoids both the verbose fully-qualified call and any trait import.

maurges · 2026-04-22T08:58:22Z

+impl core::fmt::Debug for Point {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        use generic_ec_core::CompressedEncoding;
+        f.debug_tuple("Point")
+            .field(&self.to_bytes_compressed())
+            .finish()
+    }
+}


What's the reasoning for an explicit Debug impl? Does it make it more similar to how the other curves in this crate behave? Or something else?

Removed both the explicit Debug impls for Point and Scalar — looking at ed25519.rs, it has no custom Debug on its raw types either, so this keeps things consistent.

maurges · 2026-04-22T09:23:25Z

+        let mut wide = [0u8; 114];
+        wide[..57].copy_from_slice(bytes);
+        Self(ed448_goldilocks_plus::Scalar::from_bytes_mod_order_wide(
+            GenericArray::from_slice(&wide),
+        ))


You can create wide as let mut wide = GenericArray::default(), and then there's no need to convert

Fixed — now uses GenericArray::default() directly for all the wide-buffer cases.

maurges · 2026-04-22T09:24:58Z

+impl generic_ec_core::Decode for Point {
+    fn decode(bytes: &[u8]) -> Option<Self> {
+        use group::GroupEncoding;
+        let ga = GenericArray::from_slice(bytes);


This will panic if the slice is too short. Better to use try_from_slice and return None

Fixed — added an explicit length check before calling from_slice, returning None on mismatch. (generic-array 0.14 doesn't have try_from_slice, so the length check achieves the same panic-safe result.)

maurges · 2026-04-22T09:26:16Z

+        let mut bytes_le = [0u8; 114];
+        bytes_le[..84].copy_from_slice(bytes);
+        Self(ed448_goldilocks_plus::Scalar::from_bytes_mod_order_wide(
+            GenericArray::from_slice(&bytes_le),
+        ))


You can create a GenericArray directly instead of first writing to an array

Fixed — now creates GenericArray::default() and fills it with copy_from_slice.

maurges · 2026-04-22T11:36:35Z

+
+    fn from_le_bytes_exact(bytes: &Self::Bytes) -> Option<Self> {
+        Option::from(ed448_goldilocks_plus::Scalar::from_canonical_bytes(
+            GenericArray::from_slice(bytes),


This can panic in runtime if we ever mistake the lengths. But there is an instance impl<'a, T, const N: usize> From<&'a [T; N]> for &'a GenericArray<T, ConstArrayLength<N>>, so you should be able to simply write bytes.into() here

Fixed for the [u8; 57] case using bytes.into(). For [u8; 114], generic-array 0.14 only implements From<&[T; N]> for specific sizes and 114 isn't among them, so used GenericArray::default() + copy_from_slice there instead.

sridhar-panigrahi · 2026-04-22T12:39:35Z

Thank you for the detailed review, @maurges! I've addressed all the feedback in the latest commit:

Replaced all GenericArray::from_slice calls — Decode::decode now does an explicit length check and returns None on mismatch; fixed-size array inputs use GenericArray::default() + copy_from_slice or bytes.into() (for [u8; 57], which has a From impl in generic-array 0.14; [u8; 114] does not, so the default+copy pattern is used there).
Removed the explicit Debug impls for Point and Scalar — they were unnecessary and inconsistent with how ed25519 handles it.
Simplified is_torsion_free to call self.0.is_torsion_free() directly, which avoids both the verbose fully-qualified call and any extra trait import.

Let me know if anything else needs attention!

Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi mentioned this pull request Mar 23, 2026

Add support of curves with larger scalar sizes #58

Open

Fix formatting and update wasm/nostd lockfile

e963cf0

Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com> Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi force-pushed the feat/add-curve448-support branch from 96ef316 to e963cf0 Compare March 23, 2026 21:52

maurges reviewed Apr 3, 2026

View reviewed changes

Comment thread generic-ec-curves/src/curve448.rs Outdated

maurges reviewed Apr 3, 2026

View reviewed changes

Comment thread generic-ec-curves/src/curve448.rs Outdated

maurges reviewed Apr 3, 2026

View reviewed changes

Comment thread generic-ec-curves/src/curve448.rs Outdated

maurges reviewed Apr 3, 2026

View reviewed changes

Comment thread generic-ec-curves/src/curve448.rs Outdated

sridhar-panigrahi changed the title ~~Add curve448 (Goldilocks) curve support~~ Add Ed448 (Goldilocks) curve support Apr 10, 2026

sridhar-panigrahi force-pushed the feat/add-curve448-support branch from 9261f3c to 61e45bb Compare April 10, 2026 17:49

sridhar-panigrahi force-pushed the feat/add-curve448-support branch from 61e45bb to 0081221 Compare April 10, 2026 17:53

maurges requested changes Apr 22, 2026

View reviewed changes

sridhar-panigrahi added a commit to sridhar-panigrahi/generic-ec that referenced this pull request Apr 22, 2026

Address PR LFDT-Lockness#61 reviewer feedback on ed448.rs

c2b114e

Address PR LFDT-Lockness#61 reviewer feedback on ed448.rs

882b678

Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi force-pushed the feat/add-curve448-support branch from c2b114e to 882b678 Compare April 22, 2026 12:42

sridhar-panigrahi requested a review from maurges April 22, 2026 12:45

Conversation

sridhar-panigrahi commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Uh oh!

socket-security Bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sridhar-panigrahi commented Mar 23, 2026

Uh oh!

survived commented Mar 28, 2026

Uh oh!

sridhar-panigrahi commented Mar 29, 2026

Uh oh!

sridhar-panigrahi commented Mar 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maurges commented Apr 3, 2026

Uh oh!

survived commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

survived commented Apr 8, 2026

Uh oh!

maurges commented Apr 8, 2026

Uh oh!

sridhar-panigrahi commented Apr 10, 2026

Uh oh!

maurges left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sridhar-panigrahi commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sridhar-panigrahi commented Mar 23, 2026 •

edited

Loading

socket-security Bot commented Mar 23, 2026 •

edited

Loading

survived commented Apr 8, 2026 •

edited

Loading