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

Add MLS Service #106

Merged
merged 14 commits into from
Oct 19, 2023
Merged

Add MLS Service #106

merged 14 commits into from
Oct 19, 2023

Conversation

neekolas
Copy link
Collaborator

@neekolas neekolas commented Oct 19, 2023

Summary

  • Adds a MLS delivery service, with a moderately thought through API spec. Subject to change, but I want to get something in to scaffold the service, even if some of the endpoints wind up changing.
  • Adds a few wrapper types for the message formats that will be published to the network

@@ -49,7 +50,7 @@ lint:
# MAX_LINE_LENGTH rule option.
max_line_length:
# Enforces a maximum line length
max_chars: 80
max_chars: 150
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realized that the new Protobuf formatter doesn't necessarily split lines (for example, for long RPC definitions)

@@ -0,0 +1,161 @@
// Message API
syntax = "proto3";
package xmtp.message_api.v3;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just skipped right over V2 and went straight to V3 for naming consistency with other V3 protos

@neekolas neekolas changed the title Add MLS delivery service Add MLS Service Oct 19, 2023
@neekolas neekolas marked this pull request as ready for review October 19, 2023 14:56
@neekolas neekolas requested a review from a team as a code owner October 19, 2023 14:56
proto/message_api/v3/mls.proto Outdated Show resolved Hide resolved
proto/message_api/v3/mls.proto Outdated Show resolved Hide resolved
Copy link
Contributor

@richardhuaaa richardhuaaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to get something in and iterate.

One idea I'd like to plant here is if we are able to make any of these endpoints less MLS-specific? For example, by introducing a generic OrderedTopic concept, or a generic SingleUseTopic?

proto/mls/message_contents/message.proto Outdated Show resolved Hide resolved
@neekolas
Copy link
Collaborator Author

One idea I'd like to plant here is if we are able to make any of these endpoints less MLS-specific? For example, by introducing a generic OrderedTopic concept, or a generic SingleUseTopic?

I'm open to ordered topics. I'd like to get a better understanding of what you'd want a SingleUseTopic to do.

Even with that, I think the PublishToGroup is going to be pretty MLS-specific until we have a better commit resolution strategy. That one requires not just ordering, but also rejection of messages from past epochs so that the client can refresh their state and retry the request.

@richardhuaaa
Copy link
Contributor

Just spitballing, a SingleUseTopic deletes payloads as they are read. An OrderedTopic requires payloads to have two fields, currentEpoch and nextEpoch, and publishes fail if currentEpoch doesn't match the current epoch. I say this with low confidence, I think you're deeper into what the actual validation logic would be than I am

@neekolas neekolas merged commit 65c1ae5 into main Oct 19, 2023
8 checks passed
@neekolas neekolas deleted the nmolnar/mls-delivery-service branch October 19, 2023 21:48
@github-actions
Copy link

🎉 This PR is included in version 3.30.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants