MSC4218: Improving performance of profile changes #4218
Conversation
ara4n
added
proposal
| <img width="994" alt="Screenshot 2024-10-20 at 15 17 46" src="https://github.com/user-attachments/assets/ac227603-b635-4ecc-bd60-d3446074facf"> | ||
|
|
||
|
|
||
| ### Synthetic Events |
There was a problem hiding this comment.
synth events also make an appearance in msc #2228 (thankfully looking the same) as a way of letting servers tell clients if an event has destructed.
| For unfederated rooms, no EDUs need to be sent. So how do users see profile changes in rooms? The server creates a "synthetic event" which is effectively a fake member event based on the real member event, decorated with the latest profile data at the time the EDU is received / the profile was updated. As an example, if a user has no profile then sets one, the new profile fields will appear in the synthetic event e.g `content: { membership: join }` becomes `content: { membership, join, displayname: Alice }`. The synthesised event MUST have a unique event ID as clients often deduplicate data based on the event ID. This proposal proposes a format of `{original_member_event_id}_{iteration}` e.g `$A5z9GLL8ABCYg4SKujf_ehtW52yGoGhZuQGc0nspEeU_2`. | ||
|
|
||
| As a result, clients will see no differences except: | ||
| - profile change `m.room.member` events are synthetic so they don't exist in the room DAG, so the event ID isn't real. This can break clients which ask for events in federation format as this event won't have `prev_events`, etc. The response to `GET /state/m.room.member/@user:id` would also return the _synthetic event content_, not the federation form (as would `/members`), on the basis that we care about profiles more than accurate DAG reproducibility. This could be configured in another MSC. |
There was a problem hiding this comment.
Would synthetic events be in the timeline section of /sync or only in state? Only being in state would mean profile changes aren't visible to others, which can be a feature or a bug depending on who you ask. If fake events are in the timeline, then it'd probably mess up read receipts and other such things.
There was a problem hiding this comment.
They would function the same as normal m.room.member profile changes, so they would be in the timeline. You make an excellent point regarding receipts which would be inconsistent over federation (as the marker would be for a synthetic event which doesn't exist on other servers).
I'd suggest the pragmatic approach of just not sending receipts for synthetic events, as by their definition of a state synchronisation primitive they aren't intentional messages sent which require intentional acknowledgement via receipts.
For SSS I would suggest putting synthetic events in the state section only, as that matches their intended purpose better. It does however mean you'd lose profile changes in the timeline, so that might be a controversial move.
There was a problem hiding this comment.
I'd suggest the pragmatic approach of just not sending receipts for synthetic events
Would that mean an old client can't mark a room as read if the last timeline event is a synthetic one? Or would the receipt be saved locally, just not sent over federation?
tulir
added
the
needs-implementation
|
|
||
| `m.room.member` events can be redacted to remove abusive display names and avatars. This proposal changes how profile information is stored so it has implications on the redaction logic. It is more complex now because there is an impedance mismatch between clients and federation: clients will redact _synthetic events_ which have no representation over federation, so how is this redaction communicated to other servers? | ||
| - As today, moderators will redact the `m.room.member` event, however the semantics server-side changes. | ||
| - Redacting a member event automatically redacts any corresponding `m.room.user_profile` event if they exist **now OR in the future**. |
There was a problem hiding this comment.
I may have redacted some member events a few times, either because they thought it was funny to include the bee movie script in their display name, or just because it filled too much of my screen, or because i simply thought it was funny
Maybe it woud be nice to have a way to lift this automatic redaction manually
There was a problem hiding this comment.
Automatically kick and reinvite? It's going to be janky no matter what as we aren't optimising for this behaviour, so rather than have a new API we can get by with the existing APIs.
There was a problem hiding this comment.
If it won’t be applied to old rooms I’m fine with this, will just have to find a new practical joke to use in the future :p
|
|
||
| This MSC borrows and expands upon the ideas in MSC3883, specifically: | ||
|
|
||
| > Displayname/Avatar updates should be EDUs that trigger a /profile query. |
There was a problem hiding this comment.
Thoughts on what this would imply for turning off federation profile lookups? https://element-hq.github.io/synapse/latest/usage/configuration/config_documentation.html?highlight=profile#allow_profile_lookup_over_federation
MSC4170 (merged) says that profile info is required for shared rooms and public rooms, which seems to be enough to cover the usecases of these changes, but i'm not sure if it's been implemented that way yet in server implementations
What should happen if the server isn't allowed to look at the profile of a user?
There was a problem hiding this comment.
MSC4170 (merged) says that profile info is required for shared rooms and public rooms
This is only for the CS API. SS profile look-ups can be turned off entirely.
There was a problem hiding this comment.
I mean that doesn't make sense really though. If the CSAPI demands to get profile info for users who have shared rooms, then the SS API must also do that as that is the only way you can get that information for users on other servers.
There was a problem hiding this comment.
I agree but I think the difficulty is that with the current SS endpoint for profile look-up, the receiving server cannot verify that condition. It only sees the requesting server but not the requesting user.
For reference, here's the spec PR for MSC4170 in case we want to refine (or roll back) the wording before Matrix v1.13.
|
|
||
| ### Per-room nicks | ||
|
|
||
| To support per-room nicks, there exists a new endpoint: `POST /rooms/{roomID}/user_profile` which sends a new state event type `m.room.user_profile` into the room, with the `state_key` matching the logged in user's ID. This new event has the relevant `displayname` and `avatar_url` fields. |
There was a problem hiding this comment.
Can i send this as a room admin or similar?
Usually spammers use abusive nicknames / profile pics, but sometimes the username is abusive too, to combat this what i do currently is
- Redact the member event (to hopefully avoid inclusion in prev_state)
- Send a ban event setting the displayname to something unique (e.g. a hash)
This usually results in clients primarily displaying the fake display name i set, which is nice since mxid's are currently picked by the user. Do you think it makes sense to let room mods send this event with the state key of a banned user?
I believe this could be accomplished via MSC3757 and owned state events MSC3779. If it was based on those proposals it would also be possible to use a state key like @morguldir:sulian.eu_m.tz for extended profiles like MSC4175, it also saves this proposal from needing to change auth rules / set new defaults allowing per room profiles
|
|
||
| ## Unstable prefix | ||
|
|
||
| - Room version is `org.matrix.msc4218` based off room version 11. |
There was a problem hiding this comment.
Would it be possible to deploy this to existing room versions? I guess main problem would be that people wouldn't be allowed to set per-room profiles without admin intervention, and some out of date servers wouldn't know about the new profile mechanism, but this might be worth the tradeoff?
Personally I would prefer a drastically shorter auth chain than not being totally sure if someone is out of office for the weekend :p
There was a problem hiding this comment.
It technically could be done on existing room versions, but there is a change in semantics which you have already pointed out so it's polite to actually bump the version. If we wanted to enforce that you can't do member event profile change events then this would need auth rules changing, which would absolutely need a new version.
There was a problem hiding this comment.
Oh good point, enforcing it in the auth rules seems like a great idea, and yeah i agree that it’s polite to bump here anyway
Would that then be done via ensuring that the m.room.member content doesn’t contain anything but the membership info?
|
|
||
| If the receiving server cannot get the profile info for a user due to network issues, it must retry periodically. This can introduce a delay between when the user actually changed their name and it taking effect. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
An alternative missing here is creating a new PDU specifically for user profiles, separate from m.room.member. Sending profile changes as EDUs and synthesising m.room.member for clients feels rather involved, compared to introducing a new state event that would not be a part of auth chain. The current proposal tries to maintain back-compatibility of CS API; indeed, simple clients might continue working without changes. The majority of more complete ones will still have (for one) to incorporate the new way to do per-room nicks. And there's quite a machinery introduced on the server side. I wonder if having a new PDU would be that much of a problem for clients.
There was a problem hiding this comment.
I literally cover this in the MSC :D
This implies the new event type is NOT sent to clients, which is intentional as we do not wish to introduce needless breaking API changes to all clients, AND we want to preserve the existing property of seeing a new m.room.member join the room and immediately knowing that member's name/avatar: this atomicity requirement was why they were coupled in the first place. Because of these reasons, we do not go "all in" on the new event type and synthesise fake versions of this event to communicate profile updates, even though this may initially seem like a more consistent alternative.
A new event breaks atomicity.
| @@ -0,0 +1,72 @@ | |||
| # MSC4218: Improving performance of profile changes | |||
|
|
|||
| Many users in the Matrix ecosystem have seen how slow it is to change either their display name or their avatar. It is slow because it causes O(n) updates, one for each room the user is joined to. This MSC details a mechanism to improve performance by not sending O(n) updates. It does this by introducing a new concept called "Synthetic Events". This new concept can also be conveniently applied to [communicate state resolution deltas accurately to clients](https://github.com/matrix-org/matrix-spec/issues/1209). Also conveniently, decoupling profile changes from the membership of the user decreases the length of the auth chain for that user, which decreases the amount of data that is required to be fetched in order to join a room, thus improving performance on room joins as well. | |||
There was a problem hiding this comment.
Please wrap lines to ~100 characters
There was a problem hiding this comment.
Implementation requirements:
- Server
- Client(s)
turt2live
added
s2s
Rendered