ed448-goldilocks: assorted fixes and improvements (#1302)

* `DecafPoint` is always torsion free
* Use `Uint::to_le_byte_array()` directly
* Use `is_negative()` directly
* Enforce constant-time `FieldElement::square_n()`
* Simplify `FieldElement::square_n()` and protect against zero
* Use double optimization where appropriate
* Use `Uint::is_odd()` directly
* Hide `EDWARDS_BASEPOINT_ORDER`
* Use squaring instead of multiplication where appropriate

Author: daxpedda

ed448-goldilocks/src/constants.rs

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

ed448-goldilocks/src/curve/twedwards/extended.rs

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

ed448-goldilocks/src/curve/twedwards/extensible.rs

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

ed448-goldilocks/src/decaf/points.rs

@@ -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::*;
@@ -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)
     }
 }
 
@@ -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;

ed448-goldilocks/src/edwards/extended.rs

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

ed448-goldilocks/src/field/element.rs

@@ -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,
@@ -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
@@ -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 {
+        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();
         }
 
@@ -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
@@ -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;
@@ -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();
@@ -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;

ed448-goldilocks/src/field/scalar.rs

@@ -314,7 +314,7 @@ impl<C: CurveWithScalar> Field for Scalar<C> {
     }
 
     fn double(&self) -> Self {
-        self + self
+        self.double()
     }
 
     fn invert(&self) -> CtOption<Self> {
@@ -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 ℓ
@@ -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