Page MenuHomePhabricator

[crypto] implement getting one-time keys in `OlmAPI`
ClosedPublic

Authored by kamil on Feb 20 2024, 5:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jun 25, 6:50 PM
Unknown Object (File)
Tue, Jun 25, 4:14 AM
Unknown Object (File)
Tue, Jun 25, 3:51 AM
Unknown Object (File)
Sat, Jun 22, 4:43 AM
Unknown Object (File)
Sun, Jun 16, 2:38 AM
Unknown Object (File)
Tue, Jun 11, 1:50 PM
Unknown Object (File)
Sat, Jun 1, 5:39 PM
Unknown Object (File)
May 20 2024, 11:11 AM
Subscribers

Details

Summary

Shared code to get one-time keys on the web and native.

According to this comment marking keys as published right before calling Identity.

The web part should match the native C++ code implemented in D10977 (despite one change - see inline comment below).

This code should return one-time keys in a short format - calling Identity in the next diff in the stack.

Depends on D11112

Test Plan

Call this code on both web and native:

const { olmAPI } = getConfig();
 await olmAPI.initializeCryptoAccount();
 const keys = await olmAPI.getOneTimeKeys(10);
 console.log(keys);

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil edited the test plan for this revision. (Show Details)
lib/utils/olm-utils.js
84 ↗(On Diff #37354)

@varun there is a discrepancy between code here (I just extracted it to a function) and code you implemented on native (D10977).

Looks like the native's C++ version is correct but I am asking before updating this.

web/crypto/olm-api.js
17–21 ↗(On Diff #37354)

this should be included in D11111 - mistake while creating diffs

kamil published this revision for review.Feb 20 2024, 6:13 AM
lib/utils/olm-utils.js
84 ↗(On Diff #37354)

Yeah, we want to make it work like native I think. I think that is actually tracked in ENG-6688

tomek added inline comments.
lib/utils/olm-utils.js
78–81 ↗(On Diff #37354)

This function doesn't return a Set

native/crypto/olm-api.js
33–34 ↗(On Diff #37354)

These should be readonly. Is this the same as OneTimeKeysResultValues?

varun added inline comments.
lib/utils/olm-utils.js
84 ↗(On Diff #37354)

right, the native version is correct. are you planning to make the change in this diff?

This revision is now accepted and ready to land.Feb 21 2024, 8:14 PM

address review

lib/utils/olm-utils.js
84 ↗(On Diff #37354)

yes, done

native/crypto/olm-api.js
33–34 ↗(On Diff #37354)

yes, it should be OneTimeKeysResultValues

This revision was landed with ongoing or failed builds.Feb 26 2024, 5:46 AM
This revision was automatically updated to reflect the committed changes.