From f74176f5a0238bc641f201bff4c557591113956c Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Wed, 8 May 2024 21:17:22 -0700 Subject: [PATCH 1/6] make storage id public --- basic_credential/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/basic_credential/src/lib.rs b/basic_credential/src/lib.rs index 0b59cbb18..350ea4fc6 100644 --- a/basic_credential/src/lib.rs +++ b/basic_credential/src/lib.rs @@ -109,7 +109,7 @@ impl SignatureKeyPair { } } - fn id(&self) -> StorageId { + pub fn id(&self) -> StorageId { StorageId { value: id(&self.public, self.signature_scheme), } @@ -161,10 +161,16 @@ impl SignatureKeyPair { // Storage #[derive(Debug, Serialize, Deserialize)] -struct StorageId { +pub struct StorageId { value: Vec, } +impl From> for StorageId { + fn from(vec: Vec) -> Self { + StorageId { value: vec } + } +} + // Implement key traits for the storage id impl storage::Key for StorageId {} impl storage::traits::SignaturePublicKey for StorageId {} From 25207a2b3255a904ccbe99dd9f7bd77d4fecb495 Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Fri, 10 May 2024 15:43:10 +0200 Subject: [PATCH 2/6] Return an empty vector in storage.aad when it's not found (#1577) --- memory_storage/src/lib.rs | 7 ++++++- traits/src/storage.rs | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/memory_storage/src/lib.rs b/memory_storage/src/lib.rs index dc06cb8e4..67ace3414 100644 --- a/memory_storage/src/lib.rs +++ b/memory_storage/src/lib.rs @@ -870,7 +870,12 @@ impl StorageProvider for MemoryStorage { group_id: &GroupId, ) -> Result, Self::Error> { let key = serde_json::to_vec(group_id)?; - self.read_list(AAD_LABEL, &key) + self.read::>(AAD_LABEL, &key) + .map(|v| { + // When we didn't find the value, we return an empty vector as + // required by the trait. + v.unwrap_or_default() + }) } fn write_aad>( diff --git a/traits/src/storage.rs b/traits/src/storage.rs index 9958b6be4..1f141429b 100644 --- a/traits/src/storage.rs +++ b/traits/src/storage.rs @@ -645,3 +645,5 @@ pub mod traits { pub trait ProposalRef: Entity + Key {} } // ANCHOR_END: traits + +impl Entity for Vec {} From daa00c6a586680a51143dfb03fce040835b005e5 Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Fri, 10 May 2024 16:09:04 +0200 Subject: [PATCH 3/6] Remove PartialEq requirement from associated Error type (#1578) --- openmls/src/storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmls/src/storage.rs b/openmls/src/storage.rs index 854a7a666..585ee4471 100644 --- a/openmls/src/storage.rs +++ b/openmls/src/storage.rs @@ -46,7 +46,7 @@ pub trait OpenMlsProvider: } impl< - Error: std::error::Error + PartialEq, + Error: std::error::Error, SP: StorageProvider, OP: openmls_traits::OpenMlsProvider, > OpenMlsProvider for OP From b7035faba0ce992e479e9c9941cdf884fdeb72c4 Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Fri, 10 May 2024 16:47:10 +0200 Subject: [PATCH 4/6] update storage proposal api (#1579) The memory storage clear proposal function is working now. This required a small change to the storage API such that we know the ProposalRef type. There's also a test for the proposals in the storage crate now. --- memory_storage/src/lib.rs | 19 +++++-- memory_storage/src/test_store.rs | 5 +- memory_storage/tests/proposals.rs | 81 ++++++++++++++++++++++++++++++ openmls/src/group/mls_group/mod.rs | 4 +- traits/src/storage.rs | 5 +- 5 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 memory_storage/tests/proposals.rs diff --git a/memory_storage/src/lib.rs b/memory_storage/src/lib.rs index 67ace3414..4290d5723 100644 --- a/memory_storage/src/lib.rs +++ b/memory_storage/src/lib.rs @@ -795,16 +795,25 @@ impl StorageProvider for MemoryStorage { self.delete::(EPOCH_KEY_PAIRS_LABEL, &key) } - fn clear_proposal_queue>( + fn clear_proposal_queue< + GroupId: traits::GroupId, + ProposalRef: traits::ProposalRef, + >( &self, group_id: &GroupId, ) -> Result<(), Self::Error> { + // Get all proposal refs for this group. + let proposal_refs: Vec = + self.read_list(PROPOSAL_QUEUE_REFS_LABEL, &serde_json::to_vec(group_id)?)?; let mut values = self.values.write().unwrap(); + for proposal_ref in proposal_refs { + // Delete all proposals. + let key = serde_json::to_vec(&(group_id, proposal_ref))?; + values.remove(&key); + } - let key = build_key::(QUEUED_PROPOSAL_LABEL, group_id); - - // XXX #1566: also remove the proposal refs. can't be done now because they are stored in a - // non-recoverable way + // Delete the proposal refs from the store. + let key = build_key::(PROPOSAL_QUEUE_REFS_LABEL, group_id); values.remove(&key); Ok(()) diff --git a/memory_storage/src/test_store.rs b/memory_storage/src/test_store.rs index a78d279da..f8c7b31bf 100644 --- a/memory_storage/src/test_store.rs +++ b/memory_storage/src/test_store.rs @@ -438,7 +438,10 @@ impl StorageProvider for MemoryStorage { todo!() } - fn clear_proposal_queue>( + fn clear_proposal_queue< + GroupId: traits::GroupId, + ProposalRef: traits::ProposalRef, + >( &self, _group_id: &GroupId, ) -> Result<(), Self::Error> { diff --git a/memory_storage/tests/proposals.rs b/memory_storage/tests/proposals.rs new file mode 100644 index 000000000..f0d888be0 --- /dev/null +++ b/memory_storage/tests/proposals.rs @@ -0,0 +1,81 @@ +use openmls_memory_storage::MemoryStorage; +use openmls_traits::storage::{ + traits::{self}, + Entity, Key, StorageProvider, CURRENT_VERSION, +}; +use serde::{Deserialize, Serialize}; + +// Test types +#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] +struct TestGroupId(Vec); +impl traits::GroupId for TestGroupId {} +impl Key for TestGroupId {} + +#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone, Copy)] +struct ProposalRef(usize); +impl traits::ProposalRef for ProposalRef {} +impl Key for ProposalRef {} +impl Entity for ProposalRef {} + +#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] +struct Proposal(Vec); +impl traits::QueuedProposal for Proposal {} +impl Entity for Proposal {} + +/// Write and read some proposals +#[test] +fn read_write_delete() { + let group_id = TestGroupId(b"TestGroupId".to_vec()); + let proposals = (0..10) + .map(|i| Proposal(format!("TestProposal{i}").as_bytes().to_vec())) + .collect::>(); + let storage = MemoryStorage::default(); + + // Store proposals + for (i, proposal) in proposals.iter().enumerate() { + storage + .queue_proposal(&group_id, &ProposalRef(i), proposal) + .unwrap(); + } + + // Read proposal refs + let proposal_refs_read: Vec = storage.queued_proposal_refs(&group_id).unwrap(); + assert_eq!( + (0..10).map(|i| ProposalRef(i)).collect::>(), + proposal_refs_read + ); + + // Read proposals + let proposals_read: Vec<(ProposalRef, Proposal)> = storage.queued_proposals(&group_id).unwrap(); + let proposals_expected: Vec<(ProposalRef, Proposal)> = (0..10) + .map(|i| ProposalRef(i)) + .zip(proposals.clone().into_iter()) + .collect(); + assert_eq!(proposals_expected, proposals_read); + + // Remove proposal 5 + storage.remove_proposal(&group_id, &ProposalRef(5)).unwrap(); + + let proposal_refs_read: Vec = storage.queued_proposal_refs(&group_id).unwrap(); + let mut expected = (0..10).map(|i| ProposalRef(i)).collect::>(); + expected.remove(5); + assert_eq!(expected, proposal_refs_read); + + let proposals_read: Vec<(ProposalRef, Proposal)> = storage.queued_proposals(&group_id).unwrap(); + let mut proposals_expected: Vec<(ProposalRef, Proposal)> = (0..10) + .map(|i| ProposalRef(i)) + .zip(proposals.clone().into_iter()) + .collect(); + proposals_expected.remove(5); + assert_eq!(proposals_expected, proposals_read); + + // Clear all proposals + storage + .clear_proposal_queue::(&group_id) + .unwrap(); + let proposal_refs_read: Vec = storage.queued_proposal_refs(&group_id).unwrap(); + assert!(proposal_refs_read.is_empty()); + + let proposals_read: Vec<(ProposalRef, Proposal)> = storage.queued_proposals(&group_id).unwrap(); + assert!(proposals_read.is_empty()); +} diff --git a/openmls/src/group/mls_group/mod.rs b/openmls/src/group/mls_group/mod.rs index a22d5d61d..501dc9c0d 100644 --- a/openmls/src/group/mls_group/mod.rs +++ b/openmls/src/group/mls_group/mod.rs @@ -312,7 +312,7 @@ impl MlsGroup { self.proposal_store.empty(); // Clear proposals in storage - storage.clear_proposal_queue(self.group_id())?; + storage.clear_proposal_queue::(self.group_id())?; } Ok(()) @@ -371,7 +371,7 @@ impl MlsGroup { ) -> Result<(), StorageProvider::Error> { self.group.delete(storage)?; storage.delete_group_config(self.group_id())?; - storage.clear_proposal_queue(self.group_id())?; + storage.clear_proposal_queue::(self.group_id())?; storage.delete_own_leaf_nodes(self.group_id())?; storage.delete_aad(self.group_id())?; storage.delete_group_state(self.group_id())?; diff --git a/traits/src/storage.rs b/traits/src/storage.rs index 1f141429b..f0b138fe6 100644 --- a/traits/src/storage.rs +++ b/traits/src/storage.rs @@ -531,7 +531,10 @@ pub trait StorageProvider { ) -> Result<(), Self::Error>; /// Clear the proposal queue for the grou pwith the given id. - fn clear_proposal_queue>( + fn clear_proposal_queue< + GroupId: traits::GroupId, + ProposalRef: traits::ProposalRef, + >( &self, group_id: &GroupId, ) -> Result<(), Self::Error>; From c359be1355a32ae6742f8c27ed05b92d89e69f45 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Thu, 16 May 2024 12:06:40 -0700 Subject: [PATCH 5/6] write to storage on update --- openmls/src/group/errors.rs | 2 ++ openmls/src/group/mls_group/proposal.rs | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 50e2e8317..5813434c2 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -519,6 +519,8 @@ pub enum CreateGroupContextExtProposalError { /// See [`CreateCommitError`] for more details. #[error(transparent)] CreateCommitError(#[from] CreateCommitError), + #[error("Error writing updated group data to storage.")] + StorageError(StorageError), } /// Error merging a commit. diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index b76a46bd3..be6df8e93 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -420,6 +420,11 @@ impl MlsGroup { create_commit_result.staged_commit, ))); + provider + .storage() + .write_group_state(self.group_id(), &self.group_state) + .map_err(CreateGroupContextExtProposalError::StorageError)?; + Ok(( mls_messages, create_commit_result From 6c908234c8b6da30a4d292d4fcb27a69c981db71 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Thu, 16 May 2024 12:13:43 -0700 Subject: [PATCH 6/6] add error documentation --- openmls/src/group/errors.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 5813434c2..8dc1332e5 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -519,6 +519,7 @@ pub enum CreateGroupContextExtProposalError { /// See [`CreateCommitError`] for more details. #[error(transparent)] CreateCommitError(#[from] CreateCommitError), + /// Error writing updated group to storage. #[error("Error writing updated group data to storage.")] StorageError(StorageError), }