get rid of (most of) have_events from _update_auth_events_and_context_for_auth (#6468)

have_events was a map from event_id to rejection reason (or None) for events
which are in our local database. It was used as filter on the list of
event_ids being passed into get_events_as_list. However, since
get_events_as_list will ignore any event_ids that are unknown or rejected, we
can equivalently just leave it to get_events_as_list to do the filtering.

That means that we don't have to keep `have_events` up-to-date, and can use
`have_seen_events` instead of `get_seen_events_with_rejection` in the one place
we do need it.

changelog.d/6468.misc

@@ -0,0 +1 @@
+Refactor some code in the event authentication path for clarity.

synapse/handlers/federation.py

@@ -2122,14 +2122,9 @@ class FederationHandler(BaseHandler):
         #
         # we start by checking if they are in the store, and then try calling /event_auth/.
         if missing_auth:
-            # TODO: can we use store.have_seen_events here instead?
+            have_events = yield self.store.have_seen_events(missing_auth)
-            have_events = yield self.store.get_seen_events_with_rejections(missing_auth)
+            logger.debug("Events %s are in the store", have_events)
-            logger.debug("Found events %s in the store", have_events)
+            missing_auth.difference_update(have_events)
-            missing_auth.difference_update(have_events.keys())
-        else:
-            have_events = {}
-
-        have_events.update({e.event_id: "" for e in auth_events.values()})
 
         if missing_auth:
             # If we don't have all the auth events, we need to get them.
@@ -2175,9 +2170,6 @@ class FederationHandler(BaseHandler):
                     except AuthError:
                         pass
 
-                have_events = yield self.store.get_seen_events_with_rejections(
-                    event.auth_event_ids()
-                )
             except Exception:
                 logger.exception("Failed to get auth chain")
 
@@ -2207,39 +2199,33 @@ class FederationHandler(BaseHandler):
         # idea of them.
 
         room_version = yield self.store.get_room_version(event.room_id)
-        different_event_ids = [
-            d for d in different_auth if d in have_events and not have_events[d]
-        ]
 
-        if different_event_ids:
+        # XXX: currently this checks for redactions but I'm not convinced that is
-            # XXX: currently this checks for redactions but I'm not convinced that is
+        # necessary?
-            # necessary?
+        different_events = yield self.store.get_events_as_list(different_auth)
-            different_events = yield self.store.get_events_as_list(different_event_ids)
 
-            local_view = dict(auth_events)
+        local_view = dict(auth_events)
-            remote_view = dict(auth_events)
+        remote_view = dict(auth_events)
-            remote_view.update({(d.type, d.state_key): d for d in different_events})
+        remote_view.update({(d.type, d.state_key): d for d in different_events})
 
-            new_state = yield self.state_handler.resolve_events(
+        new_state = yield self.state_handler.resolve_events(
-                room_version,
+            room_version, [list(local_view.values()), list(remote_view.values())], event
-                [list(local_view.values()), list(remote_view.values())],
+        )
-                event,
-            )
 
-            logger.info(
+        logger.info(
-                "After state res: updating auth_events with new state %s",
+            "After state res: updating auth_events with new state %s",
-                {
+            {
-                    (d.type, d.state_key): d.event_id
+                (d.type, d.state_key): d.event_id
-                    for d in new_state.values()
+                for d in new_state.values()
-                    if auth_events.get((d.type, d.state_key)) != d
+                if auth_events.get((d.type, d.state_key)) != d
-                },
+            },
-            )
+        )
 
-            auth_events.update(new_state)
+        auth_events.update(new_state)
 
-            context = yield self._update_context_for_auth_events(
+        context = yield self._update_context_for_auth_events(
-                event, context, auth_events
+            event, context, auth_events
-            )
+        )
 
         return context
 

synapse/storage/data_stores/main/events_worker.py

@@ -783,40 +783,6 @@ class EventsWorkerStore(SQLBaseStore):
             yield self.runInteraction("have_seen_events", have_seen_events_txn, chunk)
         return results
 
-    def get_seen_events_with_rejections(self, event_ids):
-        """Given a list of event ids, check if we rejected them.
-
-        Args:
-            event_ids (list[str])
-
-        Returns:
-            Deferred[dict[str, str|None):
-                Has an entry for each event id we already have seen. Maps to
-                the rejected reason string if we rejected the event, else maps
-                to None.
-        """
-        if not event_ids:
-            return defer.succeed({})
-
-        def f(txn):
-            sql = (
-                "SELECT e.event_id, reason FROM events as e "
-                "LEFT JOIN rejections as r ON e.event_id = r.event_id "
-                "WHERE e.event_id = ?"
-            )
-
-            res = {}
-            for event_id in event_ids:
-                txn.execute(sql, (event_id,))
-                row = txn.fetchone()
-                if row:
-                    _, rejected = row
-                    res[event_id] = rejected
-
-            return res
-
-        return self.runInteraction("get_seen_events_with_rejections", f)
-
     def _get_total_state_event_counts_txn(self, txn, room_id):
         """
         See get_total_state_event_counts.