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

CryptoV2 changes #1622

Merged
merged 3 commits into from
Oct 31, 2022
Merged

CryptoV2 changes #1622

merged 3 commits into from
Oct 31, 2022

Conversation

Anderas
Copy link
Contributor

@Anderas Anderas commented Oct 31, 2022

A number of changes to rust-based Crypto V2 after extensive testing with polyjuice:

  • expose VerificationState on decryption result to display trusted / untrusted sessions
  • ensure CryptoV2 gets initialized on the main queue
  • move event observers from key verification manager to crypto v2 to gain access to downloading user keys and processing outgoing requests
  • always download keys for new senders before processing to-device events
  • restore room keys from backup after recieving recovery key

@Anderas Anderas requested review from a team and aringenbach and removed request for a team October 31, 2022 08:47
@codecov
Copy link

codecov bot commented Oct 31, 2022

Codecov Report

Base: 36.37% // Head: 15.88% // Decreases project coverage by -20.49% ⚠️

Coverage data is based on head (4174724) compared to base (befcd80).
Patch coverage: 11.14% of modified lines in pull request are covered.

Additional details and impacted files
@@                    Coverage Diff                    @@
##           andy/complete_crypto    #1622       +/-   ##
=========================================================
- Coverage                 36.37%   15.88%   -20.50%     
=========================================================
  Files                       577      578        +1     
  Lines                     90876    91061      +185     
  Branches                  39535    38397     -1138     
=========================================================
- Hits                      33056    14464    -18592     
- Misses                    56837    76118    +19281     
+ Partials                    983      479      -504     
Impacted Files Coverage Δ
...trixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift 0.00% <0.00%> (ø)
...chine/MXEventDecryptionResult+DecryptedEvent.swift 0.00% <0.00%> (ø)
MatrixSDK/Crypto/KeyBackup/MXKeyBackup.m 0.00% <0.00%> (-62.14%) ⬇️
MatrixSDK/Crypto/MXCryptoV2.swift 0.79% <0.00%> (-0.13%) ⬇️
...K/Crypto/SecretStorage/MXCryptoSecretStoreV2.swift 0.00% <0.00%> (ø)
MatrixSDK/JSONModels/MXJSONModels.m 6.63% <0.00%> (-20.52%) ⬇️
MatrixSDK/MXSession.m 7.96% <0.00%> (-32.01%) ⬇️
...rypto/CryptoMachine/MXCryptoMachineUnitTests.swift 0.00% <0.00%> (ø)
...ypto/Verification/MXKeyVerificationManagerV2.swift 27.77% <10.00%> (-1.31%) ⬇️
...s/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift 51.31% <12.50%> (-2.11%) ⬇️
... and 196 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Anderas Anderas requested review from a team and paleksandrs and removed request for aringenbach and a team October 31, 2022 10:15
Copy link
Contributor

@paleksandrs paleksandrs left a comment

Choose a reason for hiding this comment

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

Looks good to me. Do you need to add a changelog for this?

@Anderas
Copy link
Contributor Author

Anderas commented Oct 31, 2022

Looks good to me. Do you need to add a changelog for this?

Its pretty much bugfixes for #1620, I just did not want to do it in the same PR for readability, so I don't think changelog is necessary

@Anderas Anderas merged commit c58594c into andy/complete_crypto Oct 31, 2022
@Anderas Anderas deleted the andy/crypto_changes branch October 31, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants