Apply suggestions from code review

Co-authored-by: Eric Eastwood <erice@element.io>

synapse/event_auth.py

@@ -216,7 +216,7 @@ async def check_state_independent_auth_rules(
             # a synapse programming error
             known_prev_state_event_ids = set(prev_state_events)
             raise RuntimeError(
-                f"Event {event.event_id} has unknown prev state events "
+                f"Event {event.event_id} has unknown prev_state_events "
                 + f"{len(prev_state_events)} != {len(prev_state_events_ids)} "
                 + f"{prev_state_events_ids - known_prev_state_event_ids} missing "
                 + f"out of {prev_state_events_ids}"
@@ -226,8 +226,8 @@ async def check_state_independent_auth_rules(
             if prev_state_event.room_id != event.room_id:
                 raise AuthError(
                     403,
-                    "During auth for event %s in room %s, found event %s in prev state events "
-                    "which is in room %s"
+                    "During auth for event %s in room %s, found event %s in prev_state_events "
+                    "which belongs to a different room %s"
                     % (
                         event.event_id,
                         event.room_id,

synapse/storage/controllers/persist_events.py

@@ -964,8 +964,8 @@ class EventsPersistenceStorageController:
         #    existing, processed events which have the to-be-persisted events as prev_state_events.
         #  - We don't care if they are an "outlier" in the main room dag, so long as they AREN'T
         #    an outlier on the state dag, which this function checks, so we don't check outlier-ness.
-        #  - We allow soft-failed events to become forward extremities, as per the MSC. We do not
-        #    allow rejected events to become forward extremities though.
+        #  - We allow *soft-failed* events to become forward extremities, as per the MSC. We do not
+        #    allow *rejected* events to become forward extremities though.
 
         rejected_events = [ev for ev, ctx in new_state_events_contexts if ctx.rejected]
         new_state_events = [

synapse/storage/databases/main/events.py

@@ -3001,7 +3001,7 @@ class PersistEventsStore:
             table="msc4242_state_dag_edges",
             key_names=["room_id", "event_id", "prev_state_event_id"],
             key_values=[
-                (event.room_id, event.event_id, pae) for pae in event.prev_state_events
+                (event.room_id, event.event_id, prev_state_event) for prev_state_event in event.prev_state_events
             ],
             value_names=(),
             value_values=(),
@@ -3527,7 +3527,7 @@ class PersistEventsStore:
         """
         state_groups = {}
         for event, context in events_and_contexts:
-            # state dag rooms allow outliers to have state, as /gme'd state dag events are nominally
+            # state dag rooms allow outliers to have state, as `/get_missing_events` state dag events are nominally
             # outliers (not present in the timeline) but do need state persisted so we can calculate
             # what the auth_events are for the event.
             if (

synapse/storage/schema/main/delta/93/04_state_dag_fwd_extrems.sql

@@ -31,7 +31,7 @@ CREATE TABLE IF NOT EXISTS msc4242_state_dag_edges(
     -- DAG room to function with an holey DAG, so these events _cannot_ be purged. To purge them, the
     -- entire room would need to be deleted.
     event_id TEXT NOT NULL REFERENCES events(event_id),
-    -- one of the prev state events for this event ID. We must have it since we must have the entire state DAG.
+    -- one of the `prev_state_events` for this event ID. We must have it since we must have the entire state DAG.
     -- can be NULL for the create event.
     prev_state_event_id TEXT REFERENCES events(event_id)
     -- calculated depth for this event ID. There is some denormalisation here as we're storing the depth

tests/storage/test_msc4242_state_dag.py

@@ -29,7 +29,7 @@ from tests.unittest import HomeserverTestCase
 
 
 class MSC4242StateDagsTests(HomeserverTestCase):
-    user_id = "@red:server"
+    user_id = "@user1:server"
     servlets = [room.register_servlets]
 
     def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:

tests/test_event_auth.py

@@ -486,7 +486,7 @@ class EventAuthTestCase(unittest.TestCase):
                         "type": "m.room.name",
                         "sender": creator,
                         "state_key": "",
-                        "content": {"name": "prev state event is in another room"},
+                        "content": {"name": "prev_state_event is in another room"},
                         "prev_events": [join_event.event_id],
                         "prev_state_events": [event_in_another_room.event_id],
                     },