Stabilize support for MSC3970: updated transaction semantics (scope to device_id)

changelog.d/15629.feature

@@ -0,0 +1 @@
+Scope transaction IDs to devices (implement [MSC3970](https://github.com/matrix-org/matrix-spec-proposals/pull/3970)).

synapse/config/experimental.py

@@ -194,9 +194,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
             "msc3981_recurse_relations", False
         )
 
-        # MSC3970: Scope transaction IDs to devices
-        self.msc3970_enabled = experimental.get("msc3970_enabled", False)
-
         # MSC4009: E.164 Matrix IDs
         self.msc4009_e164_mxids = experimental.get("msc4009_e164_mxids", False)
 

synapse/events/utils.py

@@ -361,16 +361,13 @@ def serialize_event(
     time_now_ms: int,
     *,
     config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG,
-    msc3970_enabled: bool = False,
 ) -> JsonDict:
     """Serialize event for clients
 
     Args:
         e
         time_now_ms
         config: Event serialization config
-        msc3970_enabled: Whether MSC3970 is enabled. It changes whether we should
-            include the `transaction_id` in the event's `unsigned` section.
 
     Returns:
         The serialized event dictionary.
@@ -396,7 +393,6 @@ def serialize_event(
             e.unsigned["redacted_because"],
             time_now_ms,
             config=config,
-            msc3970_enabled=msc3970_enabled,
         )
 
     # If we have a txn_id saved in the internal_metadata, we should include it in the
@@ -408,7 +404,7 @@ def serialize_event(
         # event internal metadata. Since we were not recording them before, if it hasn't
         # been recorded, we fallback to the old behaviour.
         event_device_id: Optional[str] = getattr(e.internal_metadata, "device_id", None)
-        if msc3970_enabled and event_device_id is not None:
+        if event_device_id is not None:
             if event_device_id == config.requester.device_id:
                 d["unsigned"]["transaction_id"] = txn_id
 
@@ -460,9 +456,6 @@ class EventClientSerializer:
     clients.
     """
 
-    def __init__(self, *, msc3970_enabled: bool = False):
-        self._msc3970_enabled = msc3970_enabled
-
     def serialize_event(
         self,
         event: Union[JsonDict, EventBase],
@@ -487,9 +480,7 @@ def serialize_event(
         if not isinstance(event, EventBase):
             return event
 
-        serialized_event = serialize_event(
-            event, time_now, config=config, msc3970_enabled=self._msc3970_enabled
-        )
+        serialized_event = serialize_event(event, time_now, config=config)
 
         # Check if there are any bundled aggregations to include with the event.
         if bundle_aggregations:

synapse/handlers/message.py

@@ -560,8 +560,6 @@ def __init__(self, hs: "HomeServer"):
                 expiry_ms=30 * 60 * 1000,
             )
 
-        self._msc3970_enabled = hs.config.experimental.msc3970_enabled
-
     async def create_event(
         self,
         requester: Requester,
@@ -906,7 +904,7 @@ async def get_event_from_transaction(
             An event if one could be found, None otherwise.
         """
 
-        if self._msc3970_enabled and requester.device_id:
+        if requester.device_id:
             # When MSC3970 is enabled, we lookup for events sent by the same device first,
             # and fallback to the old behaviour if none were found.
             existing_event_id = (

synapse/handlers/room_member.py

@@ -174,8 +174,6 @@ def __init__(self, hs: "HomeServer"):
         self.request_ratelimiter = hs.get_request_ratelimiter()
         hs.get_notifier().add_new_join_in_room_callback(self._on_user_joined_room)
 
-        self._msc3970_enabled = hs.config.experimental.msc3970_enabled
-
     def _on_user_joined_room(self, event_id: str, room_id: str) -> None:
         """Notify the rate limiter that a room join has occurred.
 
@@ -424,7 +422,7 @@ async def _local_membership_update(
         # do it up front for efficiency.)
         if txn_id:
             existing_event_id = None
-            if self._msc3970_enabled and requester.device_id:
+            if requester.device_id:
                 # When MSC3970 is enabled, we lookup for events sent by the same device
                 # first, and fallback to the old behaviour if none were found.
                 existing_event_id = (

synapse/rest/client/transactions.py

@@ -50,8 +50,6 @@ def __init__(self, hs: "HomeServer"):
         # for at *LEAST* 30 mins, and at *MOST* 60 mins.
         self.cleaner = self.clock.looping_call(self._cleanup, CLEANUP_PERIOD_MS)
 
-        self._msc3970_enabled = hs.config.experimental.msc3970_enabled
-
     def _get_transaction_key(self, request: IRequest, requester: Requester) -> Hashable:
         """A helper function which returns a transaction key that can be used
         with TransactionCache for idempotent requests.
@@ -79,18 +77,11 @@ def _get_transaction_key(self, request: IRequest, requester: Requester) -> Hasha
             return (path, "appservice", requester.app_service.id)
 
         # With MSC3970, we use the user ID and device ID as the transaction key
-        elif self._msc3970_enabled:
+        else:
             assert requester.user, "Requester must have a user"
             assert requester.device_id, "Requester must have a device_id"
             return (path, "user", requester.user, requester.device_id)
 
-        # Otherwise, the pre-MSC3970 behaviour is to use the access token ID
-        else:
-            assert (
-                requester.access_token_id is not None
-            ), "Requester must have an access_token_id"
-            return (path, "user", requester.access_token_id)
-
     def fetch_or_execute_request(
         self,
         request: IRequest,

synapse/server.py

@@ -777,9 +777,7 @@ def get_oidc_handler(self) -> "OidcHandler":
 
     @cache_in_self
     def get_event_client_serializer(self) -> EventClientSerializer:
-        return EventClientSerializer(
-            msc3970_enabled=self.config.experimental.msc3970_enabled
-        )
+        return EventClientSerializer()
 
     @cache_in_self
     def get_password_policy_handler(self) -> PasswordPolicyHandler:

synapse/storage/databases/main/events.py

@@ -127,8 +127,6 @@ def __init__(
         self._backfill_id_gen: AbstractStreamIdGenerator = self.store._backfill_id_gen
         self._stream_id_gen: AbstractStreamIdGenerator = self.store._stream_id_gen
 
-        self._msc3970_enabled = hs.config.experimental.msc3970_enabled
-
     @trace
     async def _persist_events_and_state_updates(
         self,
@@ -1033,7 +1031,7 @@ def _persist_transaction_ids_txn(
         # With MSC3970, we rely on the device_id instead to scope the txn_id for events.
         # We're only inserting if MSC3970 is *enabled*, because else the pre-MSC3970
         # behaviour would allow for a UNIQUE constraint violation on this table
-        if to_insert_device_id and self._msc3970_enabled:
+        if to_insert_device_id:
             self.db_pool.simple_insert_many_txn(
                 txn,
                 table="event_txn_id_device_id",