Page MenuHomePhabricator

[Cashtab] Drastically reduce the info we keep in storage
ClosedPublic

Authored by bytesofman on Sep 17 2025, 03:17.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCaadd99c8e052: [Cashtab] Drastically reduce the info we keep in storage
Summary

Note to reviewer

The goal here is to simplify and standardize storage as prep for cross-platform / app integration. So, we isolate and store wallet info that does not change in a smaller JSON wallet object. This is intended for secure storage. The android adapter puts this there.

We keep everything else out of this. It's in sqlite in android, still indexedDb for web.

The plan is, going forward, "everything else" will be migrated to a standardized sqlite, optimized for loading speed and performance.

The diff is regrettably enormous. However, changing the storage is not possible to do in an isolated and contained way. Typescript and extensive integration tests confirm expected behavior. I have also tested the migration on the extension and web. Android does not migrate, need to wipe and then test.

Focus on wallets/useWallet.ts and wallets/index.ts which is where the main changes are. Everything else is cascading from this, i.e. we no longer deal with the paths map to get addresses, we now have sk and pk and address just available in the wallet.

Longer Notes
Storage refactor before the android app launches.

The current spaghetti implementation of "keep the private keys in secure storage" for android is too insane (we take a wallet object, pull out the private keys, throw everything else into sqlite, then recombine them every time we access them).

It needs to be simpler so that we can easily extend and add cache features for performance.

Refactor wallets to store only bare-minimum wallet data. Stuff that will never be a problem in ambiguously limited storage environments like android's Preferences. In this way, the secure storage stuff is always sequested, and we only write to it when we

  • create a wallet
  • delete a wallet
  • rename a wallet

The sqlite storage can be more easily used for more frequent write write operations, like caching utxos, token data, or tx history.

For now, this means we are not storing wallet state at all. We always get it on wallet activation. With chronik, that is not really a problem. It takes <1s and we are already locking the UI until utxos sync anyway. I don't notice a performance impact in this diff.

Going forward, when we have ecash-wallet and auto-updating utxo sets, we will again cache utxos. But utxos should not be in encrypted storage. Will be more thoughtful about how we cache and use sqlite when we get there.

We will end up totally losing the wallet state and replacing it with ecash-wallet and some cache optimizations in sqlite. For now, we just lose storing the wallet state, but preserve the same interface in Cashtab state.

Test Plan

npm test

Note that there is no support for migrating from the previous android storage situation. Clear and restart. No users so no need to migrate.

./wipe-android.sh
./start-android.sh

web: test at cashtab.io

npm run extension to build and test extension, try with paybutton, share address

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-lib@4.3.3 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 272 packages, and audited 275 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.1.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests

added 1462 packages, and audited 2876 packages in 13s

304 packages are looking for funding
  run `npm fund` for details

21 vulnerabilities (3 low, 10 moderate, 3 high, 5 critical)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.38.0 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

TS2305: Module '"wallet"' has no exported member 'CashtabWallet'.
    17 |     undecimalizeTokenAmount,
    18 |     CashtabUtxo,
  > 19 |     CashtabWallet,
       |     ^^^^^^^^^^^^^
    20 |     TokenUtxo,
    21 |     NonTokenUtxo,
    22 |     SlpDecimals,


Build cashtab-tests failed with exit code 1

extensive refactor to simplify storage and wallet loading

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-lib@4.3.3 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 272 packages, and audited 275 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.1.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests

added 1462 packages, and audited 2876 packages in 13s

304 packages are looking for funding
  run `npm fund` for details

21 vulnerabilities (3 low, 10 moderate, 3 high, 5 critical)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.40.0 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

TS2345: Argument of type 'null' is not assignable to parameter of type 'Element | HTMLDocument | (Element | HTMLDocument)[] | undefined'.
    388 |         expect(screen.getByText('$1,500.00 USD')).toBeInTheDocument();
    389 |
  > 390 |         screen.debug(null, Infinity);
        |                      ^^^^
    391 |         // We see a cancel button as this wallet created this offer
    392 |         const cancelButton = screen.getByRole('button', {
    393 |             name: 'Cancel Larry Kelley (LK)',


Build cashtab-tests failed with exit code 1

build lint, android storage update, tx history use context

improve state updates to allow for simultaneous updates of wallets and activeWallets, fix issue with spinner implementation in App.tsx

remove the bash script due to phantom lint issues

bytesofman edited the test plan for this revision. (Show Details)

remove debug logs, version bump

fix address sharing in extension, remove dead code

remove debug logs, remove dead code, fix extension address sharing, fix test initializing fn name

lint, clear todos, remove unused styled component

bytesofman edited the summary of this revision. (Show Details)
Fabien requested changes to this revision.Sep 22 2025, 13:33
Fabien added a subscriber: Fabien.
Fabien added inline comments.
cashtab/extension/src/service_worker.ts
85 ↗(On Diff #55761)

I don't get it, that sounds like a change in behavior to me ?

cashtab/src/chronik/index.ts
1331 ↗(On Diff #55761)

Is that an expected failure ? looks like you would get a wrong balance to me, this should likely not be ignored

cashtab/src/components/Header/index.tsx
67 ↗(On Diff #55761)

wallet name os guaranteed to be unique ?

cashtab/src/components/Wallets/index.tsx
320 ↗(On Diff #55761)

this function is no longer async then ? Or this is an unwanted change

cashtab/src/components/Wallets/styles.ts
70 ↗(On Diff #55761)

?

cashtab/src/helpers/index.ts
178 ↗(On Diff #55761)

What's the point of these very confusing interfaces ? Can't you just use the ? and ! operators where needed to distinguish ?

232 ↗(On Diff #55761)

Starting here the layout is all broken, this makes it very hard to review

546 ↗(On Diff #55761)

Let's keep the txToJson and txFromJson together and move these elsewhere

559 ↗(On Diff #55761)

All these moves makes it harder to review and appear very unrelated

cashtab/wipe-android.sh
1 ↗(On Diff #55761)

Missing header.

You might want to put this into some subfolder

This revision now requires changes to proceed.Sep 22 2025, 13:33
bytesofman planned changes to this revision.EditedSep 22 2025, 17:31
bytesofman marked 10 inline comments as done.
  • Isolate extension address sharing patch D18664
  • Clean up the changes in helpers/index.ts so it's clear what this diff actually impacts
cashtab/extension/src/service_worker.ts
85 ↗(On Diff #55761)

yes, this is not strictly related to this diff and could be isolated. This issue was discovered in the implementation of this diff.

will isolate this extension fix.

cashtab/src/chronik/index.ts
1331 ↗(On Diff #55761)

hm yeah i will refactor here

before, we only called this in the update method, which would set an API error on any error to lock the app UI until it cleared

now, we call this with createActiveCashtabWallet, which is not necessarily called in update. But we should preserve the behavior of errors setting API error.

cashtab/src/components/Header/index.tsx
67 ↗(On Diff #55761)

unless there is some issue with corrupted storage

but, now that we have address available, might as well use that, updated

cashtab/src/components/Wallets/index.tsx
320 ↗(On Diff #55761)

right, the function is no longer async

we are using purely sync methods available from ecash-lib

before, we were using mnemonicFromSeed from bip39, which is async

cashtab/src/components/Wallets/styles.ts
70 ↗(On Diff #55761)

because we used to store wallet state for every wallet, we used to render the stored wallet balance on the wallets page

issue in that, this balance was never guaranteed to be up to date, since it was just the balance of the wallet the last time it was active

in this diff, I just stopped showing this info, since we no longer store that info for stored wallets and because it was inaccurate

we could do it better by lazy loading balance based on the stored wallet.address for all wallets. in this way it would always be current. but i thought that should be another diff.

so, could preserve this component. but more likely will want it to look different when it comes back anyway.

cashtab/src/helpers/index.ts
178 ↗(On Diff #55761)

all of these will (soon) go away.

It's a long incremental history of how cashtab distinguished between token utxos and non token utxos.

In the bch-api days this was complex and API intensive, hence the need for the slpUtxos and nonSlpUtxos keys and persisting an organized utxo set to storage.

Today, we should not even have those keys at all. token utxos are strongly typed and easy to distinguish / filter from a single set. This is what ecash-wallet does, and ecash-wallet will handle all utxo selection when it is dropped into Cashtab.

For now, the reason these haven't been lost yet is because Cashtab has a lot of complex utxo selection functions for token actions, and they take advantage of these types to prevent typescript lint errors when these functions assume token utxo inputs (as they are only called with utxos from slpUtxos. These functions will all be dropped from Cashtab when ecash-wallet is implemented.

In this review, the issue is that I worked a lot in this file, the order of things got messed up. These are not new shapes but re-arrangements, will clean it up.

232 ↗(On Diff #55761)

i'll clean this up

what happened: I thought I could lose almost all of these JSON types in this diff, since we don't really have to ever migrate this stuff; i.e. when we find old wallets in storage we build new ones with mnemonic and name we do not piecemeal migrate legacy utxos and legacy tx history.

Turns out it is not so simple though, since Cashtab still uses these functions to prepare mocks for testing various legacy migrations. So I had to pull stuff back in, messing up the order of this page.

546 ↗(On Diff #55761)

in this case, they are not actually new functions added here; they were already in this file. I've just messed up the order so they look added.

I don't think they are worth moving as should be able to lose all these JSON conversion functions after completing the full migration of test suites to typescript.

559 ↗(On Diff #55761)

indeed, will clean this up

cashtab/wipe-android.sh
1 ↗(On Diff #55761)

header added. as for other folder, yes, but Cashtab scripts in general need better organization, should be its own diff. for now, this added at the same level as start-android.sh

bytesofman added inline comments.
cashtab/extension/src/service_worker.ts
85 ↗(On Diff #55761)

so, cashtab's existing behavior -- on addr approval, the extension sends the address with the approval msg

but we were not actually using this, we were just seeing that it was approved, then fetching it from storage.

clean up type changes in helpers and wallets

cashtab/extension/src/service_worker.ts
14 ↗(On Diff #55784)

note: I have not reverted the change here that is actually captured in D18664, because I would have to engineer new type interfaces for the same problem to get this diff to work "as is"

instead, D18664 should be reviewed and landed first, and I'll rebase on that

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-lib@4.3.3 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 272 packages, and audited 275 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.1.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests

added 1462 packages, and audited 2876 packages in 13s

304 packages are looking for funding
  run `npm fund` for details

21 vulnerabilities (3 low, 10 moderate, 3 high, 5 critical)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.41.0 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

TS2459: Module '"helpers"' declares 'CashtabTxJson' locally, but it is not exported.
    15 |     StoredCashtabWallet,
    16 | } from 'wallet';
  > 17 | import { CashtabTxJson } from 'helpers';
       |          ^^^^^^^^^^^^^
    18 | import { XecTxType } from 'chronik';
    19 | import { fromHex, toHex } from 'ecash-lib';
    20 | import * as wif from 'wif';


Build cashtab-tests failed with exit code 1
This revision is now accepted and ready to land.Sep 22 2025, 23:08

simplify and standardize sorting of rendered wallets, make sure extension waits for active wallet appropriately