add unknown to EncryptionAlgorithm enum #92
Conversation
| @@ -3,6 +3,10 @@ | |||
| - `PickledInboundGroupSession.sender_signing_key` is now optional. | |||
| ([#89](https://github.com/matrix-org/matrix-rust-sdk-crypto-wasm/pull/89)) | |||
|
|
|||
| - Add `Unknown` to `EncryptionAlgorithm`, representing unsupported algorithms | |||
There was a problem hiding this comment.
Does this count as a breaking change?
There was a problem hiding this comment.
Kinda. The js-sdk code to handle this is going to be incorrect, for instance, since Unknown != Olm.
rustAlgorithms.forEach((algorithm) => {
switch (algorithm) {
case RustSdkCryptoJs.EncryptionAlgorithm.MegolmV1AesSha2:
algorithms.add("m.megolm.v1.aes-sha2");
break;
case RustSdkCryptoJs.EncryptionAlgorithm.OlmV1Curve25519AesSha2:
default:
algorithms.add("m.olm.v1.curve25519-aes-sha2");
break;
}
});... But I'm not really sure it's worth a major version bump. I'd treat it as a normal change.
Please put it at the top of the changelog though (most recent changes in the changelog are first, obviously), and resolve the conflict.
| @@ -3,6 +3,10 @@ | |||
| - `PickledInboundGroupSession.sender_signing_key` is now optional. | |||
| ([#89](https://github.com/matrix-org/matrix-rust-sdk-crypto-wasm/pull/89)) | |||
|
|
|||
| - Add `Unknown` to `EncryptionAlgorithm`, representing unsupported algorithms | |||
There was a problem hiding this comment.
Kinda. The js-sdk code to handle this is going to be incorrect, for instance, since Unknown != Olm.
rustAlgorithms.forEach((algorithm) => {
switch (algorithm) {
case RustSdkCryptoJs.EncryptionAlgorithm.MegolmV1AesSha2:
algorithms.add("m.megolm.v1.aes-sha2");
break;
case RustSdkCryptoJs.EncryptionAlgorithm.OlmV1Curve25519AesSha2:
default:
algorithms.add("m.olm.v1.curve25519-aes-sha2");
break;
}
});... But I'm not really sure it's worth a major version bump. I'd treat it as a normal change.
Please put it at the top of the changelog though (most recent changes in the changelog are first, obviously), and resolve the conflict.
|
no, it doesn't. Not until the js-sdk is updated to use this change. |
Co-authored-by: Richard van der Hoff <[email protected]>
|
I linked to the wrong issue in the PR description. Fixed now. |
Ok, but please avoid the magic github words "will fix ", which means that GH will auto-close that issue when the PR is merged, irrespective of the fact we have many more steps to do before that happens. Now fixed. |
Oh, I changed "fixes" to "will fix" because I thought that would be safe. :-/ |
Related: element-hq/element-web#26610, which will be fixed by this once matrix-js-sdk is updated