Extract common crypto interface for all flavors #8470
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bmarty
approved these changes
May 26, 2023
| @@ -246,7 +246,7 @@ internal abstract class CryptoModule { | |||
| abstract fun bindVerificationService(service: RustVerificationService): VerificationService | |||
|
|
|||
| @Binds | |||
| abstract fun bindCryptoStore(store: RealmCryptoStore): IMXCryptoStore | |||
| abstract fun bindCryptoStore(store: RustCryptoStore): IMXCommonCryptoStore | |||
There was a problem hiding this comment.
This is the location of the fix, right?
There was a problem hiding this comment.
Mainly yes.
The IMXCryptoStore, RealmCryptoStore, InboundGroupSessionStore, OlmSessionStore have all moved to the kotlin flavour source folder.
And there is the new RustCryptoStore in the rust flavour sources
|
SonarCloud Quality Gate failed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Type of change
Content
The full realmCrypto store API was still available in the rust flavor, it's problematic because several things have moved from realm and are now stored in the rust sdk.
As an example EventEditValidator was calling
deviceWithIdentityKeyfrom the legacy realm store on rust flavor, thus getting absent or outdated information.As a solution for that a IMXCommonCryptoStore interface has been extracted, that contains API that both falvors are still using. The RealmCryptoStore (IMXCryptoStore) full API has been moved to the kotlinCrypto flavor, and a RustCryptoStore has been introduced for the rustCryptoFalvor.
In a following PR we will see how to complitely remove the need for a realm crypto store, some changes are needed on the rust sdk in order to do it now (partial support is there, but not yet integrated, and some more might be needed).
One exception to that rule, the rust flavor is migrating megolm sessions lazyly, so it needs to access the legacy store for a moment.
Also added some defensive coding for some crash reported on sentry when trying to import megolm session from legacy store (added a log and tryOrNull in RequestSender for now).
The logic has changed in EventEditValidator because the rust-sdk doesn't have an API to get a device from a senderKey, you need to provide a userId as well in rust. Updated the test accordingly.
Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist