Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC3051: A scalable relation format #3051

Open
wants to merge 3 commits into
base: old_master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions proposals/3051-scalable-relations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# MSC3051: Scalable relations
Copy link
Member

Choose a reason for hiding this comment

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

Note that the spec does not use the term "relation" anywhere in the text.

Suggested change
# MSC3051: Scalable relations
# MSC3051: Scalable event relationships


Edits, reactions, replies, threads, message annotations and other MSCs have
shown, that relations between events are very powerful and useful. Currently the
format from [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) is
used. That format however limits each event to exactly one relation. As a result
Comment on lines +4 to +6
Copy link
Member

Choose a reason for hiding this comment

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

MSC2674 is now canon. Also, some minor grammar/wording improvements:

Suggested change
shown, that relations between events are very powerful and useful. Currently the
format from [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) is
used. That format however limits each event to exactly one relation. As a result
shown that relationships between events are very powerful and useful.
However, the [current format](https://spec.matrix.org/v1.7/client-server-api/#definition-mrelates_to)
limits each event to at most one relationship. As a result

events rely on other ways to represent secondary relations. For example edits
keep the relation from the previous event. Their support to change or delete
that relation is limited. In theory you could pass that in `m.new_content`, but
clients don't seem to support that and the actual deletion of a relation is
unexplored as well.

There are many cases where 2 or more relations on an event would be useful. This
Copy link
Member

@ara4n ara4n Jun 6, 2023

Choose a reason for hiding this comment

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

I agree that we seem to be coming up against more and more scenarios where having multiple relations on a single event could be useful. I've tried to summarise my original rationale at #4023 (comment) for sticking to a simple {Subject, Verb, Object} triple on relations, and concluding that the limits might outweigh the benefits - especially given the existence of extensible events, where we can decorate a given event with additional structured metadata; so why not also be able to decorate a given event with additional relations too. but tl;dr: i'd be supportive of changing to lists of relations rather than hacking around them with stuff like the is_falling_back field in MSC3440.

MSC proposes a simple way to do that and replace the currently proposed format.
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

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

I would find it useful to mention some of these use-cases. The only one I see below is "a description for multiple files", which I don't think even has a relation proposed.

Are there other use-cases you can think of that would be useful? The only one I know of is threads.

Choose a reason for hiding this comment

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

the other use case mentioned is replacing the original message's replied to message with an edit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing replies in an edit (or removing it), editing inside of threads, replying inside of threads.

Considering what relations we currently have:

  • replies
  • edits
  • threads
  • annotations
  • (references)

I can see it being useful for edits and threads, while for annotations and replies it might only be useful in combination with other relations. No idea about references, since those are currently not very well defined. I don't think it is that unlikely to say in the future there will be more relation types, that can benefit from it. (I.e. I could imagine wanting to reply to multiple messages, to show someone when something was mentioned before and other cool stuff)


## Proposal
Copy link

@chayleaf chayleaf Aug 30, 2021

Choose a reason for hiding this comment

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

You should potentially touch upon encryption as well? See https://github.com/matrix-org/matrix-doc/issues/2678 for ongoing discussion.

In short, aggregations are useful - so the server needs to be able to return all events relating to a specific message - but the server doesn't have to know any more than that. Potentially, even filtering by event type isn't needed (and if it becomes necessary, it can always be added later, adding unencrypted metadata is easier than removing it). In the unencrypted version of the message content, you could hash the event_id field using a message-specific salt, and rel_type could either be omitted or hashed as well; other data has not to be included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What @Sorunome mentioned on #2678 is an idea we worked on together. Until that is properly worked out, we would just not encrypt the relations. In theory you don't need to know the actual values to aggregate relations with APIs. You can just tell the API what values it should aggregate for you. It is just less efficient and you run into trouble, if you automatically want to include the aggregations in the unsigned section. I think encryption for relations can be solved in an independent MSC, since it is quite a difficult topic. In theory privacy sensitive clients could also just not put unencrypted relations into the event at all, although currently that would be disallowed.

Choose a reason for hiding this comment

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

i see, that makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

Creating an unencrypted protocol and trying to layer encryption on top later is not a good way to make a secure protocol. We should avoid adding leaky features until the encryption had been sorted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevincox, you can complain about that on the original relations MSC. I made this MSC to fix some issues with the original MSC, but I didn't want any big changes that would make them hard to compare or lead to additional bikeshedding. I think encrypted relations can just be a separate relation type without the need of having to define the exact format upfront, the same way that Matrix defined messages first and later added an encrypted type.


To support multiple relations per file this MSC proposes the following format:

```json
{
"content": {
"m.relations": [
{
"event_id": "$some-other-event",
"rel_type": "m.in_reply_to"
},
{
"event_id": "$some-third-event",
"rel_type": "m.replaces"
},
{
"event_id": "$event-four",
"rel_type": "org.example.custom_relation",
"key": "some_aggregation_key"
}
]
},
"event_id": "$something",
"type": "m.room.message"
}
```

This has a few benefits:

- You can relate to multiple events at the same time. (I.e. you have a
description for multiple files you sent.)
- You can have multiple different relation types at once. (I.e. an edit, that
is also a reply, or a reaction inside a thread.)
- You don't need to look up reply relations in multiple events for edits. The
edited event is canonical and can be used standalone, without having to look
up the original event to figure out, what was replied to. You can also remove
a relation with an edit now. (Useful if you replied to the wrong message or
didn't mean to reply to anyone.)
Comment on lines +50 to +54
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure I follow what this is suggested. Does this propose changes to MSC2676? I don't see how this really helps, maybe this section could use an example of an event which gets edited twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a few examples of how this would affect the other relations in 2 Appendices. Those are just ideas but not actual changes to those MSCs, since that is probably better done on those MSCs.

- This format is conceptually a lot simpler, if an event has multiple relations.
You don't run into issues with packing relations into `m.new_content`,
especially for encrypted events, etc. You just have a list of relations.

If clients want to stay backwards compatible (for a while at least), in many
instances it is possible to generate an `m.relates_to` object from the relations
list. This can be done by picking a primary relation, i.e. the edit relation,
and then packaging up the remaining relations in `m.new_content` or simply
throwing them away. Since this proposal uses `m.relations`, this does not
conflict with the current relations from the other MSCs. One can also generate
the relations object from this MSC from the old relations, since the new
relations are a strict superset, which may be useful to make handling inside of
a client easier.
Comment on lines +59 to +67
Copy link
Member

Choose a reason for hiding this comment

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

This is unclear how this would be implemented, is there a prioritized list of what relations to use? What happens if m.relates_to conflicts with m.relations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is m.relations, you use that. it is a superset of m.relates_to and if a client sends both, the m.relates_to is probably a fallback.

A concrete example of how one can implement the fallback parsing logic is here: https://github.com/Nheko-Reborn/mtxclient/pull/48/files#diff-6c2fae13f9cbfbde2c2f9e0f681b252e3d6f33df71d3f495637ce6e17b1286a9R211-R263

Basically for parsing you can always convert relations to the new format by just parsing any relation you can and stuffing the in the list. One issue is that replies might get lost, for that we use a flag to indicate, that this was generated and in that case use the normal lookup rules for what an edit is a reply to.

Emitting the fallback is a bit more tricky. What my implementation does, is that it orders relations by priority. If something is an edit, we send an edit in m.relates_to. Otherwise we just send the first relation we find, because the other relations usually don't get combined so far.

Copy link
Member

Choose a reason for hiding this comment

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

You probably want to do some assertion that what's in m.relates_to is the same as what's in m.relations, otherwise this could be used to show different clients different content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does sound reasonable, but I don't think it is that important and we don't have the same for the other fallbacks either. I.e. the edit fallback, the reply fallback. If you implement sanity checks for those, you probably want them here too. Tbh, I would prefer to keep the period where one needs to emit a fallback to a minimum, because I don't like that clients can see different things, but #2781 doesn't seem to be a priority for anyone either, so the consensus seems to be, that this is an acceptable risk.


## Potential issues

Choose a reason for hiding this comment

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

I think fallback needs to be touched upon as well - mostly just what should clients send as fallback info


### Ordering

The list of relations is not hierarchical. As such there is no order like where
you have a top level relation and a lower level relation like an edit having
priority over a reply.

I don't believe that is an issue in practice. If you edit a message with a
reply, there is a natural meaning to the combination of both relations. You can
even apply them in any order, imo. But there may be other relations, where this
causes more issues. An MSC introducing such a relation should specify how to
handle conflicts then.
Comment on lines +77 to +81
Copy link
Member

Choose a reason for hiding this comment

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

I think this is saying if you have a message that contains an edit relation and a reply relation would mean:

Find the event that the edit refers to and replace it with this event, which is now also a reply.

That seems fairly hierarchical to me and I don't see how you can apply those in the opposite order to me?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on how you implement your client. You can render the event as a reply first, and then place it at the location of the event, that was edited. Or you replace the event data in the database for that location first, then tell the UI to rerender that event, and it will naturally pick up that this event now is a reply to X.

At least in my clients, rendering events is usually a sequence if "is this a reply?", "is this an edit?", "is this in a thread?", but those things can be applied pretty much independently. There isn't really a need to order it protocol wise, because my clients just pick from the list, what they need. If you have [edit, reply] or [reply, edit], that should be easy to handle.

Alternatively, you could make it hierarchical, specify what each relation can contain as other relations. But I can't see much benefit there, it is just making a more complicated list/graph. I.e. if you have:

{
  "rel_type": "m.thread",
  "event_id": "$something",
  "m.in_reply_to": { "event_id": "$abc" }
}

What is the benefit over:

{
  {
    "rel_type": "m.thread",
    "event_id": "$something"
  },
  {
    "rel_type": "m.in_reply_to",
    "event_id": "$abc"
  }
}

In my case I found the first one to be harder to work with, because I needed to add a lot of special cases to the parser, while the second one didn't make the UI any harder to implement, while the SDK is much simpler. The first one also doesn't tell me how to extend it to support edits, that would be another special case, while in the second one it is natural. And the first one actually needs you to define an order, while a client might have an easier time, if the order was different.

I guess what I am trying to say, I don't see an explicit order that helpful. It is very much like a() && b() && c(), while that statement does have an order, if a, b and c don't have sideeffects, the result is the same, even if you reorder it.


### Conflicting relations

Some relation types should probably not be combined. For example you may
disallow editing a reaction, because clients probably won't be handling that
correctly. This MSC however does not disallow that. Relations should specify,
deepbluev7 marked this conversation as resolved.
Show resolved Hide resolved
how clients should handle that and clients sending such combinations should be
aware, that those probably won't get handled. I don't think just allowing 1
relation is the solution to handling such conflicts and I don't think they will
happen much in practice.

## Alternatives

- We could just stick with the existing proposal to only have 1 relation per
event. This is obviously limiting, but works well enough for a lot of
relation types.
- There are a few other ways to structure relations like using an object instead
of an array, etc. I believe this is the most usable one.

## Security considerations

Multiple releations may increase load on the server and the client and provide
more opportunities to introduce bad data. Servers and clients should take
additional care and validate accordingly. It should not be considerably worse
than single relations though and servers may limit relations to a reasonable
amount (like they do for devices already).
Comment on lines +105 to +109
Copy link
Member

Choose a reason for hiding this comment

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

I'm quite nervous at the potential for abuse here, it seems like it would be quite easy to put odd groups of relations together, maybe this is already possible with the current system and not made much worse though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is the biggest problem with this MSC. But I think in practice the amount of shenanigans you can do is somewhat limited. One issue I found, is that one can basically make a reply point to "itself" by having the edit relation and the reply relation point to the same event. So some clientside validation is definitely needed (same for the server side pagination APIs), but most of that is fixed by just doing basic sanity checks (maximum recursion depths, not rendering a reply relation on reactions, etc), I think most of those validations are fairly natural and you will have a harder time with the other fields in events having bad data (i.e. all the crypto events trying to cause overflows when parsing or similar). I think even if you cause an issue by making weird combinations, the result should in most cases be pretty harmless.


## Unstable prefix

Clients should use `im.nheko.relations.v1.relations` instead of `m.relations`
and `im.nheko.relations.v1.in_reply_to` as the relation type for replies in the
mean time.