From f6cd12d59998c01fb597a610958d565a004ab453 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 18 Dec 2023 10:17:41 +1100 Subject: [PATCH] fix: use at least 2 hashes for the update_expiries too until the storage server release is live we need this workaround --- ts/models/conversation.ts | 5 ++- ts/models/message.ts | 12 ++++-- .../apis/snode_api/SnodeRequestTypes.ts | 5 ++- ts/session/apis/snode_api/expireRequest.ts | 32 +++++++++++++-- .../apis/snode_api/getExpiriesRequest.ts | 4 +- .../conversations/ConversationController.ts | 9 +---- ts/session/conversations/createClosedGroup.ts | 2 +- ts/session/disappearing_messages/index.ts | 1 + ts/session/group/closed-group.ts | 40 +++++-------------- .../GetExpiriesRequest_test.ts | 7 +++- ts/util/logging.ts | 4 +- 11 files changed, 69 insertions(+), 52 deletions(-) diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index c579faebf..04de875be 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -1009,6 +1009,9 @@ export class ConversationModel extends Backbone.Model { ...expireUpdate, groupId: this.get('id'), }; + // NOTE: we agreed that outgoing ExpirationTimerUpdate **for groups** are not expiring. + expireUpdate.expirationType = 'unknown'; + expireUpdate.expireTimer = 0; const expirationTimerMessage = new ExpirationTimerUpdateMessage(expireUpdateForGroup); @@ -1834,7 +1837,7 @@ export class ConversationModel extends Backbone.Model { identifier: id, timestamp: sentAt, attachments, - expirationType: message.getExpirationType() ?? 'unknown', + expirationType: message.getExpirationType() ?? 'unknown', // Note we assume that the caller used a setting allowed for that conversation when building it. Here we just send it. expireTimer: message.getExpireTimerSeconds(), preview: preview ? [preview] : [], quote, diff --git a/ts/models/message.ts b/ts/models/message.ts index c6f20b00f..8e65eaec8 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -231,6 +231,9 @@ export class MessageModel extends Backbone.Model { public isDataExtractionNotification() { return !!this.get('dataExtractionNotification'); } + public isCallNotification() { + return !!this.get('callNotificationType'); + } public getNotificationText() { let description = this.getDescription(); @@ -488,11 +491,14 @@ export class MessageModel extends Backbone.Model { // some incoming legacy group updates are outgoing, but when synced to our other devices have just the received_at field set. // when that is the case, we don't want to render the spinning 'sending' state - if (this.get('received_at')) { + if ( + (this.isExpirationTimerUpdate() || this.isDataExtractionNotification()) && + this.get('received_at') + ) { return undefined; } - if (this.isDataExtractionNotification() || this.get('callNotificationType')) { + if (this.isDataExtractionNotification() || this.isCallNotification()) { return undefined; } @@ -1339,7 +1345,7 @@ export class MessageModel extends Backbone.Model { getConversationController().getContactProfileNameOrShortenedPubKey(dataExtraction.source), ]); } - if (this.get('callNotificationType')) { + if (this.isCallNotification()) { const displayName = getConversationController().getContactProfileNameOrShortenedPubKey( this.get('conversationId') ); diff --git a/ts/session/apis/snode_api/SnodeRequestTypes.ts b/ts/session/apis/snode_api/SnodeRequestTypes.ts index 9ffcbfd4e..1bed2be22 100644 --- a/ts/session/apis/snode_api/SnodeRequestTypes.ts +++ b/ts/session/apis/snode_api/SnodeRequestTypes.ts @@ -134,7 +134,7 @@ export type DeleteFromNodeSubRequest = { export type UpdateExpireNodeParams = { pubkey: string; pubkey_ed25519: string; - messages: Array; + messages: Array; // Must have at least 2 arguments until the next storage server release (check fakeHash) expiry: number; signature: string; extend?: boolean; @@ -159,6 +159,9 @@ export type GetExpiriesFromNodeSubRequest = { params: GetExpiriesNodeParams; }; +// Until the next storage server release is released, we need to have at least 2 hashes in the list for the `get_expiries` AND for the `update_expiries` +export const fakeHash = '///////////////////////////////////////////'; + export type OxendSubRequest = OnsResolveSubRequest | GetServiceNodesSubRequest; export type SnodeApiSubRequests = diff --git a/ts/session/apis/snode_api/expireRequest.ts b/ts/session/apis/snode_api/expireRequest.ts index 18594617a..6ad81d9a9 100644 --- a/ts/session/apis/snode_api/expireRequest.ts +++ b/ts/session/apis/snode_api/expireRequest.ts @@ -21,6 +21,7 @@ import { MAX_SUBREQUESTS_COUNT, UpdateExpiryOnNodeSubRequest, WithShortenOrExtend, + fakeHash, } from './SnodeRequestTypes'; import { doSnodeBatchRequest } from './batchRequest'; import { getSwarmFor } from './snodePool'; @@ -81,7 +82,7 @@ export async function verifyExpireMsgsResponseSignature({ export type ExpireRequestResponseResults = Record< string, - { hashes: Array; expiry: number } + { hashes: Array; expiry: number; unchangedHashes: Record } >; export async function processExpireRequestResponse( @@ -142,7 +143,7 @@ export async function processExpireRequestResponse( ); continue; } - results[nodeKey] = { hashes: updatedHashes, expiry }; + results[nodeKey] = { hashes: updatedHashes, expiry, unchangedHashes: unchangedHashes ?? {} }; } return results; @@ -212,17 +213,32 @@ async function updateExpiryOnNodes( messageHashes: expirationResult.hashes, updatedExpiryMs: expirationResult.expiry, }); + if (!isEmpty(expirationResult.unchangedHashes)) { + const unchanged = Object.entries(expirationResult.unchangedHashes); + unchanged.forEach(m => { + changesValid.push({ + messageHashes: [m[0]], + updatedExpiryMs: m[1], + }); + }); + } } const hashesRequestedButNotInResults = difference( flatten(expireRequests.map(m => m.params.messages)), - flatten(changesValid.map(c => c.messageHashes)) + [...flatten(changesValid.map(c => c.messageHashes)), fakeHash] ); if (!isEmpty(hashesRequestedButNotInResults)) { + const now = Date.now(); + window.log.debug( + 'messageHashes not found on swarm, considering them expired now():', + hashesRequestedButNotInResults, + now + ); // we requested hashes which are not part of the result. They most likely expired already so let's mark those messages as expiring now. changesValid.push({ messageHashes: hashesRequestedButNotInResults, - updatedExpiryMs: Date.now(), + updatedExpiryMs: now, }); } @@ -351,6 +367,14 @@ function groupMsgByExpiry(expiringDetails: ExpiringDetails) { } groupedBySameExpiry[expiryStr].push(messageHash); } + + Object.keys(groupedBySameExpiry).forEach(k => { + if (groupedBySameExpiry[k].length === 1) { + // We need to have at least 2 hashes until the next storage server release + groupedBySameExpiry[k].push(fakeHash); + } + }); + return groupedBySameExpiry; } diff --git a/ts/session/apis/snode_api/getExpiriesRequest.ts b/ts/session/apis/snode_api/getExpiriesRequest.ts index f3012ab8b..0b6b2f117 100644 --- a/ts/session/apis/snode_api/getExpiriesRequest.ts +++ b/ts/session/apis/snode_api/getExpiriesRequest.ts @@ -5,7 +5,7 @@ import { Snode } from '../../../data/data'; import { UserUtils } from '../../utils'; import { EmptySwarmError } from '../../utils/errors'; import { SeedNodeAPI } from '../seed_node_api'; -import { GetExpiriesFromNodeSubRequest } from './SnodeRequestTypes'; +import { GetExpiriesFromNodeSubRequest, fakeHash } from './SnodeRequestTypes'; import { doSnodeBatchRequest } from './batchRequest'; import { GetNetworkTime } from './getNetworkTime'; import { getSwarmFor } from './snodePool'; @@ -149,7 +149,7 @@ export async function buildGetExpiriesRequest({ export async function getExpiriesFromSnode({ messageHashes }: GetExpiriesFromSnodeProps) { // FIXME There is a bug in the snode code that requires at least 2 messages to be requested. Will be fixed in next storage server release if (messageHashes.length === 1) { - messageHashes.push('fakehash'); + messageHashes.push(fakeHash); } const ourPubKey = UserUtils.getOurPubKeyStrFromCache(); diff --git a/ts/session/conversations/ConversationController.ts b/ts/session/conversations/ConversationController.ts index a0abf2e0a..b0bd160aa 100644 --- a/ts/session/conversations/ConversationController.ts +++ b/ts/session/conversations/ConversationController.ts @@ -26,7 +26,6 @@ import { OpenGroupUtils } from '../apis/open_group_api/utils'; import { getSwarmPollingInstance } from '../apis/snode_api'; import { GetNetworkTime } from '../apis/snode_api/getNetworkTime'; import { SnodeNamespaces } from '../apis/snode_api/namespaces'; -import { DisappearingMessages } from '../disappearing_messages'; import { ClosedGroupMemberLeftMessage } from '../messages/outgoing/controlMessage/group/ClosedGroupMemberLeftMessage'; import { UserUtils } from '../utils'; import { ConfigurationSync } from '../utils/job_runners/jobs/ConfigurationSyncJob'; @@ -497,12 +496,8 @@ async function leaveClosedGroup(groupId: string, fromSyncMessage: boolean) { const ourLeavingMessage = new ClosedGroupMemberLeftMessage({ timestamp: networkTimestamp, groupId, - expirationType: DisappearingMessages.changeToDisappearingMessageType( - convo, - convo.getExpireTimer(), - convo.getExpirationMode() - ), - expireTimer: convo.getExpireTimer(), + expirationType: null, // we keep that one **not** expiring + expireTimer: null, }); window?.log?.info(`We are leaving the group ${groupId}. Sending our leaving message.`); diff --git a/ts/session/conversations/createClosedGroup.ts b/ts/session/conversations/createClosedGroup.ts index ca569b7ea..f1a6ad23f 100644 --- a/ts/session/conversations/createClosedGroup.ts +++ b/ts/session/conversations/createClosedGroup.ts @@ -188,7 +188,7 @@ function createInvitePromises( admins, keypair: encryptionKeyPair, timestamp: Date.now(), - expirationType: null, // Note: we do not make those messages expire as we want them available as much as possible on the swarm of the recipient + expirationType: null, // we keep that one **not** expiring expireTimer: 0, }; const message = new ClosedGroupNewMessage(messageParams); diff --git a/ts/session/disappearing_messages/index.ts b/ts/session/disappearing_messages/index.ts index a20d2eb0b..298966c1b 100644 --- a/ts/session/disappearing_messages/index.ts +++ b/ts/session/disappearing_messages/index.ts @@ -582,6 +582,7 @@ async function updateMessageExpiriesOnSwarm(messages: Array) { const newTTLs = await expireMessagesOnSnode(expiringDetails, { shortenOrExtend: 'shorten' }); const updatedMsgModels: Array = []; + window.log.debug('updateMessageExpiriesOnSwarm newTTLs: ', newTTLs); newTTLs.forEach(m => { const message = messages.find(model => model.getMessageHash() === m.messageHash); if (!message) { diff --git a/ts/session/group/closed-group.ts b/ts/session/group/closed-group.ts index 13efce4d1..dc001c681 100644 --- a/ts/session/group/closed-group.ts +++ b/ts/session/group/closed-group.ts @@ -290,12 +290,8 @@ async function sendNewName(convo: ConversationModel, name: string, messageId: st groupId, identifier: messageId, name, - expirationType: DisappearingMessages.changeToDisappearingMessageType( - convo, - convo.getExpireTimer(), - convo.getExpirationMode() - ), - expireTimer: convo.getExpireTimer(), + expirationType: null, // we keep that one **not** expiring + expireTimer: 0, }); await getMessageQueue().sendToGroup({ message: nameChangeMessage, @@ -304,7 +300,7 @@ async function sendNewName(convo: ConversationModel, name: string, messageId: st } async function sendAddedMembers( - convo: ConversationModel, + _convo: ConversationModel, addedMembers: Array, messageId: string, groupUpdate: GroupInfo @@ -324,20 +320,14 @@ async function sendAddedMembers( } const encryptionKeyPair = ECKeyPair.fromHexKeyPair(hexEncryptionKeyPair); - const expirationMode = convo.getExpirationMode() || 'off'; - const existingExpireTimer = convo.getExpireTimer() || 0; // Send the Added Members message to the group (only members already in the group will get it) const closedGroupControlMessage = new ClosedGroupAddedMembersMessage({ timestamp: Date.now(), groupId, addedMembers, identifier: messageId, - expirationType: DisappearingMessages.changeToDisappearingMessageType( - convo, - convo.getExpireTimer(), - convo.getExpirationMode() - ), - expireTimer: convo.getExpireTimer(), + expirationType: null, // we keep that one **not** expiring + expireTimer: 0, }); await getMessageQueue().sendToGroup({ message: closedGroupControlMessage, @@ -353,12 +343,8 @@ async function sendAddedMembers( members, keypair: encryptionKeyPair, identifier: messageId || uuidv4(), - expirationType: DisappearingMessages.changeToDisappearingMessageType( - convo, - existingExpireTimer, - expirationMode - ), - expireTimer: existingExpireTimer, + expirationType: null, // we keep that one **not** expiring + expireTimer: 0, }); const promises = addedMembers.map(async m => { @@ -401,12 +387,8 @@ export async function sendRemovedMembers( groupId, removedMembers, identifier: messageId, - expirationType: DisappearingMessages.changeToDisappearingMessageType( - convo, - convo.getExpireTimer(), - convo.getExpirationMode() - ), - expireTimer: convo.getExpireTimer(), + expirationType: null, // we keep that one **not** expiring + expireTimer: 0, }); // Send the group update, and only once sent, generate and distribute a new encryption key pair if needed await getMessageQueue().sendToGroup({ @@ -467,8 +449,8 @@ async function generateAndSendNewEncryptionKeyPair( groupId: toHex(groupId), timestamp: GetNetworkTime.getNowWithNetworkOffset(), encryptedKeyPairs: wrappers, - expirationType: null, // we keep that one **not** expiring (not rendered in the clients, and we need it to be as available as possible on the swarm) - expireTimer: null, + expirationType: null, // we keep that one **not** expiring + expireTimer: 0, }); distributingClosedGroupEncryptionKeyPairs.set(toHex(groupId), newKeyPair); diff --git a/ts/test/session/unit/disappearing_messages/GetExpiriesRequest_test.ts b/ts/test/session/unit/disappearing_messages/GetExpiriesRequest_test.ts index daaebcfea..97ea8e397 100644 --- a/ts/test/session/unit/disappearing_messages/GetExpiriesRequest_test.ts +++ b/ts/test/session/unit/disappearing_messages/GetExpiriesRequest_test.ts @@ -1,7 +1,10 @@ import chai, { expect } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import Sinon from 'sinon'; -import { GetExpiriesFromNodeSubRequest } from '../../../../session/apis/snode_api/SnodeRequestTypes'; +import { + GetExpiriesFromNodeSubRequest, + fakeHash, +} from '../../../../session/apis/snode_api/SnodeRequestTypes'; import { GetExpiriesFromSnodeProps, GetExpiriesRequestResponseResults, @@ -89,7 +92,7 @@ describe('GetExpiriesRequest', () => { targetNode: generateFakeSnode(), expiries: { 'FLTUh/C/6E+sWRgNtrqWPXhQqKlIrpHVKJJtZsBMWKw': 1696983251624 }, // FIXME There is a bug in the snode code that requires at least 2 messages to be requested. Will be fixed in next storage server release - messageHashes: ['FLTUh/C/6E+sWRgNtrqWPXhQqKlIrpHVKJJtZsBMWKw', 'fakehash'], + messageHashes: ['FLTUh/C/6E+sWRgNtrqWPXhQqKlIrpHVKJJtZsBMWKw', fakeHash], }; it('returns valid results if the response is valid', async () => { diff --git a/ts/util/logging.ts b/ts/util/logging.ts index 9c7a6d9bf..83c494580 100644 --- a/ts/util/logging.ts +++ b/ts/util/logging.ts @@ -24,8 +24,8 @@ const LEVELS: Record = { // Backwards-compatible logging, simple strings and no level (defaulted to INFO) function now() { - const date = new Date(); - return date.toJSON(); + const current = Date.now(); + return `${current}`; } // To avoid [Object object] in our log since console.log handles non-strings smoothly