[lib][web][keyserver] add Vodozemac library and .wasm file handling
#324
Conversation
|
|
Ashoat
left a comment
Ashoat
left a comment
There was a problem hiding this comment.
Why don't we need to change baseDevBrowserConfig or baseProdBrowserConfig in web/webpack.config.js?
web/push-notif/notif-crypto-utils.js
Outdated
|
|
||
| export type WebNotifsServiceUtilsData = { | ||
| +olmWasmPath: string, | ||
| +vodozemacWasmPath: string, |
There was a problem hiding this comment.
Shouldn't this be optional, since it's persisted and the current version won't have vodozemacWasmPath?
| accountEncryptionKey, | ||
| ), | ||
| olm.init({ locateFile: () => olmWasmPath }), | ||
| initVodozemac(vodozemacWasmPath), |
There was a problem hiding this comment.
Just wondering, why don't we call getVodozemacWasmPath() here too, like we do for desktop? Is the reason we persist the path to local storage because we want to match the WASM build to the service worker, as opposed to just getting the latest?
There was a problem hiding this comment.
We can't call getVodozemacWasmPath() because it uses window.location.origin - which is not defined for shared and service workers.
Looks like the approach was that getVodozemacWasmPath() is called on the main thread, then sends a message to all workers that persist it and use whenever needed. We had this approach I am not a fan of, calling olm.init() in many places - it will be enough to call it once, but I guess it was persisted to have it available each time.
That being said, calling getVodozemacWasmPath() in the desktop is wrong - but initOlm() was wrong too. It didn't fail because probably something else initiated .wasm file earlier (this call is no-op when .wasm is loaded).
It makes me feel this can be connected with ENG-11552.
Let me test that and see what is going on
There was a problem hiding this comment.
That being said, calling getVodozemacWasmPath() in the desktop is wrong - but initOlm() was wrong too. It didn't fail because probably something else initiated .wasm file earlier (this call is no-op when .wasm is loaded).
Please ignore that; this is a mistake. There is no service worker on Electron, notifs are decrypted on the Electron renderer process, which has a window object, which is why the existing code and previous initOlm() were fine.
| await initBackupClientModule(modulePath); | ||
| } | ||
|
|
||
| async function initVodozemacModule( |
There was a problem hiding this comment.
What's the Olm equivalent of this? Confusing that we don't have a corresponding declaration, given that Olm needs to be initialized too
There was a problem hiding this comment.
This is a testing leftover. Olm/Vodozemac are initialized as part of initializeCryptoAccount call - removing
| } else { | ||
| modulePath = `${webworkerModulesFilePath}/${DEFAULT_VODOZEMAC_FILENAME}`; | ||
| } | ||
| console.log(modulePath); |
There was a problem hiding this comment.
Did you forget to remove this console.log?
| } | ||
|
|
||
| async function initializeCryptoAccount( | ||
| olmWasmPath: string, |
There was a problem hiding this comment.
We don't seem to be using this param anymore
I think this is an artifact from the past (it was added in 2023). Later, we migrated Olm and moved all crypto operations to the worker, so I think updating |
Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Using the same pattern as have for existing `.wasm` files (Olm, SQLite and Backup Client). Currently linking my local library, so CI will fail. This should be updated to NPM package after CommE2E/vodozemac#3 is merged and published. This diff shouldn't be landed without following one with API migration, because it replaces initializing Olm with Vodozemac, so Olm is not working anymore. Depends on D15546 Test Plan: Run web and keyserver, vodozemac should be available, usage tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15547
xsanm
force-pushed
the
vodozemac-work-part-2
branch
from
f3e1025 to
080ab37
Compare
* [lib][web][keyserver] replace Olm with Vodozemac's Olm Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). There is one thing with naming which is not obvious, this is using Olm name - Olm is still part of Vodozemac crate, so I think this is still correct, but I am okay with updating this too. I tried to make API as similar as it is possible, so there are files when changing import is everything we have to do. I redefined `EncryptResult` in `lib/`. Importing it from Olm was bad idea because it was used on native too, which causes Olm/Vodozemac wasm to be bundled into native, which worked with Olm but not with Vodozemac. Depends on D15547 Test Plan: Test encrypt/decrypt and creating session with Olm and Vodozemac. Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15548 * [native] implement CXX bindings to Vodozemac (#326) Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Making vodozemac available on native. Branch needs to be updated after PRs are merged Test Plan: Tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15549
…#324) * [lib][web][keyserver] add Vodozemac library and `.wasm` file handling Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Using the same pattern as have for existing `.wasm` files (Olm, SQLite and Backup Client). Currently linking my local library, so CI will fail. This should be updated to NPM package after CommE2E/vodozemac#3 is merged and published. This diff shouldn't be landed without following one with API migration, because it replaces initializing Olm with Vodozemac, so Olm is not working anymore. Depends on D15546 Test Plan: Run web and keyserver, vodozemac should be available, usage tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15547 * [lib][web][keyserver] replace Olm with Vodozemac's Olm (#325) * [lib][web][keyserver] replace Olm with Vodozemac's Olm Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). There is one thing with naming which is not obvious, this is using Olm name - Olm is still part of Vodozemac crate, so I think this is still correct, but I am okay with updating this too. I tried to make API as similar as it is possible, so there are files when changing import is everything we have to do. I redefined `EncryptResult` in `lib/`. Importing it from Olm was bad idea because it was used on native too, which causes Olm/Vodozemac wasm to be bundled into native, which worked with Olm but not with Vodozemac. Depends on D15547 Test Plan: Test encrypt/decrypt and creating session with Olm and Vodozemac. Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15548 * [native] implement CXX bindings to Vodozemac (#326) Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Making vodozemac available on native. Branch needs to be updated after PRs are merged Test Plan: Tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15549
Summary:
ENG-11531.
Using the same pattern as we have for existing
.wasmfiles (Olm, SQLite, and Backup Client).This diff shouldn't be landed without a follow-up with the API migration, because it replaces Olm initialization with Vodozemac, so Olm is no longer working.
Depends on D15546
Test Plan: Run web and keyserver, vodozemac should be available, usage tested in the next diff
Reviewers: ashoat
Subscribers: tomek
Differential Revision: https://phab.comm.dev/D15547