fix: use seed for deterministic re-execution (#1741)

jonathanpwang · web-flow · commit 664c9d9b783e · 2025-06-15T07:40:03.000-07:00
The `NonQrHintSubEx` phantom sub-executor now uses a fixed seed so that execution is deterministic between different instances of the VM. This is not strictly necessary since the constructed non-QR could be passed between runs for reproducibility, but it seems a better property that re-runs of the VM have the same behavior (e.g., for segmentation). The only additional consideration is that now you can potentially add moduli to the VM config that take a little longer for the VM to initialize - but making it truly take a long time seems to involve breaking generalized Riemann hypothesis. Lastly, the phantom sub-executors `DecompressHintSubEx` and `NonQrHintSubEx` in Weierstrass extension were already removed from the spec since we now use the phantom sub-executors in the modular arithmetic extension, but these variants were not deleted from the code. So they are deleted in this PR. @Avaneesh-axiom do you know if this was from a bad merge previously?
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/extensions/algebra/circuit/src/modular_extension.rs b/extensions/algebra/circuit/src/modular_extension.rs
@@ -16,7 +16,7 @@ use openvm_instructions::{LocalOpcode, PhantomDiscriminant, VmOpcode};
 use openvm_mod_circuit_builder::ExprBuilderConfig;
 use openvm_rv32_adapters::{Rv32IsEqualModAdapterChip, Rv32VecHeapAdapterChip};
 use openvm_stark_backend::p3_field::PrimeField32;
-use rand::{rngs::StdRng, SeedableRng};
+use rand::Rng;
 use serde::{Deserialize, Serialize};
 use serde_with::{serde_as, DisplayFromStr};
 use strum::EnumCount;
@@ -263,6 +263,7 @@ pub(crate) mod phantom {
     use openvm_instructions::{riscv::RV32_MEMORY_AS, PhantomDiscriminant};
     use openvm_rv32im_circuit::adapters::unsafe_read_rv32_register;
     use openvm_stark_backend::p3_field::PrimeField32;
+    use rand::{rngs::StdRng, SeedableRng};
 
     use super::{find_non_qr, mod_sqrt};
 
@@ -353,7 +354,15 @@ pub(crate) mod phantom {
 
     impl NonQrHintSubEx {
         pub fn new(supported_moduli: Vec<BigUint>) -> Self {
-            let non_qrs = supported_moduli.iter().map(find_non_qr).collect();
+            // Use deterministic seed so that the non-QR are deterministic between different
+            // instances of the VM. The seed determines the runtime of Tonelli-Shanks, if the
+            // algorithm is necessary, which affects the time it takes to construct and initialize
+            // the VM but does not affect the runtime.
+            let mut rng = StdRng::from_seed([0u8; 32]);
+            let non_qrs = supported_moduli
+                .iter()
+                .map(|modulus| find_non_qr(modulus, &mut rng))
+                .collect();
             Self {
                 supported_moduli,
                 non_qrs,
@@ -457,7 +466,7 @@ pub fn mod_sqrt(x: &BigUint, modulus: &BigUint, non_qr: &BigUint) -> Option<BigU
 }
 
 // Returns a non-quadratic residue in the field
-pub fn find_non_qr(modulus: &BigUint) -> BigUint {
+pub fn find_non_qr(modulus: &BigUint, rng: &mut impl Rng) -> BigUint {
     if modulus % 4u32 == BigUint::from(3u8) {
         // p = 3 mod 4 then -1 is a quadratic residue
         modulus - BigUint::one()
@@ -466,7 +475,6 @@ pub fn find_non_qr(modulus: &BigUint) -> BigUint {
         // since 2^((p-1)/2) = (-1)^((p^2-1)/8)
         BigUint::from_u8(2u8).unwrap()
     } else {
-        let mut rng = StdRng::from_entropy();
         let mut non_qr = rng.gen_biguint_range(
             &BigUint::from_u8(2).unwrap(),
             &(modulus - BigUint::from_u8(1).unwrap()),
diff --git a/extensions/ecc/circuit/Cargo.toml b/extensions/ecc/circuit/Cargo.toml
@@ -26,8 +26,6 @@ strum = { workspace = true }
 derive_more = { workspace = true }
 derive-new = { workspace = true }
 once_cell = { workspace = true, features = ["std"] }
-eyre = { workspace = true }
-num-integer = { workspace = true }
 serde = { workspace = true }
 serde_with = { workspace = true }
 lazy_static = { workspace = true }
diff --git a/extensions/ecc/circuit/src/weierstrass_extension.rs b/extensions/ecc/circuit/src/weierstrass_extension.rs
@@ -13,8 +13,8 @@ use openvm_circuit_primitives::bitwise_op_lookup::{
     BitwiseOperationLookupBus, SharedBitwiseOperationLookupChip,
 };
 use openvm_circuit_primitives_derive::{Chip, ChipUsageGetter};
-use openvm_ecc_transpiler::{EccPhantom, Rv32WeierstrassOpcode};
-use openvm_instructions::{LocalOpcode, PhantomDiscriminant, VmOpcode};
+use openvm_ecc_transpiler::Rv32WeierstrassOpcode;
+use openvm_instructions::{LocalOpcode, VmOpcode};
 use openvm_mod_circuit_builder::ExprBuilderConfig;
 use openvm_rv32_adapters::Rv32VecHeapAdapterChip;
 use openvm_stark_backend::p3_field::PrimeField32;
@@ -224,228 +224,11 @@ impl<F: PrimeField32> VmExtension<F> for WeierstrassExtension {
                 panic!("Modulus too large");
             }
         }
-        let non_qr_hint_sub_ex = phantom::NonQrHintSubEx::new(self.supported_curves.clone());
-        builder.add_phantom_sub_executor(
-            non_qr_hint_sub_ex.clone(),
-            PhantomDiscriminant(EccPhantom::HintNonQr as u16),
-        )?;
-        builder.add_phantom_sub_executor(
-            phantom::DecompressHintSubEx::new(non_qr_hint_sub_ex),
-            PhantomDiscriminant(EccPhantom::HintDecompress as u16),
-        )?;
 
         Ok(inventory)
     }
 }
 
-pub(crate) mod phantom {
-    use std::{
-        iter::{once, repeat},
-        ops::Deref,
-    };
-
-    use eyre::bail;
-    use num_bigint::BigUint;
-    use num_integer::Integer;
-    use num_traits::One;
-    use openvm_algebra_circuit::{find_non_qr, mod_sqrt};
-    use openvm_circuit::{
-        arch::{PhantomSubExecutor, Streams},
-        system::memory::MemoryController,
-    };
-    use openvm_instructions::{riscv::RV32_MEMORY_AS, PhantomDiscriminant};
-    use openvm_rv32im_circuit::adapters::unsafe_read_rv32_register;
-    use openvm_stark_backend::p3_field::PrimeField32;
-
-    use super::CurveConfig;
-
-    // Hint for a decompression
-    // if possible is true, then `sqrt` is the decompressed y-coordinate
-    // if possible is false, then `sqrt` is such that
-    // `sqrt^2 = rhs * non_qr` where `rhs` is the rhs of the curve equation
-    pub struct DecompressionHint<T> {
-        pub possible: bool,
-        pub sqrt: T,
-    }
-
-    #[derive(derive_new::new)]
-    pub struct DecompressHintSubEx(NonQrHintSubEx);
-
-    impl Deref for DecompressHintSubEx {
-        type Target = NonQrHintSubEx;
-
-        fn deref(&self) -> &NonQrHintSubEx {
-            &self.0
-        }
-    }
-
-    impl<F: PrimeField32> PhantomSubExecutor<F> for DecompressHintSubEx {
-        fn phantom_execute(
-            &mut self,
-            memory: &MemoryController<F>,
-            streams: &mut Streams<F>,
-            _: PhantomDiscriminant,
-            a: F,
-            b: F,
-            c_upper: u16,
-        ) -> eyre::Result<()> {
-            let c_idx = c_upper as usize;
-            if c_idx >= self.supported_curves.len() {
-                bail!(
-                    "Curve index {c_idx} out of range: {} supported curves",
-                    self.supported_curves.len()
-                );
-            }
-            let curve = &self.supported_curves[c_idx];
-            let rs1 = unsafe_read_rv32_register(memory, a);
-            let num_limbs: usize = if curve.modulus.bits().div_ceil(8) <= 32 {
-                32
-            } else if curve.modulus.bits().div_ceil(8) <= 48 {
-                48
-            } else {
-                bail!("Modulus too large")
-            };
-            let mut x_limbs: Vec<u8> = Vec::with_capacity(num_limbs);
-            for i in 0..num_limbs {
-                let limb = memory.unsafe_read_cell(
-                    F::from_canonical_u32(RV32_MEMORY_AS),
-                    F::from_canonical_u32(rs1 + i as u32),
-                );
-                x_limbs.push(limb.as_canonical_u32() as u8);
-            }
-            let x = BigUint::from_bytes_le(&x_limbs);
-            let rs2 = unsafe_read_rv32_register(memory, b);
-            let rec_id = memory.unsafe_read_cell(
-                F::from_canonical_u32(RV32_MEMORY_AS),
-                F::from_canonical_u32(rs2),
-            );
-            let hint = self.decompress_point(x, rec_id.as_canonical_u32() & 1 == 1, c_idx);
-            let hint_bytes = once(F::from_bool(hint.possible))
-                .chain(repeat(F::ZERO))
-                .take(4)
-                .chain(
-                    hint.sqrt
-                        .to_bytes_le()
-                        .into_iter()
-                        .map(F::from_canonical_u8)
-                        .chain(repeat(F::ZERO))
-                        .take(num_limbs),
-                )
-                .collect();
-            streams.hint_stream = hint_bytes;
-            Ok(())
-        }
-    }
-
-    impl DecompressHintSubEx {
-        /// Given `x` in the coordinate field of the curve, and the recovery id,
-        /// return the unique `y` such that `(x, y)` is a point on the curve and
-        /// `y` has the same parity as the recovery id.
-        ///
-        /// If no such `y` exists, return the square root of `(x^3 + ax + b) * non_qr`
-        /// where `non_qr` is a quadratic nonresidue of the field.
-        fn decompress_point(
-            &self,
-            x: BigUint,
-            is_y_odd: bool,
-            curve_idx: usize,
-        ) -> DecompressionHint<BigUint> {
-            let curve = &self.supported_curves[curve_idx];
-            let alpha = ((&x * &x * &x) + (&x * &curve.a) + &curve.b) % &curve.modulus;
-            match mod_sqrt(&alpha, &curve.modulus, &self.non_qrs[curve_idx]) {
-                Some(beta) => {
-                    if is_y_odd == beta.is_odd() {
-                        DecompressionHint {
-                            possible: true,
-                            sqrt: beta,
-                        }
-                    } else {
-                        DecompressionHint {
-                            possible: true,
-                            sqrt: &curve.modulus - &beta,
-                        }
-                    }
-                }
-                None => {
-                    debug_assert_eq!(
-                        self.non_qrs[curve_idx]
-                            .modpow(&((&curve.modulus - BigUint::one()) >> 1), &curve.modulus),
-                        &curve.modulus - BigUint::one()
-                    );
-                    let sqrt = mod_sqrt(
-                        &(&alpha * &self.non_qrs[curve_idx]),
-                        &curve.modulus,
-                        &self.non_qrs[curve_idx],
-                    )
-                    .unwrap();
-                    DecompressionHint {
-                        possible: false,
-                        sqrt,
-                    }
-                }
-            }
-        }
-    }
-
-    #[derive(Clone)]
-    pub struct NonQrHintSubEx {
-        pub supported_curves: Vec<CurveConfig>,
-        pub non_qrs: Vec<BigUint>,
-    }
-
-    impl NonQrHintSubEx {
-        pub fn new(supported_curves: Vec<CurveConfig>) -> Self {
-            let non_qrs = supported_curves
-                .iter()
-                .map(|curve| find_non_qr(&curve.modulus))
-                .collect();
-            Self {
-                supported_curves,
-                non_qrs,
-            }
-        }
-    }
-
-    impl<F: PrimeField32> PhantomSubExecutor<F> for NonQrHintSubEx {
-        fn phantom_execute(
-            &mut self,
-            _: &MemoryController<F>,
-            streams: &mut Streams<F>,
-            _: PhantomDiscriminant,
-            _: F,
-            _: F,
-            c_upper: u16,
-        ) -> eyre::Result<()> {
-            let c_idx = c_upper as usize;
-            if c_idx >= self.supported_curves.len() {
-                bail!(
-                    "Curve index {c_idx} out of range: {} supported curves",
-                    self.supported_curves.len()
-                );
-            }
-            let curve = &self.supported_curves[c_idx];
-
-            let num_limbs: usize = if curve.modulus.bits().div_ceil(8) <= 32 {
-                32
-            } else if curve.modulus.bits().div_ceil(8) <= 48 {
-                48
-            } else {
-                bail!("Modulus too large")
-            };
-
-            let hint_bytes = self.non_qrs[c_idx]
-                .to_bytes_le()
-                .into_iter()
-                .map(F::from_canonical_u8)
-                .chain(repeat(F::ZERO))
-                .take(num_limbs)
-                .collect();
-            streams.hint_stream = hint_bytes;
-            Ok(())
-        }
-    }
-}
-
 // Convenience constants for constructors
 lazy_static! {
     // The constants are taken from: https://en.bitcoin.it/wiki/Secp256k1
diff --git a/extensions/ecc/guest/src/lib.rs b/extensions/ecc/guest/src/lib.rs
@@ -32,8 +32,6 @@ pub enum SwBaseFunct7 {
     SwAddNe = 0,
     SwDouble,
     SwSetup,
-    HintDecompress,
-    HintNonQr,
 }
 
 impl SwBaseFunct7 {
diff --git a/extensions/ecc/transpiler/src/lib.rs b/extensions/ecc/transpiler/src/lib.rs
@@ -1,7 +1,6 @@
 use openvm_ecc_guest::{SwBaseFunct7, OPCODE, SW_FUNCT3};
 use openvm_instructions::{
-    instruction::Instruction, riscv::RV32_REGISTER_NUM_LIMBS, LocalOpcode, PhantomDiscriminant,
-    VmOpcode,
+    instruction::Instruction, riscv::RV32_REGISTER_NUM_LIMBS, LocalOpcode, VmOpcode,
 };
 use openvm_instructions_derive::LocalOpcode;
 use openvm_stark_backend::p3_field::PrimeField32;
@@ -22,13 +21,6 @@ pub enum Rv32WeierstrassOpcode {
     SETUP_EC_DOUBLE,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, FromRepr)]
-#[repr(u16)]
-pub enum EccPhantom {
-    HintDecompress = 0x40,
-    HintNonQr = 0x41,
-}
-
 #[derive(Default)]
 pub struct EccTranspilerExtension;
 
@@ -58,26 +50,6 @@ impl<F: PrimeField32> TranspilerExtension<F> for EccTranspilerExtension {
             let curve_idx =
                 ((dec_insn.funct7 as u8) / SwBaseFunct7::SHORT_WEIERSTRASS_MAX_KINDS) as usize;
             let curve_idx_shift = curve_idx * Rv32WeierstrassOpcode::COUNT;
-            if let Some(SwBaseFunct7::HintDecompress) = SwBaseFunct7::from_repr(base_funct7) {
-                assert_eq!(dec_insn.rd, 0);
-                return Some(TranspilerOutput::one_to_one(Instruction::phantom(
-                    PhantomDiscriminant(EccPhantom::HintDecompress as u16),
-                    F::from_canonical_usize(RV32_REGISTER_NUM_LIMBS * dec_insn.rs1),
-                    F::from_canonical_usize(RV32_REGISTER_NUM_LIMBS * dec_insn.rs2),
-                    curve_idx as u16,
-                )));
-            }
-            if let Some(SwBaseFunct7::HintNonQr) = SwBaseFunct7::from_repr(base_funct7) {
-                assert_eq!(dec_insn.rd, 0);
-                assert_eq!(dec_insn.rs1, 0);
-                assert_eq!(dec_insn.rs2, 0);
-                return Some(TranspilerOutput::one_to_one(Instruction::phantom(
-                    PhantomDiscriminant(EccPhantom::HintNonQr as u16),
-                    F::ZERO,
-                    F::ZERO,
-                    curve_idx as u16,
-                )));
-            }
             if base_funct7 == SwBaseFunct7::SwSetup as u8 {
                 let local_opcode = match dec_insn.rs2 {
                     0 => Rv32WeierstrassOpcode::SETUP_EC_DOUBLE,

Original file line number	Diff line number	Diff line change
`@@ -32,8 +32,6 @@ pub enum SwBaseFunct7 {`
`32`	`32`	`SwAddNe = 0,`
`33`	`33`	`SwDouble,`
`34`	`34`	`SwSetup,`
`35`		`- HintDecompress,`
`36`		`- HintNonQr,`
`37`	`35`	`}`
`38`	`36`
`39`	`37`	`impl SwBaseFunct7 {`