Skip to content

Commit 9b9da58

Browse files
author
Dillon George
committed
Prevent Any Storage Errors from Clobbering aZKS Entry
**Current** -- Prior to this patch, AKD would incorrectly assume that any error during initialization of a `Directory` should result in an `aZKS` being created and stored in the underlying database implementation. The problem here is that this behavior happened on every error, even those which do not definitively indicate that the read for an `aZKS` passed but a record was not found. For example, if a connection issue occurs when connecting to the storage layer, the current implementation could wrongly overwrite an existing `aZKS`. In such a scenario, the original read for the `aZKS` fails during `Directory` initialization, but a subsequent write succeeds. This can result in clobbered records and unexpected behavior. **Proposed/New** -- With this change, we update the initialization logic for the `Directory` type to only create an `aZKS` if the returned `Err` indicates that the read was successful, but an `aZKS` was not found. In all other instances, we propagate the `Err` back to the caller instead of swallowing it and wrongly creating an `aZKS`.
1 parent 0094631 commit 9b9da58

3 files changed

Lines changed: 43 additions & 6 deletions

File tree

akd/src/directory.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,12 @@ where
6565
pub async fn new(storage: StorageManager<S>, vrf: V) -> Result<Self, AkdError> {
6666
let azks = Directory::<TC, S, V>::get_azks_from_storage(&storage, false).await;
6767

68-
if azks.is_err() {
69-
// generate a new azks if one is not found
70-
let azks = Azks::new::<TC, _>(&storage).await?;
71-
// store it
72-
storage.set(DbRecord::Azks(azks.clone())).await?;
68+
if let Err(AkdError::Storage(StorageError::NotFound(_))) = azks {
69+
// generate + store a new azks only if one is not found
70+
let new_azks = Azks::new::<TC, _>(&storage).await?;
71+
storage.set(DbRecord::Azks(new_azks.clone())).await?;
72+
} else {
73+
let _res = azks?;
7374
}
7475

7576
Ok(Directory {
@@ -674,6 +675,7 @@ where
674675
.get::<Azks>(&crate::append_only_zks::DEFAULT_AZKS_KEY)
675676
.await?
676677
};
678+
677679
match got {
678680
DbRecord::Azks(azks) => Ok(azks),
679681
_ => {

akd/src/storage/manager/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ impl<Db: Database> StorageManager<Db> {
181181
let started = self.transaction.begin_transaction();
182182

183183
// disable the cache cleaning since we're in a write transaction
184-
// and will want to keep cache'd objects for the life of the transaction
184+
// and will want to keep cached objects for the life of the transaction
185185
if let Some(cache) = &self.cache {
186186
cache.disable_clean();
187187
}

akd/src/tests.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::{errors::DirectoryError, test_config};
1313
use akd_core::{configuration::Configuration, hash::DIGEST_BYTES};
1414
use rand::{rngs::StdRng, SeedableRng};
1515

16+
use crate::errors::AkdError::Storage;
1617
use crate::{
1718
auditor::{audit_verify, verify_consecutive_append_only},
1819
client::{key_history_verify, lookup_verify},
@@ -34,6 +35,7 @@ use crate::{
3435
pub struct LocalDatabase;
3536

3637
unsafe impl Send for LocalDatabase {}
38+
3739
unsafe impl Sync for LocalDatabase {}
3840

3941
mockall::mock! {
@@ -130,6 +132,39 @@ fn setup_mocked_db(db: &mut MockLocalDatabase, test_db: &AsyncInMemoryDatabase)
130132
});
131133
}
132134

135+
// A test to ensure that any database error at the time a Directory is created
136+
// does not automatically attempt to create a new aZKS. Only aZKS not found errors
137+
// should assume that a successful read happened and no aZKS exists.
138+
test_config!(test_directory_azks_bootstrapping);
139+
async fn test_directory_azks_bootstrapping<TC: Configuration>() -> Result<(), AkdError> {
140+
let vrf = HardCodedAkdVRF {};
141+
142+
// Verify that a Storage error results in an error when attempting to create the Directory
143+
let mut mock_db = MockLocalDatabase {
144+
..Default::default()
145+
};
146+
mock_db
147+
.expect_get::<Azks>()
148+
.returning(|_| Err(StorageError::Connection("Fire!".to_string())));
149+
let storage = StorageManager::new_no_cache(mock_db);
150+
151+
let maybe_akd = Directory::<TC, _, _>::new(storage, vrf.clone()).await;
152+
assert!(maybe_akd.is_err());
153+
154+
// Verify that an aZKS not found error results in one being created with the Directory
155+
let mut mock_db = MockLocalDatabase {
156+
..Default::default()
157+
};
158+
let test_db = AsyncInMemoryDatabase::new();
159+
setup_mocked_db(&mut mock_db, &test_db);
160+
let storage = StorageManager::new_no_cache(mock_db);
161+
162+
let maybe_akd = Directory::<TC, _, _>::new(storage, vrf).await;
163+
assert!(maybe_akd.is_ok());
164+
165+
Ok(())
166+
}
167+
133168
// A simple test to ensure that the empty tree hashes to the correct value
134169
test_config!(test_empty_tree_root_hash);
135170
async fn test_empty_tree_root_hash<TC: Configuration>() -> Result<(), AkdError> {

0 commit comments

Comments
 (0)