Preserve conversation list ordering for message request response events

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

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<void> {
   }
 
   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<void> {
       removeFromMessageReceiverCache: ev.confirm,
       threadAci,
       groupV2Id,
+      receivedAtCounter,
+      receivedAtMs,
+      sentAt,
+      sourceType: MessageRequestResponseSource.MRR_SYNC,
       type: messageRequestResponseType,
     };
     drop(MessageRequests.onResponse(attributes));

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<void> {
   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,
       })
     );
 

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, {
+    await this.applyMessageRequestResponse(
-      viaStorageServiceSync,
+      messageRequestEnum.UNKNOWN,
-      shouldSave: false,
+      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<void> {
     const idForLogging = getConversationIdForLogging(this.attributes);
     log.info(`addMessageRequestResponseEventMessage/${idForLogging}: ${event}`);
 
-    const timestamp = Date.now();
+    let receivedAtMs: number;
-    const lastMessageTimestamp =
+    let timestamp: number;
-      // Fallback to `timestamp` since `lastMessageReceivedAtMs` is new
+    let receivedAtCounter: number;
-      this.get('lastMessageReceivedAtMs') ?? this.get('timestamp') ?? timestamp;
 
-    const maybeLastMessageTimestamp =
+    const lastMessageTimestamp =
-      event === MessageRequestResponseEvent.ACCEPT
+      this.get('lastMessageReceivedAtMs') ?? this.get('timestamp');
-        ? timestamp
+
-        : lastMessageTimestamp;
+    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,
+      // we increment sent_at by messageCountFromEnvelope to ensure consistent in-timeline
-      received_at_ms: maybeLastMessageTimestamp,
+      // 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<void> {
     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;
         }
       }
 

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,
+      source: MessageRequestResponseSource.STORAGE_SERVICE,
-      viaStorageServiceSync: true,
+      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,
+      source: MessageRequestResponseSource.STORAGE_SERVICE,
-      viaStorageServiceSync: true,
+      learnedAtMs: Date.now(),
     });
   } else if (!record.blocked) {
     // if the condition above failed the state could still be blocked=false

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<void> {
   // 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, {
+      await conversation.applyMessageRequestResponse(
-        shouldSave: true,
+        messageRequestEnum.BLOCK,
-      });
+        {
+          source: MessageRequestResponseSource.LOCAL,
+          timestamp: Date.now(),
+        },
+        { shouldSave: true }
+      );
 
       try {
         await singleProtoJobQueue.add(

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<void> {
@@ -3866,9 +3880,7 @@ export default class MessageReceiver
           item,
           'private'
         );
-        await conversation.applyMessageRequestResponse(type, {
+        await conversation.applyMessageRequestResponse(type, responseInfo);
-          fromSync: true,
-        });
       };
     }
 
@@ -3958,9 +3970,7 @@ export default class MessageReceiver
             }
             await conversation.applyMessageRequestResponse(
               messageRequestEnum.BLOCK,
-              {
+              responseInfo
-                fromSync: true,
-              }
             );
           })
         );
@@ -3975,9 +3985,7 @@ export default class MessageReceiver
             }
             await conversation.applyMessageRequestResponse(
               messageRequestEnum.ACCEPT,
-              {
+              responseInfo
-                fromSync: true,
-              }
             );
           })
         );

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;
   }
 }
 

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;
+    };