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

feat: Add wasm support #1316

Closed
wants to merge 13 commits into from

Conversation

augustocdias
Copy link
Contributor

@augustocdias augustocdias commented Mar 9, 2023

This adds wasm support to OpenMls. I've added new workflow jobs to run tests on wasm target. They'll be tested in chrome and firefox.

Some library changes were needed in order to make this work:

  • fluvio-wasm-timer was added in order to provide the std::time structs. This is basically a wrapper that will use either std::time or the web-sys crate.
  • ed25519-dalek was upgraded because it seems the underlying rand-core is not compatible with wasm
  • rayon and criterion will be available just for non wasm targets.

About rayon, at the moment for wasm it will use a regular iterator instead of the parallel. There's the option to add wasm-bindgen-rayon. This will provide parallel iterators for wasm but we could re-export the function init_thread_pool to OpenMls consumers as it needs to be called before using it (only in wasm targets) or we would have to document that they have to add this as dependency and call it. If you're willing to have it I could add it behind a feature so consumers can choose what fits best for them.

What still needs to be done is add the macro #[wasm_bindgen_test] to all tests and the call in each test module to wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser). Tests that don't have this will be ignored. At the moment I've added in just one so you can see what kinds of changes need to be done in the tests to run on wasm. If you're okay with it I could adapt and make sure all tests run on wasm. (I can do it in a separate PR if you prefer)

I've noticed that the SingatureKeyPair struct needs a random generator, shouldn't it be using OpenMlsCrypto instead of re-implementing it?

@augustocdias augustocdias requested a review from a team as a code owner March 9, 2023 13:40
@raphaelrobert raphaelrobert added enhancement New feature or request lib-feature labels Mar 20, 2023
@augustocdias
Copy link
Contributor Author

There's one test failing. After debugging I found out that in the json file: test_vectors/treekem.json the private keys are with 64 bytes (which is actually the keypair). This is causing this test to fail as the SigningKey is expected to have 32 bytes. If I read the SigningKey as a keypair, the failing test works but it breaks several other tests that are passing 32 bytes in the private key in the SignatureKeypair struct.

What should I do in this case?

@augustocdias augustocdias force-pushed the augustocdias/wasm-support branch from 4a042d4 to 42107dc Compare April 12, 2023 14:25
@franziskuskiefer franziskuskiefer added the waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 5, 2023
@raphaelrobert
Copy link
Member

Closing because this is now replaced by #1483.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lib-feature waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants