From 6c598ddcfd66658289a17e3d781568d36b997dc5 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 07:08:36 -0700
Subject: [PATCH 01/17] very initial attempt

---
 src/gadgets/insert.rs | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 64cf7299..8cb4614d 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -4,6 +4,16 @@ use crate::field::extension_field::Extendable;
 use crate::target::Target;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
+    /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise.
+    pub fn is_equal(
+        &mut self,
+        x: Target,
+        y: Target,
+    ) -> Target {
+
+    }
+
+
     /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the
     /// left, inserting at 0 and then rotating to the right.
     /// Note: `index` is not range-checked.
@@ -13,9 +23,24 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         element: ExtensionTarget<D>,
         v: Vec<ExtensionTarget<D>>,
     ) -> Vec<ExtensionTarget<D>> {
-        let mut v = self.rotate_left(index, &v);
-        v.insert(0, element);
-        self.rotate_right(index, &v)
+        let mut already_inserted = self.zero();
+        let mut new_list = Vec::new();
+
+        let mut cur_index = self.zero();
+        for i in 0..v.len() {
+            cur_index = self.add(cur_index, self.one());
+            let insert_here = self.is_equal(cur_index, index);
+
+            let mut new_item = self.zero();
+            new_item = self.add(new_item, self.mul(insert_here, element));
+            new_item = self.add(new_item, self.mul(already_inserted, v[i-1]));
+            already_inserted = self.add(already_inserted, insert_here);
+            new_item = self.add(new_item, self.mul(self.sub(self.one(), already_inserted), v[i]));
+
+            new_list.push(new_item);
+        }
+
+        new_list
     }
 }
 #[cfg(test)]

From af9a2c055c9844a8004603c22794cdbeeed5a3c8 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 14:02:20 -0700
Subject: [PATCH 02/17] some fixes

---
 src/gadgets/insert.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 8cb4614d..d1e0331d 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -35,7 +35,9 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
             new_item = self.add(new_item, self.mul(insert_here, element));
             new_item = self.add(new_item, self.mul(already_inserted, v[i-1]));
             already_inserted = self.add(already_inserted, insert_here);
-            new_item = self.add(new_item, self.mul(self.sub(self.one(), already_inserted), v[i]));
+
+            let not_already_inserted = self.sub(self.one(), already_inserted);
+            new_item = self.mul_add(not_already_inserted, v[i], new_item);
 
             new_list.push(new_item);
         }

From 0c9d675eccc370df0df819c6637c6a73c6c05075 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 14:20:47 -0700
Subject: [PATCH 03/17] fixes

---
 src/field/extension_field/target.rs | 167 ++++++++++++++++++++++++++++
 src/gadgets/insert.rs               |  26 ++---
 2 files changed, 180 insertions(+), 13 deletions(-)

diff --git a/src/field/extension_field/target.rs b/src/field/extension_field/target.rs
index 9d60847e..ac881d8f 100644
--- a/src/field/extension_field/target.rs
+++ b/src/field/extension_field/target.rs
@@ -109,6 +109,173 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         self.constant_ext_algebra(ExtensionAlgebra::ZERO)
     }
 
+    pub fn add_extension(
+        &mut self,
+        mut a: ExtensionTarget<D>,
+        b: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        for i in 0..D {
+            a.0[i] = self.add(a.0[i], b.0[i]);
+        }
+        a
+    }
+
+    pub fn add_ext_algebra(
+        &mut self,
+        mut a: ExtensionAlgebraTarget<D>,
+        b: ExtensionAlgebraTarget<D>,
+    ) -> ExtensionAlgebraTarget<D> {
+        for i in 0..D {
+            a.0[i] = self.add_extension(a.0[i], b.0[i]);
+        }
+        a
+    }
+
+    pub fn add_many_extension(&mut self, terms: &[ExtensionTarget<D>]) -> ExtensionTarget<D> {
+        let mut sum = self.zero_extension();
+        for term in terms {
+            sum = self.add_extension(sum, *term);
+        }
+        sum
+    }
+
+    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
+    pub fn sub_extension(
+        &mut self,
+        mut a: ExtensionTarget<D>,
+        b: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        for i in 0..D {
+            a.0[i] = self.sub(a.0[i], b.0[i]);
+        }
+        a
+    }
+
+    pub fn sub_ext_algebra(
+        &mut self,
+        mut a: ExtensionAlgebraTarget<D>,
+        b: ExtensionAlgebraTarget<D>,
+    ) -> ExtensionAlgebraTarget<D> {
+        for i in 0..D {
+            a.0[i] = self.sub_extension(a.0[i], b.0[i]);
+        }
+        a
+    }
+
+    pub fn mul_extension_with_const(
+        &mut self,
+        const_0: F,
+        multiplicand_0: ExtensionTarget<D>,
+        multiplicand_1: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        let gate = self.add_gate(MulExtensionGate::new(), vec![const_0]);
+
+        let wire_multiplicand_0 =
+            ExtensionTarget::from_range(gate, MulExtensionGate::<D>::wires_multiplicand_0());
+        let wire_multiplicand_1 =
+            ExtensionTarget::from_range(gate, MulExtensionGate::<D>::wires_multiplicand_1());
+        let wire_output = ExtensionTarget::from_range(gate, MulExtensionGate::<D>::wires_output());
+
+        self.route_extension(multiplicand_0, wire_multiplicand_0);
+        self.route_extension(multiplicand_1, wire_multiplicand_1);
+        wire_output
+    }
+
+    pub fn mul_extension(
+        &mut self,
+        multiplicand_0: ExtensionTarget<D>,
+        multiplicand_1: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        self.mul_extension_with_const(F::ONE, multiplicand_0, multiplicand_1)
+    }
+
+    pub fn mul_ext_algebra(
+        &mut self,
+        a: ExtensionAlgebraTarget<D>,
+        b: ExtensionAlgebraTarget<D>,
+    ) -> ExtensionAlgebraTarget<D> {
+        let mut res = [self.zero_extension(); D];
+        let w = self.constant(F::Extension::W);
+        for i in 0..D {
+            for j in 0..D {
+                let ai_bi = self.mul_extension(a.0[i], b.0[j]);
+                res[(i + j) % D] = if i + j < D {
+                    self.add_extension(ai_bi, res[(i + j) % D])
+                } else {
+                    let w_ai_bi = self.scalar_mul_ext(w, ai_bi);
+                    self.add_extension(w_ai_bi, res[(i + j) % D])
+                }
+            }
+        }
+        ExtensionAlgebraTarget(res)
+    }
+
+    pub fn mul_many_extension(&mut self, terms: &[ExtensionTarget<D>]) -> ExtensionTarget<D> {
+        let mut product = self.one_extension();
+        for term in terms {
+            product = self.mul_extension(product, *term);
+        }
+        product
+    }
+
+    /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no
+    /// performance benefit over separate muls and adds.
+    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
+    pub fn mul_add_extension(
+        &mut self,
+        a: ExtensionTarget<D>,
+        b: ExtensionTarget<D>,
+        c: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        let product = self.mul_extension(a, b);
+        self.add_extension(product, c)
+    }
+
+    /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no
+    /// performance benefit over separate muls and subs.
+    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
+    pub fn scalar_mul_add_extension(
+        &mut self,
+        a: Target,
+        b: ExtensionTarget<D>,
+        c: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        let product = self.scalar_mul_ext(a, b);
+        self.add_extension(product, c)
+    }
+
+    /// Like `mul_sub`, but for `ExtensionTarget`s. Note that, unlike `mul_sub`, this has no
+    /// performance benefit over separate muls and subs.
+    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
+    pub fn scalar_mul_sub_extension(
+        &mut self,
+        a: Target,
+        b: ExtensionTarget<D>,
+        c: ExtensionTarget<D>,
+    ) -> ExtensionTarget<D> {
+        let product = self.scalar_mul_ext(a, b);
+        self.sub_extension(product, c)
+    }
+
+    /// Returns `a * b`, where `b` is in the extension field and `a` is in the base field.
+    pub fn scalar_mul_ext(&mut self, a: Target, b: ExtensionTarget<D>) -> ExtensionTarget<D> {
+        let a_ext = self.convert_to_ext(a);
+        self.mul_extension(a_ext, b)
+    }
+
+    /// Returns `a * b`, where `b` is in the extension of the extension field, and `a` is in the
+    /// extension field.
+    pub fn scalar_mul_ext_algebra(
+        &mut self,
+        a: ExtensionTarget<D>,
+        mut b: ExtensionAlgebraTarget<D>,
+    ) -> ExtensionAlgebraTarget<D> {
+        for i in 0..D {
+            b.0[i] = self.mul_extension(a, b.0[i]);
+        }
+        b
+    }
+
     pub fn convert_to_ext(&mut self, t: Target) -> ExtensionTarget<D> {
         let zero = self.zero();
         let mut arr = [zero; D];
diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index d1e0331d..aa310568 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -5,15 +5,10 @@ use crate::target::Target;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise.
-    pub fn is_equal(
-        &mut self,
-        x: Target,
-        y: Target,
-    ) -> Target {
-
+    pub fn is_equal(&mut self, _x: Target, _y: Target) -> Target {
+        todo!()
     }
 
-
     /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the
     /// left, inserting at 0 and then rotating to the right.
     /// Note: `index` is not range-checked.
@@ -28,16 +23,21 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
 
         let mut cur_index = self.zero();
         for i in 0..v.len() {
-            cur_index = self.add(cur_index, self.one());
+            let one = self.one();
+            
+            cur_index = self.add(cur_index, one);
             let insert_here = self.is_equal(cur_index, index);
 
-            let mut new_item = self.zero();
-            new_item = self.add(new_item, self.mul(insert_here, element));
-            new_item = self.add(new_item, self.mul(already_inserted, v[i-1]));
+            let mut new_item = self.zero_extension();
+            new_item = self.scalar_mul_add_extension(insert_here, element, new_item);
+            if i > 0 {
+                new_item =
+                    self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item);
+            }
             already_inserted = self.add(already_inserted, insert_here);
 
-            let not_already_inserted = self.sub(self.one(), already_inserted);
-            new_item = self.mul_add(not_already_inserted, v[i], new_item);
+            let not_already_inserted = self.sub(one, already_inserted);
+            new_item = self.scalar_mul_add_extension(not_already_inserted, v[i], new_item);
 
             new_list.push(new_item);
         }

From 6cc06b408fab2b96d3faab7e9b2f082d692e1f92 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 14:23:05 -0700
Subject: [PATCH 04/17] small change

---
 src/gadgets/insert.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index aa310568..b7f7e331 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -21,11 +21,10 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         let mut already_inserted = self.zero();
         let mut new_list = Vec::new();
 
-        let mut cur_index = self.zero();
         for i in 0..v.len() {
             let one = self.one();
             
-            cur_index = self.add(cur_index, one);
+            let cur_index = self.constant(F::from_canonical_usize(i));
             let insert_here = self.is_equal(cur_index, index);
 
             let mut new_item = self.zero_extension();

From 647568fc7ad0c35259ee9374c30e4db1836dde6f Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 16:57:37 -0700
Subject: [PATCH 05/17] added EqualityGenerator

---
 src/generator.rs | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/src/generator.rs b/src/generator.rs
index a2b35a53..130ca0d0 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -130,3 +130,38 @@ impl<F: Field> SimpleGenerator<F> for RandomValueGenerator {
         PartialWitness::singleton_target(self.target, random_value)
     }
 }
+
+
+/// A generator for including a random value
+pub(crate) struct EqualityGenerator {
+    pub(crate) x: Target,
+    pub(crate) m: Target,
+    pub(crate) y: Target,
+}
+
+impl<F: Field> SimpleGenerator<F> for EqualityGenerator {
+    fn dependencies(&self) -> Vec<Target> {
+        vec![self.x]
+    }
+
+    fn run_once(&self, witness: &PartialWitness<F>) -> PartialWitness<F> {
+        let x_value = witness.get_target(self.x);
+
+        let y_value = if x_value == F::ZERO {
+            F::ZERO
+        } else {
+            F::ONE
+        };
+
+        let m_value = if x_value == F::ZERO {
+            F::ONE
+        } else {
+            x_value.inverse()
+        };
+
+        let mut witness = PartialWitness::new();
+        witness.set_target(self.m, m_value);
+        witness.set_target(self.y, y_value);
+        witness
+    }
+}

From 77d942f0e9df99c0cbd4d5d22f80c6f2ce5a356d Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 16:58:06 -0700
Subject: [PATCH 06/17] cleanup

---
 src/circuit_builder.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/circuit_builder.rs b/src/circuit_builder.rs
index 7ce4adb3..d9ff162e 100644
--- a/src/circuit_builder.rs
+++ b/src/circuit_builder.rs
@@ -310,16 +310,16 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
                         input: w,
                     }),
                 });
-                self.add_generator(CopyGenerator {
-                    src: Target::Wire(Wire {
+                self.generate_copy(
+                    Target::Wire(Wire {
                         gate: gate_1,
                         input: w,
                     }),
-                    dst: Target::Wire(Wire {
+                    Target::Wire(Wire {
                         gate: gate_2,
                         input: w,
                     }),
-                });
+                );
             }
         }
 

From 80758da0f4cbe62ddb2ad459601bde15d2a9e1fd Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 16:58:13 -0700
Subject: [PATCH 07/17] removed duplicate functions

---
 src/field/extension_field/target.rs | 167 ----------------------------
 1 file changed, 167 deletions(-)

diff --git a/src/field/extension_field/target.rs b/src/field/extension_field/target.rs
index ac881d8f..9d60847e 100644
--- a/src/field/extension_field/target.rs
+++ b/src/field/extension_field/target.rs
@@ -109,173 +109,6 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         self.constant_ext_algebra(ExtensionAlgebra::ZERO)
     }
 
-    pub fn add_extension(
-        &mut self,
-        mut a: ExtensionTarget<D>,
-        b: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        for i in 0..D {
-            a.0[i] = self.add(a.0[i], b.0[i]);
-        }
-        a
-    }
-
-    pub fn add_ext_algebra(
-        &mut self,
-        mut a: ExtensionAlgebraTarget<D>,
-        b: ExtensionAlgebraTarget<D>,
-    ) -> ExtensionAlgebraTarget<D> {
-        for i in 0..D {
-            a.0[i] = self.add_extension(a.0[i], b.0[i]);
-        }
-        a
-    }
-
-    pub fn add_many_extension(&mut self, terms: &[ExtensionTarget<D>]) -> ExtensionTarget<D> {
-        let mut sum = self.zero_extension();
-        for term in terms {
-            sum = self.add_extension(sum, *term);
-        }
-        sum
-    }
-
-    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
-    pub fn sub_extension(
-        &mut self,
-        mut a: ExtensionTarget<D>,
-        b: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        for i in 0..D {
-            a.0[i] = self.sub(a.0[i], b.0[i]);
-        }
-        a
-    }
-
-    pub fn sub_ext_algebra(
-        &mut self,
-        mut a: ExtensionAlgebraTarget<D>,
-        b: ExtensionAlgebraTarget<D>,
-    ) -> ExtensionAlgebraTarget<D> {
-        for i in 0..D {
-            a.0[i] = self.sub_extension(a.0[i], b.0[i]);
-        }
-        a
-    }
-
-    pub fn mul_extension_with_const(
-        &mut self,
-        const_0: F,
-        multiplicand_0: ExtensionTarget<D>,
-        multiplicand_1: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        let gate = self.add_gate(MulExtensionGate::new(), vec![const_0]);
-
-        let wire_multiplicand_0 =
-            ExtensionTarget::from_range(gate, MulExtensionGate::<D>::wires_multiplicand_0());
-        let wire_multiplicand_1 =
-            ExtensionTarget::from_range(gate, MulExtensionGate::<D>::wires_multiplicand_1());
-        let wire_output = ExtensionTarget::from_range(gate, MulExtensionGate::<D>::wires_output());
-
-        self.route_extension(multiplicand_0, wire_multiplicand_0);
-        self.route_extension(multiplicand_1, wire_multiplicand_1);
-        wire_output
-    }
-
-    pub fn mul_extension(
-        &mut self,
-        multiplicand_0: ExtensionTarget<D>,
-        multiplicand_1: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        self.mul_extension_with_const(F::ONE, multiplicand_0, multiplicand_1)
-    }
-
-    pub fn mul_ext_algebra(
-        &mut self,
-        a: ExtensionAlgebraTarget<D>,
-        b: ExtensionAlgebraTarget<D>,
-    ) -> ExtensionAlgebraTarget<D> {
-        let mut res = [self.zero_extension(); D];
-        let w = self.constant(F::Extension::W);
-        for i in 0..D {
-            for j in 0..D {
-                let ai_bi = self.mul_extension(a.0[i], b.0[j]);
-                res[(i + j) % D] = if i + j < D {
-                    self.add_extension(ai_bi, res[(i + j) % D])
-                } else {
-                    let w_ai_bi = self.scalar_mul_ext(w, ai_bi);
-                    self.add_extension(w_ai_bi, res[(i + j) % D])
-                }
-            }
-        }
-        ExtensionAlgebraTarget(res)
-    }
-
-    pub fn mul_many_extension(&mut self, terms: &[ExtensionTarget<D>]) -> ExtensionTarget<D> {
-        let mut product = self.one_extension();
-        for term in terms {
-            product = self.mul_extension(product, *term);
-        }
-        product
-    }
-
-    /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no
-    /// performance benefit over separate muls and adds.
-    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
-    pub fn mul_add_extension(
-        &mut self,
-        a: ExtensionTarget<D>,
-        b: ExtensionTarget<D>,
-        c: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        let product = self.mul_extension(a, b);
-        self.add_extension(product, c)
-    }
-
-    /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no
-    /// performance benefit over separate muls and subs.
-    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
-    pub fn scalar_mul_add_extension(
-        &mut self,
-        a: Target,
-        b: ExtensionTarget<D>,
-        c: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        let product = self.scalar_mul_ext(a, b);
-        self.add_extension(product, c)
-    }
-
-    /// Like `mul_sub`, but for `ExtensionTarget`s. Note that, unlike `mul_sub`, this has no
-    /// performance benefit over separate muls and subs.
-    /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend.
-    pub fn scalar_mul_sub_extension(
-        &mut self,
-        a: Target,
-        b: ExtensionTarget<D>,
-        c: ExtensionTarget<D>,
-    ) -> ExtensionTarget<D> {
-        let product = self.scalar_mul_ext(a, b);
-        self.sub_extension(product, c)
-    }
-
-    /// Returns `a * b`, where `b` is in the extension field and `a` is in the base field.
-    pub fn scalar_mul_ext(&mut self, a: Target, b: ExtensionTarget<D>) -> ExtensionTarget<D> {
-        let a_ext = self.convert_to_ext(a);
-        self.mul_extension(a_ext, b)
-    }
-
-    /// Returns `a * b`, where `b` is in the extension of the extension field, and `a` is in the
-    /// extension field.
-    pub fn scalar_mul_ext_algebra(
-        &mut self,
-        a: ExtensionTarget<D>,
-        mut b: ExtensionAlgebraTarget<D>,
-    ) -> ExtensionAlgebraTarget<D> {
-        for i in 0..D {
-            b.0[i] = self.mul_extension(a, b.0[i]);
-        }
-        b
-    }
-
     pub fn convert_to_ext(&mut self, t: Target) -> ExtensionTarget<D> {
         let zero = self.zero();
         let mut arr = [zero; D];

From cad7dc6904b05d53e9e9ce72aea47fb411d8b3aa Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 21:31:17 -0700
Subject: [PATCH 08/17] some progress

---
 src/gadgets/insert.rs | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index b7f7e331..4b6a15b1 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -2,11 +2,26 @@ use crate::circuit_builder::CircuitBuilder;
 use crate::field::extension_field::target::ExtensionTarget;
 use crate::field::extension_field::Extendable;
 use crate::target::Target;
+use crate::generator::EqualityGenerator;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
+    /// Evaluates to 1 if `x` equals zero, 0 otherwise.
+    pub fn is_zero(&mut self, x: Target) -> Target {
+        let m = todo!();
+        let y = todo!();
+
+        self.add_generator(EqualityGenerator {
+            x,
+            m,
+            y,
+        });
+
+        y
+    }
+
     /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise.
-    pub fn is_equal(&mut self, _x: Target, _y: Target) -> Target {
-        todo!()
+    pub fn is_equal(&mut self, x: Target, y: Target) -> Target {
+        self.is_zero(self.sub(x, y))
     }
 
     /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the

From 3959cec180bb95477117feef3b31872199e4f9a0 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 21:59:10 -0700
Subject: [PATCH 09/17] mutable borrow fix

---
 src/gadgets/insert.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 4b6a15b1..c14dcf8d 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -21,7 +21,8 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
 
     /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise.
     pub fn is_equal(&mut self, x: Target, y: Target) -> Target {
-        self.is_zero(self.sub(x, y))
+        let difference = self.sub(x, y);
+        self.is_zero(difference)
     }
 
     /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the

From 8de59c2a84d404c7a39bfed2d05719820f285212 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Wed, 30 Jun 2021 21:59:18 -0700
Subject: [PATCH 10/17] cargo fmt

---
 src/field/fft.rs             | 76 +++++++++++++++++++++---------------
 src/field/field_testing.rs   |  2 +-
 src/gadgets/insert.rs        | 13 ++----
 src/generator.rs             |  7 +---
 src/polynomial/polynomial.rs |  5 +--
 src/util/mod.rs              |  4 +-
 6 files changed, 56 insertions(+), 51 deletions(-)

diff --git a/src/field/fft.rs b/src/field/fft.rs
index af5c05a7..fa65a5ea 100644
--- a/src/field/fft.rs
+++ b/src/field/fft.rs
@@ -6,7 +6,10 @@ use crate::util::{log2_strict, reverse_index_bits};
 
 // TODO: Should really do some "dynamic" dispatch to handle the
 // different FFT algos rather than C-style enum dispatch.
-enum FftStrategy { Classic, Unrolled }
+enum FftStrategy {
+    Classic,
+    Unrolled,
+}
 
 const FFT_STRATEGY: FftStrategy = FftStrategy::Classic;
 
@@ -33,7 +36,6 @@ fn fft_classic_root_table<F: Field>(n: usize) -> FftRootTable<F> {
     root_table
 }
 
-
 fn fft_unrolled_root_table<F: Field>(n: usize) -> FftRootTable<F> {
     // Precompute a table of the roots of unity used in the main
     // loops.
@@ -67,18 +69,20 @@ fn fft_unrolled_root_table<F: Field>(n: usize) -> FftRootTable<F> {
 fn fft_dispatch<F: Field>(
     input: Vec<F>,
     zero_factor: Option<usize>,
-    root_table: Option<FftRootTable<F>>
+    root_table: Option<FftRootTable<F>>,
 ) -> Vec<F> {
     let n = input.len();
     match FFT_STRATEGY {
-        FftStrategy::Classic
-            => fft_classic(input,
-                           zero_factor.unwrap_or(0),
-                           root_table.unwrap_or_else(|| fft_classic_root_table(n))),
-        FftStrategy::Unrolled
-            => fft_unrolled(input,
-                            zero_factor.unwrap_or(0),
-                            root_table.unwrap_or_else(|| fft_unrolled_root_table(n)))
+        FftStrategy::Classic => fft_classic(
+            input,
+            zero_factor.unwrap_or(0),
+            root_table.unwrap_or_else(|| fft_classic_root_table(n)),
+        ),
+        FftStrategy::Unrolled => fft_unrolled(
+            input,
+            zero_factor.unwrap_or(0),
+            root_table.unwrap_or_else(|| fft_unrolled_root_table(n)),
+        ),
     }
 }
 
@@ -91,10 +95,12 @@ pub fn fft<F: Field>(poly: PolynomialCoeffs<F>) -> PolynomialValues<F> {
 pub fn fft_with_options<F: Field>(
     poly: PolynomialCoeffs<F>,
     zero_factor: Option<usize>,
-    root_table: Option<FftRootTable<F>>
+    root_table: Option<FftRootTable<F>>,
 ) -> PolynomialValues<F> {
     let PolynomialCoeffs { coeffs } = poly;
-    PolynomialValues { values: fft_dispatch(coeffs, zero_factor, root_table) }
+    PolynomialValues {
+        values: fft_dispatch(coeffs, zero_factor, root_table),
+    }
 }
 
 #[inline]
@@ -105,7 +111,7 @@ pub fn ifft<F: Field>(poly: PolynomialValues<F>) -> PolynomialCoeffs<F> {
 pub fn ifft_with_options<F: Field>(
     poly: PolynomialValues<F>,
     zero_factor: Option<usize>,
-    root_table: Option<FftRootTable<F>>
+    root_table: Option<FftRootTable<F>>,
 ) -> PolynomialCoeffs<F> {
     let n = poly.len();
     let lg_n = log2_strict(n);
@@ -136,7 +142,7 @@ pub fn ifft_with_options<F: Field>(
 pub(crate) fn fft_classic<F: Field>(
     input: Vec<F>,
     r: usize,
-    root_table: FftRootTable<F>
+    root_table: FftRootTable<F>,
 ) -> Vec<F> {
     let mut values = reverse_index_bits(input);
 
@@ -144,7 +150,11 @@ pub(crate) fn fft_classic<F: Field>(
     let lg_n = log2_strict(n);
 
     if root_table.len() != lg_n {
-        panic!("Expected root table of length {}, but it was {}.", lg_n, root_table.len());
+        panic!(
+            "Expected root table of length {}, but it was {}.",
+            lg_n,
+            root_table.len()
+        );
     }
 
     // After reverse_index_bits, the only non-zero elements of values
@@ -154,7 +164,8 @@ pub(crate) fn fft_classic<F: Field>(
     // element i*2^r with the value at i*2^r.  This corresponds to the
     // first r rounds of the FFT when there are 2^r zeros at the end
     // of the original input.
-    if r > 0 { // if r == 0 then this loop is a noop.
+    if r > 0 {
+        // if r == 0 then this loop is a noop.
         let mask = !((1 << r) - 1);
         for i in 0..n {
             values[i] = values[i & mask];
@@ -162,7 +173,7 @@ pub(crate) fn fft_classic<F: Field>(
     }
 
     let mut m = 1 << (r + 1);
-    for lg_m in (r+1)..=lg_n {
+    for lg_m in (r + 1)..=lg_n {
         let half_m = m / 2;
         for k in (0..n).step_by(m) {
             for j in 0..half_m {
@@ -185,11 +196,7 @@ pub(crate) fn fft_classic<F: Field>(
 /// The parameter r signifies that the first 1/2^r of the entries of
 /// input may be non-zero, but the last 1 - 1/2^r entries are
 /// definitely zero.
-fn fft_unrolled<F: Field>(
-    input: Vec<F>,
-    r_orig: usize,
-    root_table: FftRootTable<F>
-) -> Vec<F> {
+fn fft_unrolled<F: Field>(input: Vec<F>, r_orig: usize, root_table: FftRootTable<F>) -> Vec<F> {
     let n = input.len();
     let lg_n = log2_strict(input.len());
 
@@ -197,7 +204,7 @@ fn fft_unrolled<F: Field>(
 
     // FFT of a constant polynomial (including zero) is itself.
     if n < 2 {
-        return values
+        return values;
     }
 
     // The 'm' corresponds to the specialisation from the 'm' in the
@@ -206,7 +213,8 @@ fn fft_unrolled<F: Field>(
     // (See comment in fft_classic near same code.)
     let mut r = r_orig;
     let mut m = 1 << r;
-    if r > 0 { // if r == 0 then this loop is a noop.
+    if r > 0 {
+        // if r == 0 then this loop is a noop.
         let mask = !((1 << r) - 1);
         for i in 0..n {
             values[i] = values[i & mask];
@@ -225,11 +233,15 @@ fn fft_unrolled<F: Field>(
     }
 
     if n == 2 {
-        return values
+        return values;
     }
 
     if root_table.len() != (lg_n - 1) {
-        panic!("Expected root table of length {}, but it was {}.", lg_n, root_table.len());
+        panic!(
+            "Expected root table of length {}, but it was {}.",
+            lg_n,
+            root_table.len()
+        );
     }
 
     // m = 2
@@ -253,7 +265,7 @@ fn fft_unrolled<F: Field>(
 
     // m >= 4
     for lg_m in r..lg_n {
-        for k in (0..n).step_by(2*m) {
+        for k in (0..n).step_by(2 * m) {
             // Unrolled the commented loop by groups of 4 and
             // rearranged the lines. Improves runtime by about
             // 10%.
@@ -294,11 +306,10 @@ fn fft_unrolled<F: Field>(
     values
 }
 
-
 #[cfg(test)]
 mod tests {
     use crate::field::crandall_field::CrandallField;
-    use crate::field::fft::{fft, ifft, fft_with_options};
+    use crate::field::fft::{fft, fft_with_options, ifft};
     use crate::field::field::Field;
     use crate::polynomial::polynomial::{PolynomialCoeffs, PolynomialValues};
     use crate::util::{log2_ceil, log2_strict};
@@ -328,7 +339,10 @@ mod tests {
         for r in 0..4 {
             // expand ceofficients by factor 2^r by filling with zeros
             let zero_tail = coefficients.clone().lde(r);
-            assert_eq!(fft(zero_tail.clone()), fft_with_options(zero_tail, Some(r), None));
+            assert_eq!(
+                fft(zero_tail.clone()),
+                fft_with_options(zero_tail, Some(r), None)
+            );
         }
     }
 
diff --git a/src/field/field_testing.rs b/src/field/field_testing.rs
index 7190684f..1f5bff6f 100644
--- a/src/field/field_testing.rs
+++ b/src/field/field_testing.rs
@@ -323,7 +323,7 @@ macro_rules! test_arithmetic {
 
                 let v = <F as Field>::PrimeField::TWO_ADICITY;
 
-                for e in [0, 1, 2, 3, 4, v - 2, v - 1, v, v + 1, v + 2, 123*v] {
+                for e in [0, 1, 2, 3, 4, v - 2, v - 1, v, v + 1, v + 2, 123 * v] {
                     let x = F::TWO.exp(e as u64).inverse();
                     let y = F::inverse_2exp(e);
                     assert_eq!(x, y);
diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index c14dcf8d..32aa250e 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -1,8 +1,8 @@
 use crate::circuit_builder::CircuitBuilder;
 use crate::field::extension_field::target::ExtensionTarget;
 use crate::field::extension_field::Extendable;
-use crate::target::Target;
 use crate::generator::EqualityGenerator;
+use crate::target::Target;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     /// Evaluates to 1 if `x` equals zero, 0 otherwise.
@@ -10,11 +10,7 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         let m = todo!();
         let y = todo!();
 
-        self.add_generator(EqualityGenerator {
-            x,
-            m,
-            y,
-        });
+        self.add_generator(EqualityGenerator { x, m, y });
 
         y
     }
@@ -39,15 +35,14 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
 
         for i in 0..v.len() {
             let one = self.one();
-            
+
             let cur_index = self.constant(F::from_canonical_usize(i));
             let insert_here = self.is_equal(cur_index, index);
 
             let mut new_item = self.zero_extension();
             new_item = self.scalar_mul_add_extension(insert_here, element, new_item);
             if i > 0 {
-                new_item =
-                    self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item);
+                new_item = self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item);
             }
             already_inserted = self.add(already_inserted, insert_here);
 
diff --git a/src/generator.rs b/src/generator.rs
index 130ca0d0..64f21604 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -131,7 +131,6 @@ impl<F: Field> SimpleGenerator<F> for RandomValueGenerator {
     }
 }
 
-
 /// A generator for including a random value
 pub(crate) struct EqualityGenerator {
     pub(crate) x: Target,
@@ -147,11 +146,7 @@ impl<F: Field> SimpleGenerator<F> for EqualityGenerator {
     fn run_once(&self, witness: &PartialWitness<F>) -> PartialWitness<F> {
         let x_value = witness.get_target(self.x);
 
-        let y_value = if x_value == F::ZERO {
-            F::ZERO
-        } else {
-            F::ONE
-        };
+        let y_value = if x_value == F::ZERO { F::ZERO } else { F::ONE };
 
         let m_value = if x_value == F::ZERO {
             F::ONE
diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs
index 5f295030..aa06d641 100644
--- a/src/polynomial/polynomial.rs
+++ b/src/polynomial/polynomial.rs
@@ -1,13 +1,12 @@
-use std::time::Instant;
-
 use std::cmp::max;
 use std::iter::Sum;
 use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign};
+use std::time::Instant;
 
 use anyhow::{ensure, Result};
 
 use crate::field::extension_field::Extendable;
-use crate::field::fft::{fft, ifft, fft_with_options};
+use crate::field::fft::{fft, fft_with_options, ifft};
 use crate::field::field::Field;
 use crate::util::log2_strict;
 
diff --git a/src/util/mod.rs b/src/util/mod.rs
index 8fd60d53..ee3b8440 100644
--- a/src/util/mod.rs
+++ b/src/util/mod.rs
@@ -77,7 +77,9 @@ pub(crate) fn reverse_bits(n: usize, num_bits: usize) -> usize {
     // to plain '>>' is to accommodate the case n == num_bits == 0,
     // which would become `0 >> 64`. Rust thinks that any shift of 64
     // bits causes overflow, even when the argument is zero.
-    n.reverse_bits().overflowing_shr(usize::BITS - num_bits as u32).0
+    n.reverse_bits()
+        .overflowing_shr(usize::BITS - num_bits as u32)
+        .0
 }
 
 #[cfg(test)]

From f4ca0df85d0c3b54741dd3a9b7d91cfd9837f59c Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 10:35:41 -0700
Subject: [PATCH 11/17] comments and renaming

---
 src/gadgets/insert.rs |  4 ++--
 src/generator.rs      | 26 +++++++++++++-------------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 32aa250e..78bf54cc 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -1,7 +1,7 @@
 use crate::circuit_builder::CircuitBuilder;
 use crate::field::extension_field::target::ExtensionTarget;
 use crate::field::extension_field::Extendable;
-use crate::generator::EqualityGenerator;
+use crate::generator::EqualsZeroGenerator;
 use crate::target::Target;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
@@ -10,7 +10,7 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         let m = todo!();
         let y = todo!();
 
-        self.add_generator(EqualityGenerator { x, m, y });
+        self.add_generator(EqualsZeroGenerator { x, m, y });
 
         y
     }
diff --git a/src/generator.rs b/src/generator.rs
index 64f21604..f54b820e 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -131,32 +131,32 @@ impl<F: Field> SimpleGenerator<F> for RandomValueGenerator {
     }
 }
 
-/// A generator for including a random value
-pub(crate) struct EqualityGenerator {
-    pub(crate) x: Target,
-    pub(crate) m: Target,
-    pub(crate) y: Target,
+/// A generator for testing if a value equals zero
+pub(crate) struct EqualsZeroGenerator {
+    pub(crate) to_test: Target,
+    pub(crate) dummy: Target,
+    pub(crate) is_zero: Target,
 }
 
-impl<F: Field> SimpleGenerator<F> for EqualityGenerator {
+impl<F: Field> SimpleGenerator<F> for EqualsZeroGenerator {
     fn dependencies(&self) -> Vec<Target> {
-        vec![self.x]
+        vec![self.to_test]
     }
 
     fn run_once(&self, witness: &PartialWitness<F>) -> PartialWitness<F> {
-        let x_value = witness.get_target(self.x);
+        let to_test_value = witness.get_target(self.to_test);
 
-        let y_value = if x_value == F::ZERO { F::ZERO } else { F::ONE };
+        let is_zero_value = if to_test_value == F::ZERO { F::ZERO } else { F::ONE };
 
-        let m_value = if x_value == F::ZERO {
+        let dummy_value = if to_test_value == F::ZERO {
             F::ONE
         } else {
-            x_value.inverse()
+            to_test_value.inverse()
         };
 
         let mut witness = PartialWitness::new();
-        witness.set_target(self.m, m_value);
-        witness.set_target(self.y, y_value);
+        witness.set_target(self.dummy, dummy_value);
+        witness.set_target(self.is_zero, is_zero_value);
         witness
     }
 }

From 702a098054f96c26e45c797aa03f4a9661c7cb0e Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 10:36:31 -0700
Subject: [PATCH 12/17] cargo fmt

---
 src/generator.rs | 6 +++++-
 src/target.rs    | 8 ++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/generator.rs b/src/generator.rs
index f54b820e..6bc12011 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -146,7 +146,11 @@ impl<F: Field> SimpleGenerator<F> for EqualsZeroGenerator {
     fn run_once(&self, witness: &PartialWitness<F>) -> PartialWitness<F> {
         let to_test_value = witness.get_target(self.to_test);
 
-        let is_zero_value = if to_test_value == F::ZERO { F::ZERO } else { F::ONE };
+        let is_zero_value = if to_test_value == F::ZERO {
+            F::ZERO
+        } else {
+            F::ONE
+        };
 
         let dummy_value = if to_test_value == F::ZERO {
             F::ONE
diff --git a/src/target.rs b/src/target.rs
index e765f7eb..52be8b5a 100644
--- a/src/target.rs
+++ b/src/target.rs
@@ -7,11 +7,15 @@ use crate::wire::Wire;
 #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]
 pub enum Target {
     Wire(Wire),
-    PublicInput { index: usize },
+    PublicInput {
+        index: usize,
+    },
     /// A target that doesn't have any inherent location in the witness (but it can be copied to
     /// another target that does). This is useful for representing intermediate values in witness
     /// generation.
-    VirtualTarget { index: usize },
+    VirtualTarget {
+        index: usize,
+    },
 }
 
 impl Target {

From 515373653d155d39120707a7f7341f7292d90838 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 10:38:11 -0700
Subject: [PATCH 13/17] fix

---
 src/gadgets/insert.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 78bf54cc..c58e80b9 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -10,7 +10,11 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         let m = todo!();
         let y = todo!();
 
-        self.add_generator(EqualsZeroGenerator { x, m, y });
+        self.add_generator(EqualsZeroGenerator {
+            to_test: x,
+            dummy: m,
+            is_zero: y
+        });
 
         y
     }

From d84b9ec8cb94aec72abbe4c29a3feaecd2c982e5 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 10:47:13 -0700
Subject: [PATCH 14/17] is_zero function

---
 src/gadgets/insert.rs | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index c58e80b9..ca7149bf 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -7,13 +7,18 @@ use crate::target::Target;
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     /// Evaluates to 1 if `x` equals zero, 0 otherwise.
     pub fn is_zero(&mut self, x: Target) -> Target {
-        let m = todo!();
-        let y = todo!();
+        let m = self.add_virtual_target();
+        let y = self.mul(x, m);
+
+        let one = self.one();
+        let diff = self.sub(one, y);
+        let prod = self.mul(diff, x);
+        self.assert_zero(prod);
 
         self.add_generator(EqualsZeroGenerator {
             to_test: x,
             dummy: m,
-            is_zero: y
+            is_zero: y,
         });
 
         y

From efe39f2d638dd5cf3bf5f5162b502d9df1468432 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 11:21:33 -0700
Subject: [PATCH 15/17] fixed naming (zero --> nonzero), and other fixes

---
 src/gadgets/insert.rs | 26 ++++++++++++++++----------
 src/generator.rs      | 12 ++----------
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index ca7149bf..64260209 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -1,33 +1,39 @@
 use crate::circuit_builder::CircuitBuilder;
 use crate::field::extension_field::target::ExtensionTarget;
 use crate::field::extension_field::Extendable;
-use crate::generator::EqualsZeroGenerator;
+use crate::generator::NonzeroTestGenerator;
 use crate::target::Target;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
-    /// Evaluates to 1 if `x` equals zero, 0 otherwise.
-    pub fn is_zero(&mut self, x: Target) -> Target {
+    /// Evaluates to 0 if `x` equals zero, 1 otherwise.
+    pub fn is_nonzero(&mut self, x: Target) -> Target {
+        // Dummy variable.
         let m = self.add_virtual_target();
+
+        // The prover sets the dummy variable to 0 if x == 0 and to 1/x otherwise.
+        self.add_generator(NonzeroTestGenerator {
+            to_test: x,
+            dummy: m,
+        });
+
+        // Evaluates to (0) * (0) = 0 if x == 0 and (x) * (1/x) = 1 otherwise.
         let y = self.mul(x, m);
 
+        // Enforce that (1 - y) * x == 0.
         let one = self.one();
         let diff = self.sub(one, y);
         let prod = self.mul(diff, x);
         self.assert_zero(prod);
 
-        self.add_generator(EqualsZeroGenerator {
-            to_test: x,
-            dummy: m,
-            is_zero: y,
-        });
-
         y
     }
 
     /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise.
     pub fn is_equal(&mut self, x: Target, y: Target) -> Target {
         let difference = self.sub(x, y);
-        self.is_zero(difference)
+        let not_equal = self.is_nonzero(difference);
+        let one = self.one();
+        self.sub(one, not_equal)
     }
 
     /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the
diff --git a/src/generator.rs b/src/generator.rs
index 6bc12011..3795fdfa 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -132,13 +132,12 @@ impl<F: Field> SimpleGenerator<F> for RandomValueGenerator {
 }
 
 /// A generator for testing if a value equals zero
-pub(crate) struct EqualsZeroGenerator {
+pub(crate) struct NonzeroTestGenerator {
     pub(crate) to_test: Target,
     pub(crate) dummy: Target,
-    pub(crate) is_zero: Target,
 }
 
-impl<F: Field> SimpleGenerator<F> for EqualsZeroGenerator {
+impl<F: Field> SimpleGenerator<F> for NonzeroTestGenerator {
     fn dependencies(&self) -> Vec<Target> {
         vec![self.to_test]
     }
@@ -146,12 +145,6 @@ impl<F: Field> SimpleGenerator<F> for EqualsZeroGenerator {
     fn run_once(&self, witness: &PartialWitness<F>) -> PartialWitness<F> {
         let to_test_value = witness.get_target(self.to_test);
 
-        let is_zero_value = if to_test_value == F::ZERO {
-            F::ZERO
-        } else {
-            F::ONE
-        };
-
         let dummy_value = if to_test_value == F::ZERO {
             F::ONE
         } else {
@@ -160,7 +153,6 @@ impl<F: Field> SimpleGenerator<F> for EqualsZeroGenerator {
 
         let mut witness = PartialWitness::new();
         witness.set_target(self.dummy, dummy_value);
-        witness.set_target(self.is_zero, is_zero_value);
         witness
     }
 }

From 39b22a6cab780adf2e99c14218584ae3b4c8defe Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 12:00:56 -0700
Subject: [PATCH 16/17] addressed nits

---
 src/gadgets/insert.rs | 5 ++++-
 src/generator.rs      | 4 +---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 64260209..7a1ee98d 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -6,11 +6,14 @@ use crate::target::Target;
 
 impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     /// Evaluates to 0 if `x` equals zero, 1 otherwise.
+    /// From section 2 of https://github.com/mir-protocol/r1cs-workshop/blob/master/workshop.pdf,
+    /// based on an idea from https://eprint.iacr.org/2012/598.pdf.
     pub fn is_nonzero(&mut self, x: Target) -> Target {
         // Dummy variable.
         let m = self.add_virtual_target();
 
-        // The prover sets the dummy variable to 0 if x == 0 and to 1/x otherwise.
+        // The prover sets this the dummy variable to 1/x if x != 0, or to an arbitrary value if
+        // x == 0.
         self.add_generator(NonzeroTestGenerator {
             to_test: x,
             dummy: m,
diff --git a/src/generator.rs b/src/generator.rs
index 3795fdfa..a47c5267 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -151,8 +151,6 @@ impl<F: Field> SimpleGenerator<F> for NonzeroTestGenerator {
             to_test_value.inverse()
         };
 
-        let mut witness = PartialWitness::new();
-        witness.set_target(self.dummy, dummy_value);
-        witness
+        PartialWitness::singleton_target(self.dummy, dummy_value)
     }
 }

From 3d53201538ca8faef39c80e15f229847e05fdd94 Mon Sep 17 00:00:00 2001
From: Nicholas Ward <npward@berkeley.edu>
Date: Thu, 1 Jul 2021 17:43:22 -0700
Subject: [PATCH 17/17] save a gate with arithmetic

---
 src/gadgets/insert.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs
index 7a1ee98d..1f69cb24 100644
--- a/src/gadgets/insert.rs
+++ b/src/gadgets/insert.rs
@@ -23,9 +23,7 @@ impl<F: Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         let y = self.mul(x, m);
 
         // Enforce that (1 - y) * x == 0.
-        let one = self.one();
-        let diff = self.sub(one, y);
-        let prod = self.mul(diff, x);
+        let prod = self.arithmetic(F::NEG_ONE, x, y, F::ONE, x);
         self.assert_zero(prod);
 
         y