Exclude OOB memberships from the federation sender

[richvdh]

As the comment says, there is no need to process such events, and indeed we need to avoid doing so.

Fixes #12509.

[richvdh]

The new error was introduced by #12191. Until that point, these events would be silently ignored, because _get_state_group_for_events, and hence get_hosts_in_room_at_events would return an empty list for an outlier.

[DMRobertson]

Code change looks fine, but I don't have much context, so you're getting loads of questions.

Does this mean that:

• we were sending events of type (2), (2a) and (3) out over federation prior to Synapse 1.57?
• If so, presumably they ought to be rejected by other servers?

[DMRobertson]

Is out-of-band membership a spec-defined or Synapse-internal term? (Seems to be the latter.) Is there a reference for what they are somewhere?

The 3.5 cases below seem to enumerate them and explain why we don't want to send them out. It might be useful to have this list of 3.5 event types here (a more authoritative place?):

synapse/synapse/events/__init__.py

Lines 214 to 223 in c027bc0

	def is_out_of_band_membership(self) -> bool:
	"""Whether this is an out of band membership, like an invite or an invite
	rejection. This is needed as those events are marked as outliers, but
	they still need to be processed as if they're new events (e.g. updating
	invite state in the database, relaying to clients, etc).
	(Added in synapse 0.99.0, so may be unreliable for events received before that)
	"""
	return self._dict.get("out_of_band_membership", False)

[clokep]

Is out-of-band membership a spec-defined or Synapse-internal term? (Seems to be the latter.) Is there a reference for what they are somewhere?

https://matrix-org.github.io/synapse/develop/development/room-dag-concepts.html#out-of-band-membership-events has some info on it.

[richvdh]

The 3.5 cases below seem to enumerate them and explain why we don't want to send them out. It might be useful to have this list of 3.5 event types here (a more authoritative place?):

The 3.5 cases here are intended as a proof by enumeration that it's correct to drop these events in the federation sender, rather than acting as an authoritative list of what OOB memberships are.

The authoritative source for compiling this list was searching the codebase for out_of_band_membership = True (there are four such hits, corresponding to cases (1), the federation and local cases for (2), and (3)). I could add such a list to is_out_of_band_membership, or the DAG concepts doc, but it could run the risk of getting out of date.

[DMRobertson]

Thanks, that makes sense. Perhaps just the introductory sentence from https://matrix-org.github.io/synapse/develop/development/room-dag-concepts.html#out-of-band-membership-events

A special case of outlier events are some membership events for federated rooms that we aren't full members of.

would help illuminate is_out_of_band_membership? Not going to block this on that though.

[richvdh]

I've updated the docstring for is_out_of_band_membership in 947f651.

[DMRobertson]

And it's this local fallback event that is considered out-of-band?

[richvdh]

No, both the federated and local cases are out-of-band.

Any thoughts on how I can reword this paragraph to make this clear? How about:

                    # (2) rejections of invites to federated rooms - either remotely
                    #     or locally generated. (Such rejections are normally
                    #     created via federation, in which case the remote server is
                    #     responsible for sending out the rejection. If that fails,
                    #     we'll create a leave event locally, but that's only really
                    #     for the benefit of the invited user - we don't have enough
                    #     information to send it out over federation).

[DMRobertson]

I like the rework. But the underlying difficulty I'm having here is that I don't grok the meaning of out of band. It makes me think of an event that has been received by this server over some non-Matrix protocol, e.g. carrier pigeon.

[richvdh]

it is maybe not the best term (see #4405 (review), which is where we expanded the idea beyond case (1).)

It's "out of band" inasmuch as it's not sent over the regular federation/v1/send API.

[DMRobertson]

There are only two hard problems, after all.

[richvdh]

I've updated the wording here in ca5b502.

[richvdh]

Does this mean that:

* we were sending events of type (2), (2a) and (3) out over federation prior to Synapse 1.57?

No. Such events are always (I think) outliers, which meant that, pre-1.57, get_hosts_in_room_at_events would return an empty list (because resolve_state_groups_for_events would return a _StateCacheEntry with an empty state, because get_state_groups_ids would return an empty list of state groups).

1.57 changed the behaviour of get_hosts_in_room_at_events to raise an exception for an outlier, instead of returning an empty list, hence the problem.

[DMRobertson]

I think I'm happy with this. Thanks for explaining.