From 29de89385a5138516c69efd5bfc9ba3e984aa537 Mon Sep 17 00:00:00 2001 From: jotabulacios Date: Thu, 23 Jul 2026 12:01:41 -0300 Subject: [PATCH] Avoid wasteful to_affine inversions in ecrecover --- crypto/ethrex-crypto/src/lib.rs | 46 ++++++++++---------- crypto/ethrex-crypto/src/tests/ecsm_tests.rs | 30 ++++++------- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/crypto/ethrex-crypto/src/lib.rs b/crypto/ethrex-crypto/src/lib.rs index 980154e0f..c1e5d8446 100644 --- a/crypto/ethrex-crypto/src/lib.rs +++ b/crypto/ethrex-crypto/src/lib.rs @@ -112,11 +112,14 @@ fn ecsm_ecrecover(sig: &[u8; 64], recid: u8, msg: &[u8; 32]) -> Result<[u8; 64], let u2 = r_inv * s; // pk = u1·G + u2·R, accelerated via ECSM with a software fallback. + // The ECSM path takes affine inputs and returns the affine result directly: + // its inputs (G, R) and output are Z=1 points, so passing affines avoids the + // wasteful projective→affine inversions (`to_affine` of a Z=1 point still runs + // a full constant-time field inversion in k256). The rare software fallback + // still converts via `to_affine`. let g = ProjectivePoint::GENERATOR; - let pk = ecsm_lincomb2(&g, &u1, &r_proj, &u2) - .unwrap_or_else(|| ProjectivePoint::lincomb(&g, &u1, &r_proj, &u2)); - - let pk_affine = pk.to_affine(); + let pk_affine = ecsm_lincomb2(&AffinePoint::GENERATOR, &u1, &r_point, &u2) + .unwrap_or_else(|| ProjectivePoint::lincomb(&g, &u1, &r_proj, &u2).to_affine()); if bool::from(pk_affine.is_identity()) { return Err(CryptoError::RecoveryFailed); } @@ -136,21 +139,21 @@ fn ecsm_ecrecover(sig: &[u8; 64], recid: u8, msg: &[u8; 32]) -> Result<[u8; 64], /// so the caller uses the pure-Rust `ProjectivePoint::lincomb`. #[cfg(target_arch = "riscv64")] fn ecsm_lincomb2( - p1: &ProjectivePoint, + a1: &AffinePoint, k1: &Scalar, - p2: &ProjectivePoint, + a2: &AffinePoint, k2: &Scalar, -) -> Option { - lincomb2_with_oracle(p1, k1, p2, k2, ecsm_oracle) +) -> Option { + lincomb2_with_oracle(a1, k1, a2, k2, ecsm_oracle) } #[cfg(not(target_arch = "riscv64"))] fn ecsm_lincomb2( - _p1: &ProjectivePoint, + _a1: &AffinePoint, _k1: &Scalar, - _p2: &ProjectivePoint, + _a2: &AffinePoint, _k2: &Scalar, -) -> Option { +) -> Option { None } @@ -193,17 +196,17 @@ fn ecsm_oracle(x: &FieldElement, k: &Scalar) -> Option { /// Generic over the oracle so unit tests can substitute a software stand-in. #[cfg(any(target_arch = "riscv64", test))] fn lincomb2_with_oracle( - p1: &ProjectivePoint, + a1: &AffinePoint, k1: &Scalar, - p2: &ProjectivePoint, + a2: &AffinePoint, k2: &Scalar, oracle: O, -) -> Option +) -> Option where O: Fn(&FieldElement, &Scalar) -> Option, { - let a1 = p1.to_affine(); - let a2 = p2.to_affine(); + // Inputs are affine already (the ecrecover path lifts them from known Z=1 + // points), so no projective→affine inversion is needed here. if bool::from(a1.is_identity()) || bool::from(a2.is_identity()) { return None; } @@ -211,8 +214,8 @@ where return None; } - let (x1, y1) = affine_xy(&a1)?; - let (x2, y2) = affine_xy(&a2)?; + let (x1, y1) = affine_xy(a1)?; + let (x2, y2) = affine_xy(a2)?; let xa = oracle(&x1, k1)?; let xc1 = oracle(&x1, &(*k1 + Scalar::ONE))?; @@ -291,13 +294,12 @@ fn affine_xy(p: &AffinePoint) -> Option<(FieldElement, FieldElement)> { Some((x, y)) } -/// Builds a curve point from affine coordinates, returning `None` if the point +/// Builds an affine curve point from coordinates, returning `None` if the point /// is not on the curve (`AffinePoint::from_encoded_point` validates this). #[cfg(any(target_arch = "riscv64", test))] -fn point_from_xy(x: &FieldElement, y: &FieldElement) -> Option { +fn point_from_xy(x: &FieldElement, y: &FieldElement) -> Option { let ep = EncodedPoint::from_affine_coordinates(&x.to_bytes(), &y.to_bytes(), false); - let affine = Option::::from(AffinePoint::from_encoded_point(&ep))?; - Some(ProjectivePoint::from(affine)) + Option::::from(AffinePoint::from_encoded_point(&ep)) } // ── Keccak-256 over the keccak_permute precompile (riscv64 guest) ─────────── diff --git a/crypto/ethrex-crypto/src/tests/ecsm_tests.rs b/crypto/ethrex-crypto/src/tests/ecsm_tests.rs index ace1dc63a..89c911db7 100644 --- a/crypto/ethrex-crypto/src/tests/ecsm_tests.rs +++ b/crypto/ethrex-crypto/src/tests/ecsm_tests.rs @@ -36,9 +36,9 @@ fn matches_software_lincomb_on_fixed_inputs() { for (p1, k1, p2, k2) in cases { let (k1, k2) = (Scalar::from(k1), Scalar::from(k2)); let expected = ProjectivePoint::lincomb(&p1, &k1, &p2, &k2); - let got = lincomb2_with_oracle(&p1, &k1, &p2, &k2, soft_oracle) + let got = lincomb2_with_oracle(&p1.to_affine(), &k1, &p2.to_affine(), &k2, soft_oracle) .expect("non-degenerate inputs must reconstruct"); - assert_eq!(got.to_affine(), expected.to_affine()); + assert_eq!(got, expected.to_affine()); } } @@ -50,9 +50,9 @@ fn matches_software_lincomb_on_recovery_shape() { let u1 = Scalar::from(0xdead_beefu64); let u2 = Scalar::from(0x0bad_f00du64); let expected = ProjectivePoint::lincomb(&g, &u1, &r, &u2); - let got = lincomb2_with_oracle(&g, &u1, &r, &u2, soft_oracle) + let got = lincomb2_with_oracle(&g.to_affine(), &u1, &r.to_affine(), &u2, soft_oracle) .expect("non-degenerate inputs must reconstruct"); - assert_eq!(got.to_affine(), expected.to_affine()); + assert_eq!(got, expected.to_affine()); } #[test] @@ -61,8 +61,8 @@ fn edge_scalars_fall_back() { let p2 = g_times(5); let ok = Scalar::from(12345u64); for bad in [Scalar::ZERO, Scalar::ONE, -Scalar::ONE] { - assert!(lincomb2_with_oracle(&p1, &bad, &p2, &ok, soft_oracle).is_none()); - assert!(lincomb2_with_oracle(&p1, &ok, &p2, &bad, soft_oracle).is_none()); + assert!(lincomb2_with_oracle(&p1.to_affine(), &bad, &p2.to_affine(), &ok, soft_oracle).is_none()); + assert!(lincomb2_with_oracle(&p1.to_affine(), &ok, &p2.to_affine(), &bad, soft_oracle).is_none()); } } @@ -71,8 +71,8 @@ fn identity_points_fall_back() { let p = g_times(3); let k = Scalar::from(7u64); let id = ProjectivePoint::IDENTITY; - assert!(lincomb2_with_oracle(&id, &k, &p, &k, soft_oracle).is_none()); - assert!(lincomb2_with_oracle(&p, &k, &id, &k, soft_oracle).is_none()); + assert!(lincomb2_with_oracle(&id.to_affine(), &k, &p.to_affine(), &k, soft_oracle).is_none()); + assert!(lincomb2_with_oracle(&p.to_affine(), &k, &id.to_affine(), &k, soft_oracle).is_none()); } #[test] @@ -80,8 +80,8 @@ fn cancelling_and_doubling_terms_fall_back() { let p = g_times(3); let k = Scalar::from(7u64); // A = B (doubling chord) and A = −B (Q = O): both share x(A) = x(B). - assert!(lincomb2_with_oracle(&p, &k, &p, &k, soft_oracle).is_none()); - assert!(lincomb2_with_oracle(&p, &k, &(-p), &k, soft_oracle).is_none()); + assert!(lincomb2_with_oracle(&p.to_affine(), &k, &p.to_affine(), &k, soft_oracle).is_none()); + assert!(lincomb2_with_oracle(&p.to_affine(), &k, &(-p).to_affine(), &k, soft_oracle).is_none()); } #[test] @@ -100,9 +100,9 @@ fn k_half_n_minus_1_reconstructs_correctly() { let k2 = Scalar::from(99999u64); let expected = ProjectivePoint::lincomb(&p1, &k_half, &p2, &k2); - let got = lincomb2_with_oracle(&p1, &k_half, &p2, &k2, soft_oracle) + let got = lincomb2_with_oracle(&p1.to_affine(), &k_half, &p2.to_affine(), &k2, soft_oracle) .expect("k=(n-1)/2 is not near-edge and must reconstruct correctly"); - assert_eq!(got.to_affine(), expected.to_affine()); + assert_eq!(got, expected.to_affine()); } #[test] @@ -118,7 +118,7 @@ fn cross_point_cancellation_falls_back() { .expect("3 is invertible mod n"); let k1 = -(k2 * Scalar::from(7u64) * three_inv); assert!( - lincomb2_with_oracle(&p1, &k1, &p2, &k2, soft_oracle).is_none(), + lincomb2_with_oracle(&p1.to_affine(), &k1, &p2.to_affine(), &k2, soft_oracle).is_none(), "cross-point cancellation (P1 ≠ ±P2, result = O) must fall back" ); } @@ -171,7 +171,7 @@ fn odd_y_base_point_reconstructs_correctly() { let k1 = Scalar::from(54321u64); let k2 = Scalar::from(11111u64); let expected = ProjectivePoint::lincomb(&p1, &k1, &p2, &k2); - let got = lincomb2_with_oracle(&p1, &k1, &p2, &k2, soft_oracle) + let got = lincomb2_with_oracle(&p1.to_affine(), &k1, &p2.to_affine(), &k2, soft_oracle) .expect("odd-y base point is non-degenerate and must reconstruct correctly"); - assert_eq!(got.to_affine(), expected.to_affine()); + assert_eq!(got, expected.to_affine()); }