Eliminate proposal "filtering" in favor of throwing hard errors

awslabs · Jan 23, 2025 · 92feb7e · 92feb7e
1 parent c198d38
commit 92feb7e
Show file tree

Hide file tree

Showing 16 changed files with 281 additions and 2,197 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -16,6 +16,10 @@ members = [
     "mls-rs-provider-sqlite",
     "mls-rs-codec",
     "mls-rs-codec-derive",
+]
+
+exclude = [
+    "mls-rs-uniffi",
     "mls-rs-uniffi/uniffi-bindgen",
 ]
 

diff --git a/mls-rs-uniffi/src/lib.rs b/mls-rs-uniffi/src/lib.rs
@@ -206,7 +206,6 @@ impl From<mls_rs::group::proposal::Proposal> for Proposal {
 pub enum CommitEffect {
     NewEpoch {
         applied_proposals: Vec<Arc<Proposal>>,
-        unused_proposals: Vec<Arc<Proposal>>,
     },
     ReInit,
     Removed,
@@ -221,12 +220,7 @@ impl From<mls_rs::group::CommitEffect> for CommitEffect {
                     .into_iter()
                     .map(|p| Arc::new(p.proposal.into()))
                     .collect(),
-                unused_proposals: new_epoch
-                    .unused_proposals
-                    .into_iter()
-                    .map(|p| Arc::new(p.proposal.into()))
-                    .collect(),
-            },
+                            },
             group::CommitEffect::Removed {
                 new_epoch: _,
                 remover: _,
@@ -382,10 +376,7 @@ impl Client {
     /// See [`mls_rs::Client::generate_key_package_message`] for
     /// details.
     pub async fn generate_key_package_message(&self) -> Result<Message, Error> {
-        let message = self
-            .inner
-            .generate_key_package()
-            .await?;
+        let message = self.inner.generate_key_package().await?;
         Ok(message.into())
     }
 
@@ -499,8 +490,6 @@ pub struct CommitOutput {
     /// A group info that can be provided to new members in order to
     /// enable external commit functionality.
     pub group_info: Option<Arc<Message>>,
-    // TODO(mgeisler): decide if we should expose unused_proposals()
-    // as well.
 }
 
 impl TryFrom<mls_rs::group::CommitOutput> for CommitOutput {

diff --git a/mls-rs/src/external_client/group.rs b/mls-rs/src/external_client/group.rs
@@ -451,8 +451,7 @@ impl<C: ExternalClientConfig + Clone> ExternalGroup<C> {
     /// Issue an external proposal.
     ///
     /// This function is useful for reissuing external proposals that
-    /// are returned in [crate::group::NewEpoch::unused_proposals]
-    /// after a commit is processed.
+    /// were not consumed by a commit.
     #[cfg(feature = "by_ref_proposal")]
     #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)]
     pub async fn propose(
@@ -1405,7 +1404,10 @@ mod tests {
         )
         .await;
 
-        let proposal = alice.propose_update().await.unwrap();
+        let proposal = alice
+            .propose_group_context_extensions(Default::default())
+            .await
+            .unwrap();
 
         let commit_output = alice.commit(vec![]).await.unwrap();
 

diff --git a/mls-rs/src/group/commit/builder.rs b/mls-rs/src/group/commit/builder.rs
@@ -10,8 +10,6 @@ use itertools::Itertools;
 use mls_rs_codec::{MlsDecode, MlsEncode, MlsSize};
 use mls_rs_core::crypto::SignatureSecretKey;
 
-#[cfg(feature = "by_ref_proposal")]
-use crate::group::proposal_filter::ProposalInfo;
 use crate::group::proposal_filter::{ProposalBundle, ProposalSource};
 use crate::group::{EncryptionMode, RemoveProposal};
 use crate::{
@@ -57,9 +55,6 @@ use crate::group::{
     PendingCommitSnapshot, Welcome,
 };
 
-#[cfg(feature = "by_ref_proposal")]
-use crate::group::proposal_filter::CommitDirection;
-
 #[cfg(feature = "custom_proposal")]
 use crate::group::proposal::CustomProposal;
 
@@ -122,9 +117,6 @@ pub struct CommitOutput {
     /// functionality. This value is set if [`MlsRules::commit_options`] returns
     /// `allow_external_commit` set to true.
     pub external_commit_group_info: Option<MlsMessage>,
-    /// Proposals that were received in the prior epoch but not included in the following commit.
-    #[cfg(feature = "by_ref_proposal")]
-    pub unused_proposals: Vec<ProposalInfo<Proposal>>,
     /// Indicator that the commit contains a path update
     pub contains_update_path: bool,
 }
@@ -158,12 +150,6 @@ impl CommitOutput {
     pub fn external_commit_group_info(&self) -> Option<&MlsMessage> {
         self.external_commit_group_info.as_ref()
     }
-
-    /// Proposals that were received in the prior epoch but not included in the following commit.
-    #[cfg(all(feature = "ffi", feature = "by_ref_proposal"))]
-    pub fn unused_proposals(&self) -> &[ProposalInfo<Proposal>] {
-        &self.unused_proposals
-    }
 }
 
 /// Options controlling commit generation
@@ -369,16 +355,12 @@ where
         self.with_proposal(Proposal::Custom(proposal))
     }
 
-    /// Insert a proposal that was previously constructed such as when a
-    /// proposal is returned from
-    /// [`NewEpoch::unused_proposals`](super::NewEpoch::unused_proposals).
+    /// Insert a proposal that was previously not consumed by a commit
     pub fn raw_proposal(self, proposal: Proposal) -> Self {
         self.with_proposal(proposal)
     }
 
-    /// Insert proposals that were previously constructed such as when a
-    /// proposal is returned from
-    /// [`NewEpoch::unused_proposals`](super::NewEpoch::unused_proposals).
+    /// Insert proposals that were previously not consumed by a commit
     pub fn raw_proposals(self, proposals: Vec<Proposal>) -> Self {
         proposals.into_iter().fold(self, |b, p| b.with_proposal(p))
     }
@@ -684,8 +666,6 @@ where
                 &self.config.identity_provider(),
                 &self.cipher_suite_provider,
                 time,
-                #[cfg(feature = "by_ref_proposal")]
-                CommitDirection::Send,
                 #[cfg(feature = "psk")]
                 &psks
                     .iter()
@@ -1025,8 +1005,6 @@ where
             ratchet_tree,
             external_commit_group_info,
             contains_update_path: perform_path_update,
-            #[cfg(feature = "by_ref_proposal")]
-            unused_proposals: provisional_state.unused_proposals,
         };
 
         Ok((output, pending_commit))

diff --git a/mls-rs/src/group/commit/processor.rs b/mls-rs/src/group/commit/processor.rs
@@ -24,9 +24,6 @@ use crate::{
     Group,
 };
 
-#[cfg(feature = "by_ref_proposal")]
-use crate::group::proposal_filter::CommitDirection;
-
 #[cfg(feature = "psk")]
 use mls_rs_core::psk::{ExternalPskId, PreSharedKey};
 
@@ -136,8 +133,6 @@ pub(crate) async fn process_commit<'a, P: MessageProcessor<'a>>(
             &id_provider,
             &cs_provider,
             commit_processor.time_sent,
-            #[cfg(feature = "by_ref_proposal")]
-            CommitDirection::Receive,
             #[cfg(feature = "psk")]
             &commit_processor
                 .psks

diff --git a/mls-rs/src/group/framing.rs b/mls-rs/src/group/framing.rs
@@ -562,8 +562,7 @@ impl MlsMessage {
         kp.to_reference(cipher_suite).await.map(Some)
     }
 
-    /// If this is a plaintext proposal, return the proposal reference that can be matched e.g. with
-    /// [`NewEpoch::unused_proposals`](super::NewEpoch::unused_proposals).
+    /// If this is a plaintext proposal, return the proposal reference
     #[cfg(feature = "by_ref_proposal")]
     #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)]
     pub async fn into_proposal_reference<C: CipherSuiteProvider>(

diff --git a/mls-rs/src/group/message_processor.rs b/mls-rs/src/group/message_processor.rs
@@ -58,7 +58,6 @@ pub(crate) struct ProvisionalState {
     pub(crate) group_context: GroupContext,
     pub(crate) external_init_index: Option<LeafIndex>,
     pub(crate) indexes_of_added_kpkgs: Vec<LeafIndex>,
-    pub(crate) unused_proposals: Vec<ProposalInfo<Proposal>>,
 }
 
 //By default, the path field of a Commit MUST be populated. The path field MAY be omitted if
@@ -92,15 +91,13 @@ pub struct NewEpoch {
     pub epoch: u64,
     pub prior_state: GroupState,
     pub applied_proposals: Vec<ProposalInfo<Proposal>>,
-    pub unused_proposals: Vec<ProposalInfo<Proposal>>,
 }
 
 impl NewEpoch {
     pub(crate) fn new(prior_state: GroupState, provisional_state: &ProvisionalState) -> NewEpoch {
         NewEpoch {
             epoch: provisional_state.group_context.epoch,
             prior_state,
-            unused_proposals: provisional_state.unused_proposals.clone(),
             applied_proposals: provisional_state
                 .applied_proposals
                 .clone()
@@ -124,10 +121,6 @@ impl NewEpoch {
     pub fn applied_proposals(&self) -> &[ProposalInfo<Proposal>] {
         &self.applied_proposals
     }
-
-    pub fn unused_proposals(&self) -> &[ProposalInfo<Proposal>] {
-        &self.unused_proposals
-    }
 }
 
 #[cfg_attr(
@@ -878,7 +871,6 @@ mod tests {
                 confirmation_tag: Default::default(),
             },
             applied_proposals: vec![],
-            unused_proposals: vec![],
         };
 
         let effects = vec![