-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding and removing group members #746
Conversation
…tp/libxmtp into 05-01-create_validated_commit_for_mls
…ub.com:xmtp/libxmtp into 05-01-add_remove_members_using_groupmembership
@@ -144,93 +141,6 @@ impl From<Vec<Vec<u8>>> for AddressesOrInstallationIds { | |||
} | |||
} | |||
|
|||
#[derive(Debug, Clone)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding and removing members are now handled in a single action. That way we can bundle up adding missing installations with adding a new member, as a single commit.
// get current number of users in group | ||
let member_count = self.members()?.len(); | ||
if member_count + account_addresses.len() > MAX_GROUP_SIZE as usize { | ||
if member_count + inbox_id_map.len() > MAX_GROUP_SIZE as usize { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could multiple addresses in account_addresses
link to the same inbox id so that we over-calculate the group size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only until we update group.members()
to be based on inbox ID. Coming very soon
Co-authored-by: 37ng <[email protected]>
tl;dr
GroupMembership
add_missing_members
work with the new membership primitivesNext up
group.members()
from this PR. Will be updating that next