diff --git a/synapse/api/room_versions.py b/synapse/api/room_versions.py index 3e06819424..5a1bc99368 100644 --- a/synapse/api/room_versions.py +++ b/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 diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 8920eef3e6..78eb98e1e5 100644 --- a/synapse/events/builder.py +++ b/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, diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 35135eec12..efdb2c79a8 100644 --- a/synapse/handlers/room_member.py +++ b/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 diff --git a/synapse/storage/controllers/persist_events.py b/synapse/storage/controllers/persist_events.py index 86ba2b906f..2d50f49401 100644 --- a/synapse/storage/controllers/persist_events.py +++ b/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 diff --git a/tests/storage/test_msc4242_state_dag.py b/tests/storage/test_msc4242_state_dag.py index 9fe1dcabd4..d544f17469 100644 --- a/tests/storage/test_msc4242_state_dag.py +++ b/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()