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

MSC2199: Canonical DMs #2199

Open
wants to merge 39 commits into
base: old_master
Choose a base branch
from
Open

MSC2199: Canonical DMs #2199

wants to merge 39 commits into from

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Jul 30, 2019

or the One True DM™️ for a user, including optional immutability.

Rendered

Related issues:

Requires:

For project planning: element-hq/element-web#10415

@turt2live turt2live added proposal A matrix spec change proposal proposal-in-review labels Jul 30, 2019
@turt2live turt2live changed the title Immutable DMs MSC2199: Immutable DMs Jul 30, 2019
worst case for this is a split DAG on the tombstone with two tombstone state events, resolved
through state resolution. Both servers would have still come to the same conclusion on which
room to use so either of the tombstone events sent could be considered a no-op.

Copy link
Member

Choose a reason for hiding this comment

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

it’s vital to let the user choose whether old DMs get merged or turned into private chatrooms.

Copy link
Member Author

Choose a reason for hiding this comment

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

as in the option of the server tombstoning rooms and just letting them de-flag? Not sure if this can be achieved over federation very well to indicate that the server is definitely no longer considering the room as a DM.

Copy link
Member

Choose a reason for hiding this comment

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

hm. i guess i am worried about the social impact of everyone who is used to multiple DMs finding them force merged and having to create new private siderooms for their side convos. perhaps it’s not that bad though.

Copy link
Member Author

Choose a reason for hiding this comment

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

From a migration perspective the server is expected to just do nothing with them (if they don't even look like DMs to begin with, they can't conflict with DMs). They should just drop down to private rooms.

I might need to spell this out though...

Copy link
Member Author

Choose a reason for hiding this comment

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

re-reading the comment and section: this is still a valid problem. Not sure how to flag it over federation still: one server could still consider it a DM and the other a private non-DM. A state event could be used to flag the room, but what authority does any one sender have over how a room is treated for all participants? (practically speaking, all the authority but socially this is a different question).

There is also the concern of miscategorization: not everyone admins their friends after creating a DM between two users and adding some others. Maybe this can be solved by a migration process which asks the user to fix all their DMs before the server begins the migration? Possibly with the server calculating what the result would be for the client to render?

Alternatively, the existing plan isn't too bad (I hope) and might be fine as-is.

@turt2live
Copy link
Member Author

turt2live commented Jul 30, 2019

tentatively pushing this into the queue for a room version given initial steering of third party invite handling.

Edit: this has now been pushed to MSC2212, so removing label.

@turt2live turt2live added the unassigned-room-version Remove this label when things get versioned. label Jul 30, 2019
* The room is not tombstoned.
* The room is not soft-tombstoned by the server (described later in this proposal).
* No important users have been banned. Kicking or leaving the room does not affect the DM-ness
of the room.

Choose a reason for hiding this comment

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

Furthermore I can see users inviting a bot, then banning it, without bothering to adjust the powerlevel from the default. I wouldn't want this to "break" the room in some sense.

As a user I would expect an invite and a ban to result in the room acting in the same state as it was previously.

},
"events": {
"m.room.name": 100,
"m.room.avatar": 100,

Choose a reason for hiding this comment

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

Why can't I change my name or avatar in a DM? I have a "DM" right now where I have changed my name and avatar. I have similar questions about a lot of this list. Are we just disallowing everything by default because we can? I think it makes sense to actually consider each option and allow most things by default.

Maybe the RFC should explain why each feature is disabled in DM rooms?

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 just saying only the Admins of the DM (not the bots, assistants etc) can change the Room's Name & Avatar.
PL100 is not disabled.

Choose a reason for hiding this comment

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

I must be misunderstanding. It seems like this is document proposing setting the power level of all users to 50. As specified below. And this means that these items at powerlevel 100 are unachievable by regular means.

Users invited to the room get power level 50, including the creator.

Copy link
Member

Choose a reason for hiding this comment

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

Right you are, I am tangled in MSCs at this point.

In any case, this isn't the permission governing your avatar but that of the room which the client will be setting to the DM partner's avatar anyhow:

Why can't I change my name or avatar in a DM?

Your avatar is contained in the m.room.member state event avatar_url key

Choose a reason for hiding this comment

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

Right, I mixed those up. But it still applied for why I can't I set an image for the DM and why can't we set a name for a DM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me give you a common example of what happens with the current DM system. Let's say Steve and Bob have a DM. Steve's username is some random nickname that he has been using for years rather than his real name. Bob would prefer to see a DM called "Steve" instead of the nickname so Bob renames the room to "Steve".

Since that is the room name, it is now what Steve sees as the room name which is super confusing because Steve sees a direct message called Steve rather than being called Bob like it was previously.

That is why it is suggested here that participants shouldn't be allowed to set the room name or avatar. #3015 would fix the problem of allowing you to set your own name for a room that doesn't affect the name for other members.

Choose a reason for hiding this comment

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

This seems like a UX failure rather than a protocol problem. People often give themselves nicknames as a group and want to have a picture that they both want to share.

But the real issue is that I think justification should be provided in the MSC. Right now it is just a list of disabled features with no justification. It would be good to have even just a simple sentence for the rationale of each feature.


* If the room does not have a `rejoin_rule`, consider the rejoin rule as `join` for the
purposes of identification.
* Identify the room using the rules specified earlier in this proposal.

Choose a reason for hiding this comment

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

Does this line mean "Use the rules to identify if this room is a DM" or "Mark this room such that it matches the rules"?

Copy link
Member Author

Choose a reason for hiding this comment

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

the former.

Choose a reason for hiding this comment

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

It might make sense to reword slightly to be more clear. Also the "flagging them as appropriate" above could be more explicit what is intended.

specification version. Clients should stop using that preset and instead use the new endpoint
for creating DMs, as defined here.

**`POST /_matrix/client/r0/create_dm`**:
Copy link
Contributor

Choose a reason for hiding this comment

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

What if a server does not implement this endpoint? This can either be accidentally (e.g. older server impl), or intentionally (where the server is not intended to be used for Instant Messaging at all, leaving out a subset of endpoints or features such as this), what fallback logic should exist, then?

This proposal talks about server logic, but it isn't clear what happens if a server can leave this functionality out, can a server leave out this functionality? Matrix is oriented to be generalized, but most of the spec talks about Instant Messaging, i'd like for there to be a few notes about what clients can do if a server does not implement this feature, and how a server can signal this. In my opinion, Instant Messaging should be an optional-but-recommended module that server implementations can offer, else the "default" spec becomes incredibly bloated and difficult to implement for even small matrix implementations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Servers which leave out functionality are non-compliant. End of story.

Copy link
Member Author

Choose a reason for hiding this comment

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

we should have a system to make it easier on servers, but this is not the MSC to bring that system in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Servers which leave out functionality are non-compliant. End of story.

I agree on that, but i'm simply commenting on the fact that matrix 'core' spec becomes incredibly topsided towards Instant Messaging usecase, and that is made more explicit with this /create_dm endpoint.

While it's obviously needed rn for matrix's primary focused usecase (Instant Messaging), i think it's important to recognise that matrix servers might want to be geared and optimized for an entirely different usecase, while still supporting 'core' functionality (rooms, events, etc.), but not bother with Instant Messaging stuff (like this)

Copy link

Choose a reason for hiding this comment

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

I agree with @ShadowJonathan , my personal vision of matrix is a universal protocol, that can be used for IM, but not totally designed for that use case.

@jryans jryans removed the phase:2 label Mar 5, 2021
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
Although supporting multiple chats with individuals is potentially useful for some use cases,
clients are increasingly wanting to be able to support exactly one DM for a user.

Any messaging app is likely to want to differentiate DMs from all the noise, and Riot in
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Any messaging app is likely to want to differentiate DMs from all the noise, and Riot in
Any messaging app is likely to want to differentiate DMs from all the noise, and Element in

questionable. For instance, a large number of bots and bridges in the ecosystem do not currently
support encryption and generally fall silent in encrypted rooms. Bots in today's ecosystem
can use [Pantalaimon](https://github.com/matrix-org/pantalaimon) or add dedicated support
if they so choose, however bridges are still unable to decrypt messages even with Pantalaimon.
Copy link
Contributor

Choose a reason for hiding this comment

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

The Slack bridge uses Pantalaimon for encryption as of autumn 2020. While encryption is not the default for the bridge, it's stable and used in production systems.

Copy link
Member Author

Choose a reason for hiding this comment

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

From experience, this is not a sustainable way to support encryption on a bridge.

Copy link
Member Author

Choose a reason for hiding this comment

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

Random 2023 update on this: Pantalaimon is still not the sustainable option, however improvements in the matrix-rust-sdk and its bindings have led to t2bot.io (a server I run personally) to deploy its first ever production-grade encrypted appservice with relative ease/quality.

Our encryption capabilities now are significantly better than what they were back when this MSC was first written.

the new user account from the room if they don't agree.

This proposal enables encryption by default for DMs. Encryption in the specification is
moderately incomplete as of writing, making the move to enable encryption by default somewhat
Copy link
Contributor

Choose a reason for hiding this comment

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

Encryption for DMs became a default in 2020.

Copy link
Member Author

Choose a reason for hiding this comment

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

For Element, not Matrix.

4. Both Alice and Bob no longer have a viable DM: both homeservers do not consider Matrix HQ
as the replacement room, and do not treat the tombstoned room as a DM.

Ultimtely this is a wasted effort: both homeservers would start a new DM after the room was
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Ultimtely this is a wasted effort: both homeservers would start a new DM after the room was
Ultimately, this is a wasted effort: both homeservers would start a new DM after the room was

@@ -0,0 +1,722 @@
# Canonical DMs (server-side middle ground edition)

Copy link
Member

Choose a reason for hiding this comment

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

This MSC seems very very complex for what it tries to accomplish: 722 lines. For comparison, Sliding Sync is sitting at 1.4k (with pretty-printed json blobs). The rationale for such a complex change is poorly explained:

it is common for apps to expose exactly 1 chat for a conversation [...] Matrix currently does not
adequately support this as it allows for multiple rooms to represent a DM between two users

Yet this MSC allows >1 chat, it just tries to identify a canonical DM. Why? The key exchange use case is poorly explained. The MSC then goes on to explain additional features such as:

  • immutability
  • rejoinability
  • important vs unimportant users
  • soft tombstoning

Without adequately explaining why these features are necessary.

The "pitfalls with the current system" section mention the following problems:

  • being non-deterministic where two users in the same room can have a different view of that room in their clients: Alice can see it as a DM whereas Bob sees it as just another room.
  • the existing system is managed completely client-side and every client is expected to clone the same logic.

The former is awkward, but has to fall out as a condition of federation. The initiator of the DM cannot unilaterally declare Matrix HQ as my DM room with this user. Part of the complexity in this MSC is trying to explain which rooms are eligible for being DM rooms.

The latter is unfortunate, but no different from other complex systems in Matrix e.g push rules. The server hosting the user could absolutely make these decisions on the client's behalf to resolve this e.g by implementing the DM map for users, similar to how servers have default push rules to help clients.

Overall, I feel that the problems outlined in the current system can be addressed without such a complex change, notably:

  • automatically inserting DM rooms into the DM account data event based on content.is_direct (aka making the server do what clients do today) if a DM doesn't exist for this user pair. This reduces the amount of work clients need to do. There can be sanity checks on the room itself (invite only, demanding the room type be some special m.dm, etc).
  • prevent direct DM account data updates, instead exposing update paths to avoid clients stepping on each other, similar to push rules. Allow clients to overrule the DM automatically set to be a different room, or remove an existing room. This allows clients to change the DM room if the automatic selection made a bad choice, and allows clients to fix themselves if they end up in an unhappy state.
  • automatically picking the canonical DM as the room with the oldest m.room.create event. This can be done by clients or servers (e.g add a new section to the DM account event which has the canonical DM room ID).

This does allow for divergent DMs depending on if you're Alice or Bob but this is inevitable over federation (you may not even be aware of some subset of DM rooms).

Copy link
Member Author

Choose a reason for hiding this comment

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

The length is because of the organic history of the proposal: it needs a rewrite, and hasn't been touched in 2 years. Currently, the full understanding of the problem scope is spread across 3 different proposals (this text included), which is obviously not great.

The actual MSC body is an incredibly simple change: it's just surrounded by an absolute ton of text/context.

@Yadavji6380

This comment was marked as off-topic.

Request parameters (JSON body):
* `room_version` (`string`) - The version to create the room with. Same as `room_version` from
`/createRoom`.
* `invite` (`[string]`) - The list of user IDs to invite to the room. Same as `invite` from
Copy link
Contributor

@Kladki Kladki May 25, 2024

Choose a reason for hiding this comment

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

A problem with this is that because you can invite multiple users on creation, you cannot(?) make this endpoint atomically fail if you want to start a DM with someone, but the invitation fails. This just leaves you in an empty room, which has left me confused before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:core MSC which is critical to the protocol's success needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.