Skip to content

ed448-goldilocks: assorted fixes and improvements #1302

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Jul 20, 2025
Merged

ed448-goldilocks: assorted fixes and improvements #1302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
70dcc5c
`DecafPoint` is always torsion free
daxpedda Jul 12, 2025
eff37b1
Use `Uint::to_le_byte_array()` directly
daxpedda Jul 18, 2025
23938d2
Use `is_negative()` directly
daxpedda Jul 18, 2025
972302a
Enforce constant-time `FieldElement::square_n()`
daxpedda Jul 19, 2025
b6c3619
Simplify `FieldElement::square_n()` and protect against zero
daxpedda Jul 19, 2025
f1d5295
Use double optimization where appropriate
daxpedda Jul 11, 2025
0910712
Use `Uint::is_odd()` directly
daxpedda Jul 20, 2025
5a5c671
Hide `EDWARDS_BASEPOINT_ORDER`
daxpedda Jul 20, 2025
3a5b100
Use squaring instead of multiplication where appropriate
daxpedda Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions ed448-goldilocks/src/constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,3 @@ pub const DECAF_BASEPOINT: DecafPoint = DecafPoint(curve::twedwards::extended::E
Z: TWISTED_EDWARDS_BASE_POINT.Z,
T: TWISTED_EDWARDS_BASE_POINT.T,
});

/// `BASEPOINT_ORDER` is the order of the Ed448 basepoint, i.e.,
/// $$
/// \ell = 2^\{446\} + 0x8335dc163bb124b65129c96fde933d8d723a70aadc873d6d54a7bb0d.
/// $$
pub const EDWARDS_BASEPOINT_ORDER: EdwardsScalar = EdwardsScalar::new(ORDER);

/// `BASEPOINT_ORDER` is the order of the Decaf448 basepoint, i.e.,
/// $$
/// \ell = 2^\{446\} + 0x8335dc163bb124b65129c96fde933d8d723a70aadc873d6d54a7bb0d.
/// $$
pub const DECAF_BASEPOINT_ORDER: DecafScalar = DecafScalar::new(ORDER);
2 changes: 1 addition & 1 deletion ed448-goldilocks/src/curve/twedwards/extended.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl ExtendedPoint {

// Compute x
let xy = x * y;
let x_numerator = xy + xy;
let x_numerator = xy.double();
let x_denom = y.square() - (a * x.square());
let new_x = x_numerator * x_denom.invert();

Expand Down
2 changes: 1 addition & 1 deletion ed448-goldilocks/src/curve/twedwards/extensible.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ExtensiblePoint {
pub fn double(&self) -> ExtensiblePoint {
let A = self.X.square();
let B = self.Y.square();
let C = self.Z.square() + self.Z.square();
let C = self.Z.square().double();
let D = -A;
let E = (self.X + self.Y).square() - A - B;
let G = D + B;
Expand Down
6 changes: 3 additions & 3 deletions ed448-goldilocks/src/decaf/points.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::constants::{DECAF_BASEPOINT, DECAF_BASEPOINT_ORDER};
use crate::constants::DECAF_BASEPOINT;
use crate::curve::twedwards::extended::ExtendedPoint;
use crate::field::FieldElement;
use crate::*;
Expand Down Expand Up @@ -226,7 +226,7 @@ impl CofactorGroup for DecafPoint {
}

fn is_torsion_free(&self) -> Choice {
(self * DECAF_BASEPOINT_ORDER).ct_eq(&Self::IDENTITY)
Choice::from(1)
}
}

Expand Down Expand Up @@ -550,7 +550,7 @@ impl CompressedDecaf {
let (I, ok) = (v * u1_sqr).inverse_square_root();

let Dx = I * u1;
let Dxs = (s + s) * Dx;
let Dxs = s.double() * Dx;

let mut X = (Dxs * I) * v;
let k = Dxs * FieldElement::DECAF_FACTOR;
Expand Down
5 changes: 2 additions & 3 deletions ed448-goldilocks/src/edwards/extended.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use core::fmt::{Display, Formatter, LowerHex, Result as FmtResult, UpperHex};
use core::iter::Sum;
use core::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign};

use crate::constants::EDWARDS_BASEPOINT_ORDER;
use crate::curve::scalar_mul::variable_base;
use crate::curve::twedwards::extended::ExtendedPoint as TwistedExtendedPoint;
use crate::field::FieldElement;
Expand Down Expand Up @@ -673,7 +672,7 @@ impl EdwardsPoint {

// Compute x
let xy = x * y;
let x_numerator = xy + xy;
let x_numerator = xy.double();
let x_denom = y.square() - (a * x.square());
let new_x = x_numerator * x_denom.invert();

Expand Down Expand Up @@ -724,7 +723,7 @@ impl EdwardsPoint {
/// * `false` if `self` has a nonzero torsion component and is not
/// in the prime-order subgroup.
pub fn is_torsion_free(&self) -> Choice {
(self * EDWARDS_BASEPOINT_ORDER).ct_eq(&Self::IDENTITY)
(self * EdwardsScalar::new(ORDER)).ct_eq(&Self::IDENTITY)
}

/// Hash a message to a point on the curve
Expand Down
38 changes: 17 additions & 21 deletions ed448-goldilocks/src/field/element.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
use elliptic_curve::{
array::Array,
bigint::{
NonZero, U448, U704,
Integer, NonZero, U448, U704,
consts::{U56, U84, U88},
},
group::cofactor::CofactorGroup,
Expand Down Expand Up @@ -258,8 +258,7 @@ impl FieldElement {
pub const ZERO: Self = Self(ConstMontyType::new(&U448::ZERO));

pub fn is_negative(&self) -> Choice {
let bytes = self.to_bytes();
(bytes[0] & 1).into()
self.0.retrieve().is_odd()
}

/// Inverts a field element
Expand All @@ -276,13 +275,10 @@ impl FieldElement {
}

/// Squares a field element `n` times
fn square_n(&self, mut n: u32) -> FieldElement {
let mut result = self.square();
fn square_n<const N: u32>(&self) -> FieldElement {
Comment on lines -279 to +278
Copy link
Member

Choose a reason for hiding this comment

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

Curious if this change actually impacts codegen in any way (i.e. for the better)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't check the codegen, but I benchmarked the inversion method and there were no changes.

Copy link
Member

Choose a reason for hiding this comment

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

Feels kind of six of one, half dozen of another in that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this change to prevent accidental variable time usage.
But let me know and I can just remove it.

let mut result = *self;

// Decrease value by 1 since we just did a squaring
n -= 1;

for _ in 0..n {
for _ in 0..N {
result = result.square();
}

Expand Down Expand Up @@ -320,7 +316,7 @@ impl FieldElement {
}

pub fn double(&self) -> Self {
Self(self.0.add(&self.0))
Self(self.0.double())
}

/// Computes the inverse square root of a field element
Expand All @@ -333,25 +329,25 @@ impl FieldElement {
l2 = l1 * self;
l1 = l2.square();
l2 = l1 * self;
l1 = l2.square_n(3);
l1 = l2.square_n::<3>();
l0 = l2 * l1;
l1 = l0.square_n(3);
l1 = l0.square_n::<3>();
l0 = l2 * l1;
l2 = l0.square_n(9);
l2 = l0.square_n::<9>();
l1 = l0 * l2;
l0 = l1 * l1;
l0 = l1.square();
l2 = l0 * self;
l0 = l2.square_n(18);
l0 = l2.square_n::<18>();
l2 = l1 * l0;
l0 = l2.square_n(37);
l0 = l2.square_n::<37>();
l1 = l2 * l0;
l0 = l1.square_n(37);
l0 = l1.square_n::<37>();
l1 = l2 * l0;
l0 = l1.square_n(111);
l0 = l1.square_n::<111>();
l2 = l1 * l0;
l0 = l2.square();
l1 = l0 * self;
l0 = l1.square_n(223);
l0 = l1.square_n::<223>();
l1 = l2 * l0;
l2 = l1.square();
l0 = l2 * self;
Expand Down Expand Up @@ -418,7 +414,7 @@ impl FieldElement {
let e = b * c;

let mut a = n * e;
a.conditional_negate(!Choice::from(a.0.retrieve().bit(0)) ^ square);
a.conditional_negate(!a.is_negative() ^ square);

let c = e * ONE_MINUS_TWO_D;
let b = c.square();
Expand All @@ -429,7 +425,7 @@ impl FieldElement {
let b = b - Self::ONE;

let c = a.square();
let a = a + a;
let a = a.double();
let e = c + Self::ONE;
let T = a * e;
let X = a * b;
Expand Down
8 changes: 3 additions & 5 deletions ed448-goldilocks/src/field/scalar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ impl<C: CurveWithScalar> Field for Scalar<C> {
}

fn double(&self) -> Self {
self + self
self.double()
}

fn invert(&self) -> CtOption<Self> {
Expand Down Expand Up @@ -641,7 +641,7 @@ impl<C: CurveWithScalar> Scalar<C> {

/// Compute `self` + `self` mod ℓ
pub const fn double(&self) -> Self {
self.addition(self)
Self::new(self.scalar.double_mod(&ORDER))
}

/// Compute `self` - `rhs` mod ℓ
Expand Down Expand Up @@ -728,9 +728,7 @@ impl<C: CurveWithScalar> Scalar<C> {

/// Convert this `Scalar` to a little-endian byte array.
pub fn to_bytes(&self) -> [u8; 56] {
let bytes = self.scalar.to_le_bytes();
let output: [u8; 56] = core::array::from_fn(|i| bytes[i]);
output
self.scalar.to_le_byte_array().0
}

/// Invert this scalar
Expand Down