chore: Requested refactoring

src/provider/hyperkzg.rs

@@ -63,14 +63,16 @@ impl<E, NE> EvaluationEngine<E, NE>
 where
   E: Engine,
   NE: NovaEngine<GE = E::G1, Scalar = E::Fr, CE = KZGCommitmentEngine<E>>,
+  E::G1: DlogGroup<ScalarExt = E::Fr, AffineExt = E::G1Affine>,
+  // the following bounds repeat existing, satisfied bounds on associated types of the above
+  // but are required since the equality constraints we use in the above do not transitively carry bounds
+  // we should be able to remove most of those constraints when rust supports associated_type_bounds
   E::Fr: Serialize + DeserializeOwned,
   E::G1Affine: Serialize + DeserializeOwned,
+  E::G1Affine: TranscriptReprTrait<E::G1>, // TODO: this bound on DlogGroup is really unusable!
   E::G2Affine: Serialize + DeserializeOwned,
-  E::G1: DlogGroup<ScalarExt = E::Fr, AffineExt = E::G1Affine>,
-  E::Fr: PrimeFieldBits,
+  E::Fr: PrimeFieldBits + TranscriptReprTrait<E::G1>,
   <E::G1 as Group>::Base: TranscriptReprTrait<E::G1>,
-  E::Fr: TranscriptReprTrait<E::G1>,
-  E::G1Affine: TranscriptReprTrait<E::G1>, // TODO: this bound on DlogGroup is really unusable!
 {
   fn compute_challenge(
     com: &[E::G1Affine],
@@ -175,34 +177,6 @@ where
     K_x += &tmp;
     K_x
   }
-
-  fn kzg10_open(ck: &UniversalKZGParam<E>, f_x: &UniPoly<E::Fr>, u: E::Fr) -> E::G1Affine {
-    // On input f(x) and u compute the witness polynomial used to prove
-    // that f(u) = v. The main part of this is to compute the
-    // division (f(x) - f(u)) / (x - u), but we don't use a general
-    // division algorithm, we make use of the fact that the division
-    // never has a remainder, and that the denominator is always a linear
-    // polynomial. The cost is (d-1) mults + (d-1) adds in E::Scalar, where
-    // d is the degree of f.
-    //
-    // We use the fact that if we compute the quotient of f(x)/(x-u),
-    // there will be a remainder, but it'll be v = f(u).  Put another way
-    // the quotient of f(x)/(x-u) and (f(x) - f(v))/(x-u) is the
-    // same.  One advantage is that computing f(u) could be decoupled
-    // from kzg_open, it could be done later or separate from computing W.
-
-    let d = f_x.coeffs.len();
-
-    // Compute h(x) = f(x)/(x - u)
-    let mut h = vec![E::Fr::ZERO; d];
-    for i in (1..d).rev() {
-      h[i - 1] = f_x[i] + h[i] * u;
-    }
-
-    <NE::CE as CommitmentEngineTrait<NE>>::commit(ck, &h)
-      .comm
-      .to_affine()
-  }
 }
 
 impl<E, NE> EvaluationEngineTrait<NE> for EvaluationEngine<E, NE>
@@ -271,7 +245,10 @@ where
     let K_x = Self::compute_k_polynomial(&batched_Pi, &Q_x, &D, &R_x, a);
 
     // TODO: since this is a usual KZG10 we should use it as utility instead
-    let C_H = Self::kzg10_open(ck, &K_x, a);
+    let h = K_x.divide_minus_u(a).unwrap();
+    let C_H = <NE::CE as CommitmentEngineTrait<NE>>::commit(ck, &h.coeffs)
+      .comm
+      .to_affine();
 
     Ok(EvaluationArgument::<E> {
       comms,
@@ -380,7 +357,8 @@ where
       .collect::<Vec<_>>();
 
     let pairing_result = E::multi_miller_loop(pairing_input_refs.as_slice()).final_exponentiation();
-    if pairing_result.is_identity().unwrap_u8() == 0x00 {
+    let successful: bool = pairing_result.is_identity().into();
+    if !successful {
       return Err(NovaError::ProofVerifyError);
     }
     Ok(())

src/provider/non_hiding_zeromorph.rs

@@ -110,13 +110,7 @@ where
     point: &E::Fr,
   ) -> Result<(UVKZGProof<E>, UVKZGEvaluation<E>), NovaError> {
     let prover_param = prover_param.borrow();
-    let divisor = UVKZGPoly {
-      coeffs: vec![-*point, E::Fr::ONE],
-    };
-    let witness_polynomial = polynomial
-      .divide_with_q_and_r(&divisor)
-      .map(|(q, _r)| q)
-      .ok_or(NovaError::PCSError(PCSError::ZMError))?;
+    let witness_polynomial = polynomial.divide_minus_u(*point).unwrap();
     let proof = <E::G1 as DlogGroup>::vartime_multiscalar_mul(
       witness_polynomial.coeffs.as_slice(),
       &prover_param.powers_of_g()[..witness_polynomial.coeffs.len()],

src/spartan/polys/univariate.rs

@@ -74,6 +74,37 @@ impl<Scalar: PrimeField> UniPoly<Scalar> {
     }
   }
 
+  /// Divides f(x) by x-a and returns quotient polynomial with no reminder
+  /// This is a common use case for polynomial divisions in KZG-based PCS.
+  pub fn divide_minus_u(&self, u: Scalar) -> Option<Self> {
+    if self.is_zero() {
+      Some(Self::zero())
+    } else {
+      // On input f(x) and u compute the witness polynomial used to prove
+      // that f(u) = v. The main part of this is to compute the
+      // division (f(x) - f(u)) / (x - u), but we don't use a general
+      // division algorithm, we make use of the fact that the division
+      // never has a remainder, and that the denominator is always a linear
+      // polynomial. The cost is (d-1) mults + (d-1) adds in E::Scalar, where
+      // d is the degree of f.
+      //
+      // We use the fact that if we compute the quotient of f(x)/(x-u),
+      // there will be a remainder, but it'll be v = f(u).  Put another way
+      // the quotient of f(x)/(x-u) and (f(x) - f(v))/(x-u) is the
+      // same.  One advantage is that computing f(u) could be decoupled
+      // from kzg_open, it could be done later or separate from computing W.
+
+      let d = self.coeffs.len();
+
+      // Compute h(x) = f(x)/(x - u)
+      let mut h = vec![Scalar::ZERO; d];
+      for i in (1..d).rev() {
+        h[i - 1] = self.coeffs[i] + h[i] * u;
+      }
+      Some(Self::new(h))
+    }
+  }
+
   fn is_zero(&self) -> bool {
     self.coeffs.is_empty() || self.coeffs.iter().all(|c| c == &Scalar::ZERO)
   }
@@ -332,6 +363,7 @@ mod tests {
       for b_degree in 0..50 {
         let dividend = UniPoly::<Fr>::random(a_degree, rng);
         let divisor = UniPoly::<Fr>::random(b_degree, rng);
+
         if let Some((quotient, remainder)) = UniPoly::divide_with_q_and_r(&dividend, &divisor) {
           let mut prod = naive_mul(&divisor, &quotient);
           prod += &remainder;
@@ -341,6 +373,25 @@ mod tests {
     }
   }
 
+  #[test]
+  fn test_divide_minus_u() {
+    fn test_inner<Fr: PrimeField>() {
+      let rng = &mut ChaCha20Rng::from_seed([0u8; 32]);
+      let dividend = UniPoly::<Fr>::random(50, rng);
+      let u = Fr::random(rng);
+      let divisor = UniPoly::new(vec![-u, Fr::ONE]);
+
+      let (q1, _) = dividend.divide_with_q_and_r(&divisor).unwrap();
+      let q2 = dividend.divide_minus_u(u).unwrap();
+
+      assert_eq!(q1, q2);
+    }
+
+    test_inner::<pasta_curves::pallas::Scalar>();
+    test_inner::<bn256_grumpkin::bn256::Scalar>();
+    test_inner::<secp256k1::Scalar>();
+  }
+
   #[test]
   fn test_divide_polynomials_random() {
     divide_polynomials_random::<pasta_curves::pallas::Scalar>();