Details

Reviewers

Commits

rCOMMacb01b121746: [native] create Tunnelbroker session

Summary

Create a diff that gathers the necessary data to establish a proper socket connection and begins to use the context.

Depends on D9609

Test Plan

Use mock credentials (credentials that I manually added to Identity as a valid user) and verify if a session was created.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

kamil held this revision as a draft.

kamil edited the test plan for this revision. (Show Details)Oct 26 2023, 9:16 AM

kamil published this revision for review.Oct 27 2023, 5:38 AM

native/utils/tunnelbroker-utils.js
3 ↗	(On Diff #32453)	We generally prefer `import * as React from 'react`

native/root.react.js
282 ↗	(On Diff #32453)	Is `createTunnelbrokerSocket` the same between native and web? If yes why aren't we using it directly in the `TunnelbrokerProvider`?
native/utils/tunnelbroker-utils.js
15–20 ↗	(On Diff #32453)	Could you explain why there is a difference between native and web when it comes to the init message?

address review

native/root.react.js
282 ↗	(On Diff #32453)	I wanted to follow the pattern from `keyserver <> client` socket when you have a separate function for this which can act differently, but actually, it makes sense to put this code in `TunnelbrokerProvider`
native/utils/tunnelbroker-utils.js
3 ↗	(On Diff #32453)	right, my IDE automatically adds `import React from 'react';` and forgot to change it
15–20 ↗	(On Diff #32453)	On the web `deviceID` is in redux, on native it's in SQLite, this is the only difference (despite `deviceType` which is obvious). This difference leads to different code needed to retrieve `deviceID` and as an effect different ways of creating `ConnectionInitializationMessage`

native/utils/tunnelbroker-utils.js
15–20 ↗	(On Diff #32453)	Are we going to be moving the `deviceID` to SQLite on web? Will we be able to just have one logic for both platforms? If yes than we should probably add a note to the task about this.

native/utils/tunnelbroker-utils.js
15–20 ↗	(On Diff #32453)	I don't think so, it's actually more than just <where to store> `deviceID`. `DeviceID` is strongly related to the `olm`'s account - we have `olm` on native in C++ while on the web this is NPM package, so I think as soon as this looks like this we have differences here. It is better to continue retrieving `deviceID` from JS `olm` version which is the source of truth than moving this to SQLite.