From 7d1bd7f8d24a0de4ff9d55f7f614ba2706a27958 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 28 Jul 2022 15:06:26 +0100 Subject: [PATCH 1/3] Faster joins: check event auth again during resync Make sure that we re-check the auth rules during state resync, otherwise rejected events get un-rejected. --- synapse/handlers/federation_event.py | 29 +++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 2ba2b1527eac..bcc755a376f0 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -581,6 +581,13 @@ async def update_state_for_partial_state_event( event.event_id, ) return + + # since the state at this event has changed, we should now re-evaluate + # whether it should have been rejected. We must already have all of the + # auth events (from last time we went round this path), so there is no + # need to pass the origin. + await self._check_event_auth(None, event, context) + await self._store.update_state_for_partial_state_event(event, context) self._state_storage_controller.notify_event_un_partial_stated( event.event_id @@ -1624,13 +1631,15 @@ async def prep(event: EventBase) -> None: ) async def _check_event_auth( - self, origin: str, event: EventBase, context: EventContext + self, origin: Optional[str], event: EventBase, context: EventContext ) -> None: """ Checks whether an event should be rejected (for failing auth checks). Args: - origin: The host the event originates from. + origin: The host the event originates from. This is used to fetch + any missing auth events. It can be set to None, but only if we are + sure that we already have all the auth events. event: The event itself. context: The event context. @@ -1876,7 +1885,7 @@ async def _check_for_soft_fail( event.internal_metadata.soft_failed = True async def _load_or_fetch_auth_events_for_event( - self, destination: str, event: EventBase + self, destination: Optional[str], event: EventBase ) -> Collection[EventBase]: """Fetch this event's auth_events, from database or remote @@ -1892,12 +1901,19 @@ async def _load_or_fetch_auth_events_for_event( Args: destination: where to send the /event_auth request. Typically the server that sent us `event` in the first place. + + If this is None, no attempt is made to load any missing auth events: + rather, an AssertionError is raised if there are any missing events. + event: the event whose auth_events we want Returns: all of the events listed in `event.auth_events_ids`, after deduplication Raises: + AssertionError if some auth events were missing and no `destination` was + supplied. + AuthError if we were unable to fetch the auth_events for any reason. """ event_auth_event_ids = set(event.auth_event_ids()) @@ -1909,6 +1925,13 @@ async def _load_or_fetch_auth_events_for_event( ) if not missing_auth_event_ids: return event_auth_events.values() + if destination is None: + # this shouldn't happen: destination must be set unless we know we have already + # persisted the auth events. + raise AssertionError( + "_load_or_fetch_auth_events_for_event() called with no destination for " + "an event with missing auth_events" + ) logger.info( "Event %s refers to unknown auth events %s: fetching auth chain", From a3cbbb2a5e1b946a0d8e4cd07d131f82201d7f41 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 28 Jul 2022 15:08:31 +0100 Subject: [PATCH 2/3] Avoid reading the state group of rejected events Reading `EventContext.state_group` on a rejected event isn't valid (its value is actually a dummy state group which *includes* the rejected event). --- synapse/storage/databases/main/state.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/state.py b/synapse/storage/databases/main/state.py index 9674c4a7573b..f70705a0af19 100644 --- a/synapse/storage/databases/main/state.py +++ b/synapse/storage/databases/main/state.py @@ -419,13 +419,15 @@ def _update_state_for_partial_state_event_txn( # anything that was rejected should have the same state as its # predecessor. if context.rejected: - assert context.state_group == context.state_group_before_event + state_group = context.state_group_before_event + else: + state_group = context.state_group self.db_pool.simple_update_txn( txn, table="event_to_state_groups", keyvalues={"event_id": event.event_id}, - updatevalues={"state_group": context.state_group}, + updatevalues={"state_group": state_group}, ) self.db_pool.simple_delete_one_txn( @@ -440,7 +442,7 @@ def _update_state_for_partial_state_event_txn( txn.call_after( self._get_state_group_for_event.prefill, (event.event_id,), - context.state_group, + state_group, ) From 5f448b81487642e5e0100d38f70c01778164ad1d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 28 Jul 2022 15:15:27 +0100 Subject: [PATCH 3/3] changelog --- changelog.d/13413.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13413.bugfix diff --git a/changelog.d/13413.bugfix b/changelog.d/13413.bugfix new file mode 100644 index 000000000000..a0ce88427433 --- /dev/null +++ b/changelog.d/13413.bugfix @@ -0,0 +1 @@ +Faster room joins: fix a bug which caused rejected events to become un-rejected during state syncing. \ No newline at end of file