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

Add test covering deletion of stale ValueStates #142

Closed
wants to merge 2 commits into from
Closed
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
184 changes: 184 additions & 0 deletions akd/src/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,4 +833,188 @@ mod tests {

Ok(())
}

#[tokio::test]
async fn test_cleaned_user_state() -> Result<(), AkdError> {
let db = AsyncInMemoryDatabase::new();
let mut akd = Directory::<_>::new::<Blake3>(&db).await?;

// Publish twice
akd.publish::<Blake3>(
vec![
(AkdLabel("hello".to_string()), AkdValue("world".to_string())),
(
AkdLabel("hello2".to_string()),
AkdValue("world2".to_string()),
),
],
false,
)
.await?;

akd.publish::<Blake3>(
vec![
(AkdLabel("hello".to_string()), AkdValue("world".to_string())),
(
AkdLabel("hello2".to_string()),
AkdValue("world2".to_string()),
),
],
false,
)
.await?;

// Delete all user states from storage except most recent
db.clean_user_state(&[
AkdLabel("hello".to_string()),
AkdLabel("hello2".to_string()),
])
.await;

// Verify lookup, key history and audit proofs
let current_azks = akd.retrieve_current_azks().await?;
let root_hash = akd.get_root_hash::<Blake3>(&current_azks).await?;

let history_proof = akd.key_history(&AkdLabel("hello".to_string())).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value in the key history proof if the previous values have been deleted? This doesn't return error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in other words, the storage layer is returning GetError("NotFound") or something for some valuestate, and how is the proof still being constructed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, the proof is constructed for the ValueStates that are retrieved. get_user_data would only return the latest ValueState when the previous ones have been deleted, and an update proof is only created for that ValueState.

https://github.com/novifinancial/akd/blob/a409fc284865d4e69d51a218e262dbb94d25ef6e/akd/src/directory.rs#L241-L259

So wouldn't AKD avoid looking up ValueStates that are already deleted and therefore not run into the "NotFound" errors?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm OK so that's no ideal. In that case it should be basing the updates off of the state of the node, not the user data retrieved. @Jasleen1 this is a change that will need to occur. There were updates at some epochs, but there just happen to no longer be values associated to those updates.

In the current implementation it's simply stating "there was no change at the given epoch".

let (root_hashes, previous_root_hashes) =
get_key_history_hashes(&akd, &history_proof).await?;
key_history_verify::<Blake3>(
Copy link
Contributor

@slawlor slawlor Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah the client isn't presently validating the hash from plaintext_value to leaf value: ref.

Therefore to-date this client verification just assumes the leaf hash IS the hash of the plaintext_value without ever verifying it. @Jasleen1 is that intended or this is to be filled out once VRF's (#122) are included?

At which point I expect this test to fail unless we explicitly add handling support for missing plaintext_value's

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See reply to above comment - we can also wait for VRFs to be committed before merging this PR.

root_hashes,
previous_root_hashes,
AkdLabel("hello".to_string()),
history_proof,
)?;

let history_proof = akd.key_history(&AkdLabel("hello2".to_string())).await?;
let (root_hashes, previous_root_hashes) =
get_key_history_hashes(&akd, &history_proof).await?;
key_history_verify::<Blake3>(
root_hashes,
previous_root_hashes,
AkdLabel("hello2".to_string()),
history_proof,
)?;

let lookup_proof = akd.lookup(AkdLabel("hello".to_string())).await?;
lookup_verify::<Blake3>(root_hash, AkdLabel("hello".to_string()), lookup_proof)?;

let lookup_proof = akd.lookup(AkdLabel("hello2".to_string())).await?;
lookup_verify::<Blake3>(root_hash, AkdLabel("hello2".to_string()), lookup_proof)?;

let audit_proof = akd.audit(1, 2).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audit proof shouldn't care, since it (by design) doesn't need the plaintext values to determine a mutation hasn't occurred in history.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup I am aware, but I was thinking that a test to demonstrate the property couldn't hurt :)

audit_verify::<Blake3>(
akd.get_root_hash_at_epoch::<Blake3>(&current_azks, 1)
.await?,
akd.get_root_hash_at_epoch::<Blake3>(&current_azks, 2)
.await?,
audit_proof,
)
.await?;

// Perform two more publish operations
akd.publish::<Blake3>(
vec![
(AkdLabel("hello".to_string()), AkdValue("hello".to_string())),
(
AkdLabel("hello2".to_string()),
AkdValue("hello2".to_string()),
),
(
AkdLabel("hello3".to_string()),
AkdValue("world3".to_string()),
),
],
false,
)
.await?;

akd.publish::<Blake3>(
vec![
(
AkdLabel("hello".to_string()),
AkdValue("world_updated".to_string()),
),
(
AkdLabel("hello2".to_string()),
AkdValue("world2_updated".to_string()),
),
(
AkdLabel("hello3".to_string()),
AkdValue("world3_updated".to_string()),
),
],
false,
)
.await?;

// Delete all user states from storage except most recent
db.clean_user_state(&[
AkdLabel("hello".to_string()),
AkdLabel("hello2".to_string()),
AkdLabel("hello3".to_string()),
])
.await;

// Verify lookup, key history and audit proofs
let current_azks = akd.retrieve_current_azks().await?;
let root_hash = akd.get_root_hash::<Blake3>(&current_azks).await?;

let history_proof = akd.key_history(&AkdLabel("hello".to_string())).await?;
let (root_hashes, previous_root_hashes) =
get_key_history_hashes(&akd, &history_proof).await?;
key_history_verify::<Blake3>(
root_hashes,
previous_root_hashes,
AkdLabel("hello".to_string()),
history_proof,
)?;

let history_proof = akd.key_history(&AkdLabel("hello2".to_string())).await?;
let (root_hashes, previous_root_hashes) =
get_key_history_hashes(&akd, &history_proof).await?;
key_history_verify::<Blake3>(
root_hashes,
previous_root_hashes,
AkdLabel("hello2".to_string()),
history_proof,
)?;

let history_proof = akd.key_history(&AkdLabel("hello3".to_string())).await?;
let (root_hashes, previous_root_hashes) =
get_key_history_hashes(&akd, &history_proof).await?;
key_history_verify::<Blake3>(
root_hashes,
previous_root_hashes,
AkdLabel("hello3".to_string()),
history_proof,
)?;

let lookup_proof = akd.lookup(AkdLabel("hello".to_string())).await?;
lookup_verify::<Blake3>(root_hash, AkdLabel("hello".to_string()), lookup_proof)?;

let lookup_proof = akd.lookup(AkdLabel("hello3".to_string())).await?;
lookup_verify::<Blake3>(root_hash, AkdLabel("hello3".to_string()), lookup_proof)?;

let audit_proof = akd.audit(3, 4).await?;
audit_verify::<Blake3>(
akd.get_root_hash_at_epoch::<Blake3>(&current_azks, 3)
.await?,
akd.get_root_hash_at_epoch::<Blake3>(&current_azks, 4)
.await?,
audit_proof,
)
.await?;

let audit_proof = akd.audit(1, 4).await?;
audit_verify::<Blake3>(
akd.get_root_hash_at_epoch::<Blake3>(&current_azks, 1)
.await?,
akd.get_root_hash_at_epoch::<Blake3>(&current_azks, 4)
.await?,
audit_proof,
)
.await?;

Ok(())
}
}
12 changes: 12 additions & 0 deletions akd/src/storage/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ impl AsyncInMemoryDatabase {
trans: Transaction::new(),
}
}

/// Deletes all user state except the most recent for given labels
pub async fn clean_user_state(&self, keys: &[AkdLabel]) {
let mut guard = self.user_info.write().await;

for key in keys {
let username = key.0.clone();
let values = guard.entry(username).or_insert_with(Vec::new);
values.reverse();
values.truncate(1);
}
}
}

impl Default for AsyncInMemoryDatabase {
Expand Down