Don't query auth if the only difference is events that were rejected due to auth.

1 files changed, 72 insertions, 56 deletions

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 1b709e5e24..b13b7c7701 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -791,12 +791,12 @@ class FederationHandler(BaseHandler):
     @log_function
     def do_auth(self, origin, event, context, auth_events):
         # Check if we have all the auth events.
-        res = yield self.store.have_events(
+        have_events = yield self.store.have_events(
             [e_id for e_id, _ in event.auth_events]
         )
 
         event_auth_events = set(e_id for e_id, _ in event.auth_events)
-        seen_events = set(res.keys())
+        seen_events = set(have_events.keys())
 
         missing_auth = event_auth_events - seen_events
 
@@ -839,6 +839,11 @@ class FederationHandler(BaseHandler):
                             auth_events[(e.type, e.state_key)] = e
                     except AuthError:
                         pass
+
+                have_events = yield self.store.have_events(
+                    [e_id for e_id, _ in event.auth_events]
+                )
+                seen_events = set(have_events.keys())
             except:
                 # FIXME:
                 logger.exception("Failed to get auth chain")
@@ -852,64 +857,75 @@ class FederationHandler(BaseHandler):
             # Do auth conflict res.
             logger.debug("Different auth: %s", different_auth)
 
-            # 1. Get what we think is the auth chain.
-            auth_ids = self.auth.compute_auth_events(
-                event, context.current_state
-            )
-            local_auth_chain = yield self.store.get_auth_chain(auth_ids)
-
-            try:
-                # 2. Get remote difference.
-                result = yield self.replication_layer.query_auth(
-                    origin,
-                    event.room_id,
-                    event.event_id,
-                    local_auth_chain,
-                )
-
-                seen_remotes = yield self.store.have_events(
-                    [e.event_id for e in result["auth_chain"]]
+            # Only do auth resolution if we have something new to say.
+            # We can't rove an auth failure.
+            do_resolution = False
+            for e_id in different_auth:
+                if e_id in have_events:
+                    if have_events[e_id] != RejectedReason.AUTH_ERROR:
+                        do_resolution = True
+                        break
+
+            if do_resolution:
+                # 1. Get what we think is the auth chain.
+                auth_ids = self.auth.compute_auth_events(
+                    event, context.current_state
                 )
+                local_auth_chain = yield self.store.get_auth_chain(auth_ids)
 
-                # 3. Process any remote auth chain events we haven't seen.
-                for ev in result["auth_chain"]:
-                    if ev.event_id in seen_remotes.keys():
-                        continue
-
-                    if ev.event_id == event.event_id:
-                        continue
-
-                    try:
-                        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
-                        }
-                        ev.internal_metadata.outlier = True
-
-                        logger.debug(
-                            "do_auth %s different_auth: %s",
-                            event.event_id, e.event_id
-                        )
+                try:
+                    # 2. Get remote difference.
+                    result = yield self.replication_layer.query_auth(
+                        origin,
+                        event.room_id,
+                        event.event_id,
+                        local_auth_chain,
+                    )
 
-                        yield self._handle_new_event(
-                            origin, ev, auth_events=auth
-                        )
+                    seen_remotes = yield self.store.have_events(
+                        [e.event_id for e in result["auth_chain"]]
+                    )
 
-                        if ev.event_id in event_auth_events:
-                            auth_events[(ev.type, ev.state_key)] = ev
-                    except AuthError:
-                        pass
+                    # 3. Process any remote auth chain events we haven't seen.
+                    for ev in result["auth_chain"]:
+                        if ev.event_id in seen_remotes.keys():
+                            continue
+
+                        if ev.event_id == event.event_id:
+                            continue
+
+                        try:
+                            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
+                            }
+                            ev.internal_metadata.outlier = True
+
+                            logger.debug(
+                                "do_auth %s different_auth: %s",
+                                event.event_id, e.event_id
+                            )
+
+                            yield self._handle_new_event(
+                                origin, ev, auth_events=auth
+                            )
+
+                            if ev.event_id in event_auth_events:
+                                auth_events[(ev.type, ev.state_key)] = ev
+                        except AuthError:
+                            pass
 
-            except:
-                # FIXME:
-                logger.exception("Failed to query auth chain")
+                except:
+                    # FIXME:
+                    logger.exception("Failed to query auth chain")
 
-            # 4. Look at rejects and their proofs.
-            # TODO.
+                # 4. Look at rejects and their proofs.
+                # TODO.
 
-            context.current_state.update(auth_events)
-            context.state_group = None
+                context.current_state.update(auth_events)
+                context.state_group = None
 
         try:
             self.auth.check(event, auth_events=auth_events)
@@ -1028,9 +1044,9 @@ class FederationHandler(BaseHandler):
         for e in base_remote_rejected:
             reason = yield self.store.get_rejection_reason(e.event_id)
             if reason is None:
-                # FIXME: ERRR?!
-                logger.warn("Could not find reason for %s", e.event_id)
-                raise RuntimeError("Could not find reason for %s" % e.event_id)
+                # TODO: e is not in the current state, so we should
+                # construct some proof of that.
+                continue
 
             reason_map[e.event_id] = reason