Factor out a separate EventContext.for_outlier

Constructing an EventContext for an outlier is actually really simple, and
there's no sense in going via an `async` method in the `StateHandler`.

This also means that we can resolve a bunch of FIXMEs.

Author: richvdh

changelog.d/10883.misc

@@ -0,0 +1 @@
+Clean up some of the federation event authentication code for clarity.

synapse/events/snapshot.py

@@ -80,9 +80,7 @@ class EventContext:
 
             (type, state_key) -> event_id
 
-            FIXME: what is this for an outlier? it seems ill-defined. It seems like
-            it could be either {}, or the state we were given by the remote
-            server, depending on $THINGS
+            For an outlier, this is {}
 
             Note that this is a private attribute: it should be accessed via
             ``get_current_state_ids``. _AsyncEventContext impl calculates this
@@ -96,7 +94,7 @@ class EventContext:
 
             (type, state_key) -> event_id
 
-            FIXME: again, what is this for an outlier?
+            For an outlier, this is {}
 
             As with _current_state_ids, this is a private attribute. It should be
             accessed via get_prev_state_ids.
@@ -130,6 +128,14 @@ def with_state(
             delta_ids=delta_ids,
         )
 
+    @staticmethod
+    def for_outlier():
+        """Return an EventContext instance suitable for persisting an outlier event"""
+        return EventContext(
+            current_state_ids={},
+            prev_state_ids={},
+        )
+
     async def serialize(self, event: EventBase, store: "DataStore") -> dict:
         """Converts self to a type that can be serialized as JSON, and then
         deserialized by `deserialize`

synapse/handlers/federation.py

@@ -624,7 +624,7 @@ async def do_knock(
         # in the invitee's sync stream. It is stripped out for all other local users.
         event.unsigned["knock_room_state"] = stripped_room_state["knock_state_events"]
 
-        context = await self.state_handler.compute_event_context(event)
+        context = EventContext.for_outlier()
         stream_id = await self._federation_event_handler.persist_events_and_notify(
             event.room_id, [(event, context)]
         )
@@ -814,7 +814,7 @@ async def on_invite_request(
             )
         )
 
-        context = await self.state_handler.compute_event_context(event)
+        context = EventContext.for_outlier()
         await self._federation_event_handler.persist_events_and_notify(
             event.room_id, [(event, context)]
         )
@@ -843,7 +843,7 @@ async def do_remotely_reject_invite(
 
         await self.federation_client.send_leave(host_list, event)
 
-        context = await self.state_handler.compute_event_context(event)
+        context = EventContext.for_outlier()
         stream_id = await self._federation_event_handler.persist_events_and_notify(
             event.room_id, [(event, context)]
         )
@@ -1115,8 +1115,7 @@ async def _persist_auth_tree(
         events_to_context = {}
         for e in itertools.chain(auth_events, state):
             e.internal_metadata.outlier = True
-            ctx = await self.state_handler.compute_event_context(e)
-            events_to_context[e.event_id] = ctx
+            events_to_context[e.event_id] = EventContext.for_outlier()
 
         event_map = {
             e.event_id: e for e in itertools.chain(auth_events, state, [event])

synapse/handlers/federation_event.py

@@ -1221,7 +1221,7 @@ async def _auth_and_persist_fetched_events(
         async def prep(ev_info: _NewEventInfo) -> EventContext:
             event = ev_info.event
             with nested_logging_context(suffix=event.event_id):
-                res = await self._state_handler.compute_event_context(event)
+                res = EventContext.for_outlier()
                 res = await self._check_event_auth(
                     origin,
                     event,
@@ -1540,10 +1540,7 @@ async def _update_auth_events_and_context_for_auth(
                             event.event_id,
                             auth_event.event_id,
                         )
-                        missing_auth_event_context = (
-                            await self._state_handler.compute_event_context(auth_event)
-                        )
-
+                        missing_auth_event_context = EventContext.for_outlier()
                         missing_auth_event_context = await self._check_event_auth(
                             origin,
                             auth_event,

synapse/state/__init__.py

@@ -263,7 +263,9 @@ async def get_hosts_in_room_at_events(
     async def compute_event_context(
         self, event: EventBase, old_state: Optional[Iterable[EventBase]] = None
     ) -> EventContext:
-        """Build an EventContext structure for the event.
+        """Build an EventContext structure for a non-outlier event.
+
+        (for an outlier, call EventContext.for_outlier directly)
 
         This works out what the current state should be for the event, and
         generates a new state group if necessary.
@@ -278,35 +280,7 @@ async def compute_event_context(
             The event context.
         """
 
-        if event.internal_metadata.is_outlier():
-            # If this is an outlier, then we know it shouldn't have any current
-            # state. Certainly store.get_current_state won't return any, and
-            # persisting the event won't store the state group.
-
-            # FIXME: why do we populate current_state_ids? I thought the point was
-            # that we weren't supposed to have any state for outliers?
-            if old_state:
-                prev_state_ids = {(s.type, s.state_key): s.event_id for s in old_state}
-                if event.is_state():
-                    current_state_ids = dict(prev_state_ids)
-                    key = (event.type, event.state_key)
-                    current_state_ids[key] = event.event_id
-                else:
-                    current_state_ids = prev_state_ids
-            else:
-                current_state_ids = {}
-                prev_state_ids = {}
-
-            # We don't store state for outliers, so we don't generate a state
-            # group for it.
-            context = EventContext.with_state(
-                state_group=None,
-                state_group_before_event=None,
-                current_state_ids=current_state_ids,
-                prev_state_ids=prev_state_ids,
-            )
-
-            return context
+        assert not event.internal_metadata.is_outlier()
 
         #
         # first of all, figure out the state before the event