self removal commit side

xmtp_mls/src/groups/mod.rs

@@ -666,6 +666,15 @@ impl MlsGroup {
             .await
     }
 
+    pub async fn remove_self<ApiClient: XmtpApi>(
+        &self,
+        client: &Client<ApiClient>,
+    ) -> Result<(), GroupError> {
+        // get list of installations
+        // make membership update intent 
+        todo!()
+    }
+
     pub async fn remove_members<ApiClient: XmtpApi>(
         &self,
         client: &Client<ApiClient>,

xmtp_mls/src/groups/sync.rs

@@ -1218,13 +1218,13 @@ async fn apply_update_group_membership_intent<ApiClient: XmtpApi>(
     let mut new_installations: Vec<Installation> = vec![];
     let mut new_key_packages: Vec<KeyPackage> = vec![];
 
-    if !installation_diff.added_installations.is_empty() {
+    if !installation_diff.added_installations().is_empty() {
         let my_installation_id = &client.installation_public_key();
         // Go to the network and load the key packages for any new installation
         let key_packages = client
             .get_key_packages_for_installation_ids(
                 installation_diff
-                    .added_installations
+                    .added_installations()
                     .into_iter()
                     .filter(|installation| my_installation_id.ne(installation))
                     .collect(),
@@ -1238,8 +1238,24 @@ async fn apply_update_group_membership_intent<ApiClient: XmtpApi>(
         }
     }
 
+    // for everyone else OK
+    // if removed installations includes ourselves need to modify
+    // if !installations_diff.is
+    // make sure not to send a commit to remove me and my wallet ID
+    // if removing myself, cannot do both things (update membership & remove my client b/c fail)
+    // if any leaf nodes have same identity as myself, cannot remove them
+    let own_inbox_id = client.identity().inbox_id();
+
+    let removed_installations = installation_diff
+        .removed_installations
+        .iter()
+        .cloned()
+        .filter(|i| &i.inbox_id != own_inbox_id)
+        .map(|i| i.installation)
+        .collect::<HashSet<Vec<u8>>>();
+
     let leaf_nodes_to_remove: Vec<LeafNodeIndex> =
-        get_removed_leaf_nodes(openmls_group, &installation_diff.removed_installations);
+        get_removed_leaf_nodes(openmls_group, &removed_installations);
 
     if leaf_nodes_to_remove.is_empty()
         && new_key_packages.is_empty()
@@ -1279,6 +1295,8 @@ async fn apply_update_group_membership_intent<ApiClient: XmtpApi>(
     }))
 }
 
+// Filter out own identity from leaf nodes to return right list
+// for self-removes -- i.e to send the commit to the group
 fn get_removed_leaf_nodes(
     openmls_group: &mut OpenMlsGroup,
     removed_installations: &HashSet<Vec<u8>>,

xmtp_mls/src/groups/validated_commit.rs

@@ -514,7 +514,9 @@ fn expected_diff_matches_commit(
     let unknown_adds = added_installations
         .into_iter()
         .filter(|installation_id| {
-            !expected_diff.added_installations.contains(installation_id)
+            !expected_diff
+                .added_installations()
+                .contains(installation_id)
                 && !existing_installation_ids.contains(installation_id)
         })
         .collect::<Vec<Vec<u8>>>();
@@ -524,10 +526,10 @@ fn expected_diff_matches_commit(
         ));
     }
 
-    if removed_installations.ne(&expected_diff.removed_installations) {
+    if removed_installations.ne(&expected_diff.removed_installations()) {
         return Err(CommitValidationError::UnexpectedInstallationsRemoved(
             removed_installations
-                .difference(&expected_diff.removed_installations)
+                .difference(&expected_diff.removed_installations())
                 .cloned()
                 .collect::<Vec<Vec<u8>>>(),
         ));

xmtp_mls/src/identity_updates.rs

@@ -37,8 +37,24 @@ pub enum IdentityUpdateError {
 
 #[derive(Debug)]
 pub struct InstallationDiff {
-    pub added_installations: HashSet<Vec<u8>>,
-    pub removed_installations: HashSet<Vec<u8>>,
+    pub added_installations: HashSet<InboxInstallation>,
+    pub removed_installations: HashSet<InboxInstallation>,
+}
+
+impl InstallationDiff {
+    pub(crate) fn added_installations(&self) -> HashSet<Vec<u8>> {
+        self.added_installations
+            .iter()
+            .map(|i| i.installation.clone())
+            .collect()
+    }
+
+    pub(crate) fn removed_installations(&self) -> HashSet<Vec<u8>> {
+        self.removed_installations
+            .iter()
+            .map(|i| i.installation.clone())
+            .collect()
+    }
 }
 
 #[derive(Debug, Error)]
@@ -55,6 +71,21 @@ impl RetryableError for InstallationDiffError {
     }
 }
 
+#[derive(Hash, Clone, PartialEq, Eq, Debug)]
+pub struct InboxInstallation {
+    pub inbox_id: String,
+    pub installation: Vec<u8>,
+}
+
+impl From<(String, Vec<u8>)> for InboxInstallation {
+    fn from(value: (String, Vec<u8>)) -> Self {
+        InboxInstallation {
+            inbox_id: value.0,
+            installation: value.1,
+        }
+    }
+}
+
 impl<'a, ApiClient> Client<ApiClient>
 where
     ApiClient: XmtpApi,
@@ -399,8 +430,8 @@ where
         )
         .await?;
 
-        let mut added_installations: HashSet<Vec<u8>> = HashSet::new();
-        let mut removed_installations: HashSet<Vec<u8>> = HashSet::new();
+        let mut added_installations: HashSet<InboxInstallation> = HashSet::new();
+        let mut removed_installations: HashSet<InboxInstallation> = HashSet::new();
 
         // TODO: Do all of this in parallel
         for inbox_id in added_and_updated_members {
@@ -418,11 +449,21 @@ where
                 )
                 .await?;
 
-            added_installations.extend(state_diff.new_installations());
-            removed_installations.extend(state_diff.removed_installations());
+            added_installations.extend(
+                state_diff
+                    .new_installations()
+                    .into_iter()
+                    .map(|i| InboxInstallation::from((inbox_id.clone(), i))),
+            );
+            removed_installations.extend(
+                state_diff
+                    .removed_installations()
+                    .into_iter()
+                    .map(|i| InboxInstallation::from((inbox_id.clone(), i))),
+            );
         }
 
-        for inbox_id in membership_diff.removed_inboxes.iter() {
+        for inbox_id in membership_diff.removed_inboxes.iter().cloned() {
             let state_diff = self
                 .get_association_state(
                     conn,
@@ -433,7 +474,12 @@ where
                 .as_diff();
 
             // In the case of a removed member, get all the "new installations" from the diff and add them to the list of removed installations
-            removed_installations.extend(state_diff.new_installations());
+            removed_installations.extend(
+                state_diff
+                    .new_installations()
+                    .into_iter()
+                    .map(|i| InboxInstallation::from((inbox_id.clone(), i))),
+            );
         }
 
         Ok(InstallationDiff {