fix linter errors and address PR comments

- rename remaining structs
- switch from num-bigint-dig to num-bigint

Author: Avaneesh-axiom

Cargo.lock

@@ -7,5 +7,4 @@ supported_modulus = ["1157920892373161954235709850086879078532699846656405640394
 [[app_vm_config.ecc.supported_curves]]
 modulus = "115792089237316195423570985008687907853269984665640564039457584007908834671663"
 scalar = "115792089237316195423570985008687907852837564279074904382605163141518161494337"
-a = "0"
-b = "7"
+coeffs = { SwCurve = { a = "0", b = "7" } }

examples/algebra/openvm/app.vmexe

@@ -21,7 +21,6 @@ openvm-rv32-adapters = { workspace = true }
 openvm-ecc-transpiler = { workspace = true }
 
 num-bigint = { workspace = true }
-num-bigint-dig = { workspace = true }
 num-traits = { workspace = true }
 strum = { workspace = true }
 derive_more = { workspace = true }

examples/ecc/openvm.toml

@@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize};
 use serde_with::{serde_as, DisplayFromStr};
 use strum::EnumCount;
 
-use super::{EcAddNeChip, EcDoubleChip, TeEcAddChip};
+use super::{SwAddNeChip, SwDoubleChip, TeAddChip};
 
 #[serde_as]
 #[derive(Clone, Debug, derive_new::new, Serialize, Deserialize)]
@@ -37,6 +37,7 @@ pub struct CurveConfig {
     #[serde_as(as = "DisplayFromStr")]
     pub scalar: BigUint,
     // curve-specific coefficients
+    #[serde_as(as = "_")]
     pub coeffs: CurveCoeffs,
 }
 
@@ -94,15 +95,15 @@ pub struct EccExtension {
 #[derive(Chip, ChipUsageGetter, InstructionExecutor, AnyEnum, BytesStateful)]
 pub enum EccExtensionExecutor<F: PrimeField32> {
     // 32 limbs prime
-    SwEcAddNeRv32_32(EcAddNeChip<F, 2, 32>),
-    SwEcDoubleRv32_32(EcDoubleChip<F, 2, 32>),
+    SwEcAddNeRv32_32(SwAddNeChip<F, 2, 32>),
+    SwEcDoubleRv32_32(SwDoubleChip<F, 2, 32>),
     // 48 limbs prime
-    SwEcAddNeRv32_48(EcAddNeChip<F, 6, 16>),
-    SwEcDoubleRv32_48(EcDoubleChip<F, 6, 16>),
+    SwEcAddNeRv32_48(SwAddNeChip<F, 6, 16>),
+    SwEcDoubleRv32_48(SwDoubleChip<F, 6, 16>),
     // 32 limbs prime
-    TeEcAddRv32_32(TeEcAddChip<F, 2, 32>),
+    TeEcAddRv32_32(TeAddChip<F, 2, 32>),
     // 48 limbs prime
-    TeEcAddRv32_48(TeEcAddChip<F, 6, 16>),
+    TeEcAddRv32_48(TeAddChip<F, 6, 16>),
 }
 
 #[derive(ChipUsageGetter, Chip, AnyEnum, From, BytesStateful)]
@@ -169,7 +170,7 @@ impl<F: PrimeField32> VmExtension<F> for EccExtension {
             if bytes <= 32 {
                 match curve.coeffs.clone() {
                     CurveCoeffs::SwCurve(SwCurveConfig { a, b: _ }) => {
-                        let sw_add_ne_chip = EcAddNeChip::new(
+                        let sw_add_ne_chip = SwAddNeChip::new(
                             Rv32VecHeapAdapterChip::<F, 2, 2, 2, 32, 32>::new(
                                 execution_bus,
                                 program_bus,
@@ -188,7 +189,7 @@ impl<F: PrimeField32> VmExtension<F> for EccExtension {
                                 .clone()
                                 .map(|x| VmOpcode::from_usize(x + sw_start_offset)),
                         )?;
-                        let sw_double_chip = EcDoubleChip::new(
+                        let sw_double_chip = SwDoubleChip::new(
                             Rv32VecHeapAdapterChip::<F, 1, 2, 2, 32, 32>::new(
                                 execution_bus,
                                 program_bus,
@@ -211,7 +212,7 @@ impl<F: PrimeField32> VmExtension<F> for EccExtension {
                     }
 
                     CurveCoeffs::TeCurve(TeCurveConfig { a, d }) => {
-                        let te_add_chip = TeEcAddChip::new(
+                        let te_add_chip = TeAddChip::new(
                             Rv32VecHeapAdapterChip::<F, 2, 2, 2, 32, 32>::new(
                                 execution_bus,
                                 program_bus,
@@ -237,7 +238,7 @@ impl<F: PrimeField32> VmExtension<F> for EccExtension {
             } else if bytes <= 48 {
                 match curve.coeffs.clone() {
                     CurveCoeffs::SwCurve(SwCurveConfig { a, b: _ }) => {
-                        let sw_add_ne_chip = EcAddNeChip::new(
+                        let sw_add_ne_chip = SwAddNeChip::new(
                             Rv32VecHeapAdapterChip::<F, 2, 6, 6, 16, 16>::new(
                                 execution_bus,
                                 program_bus,
@@ -256,7 +257,7 @@ impl<F: PrimeField32> VmExtension<F> for EccExtension {
                                 .clone()
                                 .map(|x| VmOpcode::from_usize(x + sw_start_offset)),
                         )?;
-                        let sw_double_chip = EcDoubleChip::new(
+                        let sw_double_chip = SwDoubleChip::new(
                             Rv32VecHeapAdapterChip::<F, 1, 6, 6, 16, 16>::new(
                                 execution_bus,
                                 program_bus,
@@ -279,7 +280,7 @@ impl<F: PrimeField32> VmExtension<F> for EccExtension {
                     }
 
                     CurveCoeffs::TeCurve(TeCurveConfig { a, d }) => {
-                        let te_add_chip = TeEcAddChip::new(
+                        let te_add_chip = TeAddChip::new(
                             Rv32VecHeapAdapterChip::<F, 2, 6, 6, 16, 16>::new(
                                 execution_bus,
                                 program_bus,

examples/ecc/openvm/app.vmexe

@@ -1,11 +1,14 @@
 mod add;
 pub use add::*;
 
+mod utils;
+
 #[cfg(test)]
 mod tests;
 
 use std::sync::{Arc, Mutex};
 
+use num_bigint::BigUint;
 use openvm_circuit::{arch::VmChipWrapper, system::memory::OfflineMemory};
 use openvm_circuit_derive::InstructionExecutor;
 use openvm_circuit_primitives::var_range::SharedVariableRangeCheckerChip;
@@ -14,50 +17,36 @@ use openvm_ecc_transpiler::Rv32EdwardsOpcode;
 use openvm_mod_circuit_builder::{ExprBuilderConfig, FieldExpressionCoreChip};
 use openvm_rv32_adapters::Rv32VecHeapAdapterChip;
 use openvm_stark_backend::p3_field::PrimeField32;
+use utils::jacobi;
 
 /// BLOCK_SIZE: how many cells do we read at a time, must be a power of 2.
 /// BLOCKS: how many blocks do we need to represent one input or output
 /// For example, for bls12_381, BLOCK_SIZE = 16, each element has 3 blocks and with two elements per input AffinePoint, BLOCKS = 6.
 /// For secp256k1, BLOCK_SIZE = 32, BLOCKS = 2.
 #[derive(Chip, ChipUsageGetter, InstructionExecutor, BytesStateful)]
-pub struct TeEcAddChip<F: PrimeField32, const BLOCKS: usize, const BLOCK_SIZE: usize>(
+pub struct TeAddChip<F: PrimeField32, const BLOCKS: usize, const BLOCK_SIZE: usize>(
     VmChipWrapper<
         F,
         Rv32VecHeapAdapterChip<F, 2, BLOCKS, BLOCKS, BLOCK_SIZE, BLOCK_SIZE>,
         FieldExpressionCoreChip,
     >,
 );
 
-// converts from num_bigint::BigUint to num_bigint_dig::BigInt in order to use num_bigint_dig::algorithms::jacobi
-fn num_bigint_to_num_bigint_dig(x: &num_bigint::BigUint) -> num_bigint_dig::BigInt {
-    num_bigint_dig::BigInt::from_bytes_le(num_bigint_dig::Sign::Plus, &x.to_bytes_le())
-}
-
 impl<F: PrimeField32, const BLOCKS: usize, const BLOCK_SIZE: usize>
-    TeEcAddChip<F, BLOCKS, BLOCK_SIZE>
+    TeAddChip<F, BLOCKS, BLOCK_SIZE>
 {
     pub fn new(
         adapter: Rv32VecHeapAdapterChip<F, 2, BLOCKS, BLOCKS, BLOCK_SIZE, BLOCK_SIZE>,
         config: ExprBuilderConfig,
         offset: usize,
-        a: num_bigint::BigUint,
-        d: num_bigint::BigUint,
+        a: BigUint,
+        d: BigUint,
         range_checker: SharedVariableRangeCheckerChip,
         offline_memory: Arc<Mutex<OfflineMemory<F>>>,
     ) -> Self {
         // Ensure that the addition operation is complete
-        assert!(
-            num_bigint_dig::algorithms::jacobi(
-                &num_bigint_to_num_bigint_dig(&a),
-                &num_bigint_to_num_bigint_dig(&config.modulus)
-            ) == 1
-        );
-        assert!(
-            num_bigint_dig::algorithms::jacobi(
-                &num_bigint_to_num_bigint_dig(&d),
-                &num_bigint_to_num_bigint_dig(&config.modulus)
-            ) == -1
-        );
+        assert!(jacobi(&a.clone().into(), &config.modulus.clone().into()) == 1);
+        assert!(jacobi(&d.clone().into(), &config.modulus.clone().into()) == -1);
 
         let expr = ec_add_expr(config, range_checker.bus(), a, d);
         let core = FieldExpressionCoreChip::new(

examples/i256/openvm/app.vmexe

@@ -14,7 +14,7 @@ use openvm_rv32_adapters::{rv32_write_heap_default, Rv32VecHeapAdapterChip};
 use openvm_stark_backend::p3_field::FieldAlgebra;
 use openvm_stark_sdk::p3_baby_bear::BabyBear;
 
-use super::TeEcAddChip;
+use super::TeAddChip;
 
 const NUM_LIMBS: usize = 32;
 const LIMB_BITS: usize = 8;
@@ -118,7 +118,7 @@ fn test_add() {
         tester.address_bits(),
         bitwise_chip.clone(),
     );
-    let mut chip = TeEcAddChip::new(
+    let mut chip = TeAddChip::new(
         adapter,
         config,
         Rv32EdwardsOpcode::CLASS_OFFSET,

extensions/ecc/circuit/Cargo.toml

@@ -0,0 +1,101 @@
+use num_bigint::BigInt;
+use num_integer::Integer;
+use num_traits::{sign::Signed, One, Zero};
+
+/// Jacobi returns the Jacobi symbol (x/y), either +1, -1, or 0.
+/// The y argument must be an odd integer.
+pub fn jacobi(x: &BigInt, y: &BigInt) -> isize {
+    if !y.is_odd() {
+        panic!(
+            "invalid arguments, y must be an odd integer,but got {:?}",
+            y
+        );
+    }
+
+    let mut a = x.clone();
+    let mut b = y.clone();
+    let mut j = 1;
+
+    if b.is_negative() {
+        if a.is_negative() {
+            j = -1;
+        }
+        b = -b;
+    }
+
+    loop {
+        if b.is_one() {
+            return j;
+        }
+        if a.is_zero() {
+            return 0;
+        }
+
+        a = a.mod_floor(&b);
+        if a.is_zero() {
+            return 0;
+        }
+
+        // a > 0
+
+        // handle factors of 2 in a
+        let s = a.trailing_zeros().unwrap();
+        if s & 1 != 0 {
+            //let bmod8 = b.get_limb(0) & 7;
+            let bmod8 = mod_2_to_the_k(&b, 3);
+            if bmod8 == BigInt::from(3) || bmod8 == BigInt::from(5) {
+                j = -j;
+            }
+        }
+
+        let c = &a >> s; // a = 2^s*c
+
+        // swap numerator and denominator
+        if mod_2_to_the_k(&b, 2) == BigInt::from(3) && mod_2_to_the_k(&c, 2) == BigInt::from(3) {
+            j = -j
+        }
+
+        a = b;
+        b = c;
+    }
+}
+
+fn mod_2_to_the_k(x: &BigInt, k: u32) -> BigInt {
+    x & BigInt::from(2u32.pow(k) - 1)
+}
+#[cfg(test)]
+mod tests {
+    use num_traits::FromPrimitive;
+
+    use super::*;
+
+    #[test]
+    fn test_jacobi() {
+        let cases = [
+            [0, 1, 1],
+            [0, -1, 1],
+            [1, 1, 1],
+            [1, -1, 1],
+            [0, 5, 0],
+            [1, 5, 1],
+            [2, 5, -1],
+            [-2, 5, -1],
+            [2, -5, -1],
+            [-2, -5, 1],
+            [3, 5, -1],
+            [5, 5, 0],
+            [-5, 5, 0],
+            [6, 5, 1],
+            [6, -5, 1],
+            [-6, 5, 1],
+            [-6, -5, -1],
+        ];
+
+        for case in cases.iter() {
+            let x = BigInt::from_i64(case[0]).unwrap();
+            let y = BigInt::from_i64(case[1]).unwrap();
+
+            assert_eq!(case[2] as isize, jacobi(&x, &y), "jacobi({}, {})", x, y);
+        }
+    }
+}

extensions/ecc/circuit/src/ecc_extension.rs

@@ -3,7 +3,7 @@ use std::{cell::RefCell, rc::Rc};
 use openvm_circuit_primitives::var_range::VariableRangeCheckerBus;
 use openvm_mod_circuit_builder::{ExprBuilder, ExprBuilderConfig, FieldExpr};
 
-pub fn ec_add_ne_expr(
+pub fn sw_add_ne_expr(
     config: ExprBuilderConfig, // The coordinate field.
     range_bus: VariableRangeCheckerBus,
 ) -> FieldExpr {

extensions/ecc/circuit/src/edwards_chip/mod.rs

@@ -5,7 +5,7 @@ use num_traits::One;
 use openvm_circuit_primitives::var_range::VariableRangeCheckerBus;
 use openvm_mod_circuit_builder::{ExprBuilder, ExprBuilderConfig, FieldExpr, FieldVariable};
 
-pub fn ec_double_ne_expr(
+pub fn sw_double_ne_expr(
     config: ExprBuilderConfig, // The coordinate field.
     range_bus: VariableRangeCheckerBus,
     a_biguint: BigUint,