Clarify that stripped state in /sync response must include m.room.member event of user

[zecakeh]

And that event has the same format as in join rooms, with event_id and origin_server_ts, both for invite_state and knock_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.

• Synapse and Dendrite store the member event using the PUT /v1/invite federation endpoint request format with the stripped state in unsigned and then build the stripped state for /sync by extracting it from unsigned, and adding the rest of the event to the list:
  • https://github.com/element-hq/synapse/blob/458e6410e8388fbc6b60b5575850405939bc53eb/synapse/rest/client/sync.py#L450-L462
  • https://github.com/element-hq/dendrite/blob/8d2da78744387e55c28bb8925ae0cc70dd3e02e3/syncapi/types/types.go#L530-L574
• Conduit, Grapevine and Continuwuity create the stripped state, including the stripped member event, when receiving the invite: https://gitlab.com/famedly/conduit/-/blob/ed5b0514f52f848e53785dee5fe8de63780cf092/src/api/server_server.rs#L2160-2172

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

• Pull request includes a changelog file
• Pull request includes a sign off
• Pull request is classified as 'other changes'

[turt2live]

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.

[tulir]

From my point of view there are 2 main factors why it shouldn't be stripped:

• The timestamp of the invite is useful information for clients/users.
• The server already has all the information and there's no reason to withhold it from clients. The invite event isn't sent as stripped over federation like other state is (it can't be, considering the recipient server has to sign the PDU).
  • The AS API also sends invites in the top-level events array, so they can't be stripped there.

If ~all servers have already implemented it the way this PR describes, I'm not sure there's any benefit to an MSC

[Kladki]

Why are knocks included in this, for consistency? Do servers do this currently?

[zecakeh]

Yes, Synapse does the same, Dendrite doesn't support knocking it seems.

[Kladki]

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.

[zecakeh]

I am sorry, I actually made a mistake in the PR description, Conduit and forks only send the stripped member event.

[richvdh]

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.

[zecakeh]

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.

[richvdh]

I am sorry, I actually made a mistake in the PR description, Conduit and forks only send the stripped member event.

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.

[zecakeh]

To be clear, what would be the goal of the MSC? Just say that the m.room.member event should use the ClientEventWithoutRoomID format instead of the stripped state format?

Because currently the spec doesn't mention this event at all in the stripped state, but it should theoretically be required in the invite_state and knock_state, because it is the reason why a room appears in invite or knock in the first place.

[richvdh]

To be clear, what would be the goal of the MSC? Just say that the m.room.member event should use the ClientEventWithoutRoomID format instead of the stripped state format?

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.

[zecakeh]

I opened MSC4319 for this.

[richvdh]

Closing this for now, while we wait for the MSC to land.