frontend: add backup reminder banner. #3446

bznein · 2025-07-10T13:36:59Z

For each wallet, the first time it crosses 1000 USD/CHF/EUR, display a banner suggesting users to create a paper backup of their seed. If the default currency is different from these three, we use USD.

If the keystore is connected, the banner shows a link to the device setting where the user can see their recovery words.
If the keystore is not connected, the banner asks the user to connect it.

If amounts are hidden, we do not display the banner at all.

Video (where I manually changed the threshold from 1000 to 1, you'll need to do the same to test it if you don't have at least 1000 in a wallet) - note: the video is slightly outdated but still shows the overall flow

bannerbackup-2025-07-10_14.26.37.mp4

A few comments are left inline.

Tested on Android and Linux
Before asking for reviews, here is a check list of the most common things you might need to consider:

updating the Changelog
writing unit tests
checking if your changes affect other coins or tokens in unintended ways
testing on multiple environments (Qt, Android, ...)
having an AI review your changes

bznein · 2025-07-10T13:39:58Z

frontends/web/src/routes/account/summary/accountssummary.tsx

@@ -115,6 +117,7 @@ export const AccountsSummary = ({
      ...prevBalances,
      [code]: balance.balance
    }));
+    setBalancesTrigger(Date.now());


This is the part I don't like... I need the banner to be able to trigger again when there is a call to "update" (otherwise it would not show up when opening the app, since the accounts are not loaded yet, or when a tx is received that bumps us over the thresold), so I came up with this solution with some AI help. I don't love it and I am open to more suggestions. CC @thisconnect :)

Why don't you use accountsBalanceSummary as the trigger instead? That var even contains the fiat total per keystore (in the current main fiat). I agree Date.now() is ugly 😂

Done. I don't actually need the fiat total as discussed in another comment, but looks definitely much better

bznein · 2025-07-10T13:43:53Z

frontends/web/src/routes/account/summary/accountssummary.tsx

+              <BackupReminder
+                key={keystore.rootFingerprint}
+                keystore={keystore}
+                deviceID={Object.keys(devices)[0]}


I worked on the assumption that there is only one value here when the keystore is connected, as we can't have multiple BB02 connected at the same time. It works, but are there possibly cases I missed here?

A device might not be connected (watch-only), so [0] would throw an exception?

I have no idea why it doesn't (looking at the console, I see no errors with a watch-only wallet but not connected BB02), but I will change it anyway to be more robust

bznein · 2025-07-10T14:01:38Z

backend/handlers/handlers.go

+
+	return response{
+		Success: true,
+		Show:    balance.Cmp(big.NewRat(1000, 1)) > 0,


Change this "1000" to a lower value for testing

bznein · 2025-07-10T15:11:51Z

frontends/web/src/components/banners/backup.tsx

+    return null;
+  }
+
+  const deviceSettingsURL = `/settings/device-settings/${deviceID}`;


Is there a way to directly link to "show recovery words"?

One solution I'd love would be a link that just "highlights" the "show recovery words" after link, which would be a much better UX than automatically "clicking" it.

Not sure if either is possible TBH

I tried something that highlights it, but it's very very ugly :)

We only have the bb02 now, so there could be a direct link, but it would break modularity, as there could be different types of devices with different settings/endpoints/etc.

Your highlight attempt is not in this PR is it? It can probably be done with a GET or URL param, maybe @thisconnect can could worry about the UX. I'd leave the redirect to the settings as you did and create an issue to improve UX here if needed for @thisconnect or @shonsirsha to handle after this is merged.

Your highlight attempt is not in this PR is it?

Yeah that wasn't clear, but it was ugly enough to NOT include it in the PR :)

benma · 2025-07-11T06:03:13Z

frontends/web/src/routes/account/summary/accountssummary.tsx

+            {accountsByKeystore.map(({ keystore }) => (
+              <BackupReminder
+                key={keystore.rootFingerprint}
+                keystore={keystore}


accountsBalanceSummary?.keystoresBalance[keystore.rootFingerprint].total contains the fiat total already, but I guess the issue is that you need to revert to USD of it's not one of the three currencies?

I don't actually need the total in the banner, because the total is only used by the backend to determine whether the banner needs to be displayed. but yes, I need to revert to USD as well.

(also, AFAIK accountsBalanceSummary?.keystoresBalance[keystore.rootFingerprint].total has the total as a string, and it would be ugly to have logic to decide whether to display the banner in the frontend AND operating on strings rather than numbers, imho)

benma · 2025-07-11T06:04:27Z

frontends/web/src/routes/account/summary/accountssummary.tsx

+              <BackupReminder
+                key={keystore.rootFingerprint}
+                keystore={keystore}
+                deviceID={Object.keys(devices)[0]}


A device might not be connected (watch-only), so [0] would throw an exception?

benma · 2025-07-11T06:12:58Z

backend/accounts.go

@@ -343,6 +343,32 @@ type KeystoreBalance = struct {
 	CoinsBalance AmountsByCoin `json:"coinsBalance"`
 }

+// AccountsFiatBalance returns the total fiat balance of a list of accounts.
+func (backend *Backend) AccountsFiatBalance(accounts AccountsList, fiatUnit string) (*big.Rat, error) {


We have a lot of balance computation functions, so imho it's important to reuse/refactor as we go instead of adding more of them.

backend.keystoreBalance() contains the same code as this function. How about calling this function there? So instead of

keystoreTotalBalance := new(big.Rat) for _, account := range accountList { ... }

it could be

keystoreTotalBalance, err := backend.AccountsFiatBalance(accounts, fiatUnit) ...

Done! I had to slightly refactor the AccountsFiatBalance to als oreturn the coin balance otherwise the entire loop would still be needed, I think it looks fine now, lmk

benma · 2025-07-11T06:22:05Z

frontends/web/src/components/banners/backup.tsx

+          {t('connectKeystore.promptWithNameForBackup',
+            {
+              name: keystore.name
+            })}


We have a connectKeystore() function with an associated UX that prompts to insert/unlock a keystore. It is currently tied to an account, but it could probably be refactored without much effort to work based on the root fingerprint only.

See also:

bitbox-wallet-app/backend/accounts.go

Line 908 in 63286f4

ConnectKeystore: func() (keystore.Keystore, error) {

bitbox-wallet-app/frontends/web/src/api/account.ts

Line 455 in 63286f4

export const connectKeystore = (code: AccountCode): Promise<{ success: boolean; }> => {

Would it be better UX to have a link, which when clicked, prompts to enter the keystore, and then redirects to the relevant workflow?

It looks much better, thanks!

Right now, what I did is passing accounts[0].code to the backup banner since connectKeystore still only uses that to extract the keystore fingerprint and then uses that internally. Let me know if you want me to refactor the connectKeystore function in this/another PR!

accounts[0] might not be attached to the same keystore, i.e. it could prompt for the wrong keystore.

If you are going to use the prompt, you should probably make a commit/PR to refactor it beforehand, and rebase this PR on top of that.

accounts[0] might not be attached to the same keystore, i.e. it could prompt for the wrong keystore.

{accountsByKeystore.map(({ keystore, accounts }) => ( <BackupReminder key={keystore.rootFingerprint} keystore={keystore} accountsBalanceSummary={accountsBalanceSummary} accountCode={accounts[0].code} />

Since I'm looping through accountsByKestore shouldn't accounts[0] be attached to the right keystore? Still happy to create a different PR! Just wanted to clarify!

benma · 2025-07-11T06:25:33Z

frontends/web/src/components/banners/backup.tsx

+    return null;
+  }
+
+  const deviceSettingsURL = `/settings/device-settings/${deviceID}`;


We only have the bb02 now, so there could be a direct link, but it would break modularity, as there could be different types of devices with different settings/endpoints/etc.

Your highlight attempt is not in this PR is it? It can probably be done with a GET or URL param, maybe @thisconnect can could worry about the UX. I'd leave the redirect to the settings as you did and create an issue to improve UX here if needed for @thisconnect or @shonsirsha to handle after this is merged.

benma · 2025-07-11T06:28:23Z

frontends/web/src/components/banners/backup.tsx

+
+export const BackupReminder = ({ keystore, deviceID, trigger }: BackupReminderProps) => {
+  const { t } = useTranslation();
+  const [bannerResponse, setBannerResponse] = useState<TShowBackupBannerResponse|null>(null);


Why does the linter not complain about missing spaces around | @thisconnect ? 😄

The linter currently ignores TypeScript.

benma · 2025-07-11T06:31:49Z

frontends/web/src/routes/account/summary/accountssummary.tsx

@@ -115,6 +117,7 @@ export const AccountsSummary = ({
      ...prevBalances,
      [code]: balance.balance
    }));
+    setBalancesTrigger(Date.now());


Why don't you use accountsBalanceSummary as the trigger instead? That var even contains the fiat total per keystore (in the current main fiat). I agree Date.now() is ugly 😂

For each wallet, the first time it crosses 1000 USD/CHF/EUR, display a banner suggesting users to create a paper backup of their seed.

bznein commented Jul 10, 2025

View reviewed changes

bznein requested review from benma and thisconnect July 10, 2025 13:44

bznein marked this pull request as ready for review July 10, 2025 13:44

bznein commented Jul 10, 2025

View reviewed changes

bznein force-pushed the bannerReminder branch 4 times, most recently from 0015c32 to c158ea5 Compare July 10, 2025 15:00

bznein commented Jul 10, 2025

View reviewed changes

bznein force-pushed the bannerReminder branch 2 times, most recently from 02de97b to a3b89fc Compare July 10, 2025 18:47

benma requested changes Jul 11, 2025

View reviewed changes

bznein force-pushed the bannerReminder branch 3 times, most recently from bb7a6cf to 9946d6a Compare July 14, 2025 10:21

bznein requested a review from benma July 14, 2025 10:27

bznein force-pushed the bannerReminder branch 4 times, most recently from e4a010a to ff58486 Compare July 14, 2025 10:31

frontend: add backup reminder banner.

15f8e89

For each wallet, the first time it crosses 1000 USD/CHF/EUR, display a banner suggesting users to create a paper backup of their seed.

bznein force-pushed the bannerReminder branch from ff58486 to 15f8e89 Compare July 17, 2025 08:54

frontend: add backup reminder banner. #3446

Are you sure you want to change the base?

frontend: add backup reminder banner. #3446

Conversation

bznein commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bznein commented Jul 10, 2025 •

edited

Loading