From b10a804a03ae804aa1f29652bddee74038ec0c91 Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 22 Jan 2024 16:17:53 +0100 Subject: [PATCH] Element R: Bump matrix-rust-sdk-crypto-wasm to version 4.0.0 (#4021) * bump wasm bindings version 4.0.0 * fix test compilation with initFromStore * Fix test due to change in wasm handling of Vec<> * review: Better doc Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * review: Better doc Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * review: revert userIdentity free removal --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- package.json | 2 +- spec/unit/rust-crypto/KeyClaimManager.spec.ts | 14 +++++++++++--- spec/unit/rust-crypto/rust-crypto.spec.ts | 12 ++++++------ src/rust-crypto/KeyClaimManager.ts | 5 ++++- src/rust-crypto/RoomEncryptor.ts | 1 + src/rust-crypto/index.ts | 2 +- src/rust-crypto/rust-crypto.ts | 5 ++++- yarn.lock | 8 ++++---- 8 files changed, 32 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index a64abb5db..accf1fcfa 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ ], "dependencies": { "@babel/runtime": "^7.12.5", - "@matrix-org/matrix-sdk-crypto-wasm": "^3.6.0", + "@matrix-org/matrix-sdk-crypto-wasm": "^4.0.0", "another-json": "^0.2.0", "bs58": "^5.0.0", "content-type": "^1.0.4", diff --git a/spec/unit/rust-crypto/KeyClaimManager.spec.ts b/spec/unit/rust-crypto/KeyClaimManager.spec.ts index 5a33c508f..f2ad04cac 100644 --- a/spec/unit/rust-crypto/KeyClaimManager.spec.ts +++ b/spec/unit/rust-crypto/KeyClaimManager.spec.ts @@ -97,7 +97,10 @@ describe("KeyClaimManager", () => { await keyClaimManager.ensureSessionsForUsers(new LogSpan(logger, "test"), [u1, u2]); // check that all the calls were made - expect(olmMachine.getMissingSessions).toHaveBeenCalledWith([u1, u2]); + // We can't use directly toHaveBeenCalledWith because the UserId are cloned in the process. + const calledWith = olmMachine.getMissingSessions.mock.calls[0][0].map((u) => u.toString()); + expect(calledWith).toEqual([u1.toString(), u2.toString()]); + expect(fetchMock).toHaveFetched("https://example.com/_matrix/client/v3/keys/claim", { method: "POST", body: { k1: "v1" }, @@ -135,7 +138,10 @@ describe("KeyClaimManager", () => { // at this point, there should have been a single call to getMissingSessions, and a single fetch; and neither // call to ensureSessionsAsUsers should have completed - expect(olmMachine.getMissingSessions).toHaveBeenCalledWith([u1]); + // check that all the calls were made + // We can't use directly toHaveBeenCalledWith because the UserId are cloned in the process. + const calledWith = olmMachine.getMissingSessions.mock.calls[0][0].map((u) => u.toString()); + expect(calledWith).toEqual([u1.toString()]); expect(olmMachine.getMissingSessions).toHaveBeenCalledTimes(1); expect(fetchMock).toHaveBeenCalledTimes(1); expect(req1Resolved).toBe(false); @@ -147,7 +153,9 @@ describe("KeyClaimManager", () => { resolveMarkRequestAsSentCallback = await markRequestAsSentPromise; // the first request should now have completed, and we should have more calls and fetches - expect(olmMachine.getMissingSessions).toHaveBeenCalledWith([u2]); + // We can't use directly toHaveBeenCalledWith because the UserId are cloned in the process. + const calledWith2 = olmMachine.getMissingSessions.mock.calls[1][0].map((u) => u.toString()); + expect(calledWith2).toEqual([u2.toString()]); expect(olmMachine.getMissingSessions).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenCalledTimes(2); expect(req1Resolved).toBe(true); diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index 6d3ecfd81..dbcf0df88 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -100,7 +100,7 @@ describe("initRustCrypto", () => { jest.spyOn(StoreHandle, "open").mockResolvedValue(mockStore); const testOlmMachine = makeTestOlmMachine(); - jest.spyOn(OlmMachine, "init_from_store").mockResolvedValue(testOlmMachine); + jest.spyOn(OlmMachine, "initFromStore").mockResolvedValue(testOlmMachine); await initRustCrypto({ logger, @@ -114,7 +114,7 @@ describe("initRustCrypto", () => { }); expect(StoreHandle.open).toHaveBeenCalledWith("storePrefix", "storePassphrase"); - expect(OlmMachine.init_from_store).toHaveBeenCalledWith(expect.anything(), expect.anything(), mockStore); + expect(OlmMachine.initFromStore).toHaveBeenCalledWith(expect.anything(), expect.anything(), mockStore); }); it("suppresses the storePassphrase if storePrefix is unset", async () => { @@ -122,7 +122,7 @@ describe("initRustCrypto", () => { jest.spyOn(StoreHandle, "open").mockResolvedValue(mockStore); const testOlmMachine = makeTestOlmMachine(); - jest.spyOn(OlmMachine, "init_from_store").mockResolvedValue(testOlmMachine); + jest.spyOn(OlmMachine, "initFromStore").mockResolvedValue(testOlmMachine); await initRustCrypto({ logger, @@ -136,7 +136,7 @@ describe("initRustCrypto", () => { }); expect(StoreHandle.open).toHaveBeenCalledWith(undefined, undefined); - expect(OlmMachine.init_from_store).toHaveBeenCalledWith(expect.anything(), expect.anything(), mockStore); + expect(OlmMachine.initFromStore).toHaveBeenCalledWith(expect.anything(), expect.anything(), mockStore); }); it("Should get secrets from inbox on start", async () => { @@ -144,7 +144,7 @@ describe("initRustCrypto", () => { jest.spyOn(StoreHandle, "open").mockResolvedValue(mockStore); const testOlmMachine = makeTestOlmMachine(); - jest.spyOn(OlmMachine, "init_from_store").mockResolvedValue(testOlmMachine); + jest.spyOn(OlmMachine, "initFromStore").mockResolvedValue(testOlmMachine); await initRustCrypto({ logger, @@ -189,7 +189,7 @@ describe("initRustCrypto", () => { jest.spyOn(Migration, "migrateMegolmSessions").mockResolvedValue(undefined); const testOlmMachine = makeTestOlmMachine(); - jest.spyOn(OlmMachine, "init_from_store").mockResolvedValue(testOlmMachine); + jest.spyOn(OlmMachine, "initFromStore").mockResolvedValue(testOlmMachine); fetchMock.get("path:/_matrix/client/v3/room_keys/version", { version: "45" }); diff --git a/src/rust-crypto/KeyClaimManager.ts b/src/rust-crypto/KeyClaimManager.ts index 1500d8398..aaeed4d14 100644 --- a/src/rust-crypto/KeyClaimManager.ts +++ b/src/rust-crypto/KeyClaimManager.ts @@ -73,7 +73,10 @@ export class KeyClaimManager { throw new Error(`Cannot ensure Olm sessions: shutting down`); } logger.info("Checking for missing Olm sessions"); - const claimRequest = await this.olmMachine.getMissingSessions(userList); + // By passing the userId array to rust we transfer ownership of the items to rust, causing + // them to be invalidated on the JS side as soon as the method is called. + // As we haven't created the `userList` let's clone the users, to not break the caller from re-using it. + const claimRequest = await this.olmMachine.getMissingSessions(userList.map((u) => u.clone())); if (claimRequest) { logger.info("Making /keys/claim request"); await this.outgoingRequestProcessor.makeOutgoingRequest(claimRequest); diff --git a/src/rust-crypto/RoomEncryptor.ts b/src/rust-crypto/RoomEncryptor.ts index ef3ea777f..199db95ca 100644 --- a/src/rust-crypto/RoomEncryptor.ts +++ b/src/rust-crypto/RoomEncryptor.ts @@ -258,6 +258,7 @@ export class RoomEncryptor { await logDuration(this.prefixedLogger, "shareRoomKey", async () => { const shareMessages: ToDeviceRequest[] = await this.olmMachine.shareRoomKey( new RoomId(this.room.roomId), + // safe to pass without cloning, as it's not reused here (before or after) userList, rustEncryptionSettings, ); diff --git a/src/rust-crypto/index.ts b/src/rust-crypto/index.ts index 9108761b7..63c65317b 100644 --- a/src/rust-crypto/index.ts +++ b/src/rust-crypto/index.ts @@ -131,7 +131,7 @@ async function initOlmMachine( ): Promise { logger.debug("Init OlmMachine"); - const olmMachine = await RustSdkCryptoJs.OlmMachine.init_from_store( + const olmMachine = await RustSdkCryptoJs.OlmMachine.initFromStore( new RustSdkCryptoJs.UserId(userId), new RustSdkCryptoJs.DeviceId(deviceId), storeHandle, diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index d63b7d02c..239fae8a8 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -359,7 +359,10 @@ export class RustCrypto extends TypedEventEmitter