Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC2314: A Method to Backfill Room State #2314
MSC2314: A Method to Backfill Room State #2314
Changes from 2 commits
615f91a
5cd0a60
ae147a6
1253584
7d88e56
ec68e33
610f5e3
9558b3a
f50c3c5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 do you see this API actually being used? The "current state" isn't really a concept that is used much in s2s, as if the server wants to start participating in the room it needs a set of most recent events and their state (from which the current state can be derived).
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.
Yes, it's asking another server for it's current state, in a situation where it knows it's joined to the room but is missing state (data loss).
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.
But how does it actually use the current state? I don't think the current state is useful in rehydrating a room for example, as you need the extremities and the state at each of them.
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.
hrm, it seems I missed this thread previously. @erikjohnston raises a good point - it is very hard to see how this API could be usefully used.
If we can't figure out a practical use for this change, we should instead reject it, and back out the changes in Synapse and Sytest.
@erikjohnston any further thoughts here?
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.
It does feel like it should be useful, but I can't really see how. If this is mainly useful for rescuing a server that has suffered catastrophic memory lost then I think I'd like to see a step-by-step write up of the process of recovering from that, including any new APIs that need to be added.
Having a one stop "bootstrap this room for me" API might be useful, which returns the extremities, auth chain and state at each event (probably with some clever de-duplication there)
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.
right then. In that case, I'm going to close this MSC, and back out the changes in sytest and 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.
servers might also have to worry about malicious attempts to get the server to join rooms. A server that really wants you to join a room could send the server an event over
/send
, therefore confusing the server into thinking it might be in the room. The victim server would then use this new API, find out some information about the room (possibly a falsified join event) and push the room through as joined. In theory at some point a signature check would fail, but another workaround could be to ask a couple servers in the room (if possible) to verify the state returned by a server.