-
Notifications
You must be signed in to change notification settings - Fork 385
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
MSC3901: Deleting state #3901
base: main
Are you sure you want to change the base?
MSC3901: Deleting state #3901
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall seems pretty thorough and an accurate assessment of the 2022 discussions.
|
||
We propose to update the definition of event redaction[^spec-redactions] to | ||
specify that all redacted state events contain `m.obsolete: true` in their | ||
content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice, does this mean that content:{}
== obsolete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by ==
? It does mean that all redacted events will have content:{}
AND obsolete:true
but I don't know whether there are other circumstances where content:{}
is valid, especially in events defined outside the spec.
content. | ||
|
||
Note: `membership: "ban"` events are not considered obsolete since this | ||
information is needed in future to prevent bad actors from re-entering a room. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly regarding power DAG P2P work, we treat "invite rejection" as a special important event, as it means you cannot see into an invite-only room. We probably want to keep invite rejections in this list along with bans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note in f0eef7a, thanks. I don't know exactly what an invite rejection looks like so maybe you could suggest wording to make it more explicit?
|
||
If MSC3414 goes ahead, an obsolete encrypted state event should contain | ||
`m.obsolete: true` in its unencrypted content, as a sibling of e.g. `algorithm` | ||
and `ciphertext`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in its encrypted content surely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the part of the content that is not encrypted, so that servers can read it. Did I use ambiguous wording or am I missing something else?
When a client requests to upgrade a room using `POST /rooms/{roomId}/upgrade`, | ||
this should be interpreted by the server as a request not only to create the | ||
room, but also to invite all members of the old room to the new one, with the | ||
same power level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only apply if the upgraded room version == the existing room version. By allowing actual room version bumps we risk breaking entire servers who aren't on the latest version of synapse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we limit it to room "refactorings" (new version==old version)?
How will it break homeservers that don't support this? Won't they just see fairly normal-looking invitations, and then they themselves fail to do the part they are supposed to do? Isn't that "fine"?
proposals/3901-deleting-state.md
Outdated
include anything with a `state_key` that starts with the user's mxid plus | ||
underscore. | ||
* State whose contents include a top-level property `exclude_from_upgrade: | ||
true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels very similar to obsolete events. Some prose explaining the difference and why we can't use one flag would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One justification might be that someone creates custom events and relies on their sender
property. But, since I could not think of a concrete reason I thought I'd try deleting this entirely and see what people think. d0e774e does that.
Clients and external systems continue to use the existing room ID, and servers | ||
use room ID + room version to identify the real actual room. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does traversing history back into the old room work with this scheme? A lot of detail here is left out from the current proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this part is quite forward-looking and not fully-defined.
MSC3325 also mentions as an alternative that the room membership of each user | ||
could be set as `invited` without actually sending an invitation, to avoid | ||
invite spam. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question is what should happen if more than one m.room.tombstone
event is sent in a room. You would be able to invite spam a single user more than once.
I'm leaning towards extending the definition of the invite
join_rule
to mean "users joined to the predecessor room may join without an explicit invite". Essentially "restricted", but only for the predecessor
room. That way you cut down on both bandwidth of sending invites over federation, as well as invite state events in the room.
The fact that a user has joined via this method should be known to other clients in the room, such that they can share historical message keys for the purposes of decrypting old room messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does sound good to me. Any objections from anyone? Does anyone want to write it up in the MSC?
I just had a chat with @anoadragon453 after the very helpful Solving the Historical State Problem in Matrix The fundamental issue here with ideas going for actually deleting things might be that garbage collection, from a computer science theory perspective, fundamentally requires coordination among servers, even consensus, which Matrix cannot and does not want to provide in its current form. Everything is fine with hiding obsolete events from clients - that is just a projection of the data still present on the server, but as soon as you actually want to delete them, you will probably run into problems that require hard drawbacks like requiring that servers cannot be offline for an arbitrary length of time. Room upgrades potentially circumvent the fundamental issue because just starting a new room does not actually garbage-collect the old state, strictly-speaking. However, having a monotonically-increasing integer as sub-version to the room identifier would also require coordination / consensus, or requires hope that concurrent room upgrades do not happen in practice and network partitions are short enough, or actually ignoring everything that was written in the concurrent room instance that "lost". One way to circumvent the issue with monotonically-increasing sub-versions would be to switch to a partial order of sub-versions, i.e., a DAG, which would then map concurrent room upgrades to branches in the DAG. However, one would want have something like a "current" room version - which could be done by having a third room upgrade that merges the branched room upgrades again. Or, the room sub-version DAG could be topologically sorted, like with state resolution - and that is where we were thinking, what about “room upgrades as rooms”? Have a space as a meta-room that contains pointers to the sub-rooms, and a room upgrade is pointer in the space to a new room? Then the sub-version DAG would exactly be the regular space DAG, and topological sorting would be the regular room topological sorting. Also, it would keep room ids as-is, and servers do not need to follow all room ugprades to end up with the current room version. However, this idea also has problems, like keeping the state in the meta-room minimal, i.e., not requiring normal users to join there to be able to take part in the sub-rooms - potentially requires #2444. On the other hand, it would also massively profit from other improvements around spaces, like improved access control / power level inheritance from space to sub-rooms. |
Wow, really interesting stuff. I think this mostly makes me feel that the room sub-versions stuff is way harder than the rest, and should probably be separated out from it to allow the easier stuff to progress... |
OK, I think this MSC is in a decent state. There are various outstanding questions, but it seems to make a reasonable level of sense. The room sub-versions part is under-defined, but I think we can safely leave that until later. So the problem now becomes how to progress this further, which means attempting implementations. I am going to figure out how to take this to Element leadership, but I'd appreciate input from people from other companies on whether they are able to put effort into this. My guess on the Element side is that this will be seen as strategic long term, but not urgent enough to put time into right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For visibility, I had some thoughts that are relevant to state deletion and wrote them up recently on my blog: https://robin.town/blog/distributed-access-control
Specifically, I had the thought that if Matrix room state, as a data structure, used version vectors instead of tombstones as in https://inria.hal.science/hal-00738680, then it seems that implementations could forget deleted state immediately, without the need for coordinated room upgrades.
We propose to update the definition of [membership | ||
events](https://spec.matrix.org/v1.5/client-server-api/#mroommember) so that | ||
every event with `membership: "leave"` must also have `m.obsolete: true` in its | ||
content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, several folks have commented today in a TWIM discussion that forgetting whether a member has left vs. whether they've simply never joined, could be inconvenient for moderation.
Rendered