-
-
Notifications
You must be signed in to change notification settings - Fork 595
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
Add function to share megolm keys for historical messages, take 2 #1640
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.
I think this looks plausible - may not be a bad idea for ryan to take a quick look too as he has context on the the recents deadlock bugs.
Also important to note that this increases the crypto db version so there's no going back once you've run this.
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.
Looks good overall, thanks! 😄
@@ -192,7 +210,7 @@ utils.inherits(MegolmEncryption, EncryptionAlgorithm); | |||
* OutboundSessionInfo when setup is complete. | |||
*/ | |||
MegolmEncryption.prototype._ensureOutboundSession = async function( | |||
devicesInRoom, blocked, singleOlmCreationPhase, | |||
room, devicesInRoom, blocked, singleOlmCreationPhase, |
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.
Would it make sense for EncryptionAlgorithm
to hold the room model from the start at construction time, rather than expecting in certain functions...? It already has the room ID. Just an idea, perhaps there's a reason to prefer this instead.
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, I don't know if there's a particular reason why the EncryptionAlgorithm
only holds the room ID rather than the whole room model. But I wanted to avoid making too many unrelated changes.
@@ -1370,10 +1400,14 @@ MegolmDecryption.prototype.onRoomKeyEvent = function(event) { | |||
keysClaimed = event.getKeysClaimed(); | |||
} | |||
|
|||
const extraSessionData = {}; | |||
if (content["org.matrix.msc3061.shared_history"]) { | |||
extraSessionData.sharedHistory = 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.
Should this locally stored session data be prefixed with the MSC while it's an experiment, or do we assume it's not necessary to guard local data like that?
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 general, local data doesn't need to be guarded in the same way as data sent between client, as the local client can usually manage any changes itself.
Implements matrix-org/matrix-spec-proposals#3061