-
Notifications
You must be signed in to change notification settings - Fork 36
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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>(¤t_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>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah the client isn't presently validating the hash from Therefore to-date this client verification just assumes the leaf hash IS the hash of the At which point I expect this test to fail unless we explicitly add handling support for missing There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>(¤t_azks, 1) | ||
.await?, | ||
akd.get_root_hash_at_epoch::<Blake3>(¤t_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>(¤t_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>(¤t_azks, 3) | ||
.await?, | ||
akd.get_root_hash_at_epoch::<Blake3>(¤t_azks, 4) | ||
.await?, | ||
audit_proof, | ||
) | ||
.await?; | ||
|
||
let audit_proof = akd.audit(1, 4).await?; | ||
audit_verify::<Blake3>( | ||
akd.get_root_hash_at_epoch::<Blake3>(¤t_azks, 1) | ||
.await?, | ||
akd.get_root_hash_at_epoch::<Blake3>(¤t_azks, 4) | ||
.await?, | ||
audit_proof, | ||
) | ||
.await?; | ||
|
||
Ok(()) | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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 latestValueState
when the previous ones have been deleted, and an update proof is only created for thatValueState
.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?There was a problem hiding this comment.
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".