Skip to content
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

Better handling for new groups where the creator has multiple installations #761

Open
Tracked by #693 ...
neekolas opened this issue May 21, 2024 · 0 comments
Open
Tracked by #693 ...

Comments

@neekolas
Copy link
Contributor

neekolas commented May 21, 2024

I found an oversight in our original design.

Imagine the following situation:

  • User has 3 installations associated with their inbox. Highest sequence ID is 3.
  • The user creates a new group using installation 3, where that installation is automatically added as the first member

What should the sequence_id be on the initial GroupMembership?

If we set it to 3, we will never be able to add the first two installations since they are happened before sequence_id 3 and we only allow for updates that increment the sequence_id for the group and for installations to be added that match a diff between the old and new sequence_id.

We could set it initially to 0, and then have the add_missing_members function create a commit that adds all the missing installations. But that poses a problem: moving from 0 -> 3 will also add the group creator installation, who is already a member of the group. This is not allowed.

I've instituted a hack for this problem where we set the initial sequence_id to 0 and then allowing members to be missing from the installation diff so long as they are already members of the group. This is confusing and not ideal. Even if we can't come up with a better solution we'll want to document this clearly.

neekolas added a commit that referenced this issue May 21, 2024
## tl;dr

- Gets identity working end to end, with tests (pretty much) all uncommented
- Updates bindings to have all required methods
- Removes a bunch of TODOs in the code

## Follow-ups

There are a few changes I had to make to get everything passing that need some follow-up.
- xmtp/xmtp-node-go#391
- #760
- #761
- #750
- #762
- #763

There are also many places that need better test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant