Challenge: Save conversationIds and start queues

app/main.ts

@@ -1225,7 +1225,7 @@ async function showDebugLogWindow() {
 let permissionsPopupWindow: BrowserWindow | undefined;
 function showPermissionsPopupWindow(forCalling: boolean, forCamera: boolean) {
   // eslint-disable-next-line no-async-promise-executor
-  return new Promise<void>(async (resolve, reject) => {
+  return new Promise<void>(async (resolveFn, reject) => {
     if (permissionsPopupWindow) {
       permissionsPopupWindow.show();
       reject(new Error('Permission window already showing'));
@@ -1276,7 +1276,7 @@ function showPermissionsPopupWindow(forCalling: boolean, forCamera: boolean) {
       removeDarkOverlay();
       permissionsPopupWindow = undefined;
 
-      resolve();
+      resolveFn();
     });
 
     permissionsPopupWindow.once('ready-to-show', () => {
@@ -1501,7 +1501,9 @@ app.on('ready', async () => {
 
   // If the sql initialization takes more than three seconds to complete, we
   // want to notify the user that things are happening
-  const timeout = new Promise(resolve => setTimeout(resolve, 3000, 'timeout'));
+  const timeout = new Promise(resolveFn =>
+    setTimeout(resolveFn, 3000, 'timeout')
+  );
   // eslint-disable-next-line more/no-then
   Promise.race([sqlInitPromise, timeout]).then(maybeTimeout => {
     if (maybeTimeout !== 'timeout') {
@@ -1691,11 +1693,11 @@ async function requestShutdown() {
   }
 
   getLogger().info('requestShutdown: Requesting close of mainWindow...');
-  const request = new Promise<void>((resolve, reject) => {
+  const request = new Promise<void>((resolveFn, reject) => {
     let timeout: NodeJS.Timeout | undefined;
 
     if (!mainWindow) {
-      resolve();
+      resolveFn();
       return;
     }
 
@@ -1707,7 +1709,7 @@ async function requestShutdown() {
       }
       clearTimeoutIfNecessary(timeout);
 
-      resolve();
+      resolveFn();
     });
 
     mainWindow.webContents.send('get-ready-for-shutdown');
@@ -1720,7 +1722,7 @@ async function requestShutdown() {
       getLogger().error(
         'requestShutdown: Response never received; forcing shutdown.'
       );
-      resolve();
+      resolveFn();
     }, 2 * 60 * 1000);
   });
 
@@ -1792,6 +1794,7 @@ app.on(
 
 app.setAsDefaultProtocolClient('sgnl');
 app.setAsDefaultProtocolClient('signalcaptcha');
+
 app.on('will-finish-launching', () => {
   // open-url must be set from within will-finish-launching for macOS
   // https://stackoverflow.com/a/43949291

ts/background.ts

@@ -47,7 +47,6 @@ import { isMoreRecentThan, isOlderThan, toDayMillis } from './util/timestamp';
 import { isValidReactionEmoji } from './reactions/isValidReactionEmoji';
 import type { ConversationModel } from './models/conversations';
 import { getContact } from './messages/helpers';
-import { getMessageById } from './messages/getMessageById';
 import { createBatcher } from './util/batcher';
 import { updateConversationsWithUuidLookup } from './updateConversationsWithUuidLookup';
 import { initializeAllJobQueues } from './jobs/initializeAllJobQueues';
@@ -139,6 +138,7 @@ import { updateOurUsername } from './util/updateOurUsername';
 import { ReactionSource } from './reactions/ReactionSource';
 import { singleProtoJobQueue } from './jobs/singleProtoJobQueue';
 import { getInitialState } from './state/getInitialState';
+import { conversationJobQueue } from './jobs/conversationJobQueue';
 
 const MAX_ATTACHMENT_DOWNLOAD_AGE = 3600 * 72 * 1000;
 
@@ -194,16 +194,54 @@ export async function startApp(): Promise<void> {
   // Initialize WebAPI as early as possible
   let server: WebAPIType | undefined;
   let messageReceiver: MessageReceiver | undefined;
+  let challengeHandler: ChallengeHandler | undefined;
+
   window.storage.onready(() => {
     server = window.WebAPI.connect(
       window.textsecure.storage.user.getWebAPICredentials()
     );
     window.textsecure.server = server;
 
-    initializeAllJobQueues({
+    challengeHandler = new ChallengeHandler({
-      server,
+      storage: window.storage,
+
+      startQueue(conversationId: string) {
+        conversationJobQueue.resolveVerificationWaiter(conversationId);
+      },
+
+      requestChallenge(request) {
+        window.sendChallengeRequest(request);
+      },
+
+      async sendChallengeResponse(data) {
+        await window.textsecure.messaging.sendChallengeResponse(data);
+      },
+
+      onChallengeFailed() {
+        // TODO: DESKTOP-1530
+        // Display humanized `retryAfter`
+        showToast(ToastCaptchaFailed);
+      },
+
+      onChallengeSolved() {
+        showToast(ToastCaptchaSolved);
+      },
+
+      setChallengeStatus(challengeStatus) {
+        window.reduxActions.network.setChallengeStatus(challengeStatus);
+      },
     });
 
+    window.Whisper.events.on('challengeResponse', response => {
+      if (!challengeHandler) {
+        throw new Error('Expected challenge handler to be there');
+      }
+
+      challengeHandler.onResponse(response);
+    });
+
+    window.Signal.challengeHandler = challengeHandler;
+
     log.info('Initializing MessageReceiver');
     messageReceiver = new MessageReceiver({
       server,
@@ -709,6 +747,11 @@ export async function startApp(): Promise<void> {
       }
 
       if (window.isBeforeVersion(lastVersion, 'v5.37.0-alpha')) {
+        const legacyChallengeKey = 'challenge:retry-message-ids';
+        await removeStorageKeyJobQueue.add({
+          key: legacyChallengeKey,
+        });
+
         await window.Signal.Data.clearAllErrorStickerPackAttempts();
       }
 
@@ -1569,49 +1612,16 @@ export async function startApp(): Promise<void> {
     }
   }
 
-  let challengeHandler: ChallengeHandler | undefined;
-
   async function start() {
-    challengeHandler = new ChallengeHandler({
-      storage: window.storage,
-
-      getMessageById,
-
-      requestChallenge(request) {
-        window.sendChallengeRequest(request);
-      },
-
-      async sendChallengeResponse(data) {
-        await window.textsecure.messaging.sendChallengeResponse(data);
-      },
-
-      onChallengeFailed() {
-        // TODO: DESKTOP-1530
-        // Display humanized `retryAfter`
-        showToast(ToastCaptchaFailed);
-      },
-
-      onChallengeSolved() {
-        showToast(ToastCaptchaSolved);
-      },
-
-      setChallengeStatus(challengeStatus) {
-        window.reduxActions.network.setChallengeStatus(challengeStatus);
-      },
-    });
-
-    window.Whisper.events.on('challengeResponse', response => {
-      if (!challengeHandler) {
-        throw new Error('Expected challenge handler to be there');
-      }
-
-      challengeHandler.onResponse(response);
-    });
-
     // Storage is ready because `start()` is called from `storage.onready()`
+
+    strictAssert(challengeHandler, 'start: challengeHandler');
     await challengeHandler.load();
 
-    window.Signal.challengeHandler = challengeHandler;
+    strictAssert(server, 'start: server');
+    initializeAllJobQueues({
+      server,
+    });
 
     if (!window.storage.user.getNumber()) {
       const ourConversation =

ts/challenge.ts

@@ -12,15 +12,14 @@
 // are not immediately retried, however, until `.onOnline()` is called from
 // when we are actually online.
 
-import type { MessageModel } from './models/messages';
 import { assert } from './util/assert';
-import { isNotNil } from './util/isNotNil';
 import { isOlderThan } from './util/timestamp';
 import { parseRetryAfter } from './util/parseRetryAfter';
 import { clearTimeoutIfNecessary } from './util/clearTimeoutIfNecessary';
 import { getEnvironment, Environment } from './environment';
 import type { StorageInterface } from './types/Storage.d';
 import { HTTPError } from './textsecure/Errors';
+import type { SendMessageChallengeData } from './textsecure/Errors';
 import * as log from './logging/log';
 
 export type ChallengeResponse = {
@@ -36,11 +35,6 @@ export type IPCResponse = {
   readonly data: ChallengeResponse;
 };
 
-export enum RetryMode {
-  Retry = 'Retry',
-  NoImmediateRetry = 'NoImmediateRetry',
-}
-
 type Handler = {
   readonly token: string | undefined;
 
@@ -54,22 +48,12 @@ export type ChallengeData = {
   readonly captcha: string;
 };
 
-export type MinimalMessage = Pick<
-  MessageModel,
-  'id' | 'idForLogging' | 'getLastChallengeError' | 'retrySend'
-> & {
-  isNormalBubble(): boolean;
-  get(name: 'sent_at'): number;
-  on(event: 'sent', callback: () => void): void;
-  off(event: 'sent', callback: () => void): void;
-};
-
 export type Options = {
   readonly storage: Pick<StorageInterface, 'get' | 'put'>;
 
   requestChallenge(request: IPCRequest): void;
 
-  getMessageById(messageId: string): Promise<MinimalMessage | undefined>;
+  startQueue(conversationId: string): void;
 
   sendChallengeResponse(data: ChallengeData): Promise<void>;
 
@@ -81,25 +65,22 @@ export type Options = {
   expireAfter?: number;
 };
 
-export type StoredEntity = {
+export const STORAGE_KEY = 'challenge:conversations';
-  readonly messageId: string;
-  readonly createdAt: number;
-};
 
-type TrackedEntry = {
+export type RegisteredChallengeType = Readonly<{
-  readonly message: MinimalMessage;
+  conversationId: string;
-  readonly createdAt: number;
+  createdAt: number;
-};
+  retryAt: number;
+  token?: string;
+}>;
 
 const DEFAULT_EXPIRE_AFTER = 24 * 3600 * 1000; // one day
-const MAX_RETRIES = 5;
 const CAPTCHA_URL = 'https://signalcaptchas.org/challenge/generate.html';
 const CAPTCHA_STAGING_URL =
   'https://signalcaptchas.org/staging/challenge/generate.html';
 
-function shouldRetrySend(message: MinimalMessage): boolean {
+function shouldStartQueue(registered: RegisteredChallengeType): boolean {
-  const error = message.getLastChallengeError();
+  if (!registered.retryAt || registered.retryAt <= Date.now()) {
-  if (!error || error.retryAfter <= Date.now()) {
     return true;
   }
 
@@ -117,6 +98,8 @@ export function getChallengeURL(): string {
 // `ChallengeHandler` should be in memory at the same time because they could
 // overwrite each others storage data.
 export class ChallengeHandler {
+  private solving = 0;
+
   private isLoaded = false;
 
   private challengeToken: string | undefined;
@@ -127,13 +110,14 @@ export class ChallengeHandler {
 
   private readonly responseHandlers = new Map<number, Handler>();
 
-  private readonly trackedMessages = new Map<string, TrackedEntry>();
+  private readonly registeredConversations = new Map<
+    string,
+    RegisteredChallengeType
+  >();
 
-  private readonly retryTimers = new Map<string, NodeJS.Timeout>();
+  private readonly startTimers = new Map<string, NodeJS.Timeout>();
 
-  private readonly pendingRetries = new Set<MinimalMessage>();
+  private readonly pendingStarts = new Set<string>();
-
-  private readonly retryCountById = new Map<string, number>();
 
   constructor(private readonly options: Options) {}
 
@@ -143,43 +127,18 @@ export class ChallengeHandler {
     }
 
     this.isLoaded = true;
-    const stored: ReadonlyArray<StoredEntity> =
+    const challenges: ReadonlyArray<RegisteredChallengeType> =
-      this.options.storage.get('challenge:retry-message-ids') || [];
+      this.options.storage.get(STORAGE_KEY) || [];
 
-    log.info(`challenge: loading ${stored.length} messages`);
+    log.info(`challenge: loading ${challenges.length} challenges`);
-
-    const entityMap = new Map<string, StoredEntity>();
-    for (const entity of stored) {
-      entityMap.set(entity.messageId, entity);
-    }
-
-    const retryIds = new Set<string>(stored.map(({ messageId }) => messageId));
-
-    const maybeMessages: ReadonlyArray<MinimalMessage | undefined> =
-      await Promise.all(
-        Array.from(retryIds).map(async messageId =>
-          this.options.getMessageById(messageId)
-        )
-      );
-
-    const messages: Array<MinimalMessage> = maybeMessages.filter(isNotNil);
-
-    log.info(`challenge: loaded ${messages.length} messages`);
 
     await Promise.all(
-      messages.map(async message => {
+      challenges.map(async challenge => {
-        const entity = entityMap.get(message.id);
-        if (!entity) {
-          log.error(
-            'challenge: unexpected missing entity ' +
-              `for ${message.idForLogging()}`
-          );
-          return;
-        }
-
         const expireAfter = this.options.expireAfter || DEFAULT_EXPIRE_AFTER;
-        if (isOlderThan(entity.createdAt, expireAfter)) {
+        if (isOlderThan(challenge.createdAt, expireAfter)) {
-          log.info(`challenge: expired entity for ${message.idForLogging()}`);
+          log.info(
+            `challenge: expired challenge for conversation ${challenge.conversationId}`
+          );
           return;
         }
 
@@ -190,7 +149,7 @@ export class ChallengeHandler {
         //
         // Wait for `.onOnline()` to trigger the retries instead of triggering
         // them here immediately (if the message is ready to be retried).
-        await this.register(message, RetryMode.NoImmediateRetry, entity);
+        await this.register(challenge);
       })
     );
   }
@@ -204,89 +163,88 @@ export class ChallengeHandler {
   public async onOnline(): Promise<void> {
     this.isOnline = true;
 
-    const pending = Array.from(this.pendingRetries.values());
+    const pending = Array.from(this.pendingStarts.values());
-    this.pendingRetries.clear();
+    this.pendingStarts.clear();
 
-    log.info(`challenge: online, retrying ${pending.length} messages`);
+    log.info(`challenge: online, starting ${pending.length} queues`);
 
-    // Retry messages that matured while we were offline
+    // Start queues for challenges that matured while we were offline
-    await Promise.all(pending.map(message => this.retryOne(message)));
+    await Promise.all(
+      pending.map(conversationId => this.startQueue(conversationId))
+    );
 
-    await this.retrySend();
+    await this.startAllQueues();
+  }
+
+  public maybeSolve(conversationId: string): void {
+    const challenge = this.registeredConversations.get(conversationId);
+    if (!challenge) {
+      return;
+    }
+
+    if (this.solving > 0) {
+      return;
+    }
+
+    if (challenge.token) {
+      this.solve(challenge.token);
+    }
   }
 
   public async register(
-    message: MinimalMessage,
+    challenge: RegisteredChallengeType,
-    retry = RetryMode.Retry,
+    data?: SendMessageChallengeData
-    entity?: StoredEntity
   ): Promise<void> {
-    if (this.isRegistered(message)) {
+    const { conversationId } = challenge;
-      log.info(
+
-        `challenge: message already registered ${message.idForLogging()}`
+    if (this.isRegistered(conversationId)) {
-      );
+      log.info(`challenge: conversation ${conversationId}  already registered`);
       return;
     }
 
-    this.trackedMessages.set(message.id, {
+    this.registeredConversations.set(conversationId, challenge);
-      message,
-      createdAt: entity ? entity.createdAt : Date.now(),
-    });
     await this.persist();
 
-    // Message is already retryable - initiate new send
+    // Challenge is already retryable - start the queue
-    if (retry === RetryMode.Retry && shouldRetrySend(message)) {
+    if (shouldStartQueue(challenge)) {
       log.info(
-        `challenge: sending message immediately ${message.idForLogging()}`
+        `challenge: starting conversation ${conversationId} immediately`
       );
-      await this.retryOne(message);
+      await this.startQueue(conversationId);
       return;
     }
 
-    const error = message.getLastChallengeError();
+    const waitTime = Math.max(0, challenge.retryAt - Date.now());
-    if (!error) {
+    const oldTimer = this.startTimers.get(conversationId);
-      log.error('Unexpected message without challenge error');
-      return;
-    }
-
-    const waitTime = Math.max(0, error.retryAfter - Date.now());
-    const oldTimer = this.retryTimers.get(message.id);
     if (oldTimer) {
       clearTimeoutIfNecessary(oldTimer);
     }
-    this.retryTimers.set(
+    this.startTimers.set(
-      message.id,
+      conversationId,
       setTimeout(() => {
-        this.retryTimers.delete(message.id);
+        this.startTimers.delete(conversationId);
 
-        this.retryOne(message);
+        this.startQueue(conversationId);
       }, waitTime)
     );
 
-    log.info(
+    log.info(`challenge: tracking ${conversationId} with waitTime=${waitTime}`);
-      `challenge: tracking ${message.idForLogging()} ` +
-        `with waitTime=${waitTime}`
-    );
 
-    if (!error.data.options || !error.data.options.includes('recaptcha')) {
+    if (data && !data.options?.includes('recaptcha')) {
       log.error(
-        `challenge: unexpected options ${JSON.stringify(error.data.options)}`
+        `challenge: unexpected options ${JSON.stringify(data.options)}`
       );
     }
 
-    if (!error.data.token) {
+    if (!challenge.token) {
+      const dataString = JSON.stringify(data);
       log.error(
-        `challenge: no token in challenge error ${JSON.stringify(error.data)}`
+        `challenge: ${conversationId} is waiting; no token in data ${dataString}`
       );
-    } else if (message.isNormalBubble()) {
+      return;
-      // Display challenge dialog only for core messages
-      // (e.g. text, attachment, embedded contact, or sticker)
-      //
-      // Note: not waiting on this call intentionally since it waits for
-      // challenge to be fully completed.
-      this.solve(error.data.token);
-    } else {
-      log.info(`challenge: not a bubble message ${message.idForLogging()}`);
     }
+
+    this.solve(challenge.token);
   }
 
   public onResponse(response: IPCResponse): void {
@@ -299,13 +257,13 @@ export class ChallengeHandler {
     handler.resolve(response.data);
   }
 
-  public async unregister(message: MinimalMessage): Promise<void> {
+  public async unregister(conversationId: string): Promise<void> {
-    log.info(`challenge: unregistered ${message.idForLogging()}`);
+    log.info(`challenge: unregistered conversation ${conversationId}`);
-    this.trackedMessages.delete(message.id);
+    this.registeredConversations.delete(conversationId);
-    this.pendingRetries.delete(message);
+    this.pendingStarts.delete(conversationId);
 
-    const timer = this.retryTimers.get(message.id);
+    const timer = this.startTimers.get(conversationId);
-    this.retryTimers.delete(message.id);
+    this.startTimers.delete(conversationId);
     clearTimeoutIfNecessary(timer);
 
     await this.persist();
@@ -330,95 +288,45 @@ export class ChallengeHandler {
       'ChallengeHandler has to be loaded before persisting new data'
     );
     await this.options.storage.put(
-      'challenge:retry-message-ids',
+      STORAGE_KEY,
-      Array.from(this.trackedMessages.entries()).map(
+      Array.from(this.registeredConversations.values())
-        ([messageId, { createdAt }]) => {
-          return { messageId, createdAt };
-        }
-      )
     );
   }
 
-  private isRegistered(message: MinimalMessage): boolean {
+  public isRegistered(conversationId: string): boolean {
-    return this.trackedMessages.has(message.id);
+    return this.registeredConversations.has(conversationId);
   }
 
-  private async retrySend(force = false): Promise<void> {
+  private startAllQueues({
-    log.info(`challenge: retrySend force=${force}`);
+    force = false,
+  }: {
+    force?: boolean;
+  } = {}): void {
+    log.info(`challenge: startAllQueues force=${force}`);
 
-    const retries = Array.from(this.trackedMessages.values())
+    Array.from(this.registeredConversations.values())
-      .map(({ message }) => message)
+      .filter(challenge => force || shouldStartQueue(challenge))
-      // Sort messages in `sent_at` order
+      .forEach(challenge => this.startQueue(challenge.conversationId));
-      .sort((a, b) => a.get('sent_at') - b.get('sent_at'))
-      .filter(message => force || shouldRetrySend(message))
-      .map(message => this.retryOne(message));
-
-    await Promise.all(retries);
   }
 
-  private async retryOne(message: MinimalMessage): Promise<void> {
+  private async startQueue(conversationId: string): Promise<void> {
-    // Send is already pending
-    if (!this.isRegistered(message)) {
-      return;
-    }
-
-    // We are not online
     if (!this.isOnline) {
-      this.pendingRetries.add(message);
+      this.pendingStarts.add(conversationId);
       return;
     }
 
-    const retryCount = this.retryCountById.get(message.id) || 0;
+    await this.unregister(conversationId);
-    log.info(
-      `challenge: retrying sending ${message.idForLogging()}, ` +
-        `retry count: ${retryCount}`
-    );
 
-    if (retryCount === MAX_RETRIES) {
+    if (this.registeredConversations.size === 0) {
-      log.info(
+      this.options.setChallengeStatus('idle');
-        `challenge: dropping message ${message.idForLogging()}, ` +
-          'too many failed retries'
-      );
-
-      // Keep the message registered so that we'll retry sending it on app
-      // restart.
-      return;
     }
 
-    await this.unregister(message);
+    log.info(`startQueue: starting queue ${conversationId}`);
-
+    this.options.startQueue(conversationId);
-    let sent = false;
-    const onSent = () => {
-      sent = true;
-    };
-    message.on('sent', onSent);
-
-    try {
-      await message.retrySend();
-    } catch (error) {
-      log.error(
-        `challenge: failed to send ${message.idForLogging()} due to ` +
-          `error: ${error && error.stack}`
-      );
-    } finally {
-      message.off('sent', onSent);
-    }
-
-    if (sent) {
-      log.info(`challenge: message ${message.idForLogging()} sent`);
-      this.retryCountById.delete(message.id);
-      if (this.trackedMessages.size === 0) {
-        this.options.setChallengeStatus('idle');
-      }
-    } else {
-      log.info(`challenge: message ${message.idForLogging()} not sent`);
-
-      this.retryCountById.set(message.id, retryCount + 1);
-      await this.register(message, RetryMode.NoImmediateRetry);
-    }
   }
 
   private async solve(token: string): Promise<void> {
+    this.solving += 1;
     this.options.setChallengeStatus('required');
     this.challengeToken = token;
 
@@ -426,6 +334,7 @@ export class ChallengeHandler {
 
     // Another `.solve()` has completed earlier than us
     if (this.challengeToken === undefined) {
+      this.solving -= 1;
       return;
     }
 
@@ -445,6 +354,7 @@ export class ChallengeHandler {
     } catch (error) {
       log.error(`challenge: challenge failure, error: ${error && error.stack}`);
       this.options.setChallengeStatus('required');
+      this.solving -= 1;
       return;
     }
 
@@ -452,7 +362,8 @@ export class ChallengeHandler {
 
     this.options.setChallengeStatus('idle');
 
-    this.retrySend(true);
+    this.startAllQueues({ force: true });
+    this.solving -= 1;
   }
 
   private async sendChallengeResponse(data: ChallengeData): Promise<void> {

ts/components/CaptchaDialog.tsx

@@ -40,6 +40,7 @@ export function CaptchaDialog(props: Readonly<PropsType>): JSX.Element {
         i18n={i18n}
         title={i18n('CaptchaDialog--can-close__title')}
         onClose={() => setIsClosing(false)}
+        key="skip"
       >
         <section>
           <p>{i18n('CaptchaDialog--can-close__body')}</p>
@@ -76,6 +77,7 @@ export function CaptchaDialog(props: Readonly<PropsType>): JSX.Element {
       title={i18n('CaptchaDialog__title')}
       hasXButton
       onClose={() => setIsClosing(true)}
+      key="primary"
     >
       <section>
         <p>{i18n('CaptchaDialog__first-paragraph')}</p>

ts/jobs/conversationJobQueue.ts

@@ -22,14 +22,17 @@ import { sendReaction } from './helpers/sendReaction';
 import type { LoggerType } from '../types/Logging';
 import { ConversationVerificationState } from '../state/ducks/conversationsEnums';
 import { sleep } from '../util/sleep';
-import { SECOND } from '../util/durations';
+import { MINUTE } from '../util/durations';
 import {
   OutgoingIdentityKeyError,
+  SendMessageChallengeError,
   SendMessageProtoError,
 } from '../textsecure/Errors';
 import { strictAssert } from '../util/assert';
 import { missingCaseError } from '../util/missingCaseError';
 import { explodePromise } from '../util/explodePromise';
+import type { Job } from './Job';
+import type { ParsedJob } from './types';
 
 // Note: generally, we only want to add to this list. If you do need to change one of
 //   these values, you'll likely need to write a database migration.
@@ -135,6 +138,16 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
     }
   >();
 
+  public override async add(
+    data: Readonly<ConversationQueueJobData>,
+    insert?: (job: ParsedJob<ConversationQueueJobData>) => Promise<void>
+  ): Promise<Job<ConversationQueueJobData>> {
+    const { conversationId } = data;
+    window.Signal.challengeHandler.maybeSolve(conversationId);
+
+    return super.add(data, insert);
+  }
+
   protected parseData(data: unknown): ConversationQueueJobData {
     return conversationQueueJobDataSchema.parse(data);
   }
@@ -215,6 +228,18 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
         break;
       }
 
+      if (window.Signal.challengeHandler.isRegistered(conversationId)) {
+        log.info(
+          'captcha challenge is pending for this conversation; waiting at most 5m...'
+        );
+        // eslint-disable-next-line no-await-in-loop
+        await Promise.race([
+          this.startVerificationWaiter(conversation.id),
+          sleep(5 * MINUTE),
+        ]);
+        continue;
+      }
+
       const verificationData =
         window.reduxStore.getState().conversations
           .verificationDataByConversation[conversationId];
@@ -228,12 +253,12 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
         ConversationVerificationState.PendingVerification
       ) {
         log.info(
-          'verification is pending for this conversation; waiting at most 30s...'
+          'verification is pending for this conversation; waiting at most 5m...'
         );
         // eslint-disable-next-line no-await-in-loop
         await Promise.race([
           this.startVerificationWaiter(conversation.id),
-          sleep(30 * SECOND),
+          sleep(5 * MINUTE),
         ]);
         continue;
       }
@@ -302,25 +327,31 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
       }
     } catch (error: unknown) {
       const untrustedConversationIds: Array<string> = [];
-      if (error instanceof OutgoingIdentityKeyError) {
+
-        const failedConversation = window.ConversationController.getOrCreate(
+      const processError = (toProcess: unknown) => {
-          error.identifier,
+        if (toProcess instanceof OutgoingIdentityKeyError) {
-          'private'
+          const failedConversation = window.ConversationController.getOrCreate(
-        );
+            toProcess.identifier,
-        strictAssert(failedConversation, 'Conversation should be created');
+            'private'
-        untrustedConversationIds.push(failedConversation.id);
+          );
-      } else if (error instanceof SendMessageProtoError) {
+          strictAssert(failedConversation, 'Conversation should be created');
-        (error.errors || []).forEach(innerError => {
+          untrustedConversationIds.push(failedConversation.id);
-          if (innerError instanceof OutgoingIdentityKeyError) {
+        } else if (toProcess instanceof SendMessageChallengeError) {
-            const failedConversation =
+          window.Signal.challengeHandler.register(
-              window.ConversationController.getOrCreate(
+            {
-                innerError.identifier,
+              conversationId,
-                'private'
+              createdAt: Date.now(),
-              );
+              retryAt: toProcess.retryAt,
-            strictAssert(failedConversation, 'Conversation should be created');
+              token: toProcess.data?.token,
-            untrustedConversationIds.push(failedConversation.id);
+            },
-          }
+            toProcess.data
-        });
+          );
+        }
+      };
+
+      processError(error);
+      if (error instanceof SendMessageProtoError) {
+        (error.errors || []).forEach(processError);
       }
 
       if (untrustedConversationIds.length) {

ts/jobs/helpers/sendNormalMessage.ts

@@ -13,10 +13,7 @@ import { SignalService as Proto } from '../../protobuf';
 import { handleMessageSend } from '../../util/handleMessageSend';
 import type { CallbackResultType } from '../../textsecure/Types.d';
 import { isSent } from '../../messages/MessageSendState';
-import {
+import { isOutgoing } from '../../state/selectors/message';
-  getLastChallengeError,
-  isOutgoing,
-} from '../../state/selectors/message';
 import type { AttachmentType } from '../../textsecure/SendMessage';
 import type { LinkPreviewType } from '../../types/message/LinkPreviews';
 import type { BodyRangesType, StoryContextType } from '../../types/Util';
@@ -286,18 +283,6 @@ export async function sendNormalMessage(
 
     await messageSendPromise;
 
-    if (
-      getLastChallengeError({
-        errors: messageSendErrors,
-      })
-    ) {
-      log.info(
-        `message ${messageId} hit a spam challenge. Not retrying any more`
-      );
-      await message.saveErrors(messageSendErrors);
-      return;
-    }
-
     const didFullySend =
       !messageSendErrors.length || didSendToEveryone(message);
     if (!didFullySend) {

ts/jobs/removeStorageKeyJobQueue.ts

@@ -7,7 +7,7 @@ import { JobQueue } from './JobQueue';
 import { jobQueueDatabaseStore } from './JobQueueDatabaseStore';
 
 const removeStorageKeyJobDataSchema = z.object({
-  key: z.enum(['senderCertificateWithUuid']),
+  key: z.enum(['senderCertificateWithUuid', 'challenge:retry-message-ids']),
 });
 
 type RemoveStorageKeyJobData = z.infer<typeof removeStorageKeyJobDataSchema>;

ts/models/messages.ts

@@ -7,7 +7,6 @@ import type {
   GroupV1Update,
   MessageAttributesType,
   MessageReactionType,
-  ShallowChallengeError,
   QuotedMessageType,
   WhatIsThis,
 } from '../model-types.d';
@@ -78,7 +77,6 @@ import { handleMessageSend } from '../util/handleMessageSend';
 import { getSendOptions } from '../util/getSendOptions';
 import { findAndFormatContact } from '../util/findAndFormatContact';
 import {
-  getLastChallengeError,
   getMessagePropStatus,
   getPropsForCallHistory,
   getPropsForMessage,
@@ -1151,13 +1149,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
 
     this.set({ errors });
 
-    if (
-      !this.doNotSave &&
-      errors.some(error => error.name === 'SendMessageChallengeError')
-    ) {
-      await window.Signal.challengeHandler.register(this);
-    }
-
     if (!skipSave && !this.doNotSave) {
       await window.Signal.Data.saveMessage(this.attributes, {
         ourUuid: window.textsecure.storage.user.getCheckedUuid().toString(),
@@ -1683,10 +1674,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     return false;
   }
 
-  getLastChallengeError(): ShallowChallengeError | undefined {
-    return getLastChallengeError(this.attributes);
-  }
-
   hasAttachmentDownloads(): boolean {
     return hasAttachmentDownloads(this.attributes);
   }

ts/test-both/challenge_test.ts

@@ -7,27 +7,21 @@ import { assert } from 'chai';
 import { noop } from 'lodash';
 import * as sinon from 'sinon';
 
-import type { MinimalMessage } from '../challenge';
+import { STORAGE_KEY, ChallengeHandler } from '../challenge';
-import { ChallengeHandler } from '../challenge';
+import type { RegisteredChallengeType } from '../challenge';
-
+import { DAY, SECOND } from '../util/durations';
-type CreateMessageOptions = {
-  readonly sentAt?: number;
-  readonly retryAfter?: number;
-  readonly isNormalBubble?: boolean;
-};
 
 type CreateHandlerOptions = {
   readonly autoSolve?: boolean;
   readonly challengeError?: Error;
   readonly expireAfter?: number;
   readonly onChallengeSolved?: () => void;
-  readonly onChallengeFailed?: (retryAfter?: number) => void;
+  readonly onChallengeFailed?: (retryAt?: number) => void;
 };
 
 const NOW = Date.now();
-const ONE_DAY = 24 * 3600 * 1000;
+const NEVER_RETRY = NOW + DAY;
-const NEVER_RETRY = NOW + ONE_DAY;
+const IMMEDIATE_RETRY = NOW - DAY;
-const IMMEDIATE_RETRY = NOW - ONE_DAY;
 
 // Various timeouts in milliseconds
 const DEFAULT_RETRY_AFTER = 25;
@@ -35,15 +29,13 @@ const SOLVE_AFTER = 5;
 
 describe('ChallengeHandler', () => {
   const storage = new Map<string, any>();
-  const messageStorage = new Map<string, MinimalMessage>();
   let challengeStatus = 'idle';
-  let sent: Array<string> = [];
+  let queuesStarted: Array<string> = [];
 
   beforeEach(function beforeEach() {
     storage.clear();
-    messageStorage.clear();
     challengeStatus = 'idle';
-    sent = [];
+    queuesStarted = [];
 
     this.sandbox = sinon.createSandbox();
     this.clock = this.sandbox.useFakeTimers({
@@ -55,56 +47,16 @@ describe('ChallengeHandler', () => {
     this.sandbox.restore();
   });
 
-  const createMessage = (
+  const createChallenge = (
-    id: string,
+    conversationId: string,
-    options: CreateMessageOptions = {}
+    options: Partial<RegisteredChallengeType> = {}
-  ): MinimalMessage => {
+  ): RegisteredChallengeType => {
-    const {
-      sentAt = 0,
-      isNormalBubble = true,
-      retryAfter = NOW + DEFAULT_RETRY_AFTER,
-    } = options;
-
-    const testLocalSent = sent;
-
-    const events = new Map<string, () => void>();
-
     return {
-      id,
+      conversationId,
-      idForLogging: () => id,
+      token: '1',
-      isNormalBubble() {
+      retryAt: NOW + DEFAULT_RETRY_AFTER,
-        return isNormalBubble;
+      createdAt: NOW - SECOND,
-      },
+      ...options,
-      getLastChallengeError() {
-        return {
-          name: 'Ignored',
-          message: 'Ignored',
-          retryAfter,
-          data: { token: 'token', options: ['recaptcha'] },
-        };
-      },
-      get(name) {
-        assert.equal(name, 'sent_at');
-        return sentAt;
-      },
-      on(name, handler) {
-        if (events.get(name)) {
-          throw new Error('Duplicate event');
-        }
-        events.set(name, handler);
-      },
-      off(name, handler) {
-        assert.equal(events.get(name), handler);
-        events.delete(name);
-      },
-      async retrySend() {
-        const handler = events.get('sent');
-        if (!handler) {
-          throw new Error('Expected handler');
-        }
-        handler();
-        testLocalSent.push(this.id);
-      },
     };
   };
 
@@ -127,6 +79,10 @@ describe('ChallengeHandler', () => {
         },
       },
 
+      startQueue(conversationId: string) {
+        queuesStarted.push(conversationId);
+      },
+
       onChallengeSolved,
       onChallengeFailed,
 
@@ -143,10 +99,6 @@ describe('ChallengeHandler', () => {
         }, SOLVE_AFTER);
       },
 
-      async getMessageById(messageId) {
-        return messageStorage.get(messageId);
-      },
-
       async sendChallengeResponse() {
         if (challengeError) {
           throw challengeError;
@@ -162,200 +114,156 @@ describe('ChallengeHandler', () => {
     return handler;
   };
 
-  const isInStorage = (messageId: string) => {
+  const isInStorage = (conversationId: string) => {
-    return (storage.get('challenge:retry-message-ids') || []).some(
+    return (storage.get(STORAGE_KEY) || []).some(
-      ({ messageId: storageId }: { messageId: string }) => {
+      ({ conversationId: storageId }: { conversationId: string }) => {
-        return storageId === messageId;
+        return storageId === conversationId;
       }
     );
   };
 
-  it('should automatically retry after timeout', async function test() {
+  it('should automatically start queue after timeout', async function test() {
     const handler = await createHandler();
 
-    const one = createMessage('1');
+    const one = createChallenge('1');
-    messageStorage.set('1', one);
-
     await handler.register(one);
-    assert.isTrue(isInStorage(one.id));
+    assert.isTrue(isInStorage(one.conversationId));
     assert.equal(challengeStatus, 'required');
 
     await this.clock.nextAsync();
 
-    assert.deepEqual(sent, ['1']);
+    assert.deepEqual(queuesStarted, [one.conversationId]);
     assert.equal(challengeStatus, 'idle');
-    assert.isFalse(isInStorage(one.id));
+    assert.isFalse(isInStorage(one.conversationId));
   });
 
   it('should send challenge response', async function test() {
     const handler = await createHandler({ autoSolve: true });
 
-    const one = createMessage('1', { retryAfter: NEVER_RETRY });
+    const one = createChallenge('1', {
-    messageStorage.set('1', one);
+      retryAt: NEVER_RETRY,
-
+    });
     await handler.register(one);
     assert.equal(challengeStatus, 'required');
 
     await this.clock.nextAsync();
 
-    assert.deepEqual(sent, ['1']);
+    assert.deepEqual(queuesStarted, [one.conversationId]);
-    assert.isFalse(isInStorage(one.id));
+    assert.isFalse(isInStorage(one.conversationId));
     assert.equal(challengeStatus, 'idle');
   });
 
-  it('should send old messages', async function test() {
+  it('should send old challenges', async function test() {
     const handler = await createHandler();
 
-    // Put messages in reverse order to validate that the send order is correct
+    const challenges = [
-    const messages = [
+      createChallenge('1'),
-      createMessage('3', { sentAt: 3 }),
+      createChallenge('2'),
-      createMessage('2', { sentAt: 2 }),
+      createChallenge('3'),
-      createMessage('1', { sentAt: 1 }),
     ];
-    for (const message of messages) {
+    for (const challenge of challenges) {
-      messageStorage.set(message.id, message);
+      await handler.register(challenge);
-      await handler.register(message);
     }
 
     assert.equal(challengeStatus, 'required');
-    assert.deepEqual(sent, []);
+    assert.deepEqual(queuesStarted, []);
 
-    for (const message of messages) {
+    for (const challenge of challenges) {
       assert.isTrue(
-        isInStorage(message.id),
+        isInStorage(challenge.conversationId),
-        `${message.id} should be in storage`
+        `${challenge.conversationId} should be in storage`
       );
     }
 
     await handler.onOffline();
 
-    // Wait for messages to mature
+    // Wait for challenges to mature
     await this.clock.nextAsync();
 
-    // Create new handler to load old messages from storage
+    // Create new handler to load old challenges from storage; it will start up online
     await createHandler();
-    for (const message of messages) {
+
-      await handler.unregister(message);
+    for (const challenge of challenges) {
+      await handler.unregister(challenge.conversationId);
     }
 
-    for (const message of messages) {
+    for (const challenge of challenges) {
       assert.isFalse(
-        isInStorage(message.id),
+        isInStorage(challenge.conversationId),
-        `${message.id} should not be in storage`
+        `${challenge.conversationId} should not be in storage`
       );
     }
 
     // The order has to be correct
-    assert.deepEqual(sent, ['1', '2', '3']);
+    assert.deepEqual(queuesStarted, ['1', '2', '3']);
     assert.equal(challengeStatus, 'idle');
   });
 
-  it('should send message immediately if it is ready', async () => {
+  it('should send challenge immediately if it is ready', async () => {
     const handler = await createHandler();
 
-    const one = createMessage('1', { retryAfter: IMMEDIATE_RETRY });
+    const one = createChallenge('1', {
-    await handler.register(one);
+      retryAt: IMMEDIATE_RETRY,
-
-    assert.equal(challengeStatus, 'idle');
-    assert.deepEqual(sent, ['1']);
-  });
-
-  it('should not change challenge status on non-bubble messages', async function test() {
-    const handler = await createHandler();
-
-    const one = createMessage('1', {
-      isNormalBubble: false,
     });
     await handler.register(one);
 
     assert.equal(challengeStatus, 'idle');
-    assert.deepEqual(sent, []);
+    assert.deepEqual(queuesStarted, [one.conversationId]);
-
-    await this.clock.nextAsync();
-
-    assert.deepEqual(sent, ['1']);
   });
 
-  it('should not retry expired messages', async function test() {
+  it('should not retry expired challenges', async function test() {
     const handler = await createHandler();
 
-    const bubble = createMessage('1');
+    const one = createChallenge('1');
-    messageStorage.set('1', bubble);
+    await handler.register(one);
-    await handler.register(bubble);
+    assert.isTrue(isInStorage(one.conversationId));
-    assert.isTrue(isInStorage(bubble.id));
 
     const newHandler = await createHandler({
       autoSolve: true,
       expireAfter: -1,
     });
-    await handler.unregister(bubble);
+    await handler.unregister(one.conversationId);
 
     challengeStatus = 'idle';
     await newHandler.load();
 
     assert.equal(challengeStatus, 'idle');
-    assert.deepEqual(sent, []);
+    assert.deepEqual(queuesStarted, []);
 
     await this.clock.nextAsync();
 
     assert.equal(challengeStatus, 'idle');
-    assert.deepEqual(sent, []);
+    assert.deepEqual(queuesStarted, []);
-    assert.isFalse(isInStorage(bubble.id));
+    assert.isFalse(isInStorage(one.conversationId));
   });
 
-  it('should send messages that matured while we were offline', async function test() {
+  it('should send challenges that matured while we were offline', async function test() {
     const handler = await createHandler();
 
-    const one = createMessage('1');
+    const one = createChallenge('1');
-    messageStorage.set('1', one);
     await handler.register(one);
 
-    assert.isTrue(isInStorage(one.id));
+    assert.isTrue(isInStorage(one.conversationId));
-    assert.deepEqual(sent, []);
+    assert.deepEqual(queuesStarted, []);
     assert.equal(challengeStatus, 'required');
 
     await handler.onOffline();
 
-    // Let messages mature
+    // Let challenges mature
     await this.clock.nextAsync();
 
-    assert.isTrue(isInStorage(one.id));
+    assert.isTrue(isInStorage(one.conversationId));
-    assert.deepEqual(sent, []);
+    assert.deepEqual(queuesStarted, []);
     assert.equal(challengeStatus, 'required');
 
     // Go back online
     await handler.onOnline();
 
-    assert.isFalse(isInStorage(one.id));
+    assert.isFalse(isInStorage(one.conversationId));
-    assert.deepEqual(sent, [one.id]);
+    assert.deepEqual(queuesStarted, [one.conversationId]);
     assert.equal(challengeStatus, 'idle');
   });
 
-  it('should not retry more than 5 times', async function test() {
-    const handler = await createHandler();
-
-    const one = createMessage('1', { retryAfter: IMMEDIATE_RETRY });
-    const retrySend = sinon.stub(one, 'retrySend');
-
-    messageStorage.set('1', one);
-    await handler.register(one);
-
-    assert.isTrue(isInStorage(one.id));
-    assert.deepEqual(sent, []);
-    assert.equal(challengeStatus, 'required');
-
-    // Wait more than 5 times
-    for (let i = 0; i < 6; i += 1) {
-      await this.clock.nextAsync();
-    }
-
-    assert.isTrue(isInStorage(one.id));
-    assert.deepEqual(sent, []);
-    assert.equal(challengeStatus, 'required');
-
-    sinon.assert.callCount(retrySend, 5);
-  });
-
   it('should trigger onChallengeSolved', async function test() {
     const onChallengeSolved = sinon.stub();
 
@@ -364,8 +272,9 @@ describe('ChallengeHandler', () => {
       onChallengeSolved,
     });
 
-    const one = createMessage('1', { retryAfter: NEVER_RETRY });
+    const one = createChallenge('1', {
-    messageStorage.set('1', one);
+      retryAt: NEVER_RETRY,
+    });
     await handler.register(one);
 
     // Let the challenge go through
@@ -383,8 +292,9 @@ describe('ChallengeHandler', () => {
       onChallengeFailed,
     });
 
-    const one = createMessage('1', { retryAfter: NEVER_RETRY });
+    const one = createChallenge('1', {
-    messageStorage.set('1', one);
+      retryAt: NEVER_RETRY,
+    });
     await handler.register(one);
 
     // Let the challenge go through

ts/textsecure/Errors.ts

@@ -156,7 +156,7 @@ export class SendMessageChallengeError extends ReplayableError {
 
   public readonly data: SendMessageChallengeData | undefined;
 
-  public readonly retryAfter: number;
+  public readonly retryAt: number;
 
   constructor(identifier: string, httpError: HTTPError) {
     super({
@@ -171,7 +171,7 @@ export class SendMessageChallengeError extends ReplayableError {
 
     const headers = httpError.responseHeaders || {};
 
-    this.retryAfter = Date.now() + parseRetryAfter(headers['retry-after']);
+    this.retryAt = Date.now() + parseRetryAfter(headers['retry-after']);
 
     appendStack(this, httpError);
   }

ts/types/Storage.d.ts

@@ -24,6 +24,8 @@ import type {
   SessionResetsType,
   StorageServiceCredentials,
 } from '../textsecure/Types.d';
+import { UUIDStringType } from './UUID';
+import { RegisteredChallengeType } from '../challenge';
 
 export type SerializedCertificateType = {
   expires: number;
@@ -124,10 +126,8 @@ export type StorageAccessType = {
   preferredReactionEmoji: Array<string>;
   skinTone: number;
   unreadCount: number;
-  'challenge:retry-message-ids': ReadonlyArray<{
+  'challenge:conversations': ReadonlyArray<RegisteredChallengeType>;
-    messageId: string;
+
-    createdAt: number;
-  }>;
   deviceNameEncrypted: boolean;
   'indexeddb-delete-needed': boolean;
   senderCertificate: SerializedCertificateType;
@@ -144,6 +144,7 @@ export type StorageAccessType = {
   // Deprecated
   senderCertificateWithUuid: never;
   signaling_key: never;
+  'challenge:retry-message-ids': never;
 };
 
 export interface StorageInterface {