Avoid putting rejected events in room state

[squahtx]

When receiving an event over federation which has prev events not known
locally, Synapse will request the state before the unknown prev events
from a remote server. It is possible for the remote server to disagree
on the rejected-ness of events and return a state that includes locally
rejected events.

During state resolution, we replay state events and re-evaluate the part
of their auth that checks against room state. Thus it is possible for
events that have been rejected due to citing rejected auth events to be
accepted into the current room state at the forward extremities.

While the spec implies that this may be intended behaviour depending on
the reason the auth event was rejected, this creates problems for
Synapse, since we assume that previously rejected events cannot make it
into room state and do not populate tables such as room_memberships
for them.

For now, we amend state resolution to not admit previously rejected
events in the calculated state.

---

Previous notes

It's unclear to me whether the proposed change to state resolution is valid.

The spec says

Events that have been rejected due to failing auth based on the state at the event (rather than based on their auth chain) are handled as usual by the algorithm, unless otherwise specified.

which seems to imply that there are a class of rejected events that should make their way into the current room state.

The spec also says

Note that no events rejected due to failure to auth against their auth chain should appear in the process, as they should not appear in state (the algorithm only uses events that appear in either the state sets or in the auth chain of the events in the state sets).

RATIONALE
This helps ensure that different servers’ view of state is more likely to converge, since rejection state of an event may be different. This can happen if a third server gives an incorrect version of the state when a server joins a room via it (either due to being faulty or malicious). Convergence of state is a desirable property as it ensures that all users in the room have a (mostly) consistent view of the state of the room. If the view of the state on different servers diverges it can lead to bifurcation of the room due to e.g. servers disagreeing on who is in the room.

which seems to imply that events rejected because their auth events have been rejected should not appear in the calculated state (regardless of the reason their auth events have been rejected?). But then the rationale doesn't really work out, since chains of auth events rejected due to citing rejected auth events will remain rejected, regardless of the rejection reason at the start of the chain.

---

When receiving an event over federation which has prev events not known
locally, Synapse will request the state before the unknown prev events
from a remote server. It is possible for the remote server to disagree
on the rejected-ness of events and return a state that includes locally
rejected events.

I think it's possible for parts of the DAG to have state that includes rejected events and modifying state resolution doesn't change this. I'm unsure about the implications of this.

[squahtx]

Some help making sense of what the spec says with respect to state res would be appreciated!

[squahtx]

@erikjohnston believes that Synapse does not handle rejected events in accordance with the spec. For now, we'll ensure that previously rejected events do not get admitted into state, otherwise lots of Synapse will break.