Handle outliers in /federation/v1/event

[richvdh]

The intention here is to avoid doing state lookups for outliers in /_matrix/federation/v1/event. Unfortunately that's expanded into something of a rewrite of filter_events_for_server, which ended up trying to do that operation in a couple of places.

First of all, note that there is a long-standing fudge here, where we return outliers via /_matrix/federation/v1/event to any server that is currently in the room, even though we don't know the history_visibility in force at the time of the event, nor if the server was in the room at that time (which are normally prerequisites for allowing this). This has to be the case, because the federation protocol relies on being able to pull auth_events for a given event in this manner (and we have no way to show that the server should not be able to see those events).

It currently works somewhat via the back door, because get_state_ids_for_events returns an empty set for outliers; that equates to a missing m.room.history_visibility event, which equates to shared, ie "visible to any server which is now in the room".

filter_events_for_server does two state lookups: first of all it just looks up the history_visibility events. If those are all unrestricted, there is a fast path for it to bail out. Otherwise, it does a second state lookup, including (currently) both history_visibility events and membership events.

Rather than apply the same special-casing for outliers twice, I've refactored the code so that, rather than having a separate fast path, I've changed the "slow" path so that it only looks up membership events for events where it has already decided that there is a restricted history visibility. That therefore side-steps the outlier issue for that second state lookup.

[DMRobertson]

This was tricky to follow. I think the rewrite looks sane, and excluding the outliers seems to be a one line change (+377?).

WIth that said:

[DMRobertson]

Found the nested comprehensions tricky to read, but I don't think I can see a neater way to write it.

[richvdh]

Yeah. We could build the set incrementally:

visibility_ids = set()
for state_ids in event_to_state_ids.values():
    vis_event_id = state_ids.get(_HISTORY_VIS_KEY)
    if vis_event_id:
        visibility_ids.add(vis_event_id)

... but I'm assuming (without checking) that the comprehension is faster.

[richvdh]

@DMRobertson: Thanks for ploughing through this. Certainly it's not easy to follow from the diff. I've tried to answer your questions, though it looks like you've already grokked it pretty well.

[DMRobertson]

Aside from #12332 (comment) LGTM.

Is it worth rebasing #12191 on top of the small changes to this? Either way I can take a look at that one next.