Sliding Sync: when using $LAZY always send down all members for non-gappy syncs

[erikjohnston]

Currently, $LAZY only sends down membership events for senders of events in the timeline (if the membership hasn't previously been sent down). In particular, that means if a user's state changes and it is not in the timeline, then the client won't know the membership state has changed / been invalidated.

To make things easier for clients, lets change $LAZY to also send down all membership changes in non-gappy syncs (i.e. when limited: false). This allows clients to cache the membership list for as long as it doesn't get a gappy sync, but still ensures for large gaps the server doesn't need to send down all membership changes.

[ganfra]

Probably related issue on EXA : element-hq/element-x-android#3790

[MadLittleMods]

if a user's state changes and it is not in the timeline

@erikjohnston When does this happen? If someone changes their display name for example, the membership state event will show up in the timeline. And if the membership change happened outside of the range of the timeline_limit, then we are talking about a limited: true sync where this new logic doesn't apply anyway.

Is this targeting a scenario where some state comes over federation from some time ago which updates the current state but isn't in the timeline? (not sure this is even a thing because it will still get a recent stream_ordering which should make it come down /sync)

Probably related issue on EXA : element-hq/element-x-android#3790

@ganfra What's your exact theory for how this problem contributes to that issue?

[jmartinesp]

To clarify this a bit: the original reported issue was:

1. There is a request to join (a knock membership event) from Alice. This appears in the required_state and is taken into account by the client, and Alice's membership is updated.
2. Bob accepts/declines the request to join (so he invites/kicks Alice from the room). This membership change appears in the timeline, but not in the required_state, probably because the sender of those events (Bob) is not the member whose membership changed (Alice).
3. Some other client of Bob doesn't receive any new state event in required_state, so it still thinks Alice's membership is knock, instead of invite/leave.

Technically, this could be also solved by processing the state events in the timeline, but since those are not guaranteed to be up to date it could easily lead to inconsistencies in the clients.

[MadLittleMods]

@jmartinesp Perfect description! That makes a lot more sense 👌

So basically, we just need to fill in this FIXME

synapse/synapse/handlers/sliding_sync/__init__.py

Lines 958 to 961 in 80e39fd

	# Everyone in the timeline is relevant
	#
	# FIXME: We probably also care about invite, ban, kick, targets, etc
	# but the spec only mentions "senders".

I've made the changes with some tests in #17947 🚀

Technically, this could be also solved by processing the state events in the timeline, but since those are not guaranteed to be up to date it could easily lead to inconsistencies in the clients.

Correct choice 👍 Only rely on what the homeserver is telling you through required_state.

[MadLittleMods]

#17947 has been merged which should unblock the ElementX knocking use case 🚀

@erikjohnston highlighted that we should also handle membership changes from state reset scenarios (where the membership might change without a corresponding event). We can fix that up in a separate PR. #17732 is probably a prerequisite since it fixes up some spots where we forgot to bust the _curr_state_delta_stream_cache.

I won't be able to tackle this right away given other priorities.