Merge pull request #2151 from AleoHQ/correct_divide_by_zero

[NCC FYM] Correctly panic when dividing by zero poly

Author: howardwu

algorithms/src/fft/polynomial/dense.rs

@@ -14,10 +14,12 @@
 
 //! A polynomial represented in coefficient form.
 
+use super::PolyMultiplier;
 use crate::fft::{EvaluationDomain, Evaluations, Polynomial};
 use snarkvm_fields::{Field, PrimeField};
 use snarkvm_utilities::{cfg_iter_mut, serialize::*};
 
+use anyhow::Result;
 use num_traits::CheckedDiv;
 use rand::Rng;
 use std::{
@@ -31,8 +33,6 @@ use itertools::Itertools;
 #[cfg(not(feature = "serial"))]
 use rayon::prelude::*;
 
-use super::PolyMultiplier;
-
 /// Stores a polynomial in coefficient form.
 #[derive(Clone, PartialEq, Eq, Hash, Default, CanonicalSerialize, CanonicalDeserialize)]
 #[must_use]
@@ -156,7 +156,7 @@ impl<F: PrimeField> DensePolynomial<F> {
     pub fn divide_by_vanishing_poly(
         &self,
         domain: EvaluationDomain<F>,
-    ) -> Option<(DensePolynomial<F>, DensePolynomial<F>)> {
+    ) -> Result<(DensePolynomial<F>, DensePolynomial<F>)> {
         let self_poly = Polynomial::from(self);
         let vanishing_poly = Polynomial::from(domain.vanishing_polynomial());
         self_poly.divide_with_q_and_r(&vanishing_poly)
@@ -421,14 +421,14 @@ impl<F: Field> CheckedDiv for DensePolynomial<F> {
         let a: Polynomial<_> = self.into();
         let b: Polynomial<_> = divisor.into();
         match a.divide_with_q_and_r(&b) {
-            Some((divisor, remainder)) => {
+            Ok((divisor, remainder)) => {
                 if remainder.is_zero() {
                     Some(divisor)
                 } else {
                     None
                 }
             }
-            None => None,
+            Err(_) => None,
         }
     }
 }
@@ -592,11 +592,9 @@ mod tests {
             for b_degree in 0..70 {
                 let dividend = DensePolynomial::<Fr>::rand(a_degree, rng);
                 let divisor = DensePolynomial::<Fr>::rand(b_degree, rng);
-                if let Some((quotient, remainder)) =
-                    Polynomial::divide_with_q_and_r(&(&dividend).into(), &(&divisor).into())
-                {
-                    assert_eq!(dividend, &(&divisor * &quotient) + &remainder)
-                }
+                let (quotient, remainder) =
+                    Polynomial::divide_with_q_and_r(&(&dividend).into(), &(&divisor).into()).unwrap();
+                assert_eq!(dividend, &(&divisor * &quotient) + &remainder)
             }
         }
     }
@@ -615,6 +613,13 @@ mod tests {
         }
     }
 
+    #[test]
+    fn divide_poly_by_zero() {
+        let a = Polynomial::<Fr>::zero();
+        let b = Polynomial::<Fr>::zero();
+        assert!(a.divide_with_q_and_r(&b).is_err());
+    }
+
     #[test]
     fn mul_polynomials_random() {
         let rng = &mut TestRng::default();

algorithms/src/fft/polynomial/mod.rs

@@ -17,11 +17,11 @@
 use crate::fft::{EvaluationDomain, Evaluations};
 use snarkvm_fields::{Field, PrimeField};
 use snarkvm_utilities::{cfg_iter_mut, serialize::*, SerializationError};
+use Polynomial::*;
 
+use anyhow::{ensure, Result};
 use std::{borrow::Cow, convert::TryInto};
 
-use Polynomial::*;
-
 #[cfg(not(feature = "serial"))]
 use rayon::prelude::*;
 
@@ -218,13 +218,13 @@ impl<'a, F: Field> Polynomial<'a, F> {
     }
 
     /// Divide self by another (sparse or dense) polynomial, and returns the quotient and remainder.
-    pub fn divide_with_q_and_r(&self, divisor: &Self) -> Option<(DensePolynomial<F>, DensePolynomial<F>)> {
+    pub fn divide_with_q_and_r(&self, divisor: &Self) -> Result<(DensePolynomial<F>, DensePolynomial<F>)> {
+        ensure!(!divisor.is_zero(), "Dividing by zero polynomial is undefined");
+
         if self.is_zero() {
-            Some((DensePolynomial::zero(), DensePolynomial::zero()))
-        } else if divisor.is_zero() {
-            panic!("Dividing by zero polynomial")
+            Ok((DensePolynomial::zero(), DensePolynomial::zero()))
         } else if self.degree() < divisor.degree() {
-            Some((DensePolynomial::zero(), self.clone().into()))
+            Ok((DensePolynomial::zero(), self.clone().into()))
         } else {
             // Now we know that self.degree() >= divisor.degree();
             let mut quotient = vec![F::zero(); self.degree() - divisor.degree() + 1];
@@ -250,7 +250,7 @@ impl<'a, F: Field> Polynomial<'a, F> {
                     remainder.coeffs.pop();
                 }
             }
-            Some((DensePolynomial::from_coefficients_vec(quotient), remainder))
+            Ok((DensePolynomial::from_coefficients_vec(quotient), remainder))
         }
     }
 }

algorithms/src/snark/varuna/ahp/selectors.rs

@@ -12,19 +12,18 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use super::verifier::QueryPoints;
 use crate::fft::{DensePolynomial, EvaluationDomain};
-use anyhow::{anyhow, ensure, Result};
 use snarkvm_fields::{batch_inversion, PrimeField};
 use snarkvm_utilities::{cfg_into_iter, cfg_iter_mut, serialize::*};
 
+use anyhow::{ensure, Result};
 use itertools::Itertools;
 use std::collections::{BTreeMap, HashSet};
 
 #[cfg(not(feature = "serial"))]
 use rayon::prelude::*;
 
-use super::verifier::QueryPoints;
-
 /// Precompute a batch of selectors at challenges. We batch:
 /// - constraint domain selectors at alpha
 /// - variable domain selectors at beta
@@ -88,11 +87,13 @@ pub(crate) fn apply_randomized_selector<F: PrimeField>(
     if !remainder_witness {
         // Substituting in s_i, we get that poly_i * s_i / v_H = poly_i / v_H * (H_i.size() / H.size());
         let selector_time = start_timer!(|| "Compute selector without remainder witness");
-        let (mut h_i, remainder) =
-            poly.divide_by_vanishing_poly(*src_domain).ok_or(anyhow::anyhow!("could not divide by vanishing poly"))?;
-        ensure!(remainder.is_zero());
+
+        let (mut h_i, remainder) = poly.divide_by_vanishing_poly(*src_domain)?;
+        ensure!(remainder.is_zero(), "Failed to divide by vanishing polynomial - non-zero remainder ({remainder:?})");
+
         let multiplier = combiner * src_domain.size_as_field_element * target_domain.size_inv;
         cfg_iter_mut!(h_i.coeffs).for_each(|c| *c *= multiplier);
+
         end_timer!(selector_time);
         Ok((h_i, None))
     } else {
@@ -105,14 +106,16 @@ pub(crate) fn apply_randomized_selector<F: PrimeField>(
         // (\sum_i{c_i*s_i*poly_i})/v_H = h_1*v_H + x_g_1
         // That's what we're computing here.
         let selector_time = start_timer!(|| "Compute selector with remainder witness");
+
         let multiplier = combiner * src_domain.size_as_field_element * target_domain.size_inv;
         cfg_iter_mut!(poly.coeffs).for_each(|c| *c *= multiplier);
-        let (h_i, mut xg_i) =
-            poly.divide_by_vanishing_poly(*src_domain).ok_or(anyhow!("Could not divide by vanishing poly"))?;
+
+        let (h_i, mut xg_i) = poly.divide_by_vanishing_poly(*src_domain)?;
         xg_i = xg_i.mul_by_vanishing_poly(*target_domain);
-        let (xg_i, remainder) =
-            xg_i.divide_by_vanishing_poly(*src_domain).ok_or(anyhow!("Could not divide by vanishing poly"))?;
-        ensure!(remainder.is_zero());
+
+        let (xg_i, remainder) = xg_i.divide_by_vanishing_poly(*src_domain)?;
+        ensure!(remainder.is_zero(), "Failed to divide by vanishing polynomial - non-zero remainder ({remainder:?})");
+
         end_timer!(selector_time);
         Ok((h_i, Some(xg_i)))
     }