Implement MSC2175 to stop adding the creator field.

[clokep]

This implements MSC2175 to stop adding the creator field to the m.room.create event.

I couldn't figure out an overly trivial way to test this -- we don't seem to actually check that we add this to current create events?

Fixes #7487.

[DMRobertson]

Thanks for this---looks sane to me. I've left a few nitpicks which I'll leave to your discretion.

[DMRobertson]

Aside: Is the rooms.creator column useful? I guess it avoids looking up the create event and from the state and event_json tables (though I'd bet we're already looking in those tables anyway).

[clokep]

I'm fairly certain that we do use it, but it seems like it has been around forever.

[clokep]

Hmm, maybe not -- it was added in #10697.

[clokep]

It seems to be mentioned on #10737 too.

[DMRobertson]

Is the existing logic correct? Shouldn't it be if not room_version.msc2716_historical OR (...)?

(A => B iff not A or B.)

Additionally: I think (strictly speaking) that this makes MSC2716 depend on MSC2175?

[clokep]

Is the existing logic correct? Shouldn't it be if not room_version.msc2716_historical OR (...)?

I'm not sure, would need to ask @MadLittleMods.

Additionally: I think (strictly speaking) that this makes MSC2716 depend on MSC2175?

Yes, I guess this should probably use the same or logic that we use in other spots.

[clokep]

(I fixed this to break the dependency. 🤷 )

[MadLittleMods]

I think the logic correct. This is taking a lot of squinting so it could probably be clearer but here is the breakdown:

• If the room version supports MSC2716: we always process marker events (no early return)
• If the room version does not support MSC2716 (existing room versions), the MSC2716 experimental feature has to be enabled and the event has to be from the room creator

If we used OR here, then the second case with existing room versions would never be work.

---

synapse/synapse/handlers/federation_event.py

Lines 1522 to 1524 in b682443

	if not room_version.msc2716_historical and (
	not self._config.experimental.msc2716_enabled
	or marker_event.sender != room_creator

If we break these down into conditions, then the current boolean logic looks like: not V and (not E or not C)

is_supported_room_version = room_version.msc2716_historical # V
is_msc2716_enabled = self._config.experimental.msc2716_enabled # E
is_event_from_room_creator = marker_event.sender == room_creator # C

If we add some labels and derive a positive should_process_event condition which we can then later negate, then the boolean logic looks like not (V or (E and C)) which simplifies to the exact same logic above (see alternate form listed on Wolfram Alpha)

This probably does make more sense though (if we find it clearer, feel free to update the 3 spots where we do this):

is_event_eligible_to_process_in_existing_room_versions = is_msc2716_enabled and is_event_from_room_creator
should_process_event = is_supported_room_version or is_event_eligible_to_process_in_existing_room_versions
if not should_process_event:
    return

[DMRobertson]

Aha, thanks for double checking. (I'd missed that this logic was guarding an early return, amongst other things.)

[DMRobertson]

Looks like there's a CI thing to fix though?