-
Notifications
You must be signed in to change notification settings - Fork 112
Switch to Rust crypto #2467
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
Switch to Rust crypto #2467
Conversation
Here I've implemented an MVP for the new unified grid layout, which scales smoothly up to arbitrarily many participants. It doesn't yet have a special 1:1 layout, so in spotlight mode and 1:1s, we will still fall back to the legacy grid systems. Things that happened along the way: - The part of VideoTile that is common to both spotlight and grid tiles, I refactored into MediaView - VideoTile renamed to GridTile - Added SpotlightTile for the new, glassy spotlight designs - NewVideoGrid renamed to Grid, and refactored to be even more generic - I extracted the media name logic into a custom React hook - Deleted the BigGrid experiment
Also removing some unused settings along the way.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## livekit #2467 +/- ##
===========================================
- Coverage 25.02% 17.02% -8.00%
===========================================
Files 47 142 +95
Lines 2382 19251 +16869
Branches 434 249 -185
===========================================
+ Hits 596 3278 +2682
- Misses 1735 15973 +14238
+ Partials 51 0 -51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
As you mention in the description, this is: Based on #2466. |
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.
This really is easy 😃
If you want, you could rebase this onto livekit
to make it independent of your other PRs, and it could be merged without conflicts much sooner.
Yeah, I guess I could un-stack this one, but it's like… I know that it would create conflicts for some other things in the stack, requiring me to rebase N other changes, and it's not a very impactful or time-sensitive change, so I'd rather just keep it on the stack. |
Unified grid layout
Refactor settings to use observables
react-rxjs is the library we've been using to connect our React components to view models and consume observables. However, after spending some time with react-rxjs, I feel that it's a very heavy-handed solution. It requires us to sprinkle <Subscribe /> and <RemoveSubscribe /> components all throughout the code, and makes React go through an extra render cycle whenever we mount a component that binds to a view model. What I really want is a lightweight React hook that just gets the current value out of a plain observable, without any extra setup. Luckily the observable-hooks library with its useObservableEagerState hook seems to do just that—and it's more actively maintained, too!
Because we were hiding even the local participant during initial connection, there would be no participants, and therefore nothing to put in the spotlight. The designs don't really tell us what the connecting state should look like, so I've taken the liberty of restoring it to its former glory of showing the local participant immediately.
Includes the mobile UX optimizations and the tweaks we've made to cut down on wasted space, but does not yet include the change to embed the spotlight tile within the grid.
Codecov hasn't been working recently because Vitest doesn't report coverage by default.
This way we benefit from not having to maintain the same directory structure twice, and our linters etc. will actually lint test files by default.
Vitest provides globals primarily to make the transition from Jest more smooth. But importing its functions explicitly is considered a better pattern, and we have so few tests right now that it's trivial to migrate them all.
We no longer use Storybook.
We're not really using crypto at the moment, but with it being this easy, we should still make the switch!
Is this actually still blocked? |
Also blocked on missing rust encrypted to device sending matrix-org/matrix-js-sdk#3304 |
We're not using that API currently (though it being missing would slow our adoption of a new individual sender keys encryption scheme) |
I am wondering how we currently distribute megolm keys without toDevice? Is it just custom toDevice messages that are missing in the wasm bindings? |
that's abstracted away by the js-sdk. the rust js-sdk is internally using toDevice msg but the API is not exposed to the js layer. |
As this is now on the critical path for looking at sender key distribution I have made #2571 which has just the Rust crypto changes in it to make review easier. |
We're not really using crypto at the moment, but with it being this easy, we should still make the switch!
Based on #2466