Fix bug where we superfluously asked for current state. Change API of /query_auth/ so that we don't duplicate events in the response.

5 files changed, 43 insertions, 49 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 3471afd7e7..37e31d2b6f 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -102,6 +102,8 @@ class Auth(object):
     def check_host_in_room(self, room_id, host):
         curr_state = yield self.state.get_current_state(room_id)
 
+        logger.debug("Got curr_state %s", curr_state)
+
         for event in curr_state:
             if event.type == EventTypes.Member:
                 try:
diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py
index 1173ca817b..e1539bd0e0 100644
--- a/synapse/federation/federation_client.py
+++ b/synapse/federation/federation_client.py
@@ -357,15 +357,10 @@ class FederationClient(object):
             for e in content["auth_chain"]
         ]
 
-        missing = [
-            (yield self._check_sigs_and_hash(self.event_from_pdu_json(e)))
-            for e in content.get("missing", [])
-        ]
-
         ret = {
             "auth_chain": auth_chain,
             "rejects": content.get("rejects", []),
-            "missing": missing,
+            "missing": content.get("missing", []),
         }
 
         defer.returnValue(ret)
diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py
index 845a07a3a3..84ed0a0ba0 100644
--- a/synapse/federation/federation_server.py
+++ b/synapse/federation/federation_server.py
@@ -252,11 +252,8 @@ class FederationServer(object):
                 e.get_pdu_json(time_now)
                 for e in ret["auth_chain"]
             ],
-            "rejects": content.get("rejects", []),
-            "missing": [
-                e.get_pdu_json(time_now)
-                for e in ret.get("missing", [])
-            ],
+            "rejects": ret.get("rejects", []),
+            "missing": ret.get("missing", []),
         }
 
         defer.returnValue(
@@ -372,7 +369,10 @@ class FederationServer(object):
                             logger.exception("Failed to get PDU")
                             fetch_state = True
             else:
-                fetch_state = True
+                prevs = {e_id for e_id, _ in pdu.prev_events}
+                seen = set(have_seen.keys())
+                if prevs - seen:
+                    fetch_state = True
         else:
             fetch_state = True
 
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index cc22f21cd1..35cad4182a 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -121,38 +121,18 @@ class FederationHandler(BaseHandler):
         )
         if not is_in_room and not event.internal_metadata.is_outlier():
             logger.debug("Got event for room we're not in.")
-
-            replication = self.replication_layer
-
-            if not state:
-                state, auth_chain = yield replication.get_state_for_room(
-                    origin, room_id=event.room_id, event_id=event.event_id,
-                )
-
-            if not auth_chain:
-                auth_chain = yield replication.get_event_auth(
-                    origin,
-                    context=event.room_id,
-                    event_id=event.event_id,
-                )
-
-            for e in auth_chain:
-                e.internal_metadata.outlier = True
-                try:
-                    yield self._handle_new_event(origin, e)
-                except:
-                    logger.exception(
-                        "Failed to handle auth event %s",
-                        e.event_id,
-                    )
-
             current_state = state
 
-        if state:
+        if state and auth_chain is not None:
             for e in state:
                 e.internal_metadata.outlier = True
                 try:
-                    yield self._handle_new_event(origin, e)
+                    auth_ids = [e_id for e_id, _ in e.auth_events]
+                    auth = {
+                        (e.type, e.state_key): e for e in auth_chain
+                        if e.event_id in auth_ids
+                    }
+                    yield self._handle_new_event(origin, e, auth_events=auth)
                 except:
                     logger.exception(
                         "Failed to handle state event %s",
@@ -809,18 +789,23 @@ class FederationHandler(BaseHandler):
             )
 
             # 3. Process any remote auth chain events we haven't seen.
-            for e in result.get("missing", []):
+            for missing_id in result.get("missing", []):
                 try:
-                    auth_ids = [e_id for e_id, _ in e.auth_events]
+                    for e in result["auth_chain"]:
+                        if e.event_id == missing_id:
+                            ev = e
+                            break
+
+                    auth_ids = [e_id for e_id, _ in ev.auth_events]
                     auth = {
                         (e.type, e.state_key): e for e in result["auth_chain"]
                         if e.event_id in auth_ids
                     }
-                    e.internal_metadata.outlier = True
+                    ev.internal_metadata.outlier = True
                     yield self._handle_new_event(
-                        origin, e, auth_events=auth
+                        origin, ev, auth_events=auth
                     )
-                    auth_events[(e.type, e.state_key)] = e
+                    auth_events[(ev.type, ev.state_key)] = ev
                 except AuthError:
                     pass
 
@@ -970,5 +955,5 @@ class FederationHandler(BaseHandler):
                 }
                 for e in base_remote_rejected
             },
-            "missing": missing_locals,
+            "missing": [e.event_id for e in missing_locals],
         })
diff --git a/synapse/state.py b/synapse/state.py
index d9fdfb34be..e6632978b5 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -166,10 +166,17 @@ class StateHandler(object):
         first is the name of a state group if one and only one is involved,
         otherwise `None`.
         """
+        logger.debug("resolve_state_groups event_ids %s", event_ids)
+
         state_groups = yield self.store.get_state_groups(
             event_ids
         )
 
+        logger.debug(
+            "resolve_state_groups state_groups %s",
+            state_groups.keys()
+        )
+
         group_names = set(state_groups.keys())
         if len(group_names) == 1:
             name, state_list = state_groups.items().pop()
@@ -205,6 +212,15 @@ class StateHandler(object):
             if len(v.values()) > 1
         }
 
+        logger.debug(
+            "resolve_state_groups Unconflicted state: %s",
+            unconflicted_state.values(),
+        )
+        logger.debug(
+            "resolve_state_groups Conflicted state: %s",
+            conflicted_state.values(),
+        )
+
         if event_type:
             prev_states_events = conflicted_state.get(
                 (event_type, state_key), []
@@ -240,10 +256,6 @@ class StateHandler(object):
             1. power levels
             2. memberships
             3. other events.
-
-        :param conflicted_state:
-        :param auth_events:
-        :return:
         """
         resolved_state = {}
         power_key = (EventTypes.PowerLevels, "")