element-hq · MadLittleMods · May 23, 2024 · May 6, 2024 · May 7, 2024 · May 8, 2024
@@ -0,0 +1 @@
+Add experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync/e2ee` endpoint for To-Device messages.
@@ -332,6 +332,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
         # MSC3391: Removing account data.
         self.msc3391_enabled = experimental.get("msc3391_enabled", False)
 
+        # MSC3575 (Sliding Sync API endpoints)
+        self.msc3575_enabled: bool = experimental.get("msc3575_enabled", False)
+
         # MSC3773: Thread notifications
         self.msc3773_enabled: bool = experimental.get("msc3773_enabled", False)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync/e2ee` endpoint for To-Device messages.
MadLittleMods marked this conversation as resolved. Outdated Show resolved Hide resolved Copy link Contributor Author MadLittleMods May 13, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Thinking more on that "relieving pressure on the Sliding Sync proxy now" point. Since the proxy is just another Matrix client which uses the `/sync` v2 loop, I don't think this new `/sync/e2ee` endpoint will help anything because: There is a critical problem in the current to-device implementation, which is that events are implicitly acknowledged when the user advances the `/sync` token. This causes problems when clients need to have 2 or more sync streams open at a time, e.g a push notification process and a main process. This can cause the two processes to race to fetch the to-device events, resulting in the need for complex synchronisation rules to ensure the token is correctly and atomically exchanged between processes. Sliding Sync's implementation of to-device events removes this requirement by associating an explicit token just for to-device events. This token uses the same terminology of the current `/sync` implementation: `since` and `next_batch`. -- MSC3885: Sliding Sync Extension: To-Device messages (`/sync` v2 will acknowledge all of the To-Device events we're trying to serve with this new endpoint) Am I missing something? Copy link Contributor Author MadLittleMods May 13, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. 🤔 I think I wasn't taking into account that the proxy will be its own device that can be separate from device we can use for this endpoint. So acknowledging the To-Device messages for one device doesn't affect the other. Copy link Contributor Author MadLittleMods May 16, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Discussed this point in a meeting with @erikjohnston today and I think this might be a problem since it will be one device consuming everything. Would probably require updates to the Sliding Sync proxy to utilize both properly. Copy link Contributor kegsay May 22, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I would have thought you'd either have a device using the proxy entirely or you have a device using Synapse's SS impl entirely, not a weird mixture of both. If so, this is not a problem afaict? Copy link Member Hywan May 22, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. The idea of this endpoint isn't to remove pressure from the proxy directly. By having a new endpoint, the Matrix Rust SDK can consume it directly instead of using sliding sync proxy with a particular `conn_id` for Sliding Sync. That's the same idea with the improved `/context`, it removes the need for another use of sliding sync with another `conn_id`. Finally, the last usage of sliding sync is the room list API. Since there will be only one usage of sliding sync, the `conn_id` won't be necessary anymore. In the simplified sliding sync implementation inside synapse, it means you would not have to manage multiple connection contexts at the same time. Copy link Contributor kegsay May 22, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. By having a new endpoint, the Matrix Rust SDK can consume it directly instead of using sliding sync proxy with a particular conn_id for Sliding Sync. This isn't true though, because the proxy will consume to-device messages that would have been delivered via `/sync/e2ee`, hence why it has to be an all-or-nothing for E2EE data. For `/context` then absolutely, that can be done via synapse only. Copy link Member Hywan May 22, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Hmm indeed. So the migration should ideally be all-on-synapse, instead of half-proxy, half-synapse. Gotcha. Copy link Contributor MatMaul May 22, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. This isn't true though, because the proxy will consume to-device messages that would have been delivered via /sync/e2ee You could probably use a whole another device for all e2ee operations, but it's quite ugly and a workaround. Copy link Contributor Author MadLittleMods May 22, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. A lot of this is just sharing and summarizing the clarifications from @erikjohnston 🙇 Discussed this point a bit further and got a little more clarification. The main aim of `/sync/e2ee` and `/context` ^[2] changes is to make handling notifications easier for the clients. Being able to use `/sync/e2ee` alongside the Sliding Sync proxy to relieve pressure was just an extra thought but given that the proxy and client share the same device ^[1], will not work (see To-Device problem explained above). If usage of `/sync` v2 and `/sync/e2ee` usage is split between separate devices (as @MatMaul mentioned) it would work but I don't think people have bandwidth/desire/benefit to do it. This means that this endpoint probably won't be useful until we have a Sliding Sync implementation in Synapse itself. I started working on `/sync/e2ee` because I thought it would have some "sliding" aspect to ease my way into the concept but turns out to be just some `/sync` v2 refactoring. It's still good in terms of getting familiar with `/sync` v2 though. It may very well turn out that `/sync/e2ee` is not actually as helpful as we thought (or if `device_lists` is sufficiently heavyweight that its just as slow), but from @erikjohnston's understanding right now, it'd be quite helpful to the SDK team (I don't know the exact details here). [1] Sliding sync proxy shares the same device as the ElementX client (as far as it has been explained to me). The way it works is that EX calls the SS proxy with that token, and then the SS proxy caches it to make the requests. [2] For onlookers, the `/context` changes just entail making the endpoint work for all events like invites. I haven't looked into what's necessary but it may be as simple as adjusting the check for whether someone is joined to the room. MadLittleMods marked this conversation as resolved. Outdated Show resolved Hide resolved