Adding and removing group members

bindings_ffi/src/mls.rs

@@ -213,7 +213,7 @@ impl FfiConversations {
         let convo = self.inner_client.create_group(group_permissions)?;
         if !account_addresses.is_empty() {
             convo
-                .add_members(account_addresses, &self.inner_client)
+                .add_members(&self.inner_client, account_addresses)
                 .await?;
         }
         let out = Arc::new(FfiGroup {
@@ -431,7 +431,7 @@ impl FfiGroup {
         );
 
         group
-            .add_members(account_addresses, &self.inner_client)
+            .add_members(&self.inner_client, account_addresses)
             .await?;
 
         Ok(())
@@ -445,7 +445,7 @@ impl FfiGroup {
         );
 
         group
-            .remove_members(account_addresses, &self.inner_client)
+            .remove_members(&self.inner_client, account_addresses)
             .await?;
 
         Ok(())
@@ -663,15 +663,13 @@ impl FfiGroupMetadata {
     }
 }
 
-
 #[derive(uniffi::Object)]
 pub struct FfiGroupPermissions {
     inner: Arc<GroupMutablePermissions>,
 }
 
 #[uniffi::export]
 impl FfiGroupPermissions {
-
     pub fn policy_type(&self) -> Result<GroupPermissions, GenericError> {
         Ok(self.inner.preconfigured_policy()?.into())
     }

examples/cli/cli-client.rs

@@ -268,7 +268,7 @@ async fn main() {
                 .expect("failed to get group");
 
             group
-                .add_members(account_addresses.clone(), &client)
+                .add_members(&client, account_addresses.clone())
                 .await
                 .expect("failed to add member");
 
@@ -290,7 +290,7 @@ async fn main() {
                 .expect("failed to get group");
 
             group
-                .remove_members(account_addresses.clone(), &client)
+                .remove_members(&client, account_addresses.clone())
                 .await
                 .expect("failed to add member");
 

xmtp_mls/src/client.rs

@@ -680,7 +680,7 @@ mod tests {
 
         let alice_bob_group = alice.create_group(None).unwrap();
         alice_bob_group
-            .add_members_by_installation_id(vec![bob.installation_public_key()], &alice)
+            .add_members_by_inbox_id(&alice, vec![bob.inbox_id()])
             .await
             .unwrap();
 
@@ -753,14 +753,14 @@ mod tests {
         // Create a group and invite bola
         let amal_group = amal.create_group(None).unwrap();
         amal_group
-            .add_members_by_installation_id(vec![bola.installation_public_key()], &amal)
+            .add_members_by_inbox_id(&amal, vec![bola.inbox_id()])
             .await
             .unwrap();
         assert_eq!(amal_group.members().unwrap().len(), 2);
 
         // Now remove bola
         amal_group
-            .remove_members_by_installation_id(vec![bola.installation_public_key()], &amal)
+            .remove_members_by_inbox_id(&amal, vec![bola.inbox_id()])
             .await
             .unwrap();
         assert_eq!(amal_group.members().unwrap().len(), 1);
@@ -780,7 +780,7 @@ mod tests {
 
         // Add Bola back to the group
         amal_group
-            .add_members_by_installation_id(vec![bola.installation_public_key()], &amal)
+            .add_members_by_inbox_id(&amal, vec![bola.inbox_id()])
             .await
             .unwrap();
         bola.sync_welcomes().await.unwrap();

xmtp_mls/src/groups/group_membership.rs

@@ -26,6 +26,10 @@ impl GroupMembership {
         self.members.get(inbox_id.as_ref())
     }
 
+    pub fn inbox_ids(&self) -> Vec<String> {
+        self.members.keys().cloned().collect()
+    }
+
     pub fn diff<'inbox_id>(
         &'inbox_id self,
         new_group_membership: &'inbox_id Self,

xmtp_mls/src/groups/group_permissions.rs

@@ -437,6 +437,7 @@ pub struct PermissionsChange {
 
 impl PermissionsChange {
     #[cfg(test)]
+    #[allow(dead_code)]
     fn empty_for_testing() -> Self {
         todo!();
         // Self {

xmtp_mls/src/groups/intents.rs

@@ -8,21 +8,18 @@ use prost::{bytes::Bytes, DecodeError, Message};
 use thiserror::Error;
 
 use xmtp_proto::xmtp::mls::database::{
-    add_members_data::{Version as AddMembersVersion, V1 as AddMembersV1},
     addresses_or_installation_ids::AddressesOrInstallationIds as AddressesOrInstallationIdsProto,
     post_commit_action::{
         Installation as InstallationProto, Kind as PostCommitActionKind,
         SendWelcomes as SendWelcomesProto,
     },
-    remove_members_data::{Version as RemoveMembersVersion, V1 as RemoveMembersV1},
     send_message_data::{Version as SendMessageVersion, V1 as SendMessageV1},
     update_group_membership_data::{
         Version as UpdateGroupMembershipVersion, V1 as UpdateGroupMembershipV1,
     },
     update_metadata_data::{Version as UpdateMetadataVersion, V1 as UpdateMetadataV1},
-    AccountAddresses, AddMembersData,
-    AddressesOrInstallationIds as AddressesOrInstallationIdsProtoWrapper, InstallationIds,
-    PostCommitAction as PostCommitActionProto, RemoveMembersData, SendMessageData,
+    AccountAddresses, AddressesOrInstallationIds as AddressesOrInstallationIdsProtoWrapper,
+    InstallationIds, PostCommitAction as PostCommitActionProto, SendMessageData,
     UpdateGroupMembershipData, UpdateMetadataData,
 };
 
@@ -31,7 +28,7 @@ use crate::{
     verified_key_package::{KeyPackageVerificationError, VerifiedKeyPackage},
 };
 
-use super::group_mutable_metadata::MetadataField;
+use super::{group_membership::GroupMembership, group_mutable_metadata::MetadataField};
 
 #[derive(Debug, Error)]
 pub enum IntentError {
@@ -144,93 +141,6 @@ impl From<Vec<Vec<u8>>> for AddressesOrInstallationIds {
     }
 }
 
-#[derive(Debug, Clone)]
-pub struct AddMembersIntentData {
-    pub address_or_id: AddressesOrInstallationIds,
-}
-
-impl AddMembersIntentData {
-    pub fn new(address_or_id: AddressesOrInstallationIds) -> Self {
-        Self { address_or_id }
-    }
-
-    pub(crate) fn to_bytes(&self) -> Result<Vec<u8>, IntentError> {
-        let mut buf = Vec::new();
-        AddMembersData {
-            version: Some(AddMembersVersion::V1(AddMembersV1 {
-                addresses_or_installation_ids: Some(self.address_or_id.clone().into()),
-            })),
-        }
-        .encode(&mut buf)
-        .expect("encode error");
-
-        Ok(buf)
-    }
-
-    pub(crate) fn from_bytes(data: &[u8]) -> Result<Self, IntentError> {
-        let msg = AddMembersData::decode(data)?;
-        let address_or_id = match msg.version {
-            Some(AddMembersVersion::V1(v1)) => v1
-                .addresses_or_installation_ids
-                .ok_or(IntentError::Generic("missing payload".to_string()))?,
-            None => return Err(IntentError::Generic("missing payload".to_string())),
-        };
-
-        Ok(Self::new(address_or_id.try_into()?))
-    }
-}
-
-impl TryFrom<AddMembersIntentData> for Vec<u8> {
-    type Error = IntentError;
-
-    fn try_from(intent: AddMembersIntentData) -> Result<Self, Self::Error> {
-        intent.to_bytes()
-    }
-}
-
-#[derive(Debug, Clone)]
-pub struct RemoveMembersIntentData {
-    pub address_or_id: AddressesOrInstallationIds,
-}
-
-impl RemoveMembersIntentData {
-    pub fn new(address_or_id: AddressesOrInstallationIds) -> Self {
-        Self { address_or_id }
-    }
-
-    pub(crate) fn to_bytes(&self) -> Vec<u8> {
-        let mut buf = Vec::new();
-
-        RemoveMembersData {
-            version: Some(RemoveMembersVersion::V1(RemoveMembersV1 {
-                addresses_or_installation_ids: Some(self.address_or_id.clone().into()),
-            })),
-        }
-        .encode(&mut buf)
-        .expect("encode error");
-
-        buf
-    }
-
-    pub(crate) fn from_bytes(data: &[u8]) -> Result<Self, IntentError> {
-        let msg = RemoveMembersData::decode(data)?;
-        let address_or_id = match msg.version {
-            Some(RemoveMembersVersion::V1(v1)) => v1
-                .addresses_or_installation_ids
-                .ok_or(IntentError::Generic("missing payload".to_string()))?,
-            None => return Err(IntentError::Generic("missing payload".to_string())),
-        };
-
-        Ok(Self::new(address_or_id.try_into()?))
-    }
-}
-
-impl From<RemoveMembersIntentData> for Vec<u8> {
-    fn from(intent: RemoveMembersIntentData) -> Self {
-        intent.to_bytes()
-    }
-}
-
 #[derive(Debug, Clone)]
 pub struct UpdateMetadataIntentData {
     pub field_name: String,
@@ -289,6 +199,7 @@ impl TryFrom<Vec<u8>> for UpdateMetadataIntentData {
     }
 }
 
+#[derive(Debug, Clone)]
 pub(crate) struct UpdateGroupMembershipIntentData {
     pub membership_updates: HashMap<String, u64>,
     pub removed_members: Vec<String>,
@@ -301,6 +212,24 @@ impl UpdateGroupMembershipIntentData {
             removed_members,
         }
     }
+
+    pub fn is_empty(&self) -> bool {
+        self.membership_updates.is_empty() && self.removed_members.is_empty()
+    }
+
+    pub fn apply_to_group_membership(&self, group_membership: &GroupMembership) -> GroupMembership {
+        let mut new_membership = group_membership.clone();
+
+        for (inbox_id, sequence_id) in self.membership_updates.iter() {
+            new_membership.add(inbox_id.clone(), *sequence_id);
+        }
+
+        for inbox_id in self.removed_members.iter() {
+            new_membership.remove(inbox_id)
+        }
+
+        new_membership
+    }
 }
 
 impl From<UpdateGroupMembershipIntentData> for Vec<u8> {
@@ -335,6 +264,21 @@ impl TryFrom<Vec<u8>> for UpdateGroupMembershipIntentData {
     }
 }
 
+impl TryFrom<&Vec<u8>> for UpdateGroupMembershipIntentData {
+    type Error = IntentError;
+
+    fn try_from(data: &Vec<u8>) -> Result<Self, Self::Error> {
+        if let UpdateGroupMembershipData {
+            version: Some(UpdateGroupMembershipVersion::V1(v1)),
+        } = UpdateGroupMembershipData::decode(data.as_slice())?
+        {
+            Ok(Self::new(v1.membership_updates, v1.removed_members))
+        } else {
+            Err(IntentError::Generic("missing payload".to_string()))
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum PostCommitAction {
     SendWelcomes(SendWelcomesAction),
@@ -450,10 +394,7 @@ impl From<Vec<u8>> for PostCommitAction {
 
 #[cfg(test)]
 mod tests {
-    use xmtp_cryptography::utils::generate_local_wallet;
-
     use super::*;
-    use crate::InboxOwner;
 
     #[test]
     fn test_serialize_send_message() {
@@ -466,15 +407,22 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_serialize_add_members() {
-        let wallet = generate_local_wallet();
-        let account_address = wallet.get_address();
+    async fn test_serialize_update_membership() {
+        let mut membership_updates = HashMap::new();
+        membership_updates.insert("foo".to_string(), 123);
+
+        let intent =
+            UpdateGroupMembershipIntentData::new(membership_updates, vec!["bar".to_string()]);
 
-        let intent = AddMembersIntentData::new(vec![account_address.clone()].into());
         let as_bytes: Vec<u8> = intent.clone().try_into().unwrap();
-        let restored_intent = AddMembersIntentData::from_bytes(as_bytes.as_slice()).unwrap();
+        let restored_intent: UpdateGroupMembershipIntentData = as_bytes.try_into().unwrap();
+
+        assert_eq!(
+            intent.membership_updates,
+            restored_intent.membership_updates
+        );
 
-        assert_eq!(intent.address_or_id, restored_intent.address_or_id);
+        assert_eq!(intent.removed_members, restored_intent.removed_members);
     }
 
     #[tokio::test]