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

Introduce a mechanism for using the rust-crypto-sdk #2969

Merged
merged 7 commits into from
Dec 14, 2022

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Dec 12, 2022

This PR introduces MatrixClient.initRustCrypto, which is similar to initCrypto, except that it will use the Rust crypto SDK instead of the old libolm-based implementation.

This is very much not something you want to use in production code right now, because the integration with the rust sdk is extremely skeletal and almost everything crypto-related will raise an exception rather than doing anything useful.

It is, however, enough to demonstrate the loading of the wasmified rust sdk in element web, and a react sdk with light modifications can successfully log in and out.

Part of element-hq/element-web#21972.

Element-web notes: none


Here's what your changelog entry will look like:

✨ Features

  • Introduce a mechanism for using the rust-crypto-sdk (#2969).

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

The changes overall lgtm, with minor nits for declaring things breaking for overly clear communication. The yarn.lock is also so massive it broke something, and I'm concerned there's a package manager mismatch somewhere: I've sent you a DM to try and figure this out OOB, though is not necessarily a blocker for this PR landing.

src/client.ts Outdated
// No indexeddb support
return;
}
for (const dbname of ["matrix-js-sdk::matrix-sdk-crypto", "matrix-js-sdk::matrix-sdk-crypto-meta"]) {
Copy link
Member

Choose a reason for hiding this comment

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

I seem to recall that these database names are in a constant somewhere? If so, we should use them.

Copy link
Member

Choose a reason for hiding this comment

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

alternatively, we should consider making them constants for reasons of reliability.

Copy link
Member Author

Choose a reason for hiding this comment

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

not really, tbh: they are defined in the rust-sdk here and here.

alternatively, we should consider making them constants for reasons of reliability.

Given they are only used once, I'm unconvinced that a constant will help reliability.

The prefix does come from the js-sdk so I can factor that out to a constant.

Copy link
Member Author

@richvdh richvdh Dec 13, 2022

Choose a reason for hiding this comment

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

The prefix does come from the js-sdk so I can factor that out to a constant.

done in df8d44b.

src/client.ts Outdated Show resolved Hide resolved
// import { logger } from "../logger";

/**
* An implementation of {@link CryptoBackend} using the Rust matrix-sdk-crypto.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* An implementation of {@link CryptoBackend} using the Rust matrix-sdk-crypto.
* An implementation of {@link CryptoBackend} using the Rust matrix-sdk-crypto.
* @experimental This entire class is subject to breaking changes, without notice.

or whatever jsdoc wants to say this.

Copy link
Member

Choose a reason for hiding this comment

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

alternatively, apparently @internal will exclude it from public types, despite being public itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it's not exported explicitly, and it's only dynamically referenced, so it doesn't actually appear in the generated documentation.

I don't know how or why yarn did this, but it's not doing it any more.
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

thanks!

@richvdh richvdh merged commit 15ef8fa into develop Dec 14, 2022
@richvdh richvdh deleted the rav/element-r/rust-crypto branch December 14, 2022 11:02
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this pull request Dec 16, 2022
This PR adds an option to `config.json` which will make the js-sdk use the rust crypto sdk, instead of the libolm implementation.

To use it, you need to add something like this to `config.json`:

```
    "features": {
        "feature_rust_crypto": true
    },
```

We don't (yet) have any way to migrate a device between implementations, so the setting that was in use when you log in is persisted to the device; it is *visible* via the labs section but cannot currently be changed.

This is part of element-hq/element-web#21972, and enables the functionality added to the js-sdk in matrix-org/matrix-js-sdk#2969.
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Jan 19, 2023
* Remove extensible events v1 field population on legacy events ([\matrix-org#3040](matrix-org#3040)).
* Improve hasUserReadEvent and getUserReadUpTo realibility with threads ([\matrix-org#3031](matrix-org#3031)). Fixes element-hq/element-web#24164.
* Remove video track when muting video ([\matrix-org#3028](matrix-org#3028)). Fixes element-hq/element-call#209.
* Make poll start event type available (PSG-962) ([\matrix-org#3034](matrix-org#3034)).
* Add alt event type matching in Relations model ([\matrix-org#3018](matrix-org#3018)).
* Remove usage of v1 Identity Server API ([\matrix-org#3003](matrix-org#3003)).
* Add `device_id` to `/account/whoami` types ([\matrix-org#3005](matrix-org#3005)).
* Implement MSC3912: Relation-based redactions ([\matrix-org#2954](matrix-org#2954)).
* Introduce a mechanism for using the rust-crypto-sdk ([\matrix-org#2969](matrix-org#2969)).
* Support MSC3391: Account data deletion ([\matrix-org#2967](matrix-org#2967)).
* Fix threaded cache receipt when event holds multiple receipts ([\matrix-org#3026](matrix-org#3026)).
* Fix false key requests after verifying new device ([\matrix-org#3029](matrix-org#3029)). Fixes element-hq/element-web#24167 and element-hq/element-web#23333.
* Avoid triggering decryption errors when decrypting redacted events ([\matrix-org#3004](matrix-org#3004)). Fixes element-hq/element-web#24084.
* bugfix: upload OTKs in sliding sync mode ([\matrix-org#3008](matrix-org#3008)).
* Apply edits discovered from sync after thread is initialised ([\matrix-org#3002](matrix-org#3002)). Fixes element-hq/element-web#23921.
* Sliding sync: Fix issue where no unsubs are sent when switching rooms ([\matrix-org#2991](matrix-org#2991)).
* Threads are missing from the timeline ([\matrix-org#2996](matrix-org#2996)). Fixes element-hq/element-web#24036.
* Close all streams when a call ends ([\matrix-org#2992](matrix-org#2992)). Fixes element-hq/element-call#742.
* Resume to-device message queue after resumed sync ([\matrix-org#2920](matrix-org#2920)). Fixes matrix-org/element-web-rageshakes#17170.
* Fix browser entrypoint ([\matrix-org#3051](matrix-org#3051)). Fixes matrix-org#3013.
* Fix failure to start in firefox private browser ([\matrix-org#3058](matrix-org#3058)). Fixes element-hq/element-web#24216.
* Correctly handle limited sync responses by resetting the thread timeline ([\matrix-org#3056](matrix-org#3056)). Fixes element-hq/element-web#23952.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants