From 7230136ece460c5b7169e1252f6bfc1f08bc34ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Tr=C4=85d?= Date: Tue, 26 Sep 2023 12:24:52 +0200 Subject: [PATCH] Dzejkop/fixes (#620) * Fix: pack indices after proving * Fix: indexing an empty vec --- src/contracts/mod.rs | 9 ++---- src/lib.rs | 2 +- src/prover/mod.rs | 24 ++++++++-------- src/task_monitor/tasks/process_identities.rs | 26 ++++++++++-------- tests/common/prover_mock.rs | 29 ++++++++------------ 5 files changed, 43 insertions(+), 47 deletions(-) diff --git a/src/contracts/mod.rs b/src/contracts/mod.rs index c451eade..1c4adc3a 100644 --- a/src/contracts/mod.rs +++ b/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, + deletion_indices: Vec, identity_commitments: Vec, post_root: U256, ) -> anyhow::Result { @@ -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) diff --git a/src/lib.rs b/src/lib.rs index ed1d9811..d34ee322 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/prover/mod.rs b/src/prover/mod.rs index 25c91f52..b6ee87ac 100644 --- a/src/prover/mod.rs +++ b/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, + deletion_indices: Vec, identities: Vec, ) -> anyhow::Result { 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, + deletion_indices: Vec, 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, - identity_commitments: Vec, - merkle_proofs: Vec>, + input_hash: U256, + pre_root: U256, + post_root: U256, + deletion_indices: Vec, + identity_commitments: Vec, + merkle_proofs: Vec>, } #[cfg(test)] diff --git a/src/task_monitor/tasks/process_identities.rs b/src/task_monitor/tasks/process_identities.rs index aecef1ee..929440e1 100644 --- a/src/task_monitor/tasks/process_identities.rs +++ b/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,7 +145,12 @@ 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 @@ -151,9 +158,6 @@ async fn process_identities( // 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 diff --git a/tests/common/prover_mock.rs b/tests/common/prover_mock.rs index 6aaa4fb8..a9311ee3 100644 --- a/tests/common/prover_mock.rs +++ b/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, - identity_commitments: Vec, - merkle_proofs: Vec>, + input_hash: U256, + pre_root: U256, + post_root: U256, + deletion_indices: Vec, + identity_commitments: Vec, + merkle_proofs: Vec>, } /// 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, + 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);