Skip to content

Fix password recovery reminder triggering - #6177

Open
j0ntz wants to merge 3 commits into
developfrom
jon/pw-recovery-reminder-trigger
Open

Fix password recovery reminder triggering#6177
j0ntz wants to merge 3 commits into
developfrom
jon/pw-recovery-reminder-trigger

Conversation

@j0ntz

@j0ntz j0ntz commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Description

The password-recovery reminder is meant to fire once per balance milestone ($20 / $200 / $2,000 / $20,000 / $200,000) for accounts with no recovery key. It rarely did, for these reasons:

  1. It only ran on newTransactions. checkPasswordRecovery had exactly one dispatch site, inside the wallet.on('newTransactions') handler. It now also runs on each exchange-rate refresh, so funds that arrived while the app was closed are noticed.
  2. The rates could be missing. getExchangeRate returns 0 for a rate it has not loaded, so a partly-loaded rate set undercounts the total and credits the wrong milestone. The thunk now bails out when any funded wallet or token lacks a USD rate, and picks the check back up on a later pass.
  3. Only the last transaction counted. The guard was !transactions[finalTxIndex].isSend, so a batch whose last element was a send skipped the check even when the batch contained receives. It now uses the already-computed receivedTxs.

Trigger paths, before and after

Before, every path to the check was gated on something that was often false:

sequenceDiagram
    autonumber
    participant Chain
    participant Wallet
    participant ACM as AccountCallbackManager
    participant Check as checkPasswordRecovery

    Chain-->>Wallet: deposit lands while the app is closed
    Note over Wallet,ACM: no listener attached, so no event
    Wallet-->>ACM: app reopens, wallet syncs
    ACM--xCheck: never dispatched, the only call site was newTransactions

    Chain-->>Wallet: deposit while the app is running
    Wallet->>ACM: newTransactions([receive, send])
    ACM--xCheck: skipped, guard read transactions[last].isSend

    Chain-->>Wallet: deposit on a cold start
    Wallet->>ACM: newTransactions([receive])
    ACM->>Check: dispatch
    Note over Check: rates not loaded yet, so the total is 0<br/>lt(0, "20") returns early, and nothing re-runs it
Loading

After, the rate-refresh cycle gives the check a second, unconditional entry point, and the two broken guards are fixed:

sequenceDiagram
    autonumber
    participant Chain
    participant Wallet
    participant ACM as AccountCallbackManager
    participant Check as checkPasswordRecovery

    Chain-->>Wallet: deposit lands while the app is closed
    Wallet-->>ACM: app reopens, wallet syncs

    loop every rate refresh (30s)
        ACM->>Check: dispatch
        alt a funded wallet still has no USD rate
            Note over Check: defer to the next refresh
        else rates complete
            Check->>Check: mark every crossed level shown
            Check-->>ACM: show one reminder modal
        end
    end

    Chain-->>Wallet: deposit while the app is running
    Wallet->>ACM: newTransactions([receive, send])
    ACM->>Check: dispatch, guard is now receivedTxs.length > 0
Loading

The above also forces two supporting changes:

  • The level loop marked only the lowest crossed level and returned, so an account that jumped straight to $500 earned two modals back to back. Now every crossed level is marked and a single modal is shown. writePasswordRecoveryReminders takes a level array so the marks are one read-modify-write, not a race.
  • Light accounts (account.username == null) are skipped. They have no password to recover, and they already get the backup modal from the same handler.

Also on this branch, because it blocks CI for every PR: commit 3923d60ac on develop accidentally committed .husky/_ as a symlink to an absolute local path, so husky install fails with ENOENT: mkdir '.husky/_' and npm run prepare exits 1 anywhere that path does not exist. Travis errors on develop for that reason. The stray symlink is removed here; npm run prepare then succeeds and husky recreates the directory itself.

Asana: https://app.asana.com/0/1215088146871429/1211152484915503

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

@j0ntz

j0ntz commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

📸🪓 Test evidence

🪓 Hack-forced evidence: commented out the newTransactions dispatch and forced passwordRecoveryRemindersShown to all-false, so the rate-refresh trigger was the only one that could fire; both edits reverted after capture. Temporary uncommitted edit, reverted before commit; the marked frames prove the rendering, not the trigger.

recovery reminder on funded account

recovery reminder on funded account

🪓 HACK-FORCED: rate refresh trigger

🪓 HACK-FORCED: rate refresh trigger

Captured by the agent's in-app test run (build-and-test).

@j0ntz
j0ntz force-pushed the jon/pw-recovery-reminder-trigger branch from a8e87e6 to 4f3fd7c Compare August 28, 2026 01:30
@j0ntz
j0ntz marked this pull request as ready for review August 28, 2026 01:30
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant