Fix linked proof commitments (#342)

sunscreen/tests/linked.rs

@@ -265,7 +265,6 @@ mod linked_tests {
     }
 
     #[test]
-    #[ignore = "bug in linked input multplication"]
     fn can_compare_rationals() {
         let app = Compiler::new()
             .fhe_program(doggie)

sunscreen_compiler_common/src/graph.rs

@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::{HashSet, VecDeque};
 
 use petgraph::{
     dot::Dot,
@@ -215,14 +215,14 @@ where
         Direction::Incoming
     };
 
-    let mut ready_nodes: Vec<NodeIndex> = graph
+    let mut ready_nodes: VecDeque<NodeIndex> = graph
         .node_identifiers()
         .filter(|&x| graph.neighbors_directed(x, prev_direction).next().is_none())
         .collect();
 
     ready.extend(ready_nodes.iter());
 
-    while let Some(n) = ready_nodes.pop() {
+    while let Some(n) = ready_nodes.pop_front() {
         visited.insert(n);
 
         // Remember the next nodes from the current node in case it gets deleted.
@@ -244,7 +244,7 @@ where
             for i in graph.neighbors_directed(n, next_direction) {
                 if !ready.contains(&i) && node_ready(i) {
                     ready.insert(i);
-                    ready_nodes.push(i);
+                    ready_nodes.push_back(i);
                 }
             }
         }
@@ -253,15 +253,15 @@ where
         for i in next_nodes {
             if !ready.contains(&i) && node_ready(i) {
                 ready.insert(i);
-                ready_nodes.push(i);
+                ready_nodes.push_back(i);
             }
         }
 
         // Check for and sources/sinks the callback may have added.
         for i in added_nodes {
             if graph.neighbors_directed(i, prev_direction).next().is_none() {
                 ready.insert(i);
-                ready_nodes.push(i);
+                ready_nodes.push_back(i);
             }
         }
     }
@@ -610,10 +610,13 @@ mod tests {
         assert_eq!(
             visited,
             vec![
-                NodeIndex::from(3),
-                NodeIndex::from(1),
+                // Inputs first visited in order
                 NodeIndex::from(0),
+                NodeIndex::from(1),
+                NodeIndex::from(3),
+                // Then the addition
                 NodeIndex::from(2),
+                // And finally the multiplication which depends on the addition
                 NodeIndex::from(4)
             ]
         );
@@ -688,11 +691,15 @@ mod tests {
         assert_eq!(
             visited,
             vec![
+                // First the mul (4), which depends on (2, 3)
                 NodeIndex::from(4),
+                // The RHS of the mul (4)
+                NodeIndex::from(3),
+                // Then the LHS of the mul, which is the add (2)
                 NodeIndex::from(2),
-                NodeIndex::from(0),
+                // Then the add inputs
                 NodeIndex::from(1),
-                NodeIndex::from(3)
+                NodeIndex::from(0),
             ]
         );
     }
@@ -717,14 +724,16 @@ mod tests {
         })
         .unwrap();
 
+        // we should end up with the same order as without the deletion,
+        // as we still mark the addition as visited
         assert_eq!(
             visited,
             vec![
                 NodeIndex::from(4),
+                NodeIndex::from(3),
                 NodeIndex::from(2),
-                NodeIndex::from(0),
                 NodeIndex::from(1),
-                NodeIndex::from(3)
+                NodeIndex::from(0),
             ]
         );
     }
@@ -738,7 +747,7 @@ mod tests {
         forward_traverse_mut(&mut ir.graph, |_, n| {
             visited.push(n);
 
-            // Delete the addition
+            // Add a multplication
             if n.index() == 2 {
                 let mut transforms: GraphTransforms<NodeInfo<Operation>, EdgeInfo> =
                     GraphTransforms::new();
@@ -763,15 +772,20 @@ mod tests {
         })
         .unwrap();
 
+        // The difference from the forward traversal without append should just be the new node
+        // inserted right before the mul (4). this is because, while the new node doesn't have any
+        // unvisited dependencies, it still gets added to the _end_ of the ready queue. The (4)
+        // node still comes later because, at the time of the append, that node is _not_ yet ready
+        // and thus not yet in the queue.
         assert_eq!(
             visited,
             vec![
-                NodeIndex::from(3),
-                NodeIndex::from(1),
                 NodeIndex::from(0),
+                NodeIndex::from(1),
+                NodeIndex::from(3),
                 NodeIndex::from(2),
-                NodeIndex::from(4),
                 NodeIndex::from(5),
+                NodeIndex::from(4),
             ]
         );
     }

sunscreen_runtime/src/linked.rs

@@ -66,8 +66,6 @@ pub enum LinkedProofError {
 fn new_single_party_with_shared_generators(
     gens_capacity: usize,
     shared_generators: &[RistrettoPoint],
-    insertion_point: usize,
-    right_side_allocated: bool,
 ) -> BulletproofGens {
     let mut label = [b'G', 0, 0, 0, 0];
     let mut g = GeneratorsChain::new(&label)
@@ -79,21 +77,14 @@ fn new_single_party_with_shared_generators(
         .take(gens_capacity)
         .collect::<Vec<RistrettoPoint>>();
 
-    let mut index = insertion_point;
-    let mut left_side = !right_side_allocated;
-
-    // Insert the shared generators. Note that the order of the shared
-    // generators is reversed because the inputs in the R1CS BP are reversed
-    // after compilation.
-    for gen in shared_generators.iter() {
-        if left_side {
-            g[index] = *gen;
-            left_side = false;
-            // TODO this overflows if shared arguments are provided with no constraints
-            index -= 1;
+    // Insert the shared generators.
+    for (ix, gen) in shared_generators.iter().enumerate() {
+        // left side
+        if ix % 2 == 0 {
+            g[ix / 2] = *gen;
+        // right side
         } else {
-            h[index] = *gen;
-            left_side = true;
+            h[ix / 2] = *gen;
         }
     }
 
@@ -196,26 +187,15 @@ impl LinkedProof {
             .chain(private_inputs_bigint)
             .collect::<Vec<_>>();
 
-        let metrics = backend.metrics(
-            &program.zkp_program_fn,
-            &private_inputs_bigint,
-            &public_inputs_bigint,
-            &constant_inputs_bigint,
-        )?;
-
         let constraint_count = backend.constraint_count(
             &program.zkp_program_fn,
             &private_inputs_bigint,
             &public_inputs_bigint,
             &constant_inputs_bigint,
         )?;
 
-        let bulletproof_gens = new_single_party_with_shared_generators(
-            2 * constraint_count,
-            &shared_gens.clone(),
-            metrics.multipliers - 1,
-            metrics.final_multiplier_rhs_allocated,
-        );
+        let bulletproof_gens =
+            new_single_party_with_shared_generators(2 * constraint_count, &shared_gens.clone());
 
         let verifier_parameters = BulletproofVerifierParameters::new(
             PedersenGens::default(),