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

crypto: Allow querying InboundGroupSessions using curve key #3806

Merged
merged 9 commits into from
Sep 2, 2024

Conversation

andybalaam
Copy link
Member

@andybalaam andybalaam commented Aug 6, 2024

This PR adds a new method CryptoStore::get_inbound_group_sessions_for_device_batch, which is required for us to be able to update the information we hold about inbound group sessions when we get a /keys/query response (#3753).

Strongly suggest reviewing commit-by-commit!

Fixes #3752

@andybalaam andybalaam changed the base branch from main to andybalaam/schema-for-finding-igs-for-device August 6, 2024 13:15
@andybalaam andybalaam changed the title crypto: Create a SenderDataType enum crypto: Allow querying InboundGroupSessions using curve key Aug 6, 2024
@richvdh richvdh force-pushed the andybalaam/schema-for-finding-igs-for-device branch from 9c80560 to b08403f Compare August 7, 2024 13:19
Base automatically changed from andybalaam/schema-for-finding-igs-for-device to andyrich/sender-data-from-keys-query August 7, 2024 13:20
@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch from b08403f to bc3fdcf Compare August 7, 2024 13:25
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch from adb6b19 to 928157f Compare August 7, 2024 13:30
@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch from bc3fdcf to 5d7bf58 Compare August 8, 2024 10:46
@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch from 5d7bf58 to 833ff32 Compare August 8, 2024 11:01
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch from 928157f to 2d9fe4d Compare August 8, 2024 11:05
@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch from 833ff32 to 68dd910 Compare August 16, 2024 09:26
@richvdh

This comment was marked as resolved.

@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch 5 times, most recently from b40fefe to d785bb8 Compare August 29, 2024 10:15
@richvdh richvdh closed this Aug 29, 2024
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch from 2d9fe4d to d785bb8 Compare August 29, 2024 10:18
@richvdh richvdh reopened this Aug 29, 2024
@richvdh richvdh changed the base branch from andyrich/sender-data-from-keys-query to main August 29, 2024 10:19
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch from 3d9d3fc to d785bb8 Compare August 29, 2024 10:20
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch from d785bb8 to 99f672e Compare August 29, 2024 11:40
Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 66.21622% with 25 lines in your changes missing coverage. Please review.

Project coverage is 84.12%. Comparing base (8c5ffc9) to head (aa3757f).
Report is 32 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk-sqlite/src/crypto_store.rs 65.90% 15 Missing ⚠️
crates/matrix-sdk-crypto/src/store/traits.rs 0.00% 7 Missing ⚠️
...atrix-sdk-crypto/src/olm/group_sessions/inbound.rs 0.00% 2 Missing ⚠️
...x-sdk-crypto/src/olm/group_sessions/sender_data.rs 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3806      +/-   ##
==========================================
- Coverage   84.16%   84.12%   -0.05%     
==========================================
  Files         266      266              
  Lines       28017    28088      +71     
==========================================
+ Hits        23581    23629      +48     
- Misses       4436     4459      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@richvdh
Copy link
Member

richvdh commented Aug 29, 2024

Seems like the coverage data is wrong here. It seems like it's treating the text of the SQL statement at https://app.codecov.io/gh/matrix-org/matrix-rust-sdk/pull/3806/blob/crates/matrix-sdk-sqlite/src/crypto_store.rs#L517 as a coverage miss... but clearly that method is being executed, so the SQL statement must be being used. I wonder if it would help to pull the text out to a separate const.

@richvdh richvdh marked this pull request as ready for review August 30, 2024 14:30
@richvdh richvdh requested review from a team as code owners August 30, 2024 14:30
@richvdh richvdh requested review from Hywan and richvdh and removed request for a team August 30, 2024 14:30
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch 2 times, most recently from 3ff54b7 to 5177e3f Compare September 2, 2024 12:15
@richvdh richvdh force-pushed the andybalaam/query-for-igs-by-curve-key branch from 0f0df00 to aa3757f Compare September 2, 2024 16:47
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

@andybalaam I co-wrote this and have co-reviewed it. We're both now happy with the changes.

@richvdh richvdh merged commit d8b0f9f into main Sep 2, 2024
39 checks passed
@richvdh richvdh deleted the andybalaam/query-for-igs-by-curve-key branch September 2, 2024 17:07
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.

Sender Data: Allow looking up InboundGroupSessions with missing SenderData by device curve key
2 participants