Skip to content

Commit 210af9d

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 210af9d

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed

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: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use crate::{
3434
pub struct LocalDatabase;
3535

3636
unsafe impl Send for LocalDatabase {}
37+
3738
unsafe impl Sync for LocalDatabase {}
3839

3940
mockall::mock! {
@@ -130,6 +131,39 @@ fn setup_mocked_db(db: &mut MockLocalDatabase, test_db: &AsyncInMemoryDatabase)
130131
});
131132
}
132133

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

0 commit comments

Comments
 (0)