fix: find_non_qr doesn't work for p = 1 mod 4 (#1469)

- Fix `find_non_qr` for cases when p = 1 mod 4 but is not 5 mod 8.
- Fix the `num_limbs` of `prime_limbs` in `FieldExpr` (this issue arose
when adding tests with 224-bit moduli; it doesn't affect the soundness
of the Mod builder but will cause the correct trace to fail).

Resolves INT-3640

Author: manh9203

crates/circuits/mod-builder/src/builder.rs

@@ -331,7 +331,11 @@ impl<AB: InteractionBuilder> SubAir<AB> for FieldExpr {
             //   only the first segment will have setup called
 
             let expected = iter::empty()
-                .chain(self.builder.prime_limbs.clone())
+                .chain({
+                    let mut prime_limbs = self.builder.prime_limbs.clone();
+                    prime_limbs.resize(self.builder.num_limbs, 0);
+                    prime_limbs
+                })
                 .chain(self.setup_values.iter().flat_map(|x| {
                     big_uint_to_num_limbs(x, self.builder.limb_bits, self.builder.num_limbs)
                         .into_iter()

extensions/ecc/circuit/src/weierstrass_extension.rs

@@ -493,7 +493,10 @@ pub(crate) mod phantom {
                 &BigUint::from_u8(2).unwrap(),
                 &(modulus - BigUint::from_u8(1).unwrap()),
             );
-            let exponent = (modulus - BigUint::one()) >> 2;
+            // To check if non_qr is a quadratic nonresidue, we compute non_qr^((p-1)/2)
+            // If the result is p-1, then non_qr is a quadratic nonresidue
+            // Otherwise, non_qr is a quadratic residue
+            let exponent = (modulus - BigUint::one()) >> 1;
             while non_qr.modpow(&exponent, modulus) != modulus - BigUint::one() {
                 non_qr = rng.gen_biguint_range(
                     &BigUint::from_u8(2).unwrap(),

extensions/ecc/sw-macros/src/lib.rs

@@ -559,7 +559,7 @@ pub fn sw_init(input: TokenStream) -> TokenStream {
                         rs1 = In p1.as_ptr(),
                         rs2 = Const "x0" // will be parsed as 0 and therefore transpiled to SETUP_EC_DOUBLE
                     );
-               }
+                }
             }
         });
 

extensions/ecc/tests/programs/Cargo.toml

@@ -40,8 +40,10 @@ p256 = ["openvm-ecc-guest/p256"]
 # Note that these tests are expected to not terminate
 test_secp256k1_possible = []
 test_secp256k1_impossible = []
-test_mycurve_possible = []
-test_mycurve_impossible = []
+test_curvepoint5mod8_possible = []
+test_curvepoint5mod8_impossible = []
+test_curvepoint1mod4_possible = []
+test_curvepoint1mod4_impossible = []
 
 [profile.release]
 panic = "abort"

extensions/ecc/tests/programs/examples/decompress.rs

@@ -4,6 +4,7 @@
 use core::ops::Neg;
 extern crate alloc;
 
+use hex_literal::hex;
 use openvm::io::read_vec;
 use openvm_ecc_guest::{
     algebra::{Field, IntMod},
@@ -15,17 +16,24 @@ use openvm_ecc_guest::{
 openvm::entry!(main);
 
 openvm_algebra_moduli_macros::moduli_declare! {
+    // a prime that is 5 mod 8
+    Fp5mod8 { modulus = "115792089237316195423570985008687907853269984665640564039457584007913129639501" },
     // a prime that is 1 mod 4
-    Fp { modulus = "115792089237316195423570985008687907853269984665640564039457584007913129639501" },
+    Fp1mod4 { modulus = "0xffffffffffffffffffffffffffffffff000000000000000000000001" },
 }
 
 openvm_algebra_moduli_macros::moduli_init! {
     "0xFFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFE FFFFFC2F",
     "0xFFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFE BAAEDCE6 AF48A03B BFD25E8C D0364141",
     "115792089237316195423570985008687907853269984665640564039457584007913129639501",
+    "1000000007",
+    "0xffffffffffffffffffffffffffffffff000000000000000000000001",
+    "0xffffffffffffffffffffffffffff16a2e0b8f03e13dd29455c5c2a3d",
 }
 
-impl Field for Fp {
+const CURVE_B_5MOD8: Fp5mod8 = Fp5mod8::from_const_u8(3);
+
+impl Field for Fp5mod8 {
     const ZERO: Self = <Self as IntMod>::ZERO;
     const ONE: Self = <Self as IntMod>::ONE;
 
@@ -40,24 +48,51 @@ impl Field for Fp {
     }
 }
 
-const MY_CURVE_B: Fp = Fp::from_const_u8(3);
+const CURVE_A_1MOD4: Fp1mod4 = Fp1mod4::from_const_bytes(hex!(
+    "FEFFFFFFFFFFFFFFFFFFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000"
+));
+const CURVE_B_1MOD4: Fp1mod4 = Fp1mod4::from_const_bytes(hex!(
+    "B4FF552343390B27BAD8BFD7B7B04450563241F5ABB3040C850A05B400000000"
+));
 
-openvm_ecc_sw_macros::sw_declare! {
-    MyCurvePoint {
-        mod_type = Fp,
-        b = MY_CURVE_B,
+impl Field for Fp1mod4 {
+    const ZERO: Self = <Self as IntMod>::ZERO;
+    const ONE: Self = <Self as IntMod>::ONE;
+
+    type SelfRef<'a> = &'a Self;
+
+    fn double_assign(&mut self) {
+        IntMod::double_assign(self);
     }
+
+    fn square_assign(&mut self) {
+        IntMod::square_assign(self);
+    }
+}
+
+openvm_ecc_sw_macros::sw_declare! {
+    CurvePoint5mod8 {
+        mod_type = Fp5mod8,
+        b = CURVE_B_5MOD8,
+    },
+    CurvePoint1mod4 {
+        mod_type = Fp1mod4,
+        a = CURVE_A_1MOD4,
+        b = CURVE_B_1MOD4,
+    },
 }
 
 openvm_ecc_sw_macros::sw_init! {
     Secp256k1Point,
-    MyCurvePoint,
+    CurvePoint5mod8,
+    CurvePoint1mod4,
 }
 
 // test decompression under an honest host
 pub fn main() {
     setup_0();
     setup_2();
+    setup_4();
     setup_all_curves();
 
     let bytes = read_vec();
@@ -69,13 +104,21 @@ pub fn main() {
     // x = 5 is not on the x-coordinate of any point on the Secp256k1 curve
     test_impossible_decompression_secp256k1(&Secp256k1Coord::from_u8(5), rec_id);
 
-    let x = Fp::from_le_bytes(&bytes[64..96]);
-    let y = Fp::from_le_bytes(&bytes[96..128]);
+    let x = Fp5mod8::from_le_bytes(&bytes[64..96]);
+    let y = Fp5mod8::from_le_bytes(&bytes[96..128]);
     let rec_id = y.as_le_bytes()[0] & 1;
 
-    test_possible_decompression::<MyCurvePoint>(&x, &y, rec_id);
-    // x = 3 is not on the x-coordinate of any point on the MyCurve curve
-    test_impossible_decompression_mycurve(&Fp::from_u8(3), rec_id);
+    test_possible_decompression::<CurvePoint5mod8>(&x, &y, rec_id);
+    // x = 3 is not on the x-coordinate of any point on the CurvePoint5mod8 curve
+    test_impossible_decompression_curvepoint5mod8(&Fp5mod8::from_u8(3), rec_id);
+
+    let x = Fp1mod4::from_le_bytes(&bytes[128..160]);
+    let y = Fp1mod4::from_le_bytes(&bytes[160..192]);
+    let rec_id = y.as_le_bytes()[0] & 1;
+
+    test_possible_decompression::<CurvePoint1mod4>(&x, &y, rec_id);
+    // x = 1 is not on the x-coordinate of any point on the CurvePoint1mod4 curve
+    test_impossible_decompression_curvepoint1mod4(&Fp1mod4::from_u8(1), rec_id);
 }
 
 fn test_possible_decompression<P: WeierstrassPoint + FromCompressed<P::Coordinate>>(
@@ -98,18 +141,18 @@ fn test_possible_decompression<P: WeierstrassPoint + FromCompressed<P::Coordinat
 // The test_impossible_decompression_* functions cannot be combined into a single function with a const generic parameter
 // since the get_non_qr() function is not part of the WeierstrassPoint trait.
 
-fn test_impossible_decompression_mycurve(x: &Fp, rec_id: u8) {
-    let hint = MyCurvePoint::hint_decompress(x, &rec_id).expect("hint should be well-formed");
+fn test_impossible_decompression_curvepoint5mod8(x: &Fp5mod8, rec_id: u8) {
+    let hint = CurvePoint5mod8::hint_decompress(x, &rec_id).expect("hint should be well-formed");
     if hint.possible {
         panic!("decompression should be impossible");
     } else {
         let rhs = x * x * x
-            + x * &<MyCurvePoint as WeierstrassPoint>::CURVE_A
-            + &<MyCurvePoint as WeierstrassPoint>::CURVE_B;
-        assert_eq!(&hint.sqrt * &hint.sqrt, rhs * MyCurvePoint::get_non_qr());
+            + x * &<CurvePoint5mod8 as WeierstrassPoint>::CURVE_A
+            + &<CurvePoint5mod8 as WeierstrassPoint>::CURVE_B;
+        assert_eq!(&hint.sqrt * &hint.sqrt, rhs * CurvePoint5mod8::get_non_qr());
     }
 
-    let p = MyCurvePoint::decompress(x.clone(), &rec_id);
+    let p = CurvePoint5mod8::decompress(x.clone(), &rec_id);
     assert!(p.is_none());
 }
 
@@ -127,3 +170,18 @@ fn test_impossible_decompression_secp256k1(x: &Secp256k1Coord, rec_id: u8) {
     let p = Secp256k1Point::decompress(x.clone(), &rec_id);
     assert!(p.is_none());
 }
+
+fn test_impossible_decompression_curvepoint1mod4(x: &Fp1mod4, rec_id: u8) {
+    let hint = CurvePoint1mod4::hint_decompress(x, &rec_id).expect("hint should be well-formed");
+    if hint.possible {
+        panic!("decompression should be impossible");
+    } else {
+        let rhs = x * x * x
+            + x * &<CurvePoint1mod4 as WeierstrassPoint>::CURVE_A
+            + &<CurvePoint1mod4 as WeierstrassPoint>::CURVE_B;
+        assert_eq!(&hint.sqrt * &hint.sqrt, rhs * CurvePoint1mod4::get_non_qr());
+    }
+
+    let p = CurvePoint1mod4::decompress(x.clone(), &rec_id);
+    assert!(p.is_none());
+}