From 7b6be4b723e97124b8e5592e0f5159c3609877ac Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 11:42:34 +0000
Subject: [PATCH 01/12] Have `make_membership_event` return room version

---
 synapse/federation/federation_client.py | 41 +++++++++++++++----------
 synapse/handlers/federation.py          | 16 +++++++---
 2 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py
index af652a76596c..a5a3c177e37f 100644
--- a/synapse/federation/federation_client.py
+++ b/synapse/federation/federation_client.py
@@ -17,6 +17,7 @@
 import copy
 import itertools
 import logging
+from typing import Dict, Iterable
 
 from prometheus_client import Counter
 
@@ -404,7 +405,13 @@ def _try_destination_list(self, description, destinations, callback):
         raise SynapseError(502, "Failed to %s via any server" % (description,))
 
     def make_membership_event(
-        self, destinations, room_id, user_id, membership, content, params
+        self,
+        destinations: Iterable[str],
+        room_id: str,
+        user_id: str,
+        membership: str,
+        content: dict,
+        params: Dict[str, str],
     ):
         """
         Creates an m.room.member event, with context, without participating in the room.
@@ -417,21 +424,20 @@ def make_membership_event(
         Note that this does not append any events to any graphs.
 
         Args:
-            destinations (Iterable[str]): Candidate homeservers which are probably
+            destinations: Candidate homeservers which are probably
                 participating in the room.
-            room_id (str): The room in which the event will happen.
-            user_id (str): The user whose membership is being evented.
-            membership (str): The "membership" property of the event. Must be
-                one of "join" or "leave".
-            content (dict): Any additional data to put into the content field
-                of the event.
-            params (dict[str, str|Iterable[str]]): Query parameters to include in the
-                request.
+            room_id: The room in which the event will happen.
+            user_id: The user whose membership is being evented.
+            membership: The "membership" property of the event. Must be one of
+                "join" or "leave".
+            content: Any additional data to put into the content field of the
+                event.
+            params: Query parameters to include in the request.
         Return:
-            Deferred[tuple[str, FrozenEvent, int]]: resolves to a tuple of
-            `(origin, event, event_format)` where origin is the remote
-            homeserver which generated the event, and event_format is one of
-            `synapse.api.room_versions.EventFormatVersions`.
+            Deferred[Tuple[str, FrozenEvent, RoomVersion]]: resolves to a tuple of
+            `(origin, event, room_version)` where origin is the remote
+            homeserver which generated the event, and room_version is the
+            version of the room.
 
             Fails with a ``SynapseError`` if the chosen remote server
             returns a 300/400 code.
@@ -453,8 +459,9 @@ def send_request(destination):
 
             # Note: If not supplied, the room version may be either v1 or v2,
             # however either way the event format version will be v1.
-            room_version = ret.get("room_version", RoomVersions.V1.identifier)
-            event_format = room_version_to_event_format(room_version)
+            room_version_id = ret.get("room_version", RoomVersions.V1.identifier)
+            room_version = KNOWN_ROOM_VERSIONS.get(room_version_id)
+            event_format = room_version_to_event_format(room_version_id)
 
             pdu_dict = ret.get("event", None)
             if not isinstance(pdu_dict, dict):
@@ -478,7 +485,7 @@ def send_request(destination):
                 event_dict=pdu_dict,
             )
 
-            return (destination, ev, event_format)
+            return (destination, ev, room_version)
 
         return self._try_destination_list(
             "make_" + membership, destinations, send_request
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index d4f9a792fce4..7e2db97154ad 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -47,7 +47,7 @@
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
 from synapse.crypto.event_signing import compute_event_signature
 from synapse.event_auth import auth_types_for_event
-from synapse.events import EventBase
+from synapse.events import EventBase, room_version_to_event_format
 from synapse.events.snapshot import EventContext
 from synapse.events.validator import EventValidator
 from synapse.logging.context import (
@@ -1186,7 +1186,7 @@ def do_invite_join(self, target_hosts, room_id, joinee, content):
         """
         logger.debug("Joining %s to %s", joinee, room_id)
 
-        origin, event, event_format_version = yield self._make_and_verify_event(
+        origin, event, room_version = yield self._make_and_verify_event(
             target_hosts,
             room_id,
             joinee,
@@ -1214,6 +1214,8 @@ def do_invite_join(self, target_hosts, room_id, joinee, content):
                 target_hosts.insert(0, origin)
             except ValueError:
                 pass
+
+            event_format_version = room_version_to_event_format(room_version.identifier)
             ret = yield self.federation_client.send_join(
                 target_hosts, event, event_format_version
             )
@@ -1486,7 +1488,7 @@ def on_invite_request(self, origin, pdu):
 
     @defer.inlineCallbacks
     def do_remotely_reject_invite(self, target_hosts, room_id, user_id, content):
-        origin, event, event_format_version = yield self._make_and_verify_event(
+        origin, event, room_version = yield self._make_and_verify_event(
             target_hosts, room_id, user_id, "leave", content=content
         )
         # Mark as outlier as we don't have any state for this event; we're not
@@ -1513,7 +1515,11 @@ def do_remotely_reject_invite(self, target_hosts, room_id, user_id, content):
     def _make_and_verify_event(
         self, target_hosts, room_id, user_id, membership, content={}, params=None
     ):
-        origin, event, format_ver = yield self.federation_client.make_membership_event(
+        (
+            origin,
+            event,
+            room_version,
+        ) = yield self.federation_client.make_membership_event(
             target_hosts, room_id, user_id, membership, content, params=params
         )
 
@@ -1525,7 +1531,7 @@ def _make_and_verify_event(
         assert event.user_id == user_id
         assert event.state_key == user_id
         assert event.room_id == room_id
-        return origin, event, format_ver
+        return origin, event, room_version
 
     @defer.inlineCallbacks
     @log_function

From f01d0d5d4ccae06f722c94d537c6cb3179df8129 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 15:01:03 +0000
Subject: [PATCH 02/12] Check room_version from make_join matches returned
 state.

---
 synapse/handlers/federation.py | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 7e2db97154ad..12440bed47ad 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -44,7 +44,7 @@
     StoreError,
     SynapseError,
 )
-from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
+from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion, RoomVersions
 from synapse.crypto.event_signing import compute_event_signature
 from synapse.event_auth import auth_types_for_event
 from synapse.events import EventBase, room_version_to_event_format
@@ -1242,7 +1242,9 @@ def do_invite_join(self, target_hosts, room_id, joinee, content):
                 # FIXME
                 pass
 
-            yield self._persist_auth_tree(origin, auth_chain, state, event)
+            yield self._persist_auth_tree(
+                origin, auth_chain, state, event, room_version
+            )
 
             # Check whether this room is the result of an upgrade of a room we already know
             # about. If so, migrate over user information
@@ -1816,7 +1818,14 @@ def prep(ev_info: _NewEventInfo):
         )
 
     @defer.inlineCallbacks
-    def _persist_auth_tree(self, origin, auth_events, state, event):
+    def _persist_auth_tree(
+        self,
+        origin: str,
+        auth_events: List[EventBase],
+        state: List[EventBase],
+        event: EventBase,
+        room_version: RoomVersion,
+    ):
         """Checks the auth chain is valid (and passes auth checks) for the
         state and event. Then persists the auth chain and state atomically.
         Persists the event separately. Notifies about the persisted events
@@ -1825,10 +1834,10 @@ def _persist_auth_tree(self, origin, auth_events, state, event):
         Will attempt to fetch missing auth events.
 
         Args:
-            origin (str): Where the events came from
-            auth_events (list)
-            state (list)
-            event (Event)
+            origin: Where the events came from
+            auth_events
+            state
+            event
 
         Returns:
             Deferred
@@ -1854,10 +1863,13 @@ def _persist_auth_tree(self, origin, auth_events, state, event):
             # invalid, and it would fail auth checks anyway.
             raise SynapseError(400, "No create event in state")
 
-        room_version = create_event.content.get(
+        room_version_id = create_event.content.get(
             "room_version", RoomVersions.V1.identifier
         )
 
+        if room_version.identifier != room_version_id:
+            raise SynapseError(400, "Room version mismatch")
+
         missing_auth_events = set()
         for e in itertools.chain(auth_events, state, [event]):
             for e_id in e.auth_event_ids():

From a42b6759b51904a564eea03c0bfd13c293dc2d59 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 15:07:32 +0000
Subject: [PATCH 03/12] Use RoomVersion consistently throughout room upgrade

---
 synapse/handlers/room.py                      | 41 +++++++++++--------
 .../v2_alpha/room_upgrade_rest_servlet.py     |  3 +-
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 9f50196ea76a..f304210ac344 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -29,7 +29,7 @@
 
 from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset
 from synapse.api.errors import AuthError, Codes, NotFoundError, StoreError, SynapseError
-from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
+from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion
 from synapse.http.endpoint import parse_and_validate_server_name
 from synapse.storage.state import StateFilter
 from synapse.types import (
@@ -100,13 +100,15 @@ def __init__(self, hs):
         self.third_party_event_rules = hs.get_third_party_event_rules()
 
     @defer.inlineCallbacks
-    def upgrade_room(self, requester, old_room_id, new_version):
+    def upgrade_room(
+        self, requester: Requester, old_room_id: str, new_version: RoomVersion
+    ):
         """Replace a room with a new room with a different version
 
         Args:
-            requester (synapse.types.Requester): the user requesting the upgrade
-            old_room_id (unicode): the id of the room to be replaced
-            new_version (unicode): the new room version to use
+            requester: the user requesting the upgrade
+            old_room_id: the id of the room to be replaced
+            new_version: the new room version to use
 
         Returns:
             Deferred[unicode]: the new room id
@@ -299,18 +301,22 @@ def _update_upgraded_room_pls(
 
     @defer.inlineCallbacks
     def clone_existing_room(
-        self, requester, old_room_id, new_room_id, new_room_version, tombstone_event_id
+        self,
+        requester: Requester,
+        old_room_id: str,
+        new_room_id: str,
+        new_room_version: RoomVersion,
+        tombstone_event_id: str,
     ):
         """Populate a new room based on an old room
 
         Args:
-            requester (synapse.types.Requester): the user requesting the upgrade
-            old_room_id (unicode): the id of the room to be replaced
-            new_room_id (unicode): the id to give the new room (should already have been
+            requester: the user requesting the upgrade
+            old_room_id : the id of the room to be replaced
+            new_room_id: the id to give the new room (should already have been
                 created with _gemerate_room_id())
-            new_room_version (unicode): the new room version to use
-            tombstone_event_id (unicode|str): the ID of the tombstone event in the old
-                room.
+            new_room_version: the new room version to use
+            tombstone_event_id: the ID of the tombstone event in the old room.
         Returns:
             Deferred
         """
@@ -320,7 +326,7 @@ def clone_existing_room(
             raise SynapseError(403, "You are not permitted to create rooms")
 
         creation_content = {
-            "room_version": new_room_version,
+            "room_version": new_room_version.identifier,
             "predecessor": {"room_id": old_room_id, "event_id": tombstone_event_id},
         }
 
@@ -577,14 +583,15 @@ def create_room(self, requester, config, ratelimit=True, creator_join_profile=No
         if ratelimit:
             yield self.ratelimit(requester)
 
-        room_version = config.get(
+        room_version_id = config.get(
             "room_version", self.config.default_room_version.identifier
         )
 
-        if not isinstance(room_version, string_types):
+        if not isinstance(room_version_id, string_types):
             raise SynapseError(400, "room_version must be a string", Codes.BAD_JSON)
 
-        if room_version not in KNOWN_ROOM_VERSIONS:
+        room_version = KNOWN_ROOM_VERSIONS.get(room_version_id)
+        if room_version is None:
             raise SynapseError(
                 400,
                 "Your homeserver does not support this room version",
@@ -660,7 +667,7 @@ def create_room(self, requester, config, ratelimit=True, creator_join_profile=No
         creation_content = config.get("creation_content", {})
 
         # override any attempt to set room versions via the creation_content
-        creation_content["room_version"] = room_version
+        creation_content["room_version"] = room_version.identifier
 
         yield self._send_events_for_new_room(
             requester,
diff --git a/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py b/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py
index ca97330797ca..f357015a7001 100644
--- a/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py
+++ b/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py
@@ -64,7 +64,8 @@ async def on_POST(self, request, room_id):
         assert_params_in_dict(content, ("new_version",))
         new_version = content["new_version"]
 
-        if new_version not in KNOWN_ROOM_VERSIONS:
+        new_version = KNOWN_ROOM_VERSIONS.get(content["new_version"])
+        if new_version is None:
             raise SynapseError(
                 400,
                 "Your homeserver does not support this room version",

From 963237fe162ff8da086e6c8d3ac5808e6de6f521 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 15:08:00 +0000
Subject: [PATCH 04/12] Add rooms.room_version column and populate it in
 store_room

---
 synapse/handlers/federation.py                | 19 ++++++++++++++++--
 synapse/handlers/room.py                      | 11 +++++++---
 synapse/storage/data_stores/main/room.py      | 19 +++++++++++++-----
 .../schema/delta/57/rooms_version_column.sql  | 20 +++++++++++++++++++
 tests/storage/test_room.py                    |  7 ++++++-
 tests/storage/test_state.py                   |  5 ++++-
 tests/utils.py                                |  8 ++++++++
 7 files changed, 77 insertions(+), 12 deletions(-)
 create mode 100644 synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 12440bed47ad..8485f3df119f 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -703,8 +703,20 @@ async def _process_received_pdu(
 
         if not room:
             try:
+                prev_state_ids = await context.get_prev_state_ids()
+                create_event = await self.store.get_event(
+                    prev_state_ids[(EventTypes.Create, "")]
+                )
+
+                room_version_id = create_event.content.get(
+                    "room_version", RoomVersions.V1.identifier
+                )
+
                 await self.store.store_room(
-                    room_id=room_id, room_creator_user_id="", is_public=False
+                    room_id=room_id,
+                    room_creator_user_id="",
+                    is_public=False,
+                    room_version=KNOWN_ROOM_VERSIONS[room_version_id],
                 )
             except StoreError:
                 logger.exception("Failed to store room.")
@@ -1236,7 +1248,10 @@ def do_invite_join(self, target_hosts, room_id, joinee, content):
 
             try:
                 yield self.store.store_room(
-                    room_id=room_id, room_creator_user_id="", is_public=False
+                    room_id=room_id,
+                    room_creator_user_id="",
+                    is_public=False,
+                    room_version=room_version,
                 )
             except Exception:
                 # FIXME
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index f304210ac344..a9490782b7e6 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -153,7 +153,7 @@ def _upgrade_room(self, requester, old_room_id, new_version):
         if r is None:
             raise NotFoundError("Unknown room id %s" % (old_room_id,))
         new_room_id = yield self._generate_room_id(
-            creator_id=user_id, is_public=r["is_public"]
+            creator_id=user_id, is_public=r["is_public"], room_version=new_version,
         )
 
         logger.info("Creating new room %s to replace %s", new_room_id, old_room_id)
@@ -638,7 +638,9 @@ def create_room(self, requester, config, ratelimit=True, creator_join_profile=No
         visibility = config.get("visibility", None)
         is_public = visibility == "public"
 
-        room_id = yield self._generate_room_id(creator_id=user_id, is_public=is_public)
+        room_id = yield self._generate_room_id(
+            creator_id=user_id, is_public=is_public, room_version=room_version,
+        )
 
         directory_handler = self.hs.get_handlers().directory_handler
         if room_alias:
@@ -856,7 +858,9 @@ def send(etype, content, **kwargs):
             yield send(etype=etype, state_key=state_key, content=content)
 
     @defer.inlineCallbacks
-    def _generate_room_id(self, creator_id, is_public):
+    def _generate_room_id(
+        self, creator_id: str, is_public: str, room_version: RoomVersion,
+    ):
         # autogen room IDs and try to create it. We may clash, so just
         # try a few times till one goes through, giving up eventually.
         attempts = 0
@@ -870,6 +874,7 @@ def _generate_room_id(self, creator_id, is_public):
                     room_id=gen_room_id,
                     room_creator_user_id=creator_id,
                     is_public=is_public,
+                    room_version=room_version,
                 )
                 return gen_room_id
             except StoreError:
diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py
index 49bab62be3a6..308d930ee7e0 100644
--- a/synapse/storage/data_stores/main/room.py
+++ b/synapse/storage/data_stores/main/room.py
@@ -28,6 +28,7 @@
 
 from synapse.api.constants import EventTypes
 from synapse.api.errors import StoreError
+from synapse.api.room_versions import RoomVersion
 from synapse.storage._base import SQLBaseStore
 from synapse.storage.data_stores.main.search import SearchStore
 from synapse.storage.database import Database
@@ -758,14 +759,21 @@ def __init__(self, database: Database, db_conn, hs):
         self.config = hs.config
 
     @defer.inlineCallbacks
-    def store_room(self, room_id, room_creator_user_id, is_public):
+    def store_room(
+        self,
+        room_id: str,
+        room_creator_user_id: str,
+        is_public: bool,
+        room_version: RoomVersion,
+    ):
         """Stores a room.
 
         Args:
-            room_id (str): The desired room ID, can be None.
-            room_creator_user_id (str): The user ID of the room creator.
-            is_public (bool): True to indicate that this room should appear in
-            public room lists.
+            room_id: The desired room ID, can be None.
+            room_creator_user_id: The user ID of the room creator.
+            is_public: True to indicate that this room should appear in
+                public room lists.
+            room_version: The version of the room
         Raises:
             StoreError if the room could not be stored.
         """
@@ -779,6 +787,7 @@ def store_room_txn(txn, next_id):
                         "room_id": room_id,
                         "creator": room_creator_user_id,
                         "is_public": is_public,
+                        "room_version": room_version.identifier,
                     },
                 )
                 if is_public:
diff --git a/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql b/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql
new file mode 100644
index 000000000000..7356b349968b
--- /dev/null
+++ b/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql
@@ -0,0 +1,20 @@
+/* Copyright 2020 The Matrix.org Foundation C.I.C
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+-- We want to start storing the room version independently of
+-- `current_state_events` so that we can delete stale entries from it without
+-- losing the information.
+ALTER TABLE rooms ADD COLUMN room_version TEXT;
diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py
index 3ddaa151fefc..086adeb8fd39 100644
--- a/tests/storage/test_room.py
+++ b/tests/storage/test_room.py
@@ -17,6 +17,7 @@
 from twisted.internet import defer
 
 from synapse.api.constants import EventTypes
+from synapse.api.room_versions import RoomVersions
 from synapse.types import RoomAlias, RoomID, UserID
 
 from tests import unittest
@@ -40,6 +41,7 @@ def setUp(self):
             self.room.to_string(),
             room_creator_user_id=self.u_creator.to_string(),
             is_public=True,
+            room_version=RoomVersions.V1,
         )
 
     @defer.inlineCallbacks
@@ -68,7 +70,10 @@ def setUp(self):
         self.room = RoomID.from_string("!abcde:test")
 
         yield self.store.store_room(
-            self.room.to_string(), room_creator_user_id="@creator:text", is_public=True
+            self.room.to_string(),
+            room_creator_user_id="@creator:text",
+            is_public=True,
+            room_version=RoomVersions.V1,
         )
 
     @defer.inlineCallbacks
diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py
index d6ecf102f894..04d58fbf2479 100644
--- a/tests/storage/test_state.py
+++ b/tests/storage/test_state.py
@@ -45,7 +45,10 @@ def setUp(self):
         self.room = RoomID.from_string("!abc123:test")
 
         yield self.store.store_room(
-            self.room.to_string(), room_creator_user_id="@creator:text", is_public=True
+            self.room.to_string(),
+            room_creator_user_id="@creator:text",
+            is_public=True,
+            room_version=RoomVersions.V1,
         )
 
     @defer.inlineCallbacks
diff --git a/tests/utils.py b/tests/utils.py
index e2e9cafd791a..513f358f4fd0 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -639,9 +639,17 @@ def create_room(hs, room_id, creator_id):
     """
 
     persistence_store = hs.get_storage().persistence
+    store = hs.get_datastore()
     event_builder_factory = hs.get_event_builder_factory()
     event_creation_handler = hs.get_event_creation_handler()
 
+    yield store.store_room(
+        room_id=room_id,
+        room_creator_user_id=creator_id,
+        is_public=False,
+        room_version=RoomVersions.V1,
+    )
+
     builder = event_builder_factory.for_room_version(
         RoomVersions.V1,
         {

From e7618afef02b57689d551c30904c24e7a1bd7289 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 15:09:01 +0000
Subject: [PATCH 05/12] Read room version from rooms table.

---
 synapse/storage/data_stores/main/state.py | 31 ++++++++++++++---------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/synapse/storage/data_stores/main/state.py b/synapse/storage/data_stores/main/state.py
index 33bebd1c485a..58e1eb2cb182 100644
--- a/synapse/storage/data_stores/main/state.py
+++ b/synapse/storage/data_stores/main/state.py
@@ -60,24 +60,31 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore):
     def __init__(self, database: Database, db_conn, hs):
         super(StateGroupWorkerStore, self).__init__(database, db_conn, hs)
 
-    @defer.inlineCallbacks
-    def get_room_version(self, room_id):
+    @cached(max_entries=10000)
+    async def get_room_version(self, room_id: str) -> str:
         """Get the room_version of a given room
 
-        Args:
-            room_id (str)
-
-        Returns:
-            Deferred[str]
-
         Raises:
-            NotFoundError if the room is unknown
+            NotFoundError: if the room is unknown
         """
-        # for now we do this by looking at the create event. We may want to cache this
-        # more intelligently in future.
+
+        # First we try looking up room version from the database, but for old
+        # rooms we might not have added the room version to it yet so we fall
+        # back to previous behaviour and look in current state events.
+
+        version = await self.db.simple_select_one_onecol(
+            table="rooms",
+            keyvalues={"room_id": room_id},
+            retcol="room_version",
+            desc="get_room_version",
+            allow_none=True,
+        )
+
+        if version:
+            return version
 
         # Retrieve the room's create event
-        create_event = yield self.get_create_event_for_room(room_id)
+        create_event = await self.get_create_event_for_room(room_id)
         return create_event.content.get("room_version", "1")
 
     @defer.inlineCallbacks

From 0bb025a22928732f8fefa6e58093574946e4e27c Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 15:11:38 +0000
Subject: [PATCH 06/12] Newsfile

---
 changelog.d/6729.misc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/6729.misc

diff --git a/changelog.d/6729.misc b/changelog.d/6729.misc
new file mode 100644
index 000000000000..5537355bea8e
--- /dev/null
+++ b/changelog.d/6729.misc
@@ -0,0 +1 @@
+Record room versions in the `rooms` table.

From 892bab9742db92765cc1fe3623b0875b3de6e0e7 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 17 Jan 2020 16:13:58 +0000
Subject: [PATCH 07/12] Add background update

---
 synapse/storage/data_stores/main/room.py      | 73 ++++++++++++++++++-
 .../schema/delta/57/rooms_version_column.sql  |  4 +
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py
index 308d930ee7e0..4036710e8fc4 100644
--- a/synapse/storage/data_stores/main/room.py
+++ b/synapse/storage/data_stores/main/room.py
@@ -28,10 +28,10 @@
 
 from synapse.api.constants import EventTypes
 from synapse.api.errors import StoreError
-from synapse.api.room_versions import RoomVersion
+from synapse.api.room_versions import RoomVersion, RoomVersions
 from synapse.storage._base import SQLBaseStore
 from synapse.storage.data_stores.main.search import SearchStore
-from synapse.storage.database import Database
+from synapse.storage.database import Database, LoggingTransaction
 from synapse.types import ThirdPartyInstanceID
 from synapse.util.caches.descriptors import cached, cachedInlineCallbacks
 
@@ -612,6 +612,7 @@ def _quarantine_media_txn(
 
 class RoomBackgroundUpdateStore(SQLBaseStore):
     REMOVE_TOMESTONED_ROOMS_BG_UPDATE = "remove_tombstoned_rooms_from_directory"
+    ADD_ROOMS_ROOM_VERSION_COLUMN = "add_rooms_room_version_column"
 
     def __init__(self, database: Database, db_conn, hs):
         super(RoomBackgroundUpdateStore, self).__init__(database, db_conn, hs)
@@ -627,6 +628,11 @@ def __init__(self, database: Database, db_conn, hs):
             self._remove_tombstoned_rooms_from_directory,
         )
 
+        self.db.updates.register_background_update_handler(
+            self.ADD_ROOMS_ROOM_VERSION_COLUMN,
+            self._background_add_rooms_room_version_column,
+        )
+
     @defer.inlineCallbacks
     def _background_insert_retention(self, progress, batch_size):
         """Retrieves a list of all rooms within a range and inserts an entry for each of
@@ -695,6 +701,69 @@ def _background_insert_retention_txn(txn):
 
         defer.returnValue(batch_size)
 
+    async def _background_add_rooms_room_version_column(
+        self, progress: dict, batch_size: int
+    ):
+        """Background update to go and add room version inforamtion to `rooms`
+        table from `current_state_events` table.
+        """
+
+        last_room_id = progress.get("room_id", "")
+
+        def _background_add_rooms_room_version_column_txn(txn: LoggingTransaction):
+            sql = """
+                SELECT room_id, json FROM current_state_events
+                INNER JOIN event_json USING (room_id, event_id)
+                WHERE room_id > ? AND type = 'm.room.create' AND state_key = ''
+                ORDER BY room_id
+                LIMIT ?
+            """
+
+            txn.execute(sql, (last_room_id, batch_size))
+
+            updates = []
+            for room_id, event_json in txn:
+                event_dict = json.loads(event_json)
+                room_version_id = event_dict.get("content", {}).get(
+                    "room_version", RoomVersions.V1.identifier
+                )
+
+                creator = event_dict.get("content").get("creator")
+
+                updates.append((room_id, creator, room_version_id))
+
+            if not updates:
+                return True
+
+            new_last_room_id = ""
+            for room_id, creator, room_version_id in updates:
+                self.db.simple_upsert_txn(
+                    txn,
+                    table="rooms",
+                    keyvalues={"room_id": room_id},
+                    values={"room_version": room_version_id},
+                    insertion_values={"is_public": False, "creator": creator},
+                )
+                new_last_room_id = room_id
+
+            self.db.updates._background_update_progress_txn(
+                txn, self.ADD_ROOMS_ROOM_VERSION_COLUMN, {"room_id": new_last_room_id}
+            )
+
+            return False
+
+        end = await self.db.runInteraction(
+            "_background_add_rooms_room_version_column",
+            _background_add_rooms_room_version_column_txn,
+        )
+
+        if end:
+            await self.db.updates._end_background_update(
+                self.ADD_ROOMS_ROOM_VERSION_COLUMN
+            )
+
+        return batch_size
+
     async def _remove_tombstoned_rooms_from_directory(
         self, progress, batch_size
     ) -> int:
diff --git a/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql b/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql
index 7356b349968b..352a66f5b0b0 100644
--- a/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql
+++ b/synapse/storage/data_stores/main/schema/delta/57/rooms_version_column.sql
@@ -18,3 +18,7 @@
 -- `current_state_events` so that we can delete stale entries from it without
 -- losing the information.
 ALTER TABLE rooms ADD COLUMN room_version TEXT;
+
+
+INSERT into background_updates (update_name, progress_json)
+    VALUES ('add_rooms_room_version_column', '{}');

From 70be703c791da3ea2fbabfd34b70f2952f6ae58b Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 21 Jan 2020 11:17:51 +0000
Subject: [PATCH 08/12] Correctly raise exception if unknown room version

---
 synapse/federation/federation_client.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py
index a5a3c177e37f..969ed685f3c5 100644
--- a/synapse/federation/federation_client.py
+++ b/synapse/federation/federation_client.py
@@ -30,6 +30,7 @@
     FederationDeniedError,
     HttpResponseException,
     SynapseError,
+    UnsupportedRoomVersionError,
 )
 from synapse.api.room_versions import (
     KNOWN_ROOM_VERSIONS,
@@ -461,6 +462,9 @@ def send_request(destination):
             # however either way the event format version will be v1.
             room_version_id = ret.get("room_version", RoomVersions.V1.identifier)
             room_version = KNOWN_ROOM_VERSIONS.get(room_version_id)
+            if not room_version:
+                raise UnsupportedRoomVersionError()
+
             event_format = room_version_to_event_format(room_version_id)
 
             pdu_dict = ret.get("event", None)

From ced73b3a15ecfc6c1ada611f08d32cdd99d947b1 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 21 Jan 2020 11:29:16 +0000
Subject: [PATCH 09/12] Add comment

---
 synapse/handlers/federation.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 8485f3df119f..f824ee79a0ba 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1853,6 +1853,8 @@ def _persist_auth_tree(
             auth_events
             state
             event
+            room_version: The room version we expect this room to have, and
+                will raise if it doesn't match the version in the create event.
 
         Returns:
             Deferred

From 53a75fab583010dd942786a09c5e42878b028cb8 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 21 Jan 2020 11:29:40 +0000
Subject: [PATCH 10/12] Use explicit is not None

---
 synapse/storage/data_stores/main/state.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/storage/data_stores/main/state.py b/synapse/storage/data_stores/main/state.py
index 58e1eb2cb182..837f1e517ddd 100644
--- a/synapse/storage/data_stores/main/state.py
+++ b/synapse/storage/data_stores/main/state.py
@@ -80,7 +80,7 @@ async def get_room_version(self, room_id: str) -> str:
             allow_none=True,
         )
 
-        if version:
+        if version is not None:
             return version
 
         # Retrieve the room's create event

From e825fd00dbda60c93bf6f0ac74b6d49e1d4da66c Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 21 Jan 2020 11:32:35 +0000
Subject: [PATCH 11/12] Add comments about paranoia

---
 synapse/storage/data_stores/main/room.py  | 4 ++++
 synapse/storage/data_stores/main/state.py | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py
index 4036710e8fc4..f1589b88f484 100644
--- a/synapse/storage/data_stores/main/room.py
+++ b/synapse/storage/data_stores/main/room.py
@@ -737,6 +737,10 @@ def _background_add_rooms_room_version_column_txn(txn: LoggingTransaction):
 
             new_last_room_id = ""
             for room_id, creator, room_version_id in updates:
+                # We upsert here just in case we don't already have a row,
+                # mainly for paranoia as much badness would happen if we don't
+                # insert the row and then try and get the room version for the
+                # room.
                 self.db.simple_upsert_txn(
                     txn,
                     table="rooms",
diff --git a/synapse/storage/data_stores/main/state.py b/synapse/storage/data_stores/main/state.py
index 837f1e517ddd..bd7b0276f14d 100644
--- a/synapse/storage/data_stores/main/state.py
+++ b/synapse/storage/data_stores/main/state.py
@@ -72,6 +72,9 @@ async def get_room_version(self, room_id: str) -> str:
         # rooms we might not have added the room version to it yet so we fall
         # back to previous behaviour and look in current state events.
 
+        # We really should have an entry in the rooms table for every room we
+        # care about, but let's be a bit paranoid (at least while the background
+        # update is happening) to avoid breaking existing rooms.
         version = await self.db.simple_select_one_onecol(
             table="rooms",
             keyvalues={"room_id": room_id},

From 1474a498458cb50190d84093a44520e11cf19217 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 27 Jan 2020 14:03:11 +0000
Subject: [PATCH 12/12] Correctly handle and document
 UnsupportedRoomVersionError

---
 synapse/federation/federation_client.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py
index 969ed685f3c5..d57e8ca7a280 100644
--- a/synapse/federation/federation_client.py
+++ b/synapse/federation/federation_client.py
@@ -387,6 +387,8 @@ def _try_destination_list(self, description, destinations, callback):
                 return res
             except InvalidResponseError as e:
                 logger.warning("Failed to %s via %s: %s", description, destination, e)
+            except UnsupportedRoomVersionError:
+                raise
             except HttpResponseException as e:
                 if not 500 <= e.code < 600:
                     raise e.to_synapse_error()
@@ -440,6 +442,9 @@ def make_membership_event(
             homeserver which generated the event, and room_version is the
             version of the room.
 
+            Fails with a `UnsupportedRoomVersionError` if remote responds with
+            a room version we don't understand.
+
             Fails with a ``SynapseError`` if the chosen remote server
             returns a 300/400 code.