Port Event.unsigned field to Rust (#19708)

Similar to #19706, let's port the `unsigned` field into a Rust class.

This does change things a bit in that we now define exactly what
unsigned fields that are allowed to be added to an event, and what
actually gets persisted. This should be a noop though, as we carefully
filter out what unsigned fields we allow in from federation, for example

As a side effect of this cleanup, I think this fixes handling
`unsigned.age` on events received over federation.

changelog.d/19708.misc

@@ -0,0 +1 @@
+Port `Event.unsigned` field to Rust.

rust/src/events/mod.rs

@@ -28,12 +28,14 @@ use pyo3::{
 pub mod filter;
 mod internal_metadata;
 pub mod signatures;
+pub mod unsigned;
 
 /// Called when registering modules with python.
 pub fn register_module(py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> {
     let child_module = PyModule::new(py, "events")?;
     child_module.add_class::<internal_metadata::EventInternalMetadata>()?;
     child_module.add_class::<signatures::Signatures>()?;
+    child_module.add_class::<unsigned::Unsigned>()?;
     child_module.add_function(wrap_pyfunction!(filter::event_visible_to_server_py, m)?)?;
 
     m.add_submodule(&child_module)?;

rust/src/events/unsigned.rs

@@ -0,0 +1,429 @@
+/*
+ * This file is licensed under the Affero General Public License (AGPL) version 3.
+ *
+ * Copyright (C) 2026 Element Creations Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * See the GNU Affero General Public License for more details:
+ * <https://www.gnu.org/licenses/agpl-3.0.html>.
+ *
+ */
+
+use std::sync::{Arc, RwLock, RwLockReadGuard};
+
+use pyo3::{
+    exceptions::{PyKeyError, PyRuntimeError, PyTypeError},
+    pyclass, pymethods,
+    types::{PyAnyMethods, PyList, PyListMethods, PyMapping},
+    Bound, IntoPyObjectExt, PyAny, PyResult, Python,
+};
+use pythonize::{depythonize, pythonize};
+use serde::{Deserialize, Serialize};
+
+#[pyclass(frozen, skip_from_py_object)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(transparent)]
+pub struct Unsigned {
+    inner: Arc<RwLock<UnsignedInner>>,
+}
+
+/// The fields in the unsigned data of an event that are persisted in the
+/// database.
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+struct PersistedUnsignedFields {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    age_ts: Option<i64>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    replaces_state: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    invite_room_state: Option<Vec<serde_json::Value>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    knock_room_state: Option<Vec<serde_json::Value>>,
+}
+
+/// The inner representation of the unsigned data of an event, which includes
+/// both the fields that are persisted in the database and the fields that are
+/// only used in memory.
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+pub struct UnsignedInner {
+    #[serde(flatten)]
+    persisted_fields: PersistedUnsignedFields,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    prev_content: Option<Box<serde_json::Value>>, // We use Box to minimise stack space
+    #[serde(skip_serializing_if = "Option::is_none")]
+    prev_sender: Option<String>,
+}
+
+/// The fields that exist on the unsigned data of an event.
+///
+/// This is used when converting from python to rust, to ensure that if we add a
+/// new field we don't forget to add it to all the necessary places.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum UnsignedField {
+    AgeTs,
+    ReplacesState,
+    InviteRoomState,
+    KnockRoomState,
+    PrevContent,
+    PrevSender,
+}
+
+impl std::str::FromStr for UnsignedField {
+    type Err = ();
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "age_ts" => Ok(Self::AgeTs),
+            "replaces_state" => Ok(Self::ReplacesState),
+            "invite_room_state" => Ok(Self::InviteRoomState),
+            "knock_room_state" => Ok(Self::KnockRoomState),
+            "prev_content" => Ok(Self::PrevContent),
+            "prev_sender" => Ok(Self::PrevSender),
+            _ => Err(()),
+        }
+    }
+}
+
+impl Unsigned {
+    fn py_read(&self) -> PyResult<RwLockReadGuard<'_, UnsignedInner>> {
+        self.inner
+            .read()
+            .map_err(|_| PyRuntimeError::new_err("Unsigned lock poisoned"))
+    }
+
+    fn py_write(&self) -> PyResult<std::sync::RwLockWriteGuard<'_, UnsignedInner>> {
+        self.inner
+            .write()
+            .map_err(|_| PyRuntimeError::new_err("Unsigned lock poisoned"))
+    }
+}
+
+#[pymethods]
+impl Unsigned {
+    #[new]
+    fn py_new(unsigned: Bound<'_, PyMapping>) -> PyResult<Self> {
+        let inner = depythonize(&unsigned)?;
+
+        Ok(Self {
+            inner: Arc::new(RwLock::new(inner)),
+        })
+    }
+
+    fn __getitem__<'py>(
+        &self,
+        py: Python<'py>,
+        key: Bound<'_, PyAny>,
+    ) -> PyResult<Bound<'py, PyAny>> {
+        let key = key
+            .extract::<&str>()
+            .map_err(|_| PyTypeError::new_err("Unsigned keys must be strings"))?;
+
+        let field: UnsignedField = key
+            .parse()
+            .map_err(|_| PyKeyError::new_err(format!("Unsigned has no key '{key}'")))?;
+
+        let unsigned = self.py_read()?;
+
+        match field {
+            UnsignedField::AgeTs => Ok(unsigned
+                .persisted_fields
+                .age_ts
+                .ok_or_else(|| PyKeyError::new_err("age_ts"))?
+                .into_bound_py_any(py)?),
+            UnsignedField::ReplacesState => Ok((unsigned.persisted_fields.replaces_state)
+                .as_ref()
+                .ok_or_else(|| PyKeyError::new_err("replaces_state"))?
+                .into_bound_py_any(py)?),
+            UnsignedField::InviteRoomState => Ok(room_state_to_py(
+                py,
+                unsigned
+                    .persisted_fields
+                    .invite_room_state
+                    .as_ref()
+                    .ok_or_else(|| PyKeyError::new_err("invite_room_state"))?,
+            )?),
+            UnsignedField::KnockRoomState => Ok(room_state_to_py(
+                py,
+                unsigned
+                    .persisted_fields
+                    .knock_room_state
+                    .as_ref()
+                    .ok_or_else(|| PyKeyError::new_err("knock_room_state"))?,
+            )?),
+            UnsignedField::PrevContent => Ok(pythonize(
+                py,
+                unsigned
+                    .prev_content
+                    .as_ref()
+                    .ok_or_else(|| PyKeyError::new_err("prev_content"))?,
+            )?),
+            UnsignedField::PrevSender => Ok((unsigned.prev_sender)
+                .as_ref()
+                .ok_or_else(|| PyKeyError::new_err("prev_sender"))?
+                .into_bound_py_any(py)?),
+        }
+    }
+
+    fn __contains__(&self, key: Bound<'_, PyAny>) -> PyResult<bool> {
+        let Ok(key) = key.extract::<&str>() else {
+            return Ok(false);
+        };
+
+        let Ok(field) = key.parse::<UnsignedField>() else {
+            return Ok(false);
+        };
+
+        let unsigned = self.py_read()?;
+
+        let exists = match field {
+            UnsignedField::AgeTs => unsigned.persisted_fields.age_ts.is_some(),
+            UnsignedField::ReplacesState => unsigned.persisted_fields.replaces_state.is_some(),
+            UnsignedField::InviteRoomState => unsigned.persisted_fields.invite_room_state.is_some(),
+            UnsignedField::KnockRoomState => unsigned.persisted_fields.knock_room_state.is_some(),
+            UnsignedField::PrevContent => unsigned.prev_content.is_some(),
+            UnsignedField::PrevSender => unsigned.prev_sender.is_some(),
+        };
+
+        Ok(exists)
+    }
+
+    fn __setitem__(&self, key: Bound<'_, PyAny>, value: Bound<'_, PyAny>) -> PyResult<()> {
+        let key = key
+            .extract::<&str>()
+            .map_err(|_| PyTypeError::new_err("Unsigned keys must be strings"))?;
+
+        let field: UnsignedField = key
+            .parse()
+            .map_err(|_| PyKeyError::new_err(format!("Unsigned has no key '{key}'")))?;
+
+        let mut unsigned = self.py_write()?;
+
+        match field {
+            UnsignedField::AgeTs => unsigned.persisted_fields.age_ts = Some(value.extract()?),
+            UnsignedField::ReplacesState => {
+                unsigned.persisted_fields.replaces_state = Some(value.extract()?)
+            }
+            UnsignedField::InviteRoomState => {
+                unsigned.persisted_fields.invite_room_state = Some(room_state_from_py(value)?)
+            }
+            UnsignedField::KnockRoomState => {
+                unsigned.persisted_fields.knock_room_state = Some(room_state_from_py(value)?)
+            }
+            UnsignedField::PrevContent => {
+                unsigned.prev_content = Some(Box::new(depythonize(&value)?))
+            }
+            UnsignedField::PrevSender => unsigned.prev_sender = Some(value.extract()?),
+        }
+
+        Ok(())
+    }
+
+    fn __delitem__(&self, key: Bound<'_, PyAny>) -> PyResult<()> {
+        let key = key
+            .extract::<&str>()
+            .map_err(|_| PyTypeError::new_err("Unsigned keys must be strings"))?;
+
+        let field: UnsignedField = key
+            .parse()
+            .map_err(|_| PyKeyError::new_err(format!("Unsigned has no key '{key}'")))?;
+
+        let mut unsigned = self.py_write()?;
+
+        match field {
+            UnsignedField::AgeTs => unsigned.persisted_fields.age_ts = None,
+            UnsignedField::ReplacesState => unsigned.persisted_fields.replaces_state = None,
+            UnsignedField::InviteRoomState => unsigned.persisted_fields.invite_room_state = None,
+            UnsignedField::KnockRoomState => unsigned.persisted_fields.knock_room_state = None,
+            UnsignedField::PrevContent => unsigned.prev_content = None,
+            UnsignedField::PrevSender => unsigned.prev_sender = None,
+        }
+
+        Ok(())
+    }
+
+    #[pyo3(signature = (key, default=None))]
+    fn get<'py>(
+        &self,
+        py: Python<'py>,
+        key: Bound<'py, PyAny>,
+        default: Option<Bound<'py, PyAny>>,
+    ) -> PyResult<Option<Bound<'py, PyAny>>> {
+        match self.__getitem__(py, key) {
+            Ok(value) => Ok(Some(value)),
+            Err(err) => {
+                if err.is_instance_of::<PyKeyError>(py) {
+                    Ok(default)
+                } else {
+                    Err(err)
+                }
+            }
+        }
+    }
+
+    fn for_persistence<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> {
+        Ok(pythonize(py, &self.py_read()?.persisted_fields)?)
+    }
+
+    fn for_event<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> {
+        Ok(pythonize(py, &*self.py_read()?)?)
+    }
+}
+
+fn room_state_to_py<'py>(
+    py: Python<'py>,
+    state: &[serde_json::Value],
+) -> PyResult<Bound<'py, PyAny>> {
+    let py_list = PyList::empty(py);
+
+    for item in state {
+        py_list.append(pythonize(py, item)?)?;
+    }
+
+    py_list.into_bound_py_any(py)
+}
+
+fn room_state_from_py(value: Bound<'_, PyAny>) -> PyResult<Vec<serde_json::Value>> {
+    let py_list = value.cast::<PyList>()?;
+
+    let mut state = Vec::with_capacity(py_list.len());
+    for item in py_list.iter() {
+        state.push(pythonize::depythonize(&item)?);
+    }
+
+    Ok(state)
+}
+
+#[cfg(test)]
+mod tests {
+    use serde_json::json;
+
+    use super::*;
+
+    #[test]
+    fn test_unsigned_field_from_str_valid() {
+        assert_eq!("age_ts".parse(), Ok(UnsignedField::AgeTs));
+        assert_eq!("replaces_state".parse(), Ok(UnsignedField::ReplacesState));
+        assert_eq!(
+            "invite_room_state".parse(),
+            Ok(UnsignedField::InviteRoomState)
+        );
+        assert_eq!(
+            "knock_room_state".parse(),
+            Ok(UnsignedField::KnockRoomState)
+        );
+        assert_eq!("prev_content".parse(), Ok(UnsignedField::PrevContent));
+        assert_eq!("prev_sender".parse(), Ok(UnsignedField::PrevSender));
+    }
+
+    #[test]
+    fn test_unsigned_field_from_str_invalid() {
+        assert_eq!("".parse::<UnsignedField>(), Err(()));
+        assert_eq!("unknown".parse::<UnsignedField>(), Err(()));
+        // Case-sensitive: upper-case should not match.
+        assert_eq!("AGE_TS".parse::<UnsignedField>(), Err(()));
+        // Must be an exact match, no whitespace.
+        assert_eq!(" age_ts".parse::<UnsignedField>(), Err(()));
+    }
+
+    #[test]
+    fn test_persisted_fields_serialize_empty_is_empty_object() {
+        let fields = PersistedUnsignedFields::default();
+        let json = serde_json::to_value(&fields).unwrap();
+        assert_eq!(json, json!({}));
+    }
+
+    #[test]
+    fn test_persisted_fields_serialize_populated() {
+        let fields = PersistedUnsignedFields {
+            age_ts: Some(1234),
+            replaces_state: Some("$prev:example.com".to_string()),
+            invite_room_state: Some(vec![json!({"type": "m.room.name"})]),
+            knock_room_state: Some(vec![json!({"type": "m.room.topic"})]),
+        };
+        let json = serde_json::to_value(&fields).unwrap();
+        assert_eq!(
+            json,
+            json!({
+                "age_ts": 1234,
+                "replaces_state": "$prev:example.com",
+                "invite_room_state": [{"type": "m.room.name"}],
+                "knock_room_state": [{"type": "m.room.topic"}],
+            })
+        );
+    }
+
+    #[test]
+    fn test_unsigned_inner_flattens_persisted_fields() {
+        let inner = UnsignedInner {
+            persisted_fields: PersistedUnsignedFields {
+                age_ts: Some(99),
+                ..Default::default()
+            },
+            prev_content: Some(Box::new(json!({"body": "hi"}))),
+            prev_sender: Some("@alice:example.com".to_string()),
+        };
+
+        let json = serde_json::to_value(&inner).unwrap();
+        assert_eq!(
+            json,
+            json!({
+                "age_ts": 99,
+                "prev_content": {"body": "hi"},
+                "prev_sender": "@alice:example.com",
+            })
+        );
+    }
+
+    #[test]
+    fn test_unsigned_inner_roundtrip() {
+        let original = UnsignedInner {
+            persisted_fields: PersistedUnsignedFields {
+                age_ts: Some(10),
+                replaces_state: Some("$state:example.com".to_string()),
+                invite_room_state: None,
+                knock_room_state: None,
+            },
+            prev_content: Some(Box::new(json!({"membership": "join"}))),
+            prev_sender: None,
+        };
+
+        let json = serde_json::to_string(&original).unwrap();
+        let roundtripped: UnsignedInner = serde_json::from_str(&json).unwrap();
+
+        assert_eq!(roundtripped.persisted_fields.age_ts, Some(10));
+        assert_eq!(
+            roundtripped.persisted_fields.replaces_state.as_deref(),
+            Some("$state:example.com")
+        );
+        assert_eq!(
+            roundtripped.prev_content.as_deref(),
+            Some(&json!({"membership": "join"}))
+        );
+        assert_eq!(roundtripped.prev_sender, None);
+    }
+
+    #[test]
+    fn test_unsigned_serializes_transparently() {
+        // `Unsigned` is `#[serde(transparent)]` over its inner, so serializing
+        // an empty default should yield an empty object rather than a wrapper.
+        let unsigned = Unsigned::default();
+        let json = serde_json::to_value(&unsigned).unwrap();
+        assert_eq!(json, json!({}));
+    }
+
+    #[test]
+    fn test_unsigned_deserialize_from_flat_object() {
+        let json = json!({
+            "age_ts": 5,
+            "prev_sender": "@bob:example.com",
+        });
+        let unsigned: Unsigned = serde_json::from_value(json).unwrap();
+        let inner = unsigned.inner.read().unwrap();
+        assert_eq!(inner.persisted_fields.age_ts, Some(5));
+        assert_eq!(inner.prev_sender.as_deref(), Some("@bob:example.com"));
+    }
+}

synapse/events/__init__.py

@@ -44,7 +44,7 @@ from synapse.api.constants import (
     StickyEvent,
 )
 from synapse.api.room_versions import EventFormatVersions, RoomVersion, RoomVersions
-from synapse.synapse_rust.events import EventInternalMetadata, Signatures
+from synapse.synapse_rust.events import EventInternalMetadata, Signatures, Unsigned
 from synapse.types import (
     JsonDict,
     StateKey,
@@ -208,7 +208,7 @@ class EventBase(metaclass=abc.ABCMeta):
 
         self.room_version = room_version
         self.signatures = Signatures(signatures)
-        self.unsigned = unsigned
+        self.unsigned = Unsigned(unsigned)
         self.rejected_reason = rejected_reason
 
         self._dict = event_dict
@@ -258,9 +258,25 @@ class EventBase(metaclass=abc.ABCMeta):
         return self._dict.get("state_key")
 
     def get_dict(self) -> JsonDict:
+        """Convert the event to a dictionary suitable for serialisation."""
         d = dict(self._dict)
         d.update(
-            {"signatures": self.signatures.as_dict(), "unsigned": dict(self.unsigned)}
+            {
+                "signatures": self.signatures.as_dict(),
+                "unsigned": self.unsigned.for_event(),
+            }
+        )
+
+        return d
+
+    def get_dict_for_persistence(self) -> JsonDict:
+        """Convert the event to a dictionary suitable for persistence."""
+        d = dict(self._dict)
+        d.update(
+            {
+                "signatures": self.signatures.as_dict(),
+                "unsigned": self.unsigned.for_persistence(),
+            }
         )
 
         return d
@@ -401,7 +417,7 @@ class FrozenEvent(EventBase):
             for name, sigs in event_dict.pop("signatures", {}).items()
         }
 
-        unsigned = dict(event_dict.pop("unsigned", {}))
+        unsigned = event_dict.pop("unsigned", {})
 
         # We intern these strings because they turn up a lot (especially when
         # caching).
@@ -455,7 +471,7 @@ class FrozenEventV2(EventBase):
 
         assert "event_id" not in event_dict
 
-        unsigned = dict(event_dict.pop("unsigned", {}))
+        unsigned = event_dict.pop("unsigned", {})
 
         # We intern these strings because they turn up a lot (especially when
         # caching).

synapse/events/utils.py

@@ -47,6 +47,7 @@ from synapse.api.constants import (
 from synapse.api.errors import Codes, SynapseError
 from synapse.api.room_versions import RoomVersion
 from synapse.logging.opentracing import SynapseTags, set_tag, trace
+from synapse.synapse_rust.events import Unsigned
 from synapse.types import JsonDict, Requester
 
 from . import EventBase, FrozenEventV2, StrippedStateEvent, make_event_from_dict
@@ -987,7 +988,7 @@ def validate_canonicaljson(value: Any) -> None:
 
 
 def maybe_upsert_event_field(
-    event: EventBase, container: JsonDict, key: str, value: object
+    event: EventBase, container: Unsigned, key: str, value: object
 ) -> bool:
     """Upsert an event field, but only if this doesn't make the event too large.
 

synapse/federation/federation_base.py

@@ -307,20 +307,27 @@ def _is_invite_via_3pid(event: EventBase) -> bool:
 
 
 def parse_events_from_pdu_json(
-    pdus_json: Sequence[JsonDict], room_version: RoomVersion
+    pdus_json: Sequence[JsonDict],
+    room_version: RoomVersion,
+    received_time: int | None = None,
 ) -> list[EventBase]:
     return [
-        event_from_pdu_json(pdu_json, room_version)
+        event_from_pdu_json(pdu_json, room_version, received_time=received_time)
         for pdu_json in filter_pdus_for_valid_depth(pdus_json)
     ]
 
 
-def event_from_pdu_json(pdu_json: JsonDict, room_version: RoomVersion) -> EventBase:
+def event_from_pdu_json(
+    pdu_json: JsonDict, room_version: RoomVersion, received_time: int | None = None
+) -> EventBase:
     """Construct an EventBase from an event json received over federation
 
     Args:
         pdu_json: pdu as received over federation
         room_version: The version of the room this event belongs to
+        received_time: timestamp in ms that the event was received at. If
+            `None` then any `age` field in the `unsigned` block will be
+            dropped.
 
     Raises:
         SynapseError: if the pdu is missing required fields or is otherwise
@@ -333,6 +340,25 @@ def event_from_pdu_json(pdu_json: JsonDict, room_version: RoomVersion) -> EventB
     if "unsigned" in pdu_json:
         _strip_unsigned_values(pdu_json)
 
+        # Handle the `age` field, which is sent by some servers as part of the
+        # `unsigned` block. We convert this into an `age_ts` field, which is
+        # what Synapse uses internally. We also remove the `age` field to avoid
+        # confusion.
+        #
+        # c.f. https://github.com/matrix-org/synapse/issues/8429
+        unsigned = pdu_json["unsigned"]
+        age = unsigned.pop("age", None)
+
+        # We check that the `age` is actually an int before using it below. We
+        # don't error here as the `age` a) doesn't affect the validity of the
+        # event, and b) is best effort anyway.
+        if not isinstance(age, int):
+            age = None
+
+        unsigned.pop("age_ts", None)
+        if received_time is not None and age is not None:
+            unsigned["age_ts"] = received_time - int(age)
+
     depth = pdu_json["depth"]
     if type(depth) is not int:  # noqa: E721
         raise SynapseError(400, "Depth %r not an intger" % (depth,), Codes.BAD_JSON)

synapse/federation/federation_client.py

@@ -1574,10 +1574,15 @@ class FederationClient(FederationBase):
                 min_depth=min_depth,
                 timeout=timeout,
             )
+            received_time = self._clock.time_msec()
 
             room_version = await self.store.get_room_version(room_id)
 
-            events = parse_events_from_pdu_json(content.get("events", []), room_version)
+            events = parse_events_from_pdu_json(
+                content.get("events", []),
+                room_version,
+                received_time=received_time,
+            )
 
             signed_events = await self._check_sigs_and_hash_for_pulled_events_and_fetch(
                 destination, events, room_version=room_version

synapse/federation/federation_server.py

@@ -451,16 +451,6 @@ class FederationServer(FederationBase):
         newest_pdu_ts = 0
 
         for p in transaction.pdus:
-            # FIXME (richardv): I don't think this works:
-            #  https://github.com/matrix-org/synapse/issues/8429
-            if "unsigned" in p:
-                unsigned = p["unsigned"]
-                if "age" in unsigned:
-                    p["age"] = unsigned["age"]
-            if "age" in p:
-                p["age_ts"] = request_time - int(p["age"])
-                del p["age"]
-
             # We try and pull out an event ID so that if later checks fail we
             # can log something sensible. We don't mandate an event ID here in
             # case future event formats get rid of the key.
@@ -488,10 +478,15 @@ class FederationServer(FederationBase):
                 continue
 
             try:
-                event = event_from_pdu_json(p, room_version)
+                event = event_from_pdu_json(p, room_version, received_time=request_time)
             except SynapseError as e:
                 logger.info("Ignoring PDU for failing to deserialize: %s", e)
                 continue
+            except Exception as e:
+                # We catch all exceptions here as we don't want a single bad
+                # event to cause us to fail the whole transaction.
+                logger.exception("Error deserializing PDU: %s", e)
+                continue
 
             pdus_by_room.setdefault(room_id, []).append(event)
 

synapse/handlers/message.py

@@ -2089,7 +2089,6 @@ class EventCreationHandler:
                         returned_invite = await federation_handler.send_invite(
                             invitee.domain, event
                         )
-                        event.unsigned.pop("room_state", None)
 
                         # TODO: Make sure the signatures actually are correct.
                         event.signatures.update(returned_invite.signatures.as_dict())

synapse/storage/databases/main/events.py

@@ -2711,7 +2711,7 @@ class PersistEventsStore:
             return
 
         def event_dict(event: EventBase) -> JsonDict:
-            d = event.get_dict()
+            d = event.get_dict_for_persistence()
             d.pop("redacted", None)
             d.pop("redacted_because", None)
             return d

synapse/storage/databases/main/events_worker.py

@@ -779,14 +779,26 @@ class EventsWorkerStore(SQLBaseStore):
             events.append(event)
 
             if get_prev_content:
-                if "replaces_state" in event.unsigned:
+                # The `event` here might be in the cache, and so might have
+                # already had the `prev_content` and `prev_sender` fields added
+                # to its unsigned.
+                #
+                # We check if a) we should add the previous content, and b) if
+                # we have already added it.
+                replaces_state = "replaces_state" in event.unsigned
+                has_prev = (
+                    "prev_content" in event.unsigned and "prev_sender" in event.unsigned
+                )
+                if replaces_state and not has_prev:
                     prev = await self.get_event(
                         event.unsigned["replaces_state"],
                         get_prev_content=False,
                         allow_none=True,
                     )
                     if prev:
-                        event.unsigned = dict(event.unsigned)
+                        # This mutates the cached event, but that's fine as the
+                        # previous content/sender will be the same for all
+                        # requests for this event.
                         event.unsigned["prev_content"] = prev.content
                         event.unsigned["prev_sender"] = prev.sender
 

synapse/synapse_rust/events.pyi

@@ -12,7 +12,7 @@
 
 from typing import Any, Mapping
 
-from synapse.types import JsonDict
+from synapse.types import JsonDict, JsonMapping
 
 class EventInternalMetadata:
     def __init__(self, internal_metadata_dict: JsonDict): ...
@@ -183,3 +183,33 @@ class Signatures:
 
     def as_dict(self) -> dict[str, dict[str, str]]: ...
     """Return a copy of the signatures as a dictionary."""
+
+class Unsigned:
+    """A class representing the unsigned data of an event."""
+
+    def __init__(self, unsigned_dict: JsonMapping): ...
+    def __getitem__(self, key: str) -> Any: ...
+    """Get the value for the given key.
+
+    Raises KeyError if the key is unset or not recognised."""
+
+    def __setitem__(self, key: str, value: Any) -> None: ...
+    """Set the value for the given key.
+
+    Raises KeyError if the key is not recognised."""
+
+    def __delitem__(self, key: str) -> None: ...
+    """Delete the value for the given key.
+
+    Raises KeyError if the key is unset or not recognised."""
+
+    def __contains__(self, key: Any) -> bool: ...
+    def get(self, key: str, default: Any = None) -> Any: ...
+    """Get the value for the given key, or ``default`` if the key is unset."""
+
+    def for_persistence(self) -> JsonDict: ...
+    """Return a dict of the fields that should be persisted to the database."""
+
+    def for_event(self) -> JsonDict: ...
+    """Return a dict of all unsigned fields, including those only kept in
+    memory, suitable for inclusion in an event."""

tests/events/test_utils.py

@@ -67,25 +67,31 @@ def MockEvent(**kwargs: Any) -> EventBase:
 class TestMaybeUpsertEventField(stdlib_unittest.TestCase):
     def test_update_okay(self) -> None:
         event = make_event_from_dict({"event_id": "$1234"})
-        success = maybe_upsert_event_field(event, event.unsigned, "key", "value")
+        success = maybe_upsert_event_field(
+            event, event.unsigned, "replaces_state", "value"
+        )
         self.assertTrue(success)
-        self.assertEqual(event.unsigned["key"], "value")
+        self.assertEqual(event.unsigned["replaces_state"], "value")
 
     def test_update_not_okay(self) -> None:
         event = make_event_from_dict({"event_id": "$1234"})
         LARGE_STRING = "a" * 100_000
-        success = maybe_upsert_event_field(event, event.unsigned, "key", LARGE_STRING)
+        success = maybe_upsert_event_field(
+            event, event.unsigned, "replaces_state", LARGE_STRING
+        )
         self.assertFalse(success)
-        self.assertNotIn("key", event.unsigned)
+        self.assertNotIn("replaces_state", event.unsigned)
 
     def test_update_not_okay_leaves_original_value(self) -> None:
         event = make_event_from_dict(
-            {"event_id": "$1234", "unsigned": {"key": "value"}}
+            {"event_id": "$1234", "unsigned": {"replaces_state": "value"}}
         )
         LARGE_STRING = "a" * 100_000
-        success = maybe_upsert_event_field(event, event.unsigned, "key", LARGE_STRING)
+        success = maybe_upsert_event_field(
+            event, event.unsigned, "replaces_state", LARGE_STRING
+        )
         self.assertFalse(success)
-        self.assertEqual(event.unsigned["key"], "value")
+        self.assertEqual(event.unsigned["replaces_state"], "value")
 
 
 class PruneEventTestCase(stdlib_unittest.TestCase):
@@ -623,7 +629,7 @@ class CloneEventTestCase(stdlib_unittest.TestCase):
             {
                 "type": "A",
                 "event_id": "$test:domain",
-                "unsigned": {"a": 1, "b": 2},
+                "unsigned": {"age_ts": 1, "replaces_state": "2"},
             },
             RoomVersions.V1,
             {"txn_id": "txn"},
@@ -634,10 +640,14 @@ class CloneEventTestCase(stdlib_unittest.TestCase):
         self.assertEqual(original.internal_metadata.instance_name, "worker1")
 
         cloned = clone_event(original)
-        cloned.unsigned["b"] = 3
+        cloned.unsigned["age_ts"] = 3
 
-        self.assertEqual(original.unsigned, {"a": 1, "b": 2})
-        self.assertEqual(cloned.unsigned, {"a": 1, "b": 3})
+        self.assertEqual(
+            original.unsigned.for_event(), {"age_ts": 1, "replaces_state": "2"}
+        )
+        self.assertEqual(
+            cloned.unsigned.for_event(), {"age_ts": 3, "replaces_state": "2"}
+        )
         self.assertEqual(cloned.internal_metadata.stream_ordering, 1234)
         self.assertEqual(cloned.internal_metadata.instance_name, "worker1")
         self.assertEqual(cloned.internal_metadata.txn_id, "txn")

tests/federation/test_federation_server.py

@@ -754,9 +754,9 @@ class StripUnsignedFromEventsTestCase(unittest.TestCase):
             },
         }
 
-        filtered_event2 = event_from_pdu_json(event2, RoomVersions.V1)
-        self.assertIn("age", filtered_event2.unsigned)
-        self.assertEqual(14, filtered_event2.unsigned["age"])
+        filtered_event2 = event_from_pdu_json(event2, RoomVersions.V1, received_time=20)
+        self.assertIn("age_ts", filtered_event2.unsigned)
+        self.assertEqual(6, filtered_event2.unsigned["age_ts"])
         self.assertNotIn("more warez", filtered_event2.unsigned)
         # Invite_room_state is allowed in events of type m.room.member
         self.assertIn("invite_room_state", filtered_event2.unsigned)
@@ -779,8 +779,8 @@ class StripUnsignedFromEventsTestCase(unittest.TestCase):
                 "invite_room_state": [],
             },
         }
-        filtered_event3 = event_from_pdu_json(event3, RoomVersions.V1)
-        self.assertIn("age", filtered_event3.unsigned)
+        filtered_event3 = event_from_pdu_json(event3, RoomVersions.V1, received_time=20)
+        self.assertIn("age_ts", filtered_event3.unsigned)
         # Invite_room_state field is only permitted in event type m.room.member
         self.assertNotIn("invite_room_state", filtered_event3.unsigned)
         self.assertNotIn("more warez", filtered_event3.unsigned)