Respond to review.

storage-proofs/core/src/gadgets/insertion.rs

@@ -21,22 +21,21 @@ pub fn insert<E: Engine, CS: ConstraintSystem<E>>(
     let size = elements.len() + 1;
     assert_eq!(1 << bits.len(), size);
 
-    match size {
-        // For the sizes we know we need, we can take advantage of redundancy in the candidate selection at each position.
-        // This allows us to accomplish insertion with fewer constraints, if we hand-optimize.
-        s if (s == 4) => return insert_4(cs, element, bits, elements),
-        s if (s == 8) => return insert_8(cs, element, bits, elements),
-
-        // We don't need a special case for size 2 because the general algorithm
-        // collapses to `conditionally_reverse` when size = 2.
-
-        // If no special cases have been hand-coded, use the general algorithm.
-        // This costs size * (size - 1) constraints.
-
-        // Future work: In theory, we could compile arbitrary lookup tables to minimize constraints and avoid
-        // the most general case except when actually required — which it never is for simple insertion.
-        _ => (),
-    }
+    // For the sizes we know we need, we can take advantage of redundancy in the candidate selection at each position.
+    // This allows us to accomplish insertion with fewer constraints, if we hand-optimize.
+    // We don't need a special case for size 2 because the general algorithm
+    // collapses to `conditionally_reverse` when size = 2.
+    //
+    // If no special cases have been hand-coded, use the general algorithm.
+    // This costs size * (size - 1) constraints.
+    //
+    // Future work: In theory, we could compile arbitrary lookup tables to minimize constraints and avoid
+    // the most general case except when actually required — which it never is for simple insertion.
+    if size == 4 {
+        return insert_4(cs, element, bits, elements);
+    } else if size == 8 {
+        return insert_8(cs, element, bits, elements);
+    };
 
     // Running example choices, represent inserting x into [1, 2, 3].
 
@@ -111,19 +110,17 @@ pub fn insert_4<E: Engine, CS: ConstraintSystem<E>>(
     let (b0, b1) = (&bits[0], &bits[1]);
     let (a, b, c, d) = (&element, &elements[0], &elements[1], &elements[2]);
 
-    /// Define constrain macro to allow legible definition of positional constraints.
+    /// Define witness macro to allow legible definition of positional constraints.
     /// See example expansions in comment to first usages below.
     macro_rules! witness {
         ( $var:ident <== if $cond:ident { $a:expr } else { $b:expr }) => {
             let $var = pick(cs.namespace(|| stringify!($var)), $cond, $a, $b)?;
         };
-
-        // Handle conditions which are references.
-        ( $var:ident <== if &$cond:ident { $a:expr } else { $b:expr }) => {
-            let $var = pick(cs.namespace(|| stringify!($var)), &$cond, $a, $b)?;
-        };
     }
 
+    // Witness naming convention:
+    // `p0_x0` means "Output position 0 when b0 is unknown (x) and b1 is 0."
+
     // Declaration:
     witness!(p0_x0 <== if b0 { b } else { a });
     witness!(p0 <== if b1 { b } else { &p0_x0 });
@@ -204,14 +201,14 @@ pub fn insert_8<E: Engine, CS: ConstraintSystem<E>>(
         _ => panic!("bits must be allocated and unnegated"),
     };
 
-    /// Define constrain macro to allow legible definition of positional constraints.
+    /// Define witness macro to allow legible definition of positional constraints.
     /// See example expansions in comment to first usages below.
     macro_rules! witness {
         ( $var:ident <== if $cond:ident { $a:expr } else { $b:expr }) => {
             let $var = pick(cs.namespace(|| stringify!($var)), $cond, $a, $b)?;
         };
 
-        // Handle conditions which are references.
+        // Match condition terms which are explict syntactic references.
         ( $var:ident <== if &$cond:ident { $a:expr } else { $b:expr }) => {
             let $var = pick(cs.namespace(|| stringify!($var)), &$cond, $a, $b)?;
         };
@@ -308,11 +305,8 @@ where
         }
     })?;
 
-    // Enforce (b - a) * cond = (b - c)
-    // ((b * cond) - (a * cond)) - b + c = 0
-    // c = (a * cond) + b - (b * cond)
-    // c = (cond) * a + (1 - cond) * b
-    // i.e. c = if cond { a } else { b }
+    // Constrain (b - a) * condition = (b - c), ensuring c = a iff
+    // condition is true, otherwise c = b.
     cs.enforce(
         || "pick",
         |lc| lc + b.get_variable() - a.get_variable(),
@@ -450,8 +444,8 @@ mod tests {
 
                 // One selection for each element of the result.
                 let expected_constraints = match size {
-                    s if s == 8 => 22, // unoptimized, would be 56
-                    s if s == 4 => 8,  // unoptimized, would be 12
+                    8 => 22, // unoptimized, would be 56
+                    4 => 8,  // unoptimized, would be 12
                     _ => size * (size - 1),
                 };