From 090cf797870160b67dfa22b894e6b1a73380c985 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Tue, 10 Aug 2021 11:48:53 -0700 Subject: [PATCH] Replace some old division code (#171) - Delete unsafe methods - Have related methods call the new div_add_extension method to simplify --- src/fri/recursive_verifier.rs | 4 --- src/gadgets/arithmetic.rs | 28 ++-------------- src/gadgets/arithmetic_extension.rs | 51 +++-------------------------- src/plonk/plonk_common.rs | 4 +-- 4 files changed, 8 insertions(+), 79 deletions(-) diff --git a/src/fri/recursive_verifier.rs b/src/fri/recursive_verifier.rs index 0ee786f7..f441f154 100644 --- a/src/fri/recursive_verifier.rs +++ b/src/fri/recursive_verifier.rs @@ -231,8 +231,6 @@ impl, const D: usize> CircuitBuilder { let single_composition_eval = alpha.reduce_base(&single_evals, self); let single_numerator = self.sub_extension(single_composition_eval, precomputed_reduced_evals.single); - // This division is safe because the denominator will be nonzero unless zeta is in the - // codeword domain, which occurs with negligible probability given a large extension field. sum = self.div_add_extension(single_numerator, vanish_zeta, sum); alpha.reset(); @@ -258,8 +256,6 @@ impl, const D: usize> CircuitBuilder { self.sub_two_extension(zs_composition_eval, interpol_val, subgroup_x, zeta_right); let zs_denominator = self.mul_extension(vanish_zeta, vanish_zeta_right); sum = alpha.shift(sum, self); - // This division is safe because the denominator will be nonzero unless zeta is in the - // codeword domain, which occurs with negligible probability given a large extension field. sum = self.div_add_extension(zs_numerator, zs_denominator, sum); sum diff --git a/src/gadgets/arithmetic.rs b/src/gadgets/arithmetic.rs index 9779da7e..27a204bc 100644 --- a/src/gadgets/arithmetic.rs +++ b/src/gadgets/arithmetic.rs @@ -159,31 +159,9 @@ impl, const D: usize> CircuitBuilder { /// Computes `x / y`. Results in an unsatisfiable instance if `y = 0`. pub fn div(&mut self, x: Target, y: Target) -> Target { - let y_inv = self.inverse(y); - self.mul(x, y_inv) - } - - /// Computes `q = x / y` by witnessing `q` and requiring that `q * y = x`. This can be unsafe in - /// some cases, as it allows `0 / 0 = `. - pub fn div_unsafe(&mut self, x: Target, y: Target) -> Target { - // Check for special cases where we can determine the result without an `ArithmeticGate`. - let zero = self.zero(); - let one = self.one(); - if x == zero { - return zero; - } - if y == one { - return x; - } - if let (Some(x_const), Some(y_const)) = - (self.target_as_constant(x), self.target_as_constant(y)) - { - return self.constant(x_const / y_const); - } - - let x_ext = self.convert_to_ext(x); - let y_ext = self.convert_to_ext(y); - self.div_unsafe_extension(x_ext, y_ext).0[0] + let x = self.convert_to_ext(x); + let y = self.convert_to_ext(y); + self.div_extension(x, y).0[0] } /// Computes `1 / x`. Results in an unsatisfiable instance if `x = 0`. diff --git a/src/gadgets/arithmetic_extension.rs b/src/gadgets/arithmetic_extension.rs index b7db20b9..03395211 100644 --- a/src/gadgets/arithmetic_extension.rs +++ b/src/gadgets/arithmetic_extension.rs @@ -551,19 +551,8 @@ impl, const D: usize> CircuitBuilder { x: ExtensionTarget, y: ExtensionTarget, ) -> ExtensionTarget { - let inv = self.add_virtual_extension_target(); - let one = self.one_extension(); - self.add_generator(QuotientGeneratorExtension { - numerator: one, - denominator: y, - quotient: inv, - }); - - // Enforce that x times its purported inverse equals 1. - let (y_inv, res) = self.mul_two_extension(y, inv, x, inv); - self.assert_equal_extension(y_inv, one); - - res + let zero = self.zero_extension(); + self.div_add_extension(x, y, zero) } /// Computes ` x / y + z`. @@ -590,42 +579,10 @@ impl, const D: usize> CircuitBuilder { res } - /// Computes `q = x / y` by witnessing `q` and requiring that `q * y = x`. This can be unsafe in - /// some cases, as it allows `0 / 0 = `. - pub fn div_unsafe_extension( - &mut self, - x: ExtensionTarget, - y: ExtensionTarget, - ) -> ExtensionTarget { - let quotient = self.add_virtual_extension_target(); - self.add_generator(QuotientGeneratorExtension { - numerator: x, - denominator: y, - quotient, - }); - - // Enforce that q y = x. - let q_y = self.mul_extension(quotient, y); - self.assert_equal_extension(q_y, x); - - quotient - } - /// Computes `1 / x`. Results in an unsatisfiable instance if `x = 0`. pub fn inverse_extension(&mut self, x: ExtensionTarget) -> ExtensionTarget { - let inv = self.add_virtual_extension_target(); let one = self.one_extension(); - self.add_generator(QuotientGeneratorExtension { - numerator: one, - denominator: x, - quotient: inv, - }); - - // Enforce that x times its purported inverse equals 1. - let x_inv = self.mul_extension(x, inv); - self.assert_equal_extension(x_inv, one); - - inv + self.div_extension(one, x) } } @@ -759,7 +716,7 @@ mod tests { let yt = builder.constant_extension(y); let zt = builder.constant_extension(z); let comp_zt = builder.div_extension(xt, yt); - let comp_zt_unsafe = builder.div_unsafe_extension(xt, yt); + let comp_zt_unsafe = builder.div_extension(xt, yt); builder.assert_equal_extension(zt, comp_zt); builder.assert_equal_extension(zt, comp_zt_unsafe); diff --git a/src/plonk/plonk_common.rs b/src/plonk/plonk_common.rs index 05812688..890a67c5 100644 --- a/src/plonk/plonk_common.rs +++ b/src/plonk/plonk_common.rs @@ -1,7 +1,5 @@ use std::borrow::Borrow; -use num::Integer; - use crate::field::extension_field::target::ExtensionTarget; use crate::field::extension_field::Extendable; use crate::field::field_types::Field; @@ -148,7 +146,7 @@ pub(crate) fn eval_l_1_recursively, const D: usize>( one, neg_one, ); - builder.div_unsafe_extension(eval_zero_poly, denominator) + builder.div_extension(eval_zero_poly, denominator) } /// For each alpha in alphas, compute a reduction of the given terms using powers of alpha.