From eae825194ba19f3bc6c93a184b55a79dd7ba0018 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 28 Oct 2020 19:52:56 +0000 Subject: [PATCH 01/22] Threading --- proposals/0-threading.md | 196 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 proposals/0-threading.md diff --git a/proposals/0-threading.md b/proposals/0-threading.md new file mode 100644 index 00000000000..c7f467ed215 --- /dev/null +++ b/proposals/0-threading.md @@ -0,0 +1,196 @@ +### MSC: Threading + +*This MSC supersedes https://github.com/matrix-org/matrix-doc/issues/1198 * + +Matrix does not have arbitrarily nested threading for events. This is a desirable feature for implementing clones of social +media websites like Twitter and Reddit. The aim of this MSC is to define the simplest possible API shape to implement threading +in a useful way. This MSC does NOT attempt to consider use cases like editing or reactions, which have different requirements +to simple threading (replacing event content and aggregating reactions respectively). + +The API can be broken down into 2 sections: + - Making relationships: specifying a relationship between two events. + - Querying relationships: asking the server for relationships between events. + +The rest of this proposal will outline the proposed API shape along with the considerations and justifications for it. + +#### Making relationships + +Relationships are made when sending or updating events. The proposed API shape is identical to +[MSC1849](https://github.com/matrix-org/matrix-doc/blob/matthew/msc1849/proposals/1849-aggregations.md): +``` +{ + "type": "m.room.message", + "content": { + "body": "i <3 shelties", + "m.relationship": { + "rel_type": "m.reference", + "event_id": "$another_event_id" + } + } +} +``` + +Justifications for this were as follows: + - Quicker iterations by having it in event content rather than at the top-level (at the `event_id` level). + - Ability for relationships to be modified post-event creation (e.g by editing the event). + - Doesn't require any additional server-side work (as opposed to adding the event ID as a query param e.g `?in-reply-to=$foo:bar`). + +Drawbacks include: + - Additional work required for threading to work with E2EE. See MSC1849 for proposals, but they all boil down to having the `m.relationship` + field unencrypted in the event `content`. + +Additional concerns: + - The name of the `rel_type` is prone to bike-shedding: "m.reference", "m.reply", "m.in_reply_to", etc. This proposal opts for the decisions made + in MSC1849 which is "m.reference". + +Edge cases: + - Any event `type` can have an `m.relationship` field in its `content`. + - Redacting an event with an `m.relationship` field DOES NOT remove the relationship. Instead, it is preserved similar to how `membership` + is preserved for `m.room.member` events, with the following rules: + * Remove all fields except `rel_type` and `event_id`. + * If `rel_type` is not any of the three types `m.reference`, `m.annotation` or `m.replace` then remove it. + * If `event_id` is not a valid event ID (`$` sigil, correct max length), then remove it. + The decision to preserve this field is made so that users can delete offensive material without breaking the structure of a thread. This is + different to MSC1849 which proposes to delete the relationship entirely. + - It is an error to reference an event ID that the server is unaware of. Servers MUST check that they have the event in question: it need not + be part of the connected DAG; it can be an outlier. This prevents erroneous relationships being made by abusing the CS API. + - It is NOT an error to reference an event ID in another room. Cross-room threading is allowed and this proposal goes into more detail on how + servers should handle this as a possible extension. + - It is an error to reference yourself. + + +#### Querying relationships + +Relationships are queryed via a new CS API endpoint: +``` +POST /_matrix/client/r0/event_relationships +{ + "event_id": "$abc123", // the anchor point for the search, must be in a room you are allowed to see (normal history visibility checks apply) + "max_depth": 4, // if negative unbounded, default: 3. + "max_breadth": 10, // if negative unbounded, default: 10. + "limit": 100, // the maximum number of events to return, server can override this, default: 100. + "depth_first": true|false, // how to walk the DAG, if false, breadth first, default: false. + "recent_first": true|false, // how to select nodes at the same level, if false oldest_first - servers compare against origin_server_ts, default: true. + "include_parent": true|false, // if event_id has a parent relation, include it in the response, default: false. + "include_children": true|false // if there are events which reply to $event_id, include them all (depth:1) in the response: default: false. + "direction": up|down // if up, parent events (the events $event_id is replying to) are returned. If down, children events (events which reference $event_id) are returned, default: "down". + "batch": "opaque_string" // A token to use if this is a subsequent HTTP hit, default: "". +} +``` +which returns: +``` +{ + "events": [ // the returned events, ordered by the 'closest' (by number of hops) to the anchor point. + { ... }, { ... }, { ... }, + ], + "next_batch": "opaque_string" // A token which can be used to retrieve the next batch of events, if the response is limited. + // Optional: can be omitted if the server doesn't implement threaded pagination. +} +``` + +Justifications for the request API shape are as follows: + - The HTTP path: cross-room threading is allowed hence the path not being underneath `/rooms`. An alternative could be + `/events/$event_id/relationships` but there's already an `/events/$event_id` deprecated endpoint and nesting this new MSC + underneath a deprecated endpoint conveys the wrong meaning. + - The HTTP method: there's a lot of data to provide to the server, and GET requests shouldn't have an HTTP body, hence opting + for POST. The same request can produce different results over time so PUT isn't acceptable as an alternative. + - The anchor point: pinning queries on an event is desirable as often websites have permalinks to events with replies underneath. + - The max depth: Very few UIs show depths deeper than a few levels, so allowing this to be constrained in the API is desirable. + - The max breadth: Very few UIs show breadths wider than a few levels, so allowing this to be constrained in the API is desirable. + - The limit: For very large threads, a max depth/breadth can quickly result in huge numbers of events, so bounding the overall + number of events is desirable. Furthermore, querying relationships is computationally expensive on the server, hence allowing + it to arbitrarily override the client's limit (to avoid malicious clients setting a very high limit). + - The depth first flag: Some UIs show a 'conversation thread' first which is depth-first (e.g Twitter), whereas others show + immediate replies first with a little bit of depth (e.g Reddit). + - The recent first flag: Some UIs show recent events first whereas others show the most up-voted or by some other metric. + This MSC does not specify how to sort by up-votes, but it leaves it possible in a compatible way (e.g by adding a + `sort_by_reaction: 👍` which takes precedence which then uses `recent_first` to tie-break). + - The include parent flag: Some UIs allow permalinks in the middle of a conversation, with a "Replying to [link to parent]" + message. Allowing this parent to be retrieved in one API hit is desirable. + - The include_children flag: Some UIs allow permalinks in the middle of a conversation, with immediate children responses + visible. Allowing the children to be retrieved in one API hit is desirable. + - The direction enum: The decision for literal `up` and `down` makes for easier reading than `is_direction_up: false` or + equivalent. The direction is typically `down` - find all children from this event - but there is no reason why this cannot + be inverted to walk up the DAG instead. + - The batch token: This allows clients to retrieve additional results. It's contained inside the HTTP body rather than as + a query param for simplicity - all the required data that the server needs is in the HTTP body. This token is optional as + paginating is reasonably complex and should be opt-in to allow for ease of server implementation. + +Justifications for the response API shape are as follows: + - The events array: There are many possible ways to structure the thread, and the best way is known only to the client + implementation. This API shape is unopinionated and simple. + - The next batch token: Its presence indicates if there are more events and it is opaque to allow server implementations the + flexibility for their own token format. There is no 'prev batch' token as it is intended for clients to request and persist + the data on their side rather than page through results like traditional pagination. + +Server implementation: + - Sanity check request and set defaults. + - Can the user see (according to history visibility) `event_id`? If no, reject the request, else continue. + - Retrieve the event. Add it to response array. + - If `include_parent: true` and there is a valid `m.relationship` field in the event, retrieve the referenced event. + Apply history visibility check to that event and if it passes, add it to the response array. + - If `include_children: true`, lookup all events which have `event_id` as an `m.relationship` - this will almost certainly require + servers to store this lookup in a dedicated table when events are created. Apply history visibility checks to all these + events and add the ones which pass into the response array, honouring the `recent_first` flag and the `limit`. + - Begin to walk the thread DAG in the `direction` specified, either depth or breadth first according to the `depth_first` flag, + honouring the `limit`, `max_depth` and `max_breadth` values. For events at the same level, honour the `recent_first` flag. + If the event has been added to the response array already, do not include it a second time. If an event fails history visibiilty + checks, do not add it to the response array and do not follow any references it may have. + - When the thread DAG has been fully visited or the limit is reached, return the response array (and a `next_batch` if the request + was limited). If a request comes in with the `next_batch` set to a valid value, continue walking the thread DAG from where it + was previously left, ensuring that no duplicate events are sent, and that any `max_depth` or `max_breath` are honoured + _based on the original request_ - the max values always relate to the original `event_id`, NOT the event ID previously stopped at. + +#### Cross-room threading extension + +This MSC expands on the basic form to allow cross-room threading by allowing 2 extra fields to be specified +in the `m.relationship` object: `servers` and `room_id`: +``` +{ + "type": "m.room.message", + "content": { + "body": "i <3 shelties", + "m.relationship": { + "rel_type": "m.reference", + "event_id": "$another_event_id", + "servers": [ "localhost", "anotherhost" ], + "room_id": "!someroomid:anotherhost", + } + } +} +``` + +Only servers can set these fields. If clients attempt to set them they will be replaced. The server should set these fields +when an event is sent according to the following rules: + - Check the client can view the event ID in question. If they cannot, reject the request. + - Check that the room's `m.room.create` event allows cross-room threading by the presence of `"m.cross_room_threading": true` + in the `content` field. If absent, it is `false`. + - Fetch the servers *currently in the room* for the event. Add them all to `servers`. + - Fetch the room ID that the event belongs to. Add it to `room_id`. + +This proposal does not require any changes to `/createRoom` as `"m.cross_room_threading": true` can be specified via the +`creation_content` field in that API. + +The `POST /relationships` endpoint includes a new field: + - `auto_join: true|false`: if `true`, will automatically join the user calling `/relationships` to rooms they are not joined to + in order to explore the thread. Default: `false`. + +Server implementation: + - When walking the thread DAG, if there is an event that is not known, check the relationship for `room_id` and `servers`. If + they exist, peek into the room ([MSC2444](https://github.com/matrix-org/matrix-doc/pull/2444)), or if that is not supported, + join the room as the user querying `/relationships` if and only if `"auto_join": true` in the `/relationships` request. This + is required in order to allow the event to be retrieved. Server implementations can treat the auto join as the same as if the + client made a request to [/join/{roomIdOrAlias}](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-join-roomidoralias) + with the `server_name` query parameters set to those in `servers`. + +Security considerations: + - Allowing cross-room threading leaks the event IDs in a given room, as well as which servers are in the room at the point + the reply is sent. It is for this reason that there is an opt-in flag at room creation time. + +Justifications: + - Because the client needs to be able to view the event being replied to, it is impossible for the replying client to + respond to an event which the server is unaware of. However, it is entirely possible for queries to contain events + which the server is unaware of if the sender was on another homeserver. For this reason, we need to contain routing + information *somewhere* so servers can retrieve the event and continue navigating the thread. As the client and server + already have the event which contains a relationship to another event inside an unknown room, the simplest option is to + also contain the routing information with that relationship. \ No newline at end of file From 4dcc8c62d4fc3b70d829d47d0d679324252aa831 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 28 Oct 2020 19:53:57 +0000 Subject: [PATCH 02/22] MSC2836 --- proposals/{0-threading.md => 2836-threading.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename proposals/{0-threading.md => 2836-threading.md} (99%) diff --git a/proposals/0-threading.md b/proposals/2836-threading.md similarity index 99% rename from proposals/0-threading.md rename to proposals/2836-threading.md index c7f467ed215..cef86cfa74c 100644 --- a/proposals/0-threading.md +++ b/proposals/2836-threading.md @@ -1,4 +1,4 @@ -### MSC: Threading +### MSC2836: Threading *This MSC supersedes https://github.com/matrix-org/matrix-doc/issues/1198 * From 2b58fff4631a2b645a166d14660517e1d964cc0f Mon Sep 17 00:00:00 2001 From: Kegsay Date: Wed, 28 Oct 2020 22:26:13 +0000 Subject: [PATCH 03/22] Update proposals/2836-threading.md Co-authored-by: Matthew Hodgson --- proposals/2836-threading.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index cef86cfa74c..fb4e9e078df 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -1,6 +1,6 @@ ### MSC2836: Threading -*This MSC supersedes https://github.com/matrix-org/matrix-doc/issues/1198 * +*This MSC probably supersedes https://github.com/matrix-org/matrix-doc/issues/1198* Matrix does not have arbitrarily nested threading for events. This is a desirable feature for implementing clones of social media websites like Twitter and Reddit. The aim of this MSC is to define the simplest possible API shape to implement threading @@ -193,4 +193,4 @@ Justifications: which the server is unaware of if the sender was on another homeserver. For this reason, we need to contain routing information *somewhere* so servers can retrieve the event and continue navigating the thread. As the client and server already have the event which contains a relationship to another event inside an unknown room, the simplest option is to - also contain the routing information with that relationship. \ No newline at end of file + also contain the routing information with that relationship. From 4fcd3fcf49cfad0246a48dc48281a3de386259c3 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Thu, 29 Oct 2020 10:21:46 +0000 Subject: [PATCH 04/22] Update proposals/2836-threading.md Co-authored-by: Matthew Hodgson --- proposals/2836-threading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index fb4e9e078df..85ec0f301d2 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -138,7 +138,7 @@ Server implementation: checks, do not add it to the response array and do not follow any references it may have. - When the thread DAG has been fully visited or the limit is reached, return the response array (and a `next_batch` if the request was limited). If a request comes in with the `next_batch` set to a valid value, continue walking the thread DAG from where it - was previously left, ensuring that no duplicate events are sent, and that any `max_depth` or `max_breath` are honoured + was previously left, ensuring that no duplicate events are sent, and that any `max_depth` or `max_breadth` are honoured _based on the original request_ - the max values always relate to the original `event_id`, NOT the event ID previously stopped at. #### Cross-room threading extension From 1af2e3cfd95b13d6a4dc1a57a341bea6cf7d8797 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Thu, 29 Oct 2020 11:14:36 +0000 Subject: [PATCH 05/22] Update 2836-threading.md --- proposals/2836-threading.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 85ec0f301d2..4193e99c931 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -83,8 +83,10 @@ which returns: "events": [ // the returned events, ordered by the 'closest' (by number of hops) to the anchor point. { ... }, { ... }, { ... }, ], - "next_batch": "opaque_string" // A token which can be used to retrieve the next batch of events, if the response is limited. + "next_batch": "opaque_string", // A token which can be used to retrieve the next batch of events, if the response is limited. // Optional: can be omitted if the server doesn't implement threaded pagination. + "limited": true|false // True if there are more events to return because the `limit` was reached. Servers are not obligated + // to return more events, see if the next_batch token is provided or not. } ``` @@ -122,6 +124,8 @@ Justifications for the response API shape are as follows: - The next batch token: Its presence indicates if there are more events and it is opaque to allow server implementations the flexibility for their own token format. There is no 'prev batch' token as it is intended for clients to request and persist the data on their side rather than page through results like traditional pagination. + - The limited flag: Required in order to distinguish between "no more events" and "more events but I don't allow pagination". + This additional state cannot be accurately represented by an empty `next_batch` token. Server implementation: - Sanity check request and set defaults. @@ -133,9 +137,17 @@ Server implementation: servers to store this lookup in a dedicated table when events are created. Apply history visibility checks to all these events and add the ones which pass into the response array, honouring the `recent_first` flag and the `limit`. - Begin to walk the thread DAG in the `direction` specified, either depth or breadth first according to the `depth_first` flag, - honouring the `limit`, `max_depth` and `max_breadth` values. For events at the same level, honour the `recent_first` flag. + honouring the `limit`, `max_depth` and `max_breadth` values according to the following rules: + * If the response array is `>= limit`, stop. + * If already processed event, skip. + * Check how deep the event is compared to `event_id`, does it *exceed* (greater than) `max_depth`? If yes, skip. + * Check what number child this event is (ordered by `recent_first`) compared to its parent, does it *exceed* (greater than) `max_breadth`? If yes, skip. + * Process the event. If the event has been added to the response array already, do not include it a second time. If an event fails history visibiilty - checks, do not add it to the response array and do not follow any references it may have. + checks, do not add it to the response array and do not follow any references it may have. This algorithm bounds an infinite DAG + into a "window" (governed by `max_depth` and `max_breadth`) and serves up to `limit` events at a time, until the entire window + has been served. Critically, the `limit` _has not been reached_ when the algorithm hits a `max_depth` or `max_breadth`, it is only + reached when the response array is `>= limit`. - When the thread DAG has been fully visited or the limit is reached, return the response array (and a `next_batch` if the request was limited). If a request comes in with the `next_batch` set to a valid value, continue walking the thread DAG from where it was previously left, ensuring that no duplicate events are sent, and that any `max_depth` or `max_breadth` are honoured From d5ddccbb6900c94311d493adbf09e716fe0bc31a Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 17 Nov 2020 14:06:29 +0000 Subject: [PATCH 06/22] Cross-room threading: move room_id/servers to unsigned --- proposals/2836-threading.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 4193e99c931..30f281886d5 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -156,7 +156,7 @@ Server implementation: #### Cross-room threading extension This MSC expands on the basic form to allow cross-room threading by allowing 2 extra fields to be specified -in the `m.relationship` object: `servers` and `room_id`: +in the `unsigned` section of the event: `relationship_servers` and `relationship_room_id`: ``` { "type": "m.room.message", @@ -165,20 +165,22 @@ in the `m.relationship` object: `servers` and `room_id`: "m.relationship": { "rel_type": "m.reference", "event_id": "$another_event_id", - "servers": [ "localhost", "anotherhost" ], - "room_id": "!someroomid:anotherhost", } + }, + "unsigned": { + "relationship_room_id":"!someroomid:anotherhost", + "relationship_servers": [ "localhost", "anotherhost" ] } } ``` -Only servers can set these fields. If clients attempt to set them they will be replaced. The server should set these fields +Only servers can set these fields. The server should set these fields when an event is sent according to the following rules: - Check the client can view the event ID in question. If they cannot, reject the request. - Check that the room's `m.room.create` event allows cross-room threading by the presence of `"m.cross_room_threading": true` in the `content` field. If absent, it is `false`. - - Fetch the servers *currently in the room* for the event. Add them all to `servers`. - - Fetch the room ID that the event belongs to. Add it to `room_id`. + - Fetch the servers *currently in the room* for the event. Add them all to `relationship_servers`. + - Fetch the room ID that the event belongs to. Add it to `relationship_room_id`. This proposal does not require any changes to `/createRoom` as `"m.cross_room_threading": true` can be specified via the `creation_content` field in that API. @@ -188,12 +190,12 @@ The `POST /relationships` endpoint includes a new field: in order to explore the thread. Default: `false`. Server implementation: - - When walking the thread DAG, if there is an event that is not known, check the relationship for `room_id` and `servers`. If + - When walking the thread DAG, if there is an event that is not known, check the relationship for `relationship_room_id` and `relationship_servers`. If they exist, peek into the room ([MSC2444](https://github.com/matrix-org/matrix-doc/pull/2444)), or if that is not supported, join the room as the user querying `/relationships` if and only if `"auto_join": true` in the `/relationships` request. This is required in order to allow the event to be retrieved. Server implementations can treat the auto join as the same as if the client made a request to [/join/{roomIdOrAlias}](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-join-roomidoralias) - with the `server_name` query parameters set to those in `servers`. + with the `server_name` query parameters set to those in `relationship_servers`. Security considerations: - Allowing cross-room threading leaks the event IDs in a given room, as well as which servers are in the room at the point @@ -206,3 +208,5 @@ Justifications: information *somewhere* so servers can retrieve the event and continue navigating the thread. As the client and server already have the event which contains a relationship to another event inside an unknown room, the simplest option is to also contain the routing information with that relationship. + - The fields are in the `unsigned` section so clients cannot artificially set them, and because `unsigned` is where a lot + of other server-calculated metadata resides. From 9835503c6f63ff2de61a85a3eae734781be6ef06 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Nov 2020 16:05:44 +0000 Subject: [PATCH 07/22] Add backrefs and federated /event_relationships --- proposals/2836-threading.md | 112 +++++++++++++++++++++++++++++++++--- 1 file changed, 103 insertions(+), 9 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 30f281886d5..c108a412cde 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -39,10 +39,6 @@ Drawbacks include: - Additional work required for threading to work with E2EE. See MSC1849 for proposals, but they all boil down to having the `m.relationship` field unencrypted in the event `content`. -Additional concerns: - - The name of the `rel_type` is prone to bike-shedding: "m.reference", "m.reply", "m.in_reply_to", etc. This proposal opts for the decisions made - in MSC1849 which is "m.reference". - Edge cases: - Any event `type` can have an `m.relationship` field in its `content`. - Redacting an event with an `m.relationship` field DOES NOT remove the relationship. Instead, it is preserved similar to how `membership` @@ -56,8 +52,8 @@ Edge cases: be part of the connected DAG; it can be an outlier. This prevents erroneous relationships being made by abusing the CS API. - It is NOT an error to reference an event ID in another room. Cross-room threading is allowed and this proposal goes into more detail on how servers should handle this as a possible extension. - - It is an error to reference yourself. - + - It is an error to reference yourself. Cyclical loops are still possible by using multiple events and servers should guard against this by + only visiting events once. #### Querying relationships @@ -153,6 +149,26 @@ Server implementation: was previously left, ensuring that no duplicate events are sent, and that any `max_depth` or `max_breadth` are honoured _based on the original request_ - the max values always relate to the original `event_id`, NOT the event ID previously stopped at. +##### Querying relationships over federation + +Relationships can be queried over federation using a new endpoint which is the same as the CS API format. See the CS API section for more info. The path +used for this new federation endpoint is `/_matrix/federation/v1/event_relationships`. + +Justification: + - In an ideal world, every server would have the complete room DAG and would therefore be able to explore the full scope of a thread in a room. However, + over federation, servers have an incomplete view of the room and will be missing many events. In absence of a specific API to explore threads over + federation, joining a room with threads will result in an incomplete view. + - The requirements here are similar to the [Event Context API](https://matrix.org/docs/spec/client_server/r0.6.0#id131) but due to the lack of a federated + form of this API any requests for events the server is unaware of will incorrectly return `404 Not Found`. + - The same API shape is proposed to allow code reuse and because the same concerns and requirements are present for both federation and client-server. + +Server behaviour: + - When receiving a request to `/event_relationships`: ensure the server is in the room then walk the thread in the same manner as the CS API form. Do not + make outbound `/event_relationships` requests on behalf of this request to avoid routing loops where 2 servers infinitely call `/event_relationships` to + each other. + - Servers should make outbound `/event_relationships` requests *for client requests* when they encounter an event ID they do not have. The event may have + happened much earlier in the room which another server in the room can satisfy. + #### Cross-room threading extension This MSC expands on the basic form to allow cross-room threading by allowing 2 extra fields to be specified @@ -185,17 +201,19 @@ when an event is sent according to the following rules: This proposal does not require any changes to `/createRoom` as `"m.cross_room_threading": true` can be specified via the `creation_content` field in that API. -The `POST /relationships` endpoint includes a new field: - - `auto_join: true|false`: if `true`, will automatically join the user calling `/relationships` to rooms they are not joined to +The `POST /event_relationships` endpoint includes a new field: + - `auto_join: true|false`: if `true`, will automatically join the user calling `/event_relationships` to rooms they are not joined to in order to explore the thread. Default: `false`. Server implementation: - When walking the thread DAG, if there is an event that is not known, check the relationship for `relationship_room_id` and `relationship_servers`. If they exist, peek into the room ([MSC2444](https://github.com/matrix-org/matrix-doc/pull/2444)), or if that is not supported, - join the room as the user querying `/relationships` if and only if `"auto_join": true` in the `/relationships` request. This + join the room as the user querying `/event_relationships` if and only if `"auto_join": true` in the `/event_relationships` request. This is required in order to allow the event to be retrieved. Server implementations can treat the auto join as the same as if the client made a request to [/join/{roomIdOrAlias}](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-join-roomidoralias) with the `server_name` query parameters set to those in `relationship_servers`. + - After joining the room, the server should use the federated API `/event_relationships` to explore the thread in the newly joined room as the server + will not have those events. These events can be stored as outliers or dropped depending on storage requirements. Security considerations: - Allowing cross-room threading leaks the event IDs in a given room, as well as which servers are in the room at the point @@ -210,3 +228,79 @@ Justifications: also contain the routing information with that relationship. - The fields are in the `unsigned` section so clients cannot artificially set them, and because `unsigned` is where a lot of other server-calculated metadata resides. + +##### Backreferences + +Using the threading model proposed above, events contain information about their parents but parents do not contain information about their +children. This means that events can follow their path up to the root event but cannot explore siblings not on that root path. "Backreferences" +means adding additional metadata to allow the discovery of child events from a given parent event. These backreferences are necessary over +federation where each server has an incomplete view of the thread. Consider: + +``` +Letters = Servers +Numbers = Events + +!room1 (A,B) !room2 (B,C) + 1 <- parent + | + +-----------------+ + | | + 2 3 <- children +``` +- When event 2 is created, only servers A,B will know this. +- A backreference event needs to be created in !room2 in order for C to ever be aware of event 2. +- When C is aware of event 2, it can then peek/join into !room1 to receive updates to that fork. +- This means that **in order to create a relationship, the user must be in parent event's room**. + +The backreference event MUST be sent by the server on behalf of the client and look like: +``` +{ + "type": "m.room.backref", + "content": { + "event_id": "$parent_event_id", + "m.relationship": { + "rel_type": "m.child", + "event_id": "$child_event_id", + } + }, + "sender": "@user_making_relation:localhost", + "room_id": "!parent_event:room", + "unsigned": { + "relationship_room_id":"!someroomid:anotherhost", + "relationship_servers": [ "localhost", "anotherhost" ] + } +} +``` + +Justification: + - Backreference events have to exist so servers can be aware of new child threads in different rooms. Without it, + servers will only be able to walk from their event up to the root and not explore the entire thread. Furthermore, + servers will not converge on a consistent view of the thread as some servers have information that others are missing. + - We re-use the `m.relationship` shape because the event is a literal relationship. + - The type `m.room.backref` is used so clients can filter these out if they don't know or want to render them on the UI. + - We re-use the `unsigned.relationship_*` fields as routing information is critical in order to follow backreferences. + +Edge cases: + - It is not an error if the user does not have permission to send this event into the parent room. + - The child event should be sent first, and only after success should the backreference event be sent. This is already + implied because the backreference event needs to reference the event ID of the child. + +Security considerations: + - We omit the `content` of the child event and only share event metadata with the parent room for privacy. This + can be displayed on the UI as "Alice referenced this event in another room" which can be hyperlinked to the + room in question. + +Server behaviour: + - When sending an event: if an event contains a valid `m.relationship` - where valid means that the user is joined to both child/parent rooms - + send the event then create the backreference event and send it into the parent room on a best-effort basis. Failures + for any reason are non-fatal. + - When receiving a backreference event: persist the relationship between parent/child for use in `/event_relationships`. + +The net result of this is: + - 1: A client makes an `/event_relationships` request with `auto_join: true`. The server begins walking the thread. + - 2: The server encounters an event ID it does not have and the `unsigned` metadata indicates it is in a different room. + - 3: The server joins the room on behalf of the client. + - 4: The server calls the federated form of `/event_relationships` to the server joined via. + - 5: The server continues until it find an unknown event ID again then loops back to Step 2 until the request is satisfied. + - 6: Subsequent children made in this thread will be actively pushed to the server as normal events with the type `m.room.backref`. + This provides additional search paths for the next `/event_relationships` request and can be actively shown in the client's UI. From 5c7c8f35e6de3510c2454b7b32416f9f2a51836c Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Nov 2020 16:08:46 +0000 Subject: [PATCH 08/22] Clarify --- proposals/2836-threading.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index c108a412cde..6482ebd672e 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -158,8 +158,8 @@ Justification: - In an ideal world, every server would have the complete room DAG and would therefore be able to explore the full scope of a thread in a room. However, over federation, servers have an incomplete view of the room and will be missing many events. In absence of a specific API to explore threads over federation, joining a room with threads will result in an incomplete view. - - The requirements here are similar to the [Event Context API](https://matrix.org/docs/spec/client_server/r0.6.0#id131) but due to the lack of a federated - form of this API any requests for events the server is unaware of will incorrectly return `404 Not Found`. + - The requirements here have a lot in common with the [Event Context API](https://matrix.org/docs/spec/client_server/r0.6.0#id131). However, the context + API has no federated equivalent. This means any requests for events the server is unaware of will incorrectly return `404 Not Found`. - The same API shape is proposed to allow code reuse and because the same concerns and requirements are present for both federation and client-server. Server behaviour: From 77db3f6f354ed8ba6f15bc696c2423ec94301997 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Nov 2020 17:15:27 +0000 Subject: [PATCH 09/22] Add auth_chain to fed response --- proposals/2836-threading.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 6482ebd672e..dccde435b49 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -152,7 +152,22 @@ Server implementation: ##### Querying relationships over federation Relationships can be queried over federation using a new endpoint which is the same as the CS API format. See the CS API section for more info. The path -used for this new federation endpoint is `/_matrix/federation/v1/event_relationships`. +used for this new federation endpoint is `/_matrix/federation/v1/event_relationships`. There is one additional response field: `auth_chain` which contains +all the necessary auth events for the events in `events`, e.g: +``` +{ + "events": [ // the returned events, ordered by the 'closest' (by number of hops) to the anchor point. + { ... }, { ... }, { ... }, + ], + "next_batch": "opaque_string", // A token which can be used to retrieve the next batch of events, if the response is limited. + // Optional: can be omitted if the server doesn't implement threaded pagination. + "limited": true|false, // True if there are more events to return because the `limit` was reached. Servers are not obligated + // to return more events, see if the next_batch token is provided or not. + "auth_chain": [ // The auth events required to authenticate events in `events` + { ... }, { ... }, { ... }, + ] +} +``` Justification: - In an ideal world, every server would have the complete room DAG and would therefore be able to explore the full scope of a thread in a room. However, From 48f386ace5eac24d0fd7b9bc1a65e73500d76e1a Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Nov 2020 17:15:47 +0000 Subject: [PATCH 10/22] Clarify --- proposals/2836-threading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index dccde435b49..3f9a9c3fedb 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -163,7 +163,7 @@ all the necessary auth events for the events in `events`, e.g: // Optional: can be omitted if the server doesn't implement threaded pagination. "limited": true|false, // True if there are more events to return because the `limit` was reached. Servers are not obligated // to return more events, see if the next_batch token is provided or not. - "auth_chain": [ // The auth events required to authenticate events in `events` + "auth_chain": [ // The auth events required to authenticate events in `events`, in any order without duplicates. { ... }, { ... }, { ... }, ] } From 0ed3c9610e141aaa078dc310c41859bb74d6c9dc Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 19 Nov 2020 17:18:27 +0000 Subject: [PATCH 11/22] More clarification --- proposals/2836-threading.md | 1 + 1 file changed, 1 insertion(+) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 3f9a9c3fedb..9b408957e8a 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -181,6 +181,7 @@ Server behaviour: - When receiving a request to `/event_relationships`: ensure the server is in the room then walk the thread in the same manner as the CS API form. Do not make outbound `/event_relationships` requests on behalf of this request to avoid routing loops where 2 servers infinitely call `/event_relationships` to each other. + - For each event returned: include all `auth_events` for that event recursively to create an auth chain and add them to `auth_chain`. - Servers should make outbound `/event_relationships` requests *for client requests* when they encounter an event ID they do not have. The event may have happened much earlier in the room which another server in the room can satisfy. From 1e0a01dfad4f148029e4729d7a9c1b468e158583 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Nov 2020 10:13:58 +0000 Subject: [PATCH 12/22] s/backref/child ref/g --- proposals/2836-threading.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 9b408957e8a..94fa682567e 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -245,11 +245,11 @@ Justifications: - The fields are in the `unsigned` section so clients cannot artificially set them, and because `unsigned` is where a lot of other server-calculated metadata resides. -##### Backreferences +##### Child references Using the threading model proposed above, events contain information about their parents but parents do not contain information about their -children. This means that events can follow their path up to the root event but cannot explore siblings not on that root path. "Backreferences" -means adding additional metadata to allow the discovery of child events from a given parent event. These backreferences are necessary over +children. This means that events can follow their path up to the root event but cannot explore siblings not on that root path. "Child references" +means adding additional metadata to allow the discovery of child events from a given parent event. These references are necessary over federation where each server has an incomplete view of the thread. Consider: ``` @@ -264,14 +264,14 @@ Numbers = Events 2 3 <- children ``` - When event 2 is created, only servers A,B will know this. -- A backreference event needs to be created in !room2 in order for C to ever be aware of event 2. +- A child reference event needs to be created in !room2 in order for C to ever be aware of event 2. - When C is aware of event 2, it can then peek/join into !room1 to receive updates to that fork. - This means that **in order to create a relationship, the user must be in parent event's room**. -The backreference event MUST be sent by the server on behalf of the client and look like: +The reference event MUST be sent by the server on behalf of the client and look like: ``` { - "type": "m.room.backref", + "type": "m.room.reference", "content": { "event_id": "$parent_event_id", "m.relationship": { @@ -289,17 +289,17 @@ The backreference event MUST be sent by the server on behalf of the client and l ``` Justification: - - Backreference events have to exist so servers can be aware of new child threads in different rooms. Without it, + - Child reference events have to exist so servers can be aware of new child threads in different rooms. Without it, servers will only be able to walk from their event up to the root and not explore the entire thread. Furthermore, servers will not converge on a consistent view of the thread as some servers have information that others are missing. - We re-use the `m.relationship` shape because the event is a literal relationship. - - The type `m.room.backref` is used so clients can filter these out if they don't know or want to render them on the UI. - - We re-use the `unsigned.relationship_*` fields as routing information is critical in order to follow backreferences. + - The type `m.room.reference` is used so clients can filter these out if they don't know or want to render them on the UI. + - We re-use the `unsigned.relationship_*` fields as routing information is critical in order to follow references. Edge cases: - It is not an error if the user does not have permission to send this event into the parent room. - - The child event should be sent first, and only after success should the backreference event be sent. This is already - implied because the backreference event needs to reference the event ID of the child. + - The child event should be sent first, and only after success should the reference event be sent. This is already + implied because the reference event needs to reference the event ID of the child. Security considerations: - We omit the `content` of the child event and only share event metadata with the parent room for privacy. This @@ -308,9 +308,9 @@ Security considerations: Server behaviour: - When sending an event: if an event contains a valid `m.relationship` - where valid means that the user is joined to both child/parent rooms - - send the event then create the backreference event and send it into the parent room on a best-effort basis. Failures + send the event then create the reference event and send it into the parent room on a best-effort basis. Failures for any reason are non-fatal. - - When receiving a backreference event: persist the relationship between parent/child for use in `/event_relationships`. + - When receiving a reference event: persist the relationship between parent/child for use in `/event_relationships`. The net result of this is: - 1: A client makes an `/event_relationships` request with `auto_join: true`. The server begins walking the thread. @@ -318,5 +318,5 @@ The net result of this is: - 3: The server joins the room on behalf of the client. - 4: The server calls the federated form of `/event_relationships` to the server joined via. - 5: The server continues until it find an unknown event ID again then loops back to Step 2 until the request is satisfied. - - 6: Subsequent children made in this thread will be actively pushed to the server as normal events with the type `m.room.backref`. + - 6: Subsequent children made in this thread will be actively pushed to the server as normal events with the type `m.room.reference`. This provides additional search paths for the next `/event_relationships` request and can be actively shown in the client's UI. From 0a27d4c0642ea45be7b0c11b5ae17952acef2dd3 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 20 Nov 2020 14:12:58 +0000 Subject: [PATCH 13/22] Add section on exploring dense threads --- proposals/2836-threading.md | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 94fa682567e..820c7826242 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -185,6 +185,45 @@ Server behaviour: - Servers should make outbound `/event_relationships` requests *for client requests* when they encounter an event ID they do not have. The event may have happened much earlier in the room which another server in the room can satisfy. +#### Exploring dense threads + +The proposed API so far has no mechanism to: + - List the absolute number of children for a given event. + - Check that the server has all the children for a given parent event. + +To aid this, all events returned from `/event_relationships` SHOULD have 2 additional fields in the `unsigned` section of the event: + - `children`: A map of `rel_type` to the number of children with that relation. + - `children_hash`: The SHA256 of all the event IDs of the known children, deduplicated and sorted lexicographically. + +For example, an event `$AAA` with three children: `$BBB`, `$CCC`, `$DDD` where the first two are of `rel_type` "m.reference" and the last is "custom", should +produce an `unsigned` section of: +``` +unsigned: { + children: { + "m.reference": 2, + "custom": 1 + }, + children_hash: "184e901fca089a2abc228330426203c45f647aab58d90eca2ad27871a5dd61bd" // SHA256("$BBB$CCC$DDD") +} +``` + +Justification: + - This allows clients to display more detailed "see more" links e.g. "... and 56 more replies". + - This allows servers to identify when a federated `/event_relationships` request has been window culled by `max_depth` or `max_breadth`. + - Separating out the counts by `rel_type` allows clients to accurately determine whether children are threaded replies or some other relation. + +Server behaviour: + - When processing an `/event_relationships` response from another server, check that the total number of children matches the number of children that have + been retrieved. If they differ, mark that event as "unexplored". If they match, check that the SHA256 matches `children_hash`. If they differ, mark that + event as "unexplored", else mark it as "explored". + - When processing an `/event_relationships` request from a client, if you encounter an "unexplored" event, perform a federated `/event_relationships` request + with the desired parameters to satisfy the client request. + - Persist any new events from this request, doing the same children count/hash checks and then mark this event as "explored" **regardless of whether the children hash matches**. + This guards against permanently having unexplored events if your server has events the target server does not have. + - Explored events will always remain up-to-date assuming federation between the two servers remains intact. If there is a long outage, any new child will be + marked as "unexplored" and trigger an `/event_relationships` request, akin to how the `/send` federation API will trigger `/get_missing_events` in the event + of an unknown `prev_event`. + #### Cross-room threading extension This MSC expands on the basic form to allow cross-room threading by allowing 2 extra fields to be specified From cab807734f8aaef23d21fe9323a11593a2e982ac Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 26 Nov 2020 14:27:12 +0000 Subject: [PATCH 14/22] Base64 not hex --- proposals/2836-threading.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 820c7826242..2a11b830b26 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -193,7 +193,7 @@ The proposed API so far has no mechanism to: To aid this, all events returned from `/event_relationships` SHOULD have 2 additional fields in the `unsigned` section of the event: - `children`: A map of `rel_type` to the number of children with that relation. - - `children_hash`: The SHA256 of all the event IDs of the known children, deduplicated and sorted lexicographically. + - `children_hash`: The base64 string of the SHA256 of all the event IDs of the known children, deduplicated and sorted lexicographically. For example, an event `$AAA` with three children: `$BBB`, `$CCC`, `$DDD` where the first two are of `rel_type` "m.reference" and the last is "custom", should produce an `unsigned` section of: @@ -203,7 +203,7 @@ unsigned: { "m.reference": 2, "custom": 1 }, - children_hash: "184e901fca089a2abc228330426203c45f647aab58d90eca2ad27871a5dd61bd" // SHA256("$BBB$CCC$DDD") + children_hash: "GE6QH8oImiq8IoMwQmIDxF9keqtY2Q7KKtJ4caXdYb0=" // base64(SHA256("$BBB$CCC$DDD")) } ``` From a0a9b4ff6c87a139d0339718b7be6047949d9850 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 27 Nov 2020 15:19:52 +0000 Subject: [PATCH 15/22] Remove cross-room threading; expand on when to explore children --- proposals/2836-threading.md | 184 ++++++------------------------------ 1 file changed, 29 insertions(+), 155 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 2a11b830b26..04660f6fdf0 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -50,8 +50,7 @@ Edge cases: different to MSC1849 which proposes to delete the relationship entirely. - It is an error to reference an event ID that the server is unaware of. Servers MUST check that they have the event in question: it need not be part of the connected DAG; it can be an outlier. This prevents erroneous relationships being made by abusing the CS API. - - It is NOT an error to reference an event ID in another room. Cross-room threading is allowed and this proposal goes into more detail on how - servers should handle this as a possible extension. + - It is an error to reference an event ID in another room. - It is an error to reference yourself. Cyclical loops are still possible by using multiple events and servers should guard against this by only visiting events once. @@ -87,9 +86,9 @@ which returns: ``` Justifications for the request API shape are as follows: - - The HTTP path: cross-room threading is allowed hence the path not being underneath `/rooms`. An alternative could be - `/events/$event_id/relationships` but there's already an `/events/$event_id` deprecated endpoint and nesting this new MSC - underneath a deprecated endpoint conveys the wrong meaning. + - The HTTP path: cross-room threading is not currently allowed but is possible hence the path not being underneath `/rooms`. + An alternative could be `/events/$event_id/relationships` but there's already an `/events/$event_id` deprecated endpoint and + nesting this new MSC underneath a deprecated endpoint conveys the wrong meaning. - The HTTP method: there's a lot of data to provide to the server, and GET requests shouldn't have an HTTP body, hence opting for POST. The same request can produce different results over time so PUT isn't acceptable as an alternative. - The anchor point: pinning queries on an event is desirable as often websites have permalinks to events with replies underneath. @@ -144,7 +143,7 @@ Server implementation: into a "window" (governed by `max_depth` and `max_breadth`) and serves up to `limit` events at a time, until the entire window has been served. Critically, the `limit` _has not been reached_ when the algorithm hits a `max_depth` or `max_breadth`, it is only reached when the response array is `>= limit`. - - When the thread DAG has been fully visited or the limit is reached, return the response array (and a `next_batch` if the request + - When the thread DAG has been fully visited or the limit is reached, return the response array as `events` (and a `next_batch` if the request was limited). If a request comes in with the `next_batch` set to a valid value, continue walking the thread DAG from where it was previously left, ensuring that no duplicate events are sent, and that any `max_depth` or `max_breadth` are honoured _based on the original request_ - the max values always relate to the original `event_id`, NOT the event ID previously stopped at. @@ -174,7 +173,7 @@ Justification: over federation, servers have an incomplete view of the room and will be missing many events. In absence of a specific API to explore threads over federation, joining a room with threads will result in an incomplete view. - The requirements here have a lot in common with the [Event Context API](https://matrix.org/docs/spec/client_server/r0.6.0#id131). However, the context - API has no federated equivalent. This means any requests for events the server is unaware of will incorrectly return `404 Not Found`. + API has no federated equivalent. This means any event context requests for events the server is unaware of will incorrectly return `404 Not Found`. - The same API shape is proposed to allow code reuse and because the same concerns and requirements are present for both federation and client-server. Server behaviour: @@ -182,8 +181,9 @@ Server behaviour: make outbound `/event_relationships` requests on behalf of this request to avoid routing loops where 2 servers infinitely call `/event_relationships` to each other. - For each event returned: include all `auth_events` for that event recursively to create an auth chain and add them to `auth_chain`. - - Servers should make outbound `/event_relationships` requests *for client requests* when they encounter an event ID they do not have. The event may have - happened much earlier in the room which another server in the room can satisfy. + - Servers should make outbound `/event_relationships` requests *for client requests* when they encounter an event ID they do not have, or they suspect + that the event has children the server does not have (see the next section). The event may have happened much earlier in the room which another server + in the room can satisfy. #### Exploring dense threads @@ -208,154 +208,28 @@ unsigned: { ``` Justification: + - Without this information, it's impossible to know if an event has children _at all_. Servers need to know if children exist so they can fetch them + over federation. - This allows clients to display more detailed "see more" links e.g. "... and 56 more replies". - - This allows servers to identify when a federated `/event_relationships` request has been window culled by `max_depth` or `max_breadth`. + - This allows servers to identify when a federated `/event_relationships` request has been window culled by `max_depth` or `max_breadth`, or has just not + been explored yet. - Separating out the counts by `rel_type` allows clients to accurately determine whether children are threaded replies or some other relation. Server behaviour: - - When processing an `/event_relationships` response from another server, check that the total number of children matches the number of children that have - been retrieved. If they differ, mark that event as "unexplored". If they match, check that the SHA256 matches `children_hash`. If they differ, mark that - event as "unexplored", else mark it as "explored". - - When processing an `/event_relationships` request from a client, if you encounter an "unexplored" event, perform a federated `/event_relationships` request - with the desired parameters to satisfy the client request. - - Persist any new events from this request, doing the same children count/hash checks and then mark this event as "explored" **regardless of whether the children hash matches**. - This guards against permanently having unexplored events if your server has events the target server does not have. + - When processing an `/event_relationships` response from another server: + * For each event, check the children count and hash. If there is no `unsigned` section assume the count to be 0. + * If this event is new, persist it. If this event is not new, compare the children counts. The event with a higher children count should be persisted: + specifically the `unsigned` section should be persisted as the event itself is immutable. + * When returning events to another server or to a client, always return the `unsigned` section which produces the *most* children, and NOT the number + of children the server currently has fetched. + - When processing an `/event_relationships` request from a client: + * If the event ID is unknown, perform a federated `/event_relationships` request. Alternatively, if the event is known **and there are unexplored children**, + perform a federated `/event_relationships` request. + * An event has unexplored children if the `unsigned` child count on the parent does not match how many children the server believes the parent to have. + * Regardless of whether the federated `/event_relationships` request returns the missing children, mark the event as explored afterwards. This prevents + constantly hitting federation when walking over this event. The easiest way to mark the event as explored is to remember what the highest children count was + when the most recent federated request was made. If that number differs from the current `unsigned` count then it is unexplored. - Explored events will always remain up-to-date assuming federation between the two servers remains intact. If there is a long outage, any new child will be - marked as "unexplored" and trigger an `/event_relationships` request, akin to how the `/send` federation API will trigger `/get_missing_events` in the event - of an unknown `prev_event`. - -#### Cross-room threading extension - -This MSC expands on the basic form to allow cross-room threading by allowing 2 extra fields to be specified -in the `unsigned` section of the event: `relationship_servers` and `relationship_room_id`: -``` -{ - "type": "m.room.message", - "content": { - "body": "i <3 shelties", - "m.relationship": { - "rel_type": "m.reference", - "event_id": "$another_event_id", - } - }, - "unsigned": { - "relationship_room_id":"!someroomid:anotherhost", - "relationship_servers": [ "localhost", "anotherhost" ] - } -} -``` - -Only servers can set these fields. The server should set these fields -when an event is sent according to the following rules: - - Check the client can view the event ID in question. If they cannot, reject the request. - - Check that the room's `m.room.create` event allows cross-room threading by the presence of `"m.cross_room_threading": true` - in the `content` field. If absent, it is `false`. - - Fetch the servers *currently in the room* for the event. Add them all to `relationship_servers`. - - Fetch the room ID that the event belongs to. Add it to `relationship_room_id`. - -This proposal does not require any changes to `/createRoom` as `"m.cross_room_threading": true` can be specified via the -`creation_content` field in that API. - -The `POST /event_relationships` endpoint includes a new field: - - `auto_join: true|false`: if `true`, will automatically join the user calling `/event_relationships` to rooms they are not joined to - in order to explore the thread. Default: `false`. - -Server implementation: - - When walking the thread DAG, if there is an event that is not known, check the relationship for `relationship_room_id` and `relationship_servers`. If - they exist, peek into the room ([MSC2444](https://github.com/matrix-org/matrix-doc/pull/2444)), or if that is not supported, - join the room as the user querying `/event_relationships` if and only if `"auto_join": true` in the `/event_relationships` request. This - is required in order to allow the event to be retrieved. Server implementations can treat the auto join as the same as if the - client made a request to [/join/{roomIdOrAlias}](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-join-roomidoralias) - with the `server_name` query parameters set to those in `relationship_servers`. - - After joining the room, the server should use the federated API `/event_relationships` to explore the thread in the newly joined room as the server - will not have those events. These events can be stored as outliers or dropped depending on storage requirements. - -Security considerations: - - Allowing cross-room threading leaks the event IDs in a given room, as well as which servers are in the room at the point - the reply is sent. It is for this reason that there is an opt-in flag at room creation time. - -Justifications: - - Because the client needs to be able to view the event being replied to, it is impossible for the replying client to - respond to an event which the server is unaware of. However, it is entirely possible for queries to contain events - which the server is unaware of if the sender was on another homeserver. For this reason, we need to contain routing - information *somewhere* so servers can retrieve the event and continue navigating the thread. As the client and server - already have the event which contains a relationship to another event inside an unknown room, the simplest option is to - also contain the routing information with that relationship. - - The fields are in the `unsigned` section so clients cannot artificially set them, and because `unsigned` is where a lot - of other server-calculated metadata resides. - -##### Child references - -Using the threading model proposed above, events contain information about their parents but parents do not contain information about their -children. This means that events can follow their path up to the root event but cannot explore siblings not on that root path. "Child references" -means adding additional metadata to allow the discovery of child events from a given parent event. These references are necessary over -federation where each server has an incomplete view of the thread. Consider: - -``` -Letters = Servers -Numbers = Events - -!room1 (A,B) !room2 (B,C) - 1 <- parent - | - +-----------------+ - | | - 2 3 <- children -``` -- When event 2 is created, only servers A,B will know this. -- A child reference event needs to be created in !room2 in order for C to ever be aware of event 2. -- When C is aware of event 2, it can then peek/join into !room1 to receive updates to that fork. -- This means that **in order to create a relationship, the user must be in parent event's room**. - -The reference event MUST be sent by the server on behalf of the client and look like: -``` -{ - "type": "m.room.reference", - "content": { - "event_id": "$parent_event_id", - "m.relationship": { - "rel_type": "m.child", - "event_id": "$child_event_id", - } - }, - "sender": "@user_making_relation:localhost", - "room_id": "!parent_event:room", - "unsigned": { - "relationship_room_id":"!someroomid:anotherhost", - "relationship_servers": [ "localhost", "anotherhost" ] - } -} -``` - -Justification: - - Child reference events have to exist so servers can be aware of new child threads in different rooms. Without it, - servers will only be able to walk from their event up to the root and not explore the entire thread. Furthermore, - servers will not converge on a consistent view of the thread as some servers have information that others are missing. - - We re-use the `m.relationship` shape because the event is a literal relationship. - - The type `m.room.reference` is used so clients can filter these out if they don't know or want to render them on the UI. - - We re-use the `unsigned.relationship_*` fields as routing information is critical in order to follow references. - -Edge cases: - - It is not an error if the user does not have permission to send this event into the parent room. - - The child event should be sent first, and only after success should the reference event be sent. This is already - implied because the reference event needs to reference the event ID of the child. - -Security considerations: - - We omit the `content` of the child event and only share event metadata with the parent room for privacy. This - can be displayed on the UI as "Alice referenced this event in another room" which can be hyperlinked to the - room in question. - -Server behaviour: - - When sending an event: if an event contains a valid `m.relationship` - where valid means that the user is joined to both child/parent rooms - - send the event then create the reference event and send it into the parent room on a best-effort basis. Failures - for any reason are non-fatal. - - When receiving a reference event: persist the relationship between parent/child for use in `/event_relationships`. - -The net result of this is: - - 1: A client makes an `/event_relationships` request with `auto_join: true`. The server begins walking the thread. - - 2: The server encounters an event ID it does not have and the `unsigned` metadata indicates it is in a different room. - - 3: The server joins the room on behalf of the client. - - 4: The server calls the federated form of `/event_relationships` to the server joined via. - - 5: The server continues until it find an unknown event ID again then loops back to Step 2 until the request is satisfied. - - 6: Subsequent children made in this thread will be actively pushed to the server as normal events with the type `m.room.reference`. - This provides additional search paths for the next `/event_relationships` request and can be actively shown in the client's UI. + marked as "unexplored" (because the parent event will be missing) and trigger an `/event_relationships` request, akin to how the `/send` federation API will + trigger `/get_missing_events` in the event of an unknown `prev_event`. This will then pull in events heading up to the root event, along with `unsigned` children + counts of any potential branches in the thread. From 75c5ba09742d6846fb4d3c999fa83d03eb975db5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 27 Nov 2020 20:35:04 +0000 Subject: [PATCH 16/22] Clarifications and formatting --- proposals/2836-threading.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 04660f6fdf0..e3e0ea0eaf1 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -46,10 +46,13 @@ Edge cases: * Remove all fields except `rel_type` and `event_id`. * If `rel_type` is not any of the three types `m.reference`, `m.annotation` or `m.replace` then remove it. * If `event_id` is not a valid event ID (`$` sigil, correct max length), then remove it. + The decision to preserve this field is made so that users can delete offensive material without breaking the structure of a thread. This is different to MSC1849 which proposes to delete the relationship entirely. - It is an error to reference an event ID that the server is unaware of. Servers MUST check that they have the event in question: it need not - be part of the connected DAG; it can be an outlier. This prevents erroneous relationships being made by abusing the CS API. + be part of the connected DAG; it can be an outlier. This prevents erroneous relationships being made by abusing the CS API. Note that it is + expected that events over federation will reference event IDs that the receiving server is unaware of: this is allowed. This check is only + performed when *clients* attempt to make new references. - It is an error to reference an event ID in another room. - It is an error to reference yourself. Cyclical loops are still possible by using multiple events and servers should guard against this by only visiting events once. @@ -118,7 +121,7 @@ Justifications for the response API shape are as follows: implementation. This API shape is unopinionated and simple. - The next batch token: Its presence indicates if there are more events and it is opaque to allow server implementations the flexibility for their own token format. There is no 'prev batch' token as it is intended for clients to request and persist - the data on their side rather than page through results like traditional pagination. + the data on their side rather than page back and forth through results like traditional pagination. - The limited flag: Required in order to distinguish between "no more events" and "more events but I don't allow pagination". This additional state cannot be accurately represented by an empty `next_batch` token. @@ -137,9 +140,10 @@ Server implementation: * If already processed event, skip. * Check how deep the event is compared to `event_id`, does it *exceed* (greater than) `max_depth`? If yes, skip. * Check what number child this event is (ordered by `recent_first`) compared to its parent, does it *exceed* (greater than) `max_breadth`? If yes, skip. - * Process the event. - If the event has been added to the response array already, do not include it a second time. If an event fails history visibiilty - checks, do not add it to the response array and do not follow any references it may have. This algorithm bounds an infinite DAG + * Process the event. If the event has been added to the response array already, do not include it a second time. If an event fails history visibiilty + checks, do not add it to the response array and do not follow any references it may have. + + This algorithm bounds an infinite DAG into a "window" (governed by `max_depth` and `max_breadth`) and serves up to `limit` events at a time, until the entire window has been served. Critically, the `limit` _has not been reached_ when the algorithm hits a `max_depth` or `max_breadth`, it is only reached when the response array is `>= limit`. @@ -183,7 +187,7 @@ Server behaviour: - For each event returned: include all `auth_events` for that event recursively to create an auth chain and add them to `auth_chain`. - Servers should make outbound `/event_relationships` requests *for client requests* when they encounter an event ID they do not have, or they suspect that the event has children the server does not have (see the next section). The event may have happened much earlier in the room which another server - in the room can satisfy. + in the room has. #### Exploring dense threads From d6ed3d57f8c024e28d1414b15fb61f3a308210ab Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 27 Nov 2020 20:36:37 +0000 Subject: [PATCH 17/22] Add comment about same num children but differing hashes --- proposals/2836-threading.md | 1 + 1 file changed, 1 insertion(+) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index e3e0ea0eaf1..0a328fc0434 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -230,6 +230,7 @@ Server behaviour: * If the event ID is unknown, perform a federated `/event_relationships` request. Alternatively, if the event is known **and there are unexplored children**, perform a federated `/event_relationships` request. * An event has unexplored children if the `unsigned` child count on the parent does not match how many children the server believes the parent to have. + In addition, if the counts match but the hashes do not match, then the event is unexplored. * Regardless of whether the federated `/event_relationships` request returns the missing children, mark the event as explored afterwards. This prevents constantly hitting federation when walking over this event. The easiest way to mark the event as explored is to remember what the highest children count was when the most recent federated request was made. If that number differs from the current `unsigned` count then it is unexplored. From 496c9e1adf08538da1babf02a69c9a68c8fa38fd Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 27 Nov 2020 20:37:35 +0000 Subject: [PATCH 18/22] More comments on mismatched hashes --- proposals/2836-threading.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 0a328fc0434..26e34844a5a 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -233,7 +233,8 @@ Server behaviour: In addition, if the counts match but the hashes do not match, then the event is unexplored. * Regardless of whether the federated `/event_relationships` request returns the missing children, mark the event as explored afterwards. This prevents constantly hitting federation when walking over this event. The easiest way to mark the event as explored is to remember what the highest children count was - when the most recent federated request was made. If that number differs from the current `unsigned` count then it is unexplored. + when the most recent federated request was made, along with the hash of those children. If that number differs from the current `unsigned` count + or the numbers match but the hashes differ then it is unexplored. - Explored events will always remain up-to-date assuming federation between the two servers remains intact. If there is a long outage, any new child will be marked as "unexplored" (because the parent event will be missing) and trigger an `/event_relationships` request, akin to how the `/send` federation API will trigger `/get_missing_events` in the event of an unknown `prev_event`. This will then pull in events heading up to the root event, along with `unsigned` children From 101e98d595985c13629e7903516ee1664f834293 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 27 Nov 2020 20:42:11 +0000 Subject: [PATCH 19/22] Add comment on tradeoff between spamming servers and missing children --- proposals/2836-threading.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 26e34844a5a..15adce70f1c 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -235,6 +235,10 @@ Server behaviour: constantly hitting federation when walking over this event. The easiest way to mark the event as explored is to remember what the highest children count was when the most recent federated request was made, along with the hash of those children. If that number differs from the current `unsigned` count or the numbers match but the hashes differ then it is unexplored. + * Due to marking events as explored regardless of what is returned from the request, it is possible for it to be known that there are missing children on + an event but being unable to retrieve them. This can happen if the only server with those events goes down before they can be replicated via `/send` to + other servers, but after another server pulls in the parent event with the right `unsigned` counts. The alternative would be to constantly ask servers + if they have the children which would increase federation traffic needlessly. - Explored events will always remain up-to-date assuming federation between the two servers remains intact. If there is a long outage, any new child will be marked as "unexplored" (because the parent event will be missing) and trigger an `/event_relationships` request, akin to how the `/send` federation API will trigger `/get_missing_events` in the event of an unknown `prev_event`. This will then pull in events heading up to the root event, along with `unsigned` children From ee443928c305fc63c572626a9264c05a60244fad Mon Sep 17 00:00:00 2001 From: Kegsay Date: Fri, 27 Nov 2020 20:44:24 +0000 Subject: [PATCH 20/22] Update proposals/2836-threading.md Co-authored-by: DeepBlueV7.X --- proposals/2836-threading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 15adce70f1c..7dfecd652d4 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -140,7 +140,7 @@ Server implementation: * If already processed event, skip. * Check how deep the event is compared to `event_id`, does it *exceed* (greater than) `max_depth`? If yes, skip. * Check what number child this event is (ordered by `recent_first`) compared to its parent, does it *exceed* (greater than) `max_breadth`? If yes, skip. - * Process the event. If the event has been added to the response array already, do not include it a second time. If an event fails history visibiilty + * Process the event. If the event has been added to the response array already, do not include it a second time. If an event fails history visibility checks, do not add it to the response array and do not follow any references it may have. This algorithm bounds an infinite DAG From 7628a4d36eeb5447ce6b81bb22fc1b2638643514 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 30 Nov 2020 16:16:55 +0000 Subject: [PATCH 21/22] Add room_id to request --- proposals/2836-threading.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 7dfecd652d4..12ca2d20160 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -64,6 +64,7 @@ Relationships are queryed via a new CS API endpoint: POST /_matrix/client/r0/event_relationships { "event_id": "$abc123", // the anchor point for the search, must be in a room you are allowed to see (normal history visibility checks apply) + "room_id": "!foo:localhost" // Optional. A hint to the server which is used if the event ID is not found. The room ID can be used to work out which servers may have the event. "max_depth": 4, // if negative unbounded, default: 3. "max_breadth": 10, // if negative unbounded, default: 10. "limit": 100, // the maximum number of events to return, server can override this, default: 100. @@ -95,6 +96,12 @@ Justifications for the request API shape are as follows: - The HTTP method: there's a lot of data to provide to the server, and GET requests shouldn't have an HTTP body, hence opting for POST. The same request can produce different results over time so PUT isn't acceptable as an alternative. - The anchor point: pinning queries on an event is desirable as often websites have permalinks to events with replies underneath. + - The `room_id`: The intention is to support the idea of permalinks to events. Even if event IDs were + [global](https://github.com/matrix-org/matrix-doc/blob/travis/msc/global-event-ids/proposals/2848-global-event-ids.md), + not all servers will have all events in a room. This means that a server may be allowed to see an event, they just need to request + it from the right server. The room ID is a convenient way to map to a list of servers who may have this event. This is optional, + and the lack of a room ID just means if the event ID in the request is not known to the server, the request fails. If the room ID + is specified, then a federated `/event_relationships` request can be made (see next section). - The max depth: Very few UIs show depths deeper than a few levels, so allowing this to be constrained in the API is desirable. - The max breadth: Very few UIs show breadths wider than a few levels, so allowing this to be constrained in the API is desirable. - The limit: For very large threads, a max depth/breadth can quickly result in huge numbers of events, so bounding the overall From d9bf91856df5c7d005c1b4c9e54602c72e624a9e Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 1 Dec 2020 11:45:03 +0000 Subject: [PATCH 22/22] Add notes around rejecting missing events --- proposals/2836-threading.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/proposals/2836-threading.md b/proposals/2836-threading.md index 12ca2d20160..ed77b4684a7 100644 --- a/proposals/2836-threading.md +++ b/proposals/2836-threading.md @@ -50,9 +50,10 @@ Edge cases: The decision to preserve this field is made so that users can delete offensive material without breaking the structure of a thread. This is different to MSC1849 which proposes to delete the relationship entirely. - It is an error to reference an event ID that the server is unaware of. Servers MUST check that they have the event in question: it need not - be part of the connected DAG; it can be an outlier. This prevents erroneous relationships being made by abusing the CS API. Note that it is - expected that events over federation will reference event IDs that the receiving server is unaware of: this is allowed. This check is only - performed when *clients* attempt to make new references. + be part of the connected DAG; it can be an outlier. If the server does not have this event but can fetch this event from another server + in the room using the `room_id` key as a hint then this is acceptable. This prevents erroneous relationships being made by abusing the CS API. + Note that it is expected that events over federation will reference event IDs that the receiving server is unaware of: this is allowed. + This check is only performed when *clients* attempt to make new references. - It is an error to reference an event ID in another room. - It is an error to reference yourself. Cyclical loops are still possible by using multiple events and servers should guard against this by only visiting events once. @@ -134,6 +135,8 @@ Justifications for the response API shape are as follows: Server implementation: - Sanity check request and set defaults. + - Does the server have the event given by `event_id`? If yes, continue. If no, check for a `room_id` key. If one exists then ask + servers in the room for `event_id` (e.g via `/event_relationships`). If no server has the event, reject the request. - Can the user see (according to history visibility) `event_id`? If no, reject the request, else continue. - Retrieve the event. Add it to response array. - If `include_parent: true` and there is a valid `m.relationship` field in the event, retrieve the referenced event.