Review comments

synapse/api/room_versions.py

@@ -114,6 +114,12 @@ class RoomVersion:
     # MSC3757: Restricting who can overwrite a state event
     msc3757_enabled: bool
     # MSC4242: Creates events with prev_state_events instead of auth_events and derives state from it.
+    # 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.
     msc4242_state_dags: bool
     # MSC4289: Creator power enabled
     msc4289_creator_power_enabled: bool

synapse/events/builder.py

@@ -194,10 +194,10 @@ class EventBuilder:
             auth_event_ids = []
 
         # Calculate auth_events for non-create events
-        # this block must not be hit for MSC4242 rooms as it resolves state with prev_events
         if auth_event_ids is None:
             # Every non-create event must have a room ID
             assert self.room_id is not None
+            # this block must not be hit for MSC4242 rooms as it resolves state with prev_events
             assert not self.room_version.msc4242_state_dags
             state_ids = await self._state.compute_state_after_events(
                 self.room_id,

synapse/handlers/room_member.py

@@ -2131,6 +2131,8 @@ class RoomMemberMasterHandler(RoomMemberHandler):
                 list(previous_membership_event.auth_event_ids()) + prev_event_ids
             )
 
+        # Either one is set or the other
+        assert prev_state_events or auth_event_ids
         # Try several times, it could fail with PartialStateConflictError
         # in handle_new_client_event, cf comment in except block.
         max_retries = 5

synapse/storage/controllers/persist_events.py

@@ -966,7 +966,8 @@ class EventsPersistenceStorageController:
             ev for ev, ctx in new_state_events_contexts if not ctx.rejected
         ]
         # We want to check that we are not missing any prev_state_events.
-        # To do this, we include rejected events in this check.
+        # To do this, we include rejected events in this check because other events may point to them.
+        # If we didn't include them, we might incorrectly say we are missing events when we are not.
         all_new_state_events = set(rejected_events + new_state_events)
 
         # First, verify that we know all prev_state_events. If we fail this check then we don't have

tests/storage/test_msc4242_state_dag.py

@@ -58,12 +58,12 @@ class MSC4242StateDagsTests(HomeserverTestCase):
         """
         # they don't change for messages
         first_event_id = self.helper.send(self.room_id, body="test1")["event_id"]
-        first_prev_state_event = self._get_prev_state_events(first_event_id)
-        assert len(first_prev_state_event) == 1
+        first_prev_state_events = self._get_prev_state_events(first_event_id)
+        assert len(first_prev_state_events) == 1
         second_id = self.helper.send(self.room_id, body="test2")["event_id"]
-        second_prev_state_event = self._get_prev_state_events(second_id)
-        assert len(second_prev_state_event) == 1
-        self.assertEquals(first_prev_state_event, second_prev_state_event)
+        second_prev_state_events = self._get_prev_state_events(second_id)
+        assert len(second_prev_state_events) == 1
+        self.assertEquals(first_prev_state_events, second_prev_state_events)
 
         # send an auth event, which should change the prev_state_events on *subsequent* events
         join_rule_state_event_id = self.helper.send_state(
@@ -77,12 +77,12 @@ class MSC4242StateDagsTests(HomeserverTestCase):
         join_rule_prev_state_event_ids = self._get_prev_state_events(
             join_rule_state_event_id
         )
-        self.assertEquals(second_prev_state_event, join_rule_prev_state_event_ids)
+        self.assertEquals(second_prev_state_events, join_rule_prev_state_event_ids)
 
         # prev_state_events should always point to the join rule now
         third_event_id = self.helper.send(self.room_id, body="test3")["event_id"]
-        third_prev_state_event = self._get_prev_state_events(third_event_id)
-        self.assertEquals(third_prev_state_event, [join_rule_state_event_id])
+        third_prev_state_events = self._get_prev_state_events(third_event_id)
+        self.assertEquals(third_prev_state_events, [join_rule_state_event_id])
         # and non-auth state should also update prev_state_events
         name_state_event_id = self.helper.send_state(
             self.room_id,
@@ -95,11 +95,11 @@ class MSC4242StateDagsTests(HomeserverTestCase):
         name_prev_state_event_ids = self._get_prev_state_events(name_state_event_id)
         self.assertEquals(name_prev_state_event_ids, [join_rule_state_event_id])
         fourth_event_id = self.helper.send(self.room_id, body="test4")["event_id"]
-        fourth_prev_state_event = self._get_prev_state_events(fourth_event_id)
-        self.assertEquals(fourth_prev_state_event, [name_state_event_id])
+        fourth_prev_state_events = self._get_prev_state_events(fourth_event_id)
+        self.assertEquals(fourth_prev_state_events, [name_state_event_id])
 
 
-class MSC4242EventPersistenceAuthDagsStoreTestCase(HomeserverTestCase):
+class MSC4242EventPersistenceStateDagsStoreTestCase(HomeserverTestCase):
     servlets = [
         room.register_servlets,
     ]
@@ -139,7 +139,6 @@ class MSC4242EventPersistenceAuthDagsStoreTestCase(HomeserverTestCase):
         id: str,
         prev_state_events: list[str],
         rejected: bool = False,
-        soft_failed: bool = False,
     ) -> tuple[FrozenEventVMSC4242, EventContext]:
         ev = make_event_from_dict(
             {
@@ -154,8 +153,6 @@ class MSC4242EventPersistenceAuthDagsStoreTestCase(HomeserverTestCase):
             },
             room_version=RoomVersions.MSC4242v12,
         )
-        if soft_failed:
-            ev.internal_metadata.soft_failed = True
         assert isinstance(ev, FrozenEventVMSC4242)
         ev._event_id = id
         ctx = Mock()