-
Notifications
You must be signed in to change notification settings - Fork 40
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
move add and remove members to use wallet addresses #363
Conversation
I think something must have gotten miscommunicated in the goals here. We do need some mechanism to add members by What we're hoping to achieve here is that when you are adding or removing a member by wallet address we look up the full list of |
Looking through the discussion, including my own comments, I completely see how this confusion happened |
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.
Apologies for the confusion! I could probably have talked through the issue more in advance just to make sure we were all on the same page. Thanks for tackling this, and for always refactoring and improving the code quality!
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.
This looks great. Thank you @insipx
closes #357
retry_async
to allow retrying functions that accept a mutable refremove add_members_by_installation_id
andremove_members_by_installation_id
wallet_address
stored on intent instead of KeyPackagesxmtp_cryptography
(Does not cover EIP-55)I accidentally pulled some changes from #354 from proto, so this should be merged after that (just some changes to GroupMembership names/tests that need to be merged in)