`/sync` and `/members` do not return "current state"

[richvdh]

Consider a DAG like this:

             E1
           ↗    ↖
          |      S2
          |
        --|---
          |
          E3

The client then initialsyncs, with limit=1 (or maybe there are lots of events at E3), represented by the horizontal dashed line.

The "current state" (as defined by "what will be the state of the room if I send an event right now", which is what is important for encryption, etc) includes S2. However, the result from /sync does not include S2 (and will not, at least until there is an event in the DAG which joins the forks).

[richvdh]

In general, this seems like a major problem. As long as there is no event in the DAG which references S2 as a prev_event, S2 will not be served to the client. If S2 is, say, a membership event, then this could lead to UTDs.

[kegsay]

Specficially this would in the common case cause a single UTD, as the act of sending the encrypted message would merge the forks and cause S2 to arrive on the client. The UTD cause would be not sharing room keys with the new user, for potentially a long time after they joined.

[richvdh]

Specficially this would in the common case cause a single UTD,

Unfortunately I think it's considerably worse than that. Per https://github.com/element-hq/synapse/issues/16941, Synapse still won't send down S2 to the client even after the DAG is merged. I've proposed an improvement to that, but it still doesn't help clients which have lazy-loading enabled (which is basically all the ones we care about).

[richvdh]

This also extends to /members (at least when called with an ?at= param, which is what you have to do for lazy-loading).

[richvdh]

As long as there is no event in the DAG which references S2 as a prev_event, S2 will not be served to the client. If S2 is, say, a membership event, then this could lead to UTDs.

It's not even clear that events that reference S2 are sufficient to resolve this situation. Consider:

             E1
           ↗    ↖
          |      S2
          |       |
        --|-------|--
          |       |
          E3      |
          |       |
        --|-------|--
          |       |
          E4      |
          |       E5
          E6

What happens now? I'm pretty sure that S2 is not returned here.

Fundamentally, the expected behaviour is very poorly defined.

[erikjohnston]

The correct fix for all of this (probably) is to switch to using current_state_delta_stream table to track the state changes, rather than looking at the state at individual events. This is not a small piece of work.

[kegsay]

@erikjohnston would it help at all that at least for SSS we only care about returning the current state? We may not be able to fix it for sync v2 easily, but can we for SSS?

[erikjohnston]

@erikjohnston would it help at all that at least for SSS we only care about returning the current state? We may not be able to fix it for sync v2 easily, but can we for SSS?

Yup! The SSS implementation actually tracks current state properly