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

Fix immediate deletion edge case #746

Merged
merged 2 commits into from
Jun 13, 2024
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
12 changes: 11 additions & 1 deletion src/task_monitor/tasks/delete_identities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<App>,
Expand Down Expand Up @@ -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);
Expand Down
56 changes: 56 additions & 0 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,62 @@ pub async fn test_inclusion_proof(
);
}

#[instrument(skip_all)]
pub async fn test_inclusion_proof_mined(
uri: &str,
client: &Client<HttpConnector>,
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::<serde_json::Value>(&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,
Expand Down
5 changes: 5 additions & 0 deletions tests/common/test_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
141 changes: 141 additions & 0 deletions tests/immediate_deletion.rs
Original file line number Diff line number Diff line change
@@ -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(&micro_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<Field> = 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 * 2)).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 * 2)).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(())
}
Loading