-
Notifications
You must be signed in to change notification settings - Fork 252
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: Fetch immediately-available sender data when we receive a room key #3590
Conversation
cc82725
to
d53fe25
Compare
626c497
to
f4ad80b
Compare
d53fe25
to
f1ad14a
Compare
73cfc13
to
ae2756b
Compare
f1ad14a
to
7152714
Compare
Rebasing because this is a mess after the merge of #3556 |
289f653
to
dd5baf8
Compare
Rebased on top of #3633 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3590 +/- ##
=======================================
Coverage 84.37% 84.37%
=======================================
Files 258 259 +1
Lines 26656 26722 +66
=======================================
+ Hits 22491 22547 +56
- Misses 4165 4175 +10 ☔ View full report in Codecov by Sentry. |
4ad12a0
to
3f79f0f
Compare
1c5f302
to
25ed92f
Compare
3f79f0f
to
5d3f9ed
Compare
@poljar this is now rebased and ready to review. |
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 we might simplify this a bit. Please correct me if I'm wrong.
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
7530f67
to
41d024b
Compare
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 that this can be further simplified if we use the functionality the Device
offers.
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
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.
Alright, I think this is functionally fine, except maybe one thing I raised we should discuss.
I left a bunch of documentation nits, feel free to fight back on them if you disagree.
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs
Outdated
Show resolved
Hide resolved
…larly to ones signed by unknown keys
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 that this is good now, let's just clear up the history I wonder why our CI check for fixup commits didn't kick in, perhaps because the PR is older than the CI check?
Or maybe because I just manually named the commits with |
Part of #3543. Builds on top of #3556
Implements the "fast lane" as described in #3544
This will begin to populate
InboundGroupSession
s with the newSenderData
struct introduced in #3556 but it will only do it when the information is already available in the store. Future PRs for this issue will query Matrix APIs using spawned async tasks.Future issues will do retries and migration of old sessions.