Fix password recovery reminder triggering

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased (develop)
 
+- fixed: Show the password recovery reminder at every balance milestone, including for funds that arrived while the app was closed or before the exchange rates loaded.
+
 ## 4.51.0 (staging)
 
 - added: Push info-server attestation tokens into edge-core-js via `setAttestationToken` so the login server can skip CAPTCHA for attested devices, and allow `LOGIN_SERVER` / `INFO_SERVER` env overrides for local E2E stacks.

eslint.config.mjs

@@ -126,7 +126,6 @@ export default [
       'src/actions/NotificationActions.ts',
       'src/actions/PaymentProtoActions.tsx',
       'src/actions/ReceiveDropdown.tsx',
-      'src/actions/RecoveryReminderActions.tsx',
 
       'src/actions/ScamWarningActions.tsx',
       'src/actions/ScanActions.tsx',
@@ -316,7 +315,7 @@ export default [
       'src/components/scenes/WcConnectScene.tsx',
       'src/components/scenes/WcDisconnectScene.tsx',
       'src/components/scenes/WebViewScene.tsx',
-      'src/components/services/AccountCallbackManager.tsx',
+
       'src/components/services/ActionQueueService.ts',
       'src/components/services/AirshipInstance.tsx',
       'src/components/services/AutoLogout.ts',

src/__tests__/actions/RecoveryReminderActions.test.ts

@@ -0,0 +1,200 @@
+import { beforeEach, describe, expect, it, jest } from '@jest/globals'
+import type { EdgeAccount, EdgeCurrencyWallet } from 'edge-core-js'
+
+import { checkPasswordRecovery } from '../../actions/RecoveryReminderActions'
+import type { PasswordReminderLevels } from '../../actions/SettingsActions'
+import type { RootState } from '../../reducers/RootReducer'
+import type { Action, Dispatch } from '../../types/reduxTypes'
+
+// Provide a virtual env.json so importing env.ts does not fail:
+jest.mock('../../../env.json', () => ({}), { virtual: true })
+
+const mockShowModal = jest.fn()
+const mockWriteReminders =
+  jest.fn<(account: EdgeAccount, levels: string[]) => void>()
+
+jest.mock('../../components/services/AirshipInstance', () => ({
+  Airship: {
+    show: async () => {
+      mockShowModal()
+      return 'cancel'
+    }
+  },
+  showError: () => {}
+}))
+jest.mock('../../components/modals/ButtonsModal', () => ({
+  ButtonsModal: () => null
+}))
+jest.mock('../../actions/SettingsActions', () => ({
+  writePasswordRecoveryReminders: async (
+    account: EdgeAccount,
+    levels: string[]
+  ) => {
+    mockWriteReminders(account, levels)
+  }
+}))
+
+type Navigation = Parameters<typeof checkPasswordRecovery>[0]
+const navigation = {
+  push: () => {}
+} as unknown as Navigation
+
+const noneShown: PasswordReminderLevels = {
+  '20': false,
+  '200': false,
+  '2000': false,
+  '20000': false,
+  '200000': false
+}
+
+/**
+ * A one-wallet account whose single balance is `btc` BTC.
+ */
+const makeWallet = (btc: string): EdgeCurrencyWallet =>
+  ({
+    id: 'wallet-1',
+    currencyInfo: {
+      pluginId: 'bitcoin',
+      currencyCode: 'BTC',
+      denominations: [{ name: 'BTC', multiplier: '100000000' }]
+    },
+    currencyConfig: { allTokens: {} },
+    balanceMap: new Map([[null, btc]])
+  } as unknown as EdgeCurrencyWallet)
+
+interface StateOptions {
+  balance?: string
+  hasRate?: boolean
+  recoveryKey?: string
+  remindersShown?: Partial<PasswordReminderLevels>
+  username?: string | null
+}
+
+const makeState = (opts: StateOptions = {}): RootState => {
+  const {
+    balance = '0',
+    hasRate = true,
+    recoveryKey,
+    remindersShown = {},
+    username = 'test-user'
+  } = opts
+
+  const account = {
+    recoveryKey,
+    username,
+    currencyWallets: { 'wallet-1': makeWallet(balance) }
+  } as unknown as EdgeAccount
+
+  return {
+    core: { account },
+    exchangeRates: {
+      crypto: hasRate
+        ? { bitcoin: { '': { 'iso:USD': { current: 100000 } } } }
+        : {},
+      fiat: {}
+    },
+    ui: {
+      settings: {
+        passwordRecoveryRemindersShown: { ...noneShown, ...remindersShown }
+      }
+    }
+  } as unknown as RootState
+}
+
+/**
+ * Run the thunk and report what it dispatched and wrote.
+ */
+const run = async (
+  state: RootState
+): Promise<{ levels: string[]; modalShown: boolean }> => {
+  const actions: Action[] = []
+  const dispatch = ((action: Action) => {
+    actions.push(action)
+    return action
+  }) as Dispatch
+
+  checkPasswordRecovery(navigation)(dispatch, () => state)
+  // Let the modal promise settle:
+  await Promise.resolve()
+
+  const levels = actions
+    .filter(
+      action => action.type === 'UPDATE_SHOW_PASSWORD_RECOVERY_REMINDER_MODAL'
+    )
+    .map(action => String((action as { data: string }).data))
+
+  return { levels, modalShown: mockShowModal.mock.calls.length > 0 }
+}
+
+describe('checkPasswordRecovery', () => {
+  beforeEach(() => {
+    mockShowModal.mockClear()
+    mockWriteReminders.mockClear()
+  })
+
+  it('does nothing below the lowest level', async () => {
+    // 0.0001 BTC at $100k = $10:
+    const { levels, modalShown } = await run(makeState({ balance: '10000' }))
+    expect(levels).toEqual([])
+    expect(modalShown).toBe(false)
+  })
+
+  it('shows the reminder once the balance crosses $20', async () => {
+    // 0.0005 BTC at $100k = $50:
+    const { levels, modalShown } = await run(makeState({ balance: '50000' }))
+    expect(levels).toEqual(['20'])
+    expect(modalShown).toBe(true)
+    expect(mockWriteReminders).toHaveBeenCalledWith(expect.anything(), ['20'])
+  })
+
+  it('marks every crossed level but shows one modal', async () => {
+    // 0.005 BTC at $100k = $500, which passes both $20 and $200:
+    const { levels, modalShown } = await run(makeState({ balance: '500000' }))
+    expect(levels).toEqual(['20', '200'])
+    expect(mockShowModal).toHaveBeenCalledTimes(1)
+    expect(modalShown).toBe(true)
+  })
+
+  it('skips levels that were already shown', async () => {
+    const { levels, modalShown } = await run(
+      makeState({ balance: '500000', remindersShown: { '20': true } })
+    )
+    expect(levels).toEqual(['200'])
+    expect(modalShown).toBe(true)
+  })
+
+  it('does nothing when every crossed level was shown', async () => {
+    const { levels, modalShown } = await run(
+      makeState({
+        balance: '500000',
+        remindersShown: { '20': true, '200': true }
+      })
+    )
+    expect(levels).toEqual([])
+    expect(modalShown).toBe(false)
+  })
+
+  it('waits when a funded wallet has no exchange rate yet', async () => {
+    const { levels, modalShown } = await run(
+      makeState({ balance: '500000', hasRate: false })
+    )
+    expect(levels).toEqual([])
+    expect(modalShown).toBe(false)
+  })
+
+  it('skips accounts that already have recovery set up', async () => {
+    const { levels, modalShown } = await run(
+      makeState({ balance: '500000', recoveryKey: 'abcd' })
+    )
+    expect(levels).toEqual([])
+    expect(modalShown).toBe(false)
+  })
+
+  it('skips light accounts', async () => {
+    const { levels, modalShown } = await run(
+      makeState({ balance: '500000', username: null })
+    )
+    expect(levels).toEqual([])
+    expect(modalShown).toBe(false)
+  })
+})

src/actions/RecoveryReminderActions.tsx

@@ -1,57 +1,103 @@
-import { lt } from 'biggystring'
+import { gte } from 'biggystring'
 import * as React from 'react'
 
 import { writePasswordRecoveryReminders } from '../actions/SettingsActions'
 import { ButtonsModal } from '../components/modals/ButtonsModal'
 import { Airship, showError } from '../components/services/AirshipInstance'
 import { lstrings } from '../locales/strings'
-import type { ThunkAction } from '../types/reduxTypes'
+import { getExchangeRate } from '../selectors/WalletSelectors'
+import type { RootState, ThunkAction } from '../types/reduxTypes'
 import type { NavigationBase } from '../types/routerTypes'
 import { isMaestro } from '../util/maestro'
-import { getTotalFiatAmountFromExchangeRates } from '../util/utils'
+import { getTotalFiatAmountFromExchangeRates, zeroString } from '../util/utils'
 
 const levels = ['20', '200', '2000', '20000', '200000'] as const
 
 /**
  * Show a modal if the user's balance is over one of the limits &
  * they don't have recovery set up.
+ *
+ * This runs on each exchange-rate refresh as well as on incoming
+ * transactions, since funds can arrive while the app is closed and the rates
+ * needed to price them can land after the transaction does.
  */
 export function checkPasswordRecovery(
   navigation: NavigationBase
 ): ThunkAction<void> {
   return (dispatch, getState) => {
     const state = getState()
     const { account } = state.core
+    // Light accounts have no password to recover:
+    if (account.username == null) return
     if (account.recoveryKey != null) return
     if (isMaestro()) return
 
+    // An incomplete rate set undercounts the balance, which would credit the
+    // wrong milestone. Skip this round and wait for the rates to land:
+    if (!hasRatesForAllBalances(state)) return
+
     const totalDollars = getTotalFiatAmountFromExchangeRates(state, 'iso:USD')
     const { passwordRecoveryRemindersShown } = state.ui.settings
 
-    // Loop towards the highest non-shown level less than our balance:
-    for (const level of levels) {
-      if (passwordRecoveryRemindersShown[level]) continue
-      if (lt(totalDollars, level)) return
+    // Every level the balance has passed, whether or not it was passed just
+    // now. A balance that jumps straight to $500 has crossed both $20 and
+    // $200, and is owed one reminder, not two:
+    const crossedLevels = levels.filter(level => gte(totalDollars, level))
+    const newLevels = crossedLevels.filter(
+      level => !passwordRecoveryRemindersShown[level]
+    )
+    if (newLevels.length === 0) return
 
-      // Mark this level as shown:
+    // Mark them shown before showing the modal, so the next check doesn't
+    // stack a second one on top:
+    for (const level of newLevels) {
       dispatch({
         type: 'UPDATE_SHOW_PASSWORD_RECOVERY_REMINDER_MODAL',
         data: level
       })
-      writePasswordRecoveryReminders(account, level).catch(error => {
-        showError(error)
-      })
-      showReminderModal(navigation).catch(error => {
+    }
+    writePasswordRecoveryReminders(account, newLevels).catch(
+      (error: unknown) => {
         showError(error)
-      })
-      return
+      }
+    )
+    showReminderModal(() => {
+      navigation.push('passwordRecovery')
+    }).catch((error: unknown) => {
+      showError(error)
+    })
+  }
+}
+
+/**
+ * True when every funded wallet & token has a USD exchange rate.
+ *
+ * `getExchangeRate` returns 0 for a rate it hasn't loaded, so a partly-loaded
+ * rate set is indistinguishable from a small balance by total alone.
+ */
+function hasRatesForAllBalances(state: RootState): boolean {
+  const { exchangeRates } = state
+  const { currencyWallets } = state.core.account
+  for (const walletId of Object.keys(currencyWallets)) {
+    const wallet = currencyWallets[walletId]
+    for (const [tokenId, nativeBalance] of wallet.balanceMap.entries()) {
+      if (zeroString(nativeBalance)) continue
+      const rate = getExchangeRate(
+        exchangeRates,
+        wallet.currencyInfo.pluginId,
+        tokenId,
+        'iso:USD'
+      )
+      if (rate === 0) return false
     }
   }
+  return true
 }
 /**
- * Actually show the password reminder modal.
+ * Actually show the password reminder modal, calling `onSetUp` if the user
+ * chooses to set recovery up now.
  */
-async function showReminderModal(navigation: NavigationBase) {
+async function showReminderModal(onSetUp: () => void): Promise<void> {
   const reply = await Airship.show<'ok' | 'cancel' | undefined>(bridge => (
     <ButtonsModal
       bridge={bridge}
@@ -63,5 +109,5 @@ async function showReminderModal(navigation: NavigationBase) {
       }}
     />
   ))
-  if (reply === 'ok') navigation.push('passwordRecovery')
+  if (reply === 'ok') onSetUp()
 }

src/actions/SettingsActions.tsx

@@ -475,13 +475,15 @@ export const writeWalletsSort = async (
 
 export async function writePasswordRecoveryReminders(
   account: EdgeAccount,
-  level: PasswordReminderTime
+  levels: PasswordReminderTime[]
 ): Promise<void> {
   const settings = await readSyncedSettings(account)
   const passwordRecoveryRemindersShown = {
     ...settings.passwordRecoveryRemindersShown
   }
-  passwordRecoveryRemindersShown[level] = true
+  for (const level of levels) {
+    passwordRecoveryRemindersShown[level] = true
+  }
   const updatedSettings = { ...settings, passwordRecoveryRemindersShown }
   await writeSyncedSettings(account, updatedSettings)
 }