Reduce scalar reduction from byte-wise to limb-wise arithmetic

README.md

@@ -52,7 +52,7 @@ let instruction = verify(&ID, &public_key, &signature, message);
 ### Constraints
 
 - **Verification criteria.** The program always applies [ZIP-215]: the
-  cofactored equation `[8](S·B − H(R‖A‖M)·A) == [8]R`.
+  cofactored equation `[8](S·B − H(R‖A‖M)·A − R) == identity`.
   Small-order and non-canonical points are accepted. Programs needing a
   different variant (e.g. `verify_strict`) should depend on the
   `solana-ed25519-verify` library directly (see
@@ -78,18 +78,22 @@ non-canonical encodings, and small-order rejection (see Henry de Valence's
 [It's 255:19AM]). The `solana-ed25519-verify` crate exposes these as independent
 knobs via `VerificationCriteria`:
 
-| Knob                   | Effect when enabled                                                                          | Extra syscalls                                             |
-| ---------------------- | -------------------------------------------------------------------------------------------- | ---------------------------------------------------------- |
-| `cofactored`           | Use `[8](S·B − H·A − R) == identity` instead of the cofactorless `S·B − H·A − R == identity` | +3 `sol_curve_group_op` (multiply-by-8 as three doublings) |
-| `require_canonical_a`  | Reject public keys whose `y`-coordinate is `≥ p`                                             | none                                                       |
-| `require_canonical_r`  | Reject signature `R` whose `y`-coordinate is `≥ p`                                           | none                                                       |
-| `reject_small_order_a` | Reject small-order (torsion) public keys                                                     | +3 `sol_curve_group_op`                                    |
-| `reject_small_order_r` | Reject small-order signature `R` values                                                      | +3 `sol_curve_group_op`                                    |
+| Knob                   | Effect when enabled                                                                          | Extra syscalls                                    |
+| ---------------------- | -------------------------------------------------------------------------------------------- | ------------------------------------------------- |
+| `cofactored`           | Use `[8](S·B − H·A − R) == identity` instead of the cofactorless `S·B − H·A − R == identity` | +3 `sol_curve_group_op` (1,419 CU), fallback only |
+| `require_canonical_a`  | Reject public keys whose `y`-coordinate is `≥ p`                                             | none                                              |
+| `require_canonical_r`  | Reject signature `R` whose `y`-coordinate is `≥ p`                                           | none                                              |
+| `reject_small_order_a` | Reject small-order (torsion) public keys                                                     | +3 `sol_curve_group_op` (1,419 CU)                |
+| `reject_small_order_r` | Reject small-order signature `R` values                                                      | +3 `sol_curve_group_op` (1,419 CU)                |
 
-Canonical `S` (`S < L`) is enforced for every criteria set and so has no knob:
-`sol_curve_multiscalar_mul` converts scalars through
-`Scalar::from_canonical_bytes` and rejects anything out of range before any
-group operation runs.
+The cofactor multiplication is only reached when a signature fails the exact
+comparison first, so it costs nothing on the path a valid signature follows.
+
+Canonical `S` (`S < L`) has no knob. Every profile worth targeting requires it —
+accepting `S ≥ L` reintroduces signature malleability — and
+`sol_curve_multiscalar_mul` enforces it regardless, converting scalars through
+`Scalar::from_canonical_bytes` and rejecting out-of-range values before any group
+operation runs.
 
 ```rust
 use solana_ed25519_verify::{Ed25519Verifier, VerificationCriteria};
@@ -145,9 +149,11 @@ Solana-specific type.
 | `InvalidEncoding`       | `A` doesn't decode to a valid point, or `S` is non-canonical (`S ≥ L`) |
 | `SignatureMismatch`     | Every input decoded successfully, but the equation doesn't hold        |
 
-`InvalidEncoding` intentionally does not distinguish a malformed public key
-from a non-canonical `S` scalar: the curve syscall that consumes both reports
-only overall success or failure, with no further detail attached.
+`InvalidEncoding` does not distinguish a malformed public key from a
+non-canonical `S` scalar: the syscall that consumes both reports only overall
+success or failure. Telling them apart would mean an explicit `S < L` comparison,
+or decoding `A` ahead of the syscall — compute units spent on every signature for
+precision that only helps malformed input.
 
 The on-chain program collapses all of these to
 `ProgramError::InvalidInstructionData` — see [Constraints](#constraints).

ed25519-verify/src/config.rs

@@ -25,27 +25,30 @@
 /// are public so callers can compose arbitrary combinations, typically by
 /// starting from a preset and overriding a single knob:
 ///
-/// Canonical `S` (`S < L`) is enforced for every criteria set and so has no
-/// corresponding field: the curve back-end converts scalars through
-/// `Scalar::from_canonical_bytes` and rejects anything out of range before any
-/// group operation runs.
-///
 /// ```
 /// use solana_ed25519_verify::VerificationCriteria;
 ///
-/// let strict_s = VerificationCriteria {
+/// let custom = VerificationCriteria {
 ///     reject_small_order_a: true,
 ///     ..VerificationCriteria::zip215()
 /// };
 /// ```
+///
+/// Canonical `S` (`S < L`) has no corresponding field. Every profile worth
+/// targeting requires it — accepting `S >= L` reintroduces signature
+/// malleability — and the multiscalar-mul syscall enforces it regardless,
+/// converting scalars through `Scalar::from_canonical_bytes` and rejecting
+/// out-of-range values before any group operation runs.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct VerificationCriteria {
-    /// Use the cofactored verification equation `[8](S·B − H·A) == [8]R`.
+    /// Use the cofactored verification equation
+    /// `[8](S·B − H·A − R) == identity`.
     ///
-    /// When `false`, the cofactorless equation `S·B − H·A == R` is used, which
-    /// rejects mixed-order points that the cofactored equation tolerates. The
-    /// cofactored form costs one extra multiplication by the cofactor 8, which
-    /// the verifier performs as three `sol_curve_group_op` additions.
+    /// When `false`, the cofactorless equation `S·B − H·A − R == identity` is
+    /// used, which rejects mixed-order points that the cofactored equation
+    /// tolerates. The cofactored form costs one extra multiplication by the
+    /// cofactor 8, which the verifier performs as three `sol_curve_group_op`
+    /// additions.
     pub cofactored: bool,
     /// Reject public keys whose compressed `y`-coordinate is `>= p` (a
     /// non-canonical encoding of a reduced point).
@@ -87,13 +90,12 @@ impl VerificationCriteria {
 
     /// The criteria enforced by `ed25519_dalek::VerifyingKey::verify_strict`.
     ///
-    /// Cofactorless verification with canonical `R` and small-order rejection
-    /// for both `A` and `R`. Mirrors ed25519-dalek 2.x exactly, including the
-    /// detail that a non-canonically encoded public key `A` is *not* rejected
-    /// — dalek's `VerifyingKey::from_bytes` decompresses `A` (reducing `y`
-    /// modulo `p`) without a canonicity check, and `verify_strict` only
-    /// re-encodes and compares `R`. Every signature this preset accepts is
-    /// accepted by dalek's `verify_strict`, and vice versa.
+    /// Cofactorless, with canonical `R` and small-order rejection for both `A`
+    /// and `R`. Note that a non-canonically encoded `A` is *not* rejected:
+    /// dalek's `VerifyingKey::from_bytes` decompresses `A` (reducing `y` modulo
+    /// `p`) without a canonicity check, and `verify_strict` only re-encodes and
+    /// compares `R`. The match is exact in both directions, cross-checked in
+    /// the test suite.
     pub const fn dalek_verify_strict() -> Self {
         Self {
             cofactored: false,

ed25519-verify/src/constants.rs

@@ -16,6 +16,35 @@ pub(crate) const BASEPOINT_ORDER: [u8; 32] = [
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10,
 ];
 
+/// Reinterprets a 32-byte little-endian integer as four little-endian 64-bit
+/// limbs.
+///
+/// Evaluated at compile time so that limb constants are derived from their byte
+/// counterparts rather than transcribed by hand a second time.
+const fn to_le_limbs(bytes: [u8; 32]) -> [u64; 4] {
+    let mut limbs = [0u64; 4];
+    let mut limb_index = 0;
+
+    while limb_index < 4 {
+        let mut limb = 0u64;
+        let mut byte_index = 0;
+        while byte_index < 8 {
+            limb |= (bytes[limb_index * 8 + byte_index] as u64) << (byte_index * 8);
+            byte_index += 1;
+        }
+        limbs[limb_index] = limb;
+        limb_index += 1;
+    }
+
+    limbs
+}
+
+/// [`BASEPOINT_ORDER`] as little-endian 64-bit limbs.
+///
+/// Used by the limb-wise reduction in [`crate::scalar`], where a 64-bit borrow
+/// chain replaces a byte-wise one.
+pub(crate) const BASEPOINT_ORDER_LIMBS: [u64; 4] = to_le_limbs(BASEPOINT_ORDER);
+
 /// Field modulus `p = 2^255 - 19` in little-endian form.
 pub(crate) const FIELD_MODULUS: [u8; 32] = [
     0xed, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
@@ -45,14 +74,16 @@ pub(crate) const EDWARDS_IDENTITY_COMPRESSED: PodEdwardsPoint =
 
 #[cfg(test)]
 mod tests {
-    use {super::*, curve25519_dalek::traits::Identity};
+    use {
+        super::*,
+        curve25519_dalek::{scalar::Scalar, traits::Identity},
+    };
 
-    // These two constants are hand-transcribed byte literals with no
-    // compiler check that they encode what the doc comments claim.
-    // Cross-checking them against `curve25519-dalek`'s own constants (an
-    // independent implementation, not just a second copy of the same
-    // literal) catches a transcription error immediately instead of relying
-    // on review.
+    // These constants are hand-transcribed byte literals with no compiler check
+    // that they encode what the doc comments claim. Cross-checking them against
+    // `curve25519-dalek` (an independent implementation, not just a second copy
+    // of the same literal) catches a transcription error immediately instead of
+    // relying on review.
 
     #[test]
     fn negated_basepoint_constant_matches_curve25519_dalek() {
@@ -73,4 +104,33 @@ mod tests {
 
         assert_eq!(EDWARDS_IDENTITY_COMPRESSED_BYTES, expected);
     }
+
+    #[test]
+    fn basepoint_order_matches_curve25519_dalek() {
+        // dalek does not export `L` directly, but `from_canonical_bytes` accepts
+        // exactly the values below it. `L - 1` being accepted and `L` itself
+        // being rejected pins the constant to dalek's group order.
+        let mut order_minus_one = BASEPOINT_ORDER;
+        order_minus_one[0] -= 1;
+
+        assert!(Scalar::from_canonical_bytes(order_minus_one)
+            .into_option()
+            .is_some());
+        assert!(Scalar::from_canonical_bytes(BASEPOINT_ORDER)
+            .into_option()
+            .is_none());
+    }
+
+    #[test]
+    fn basepoint_order_limbs_round_trip_to_bytes() {
+        // Rebuilds the byte constant from the limbs using `to_le_bytes` rather
+        // than the shift loop in `to_le_limbs`, so the two directions are not
+        // the same code checked against itself.
+        let mut bytes = [0u8; 32];
+        for (chunk, limb) in bytes.chunks_exact_mut(8).zip(BASEPOINT_ORDER_LIMBS) {
+            chunk.copy_from_slice(&limb.to_le_bytes());
+        }
+
+        assert_eq!(bytes, BASEPOINT_ORDER);
+    }
 }

ed25519-verify/src/error.rs

@@ -31,12 +31,10 @@ pub enum Ed25519VerifyError {
     /// The public key does not decode to a valid Edwards curve point, or the
     /// signature's `S` scalar is not canonical (`S >= L`).
     ///
-    /// These two causes are not distinguished: telling them apart would
-    /// require either a canonical-`S` check duplicating work the curve
-    /// back-end already performs internally, or a public-key decode check
-    /// ahead of the syscall that computes the verification equation. Both
-    /// would cost compute units on every signature to add precision that
-    /// only benefits the malformed-input case.
+    /// These two causes are not distinguished: the syscall that consumes both
+    /// reports only overall success or failure. Telling them apart would mean an
+    /// explicit `S < L` comparison, or decoding `A` ahead of the syscall — compute
+    /// units spent on every signature for precision that only helps malformed input.
     InvalidEncoding,
     /// The public key, signature, and message all decoded successfully, but
     /// the signature does not satisfy the verification equation.

ed25519-verify/src/scalar.rs

@@ -1,7 +1,7 @@
 //! Small scalar- and field-element helpers needed to assemble Ed25519
 //! verification around syscalls.
 
-use crate::constants::{BASEPOINT_ORDER, FIELD_MODULUS};
+use crate::constants::{BASEPOINT_ORDER_LIMBS, FIELD_MODULUS};
 
 /// Returns `true` if `encoding` is a canonical compressed Edwards point.
 ///
@@ -17,49 +17,66 @@ pub(crate) fn is_canonical_point_encoding(encoding: &[u8; 32]) -> bool {
 }
 
 /// Reduces a 64-byte little-endian integer modulo the ed25519 base point order.
+///
+/// Bit-serial long division: shift the remainder left, shift in the next bit of
+/// the dividend, and subtract `L` whenever the result reaches it. The remainder
+/// is carried as 64-bit limbs rather than bytes, so each step touches four words
+/// instead of thirty-two.
 pub(crate) fn reduce_wide(wide: &[u8; 64]) -> [u8; 32] {
-    let mut remainder = [0u8; 32];
+    let mut remainder = [0u64; 4];
 
     for bit_index in (0..512).rev() {
         shl1(&mut remainder);
-        if (wide[bit_index / 8] >> (bit_index % 8)) & 1 == 1 {
-            remainder[0] |= 1;
-        }
-        if !cmp_le(&remainder, &BASEPOINT_ORDER).is_lt() {
-            sub_assign(&mut remainder, &BASEPOINT_ORDER);
-        }
+        remainder[0] |= u64::from((wide[bit_index / 8] >> (bit_index % 8)) & 1);
+        conditional_sub_order(&mut remainder);
     }
 
-    remainder
+    let mut reduced = [0u8; 32];
+    for (chunk, limb) in reduced.chunks_exact_mut(8).zip(remainder) {
+        chunk.copy_from_slice(&limb.to_le_bytes());
+    }
+    reduced
 }
 
-fn shl1(value: &mut [u8; 32]) {
-    let mut carry = 0u8;
-    for byte in value {
-        let next_carry = *byte >> 7;
-        *byte = (*byte << 1) | carry;
+/// Doubles `value` in place, discarding the bit shifted out of the top.
+///
+/// The caller keeps `value < L < 2^253` between iterations, so the discarded bit
+/// is always zero.
+fn shl1(value: &mut [u64; 4]) {
+    let mut carry = 0u64;
+    for limb in value {
+        let next_carry = *limb >> 63;
+        *limb = (*limb << 1) | carry;
         carry = next_carry;
     }
 }
 
-fn sub_assign(left: &mut [u8; 32], right: &[u8; 32]) {
-    let mut borrow = 0u16;
-    for (left_byte, right_byte) in left.iter_mut().zip(right) {
-        let minuend = u16::from(*left_byte);
-        let subtrahend = u16::from(*right_byte) + borrow;
-        if minuend >= subtrahend {
-            *left_byte = (minuend - subtrahend) as u8;
-            borrow = 0;
-        } else {
-            *left_byte = (minuend + 256 - subtrahend) as u8;
-            borrow = 1;
-        }
+/// Subtracts `L` from `value` when `value >= L`, without branching on the input.
+fn conditional_sub_order(value: &mut [u64; 4]) {
+    let mut difference = [0u64; 4];
+    let mut borrow = 0u64;
+
+    for index in 0..4 {
+        let (partial, borrow_from_order) =
+            value[index].overflowing_sub(BASEPOINT_ORDER_LIMBS[index]);
+        let (limb, borrow_from_carry) = partial.overflowing_sub(borrow);
+        difference[index] = limb;
+        borrow = u64::from(borrow_from_order | borrow_from_carry);
+    }
+
+    // A borrow out of the top limb means `value < L`, so the difference is
+    // discarded. `mask` is all-ones exactly when the subtraction should apply.
+    let mask = borrow.wrapping_sub(1);
+    for index in 0..4 {
+        value[index] = (value[index] & !mask) | (difference[index] & mask);
     }
 }
 
 pub(crate) fn cmp_le(left: &[u8; 32], right: &[u8; 32]) -> core::cmp::Ordering {
-    for (left_byte, right_byte) in left.iter().zip(right).rev() {
-        match left_byte.cmp(right_byte) {
+    for index in (0..4).rev() {
+        let left_limb = u64::from_le_bytes(left[index * 8..index * 8 + 8].try_into().unwrap());
+        let right_limb = u64::from_le_bytes(right[index * 8..index * 8 + 8].try_into().unwrap());
+        match left_limb.cmp(&right_limb) {
             core::cmp::Ordering::Equal => {}
             ordering => return ordering,
         }
@@ -69,13 +86,59 @@ pub(crate) fn cmp_le(left: &[u8; 32], right: &[u8; 32]) -> core::cmp::Ordering {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
+    use {super::*, crate::constants::BASEPOINT_ORDER};
+
+    fn wide_from_low_32(low: &[u8; 32]) -> [u8; 64] {
+        let mut wide = [0u8; 64];
+        wide[..32].copy_from_slice(low);
+        wide
+    }
 
     #[test]
     fn reduces_group_order_to_zero() {
+        assert_eq!(reduce_wide(&wide_from_low_32(&BASEPOINT_ORDER)), [0; 32]);
+    }
+
+    #[test]
+    fn reduces_order_boundaries() {
+        // L - 1 is already reduced and must pass through unchanged.
+        let mut order_minus_one = BASEPOINT_ORDER;
+        order_minus_one[0] -= 1;
+        assert_eq!(
+            reduce_wide(&wide_from_low_32(&order_minus_one)),
+            order_minus_one
+        );
+
+        // L + 1 must come back as 1.
+        let mut order_plus_one = BASEPOINT_ORDER;
+        order_plus_one[0] += 1;
+        let mut one = [0u8; 32];
+        one[0] = 1;
+        assert_eq!(reduce_wide(&wide_from_low_32(&order_plus_one)), one);
+    }
+
+    // `reduce_wide` is hand-rolled modular arithmetic with a branch-free carry
+    // chain, so it is cross-checked against curve25519-dalek's own wide
+    // reduction rather than against a second copy of the same reasoning.
+    #[test]
+    fn matches_curve25519_dalek_wide_reduction() {
         let mut wide = [0u8; 64];
-        wide[..32].copy_from_slice(&BASEPOINT_ORDER);
-        assert_eq!(reduce_wide(&wide), [0; 32]);
+
+        for round in 0..32u32 {
+            for (index, byte) in wide.iter_mut().enumerate() {
+                *byte = (index as u32).wrapping_mul(31).wrapping_add(round * 7) as u8;
+            }
+
+            let expected =
+                curve25519_dalek::scalar::Scalar::from_bytes_mod_order_wide(&wide).to_bytes();
+            assert_eq!(reduce_wide(&wide), expected, "round {round}");
+        }
+
+        // Saturated input exercises the widest possible quotient.
+        let saturated = [0xff; 64];
+        let expected =
+            curve25519_dalek::scalar::Scalar::from_bytes_mod_order_wide(&saturated).to_bytes();
+        assert_eq!(reduce_wide(&saturated), expected);
     }
 
     #[test]