-
-
Notifications
You must be signed in to change notification settings - Fork 119
Clarify that stripped state in /sync
response must include m.room.member
event of user
#2181
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
base: main
Are you sure you want to change the base?
Conversation
…mber` event of user And that event has the same format as in join rooms, with `event_id` and `origin_server_ts`. This has always been the case in homeserver implementations. Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
We spent quite a bit of time around Matrix 1.0 to ensure stripped state was in fact stripped state, including noting that Synapse in particular at the time over-populated events. This feels like a case where the ecosystem has standardized around Synapse's behaviour rather than the spec itself, but may indeed be reasonable to land as-is. I'm currently torn on whether to ask for an MSC - thoughts are welcome. |
From my point of view there are 2 main factors why it shouldn't be stripped:
If ~all servers have already implemented it the way this PR describes, I'm not sure there's any benefit to an MSC |
MUST also include the `m.room.member` event of the user with a membership of | ||
`knock`, and using the same event format as joined rooms with the `event_id` and | ||
`origin_server_ts` fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are knocks included in this, for consistency? Do servers do this currently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Synapse does the same, Dendrite doesn't support knocking it seems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checked, Conduit sends the knock event using the sync format if it's remote, and stripped if it's a local knock, with the former being a mistake really. 🫣 as stripped state, nevermind.
I am sorry, I actually made a mistake in the PR description, Conduit and forks only send the stripped member event. |
The [stripped state events](/client-server-api/#stripped-state) that form the | ||
invite state. | ||
MUST also include the `m.room.member` event of the user with a membership of | ||
`invite`, and using the same event format as joined rooms with the `event_id` | ||
and `origin_server_ts` fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to change more words here. Something like:
A list of events comprising:
* The [stripped state](/client-server-api/#stripped-state) of the room.
* The user's current `m.room.member` event (with membership of `invite`), using the normal `ClientEventWithoutRoomID` format.
Also we'll need to change the description
on the InviteState
object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we need to be more explicit about the difference between both of those, because the m.room.member
should be a MUST, while the other events in the stripped state are a SHOULD.
Given the inconsistency between servers, I'm leaning to the idea that an MSC would be better, if only so that we can discuss which is the correct behaviour here. |
To be clear, what would be the goal of the MSC? Just say that the Because currently the spec doesn't mention this event at all in the stripped state, but it should theoretically be required in the |
I guess that's up to the author of the MSC: they need to convince readers that it should be one format rather than the other. |
I opened MSC4319 for this. |
And that event has the same format as in join rooms, with
event_id
andorigin_server_ts
, both forinvite_state
andknock_state
.This is a clarification because it has always been the case in some homeserver implementations. It's a bit hard to find places in code to point to because there is no direct link between the function where the format of the event can be seen and the function where the sync response is built.
PUT /v1/invite
federation endpoint request format with the stripped state inunsigned
and then build the stripped state for/sync
by extracting it fromunsigned
, and adding the rest of the event to the list:Dendrite has a test that makes the expected output format clear:
https://github.com/element-hq/dendrite/blob/8d2da78744387e55c28bb8925ae0cc70dd3e02e3/syncapi/types/types_test.go#L86
Fixes #2138.
Pull Request Checklist