Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC3be0c4fa6c8f: [Cashtab] Init storage abstraction layer to support cross-platform app

Summary

We would like Cashtab to be available as a mobile app. Currently, it is available as a webapp and a browser extension.

Both the extension and the webapp are using indexedDb, i.e. web storage. This is the best we can do with a web browser for a web app. However, we want to support native mobile storage for mobile apps, and we also want to support the chrome storage API for the extension (which is persistent).

In this diff, we init an abstraction layer for Cashtab's storage. For now we only support the extension. We migrate web storage to extension storage API.

After this, we will add support for iOS and Android native storage for capacitor.

Test Plan

npm test

npm run extension

navigate to brave://extensions and update

cd modules/cashtab-connect/demo
npm start

navigate to localhost:3000, check logs for extension, confirm migration, check that you can just see your wallets as before, confirm address sharing still works

Migration risk is mitigated here by not wiping localforage. It's not practical to test migration in unit tests / integration tests as we end up mocking all of the storage APIs and then ... "hey look the mock was called"

I did a few migrations including settings, contacts, multiple wallets by building old extension, setting it up, then installing the new one and opening it, checking the logs.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

capacitor-init

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 34098
Build 67665: Build Diff	cashtab-tests
Build 67664: arc lint + arc unit

Event Timeline

bytesofman created this revision.Aug 9 2025, 10:38

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 9 2025, 10:38

Harbormaster completed remote builds in B34085: Diff 55152.Aug 9 2025, 11:14

remove unused capacitor code, support extension storage migration, minor version bump

do not double define wallet types

Harbormaster completed remote builds in B34087: Diff 55154.Aug 9 2025, 11:43

Harbormaster completed remote builds in B34086: Diff 55153.

special helper functions for extension storage API, handle extension migration

bytesofman edited the test plan for this revision. (Show Details)Aug 9 2025, 14:52

Harbormaster completed remote builds in B34088: Diff 55155.Aug 9 2025, 14:57

add tests for extension storage helpers, handle potential legacy migrations in extension

remove the 5mb quota that does not exist

Harbormaster completed remote builds in B34089: Diff 55156.Aug 9 2025, 15:08

lose the isSecure keys which are informational only, replace with comments

Harbormaster completed remote builds in B34090: Diff 55157.Aug 9 2025, 15:12

bytesofman edited the summary of this revision. (Show Details)Aug 9 2025, 15:14

Harbormaster completed remote builds in B34091: Diff 55158.Aug 9 2025, 15:17

remove wrapper-only storage conversion functions

revert unrelated type changes in extension

Harbormaster completed remote builds in B34093: Diff 55160.Aug 9 2025, 15:27

lint

Harbormaster completed remote builds in B34092: Diff 55159.Aug 9 2025, 15:30

Harbormaster completed remote builds in B34094: Diff 55161.Aug 9 2025, 15:33

back out unrelated changes, standardize adapter API but preserve json conversion for migration

Harbormaster completed remote builds in B34098: Diff 55166.Aug 9 2025, 20:55

rebase

Harbormaster completed remote builds in B34108: Diff 55182.Aug 11 2025, 17:00

clean up migration, improve comments

Harbormaster completed remote builds in B34110: Diff 55184.Aug 11 2025, 17:40

bytesofman edited the test plan for this revision. (Show Details)Aug 11 2025, 18:30

bytesofman published this revision for review.Aug 11 2025, 18:36

bytesofman added inline comments.

cashtab/src/components/AppModes/Extension.tsx
23 ↗	(On Diff #55184)	we used to have a purpose-built method of only migrating and storing the addresses to extension storage the main reason is bc, when this was implemented, we had not yet organized and centralized all storage operations to the `useWallet` hook at least, I think that was why I didn't just do a full migration at the time...would have been better than this

Actually no change requested but questions about how it works.
I understand the goal and I think the code is much better but without this deeper understanding I will not be confident regarding my review for the edge cases.

cashtab/extension/src/service_worker.ts
124 ↗	(On Diff #55184)	'address' is not part of the declared storage keys, how did it work ?
151 ↗	(On Diff #55184)	Does that mean there is a format change in the stored data ? So users will "lose" their wallet and have to recover ?
163 ↗	(On Diff #55184)	why is the active one the first ? Can't you have several wallets in the extension ?
cashtab/src/components/AppModes/Extension.tsx
57 ↗	(On Diff #55184)	So my understanding is that the storage format differ between the web and the extension variants ?

This revision now requires changes to proceed.Aug 12 2025, 20:16

answered inline, lmk any other questions

The main thing is that Cashtab storage was not centrally organized in the useWallet.ts file until relatively recently. So, Cashtab used to directly access storage across many components, and there was not an easy way to validate it, migrate it, update it.

Now it is all organized in useWallet, and we even have many tests to ensure migration of older formats to the latest format.

This diff actually does not change the format at all. For the extension, it just moves it to chrome.storage instead of indexedDb. This should solve the problem of disappearing storage, as extension storage is not a temporary internet file, it is managed by the extension similar to app storage. chrome.storage is where a chrome extension should be storing all of its data. Cashtab was not doing this in the past because storage had to be cleaned up and organized before a migration was possible, and then the extension had to be cleaned up enough to handle a migration, and then still needed a good reason to do it.

The reason to do this now is that, by abstracting the storage API in Cashtab, we should be able to use substantially the same codebase for the extension (now with persistent storage), web, android, and iOS.

Cashtab storage has for some time required helper methods, since we cannot store bigint or Maps. Expectation is that the data stored on every platform will be the same. Cashtab has existing logic to convert the wallets key and the cashtabCache key to and from JSON when it is going in or coming out of storage. This logic is expected to be the same across platforms.

cashtab/extension/src/service_worker.ts
124 ↗	(On Diff #55184)	this was a one-off thing for the extension I wanted to add address sharing as a feature for the extension. However, it was not possible for a webapp to request data from localforage an extension can only share data from chrome.storage, not from localforage (i.e. indexedDb) wanting to get the feature to work and not being able to practically overcome the tech debt of migrating all extension storage from localforage->chrome.storage at the time, I instead implemented a system where only the active address was stored in chrome.storage it was overwritten every time the active wallet changed. So the `address` key in chrome.storage was always the address of the active wallet
151 ↗	(On Diff #55184)	Yes, there is a format change in the stored data. But this is not new nor is it related to the migration. indexedDb is not able to store bigints or Maps. So Cashtab has helper functions that are called before and after storing wallets and cashtabCache. This logic is in `cashtab/src/wallet/useWallet.ts` For this specific `fetchAddress` function -- because we only need to get the address -- we actually do not bother to convert the stored data to bigint and maps. The address is available in the JSON object, so we just get it from the JSON object. My thinking was that it was better to keep all of this conversion logic in one place -- useWallet -- rather than recreate it here for the extension, when we do not need it to get the address, which is the only thing we want to share. We can't just import these conversion functions from `cashtab/src` because of the way the extension build works (we could have these JSON replacer/reviver functions shared between extension src and cashtab src, but it would require some refactoring, imo not worth it unless we need it for some extension action).
163 ↗	(On Diff #55184)	this is a Cashtab-specific convention. when the user chooses an active wallet, storage is updated so that this wallet is at index 0. cashtab everywhere gets the active wallet this way. Arguably this is not the best way to do it, we could instead have an `isActive` key or similar, which would save us some sorting headaches currently worked out in Cashtab. But changing this would be unrelated to this diff.
cashtab/src/components/AppModes/Extension.tsx
57 ↗	(On Diff #55184)	No, after this migration the storage format will be identical between them. Before this diff Extension used indexedDb, which was associated with the "domain" `chrome-extension://gokcapblagndfmmebhlipionngmimbpi/` Cashtab used indexedDb, which was associated with the domain `cashtab.com` Extension had a unique "address" key in chrome.storage which only stored the address of the active wallet. the code around this comment is related to updating this address whenever an extension user changed their active wallet After this diff Extension uses chrome.storage, which stores the same JSONs at the same keys as Cashtab web with indexedDb Extension does not wipe indexedDb associated with the "domain" `chrome-extension://gokcapblagndfmmebhlipionngmimbpi/`, in case there is some issue with the migration, should be available We do not do any cleanup of the chrome.storage `address` key ... but this does not interfere with the other keys of Cashtab storage, it's just no longer used Cashtab web storage is unchanged

OK this is clear now, and the code looks good. The only remaining question is whether we should have an upgrade workflow for the extension users: this code does it automatically with no question asked, and there is always a risk of issue when massaging the user wallet. Should it be opt-in for some time (like a popup telling you to save your seed and proceed to migration), before we make it automatic ? Or just make it a blocking mandatory popup but ask the user to save the seed and confirm ?

In D18468#421431, @Fabien wrote:

OK this is clear now, and the code looks good. The only remaining question is whether we should have an upgrade workflow for the extension users: this code does it automatically with no question asked, and there is always a risk of issue when massaging the user wallet. Should it be opt-in for some time (like a popup telling you to save your seed and proceed to migration), before we make it automatic ? Or just make it a blocking mandatory popup but ask the user to save the seed and confirm ?

imo it's cleaner to just do it. All previous Cashtab migrations have worked this way.

From the user perspective, there is no difference in behavior between the unmigrated extension and the migrated extension. So if they opt in, unless there is a migration issue, the only thing they will notice is that they stop being asked to opt in.
While there is some risk of the migration experiencing issues, the current extension storage behavior has a (worse) known issue of randomly nuking seeds (this can happen in web too, but in web it is at least mitigated by checking for persistent storage status and asking for notification permissions, which increases the odds of most browsers leaving the storage alone).
It will need to be done eventually no matter what.

Fabien accepted this revision.Aug 13 2025, 12:51

This revision is now accepted and ready to land.Aug 13 2025, 12:51

Closed by commit rABC3be0c4fa6c8f: [Cashtab] Init storage abstraction layer to support cross-platform app (authored by bytesofman). · Explain WhyAug 13 2025, 15:13

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABC3be0c4fa6c8f: [Cashtab] Init storage abstraction layer to support cross-platform app.

[Cashtab] Init storage abstraction layer to support cross-platform app
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 55166

cashtab/extension/public/manifest.json

cashtab/extension/src/service_worker.ts

cashtab/package-lock.json

cashtab/package.json

cashtab/src/components/App/App.tsx

cashtab/src/components/App/fixtures/helpers.js

cashtab/src/components/AppModes/Extension.tsx

cashtab/src/config/storage.ts

cashtab/src/platform/tests/platform.test.ts

cashtab/src/platform/adapters/extension.ts

cashtab/src/platform/adapters/extensionMigration.ts

cashtab/src/platform/adapters/web.ts

cashtab/src/platform/detection.ts

cashtab/src/platform/index.ts

cashtab/src/platform/storage.ts

cashtab/src/platform/types.ts

cashtab/src/platform/types/chrome.ts

cashtab/src/wallet/useWallet.ts

[Cashtab] Init storage abstraction layer to support cross-platform appClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 55166

cashtab/extension/public/manifest.json

cashtab/extension/src/service_worker.ts

cashtab/package-lock.json

cashtab/package.json

cashtab/src/components/App/App.tsx

cashtab/src/components/App/fixtures/helpers.js

cashtab/src/components/AppModes/Extension.tsx

cashtab/src/config/storage.ts

cashtab/src/platform/__tests__/platform.test.ts

cashtab/src/platform/adapters/extension.ts

cashtab/src/platform/adapters/extensionMigration.ts

cashtab/src/platform/adapters/web.ts

cashtab/src/platform/detection.ts

cashtab/src/platform/index.ts

cashtab/src/platform/storage.ts

cashtab/src/platform/types.ts

cashtab/src/platform/types/chrome.ts

cashtab/src/wallet/useWallet.ts

[Cashtab] Init storage abstraction layer to support cross-platform app
ClosedPublic
Actions

Revision Contents
Changeset List

cashtab/src/platform/tests/platform.test.ts