Skip to content

Commit

Permalink
Improve NotFound Error Accuracy in key_history (#452)
Browse files Browse the repository at this point in the history
* Improve NotFound Error Accuracy in key_history

**Issue**
Prior to this patch, the `Directory::key_history` API had potential to return
a `HistoryProof` with no `UpdateProof`'s included in it. Considering that `HistoryProof`s
can be thought of as an array of `UpdateProof`'s (see: https://docs.rs/akd/0.11.0/akd/struct.HistoryProof.html),
the presence of at least one `UpdateProof` is required.

The issue detailed above can happen in instances where we perform a read of user states
during a publish operation. For example:
- The `Directory` has a latest epoch of 10.
- A client of AKD performs a read during a publish operation.
- `Directory::key_history` reads user states from storage and sees a User U's states
  with an epoch of 11, indicating that User U has just been added to AKD.
- Existing checks which build the `UpdateProof`s to return in the `HistoryProof`
  filter out the user state associated with epoch 11 since the `Directory`'s latest
  epoch is only 10.

Despite filtering out User U's `ValueState` for epoch 11 correctly (i.e. we cannot include
data ahead of the `Directory`), AKD would return a `HistoryProof` containing 0 `UpdateProof`'s.
As a result, verification for the `HistoryProof` would fail.

**Solution**
The solution to the issue mentioned above simply filters out any user's `ValueState`'s which
have an epoch greater than the the `Directory`'s current epoch, which then triggers an existing
check to return an `AkdError::Storage(StroageError::NotFound)` error. The error more appropriately
indicates that the `Directory` does not technically have history for the user yet, at least not
until it's latest epoch is updated (via its `aZKS`).

**Verfication**
To verify that the solution works as intended, we update the `test_errors::test_read_during_publish`
test to insert a label which has an epoch greater than the directory's. Prior to including
changes made to the directory in this patch, the resultant `HistoryProof` fails to verify due
to `UpdateProof`'s not being present:

```
thread 'tests::test_errors::test_read_during_publish_experimental_config' panicked at akd/src/tests/test_errors.rs:269:10:
called `Result::unwrap()` on an `Err` value: HistoryProof("No update proofs included in the proof of user AkdLabel([104, 101, 108, 108, 111]) at epoch 2!")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[00:00:00.057] INFO   No cache found, skipping preload (append_only_zks:651)
test tests::test_errors::test_read_during_publish_experimental_config ... FAILED
thread 'tests::test_errors::test_read_during_publish_whatsapp_v1_config' panicked at akd/src/tests/test_errors.rs:269:10:
called `Result::unwrap()` on an `Err` value: HistoryProof("No update proofs included in the proof of user AkdLabel([104, 101, 108, 108, 111]) at epoch 2!")
test tests::test_errors::test_read_during_publish_whatsapp_v1_config ... FAILED

failures:

failures:
    tests::test_errors::test_read_during_publish_experimental_config
    tests::test_errors::test_read_during_publish_whatsapp_v1_config

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 100 filtered out; finished in 0.05s
```

After the change, we can see that the `Directory::key_history` API not more accruately
returns an `AkdError::Storage(StoragerError::NotFound)` error:

```
    // History proof for the most recently added key (ahead of directory epoch) should
    // result in the entry not being found.
    let recently_added_label_result = akd
        .key_history(&AkdLabel::from("hello3"), HistoryParams::default())
        .await;
    assert!(matches!(
        recently_added_label_result,
        Err(AkdError::Storage(StorageError::NotFound(_)))
    ));
```

```
running 2 tests
test tests::test_errors::test_read_during_publish_experimental_config ... ok
test tests::test_errors::test_read_during_publish_whatsapp_v1_config ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 100 filtered out; finished in 0.03s
```

* Prepare for 0.12.0-pre.9 Release

---------

Co-authored-by: Dillon George <[email protected]>
  • Loading branch information
dillonrg and Dillon George authored Aug 29, 2024
1 parent f9bbe29 commit fcd665a
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 21 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## 0.12.0-pre.9 (August 28, 2024)
* Fixed a bug in key history API that can incorrectly return HistoryProofs without UpdateProofs.

## 0.12.0-pre.8 (August 23, 2024)
* Fixed a bug in key history API that occurs when a proof is being generated while a transaction is
being committed
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Installation
Add the following line to the dependencies of your `Cargo.toml`:

```
akd = "0.12.0-pre.8"
akd = "0.12.0-pre.9"
```

### Minimum Supported Rust Version
Expand Down
4 changes: 2 additions & 2 deletions akd/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "akd"
version = "0.12.0-pre.8"
version = "0.12.0-pre.9"
authors = ["akd contributors"]
description = "An implementation of an auditable key directory"
license = "MIT OR Apache-2.0"
Expand Down Expand Up @@ -52,7 +52,7 @@ default = [

[dependencies]
## Required dependencies ##
akd_core = { version = "0.12.0-pre.8", path = "../akd_core", default-features = false, features = [
akd_core = { version = "0.12.0-pre.9", path = "../akd_core", default-features = false, features = [
"vrf",
] }
async-recursion = "1"
Expand Down
24 changes: 10 additions & 14 deletions akd/src/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,12 @@ where
let current_epoch = current_azks.get_latest_epoch();
let mut user_data = self.storage.get_user_data(akd_label).await?.states;

// reverse sort from highest epoch to lowest
// Ignore states in storage which are ahead of the current directory epoch
user_data.retain(|vs| vs.epoch <= current_epoch);
// Reverse sort from highest epoch to lowest
user_data.sort_by(|a, b| b.epoch.cmp(&a.epoch));

// apply filters specified by HistoryParams struct
// Apply filters specified by HistoryParams struct
user_data = match params {
HistoryParams::Complete => user_data,
HistoryParams::MostRecent(n) => user_data.into_iter().take(n).collect::<Vec<_>>(),
Expand All @@ -482,11 +484,8 @@ where
let mut start_version = user_data[0].version;
let mut end_version = user_data[0].version;
for user_state in &user_data {
// Ignore states in storage that are ahead of current directory epoch
if user_state.epoch <= current_epoch {
start_version = std::cmp::min(user_state.version, start_version);
end_version = std::cmp::max(user_state.version, end_version);
}
start_version = std::cmp::min(user_state.version, start_version);
end_version = std::cmp::max(user_state.version, end_version);
}

if start_version == 0 || end_version == 0 {
Expand Down Expand Up @@ -528,13 +527,10 @@ where
// The creation of update proofs should happen only after the preload operation (to prevent cache misses).
let mut update_proofs = Vec::<UpdateProof>::new();
for user_state in &user_data {
// Ignore states in storage that are ahead of current directory epoch
if user_state.epoch <= current_epoch {
let proof = self
.create_single_update_proof(akd_label, user_state)
.await?;
update_proofs.push(proof);
}
let proof = self
.create_single_update_proof(akd_label, user_state)
.await?;
update_proofs.push(proof);
}

let mut past_marker_vrf_proofs = vec![];
Expand Down
21 changes: 20 additions & 1 deletion akd/src/tests/test_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,11 @@ async fn test_read_during_publish<TC: Configuration>() -> Result<(), AkdError> {
// Make the current azks a "checkpoint" to reset to later
let checkpoint_azks = akd.retrieve_azks().await.unwrap();

// Publish for the third time
// Publish for the third time with a new label
akd.publish(vec![
(AkdLabel::from("hello"), AkdValue::from("world_3")),
(AkdLabel::from("hello2"), AkdValue::from("world2_3")),
(AkdLabel::from("hello3"), AkdValue::from("world3")),
])
.await
.unwrap();
Expand Down Expand Up @@ -250,6 +251,24 @@ async fn test_read_during_publish<TC: Configuration>() -> Result<(), AkdError> {
)
.unwrap();

// Lookup proof for the most recently added key (ahead of directory epoch) should
// result in the entry not being found.
let recently_added_lookup_result = akd.lookup(AkdLabel::from("hello3")).await;
assert!(matches!(
recently_added_lookup_result,
Err(AkdError::Storage(StorageError::NotFound(_)))
));

// History proof for the most recently added key (ahead of directory epoch) should
// result in the entry not being found.
let recently_added_history_result = akd
.key_history(&AkdLabel::from("hello3"), HistoryParams::default())
.await;
assert!(matches!(
recently_added_history_result,
Err(AkdError::Storage(StorageError::NotFound(_)))
));

// Audit proof should only work up until checkpoint's epoch
let audit_proof = akd.audit(1, 2).await.unwrap();
audit_verify::<TC>(vec![root_hash_1, root_hash_2], audit_proof)
Expand Down
2 changes: 1 addition & 1 deletion akd_core/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "akd_core"
version = "0.12.0-pre.8"
version = "0.12.0-pre.9"
authors = ["akd contributors"]
description = "Core utilities for the akd crate"
license = "MIT OR Apache-2.0"
Expand Down
2 changes: 1 addition & 1 deletion akd_core/src/types/node_label/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use alloc::vec::Vec;
#[cfg(test)]
mod tests;

/// Represents the label of a AKD node
/// Represents the label of an AKD node
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[cfg_attr(
feature = "serde_serialization",
Expand Down
2 changes: 1 addition & 1 deletion examples/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "examples"
version = "0.12.0-pre.8"
version = "0.12.0-pre.9"
authors = ["akd contributors"]
license = "MIT OR Apache-2.0"
edition = "2021"
Expand Down

0 comments on commit fcd665a

Please sign in to comment.