Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use deletion batch size during delete_identities and process_identities. #613

Merged
merged 4 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/contracts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,14 @@
self.tree_depth
}

pub async fn max_batch_size(&self) -> usize {
pub async fn max_insertion_batch_size(&self) -> usize {
self.insertion_prover_map.read().await.max_batch_size()
}

pub async fn max_deletion_batch_size(&self) -> usize {
self.deletion_prover_map.read().await.max_batch_size()
}

#[must_use]
pub const fn initial_leaf_value(&self) -> Field {
self.initial_leaf_value
Expand Down Expand Up @@ -375,7 +379,7 @@
.await?
.context("Missing tx")?;

use ethers::abi::AbiDecode;

Check warning on line 382 in src/contracts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

adding items after statements is confusing, since items exist from the start of the scope

warning: adding items after statements is confusing, since items exist from the start of the scope --> src/contracts/mod.rs:382:9 | 382 | use ethers::abi::AbiDecode; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements = note: `#[warn(clippy::items_after_statements)]` implied by `#[warn(clippy::pedantic)]`

Check warning on line 382 in src/contracts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

adding items after statements is confusing, since items exist from the start of the scope

warning: adding items after statements is confusing, since items exist from the start of the scope --> src/contracts/mod.rs:382:9 | 382 | use ethers::abi::AbiDecode; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements = note: `#[warn(clippy::items_after_statements)]` implied by `#[warn(clippy::pedantic)]`

Check warning on line 382 in src/contracts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

adding items after statements is confusing, since items exist from the start of the scope

warning: adding items after statements is confusing, since items exist from the start of the scope --> src/contracts/mod.rs:382:9 | 382 | use ethers::abi::AbiDecode; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements = note: `#[warn(clippy::items_after_statements)]` implied by `#[warn(clippy::pedantic)]`
let delete_identities = DeleteIdentitiesCall::decode(&tx.input)?;

let packed_deletion_indices: &[u8] = delete_identities.packed_deletion_indices.as_ref();
Expand All @@ -383,7 +387,7 @@

tracing::error!("unpacked = {indices:?}");

let padding_index = 2u32.pow(self.tree_depth as u32);

Check warning on line 390 in src/contracts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/contracts/mod.rs:390:38 | 390 | let padding_index = 2u32.pow(self.tree_depth as u32); | ^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation = note: `#[warn(clippy::cast_possible_truncation)]` implied by `#[warn(clippy::pedantic)]` help: ... or use `try_from` and handle the error accordingly | 390 | let padding_index = 2u32.pow(u32::try_from(self.tree_depth)); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 390 in src/contracts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/contracts/mod.rs:390:38 | 390 | let padding_index = 2u32.pow(self.tree_depth as u32); | ^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation = note: `#[warn(clippy::cast_possible_truncation)]` implied by `#[warn(clippy::pedantic)]` help: ... or use `try_from` and handle the error accordingly | 390 | let padding_index = 2u32.pow(u32::try_from(self.tree_depth)); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 390 in src/contracts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/contracts/mod.rs:390:38 | 390 | let padding_index = 2u32.pow(self.tree_depth as u32); | ^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation = note: `#[warn(clippy::cast_possible_truncation)]` implied by `#[warn(clippy::pedantic)]` help: ... or use `try_from` and handle the error accordingly | 390 | let padding_index = 2u32.pow(u32::try_from(self.tree_depth)); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Ok(indices
.into_iter()
Expand Down
3 changes: 0 additions & 3 deletions src/task_monitor/tasks/delete_identities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ async fn delete_identities(
loop {
let deletions = database.get_deletions().await?;
if deletions.is_empty() {
// Sleep for one hour
// TODO: should we make this dynamic? This causes an issue with tests so its set
// to 1 sec atm
tokio::time::sleep(tokio::time::Duration::from_secs(5)).await;
continue;
}
Expand Down
13 changes: 12 additions & 1 deletion src/task_monitor/tasks/process_identities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
identity_manager.await_clean_slate().await?;

info!("Starting identity processor.");
let batch_size = identity_manager.max_batch_size().await;

// We start a timer and force it to perform one initial tick to avoid an
// immediate trigger.
Expand Down Expand Up @@ -101,6 +100,12 @@
// 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{
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;
Expand Down Expand Up @@ -138,6 +143,12 @@
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{
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() {
Expand Down Expand Up @@ -186,37 +197,37 @@
updates: &[AppliedTreeUpdate],
) -> AnyhowResult<()> {
// If the update is an insertion
if updates
.first()
.context("Updates should be > 1")?
.update
.element
!= Hash::ZERO
{
let prover = identity_manager
.get_suitable_insertion_prover(updates.len())
.await?;

info!(
"Sending timed-out insertion batch with {}/{} updates.",
updates.len(),
prover.batch_size()
);

insert_identities(database, identity_manager, batching_tree, updates, prover).await?;
} else {
let prover = identity_manager
.get_suitable_deletion_prover(updates.len())
.await?;

info!(
"Sending timed-out deletion batch with {}/{} updates.",
updates.len(),
prover.batch_size()
);

delete_identities(database, identity_manager, batching_tree, updates, prover).await?;
}

Check warning on line 230 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary `!=` operation

warning: unnecessary `!=` operation --> src/task_monitor/tasks/process_identities.rs:200:5 | 200 | / if updates 201 | | .first() 202 | | .context("Updates should be > 1")? 203 | | .update ... | 229 | | delete_identities(database, identity_manager, batching_tree, updates, prover).await?; 230 | | } | |_____^ | = help: change to `==` and swap the blocks of the `if`/`else` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else = note: `#[warn(clippy::if_not_else)]` implied by `#[warn(clippy::pedantic)]`

Check warning on line 230 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary `!=` operation

warning: unnecessary `!=` operation --> src/task_monitor/tasks/process_identities.rs:200:5 | 200 | / if updates 201 | | .first() 202 | | .context("Updates should be > 1")? 203 | | .update ... | 229 | | delete_identities(database, identity_manager, batching_tree, updates, prover).await?; 230 | | } | |_____^ | = help: change to `==` and swap the blocks of the `if`/`else` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else = note: `#[warn(clippy::if_not_else)]` implied by `#[warn(clippy::pedantic)]`

Check warning on line 230 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary `!=` operation

warning: unnecessary `!=` operation --> src/task_monitor/tasks/process_identities.rs:200:5 | 200 | / if updates 201 | | .first() 202 | | .context("Updates should be > 1")? 203 | | .update ... | 229 | | delete_identities(database, identity_manager, batching_tree, updates, prover).await?; 230 | | } | |_____^ | = help: change to `==` and swap the blocks of the `if`/`else` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else = note: `#[warn(clippy::if_not_else)]` implied by `#[warn(clippy::pedantic)]`

Ok(())
}
Expand Down Expand Up @@ -255,11 +266,11 @@
.map(|update| update.update.element)
.collect::<Vec<_>>();

panic!(
"Identities are not consecutive leaves in the tree (leaf_indexes = {:?}, \
commitments = {:?})",
leaf_indexes, commitments
);

Check warning on line 273 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

variables can be used directly in the `format!` string

warning: variables can be used directly in the `format!` string --> src/task_monitor/tasks/process_identities.rs:269:13 | 269 | / panic!( 270 | | "Identities are not consecutive leaves in the tree (leaf_indexes = {:?}, \ 271 | | commitments = {:?})", 272 | | leaf_indexes, commitments 273 | | ); | |_____________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

Check warning on line 273 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

variables can be used directly in the `format!` string

warning: variables can be used directly in the `format!` string --> src/task_monitor/tasks/process_identities.rs:269:13 | 269 | / panic!( 270 | | "Identities are not consecutive leaves in the tree (leaf_indexes = {:?}, \ 271 | | commitments = {:?})", 272 | | leaf_indexes, commitments 273 | | ); | |_____________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

Check warning on line 273 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

variables can be used directly in the `format!` string

warning: variables can be used directly in the `format!` string --> src/task_monitor/tasks/process_identities.rs:269:13 | 269 | / panic!( 270 | | "Identities are not consecutive leaves in the tree (leaf_indexes = {:?}, \ 271 | | commitments = {:?})", 272 | | leaf_indexes, commitments 273 | | ); | |_____________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
}

last_index = update.update.leaf_index;
Expand Down Expand Up @@ -407,28 +418,28 @@
Ok(())
}

pub async fn delete_identities(
database: &Database,
identity_manager: &IdentityManager,
batching_tree: &TreeVersion<Intermediate>,
updates: &[AppliedTreeUpdate],
prover: ReadOnlyProver<'_, Prover>,
) -> AnyhowResult<()> {
TaskMonitor::log_identities_queues(database).await?;

if updates.is_empty() {
warn!("Identity commit requested with zero identities. Continuing.");
return Ok(());
}

debug!("Starting identity commit for {} identities.", updates.len());

// Grab the initial conditions before the updates are applied to the tree.
let pre_root: U256 = batching_tree.get_root().into();

let mut deletion_indices = updates
.iter()
.map(|f| f.update.leaf_index as u32)

Check warning on line 442 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:442:18 | 442 | .map(|f| f.update.leaf_index as u32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 442 | .map(|f| u32::try_from(f.update.leaf_index)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 442 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:442:18 | 442 | .map(|f| f.update.leaf_index as u32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 442 | .map(|f| u32::try_from(f.update.leaf_index)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 442 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:442:18 | 442 | .map(|f| f.update.leaf_index as u32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 442 | .map(|f| u32::try_from(f.update.leaf_index)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.collect::<Vec<u32>>();

let commitments =
Expand Down Expand Up @@ -469,7 +480,7 @@
// ensure that our batches match that size. We do this by padding deletion
// indices with tree.depth() ^ 2. The deletion prover will skip the proof for
// any deletion with an index greater than the max tree depth
let pad_index = 2_u32.pow(latest_tree_from_updates.depth() as u32);

Check warning on line 483 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:483:31 | 483 | let pad_index = 2_u32.pow(latest_tree_from_updates.depth() as u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 483 | let pad_index = 2_u32.pow(u32::try_from(latest_tree_from_updates.depth())); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 483 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:483:31 | 483 | let pad_index = 2_u32.pow(latest_tree_from_updates.depth() as u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 483 | let pad_index = 2_u32.pow(u32::try_from(latest_tree_from_updates.depth())); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 483 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:483:31 | 483 | let pad_index = 2_u32.pow(latest_tree_from_updates.depth() as u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 483 | let pad_index = 2_u32.pow(u32::try_from(latest_tree_from_updates.depth())); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

if commitment_count != batch_size {
let padding = batch_size - commitment_count;
Expand Down Expand Up @@ -532,7 +543,7 @@
let transaction_id = identity_manager
.delete_identities(
proof,
batch_size as u32,

Check warning on line 546 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:546:13 | 546 | batch_size as u32, | ^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 546 | u32::try_from(batch_size), | ~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 546 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:546:13 | 546 | batch_size as u32, | ^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 546 | u32::try_from(batch_size), | ~~~~~~~~~~~~~~~~~~~~~~~~~

Check warning on line 546 in src/task_monitor/tasks/process_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers

warning: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers --> src/task_monitor/tasks/process_identities.rs:546:13 | 546 | batch_size as u32, | ^^^^^^^^^^^^^^^^^ | = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation help: ... or use `try_from` and handle the error accordingly | 546 | u32::try_from(batch_size), | ~~~~~~~~~~~~~~~~~~~~~~~~~
packed_deletion_indices,
pre_root,
post_root,
Expand Down
Loading