From 660421fd2a4c935b31955c2d1fcd228dff7442fc Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Tue, 8 Jul 2025 12:39:40 -0500 Subject: [PATCH] Preserve conversation list ordering for message request response events Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com> --- ts/background.ts | 14 +++- ts/messageModifiers/MessageRequests.ts | 21 ++++- ts/models/conversations.ts | 102 +++++++++++++++++++----- ts/services/storageRecordOps.ts | 9 ++- ts/state/ducks/conversations.ts | 27 +++++-- ts/textsecure/MessageReceiver.ts | 26 +++--- ts/textsecure/messageReceiverEvents.ts | 15 ++++ ts/types/MessageRequestResponseEvent.ts | 25 ++++++ 8 files changed, 195 insertions(+), 44 deletions(-) diff --git a/ts/background.ts b/ts/background.ts index 0ecdbc3ea3..3ad76e4116 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -219,6 +219,7 @@ import { isLocalBackupsEnabled } from './util/isLocalBackupsEnabled'; import { NavTab } from './state/ducks/nav'; import { Page } from './components/Preferences'; import { EditState } from './components/ProfileEditor'; +import { MessageRequestResponseSource } from './types/MessageRequestResponseEvent'; const log = createLogger('background'); @@ -3398,7 +3399,14 @@ export async function startApp(): Promise { } function onMessageRequestResponse(ev: MessageRequestResponseEvent): void { - const { threadAci, groupV2Id, messageRequestResponseType } = ev; + const { + threadAci, + groupV2Id, + messageRequestResponseType, + receivedAtCounter, + receivedAtMs, + sentAt, + } = ev; log.info('onMessageRequestResponse', { threadAci, @@ -3418,6 +3426,10 @@ export async function startApp(): Promise { removeFromMessageReceiverCache: ev.confirm, threadAci, groupV2Id, + receivedAtCounter, + receivedAtMs, + sentAt, + sourceType: MessageRequestResponseSource.MRR_SYNC, type: messageRequestResponseType, }; drop(MessageRequests.onResponse(attributes)); diff --git a/ts/messageModifiers/MessageRequests.ts b/ts/messageModifiers/MessageRequests.ts index 830ec81b39..b392e68850 100644 --- a/ts/messageModifiers/MessageRequests.ts +++ b/ts/messageModifiers/MessageRequests.ts @@ -7,6 +7,7 @@ import * as Errors from '../types/errors'; import { createLogger } from '../logging/log'; import { drop } from '../util/drop'; import { getConversationIdForLogging } from '../util/idForLogging'; +import type { MessageRequestResponseSource } from '../types/MessageRequestResponseEvent'; const log = createLogger('MessageRequests'); @@ -14,6 +15,12 @@ export type MessageRequestAttributesType = { envelopeId: string; groupV2Id?: string; removeFromMessageReceiverCache: () => unknown; + receivedAtMs: number; + receivedAtCounter: number; + sourceType: + | MessageRequestResponseSource.BLOCK_SYNC + | MessageRequestResponseSource.MRR_SYNC; + sentAt: number; threadAci?: AciString; type: number; }; @@ -64,7 +71,14 @@ export async function onResponse( sync: MessageRequestAttributesType ): Promise { messageRequests.set(sync.envelopeId, sync); - const { threadAci, groupV2Id } = sync; + const { + threadAci, + groupV2Id, + receivedAtMs, + sentAt, + receivedAtCounter, + sourceType, + } = sync; const logId = `MessageRequests.onResponse(groupv2(${groupV2Id}) ${threadAci}`; @@ -92,7 +106,10 @@ export async function onResponse( drop( conversation.applyMessageRequestResponse(sync.type, { - fromSync: true, + source: sourceType, + receivedAtCounter, + receivedAtMs, + timestamp: sentAt, }) ); diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 3b59605cb5..973f4eabdb 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -169,7 +169,11 @@ import { incrementMessageCounter } from '../util/incrementMessageCounter'; import { generateMessageId } from '../util/generateMessageId'; import { getMessageAuthorText } from '../util/getMessageAuthorText'; import { downscaleOutgoingAttachment } from '../util/attachments'; -import { MessageRequestResponseEvent } from '../types/MessageRequestResponseEvent'; +import { + MessageRequestResponseSource, + type MessageRequestResponseInfo, + MessageRequestResponseEvent, +} from '../types/MessageRequestResponseEvent'; import type { AddressableMessage } from '../textsecure/messageReceiverEvents'; import { getConversationIdentifier, @@ -188,6 +192,7 @@ import { safeSetTimeout } from '../util/timeout'; import { getTypingIndicatorSetting } from '../types/Util'; import { INITIAL_EXPIRE_TIMER_VERSION } from '../util/expirationTimer'; import { maybeNotify } from '../messages/maybeNotify'; +import { missingCaseError } from '../util/missingCaseError'; const log = createLogger('conversations'); @@ -1011,10 +1016,19 @@ export class ConversationModel extends window.Backbone // Drop existing message request state to avoid sending receipts and // display MR actions. const messageRequestEnum = Proto.SyncMessage.MessageRequestResponse.Type; - await this.applyMessageRequestResponse(messageRequestEnum.UNKNOWN, { - viaStorageServiceSync, - shouldSave: false, - }); + await this.applyMessageRequestResponse( + messageRequestEnum.UNKNOWN, + viaStorageServiceSync + ? { + source: MessageRequestResponseSource.STORAGE_SERVICE, + learnedAtMs: Date.now(), + } + : { + source: MessageRequestResponseSource.LOCAL, + timestamp: Date.now(), + }, + { shouldSave: false } + ); window.reduxActions?.stories.removeAllContactStories(this.id); const serviceId = this.getServiceId(); @@ -2320,27 +2334,55 @@ export class ConversationModel extends window.Backbone } async addMessageRequestResponseEventMessage( - event: MessageRequestResponseEvent + event: MessageRequestResponseEvent, + responseInfo: MessageRequestResponseInfo, + { messageCountFromEnvelope }: { messageCountFromEnvelope: number } ): Promise { const idForLogging = getConversationIdForLogging(this.attributes); log.info(`addMessageRequestResponseEventMessage/${idForLogging}: ${event}`); - const timestamp = Date.now(); - const lastMessageTimestamp = - // Fallback to `timestamp` since `lastMessageReceivedAtMs` is new - this.get('lastMessageReceivedAtMs') ?? this.get('timestamp') ?? timestamp; + let receivedAtMs: number; + let timestamp: number; + let receivedAtCounter: number; - const maybeLastMessageTimestamp = - event === MessageRequestResponseEvent.ACCEPT - ? timestamp - : lastMessageTimestamp; + const lastMessageTimestamp = + this.get('lastMessageReceivedAtMs') ?? this.get('timestamp'); + + const { source } = responseInfo; + switch (source) { + case MessageRequestResponseSource.LOCAL: + receivedAtMs = responseInfo.timestamp; + receivedAtCounter = incrementMessageCounter(); + timestamp = responseInfo.timestamp; + break; + case MessageRequestResponseSource.MRR_SYNC: + receivedAtMs = responseInfo.receivedAtMs; + receivedAtCounter = responseInfo.receivedAtCounter; + timestamp = responseInfo.timestamp; + break; + case MessageRequestResponseSource.BLOCK_SYNC: + receivedAtMs = lastMessageTimestamp ?? responseInfo.receivedAtMs; + receivedAtCounter = responseInfo.receivedAtCounter; + timestamp = responseInfo.timestamp; + break; + case MessageRequestResponseSource.STORAGE_SERVICE: + receivedAtMs = lastMessageTimestamp ?? responseInfo.learnedAtMs; + receivedAtCounter = incrementMessageCounter(); + timestamp = responseInfo.learnedAtMs; + break; + default: + throw missingCaseError(source); + } const message = new MessageModel({ - ...generateMessageId(incrementMessageCounter()), + ...generateMessageId(receivedAtCounter), conversationId: this.id, type: 'message-request-response-event', - sent_at: maybeLastMessageTimestamp, - received_at_ms: maybeLastMessageTimestamp, + // we increment sent_at by messageCountFromEnvelope to ensure consistent in-timeline + // ordering when we add multiple messages from a single envelope (e.g. a + // BlOCK_AND_SPAM MRRSync) + sent_at: timestamp + messageCountFromEnvelope, + received_at_ms: receivedAtMs, readStatus: ReadStatus.Read, seenStatus: SeenStatus.NotApplicable, timestamp, @@ -2361,11 +2403,15 @@ export class ConversationModel extends window.Backbone async applyMessageRequestResponse( response: Proto.SyncMessage.MessageRequestResponse.Type, - { fromSync = false, viaStorageServiceSync = false, shouldSave = true } = {} + responseInfo: MessageRequestResponseInfo, + { shouldSave = true }: { shouldSave?: boolean } = {} ): Promise { try { const messageRequestEnum = Proto.SyncMessage.MessageRequestResponse.Type; - const isLocalAction = !fromSync && !viaStorageServiceSync; + const { source } = responseInfo; + const isLocalAction = source === MessageRequestResponseSource.LOCAL; + const viaStorageServiceSync = + source === MessageRequestResponseSource.STORAGE_SERVICE; const currentMessageRequestState = this.get('messageRequestResponseType'); const hasSpam = (messageRequestValue: number | undefined): boolean => { @@ -2391,6 +2437,8 @@ export class ConversationModel extends window.Backbone const wasPreviouslyAccepted = this.getAccepted(); if (didResponseChange) { + let messageCount = 0; + if (response === messageRequestEnum.ACCEPT) { // Only add a message if the user unblocked this conversation, or took an // explicit action to accept the message request on one of their devices @@ -2399,25 +2447,35 @@ export class ConversationModel extends window.Backbone this.addMessageRequestResponseEventMessage( didUnblock ? MessageRequestResponseEvent.UNBLOCK - : MessageRequestResponseEvent.ACCEPT + : MessageRequestResponseEvent.ACCEPT, + responseInfo, + { messageCountFromEnvelope: messageCount } ) ); + messageCount += 1; } } if (hasBlock(response) && didBlockChange) { drop( this.addMessageRequestResponseEventMessage( - MessageRequestResponseEvent.BLOCK + MessageRequestResponseEvent.BLOCK, + responseInfo, + { messageCountFromEnvelope: messageCount } ) ); + messageCount += 1; } + if (hasSpam(response) && didSpamChange) { drop( this.addMessageRequestResponseEventMessage( - MessageRequestResponseEvent.SPAM + MessageRequestResponseEvent.SPAM, + responseInfo, + { messageCountFromEnvelope: messageCount } ) ); + messageCount += 1; } } diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 0be5804c9a..e5a945682d 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -96,6 +96,7 @@ import { getSealedSenderIndicatorSetting, getTypingIndicatorSetting, } from '../types/Util'; +import { MessageRequestResponseSource } from '../types/MessageRequestResponseEvent'; const log = createLogger('storageRecordOps'); @@ -752,15 +753,15 @@ function applyMessageRequestState( if (record.blocked) { void conversation.applyMessageRequestResponse(messageRequestEnum.BLOCK, { - fromSync: true, - viaStorageServiceSync: true, + source: MessageRequestResponseSource.STORAGE_SERVICE, + learnedAtMs: Date.now(), }); } else if (record.whitelisted) { // unblocking is also handled by this function which is why the next // condition is part of the else-if and not separate void conversation.applyMessageRequestResponse(messageRequestEnum.ACCEPT, { - fromSync: true, - viaStorageServiceSync: true, + source: MessageRequestResponseSource.STORAGE_SERVICE, + learnedAtMs: Date.now(), }); } else if (!record.blocked) { // if the condition above failed the state could still be blocked=false diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 37b7f3be91..a6c6c7efc0 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -223,6 +223,7 @@ import { MessageModel } from '../../models/messages'; import type { ConversationModel } from '../../models/conversations'; import { EditState } from '../../components/ProfileEditor'; import { Page } from '../../components/Preferences'; +import { MessageRequestResponseSource } from '../../types/MessageRequestResponseEvent'; const log = createLogger('conversations'); @@ -3627,7 +3628,14 @@ async function syncMessageRequestResponse( ): Promise { // In GroupsV2, this may modify the server. We only want to continue if those // server updates were successful. - await conversation.applyMessageRequestResponse(response, { shouldSave }); + await conversation.applyMessageRequestResponse( + response, + { + source: MessageRequestResponseSource.LOCAL, + timestamp: Date.now(), + }, + { shouldSave } + ); const groupId = conversation.getGroupIdBuffer(); @@ -3840,8 +3848,10 @@ function acceptConversation( await conversation.applyMessageRequestResponse( messageRequestEnum.ACCEPT, { - shouldSave: true, - } + source: MessageRequestResponseSource.LOCAL, + timestamp: Date.now(), + }, + { shouldSave: true } ); try { @@ -3916,9 +3926,14 @@ function blockConversation( } else { // In GroupsV2, this may modify the server. We only want to continue if those // server updates were successful. - await conversation.applyMessageRequestResponse(messageRequestEnum.BLOCK, { - shouldSave: true, - }); + await conversation.applyMessageRequestResponse( + messageRequestEnum.BLOCK, + { + source: MessageRequestResponseSource.LOCAL, + timestamp: Date.now(), + }, + { shouldSave: true } + ); try { await singleProtoJobQueue.add( diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 7ab4864b6a..5d5fe61a10 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/ts/textsecure/MessageReceiver.ts @@ -165,6 +165,10 @@ import { fromAciUuidBytesOrString, fromPniUuidBytesOrUntaggedString, } from '../util/ServiceId'; +import { + type MessageRequestResponseInfo, + MessageRequestResponseSource, +} from '../types/MessageRequestResponseEvent'; const log = createLogger('MessageReceiver'); @@ -3224,6 +3228,9 @@ export default class MessageReceiver ), messageRequestResponseType: sync.type, groupV2Id: groupV2IdString, + receivedAtCounter: envelope.receivedAtCounter, + receivedAtMs: envelope.receivedAtDate, + sentAt: envelope.timestamp, }, this.#removeFromCache.bind(this, envelope) ); @@ -3858,6 +3865,13 @@ export default class MessageReceiver logUnexpectedUrgentValue(envelope, 'blockSync'); + const responseInfo: MessageRequestResponseInfo = { + source: MessageRequestResponseSource.BLOCK_SYNC, + receivedAtCounter: envelope.receivedAtCounter, + receivedAtMs: envelope.receivedAtDate, + timestamp: envelope.timestamp, + }; + function getAndApply( type: Proto.SyncMessage.MessageRequestResponse.Type ): (value: string) => Promise { @@ -3866,9 +3880,7 @@ export default class MessageReceiver item, 'private' ); - await conversation.applyMessageRequestResponse(type, { - fromSync: true, - }); + await conversation.applyMessageRequestResponse(type, responseInfo); }; } @@ -3958,9 +3970,7 @@ export default class MessageReceiver } await conversation.applyMessageRequestResponse( messageRequestEnum.BLOCK, - { - fromSync: true, - } + responseInfo ); }) ); @@ -3975,9 +3985,7 @@ export default class MessageReceiver } await conversation.applyMessageRequestResponse( messageRequestEnum.ACCEPT, - { - fromSync: true, - } + responseInfo ); }) ); diff --git a/ts/textsecure/messageReceiverEvents.ts b/ts/textsecure/messageReceiverEvents.ts index b11b33dd84..bac35876b5 100644 --- a/ts/textsecure/messageReceiverEvents.ts +++ b/ts/textsecure/messageReceiverEvents.ts @@ -325,6 +325,9 @@ export type MessageRequestResponseOptions = { messageRequestResponseType: Proto.SyncMessage.IMessageRequestResponse['type']; groupId?: string; groupV2Id?: string; + receivedAtCounter: number; + receivedAtMs: number; + sentAt: number; }; export class MessageRequestResponseEvent extends ConfirmableEvent { @@ -338,6 +341,12 @@ export class MessageRequestResponseEvent extends ConfirmableEvent { public readonly envelopeId?: string; + public readonly receivedAtMs: number; + + public readonly receivedAtCounter: number; + + public readonly sentAt: number; + constructor( { envelopeId, @@ -345,6 +354,9 @@ export class MessageRequestResponseEvent extends ConfirmableEvent { messageRequestResponseType, groupId, groupV2Id, + receivedAtMs, + receivedAtCounter, + sentAt, }: MessageRequestResponseOptions, confirm: ConfirmCallback ) { @@ -355,6 +367,9 @@ export class MessageRequestResponseEvent extends ConfirmableEvent { this.messageRequestResponseType = messageRequestResponseType; this.groupId = groupId; this.groupV2Id = groupV2Id; + this.receivedAtMs = receivedAtMs; + this.receivedAtCounter = receivedAtCounter; + this.sentAt = sentAt; } } diff --git a/ts/types/MessageRequestResponseEvent.ts b/ts/types/MessageRequestResponseEvent.ts index 8cacbf492d..e6b73f7f0f 100644 --- a/ts/types/MessageRequestResponseEvent.ts +++ b/ts/types/MessageRequestResponseEvent.ts @@ -6,3 +6,28 @@ export enum MessageRequestResponseEvent { UNBLOCK = 'UNBLOCK', SPAM = 'SPAM', } + +export enum MessageRequestResponseSource { + LOCAL = 'LOCAL', + MRR_SYNC = 'MRR_SYNC', + BLOCK_SYNC = 'BLOCK_SYNC', + STORAGE_SERVICE = 'STORAGE_SERVICE', +} + +export type MessageRequestResponseInfo = + | { + source: MessageRequestResponseSource.LOCAL; + timestamp: number; + } + | { + source: MessageRequestResponseSource.STORAGE_SERVICE; + learnedAtMs: number; + } + | { + source: + | MessageRequestResponseSource.BLOCK_SYNC + | MessageRequestResponseSource.MRR_SYNC; + timestamp: number; + receivedAtMs: number; + receivedAtCounter: number; + };