Page MenuHomePhabricator
Differential D11301

[native] Broadcast device list update and send backup key stub
ClosedPublic
Actions

Authored by bartek on Mar 11 2024, 6:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 27 2025, 10:08 PM
Unknown Object (File)
Feb 27 2025, 7:26 PM
Unknown Object (File)
Feb 27 2025, 7:26 PM
Unknown Object (File)
Feb 27 2025, 7:26 PM
Unknown Object (File)
Feb 27 2025, 7:26 PM
Unknown Object (File)
Feb 27 2025, 7:26 PM
Unknown Object (File)
Feb 27 2025, 7:26 PM
Unknown Object (File)
Feb 27 2025, 3:19 AM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
kamil
michal
Commits
rCOMM6bcc2eac04e6: [native] Broadcast device list update and send backup key stub
Summary

Primary device should broadcast the device list update to all peers. However, we don't have peer syncing yet, so as a stub, it only broadcasts it to all devices from its device list.
Also, stubbed message containing backup data key, which will be needed for backup restore on secondary device.

Depends on D11296

Test Plan

Added a few devices to the device list - made sure that after successful QR auth flow, all devices receive the device list update. Also, secondary device displays a console log with stub backup key.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Mar 11 2024, 6:49 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 11 2024, 6:49 AM
Harbormaster failed remote builds in B27440: Diff 37988!Mar 11 2024, 7:06 AM
bartek edited the summary of this revision. (Show Details)Mar 11 2024, 7:15 AM
bartek edited the test plan for this revision. (Show Details)
bartek updated this revision to Diff 37994.Mar 11 2024, 7:20 AM

Rebase

Harbormaster failed remote builds in B27446: Diff 37994!Mar 11 2024, 7:35 AM
bartek updated this revision to Diff 38012.Mar 12 2024, 12:10 AM

Rebase

Harbormaster failed remote builds in B27463: Diff 38012!Mar 12 2024, 12:21 AM
bartek updated this revision to Diff 38016.Mar 12 2024, 1:49 AM

Commit missing changes

Harbormaster completed remote builds in B27467: Diff 38016.Mar 12 2024, 2:06 AM
bartek published this revision for review.Mar 12 2024, 2:20 AM
ashoat added inline comments.Mar 12 2024, 3:34 PM
native/profile/secondary-device-qr-code-scanner.react.js
62 ↗(On Diff #38016)

Should we add an invariant here that deviceLists.length > 0?

native/qr-code/qr-code-screen.react.js
103 ↗(On Diff #38016)

Is this console.log left over from debugging?

kamil requested changes to this revision.Mar 13 2024, 6:03 AM
kamil added inline comments.
lib/types/tunnelbroker/peer-to-peer-message-types.js
26 ↗(On Diff #38016)

We should discuss whether this update should be sent encrypted via olm and create a task to add it in the future, then this also should be sent as subtype of ENCRYPTED_MESSAGE

This revision now requires changes to proceed.Mar 13 2024, 6:03 AM
bartek added a child revision: D11330: [lib][native] Extract QR code to a spearate component.Mar 14 2024, 10:37 AM
bartek mentioned this in D11331: [web] Add QR auth handler logic.Mar 14 2024, 10:39 AM
bartek added inline comments.Mar 18 2024, 5:21 AM
lib/types/tunnelbroker/peer-to-peer-message-types.js
26 ↗(On Diff #38016)

Yes, good point, I was thinking about this too.
I'm not sure if it's possible at this moment, so I created https://linear.app/comm/issue/ENG-7245/broadcast-device-list-updates-via-olm-sessions to address this

native/qr-code/qr-code-screen.react.js
103 ↗(On Diff #38016)

It is replaced by actual backup restore call later in the stack

bartek updated this revision to Diff 38128.Mar 18 2024, 5:22 AM

Add invariant

Harbormaster completed remote builds in B27557: Diff 38128.Mar 18 2024, 5:40 AM
kamil accepted this revision.Mar 18 2024, 6:42 AM
This revision is now accepted and ready to land.Mar 18 2024, 6:42 AM
Closed by commit rCOMM6bcc2eac04e6: [native] Broadcast device list update and send backup key stub (authored by bartek). · Explain WhyMar 21 2024, 4:29 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM6bcc2eac04e6: [native] Broadcast device list update and send backup key stub.
bartek mentioned this in rCOMM725663b26bf7: [web] Add QR auth handler logic.

Revision Contents
Changeset List

PathSize
native/
profile/
43 lines
qr-code/
9 lines

Diff 38012

View Options

native/profile/secondary-device-qr-code-scanner.react.js

// @flow // @flow
import { addCommunityActionType } from '../actions/community-actions.js'; import { addCommunityActionType } from '../actions/community-actions.js';
import { siweAuthActionTypes } from '../actions/siwe-actions.js';
import {
logInActionTypes,
keyserverRegisterActionTypes,
} from '../actions/user-actions.js';
import { import {
communityStoreOpsHandlers, communityStoreOpsHandlers,
type CommunityStoreOperation, type CommunityStoreOperation,
type ReplaceCommunityOperation, type ReplaceCommunityOperation,
type RemoveAllCommunitiesOperation,
} from '../ops/community-store-ops.js'; } from '../ops/community-store-ops.js';
import type { CommunityStore } from '../types/community-types.js'; import type {
CommunityStore,
CommunityInfo,
} from '../types/community-types.js';
import { defaultEnabledApps } from '../types/enabled-apps.js';
import type { BaseAction } from '../types/redux-types'; import type { BaseAction } from '../types/redux-types';
import type { RawThreadInfos } from '../types/thread-types';
import { threadTypeIsCommunityRoot } from '../types/thread-types-enum.js';
const { processStoreOperations: processStoreOps } = communityStoreOpsHandlers; const { processStoreOperations: processStoreOps } = communityStoreOpsHandlers;
export default function reduceCommunityStore( export default function reduceCommunityStore(
state: CommunityStore, state: CommunityStore,
action: BaseAction, action: BaseAction,
newThreadInfos: RawThreadInfos,
): { ): {
communityStore: CommunityStore, communityStore: CommunityStore,
communityStoreOperations: $ReadOnlyArray<CommunityStoreOperation>, communityStoreOperations: $ReadOnlyArray<CommunityStoreOperation>,
} { } {
if (action.type === addCommunityActionType) { if (
action.type === logInActionTypes.success ||
action.type === siweAuthActionTypes.success ||
action.type === keyserverRegisterActionTypes.success
) {
kamilUnsubmitted
Not Done
Inline Actions

wondering, shouldn't we also add action.type === fullStateSyncActionType?

kamil: wondering, shouldn't we also add `action.type === fullStateSyncActionType`?
ginsuAuthorUnsubmitted
Done
Inline Actions

I spoke to @atul about this and from what I understood we don't need this type since the community store is not synced with the keyserver

ginsu: I spoke to @atul about this and from what I understood we don't need this type since the…
kamilUnsubmitted
Not Done
Inline Actions

But based on the code below, communities are computed based on newThreadInfos which are synced - see here (cc. @atul)

I am afraid that on fullStateSyncActionType we will have inconsistency between threads (threadTypeIsCommunityRoot) and communities in this store

kamil: But based on the code below, communities are computed based on `newThreadInfos` which are…
const removeAllOpertation: RemoveAllCommunitiesOperation = {
type: 'remove_all_communities',
};
const replaceOperations = Object.keys(newThreadInfos)
.filter(threadID =>
threadTypeIsCommunityRoot(newThreadInfos[threadID].type),
)
.map(threadID => {
const communityInfo: CommunityInfo = {
enabledApps: defaultEnabledApps,
};
return {
type: 'replace_community',
payload: {
id: threadID,
communityInfo,
},
};
});
atulUnsubmitted
Not Done
Inline Actions

You could do something like following:

const replaceOperations = Object.keys(newThreadInfos).reduce((acc, threadID) => {
  if (threadTypeIsCommunityRoot(newThreadInfos[threadID].type)) {
    const communityInfo = {
      enabledApps: defaultEnabledApps,
    };
    acc.push({
      type: 'replace_community',
      payload: {
        id: threadID,
        communityInfo,
      },
    });
  }
  return acc;
}, []);

(generated with ChatGPT)

that lets us avoid an iteration, but probably doesn't make a difference

atul: You could do something like following: ```lang=js const replaceOperations = Object.keys…
const commmunityStoreOperations = [
removeAllOpertation,
...replaceOperations,
];
return {
communityStore: processStoreOps(state, commmunityStoreOperations),
communityStoreOperations: commmunityStoreOperations,
};
} else if (action.type === addCommunityActionType) {
const replaceOperation: ReplaceCommunityOperation = { const replaceOperation: ReplaceCommunityOperation = {
type: 'replace_community', type: 'replace_community',
payload: { payload: {
id: action.payload.id, id: action.payload.id,
communityInfo: { communityInfo: {
...action.payload.newCommunityInfo, ...action.payload.newCommunityInfo,
}, },
}, },
Show All 13 Lines
View Options

native/qr-code/qr-code-screen.react.js

// @flow // @flow
import { addCommunityActionType } from '../actions/community-actions.js'; import { addCommunityActionType } from '../actions/community-actions.js';
import { siweAuthActionTypes } from '../actions/siwe-actions.js';
import {
logInActionTypes,
keyserverRegisterActionTypes,
} from '../actions/user-actions.js';
import { import {
communityStoreOpsHandlers, communityStoreOpsHandlers,
type CommunityStoreOperation, type CommunityStoreOperation,
type ReplaceCommunityOperation, type ReplaceCommunityOperation,
type RemoveAllCommunitiesOperation,
} from '../ops/community-store-ops.js'; } from '../ops/community-store-ops.js';
import type { CommunityStore } from '../types/community-types.js'; import type {
CommunityStore,
CommunityInfo,
} from '../types/community-types.js';
import { defaultEnabledApps } from '../types/enabled-apps.js';
import type { BaseAction } from '../types/redux-types'; import type { BaseAction } from '../types/redux-types';
import type { RawThreadInfos } from '../types/thread-types';
import { threadTypeIsCommunityRoot } from '../types/thread-types-enum.js';
const { processStoreOperations: processStoreOps } = communityStoreOpsHandlers; const { processStoreOperations: processStoreOps } = communityStoreOpsHandlers;
export default function reduceCommunityStore( export default function reduceCommunityStore(
state: CommunityStore, state: CommunityStore,
action: BaseAction, action: BaseAction,
newThreadInfos: RawThreadInfos,
): { ): {
communityStore: CommunityStore, communityStore: CommunityStore,
communityStoreOperations: $ReadOnlyArray<CommunityStoreOperation>, communityStoreOperations: $ReadOnlyArray<CommunityStoreOperation>,
} { } {
if (action.type === addCommunityActionType) { if (
action.type === logInActionTypes.success ||
action.type === siweAuthActionTypes.success ||
action.type === keyserverRegisterActionTypes.success
) {
kamilUnsubmitted
Not Done
Inline Actions

wondering, shouldn't we also add action.type === fullStateSyncActionType?

kamil: wondering, shouldn't we also add `action.type === fullStateSyncActionType`?
ginsuAuthorUnsubmitted
Done
Inline Actions

I spoke to @atul about this and from what I understood we don't need this type since the community store is not synced with the keyserver

ginsu: I spoke to @atul about this and from what I understood we don't need this type since the…
kamilUnsubmitted
Not Done
Inline Actions

But based on the code below, communities are computed based on newThreadInfos which are synced - see here (cc. @atul)

I am afraid that on fullStateSyncActionType we will have inconsistency between threads (threadTypeIsCommunityRoot) and communities in this store

kamil: But based on the code below, communities are computed based on `newThreadInfos` which are…
const removeAllOpertation: RemoveAllCommunitiesOperation = {
type: 'remove_all_communities',
};
const replaceOperations = Object.keys(newThreadInfos)
.filter(threadID =>
threadTypeIsCommunityRoot(newThreadInfos[threadID].type),
)
.map(threadID => {
const communityInfo: CommunityInfo = {
enabledApps: defaultEnabledApps,
};
return {
type: 'replace_community',
payload: {
id: threadID,
communityInfo,
},
};
});
atulUnsubmitted
Not Done
Inline Actions

You could do something like following:

const replaceOperations = Object.keys(newThreadInfos).reduce((acc, threadID) => {
  if (threadTypeIsCommunityRoot(newThreadInfos[threadID].type)) {
    const communityInfo = {
      enabledApps: defaultEnabledApps,
    };
    acc.push({
      type: 'replace_community',
      payload: {
        id: threadID,
        communityInfo,
      },
    });
  }
  return acc;
}, []);

(generated with ChatGPT)

that lets us avoid an iteration, but probably doesn't make a difference

atul: You could do something like following: ```lang=js const replaceOperations = Object.keys…
const commmunityStoreOperations = [
removeAllOpertation,
...replaceOperations,
];
return {
communityStore: processStoreOps(state, commmunityStoreOperations),
communityStoreOperations: commmunityStoreOperations,
};
} else if (action.type === addCommunityActionType) {
const replaceOperation: ReplaceCommunityOperation = { const replaceOperation: ReplaceCommunityOperation = {
type: 'replace_community', type: 'replace_community',
payload: { payload: {
id: action.payload.id, id: action.payload.id,
communityInfo: { communityInfo: {
...action.payload.newCommunityInfo, ...action.payload.newCommunityInfo,
}, },
}, },
Show All 13 Lines