From 402831ad8857bf14a52120891a3486b5dc1606d5 Mon Sep 17 00:00:00 2001 From: Dzejkop Date: Thu, 13 Jun 2024 13:44:46 +0200 Subject: [PATCH 1/2] Fix immediate deletion edge case --- src/task_monitor/tasks/delete_identities.rs | 12 +- tests/common/mod.rs | 56 ++++++++ tests/common/test_config.rs | 5 + tests/immediate_deletion.rs | 141 ++++++++++++++++++++ 4 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 tests/immediate_deletion.rs diff --git a/src/task_monitor/tasks/delete_identities.rs b/src/task_monitor/tasks/delete_identities.rs index 90a091b1..3693254f 100644 --- a/src/task_monitor/tasks/delete_identities.rs +++ b/src/task_monitor/tasks/delete_identities.rs @@ -11,7 +11,7 @@ use tracing::info; use crate::app::App; use crate::database::query::DatabaseQuery; use crate::database::types::DeletionEntry; -use crate::identity_tree::Hash; +use crate::identity_tree::{Hash, TreeVersionReadOps}; pub async fn delete_identities( app: Arc, @@ -54,6 +54,16 @@ pub async fn delete_identities( let _guard = pending_insertions_mutex.lock().await; + if deletions.len() == 1 { + let last_insertion_idx = app.tree_state()?.latest_tree().next_leaf() - 1; + + let only_deletion_idx = *leaf_indices.first().unwrap(); + + if only_deletion_idx == last_insertion_idx { + continue; + } + } + // Delete the commitments at the target leaf indices in the latest tree, // generating the proof for each update let data = app.tree_state()?.latest_tree().delete_many(&leaf_indices); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 43df3e2f..c10811cf 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -300,6 +300,62 @@ pub async fn test_inclusion_proof( ); } +#[instrument(skip_all)] +pub async fn test_inclusion_proof_mined( + uri: &str, + client: &Client, + leaf: &Hash, + expect_failure: bool, +) { + for i in 0..NUM_ATTEMPTS_FOR_INCLUSION_PROOF { + let body = construct_inclusion_proof_body(leaf); + info!(?uri, "Contacting"); + let req = Request::builder() + .method("POST") + .uri(uri.to_owned() + "/inclusionProof") + .header("Content-Type", "application/json") + .body(body) + .expect("Failed to create inclusion proof hyper::Body"); + + let mut response = client + .request(req) + .await + .expect("Failed to execute request."); + + if expect_failure { + assert!(!response.status().is_success()); + return; + } else { + assert!(response.status().is_success()); + } + + let bytes = hyper::body::to_bytes(response.body_mut()) + .await + .expect("Failed to convert response body to bytes"); + let result = String::from_utf8(bytes.into_iter().collect()) + .expect("Could not parse response bytes to utf-8"); + let result_json = serde_json::from_str::(&result) + .expect("Failed to parse response as json"); + let status = result_json["status"] + .as_str() + .expect("Failed to get status"); + + if status == "pending" { + info!("Got pending, waiting 5 seconds, iteration {}", i); + tokio::time::sleep(Duration::from_secs(5)).await; + } else if status == "mined" { + return; + } else { + panic!("Unexpected status: {}", status); + } + } + + panic!( + "Failed to get an inclusion proof after {} attempts!", + NUM_ATTEMPTS_FOR_INCLUSION_PROOF + ); +} + #[instrument(skip_all)] pub async fn test_inclusion_status( uri: &str, diff --git a/tests/common/test_config.rs b/tests/common/test_config.rs index ade3569d..5498334e 100644 --- a/tests/common/test_config.rs +++ b/tests/common/test_config.rs @@ -62,6 +62,11 @@ impl TestConfigBuilder { self } + pub fn batch_deletion_timeout(mut self, batch_deletion_timeout: Duration) -> Self { + self.batch_deletion_timeout = batch_deletion_timeout; + self + } + pub fn tree_depth(mut self, tree_depth: usize) -> Self { self.tree_depth = tree_depth; self diff --git a/tests/immediate_deletion.rs b/tests/immediate_deletion.rs new file mode 100644 index 00000000..3a543fa1 --- /dev/null +++ b/tests/immediate_deletion.rs @@ -0,0 +1,141 @@ +#![allow(clippy::needless_range_loop)] + +mod common; + +use std::time::Duration; + +use common::prelude::*; +use common::test_inclusion_proof_mined; + +use crate::common::test_delete_identity; + +const IDLE_TIME: u64 = 7; + +#[tokio::test] +async fn immediate_deletion() -> anyhow::Result<()> { + // Initialize logging for the test. + init_tracing_subscriber(); + info!("Starting integration test"); + + let insertion_batch_size: usize = 8; + let deletion_batch_size: usize = 3; + + let mut ref_tree = PoseidonTree::new(DEFAULT_TREE_DEPTH + 1, ruint::Uint::ZERO); + let initial_root: U256 = ref_tree.root().into(); + + let docker = Cli::default(); + let (mock_chain, db_container, insertion_prover_map, deletion_prover_map, micro_oz) = + spawn_deps( + initial_root, + &[insertion_batch_size], + &[deletion_batch_size], + DEFAULT_TREE_DEPTH as u8, + &docker, + ) + .await?; + + let mock_insertion_prover = &insertion_prover_map[&insertion_batch_size]; + let mock_deletion_prover = &deletion_prover_map[&deletion_batch_size]; + + let db_socket_addr = db_container.address(); + let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database"); + + let temp_dir = tempfile::tempdir()?; + info!( + "temp dir created at: {:?}", + temp_dir.path().join("testfile") + ); + + let config = TestConfigBuilder::new() + .db_url(&db_url) + .oz_api_url(µ_oz.endpoint()) + .oz_address(micro_oz.address()) + .identity_manager_address(mock_chain.identity_manager.address()) + .primary_network_provider(mock_chain.anvil.endpoint()) + .cache_file(temp_dir.path().join("testfile").to_str().unwrap()) + .batch_deletion_timeout(Duration::from_secs(1)) // We'll make deletion timeout really short + .add_prover(mock_insertion_prover) + .add_prover(mock_deletion_prover) + .build()?; + + let (_, app_handle, local_addr) = spawn_app(config.clone()) + .await + .expect("Failed to spawn app."); + + let test_identities = generate_test_identities(insertion_batch_size * 3); + let identities_ref: Vec = test_identities + .iter() + .map(|i| Hash::from_str_radix(i, 16).unwrap()) + .collect(); + + let uri = "http://".to_owned() + &local_addr.to_string(); + let client = Client::new(); + + // Insert enough identities to trigger an batch to be sent to the blockchain. + for i in 0..insertion_batch_size { + test_insert_identity(&uri, &client, &mut ref_tree, &identities_ref, i).await; + } + + tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await; + + // Check that we can also get these inclusion proofs back. + for i in 0..insertion_batch_size { + test_inclusion_proof( + &uri, + &client, + i, + &ref_tree, + &Hash::from_str_radix(&test_identities[i], 16) + .expect("Failed to parse Hash from test leaf"), + false, + ) + .await; + } + + // Only delete the last identity from the previous batch + // which would create a duplicate root + test_delete_identity( + &uri, + &client, + &mut ref_tree, + &identities_ref, + insertion_batch_size - 1, + false, + ) + .await; + + tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await; + + // Ensure the identity has not yet been deleted + test_inclusion_proof_mined( + &uri, + &client, + &Hash::from_str_radix(&test_identities[insertion_batch_size - 1], 16) + .expect("Failed to parse Hash from test leaf"), + false, + ) + .await; + + // Delete another identity to trigger a deletion batch + test_delete_identity(&uri, &client, &mut ref_tree, &identities_ref, 0, false).await; + + tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await; + + test_inclusion_proof_mined( + &uri, + &client, + &Hash::from_str_radix(&test_identities[insertion_batch_size - 1], 16) + .expect("Failed to parse Hash from test leaf"), + true, + ) + .await; + + // Shutdown the app and reset the mock shutdown, allowing us to test the + // behaviour with saved data. + info!("Stopping the app for testing purposes"); + shutdown(); + app_handle.await.unwrap(); + reset_shutdown(); + + Ok(()) +} From d2fd6a7f2aef321f8988b3d1b86775bc2bc23c64 Mon Sep 17 00:00:00 2001 From: Dzejkop Date: Thu, 13 Jun 2024 13:53:35 +0200 Subject: [PATCH 2/2] Fix --- tests/immediate_deletion.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/immediate_deletion.rs b/tests/immediate_deletion.rs index 3a543fa1..c664ff06 100644 --- a/tests/immediate_deletion.rs +++ b/tests/immediate_deletion.rs @@ -104,7 +104,7 @@ async fn immediate_deletion() -> anyhow::Result<()> { ) .await; - tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await; + tokio::time::sleep(Duration::from_secs(IDLE_TIME * 2)).await; // Ensure the identity has not yet been deleted test_inclusion_proof_mined( @@ -119,7 +119,7 @@ async fn immediate_deletion() -> anyhow::Result<()> { // Delete another identity to trigger a deletion batch test_delete_identity(&uri, &client, &mut ref_tree, &identities_ref, 0, false).await; - tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await; + tokio::time::sleep(Duration::from_secs(IDLE_TIME * 2)).await; test_inclusion_proof_mined( &uri,