From b1c8709f7e670f6767ea8fdd02c7b92ae6a07684 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Fri, 28 Jan 2022 10:55:17 -0800
Subject: [PATCH] addressed more comments

---
 plonky2/src/curve/ecdsa.rs       |  2 ++
 plonky2/src/gadgets/ecdsa.rs     |  7 ++-----
 plonky2/src/gadgets/nonnative.rs | 35 --------------------------------
 plonky2/src/iop/generator.rs     |  9 +-------
 4 files changed, 5 insertions(+), 48 deletions(-)

diff --git a/plonky2/src/curve/ecdsa.rs b/plonky2/src/curve/ecdsa.rs
index 787567aa..e86b40d8 100644
--- a/plonky2/src/curve/ecdsa.rs
+++ b/plonky2/src/curve/ecdsa.rs
@@ -26,6 +26,8 @@ pub fn verify_message<C: Curve>(
 ) -> bool {
     let ECDSASignature { r, s } = sig;
 
+    assert!(pk.0.is_valid());
+
     let c = s.inverse();
     let u1 = msg * c;
     let u2 = r * c;
diff --git a/plonky2/src/gadgets/ecdsa.rs b/plonky2/src/gadgets/ecdsa.rs
index 9a7a8257..2acd1c80 100644
--- a/plonky2/src/gadgets/ecdsa.rs
+++ b/plonky2/src/gadgets/ecdsa.rs
@@ -2,13 +2,9 @@ use std::marker::PhantomData;
 
 use crate::curve::curve_types::Curve;
 use crate::field::extension_field::Extendable;
-use crate::gadgets::arithmetic_u32::U32Target;
-use crate::gadgets::biguint::BigUintTarget;
 use crate::gadgets::curve::AffinePointTarget;
 use crate::gadgets::nonnative::NonNativeTarget;
 use crate::hash::hash_types::RichField;
-use crate::hash::poseidon::PoseidonHash;
-use crate::iop::target::{BoolTarget, Target};
 use crate::plonk::circuit_builder::CircuitBuilder;
 
 pub struct ECDSASecretKeyTarget<C: Curve>(NonNativeTarget<C::ScalarField>);
@@ -28,6 +24,8 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     ) {
         let ECDSASignatureTarget { r, s } = sig;
 
+        self.curve_assert_valid(&pk.0);
+
         let c = self.inv_nonnative(&s);
         let u1 = self.mul_nonnative(&msg, &c);
         let u2 = self.mul_nonnative(&r, &c);
@@ -62,7 +60,6 @@ mod tests {
     use crate::plonk::verifier::verify;
 
     #[test]
-    #[ignore]
     fn test_ecdsa_circuit() -> Result<()> {
         const D: usize = 2;
         type C = PoseidonGoldilocksConfig;
diff --git a/plonky2/src/gadgets/nonnative.rs b/plonky2/src/gadgets/nonnative.rs
index b513abbb..293a7183 100644
--- a/plonky2/src/gadgets/nonnative.rs
+++ b/plonky2/src/gadgets/nonnative.rs
@@ -624,41 +624,6 @@ mod tests {
         verify(proof, &data.verifier_only, &data.common)
     }
 
-    fn test_nonnative_many_muls_helper(num: usize) {
-        type FF = Secp256K1Base;
-        const D: usize = 2;
-        type C = PoseidonGoldilocksConfig;
-        type F = <C as GenericConfig<D>>::F;
-
-        let config = CircuitConfig::standard_ecc_config();
-        let mut unop_builder = CircuitBuilder::<F, D>::new(config.clone());
-        let mut op_builder = CircuitBuilder::<F, D>::new(config);
-
-        let ffs: Vec<_> = (0..num).map(|_| FF::rand()).collect();
-
-        let op_targets: Vec<_> = ffs
-            .iter()
-            .map(|&x| op_builder.constant_nonnative(x))
-            .collect();
-        op_builder.mul_many_nonnative(&op_targets);
-
-        let unop_targets: Vec<_> = ffs
-            .iter()
-            .map(|&x| unop_builder.constant_nonnative(x))
-            .collect();
-        let mut result = unop_targets[0].clone();
-        for i in 1..unop_targets.len() {
-            result = unop_builder.mul_nonnative(&result, &unop_targets[i]);
-        }
-    }
-
-    #[test]
-    fn test_nonnative_many_muls() {
-        for num in 2..10 {
-            test_nonnative_many_muls_helper(num);
-        }
-    }
-
     #[test]
     fn test_nonnative_neg() -> Result<()> {
         type FF = Secp256K1Base;
diff --git a/plonky2/src/iop/generator.rs b/plonky2/src/iop/generator.rs
index 5d36ed1d..fb089d29 100644
--- a/plonky2/src/iop/generator.rs
+++ b/plonky2/src/iop/generator.rs
@@ -170,14 +170,7 @@ impl<F: Field> GeneratedValues<F> {
     }
 
     pub fn set_biguint_target(&mut self, target: BigUintTarget, value: BigUint) {
-        let base = BigUint::from_u64(1 << 32).unwrap();
-        let mut limbs = Vec::new();
-        let mut current = value.clone();
-        while current > BigUint::zero() {
-            let (div, rem) = current.div_rem(&base);
-            current = div;
-            limbs.push(rem.to_u64_digits()[0] as u32);
-        }
+        let mut limbs = value.to_u32_digits();
 
         assert!(target.num_limbs() >= limbs.len());