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: Immutable DMs #2199

Open
wants to merge 30 commits into
base: master
from

Conversation

@turt2live
Copy link
Member

commented Jul 30, 2019

or the One True DM™️ for a user.

Rendered

Related issues:

Requires:

For project planning: vector-im/riot-web#10415

@turt2live turt2live changed the title Immutable DMs MSC2199: Immutable DMs Jul 30, 2019

proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
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.

This comment has been minimized.

Copy link
@ara4n

ara4n Jul 30, 2019

Member

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

This comment has been minimized.

Copy link
@turt2live

turt2live Jul 30, 2019

Author Member

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.

This comment has been minimized.

Copy link
@ara4n

ara4n Jul 30, 2019

Member

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.

This comment has been minimized.

Copy link
@turt2live

turt2live Jul 30, 2019

Author Member

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...

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 6, 2019

Author Member

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.

proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved

turt2live added some commits Jul 30, 2019

proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
@turt2live

This comment has been minimized.

Copy link
Member Author

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 requested a review from ara4n Aug 6, 2019

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@ara4n This should now reflect what happens in a world where we reuse old DMs people have left. If you get a chance, please take another look.

@richvdh
Copy link
Member

left a comment

Looks generally good to me.

proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved

#### A note about leaving DMs

Unless an important user is banned, the DM is still considered alive and should be able to

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 15, 2019

Member

surely a single important user being banned should not kill the DM as long as there are at least two other unbanned important users?

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 15, 2019

Author Member

The complexity skyrockets when we don't kill it:

  • User A has a DM with User B
  • User A, B, and C all have a DM with each other.
  • User C is banned from the common DM.
  • This would now leave User A with two DMs with User B, which means the room needs to be resolved to a single room (tombstone).
  • User C becomes unbanned.
  • User C is potentially redirected to a private chat they cannot join, starting a new DM.
  • History between the rooms is lost.

So instead if we just kill it:

  • User A has a DM with User B
  • User A, B, and C all have a DM with each other.
  • User C is banned from the common DM.
  • The common room is considered dead. User A and B need to do nothing with their existing DM.
  • User C is unbanned, bringing the room back to life.
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved

The room creation presets `private_chat` and `trusted_private_chat` are deprecated and to be
removed in a future specification version. Clients should stop using those presets and instead
use `immutable_direct_chat` as defined here. The new preset has the following effects on the

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 15, 2019

Member

a thought: I'd like us to figure out whether to call them direct chats or DMs, and be vaguely consistent, at least among identifiers within the spec (cf immutable_direct_chat vs m.dm vs m.direct).

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 17, 2019

Author Member

I'd vote for DMs (m.dm, immutable_dm, etc) to be clear in what the room is supposed to be. Thoughts?

proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
`400 M_BAD_STATE` when the request contains conflicting properties. Future extensions to
`/createRoom` are expected to be included in the forbidden list where appropriate.

Servers MUST return an existing room ID if the server already knows about a DM between the

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 15, 2019

Member

It's beginning to feel like a new endpoint would be preferable to overloading createRoom

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 15, 2019

Author Member

I'd rather not clone createRoom just to handle DMs. The added restrictions are not all that complicated.

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 15, 2019

Member

noo, but "return the existing room if one already exists" seems a step-change in behaviour.

Also, I'm not sure you would be cloning createRoom: there are lots of knobs on createRoom which wouldn't be supported (or at least, the world would be much simpler if we didn't have to support them).

(If I had my way createRoom would be taken out back and shot, not least because of its camelcase name).

But it's not a thing I have a strong opinion on, and tbf I haven't worked through all the implications of a separate endpoint.

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 15, 2019

Author Member

I'll play around with it. The idea of making presets their own endpoints does seem appealing, and we don't need to fully clone createRoom indeed.

(ftr, I also want to shoot it because it has a 5-step process for creating a room with if statements in the middle)

proposals/2199-immutable-dms.md Show resolved Hide resolved
@erikjohnston
Copy link
Member

left a comment

A few comments.

proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Outdated Show resolved Hide resolved
proposals/2199-immutable-dms.md Show resolved Hide resolved
Some clients are investigating the usage of immutable DMs for security and privacy related
functionality of their design. For example, client designers are looking into handling key
verification and cross-signing within the immutable DM (if such a concept existed) rather than
in a floating dialog. To accomplish this in a safe and secure way it may be desirable to have

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Aug 15, 2019

Member

Given that a) there may be external users/bots in the room and b) you may end up with a bunch of ex-DM rooms given the anti-glare stuff/handling leaves/etc, I'm not sure how well this proposal support such things.

(As well as DMs having different reliability guarantees than the existing "to device messages" etc)

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 15, 2019

Author Member

A large part of this is a concern for the people working on actually designing the feature. This MSC should be designed such that they can hit /createRoom (or equiv, if applicable) and magically get pointed to a room. Clients can track which rooms are DMs and such, or they can play dumb and hit /createRoom: either way they should end up with a relatively straightforward way to pick the right room.

@erikjohnston

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

There is nothing show stopping here, but it is quite fiddly and convoluted (understandably) so I think we should be careful that these are the semantics we want going forward.

One thing I think that is missing is a list of deficiencies with the current spec (I may be blind), as currently its hard for me tell whether this proposal actually solves the problem it sets out to solve. (For example will the handling of soft-tombstoned rooms be any easier than the handling of multiple DMs in the current system, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.