Review comments

synapse/event_auth.py

@@ -216,7 +216,7 @@ async def check_state_independent_auth_rules(
             known_prev_state_event_ids = set(prev_state_events)
             raise AssertionError(
                 f"Event {event.event_id} has unknown prev_state_events "
-                + f"{len(prev_state_events)} != {len(prev_state_events_ids)} "
+                + f"({len(prev_state_events)}/{len(prev_state_events_ids)} known)"
                 + f"{prev_state_events_ids - known_prev_state_event_ids} missing "
                 + f"out of {prev_state_events_ids}"
             )

synapse/handlers/admin.py

@@ -32,7 +32,7 @@ import attr
 
 from synapse.api.constants import Direction, EventTypes, Membership
 from synapse.api.errors import SynapseError
-from synapse.events import EventBase
+from synapse.events import EventBase, FrozenEventVMSC4242
 from synapse.types import (
     JsonMapping,
     Requester,
@@ -487,10 +487,8 @@ class AdminHandler:
                 try:
                     prev_state_events = None
                     if room_version.msc4242_state_dags:
-                        # TODO(kegan): better way to typecast and get at this field?
-                        prev_state_events = event.get_dict().get(
-                            "prev_state_events", None
-                        )
+                        assert isinstance(event, FrozenEventVMSC4242)
+                        prev_state_events = event.prev_state_events
                         assert prev_state_events is not None, (
                             "Parent event of redaction has no `prev_state_events` which should be impossible as `prev_state_events` is a required field in MSC4242 rooms"
                         )

synapse/handlers/room.py

@@ -1486,8 +1486,8 @@ class RoomCreationHandler:
 
         # the most recently created event
         prev_event: list[str] = []
-        # the most recently created state event
-        prev_state_event: list[str] | None = (
+        # This should be the most recently created state event as we create each event
+        prev_state_events: list[str] | None = (
             [] if room_version.msc4242_state_dags else None
         )
 
@@ -1517,7 +1517,7 @@ class RoomCreationHandler:
             """
             nonlocal depth
             nonlocal prev_event
-            nonlocal prev_state_event
+            nonlocal prev_state_events
 
             # Create the event dictionary.
             event_dict = {"type": etype, "content": content}
@@ -1531,7 +1531,7 @@ class RoomCreationHandler:
                 creator,
                 event_dict,
                 prev_event_ids=prev_event,
-                prev_state_events=prev_state_event,
+                prev_state_events=prev_state_events,
                 depth=depth,
                 # Take a copy to ensure each event gets a unique copy of
                 # state_map since it is modified below.
@@ -1543,7 +1543,7 @@ class RoomCreationHandler:
             prev_event = [new_event.event_id]
             state_map[(new_event.type, new_event.state_key)] = new_event.event_id
             if room_version.msc4242_state_dags and event_exists_in_state_dag(new_event):
-                prev_state_event = [new_event.event_id]
+                prev_state_events = [new_event.event_id]
             return new_event, new_unpersisted_context
 
         preset_config, config = self._room_preset_config(room_config)
@@ -1577,7 +1577,7 @@ class RoomCreationHandler:
         )
         last_sent_event_id = ev.event_id
         if room_version.msc4242_state_dags:
-            prev_state_event = [ev.event_id]
+            prev_state_events = [ev.event_id]
 
         member_event_id, _ = await self.room_member_handler.update_membership(
             creator,
@@ -1589,11 +1589,11 @@ class RoomCreationHandler:
             new_room=True,
             prev_event_ids=[last_sent_event_id],
             depth=depth,
-            prev_state_events=prev_state_event,
+            prev_state_events=prev_state_events,
         )
         prev_event = [member_event_id]
         if room_version.msc4242_state_dags:
-            prev_state_event = [member_event_id]
+            prev_state_events = [member_event_id]
 
         # update the depth and state map here as the membership event has been created
         # through a different code path

synapse/state/__init__.py

@@ -239,32 +239,6 @@ class StateHandler:
         )
         return await ret.get_state(self._state_storage_controller, state_filter)
 
-    # TODO: Remove as this is unused
-    async def get_current_user_ids_in_room(
-        self, room_id: str, latest_event_ids: StrCollection
-    ) -> set[str]:
-        """
-        Get the users IDs who are currently in a room.
-
-        Note: This is much slower than using the equivalent method
-        `DataStore.get_users_in_room` or `DataStore.get_users_in_room_with_profiles`,
-        so this should only be used when wanting the users at a particular point
-        in the room.
-
-        Args:
-            room_id: The ID of the room.
-            latest_event_ids: Precomputed list of latest event IDs. Will be computed if None.
-        Returns:
-            Set of user IDs in the room.
-        """
-
-        assert latest_event_ids is not None
-
-        logger.debug("calling resolve_state_groups from get_current_user_ids_in_room")
-        entry = await self.resolve_state_groups_for_events(room_id, latest_event_ids)
-        state = await entry.get_state(self._state_storage_controller, StateFilter.all())
-        return await self.store.get_joined_user_ids_from_state(room_id, state)
-
     async def get_hosts_in_room_at_events(
         self, room_id: str, event_ids: StrCollection
     ) -> frozenset[str]:

synapse/storage/controllers/persist_events.py

@@ -943,6 +943,13 @@ class EventsPersistenceStorageController:
             SynapseError: if the new events include unknown prev_state_events
             AssertionError: if there are no state DAG forward extremities remaining in the room
         """
+        # Events are always processed in causal order without any gaps in the DAG
+        # (prev_state_events are always known), guaranteeing that processed events have a path to the
+        # create event. This is an emergent property of state DAGs as asserting that there is a path
+        # to the create event every time we insert an event would be prohibitively expensive.
+        # This is similar to how doubly-linked lists can potentially not refer to previous items correctly
+        # without verifying the list's integrity, but doing it on every insert is too expensive.
+
         # filter out events which don't belong in the state dag.
         new_state_events_contexts = [
             (e, ctx) for e, ctx in event_contexts if event_exists_in_state_dag(e)