From 760ce5caa551584b84301175feba6cd91c93828e Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Tue, 9 May 2023 13:37:31 +1000 Subject: [PATCH] fix: add the maxSizeMap to have priority per retrieve namespaces --- ts/components/menu/Menu.tsx | 2 +- ts/receiver/closedGroups.ts | 7 +-- ts/receiver/configMessage.ts | 1 + .../opengroupV2/OpenGroupManagerV2.ts | 1 + ts/session/apis/snode_api/namespaces.ts | 57 ++++++++++++++++++- ts/session/apis/snode_api/retrieveRequest.ts | 3 + ts/session/apis/snode_api/swarmPolling.ts | 2 +- .../conversations/ConversationController.ts | 49 ++++++++++------ ts/session/conversations/createClosedGroup.ts | 18 +----- ts/session/group/closed-group.ts | 5 +- .../job_runners/jobs/ConfigurationSyncJob.ts | 2 +- .../libsession_utils_user_groups.ts | 8 ++- .../session/unit/onion/SnodeNamespace_test.ts | 30 ++++++++++ 13 files changed, 137 insertions(+), 48 deletions(-) create mode 100644 ts/test/session/unit/onion/SnodeNamespace_test.ts diff --git a/ts/components/menu/Menu.tsx b/ts/components/menu/Menu.tsx index 813047bd9..d22ca79c6 100644 --- a/ts/components/menu/Menu.tsx +++ b/ts/components/menu/Menu.tsx @@ -324,7 +324,7 @@ export const CopyMenuItem = (): JSX.Element | null => { return ( { - copyPublicKeyByConvoId(convoId); + void copyPublicKeyByConvoId(convoId); }} > {copyIdLabel} diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 981f7553a..a2474de8d 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -289,12 +289,7 @@ export async function handleNewClosedGroup( // ***** Creating a new group ***** window?.log?.info('Received a new ClosedGroup of id:', groupId); - await ClosedGroup.addUpdateMessage( - convo, - { newName: name, joiningMembers: members }, - envelope.senderIdentity || envelope.source, // new group message are coming as session messages - envelopeTimestamp - ); + // we don't want the initial "AAA,BBB and You joined the group" // We only set group admins on group creation const groupDetails: ClosedGroup.GroupInfo = { diff --git a/ts/receiver/configMessage.ts b/ts/receiver/configMessage.ts index 84be29005..b930acaca 100644 --- a/ts/receiver/configMessage.ts +++ b/ts/receiver/configMessage.ts @@ -424,6 +424,7 @@ async function handleLegacyGroupUpdate(latestEnvelopeTimestamp: number) { } if (changes) { + // this commit will grab the latest encryption keypair and add it to the user group wrapper if needed await legacyGroupConvo.commit(); } diff --git a/ts/session/apis/open_group_api/opengroupV2/OpenGroupManagerV2.ts b/ts/session/apis/open_group_api/opengroupV2/OpenGroupManagerV2.ts index 1bab0b823..a6ae4ac66 100644 --- a/ts/session/apis/open_group_api/opengroupV2/OpenGroupManagerV2.ts +++ b/ts/session/apis/open_group_api/opengroupV2/OpenGroupManagerV2.ts @@ -209,6 +209,7 @@ export class OpenGroupManagerV2 { // will need it and access it from the db await OpenGroupData.saveV2OpenGroupRoom(room); + // TODOLATER make the requests made here retry a few times (can fail when trying to join a group too soon after a restart) const roomInfos = await openGroupV2GetRoomInfoViaOnionV4({ serverPubkey: serverPublicKey, serverUrl, diff --git a/ts/session/apis/snode_api/namespaces.ts b/ts/session/apis/snode_api/namespaces.ts index 7bfbb7ffa..a1558953d 100644 --- a/ts/session/apis/snode_api/namespaces.ts +++ b/ts/session/apis/snode_api/namespaces.ts @@ -1,3 +1,4 @@ +import { last, orderBy } from 'lodash'; import { assertUnreachable } from '../../../types/sqlSharedTypes'; export enum SnodeNamespaces { @@ -32,7 +33,7 @@ export enum SnodeNamespaces { /** * This is the namespace used to sync the closed group details for each of the closed groups we are polling */ - ClosedGroupInfo = 1, + // ClosedGroupInfo = 1, } type PickEnum = { @@ -41,7 +42,7 @@ type PickEnum = { export type SnodeNamespacesGroup = PickEnum< SnodeNamespaces, - SnodeNamespaces.ClosedGroupInfo | SnodeNamespaces.ClosedGroupMessage + SnodeNamespaces.ClosedGroupMessage // | SnodeNamespaces.ClosedGroupInfo >; export type SnodeNamespacesUser = PickEnum< @@ -62,7 +63,7 @@ function isUserConfigNamespace(namespace: SnodeNamespaces) { case SnodeNamespaces.UserGroups: case SnodeNamespaces.ConvoInfoVolatile: return true; - case SnodeNamespaces.ClosedGroupInfo: + // case SnodeNamespaces.ClosedGroupInfo: case SnodeNamespaces.ClosedGroupMessage: return false; @@ -76,6 +77,56 @@ function isUserConfigNamespace(namespace: SnodeNamespaces) { } } +function namespacePriority(namespace: SnodeNamespaces): number { + switch (namespace) { + case SnodeNamespaces.UserMessages: + return 10; + case SnodeNamespaces.UserContacts: + return 1; + case SnodeNamespaces.UserProfile: + return 1; + case SnodeNamespaces.UserGroups: + return 1; + case SnodeNamespaces.ConvoInfoVolatile: + return 1; + case SnodeNamespaces.ClosedGroupMessage: + return 10; + + default: + try { + assertUnreachable(namespace, `isUserConfigNamespace case not handled: ${namespace}`); + } catch (e) { + window.log.warn(`isUserConfigNamespace case not handled: ${namespace}: ${e.message}`); + return 1; + } + } +} + +function maxSizeMap(namespaces: Array) { + let lastSplit = 1; + const withPriorities = namespaces.map(namespace => { + return { namespace, priority: namespacePriority(namespace) }; + }); + const groupedByPriorities: Array<{ priority: number; namespaces: Array }> = []; + withPriorities.forEach(item => { + if (!groupedByPriorities.find(p => p.priority === item.priority)) { + groupedByPriorities.push({ priority: item.priority, namespaces: [] }); + } + groupedByPriorities.find(p => p.priority === item.priority)?.namespaces.push(item.namespace); + }); + + const sortedDescPriorities = orderBy(groupedByPriorities, ['priority'], ['desc']); + const lowestPriority = last(sortedDescPriorities)?.priority || 1; + const sizeMap = sortedDescPriorities.flatMap(m => { + const paddingForLowerPriority = m.priority === lowestPriority ? 0 : 1; + const splitsForPriority = paddingForLowerPriority + m.namespaces.length; + lastSplit *= splitsForPriority; + return m.namespaces.map(namespace => ({ namespace, maxSize: -lastSplit })); + }); + return sizeMap; +} + export const SnodeNamespace = { isUserConfigNamespace, + maxSizeMap, }; diff --git a/ts/session/apis/snode_api/retrieveRequest.ts b/ts/session/apis/snode_api/retrieveRequest.ts index 37f6454b1..e148ad8b3 100644 --- a/ts/session/apis/snode_api/retrieveRequest.ts +++ b/ts/session/apis/snode_api/retrieveRequest.ts @@ -22,13 +22,16 @@ async function buildRetrieveRequest( ourPubkey: string, configHashesToBump: Array | null ): Promise> { + const maxSizeMap = SnodeNamespace.maxSizeMap(namespaces); const retrieveRequestsParams: Array = await Promise.all( namespaces.map(async (namespace, index) => { + const foundMaxSize = maxSizeMap.find(m => m.namespace === namespace)?.maxSize; const retrieveParam = { pubkey, last_hash: lastHashes.at(index) || '', namespace, timestamp: GetNetworkTime.getNowWithNetworkOffset(), + max_size: foundMaxSize, }; if (namespace === SnodeNamespaces.ClosedGroupMessage) { diff --git a/ts/session/apis/snode_api/swarmPolling.ts b/ts/session/apis/snode_api/swarmPolling.ts index d2035e719..6b497652b 100644 --- a/ts/session/apis/snode_api/swarmPolling.ts +++ b/ts/session/apis/snode_api/swarmPolling.ts @@ -319,7 +319,7 @@ export class SwarmPolling { ) { // that pubkey is not tracked in the wrapper anymore. Just discard those messages and make sure we are not polling // TODOLATER we might need to do something like this for the new closed groups once released - await getSwarmPollingInstance().removePubkey(polledPubkey); + getSwarmPollingInstance().removePubkey(polledPubkey); } else { // trigger the handling of all the other messages, not shared config related newMessages.forEach(m => { diff --git a/ts/session/conversations/ConversationController.ts b/ts/session/conversations/ConversationController.ts index 1d37bb9b2..7849443b4 100644 --- a/ts/session/conversations/ConversationController.ts +++ b/ts/session/conversations/ConversationController.ts @@ -24,6 +24,7 @@ import { getSwarmPollingInstance } from '../apis/snode_api'; import { SnodeNamespaces } from '../apis/snode_api/namespaces'; import { ClosedGroupMemberLeftMessage } from '../messages/outgoing/controlMessage/group/ClosedGroupMemberLeftMessage'; import { UserUtils } from '../utils'; +import { isEmpty } from 'lodash'; let instance: ConversationController | null; @@ -458,30 +459,42 @@ async function leaveClosedGroup(groupId: string, fromSyncMessage: boolean) { getSwarmPollingInstance().removePubkey(groupId); - if (!fromSyncMessage) { - // Send the update to the group - const ourLeavingMessage = new ClosedGroupMemberLeftMessage({ - timestamp: networkTimestamp, - groupId, - identifier: dbMessage.id as string, - }); + if (fromSyncMessage) { + // no need to send our leave message as our other device should already have sent it. + await cleanUpFullyLeftLegacyGroup(groupId); + return; + } - window?.log?.info(`We are leaving the group ${groupId}. Sending our leaving message.`); - // sent the message to the group and once done, remove everything related to this group - const wasSent = await getMessageQueue().sendToPubKeyNonDurably({ - message: ourLeavingMessage, - namespace: SnodeNamespaces.ClosedGroupMessage, - pubkey: PubKey.cast(groupId), - }); + const keypair = await Data.getLatestClosedGroupEncryptionKeyPair(groupId); + if (!keypair || isEmpty(keypair) || isEmpty(keypair.publicHex) || isEmpty(keypair.privateHex)) { + // if we do not have a keypair, we won't be able to send our leaving message neither, so just skip sending it. + // this can happen when getting a group from a broken libsession usergroup wrapper, but not only. + await cleanUpFullyLeftLegacyGroup(groupId); + return; + } + + // Send the update to the group + const ourLeavingMessage = new ClosedGroupMemberLeftMessage({ + timestamp: networkTimestamp, + groupId, + identifier: dbMessage.id as string, + }); + + window?.log?.info(`We are leaving the group ${groupId}. Sending our leaving message.`); + + // if we do not have a keypair for that group, we can't send our leave message, so just skip the message sending part + const wasSent = await getMessageQueue().sendToPubKeyNonDurably({ + message: ourLeavingMessage, + namespace: SnodeNamespaces.ClosedGroupMessage, + pubkey: PubKey.cast(groupId), + }); + if (wasSent) { window?.log?.info( `Leaving message sent ${groupId}. Removing everything related to this group.` ); - if (wasSent) { - await cleanUpFullyLeftLegacyGroup(groupId); - } - } else { await cleanUpFullyLeftLegacyGroup(groupId); } + // if we failed to send our leaving message, don't remove everything yet as we might want to retry sending our leaving message later. } async function cleanUpFullyLeftLegacyGroup(groupId: string) { diff --git a/ts/session/conversations/createClosedGroup.ts b/ts/session/conversations/createClosedGroup.ts index e2fe7f308..24d5ae051 100644 --- a/ts/session/conversations/createClosedGroup.ts +++ b/ts/session/conversations/createClosedGroup.ts @@ -1,7 +1,6 @@ import _ from 'lodash'; import { ClosedGroup, getMessageQueue } from '..'; import { ConversationTypeEnum } from '../../models/conversationAttributes'; -import { MessageModel } from '../../models/message'; import { addKeyPairToCacheAndDBIfNeeded } from '../../receiver/closedGroups'; import { ECKeyPair } from '../../receiver/keypairs'; import { openConversationWithMessages } from '../../state/ducks/conversations'; @@ -64,13 +63,8 @@ export async function createClosedGroup(groupName: string, members: Array, encryptionKeyPair: ECKeyPair, - dbMessage: MessageModel, existingExpireTimer: number, isRetry: boolean = false ): Promise { @@ -128,7 +120,6 @@ async function sendToGroupMembers( groupName, admins, encryptionKeyPair, - dbMessage, existingExpireTimer ); window?.log?.info(`Sending invites for group ${groupPublicKey} to ${listOfMembers}`); @@ -184,7 +175,6 @@ async function sendToGroupMembers( groupName, admins, encryptionKeyPair, - dbMessage, existingExpireTimer, isRetrySend ); @@ -202,7 +192,6 @@ function createInvitePromises( groupName: string, admins: Array, encryptionKeyPair: ECKeyPair, - dbMessage: MessageModel, existingExpireTimer: number ) { return listOfMembers.map(async m => { @@ -213,7 +202,6 @@ function createInvitePromises( admins, keypair: encryptionKeyPair, timestamp: Date.now(), - identifier: dbMessage.id, expireTimer: existingExpireTimer, }; const message = new ClosedGroupNewMessage(messageParams); diff --git a/ts/session/group/closed-group.ts b/ts/session/group/closed-group.ts index 06559baec..17a289044 100644 --- a/ts/session/group/closed-group.ts +++ b/ts/session/group/closed-group.ts @@ -26,6 +26,7 @@ import { ClosedGroupNewMessage } from '../messages/outgoing/controlMessage/group import { ClosedGroupRemovedMembersMessage } from '../messages/outgoing/controlMessage/group/ClosedGroupRemovedMembersMessage'; import { UserUtils } from '../utils'; import { fromHexToArray, toHex } from '../utils/String'; +import { GetNetworkTime } from '../apis/snode_api/getNetworkTime'; export type GroupInfo = { id: string; @@ -419,7 +420,7 @@ async function generateAndSendNewEncryptionKeyPair( const keypairsMessage = new ClosedGroupEncryptionPairMessage({ groupId: toHex(groupId), - timestamp: Date.now(), + timestamp: GetNetworkTime.getNowWithNetworkOffset(), encryptedKeyPairs: wrappers, }); @@ -433,7 +434,9 @@ async function generateAndSendNewEncryptionKeyPair( distributingClosedGroupEncryptionKeyPairs.delete(toHex(groupId)); await addKeyPairToCacheAndDBIfNeeded(toHex(groupId), newKeyPair.toHexKeyPair()); + await groupConvo?.commit(); // this makes sure to include the new encryption keypair in the libsession usergroup wrapper }; + // this is to be sent to the group pubkey adress await getMessageQueue().sendToGroup({ message: keypairsMessage, diff --git a/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts b/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts index efd61d9ea..d2a64a26c 100644 --- a/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts +++ b/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts @@ -296,7 +296,7 @@ async function queueNewJobIfNeeded() { // if we did run at t=100, and it is currently t=110, the difference is 10 const diff = Math.max(Date.now() - lastRunConfigSyncJobTimestamp, 0); // but we want to run every 30, so what we need is actually `30-10` from now = 20 - const leftBeforeNextTick = Math.max(defaultMsBetweenRetries - diff, 0); + const leftBeforeNextTick = Math.max(defaultMsBetweenRetries - diff, 1000); window.log.debug('Scheduling ConfSyncJob: LATER'); await runners.configurationSyncRunner.addJob( diff --git a/ts/session/utils/libsession/libsession_utils_user_groups.ts b/ts/session/utils/libsession/libsession_utils_user_groups.ts index ea7aa3d57..560cea9ec 100644 --- a/ts/session/utils/libsession/libsession_utils_user_groups.ts +++ b/ts/session/utils/libsession/libsession_utils_user_groups.ts @@ -132,14 +132,18 @@ async function insertGroupsFromDBIntoWrapperAndRefresh(convoId: string): Promise window.log.debug(`inserting into usergroup wrapper "${foundConvo.id}"... }`); // this does the create or the update of the matching existing legacy group - if ( !isEmpty(wrapperLegacyGroup.name) && !isEmpty(wrapperLegacyGroup.encPubkey) && !isEmpty(wrapperLegacyGroup.encSeckey) ) { - console.warn('inserting into user wrapper', wrapperLegacyGroup); + window.log.debug('inserting into user wrapper', wrapperLegacyGroup); await UserGroupsWrapperActions.setLegacyGroup(wrapperLegacyGroup); + } else { + window.log.debug( + 'not inserting legacy group as name, or encryption keypair is empty', + foundConvo.id + ); } await refreshCachedUserGroup(convoId); diff --git a/ts/test/session/unit/onion/SnodeNamespace_test.ts b/ts/test/session/unit/onion/SnodeNamespace_test.ts new file mode 100644 index 000000000..b6a88952a --- /dev/null +++ b/ts/test/session/unit/onion/SnodeNamespace_test.ts @@ -0,0 +1,30 @@ +import { expect } from 'chai'; +import Sinon from 'sinon'; +import { SnodeNamespace } from '../../../../session/apis/snode_api/namespaces'; + +// tslint:disable-next-line: max-func-body-length +describe('Snode namespaces', () => { + describe('maxSizeMap', () => { + afterEach(() => { + Sinon.restore(); + }); + + it('single namespace 0 returns -1', () => { + expect(SnodeNamespace.maxSizeMap([0])).to.be.deep.eq([{ namespace: 0, maxSize: -1 }]); + }); + + it('single namespace config 5 returns -1', () => { + expect(SnodeNamespace.maxSizeMap([5])).to.be.deep.eq([{ namespace: 5, maxSize: -1 }]); + }); + + it('multiple namespaces config 0,2,3,4,5 returns [-2,-8,-8,-8,-8]', () => { + expect(SnodeNamespace.maxSizeMap([0, 2, 3, 4, 5])).to.be.deep.eq([ + { namespace: 0, maxSize: -2 }, // 0 has a priority of 10 so takes its own bucket + { namespace: 2, maxSize: -8 }, // the 4 other ones are sharing the next bucket + { namespace: 3, maxSize: -8 }, + { namespace: 4, maxSize: -8 }, + { namespace: 5, maxSize: -8 }, + ]); + }); + }); +});