Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC3289: Room version 8 #3289

Merged
merged 4 commits into from
Aug 8, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions proposals/3289-rooms-v8.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# MSC3289: Room Version 8

A new room version, `8`, is proposed using [room version 7](https://spec.matrix.org/unstable/rooms/v7/)
clokep marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about what I would expect to see before I'm happy to resolve my concern about the lack of an implementation.

Implementation: matrix-org/synapse@ccdcc7f

This is noted, and thanks, but I'd like to see the following:

  • a Synapse has been deployed with a test room version exactly matching the proposed parameters and that people have actually sent messages through it and they have been correctly received. In particular my understanding is that substantial parts of MSC3083 haven't yet been tested outside a development environment.
  • Given our recent discovery of the problem with redactions, I think some manual testing that auth still works when various bits of the room state are redacted would be wise.
  • I'd like to know that somebody has tested the test room version with Sytest's --room-version (and Complement's equivalent, if there is one).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to know that somebody has tested the test room version with Sytest's --room-version (and Complement's equivalent, if there is one).

Thia has been done, but needs matrix-org/sytest#1100 so that the fake server knows about the room version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, I was somewhat expecting the sytesting to happen against org.matrix.msc3083.v2 (or whatever) to save having to wait for this MSC to complete. But I realise that means more sytest PRs, and your approach of using a sytest branch is equally valid.

@clokep: how are the other two bullet points looking?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given our recent discovery of the problem with redactions, I think some manual testing that auth still works when various bits of the room state are redacted would be wise.

I tested this manually again by:

  1. Creating a test room set to join via a space.
  2. Ensuring that I could join the space, then room.
  3. Ensuring that joining the room first does not work.
  4. Ensuring that invites work.
  5. I then redacted the m.room.join_rules event.
  6. I then repeated tests 2 - 4 with new users and ensured the checks were still enforced by the server.

Does that sound sufficient or have I missed something?

a Synapse has been deployed with a test room version exactly matching the proposed parameters and that people have actually sent messages through it and they have been correctly received. In particular my understanding is that substantial parts of MSC3083 haven't yet been tested outside a development environment.

matrix.org now supports the org.matrix.msc3083.v2 room and I have created a test room which is joinable via membership in #community:matrix.org. This is not quite "exactly matching the proposed parameters" however as org.matrix.msc3083.v2 is based on room version 6, not room version 7 (it doesn't support knocking).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it sounds grand to me.

as a base and incorporating the following MSCs:

* [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083) - Restricting room
membership based on membership in other rooms
clokep marked this conversation as resolved.
Show resolved Hide resolved
uhoreg marked this conversation as resolved.
Show resolved Hide resolved

Though other MSCs are capable of being included in this version, they do not have
sufficient implementation to be considered for this room version. A future room
version may include them.
clokep marked this conversation as resolved.
Show resolved Hide resolved

Room version `8` upon being added to the specification shall be considered stable.
No other room versions are affected by this MSC. Before room version `8` can enter
the specification, MSC3083 needs to complete its final comment period.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved