diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 104cd0308..b90b4b2e2 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -666,6 +666,15 @@ impl MlsGroup { .await } + pub async fn remove_self( + &self, + client: &Client, + ) -> Result<(), GroupError> { + // get list of installations + // make membership update intent + todo!() + } + pub async fn remove_members( &self, client: &Client, diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs index be28495ce..8e84096d8 100644 --- a/xmtp_mls/src/groups/sync.rs +++ b/xmtp_mls/src/groups/sync.rs @@ -1218,13 +1218,13 @@ async fn apply_update_group_membership_intent( let mut new_installations: Vec = vec![]; let mut new_key_packages: Vec = 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( } } + // 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::>>(); + let leaf_nodes_to_remove: Vec = - 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( })) } +// 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>, diff --git a/xmtp_mls/src/groups/validated_commit.rs b/xmtp_mls/src/groups/validated_commit.rs index 03f0b3b98..85c4bf1b3 100644 --- a/xmtp_mls/src/groups/validated_commit.rs +++ b/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::>>(); @@ -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::>>(), )); diff --git a/xmtp_mls/src/identity_updates.rs b/xmtp_mls/src/identity_updates.rs index 8397b64f2..21b6c821e 100644 --- a/xmtp_mls/src/identity_updates.rs +++ b/xmtp_mls/src/identity_updates.rs @@ -37,8 +37,24 @@ pub enum IdentityUpdateError { #[derive(Debug)] pub struct InstallationDiff { - pub added_installations: HashSet>, - pub removed_installations: HashSet>, + pub added_installations: HashSet, + pub removed_installations: HashSet, +} + +impl InstallationDiff { + pub(crate) fn added_installations(&self) -> HashSet> { + self.added_installations + .iter() + .map(|i| i.installation.clone()) + .collect() + } + + pub(crate) fn removed_installations(&self) -> HashSet> { + 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, +} + +impl From<(String, Vec)> for InboxInstallation { + fn from(value: (String, Vec)) -> Self { + InboxInstallation { + inbox_id: value.0, + installation: value.1, + } + } +} + impl<'a, ApiClient> Client where ApiClient: XmtpApi, @@ -399,8 +430,8 @@ where ) .await?; - let mut added_installations: HashSet> = HashSet::new(); - let mut removed_installations: HashSet> = HashSet::new(); + let mut added_installations: HashSet = HashSet::new(); + let mut removed_installations: HashSet = 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 {