Port `Event.unsigned` field to Rust by erikjohnston · Pull Request #19708 · element-hq/synapse

erikjohnston · 2026-04-17T18:58:23Z

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.

We've always done this copy, I'm not sure why. We don't share the unsigned field between events so it's not that. We also edit the event that is in the cache, so its not even to stop it from getting added to the cached event.

Copilot

Pull request overview

Ports the Event.unsigned field to a Rust-backed Unsigned class, tightening which unsigned keys are retained/persisted and improving federation handling of unsigned.age by converting it into Synapse’s internal age_ts.

Changes:

Introduce Rust Unsigned type with separate “for event” vs “for persistence” views.
Update event persistence to store a persistence-safe event dict, and update federation parsing to compute unsigned.age_ts from unsigned.age using a received timestamp.
Update tests to reflect the new age_ts behavior and the restricted unsigned key set.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/federation/test_federation_server.py	Updates federation tests to assert `age_ts` behavior using `received_time`.
tests/events/test_utils.py	Updates unsigned-field tests to use recognized unsigned keys and `for_event()` assertions.
synapse/synapse_rust/events.pyi	Adds Python typing stub for the new Rust-backed `Unsigned` class.
synapse/storage/databases/main/events_worker.py	Removes redundant `dict(...)` conversion before adding `prev_*` unsigned fields.
synapse/storage/databases/main/events.py	Persists events via `get_dict_for_persistence()` to avoid persisting in-memory-only unsigned fields.
synapse/handlers/message.py	Comments out a `room_state` unsigned mutation during remote invite signing flow.
synapse/federation/federation_server.py	Passes `request_time` into PDU parsing to compute `age_ts`.
synapse/federation/federation_client.py	Passes a consistent `received_time` when parsing fetched missing events.
synapse/federation/federation_base.py	Adds `received_time` plumbing and converts `unsigned.age` → `unsigned.age_ts`.
synapse/events/utils.py	Updates `maybe_upsert_event_field` to accept the Rust `Unsigned` container.
synapse/events/init.py	Wraps event unsigned in Rust `Unsigned`; adds `get_dict_for_persistence()`; adjusts unsigned extraction.
rust/src/events/unsigned.rs	New Rust implementation of `Unsigned` with allowed-field enforcement and persistence/event views.
rust/src/events/mod.rs	Registers the new Rust `Unsigned` class with the Python module.
changelog.d/19708.misc	Adds changelog entry for the unsigned port.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

anoadragon453 · 2026-05-01T13:09:20Z

+impl<'a, 'py> FromPyObject<'a, 'py> for PersistedUnsignedFields {
+    type Error = PyErr;
+
+    fn extract(obj: pyo3::Borrowed<'a, 'py, PyAny>) -> Result<Self, Self::Error> {
+        Ok(depythonize(&obj)?)
+    }
+}


Is this any different from the blanket implementation you get from #[pyclass(from_py_object)]?

Yes, as the default is to convert the fields to attributes (rather than dict items). We actually don't rely on these traits now, so for clarity I've removed them and replaced usages with explicit depythonize.

anoadragon453 · 2026-05-01T13:14:56Z

                            invitee.domain, event
                        )
-                        event.unsigned.pop("room_state", None)
+                        # event.unsigned.pop("room_state", None)


Accidentally left in?

Ooops, how did I miss that. This is actually a no-op since that field doesn't actually exist (it's called 'invite_room_state'), so I've removed it.

9ed9094

This isn't actually what the field is called, so it's always a no-op. (The actual field name is 'invite_room_state').

We didn't actually rely on them, and it is a bit misleading given the Python equivalents are dicts (rather than proper objects).

Turns out the we have a `clone_event(..)` that roundtrips the unsigned field, and so we need to also parse the unpersisted fields. When we receive a PDU over federation we filter the fields, so we don't need to worry that we accidentally accept internal fields from remote sources here.

Teh `depythonize` accepts any mapping happily.

This isn't really related to this PR (which just moves the logic around), but got raised in review.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

synapse/federation/federation_base.py:365

Typo in the error message: "intger" should be "integer".

    if type(depth) is not int:  # noqa: E721
        raise SynapseError(400, "Depth %r not an intger" % (depth,), Codes.BAD_JSON)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

erikjohnston · 2026-05-05T10:30:12Z

+        # 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"]


Added a blunt try/except so that failing to deserialize doesn't block the entire transaction: a5bcd34.

We should probably do something better here, like make make_event_from_dict always return a SynapseError for invalid data, but that is quite subtle and not really what happens today.

Question: should we drop the invite_room_state if its the wrong format rather than dropping the entire event?

erikjohnston · 2026-05-05T10:02:42Z

            if get_prev_content:
                if "replaces_state" in event.unsigned:
                    prev = await self.get_event(
                        event.unsigned["replaces_state"],
                        get_prev_content=False,
                        allow_none=True,
                    )
                    if prev:
-                        event.unsigned = dict(event.unsigned)
                        event.unsigned["prev_content"] = prev.content
                        event.unsigned["prev_sender"] = prev.sender


This is fine, I've added some comments and an extra check in d2d4c9e

…ield

erikjohnston · 2026-05-05T10:25:50Z

The complement tests are failing due to not having pulled in develop (as I haven't merged the base branch yet)

Port Event.unsigned field to Rust

764b1eb

erikjohnston force-pushed the erikj/port_unsigned_field branch from 0935c3d to 764b1eb Compare April 17, 2026 19:05

erikjohnston added 3 commits April 17, 2026 20:06

Newsfile

6a2c1ee

Fix 'unsinged.age' over federation

3b3e960

Fix tests for maybe_upsert_event_field

4467294

erikjohnston force-pushed the erikj/port_unsigned_field branch from e0bb615 to 4467294 Compare April 18, 2026 15:11

erikjohnston marked this pull request as ready for review April 19, 2026 17:25

erikjohnston requested a review from a team as a code owner April 19, 2026 17:25

erikjohnston added 3 commits April 22, 2026 16:13

There is no need to copy the unsigned field

3ac1c80

We've always done this copy, I'm not sure why. We don't share the unsigned field between events so it's not that. We also edit the event that is in the cache, so its not even to stop it from getting added to the cached event.

Allow serialization

58ae5ac

Add tests

888421c

MadLittleMods added rust Z-Rust labels Apr 22, 2026

erikjohnston mentioned this pull request Apr 24, 2026

Port event content to Rust #19725

Draft

anoadragon453 requested a review from Copilot May 1, 2026 13:15

Copilot started reviewing on behalf of anoadragon453 May 1, 2026 13:15 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread synapse/federation/federation_base.py

Comment thread synapse/events/__init__.py

Comment thread synapse/events/__init__.py

Comment thread rust/src/events/unsigned.rs Outdated

Comment thread synapse/handlers/message.py Outdated

anoadragon453 requested changes May 1, 2026

View reviewed changes

erikjohnston added 4 commits May 5, 2026 09:57

Remove popping of 'room_state'

9ed9094

This isn't actually what the field is called, so it's always a no-op. (The actual field name is 'invite_room_state').

Remove the Into/FromPyObject traits

41b6b29

We didn't actually rely on them, and it is a bit misleading given the Python equivalents are dicts (rather than proper objects).

Accept Mapping in Unsigned constructor

e1c8e02

Teh `depythonize` accepts any mapping happily.

erikjohnston force-pushed the erikj/port_unsigned_field branch from 4ee1ea0 to 072b09c Compare May 5, 2026 09:16

Explicitly handle non-int age

a99443f

This isn't really related to this PR (which just moves the logic around), but got raised in review.

erikjohnston force-pushed the erikj/port_unsigned_field branch from 072b09c to a99443f Compare May 5, 2026 09:27

erikjohnston requested a review from Copilot May 5, 2026 09:33

Copilot started reviewing on behalf of erikjohnston May 5, 2026 09:34 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

erikjohnston added 2 commits May 5, 2026 11:01

Explicitly handle the case of prev fields already added

d2d4c9e

Merge branch 'erikj/port_signatures_field' into erikj/port_unsigned_f…

607902e

…ield

Add blunt handler for catching all bad PDUs

a5bcd34

erikjohnston requested a review from anoadragon453 May 5, 2026 10:30

Conversation

erikjohnston commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 May 1, 2026

Choose a reason for hiding this comment

Uh oh!

erikjohnston May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anoadragon453 May 1, 2026

Choose a reason for hiding this comment

Uh oh!

erikjohnston May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

erikjohnston May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erikjohnston May 5, 2026

Choose a reason for hiding this comment

Uh oh!

erikjohnston commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

erikjohnston commented Apr 17, 2026 •

edited

Loading