Dzejkop/fixes (#620)

* Fix: pack indices after proving

* Fix: indexing an empty vec

src/contracts/mod.rs

@@ -241,7 +241,7 @@ impl IdentityManager {
     pub async fn prepare_deletion_proof(
         prover: ReadOnlyProver<'_, Prover>,
         pre_root: U256,
-        packed_deletion_indices: Vec<u8>,
+        deletion_indices: Vec<u32>,
         identity_commitments: Vec<Identity>,
         post_root: U256,
     ) -> anyhow::Result<Proof> {
@@ -252,12 +252,7 @@ impl IdentityManager {
         );
 
         let proof_data: Proof = prover
-            .generate_deletion_proof(
-                pre_root,
-                post_root,
-                packed_deletion_indices,
-                identity_commitments,
-            )
+            .generate_deletion_proof(pre_root, post_root, deletion_indices, identity_commitments)
             .await?;
 
         Ok(proof_data)

src/lib.rs

@@ -12,7 +12,7 @@ pub mod secret;
 mod serde_utils;
 pub mod server;
 mod task_monitor;
-mod utils;
+pub mod utils;
 
 use std::sync::Arc;
 

src/prover/mod.rs

@@ -29,6 +29,7 @@ use url::Url;
 
 use crate::prover::identity::Identity;
 use crate::serde_utils::JsonStrWrapper;
+use crate::utils::index_packing::pack_indices;
 
 /// The endpoint used for proving operations.
 const MTB_PROVE_ENDPOINT: &str = "prove";
@@ -249,7 +250,7 @@ impl Prover {
         &self,
         pre_root: U256,
         post_root: U256,
-        packed_deletion_indices: Vec<u8>,
+        deletion_indices: Vec<u32>,
         identities: Vec<Identity>,
     ) -> anyhow::Result<Proof> {
         if identities.len() != self.batch_size {
@@ -266,13 +267,13 @@ impl Prover {
             .unzip();
 
         let input_hash =
-            compute_deletion_proof_input_hash(packed_deletion_indices.clone(), pre_root, post_root);
+            compute_deletion_proof_input_hash(deletion_indices.clone(), pre_root, post_root);
 
         let proof_input = DeletionProofInput {
             input_hash,
             pre_root,
             post_root,
-            packed_deletion_indices,
+            deletion_indices,
             identity_commitments,
             merkle_proofs,
         };
@@ -355,7 +356,7 @@ pub fn compute_insertion_proof_input_hash(
 // TODO: check this and update docs
 
 pub fn compute_deletion_proof_input_hash(
-    packed_deletion_indices: Vec<u8>,
+    deletion_indices: Vec<u32>,
     pre_root: U256,
     post_root: U256,
 ) -> U256 {
@@ -369,7 +370,8 @@ pub fn compute_deletion_proof_input_hash(
     let mut bytes = vec![];
 
     // Append packed_deletion_indices
-    bytes.extend_from_slice(&packed_deletion_indices);
+    let packed_indices = pack_indices(&deletion_indices);
+    bytes.extend_from_slice(&packed_indices);
 
     // Append pre_root and post_root bytes
     bytes.extend_from_slice(&pre_root_bytes);
@@ -410,12 +412,12 @@ struct InsertionProofInput {
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct DeletionProofInput {
-    input_hash:              U256,
-    pre_root:                U256,
-    post_root:               U256,
-    packed_deletion_indices: Vec<u8>,
-    identity_commitments:    Vec<U256>,
-    merkle_proofs:           Vec<Vec<U256>>,
+    input_hash:           U256,
+    pre_root:             U256,
+    post_root:            U256,
+    deletion_indices:     Vec<u32>,
+    identity_commitments: Vec<U256>,
+    merkle_proofs:        Vec<Vec<U256>>,
 }
 
 #[cfg(test)]

src/task_monitor/tasks/process_identities.rs

@@ -100,16 +100,18 @@ async fn process_identities(
                 // If the timer has fired we want to insert whatever
                 // identities we have, even if it's not many. This ensures
                 // a minimum quality of service for API users.
-                let batch_size = if batching_tree.peek_next_updates(1)[0].update.element == Hash::ZERO{
+                let next_update = batching_tree.peek_next_updates(1);
+                if next_update.is_empty() {
+                    continue;
+                }
+
+                let batch_size = if next_update[0].update.element == Hash::ZERO {
                     identity_manager.max_deletion_batch_size().await
                 }else{
                     identity_manager.max_insertion_batch_size().await
                 };
 
                 let updates = batching_tree.peek_next_updates(batch_size);
-                if updates.is_empty() {
-                    continue;
-                }
 
                 commit_identities(
                     database,
@@ -143,17 +145,19 @@ async fn process_identities(
                 let should_process_anyway =
                     timeout_secs.abs_diff(diff_secs) <= DEBOUNCE_THRESHOLD_SECS;
 
-                let batch_size = if batching_tree.peek_next_updates(1)[0].update.element == Hash::ZERO{
+                let next_update = batching_tree.peek_next_updates(1);
+                if next_update.is_empty() {
+                    continue;
+                }
+
+                let batch_size = if next_update[0].update.element == Hash::ZERO {
                     identity_manager.max_deletion_batch_size().await
                 }else{
                     identity_manager.max_insertion_batch_size().await
                 };
 
                 // We have _at most_ one complete batch here.
                 let updates = batching_tree.peek_next_updates(batch_size);
-                if updates.is_empty() {
-                    continue;
-                }
 
                 // If there are not enough identities to insert at this
                 // stage we can wait. The timer will ensure that the API
@@ -524,18 +528,18 @@ pub async fn delete_identities(
 
     identity_manager.validate_merkle_proofs(&identity_commitments)?;
 
-    let packed_deletion_indices = pack_indices(&deletion_indices);
-
     // We prepare the proof before reserving a slot in the pending identities
     let proof = IdentityManager::prepare_deletion_proof(
         prover,
         pre_root,
-        packed_deletion_indices.clone(),
+        deletion_indices.clone(),
         identity_commitments,
         post_root,
     )
     .await?;
 
+    let packed_deletion_indices = pack_indices(&deletion_indices);
+
     info!(?pre_root, ?post_root, "Submitting deletion batch");
 
     // With all the data prepared we can submit the identities to the on-chain

tests/common/prover_mock.rs

@@ -13,6 +13,7 @@ use ethers::utils::keccak256;
 use hyper::StatusCode;
 use semaphore::poseidon_tree::{Branch, Proof as TreeProof};
 use serde::{Deserialize, Serialize};
+use signup_sequencer::utils::index_packing::pack_indices;
 use tokio::sync::Mutex;
 
 /// A representation of an error from the prover.
@@ -53,12 +54,12 @@ struct InsertionProofInput {
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct DeletionProofInput {
-    input_hash:              U256,
-    pre_root:                U256,
-    post_root:               U256,
-    packed_deletion_indices: Vec<u8>,
-    identity_commitments:    Vec<U256>,
-    merkle_proofs:           Vec<Vec<U256>>,
+    input_hash:           U256,
+    pre_root:             U256,
+    post_root:            U256,
+    deletion_indices:     Vec<u32>,
+    identity_commitments: Vec<U256>,
+    merkle_proofs:        Vec<Vec<U256>>,
 }
 
 /// The proof response from the prover.
@@ -319,7 +320,7 @@ impl Prover {
 
         // Calculate the input hash based on the prover parameters.
         let input_hash = Self::compute_deletion_proof_input_hash(
-            input.packed_deletion_indices.clone(),
+            &input.deletion_indices,
             input.pre_root,
             input.post_root,
         );
@@ -333,15 +334,7 @@ impl Prover {
         let empty_leaf = U256::zero();
         let mut last_root = input.pre_root;
 
-        let mut deletion_indices = vec![];
-
-        for bytes in input.packed_deletion_indices.chunks(4) {
-            let mut val: [u8; 4] = Default::default();
-            val.copy_from_slice(bytes);
-            deletion_indices.push(u32::from_be_bytes(val));
-        }
-
-        for (leaf_index, merkle_proof) in deletion_indices.iter().zip(input.merkle_proofs) {
+        for (leaf_index, merkle_proof) in input.deletion_indices.iter().zip(input.merkle_proofs) {
             if *leaf_index == 2u32.pow(self.tree_depth as u32) {
                 continue;
             }
@@ -436,10 +429,12 @@ impl Prover {
     ///   32 bits * batchSize ||   256   ||    256
     /// ```
     pub fn compute_deletion_proof_input_hash(
-        packed_deletion_indices: Vec<u8>,
+        deletion_indices: &[u32],
         pre_root: U256,
         post_root: U256,
     ) -> U256 {
+        let packed_deletion_indices = pack_indices(deletion_indices);
+
         // Convert pre_root and post_root to bytes
         let mut pre_root_bytes = vec![0u8; 32];
         pre_root.to_big_endian(&mut pre_root_bytes);