Page MenuHomePhabricator

[Cashtab] Parse tx history async to resolve UI lock issue on homescreen
ClosedPublic

Authored by bytesofman on Thu, Dec 12, 15:03.

Details

Summary

Cashtab has a bug in tx parsing that causes the UI to lock while tx history is rendered. It's especially noticeable on wallets with Agora txs in history.
'
Tx parsing should be done async. The parsed tx should then be passed to the Tx component for rendering.

We have instead been using the Tx component to parse and render txs.

This diff refactors Cashtab tx parsing so that all parsing is handled by the parseTx function, which is called async on each tx. When the history is ready, it is passed to the Tx component for rendering.

The size of the refactor makes this a difficult review. However, now that parsedTx is strongly typed, the logic changes can be deduced from the new type definitions. We preserve all of the rendering tests, which are updated for some differences in parsing (for example, we now parse each tokenEntry insteead of just the first).

Test Plan

npm test

This diff is deployed at the test site https://cashtab-local-dev.netlify.app/#/

Can confirm there that load times are significantly improved for wallets with Agora txs in history.

This diff also fixes the issue of the "menu expand animation goes slow"

night and day performance difference

Diff Detail

Repository
rABC Bitcoin ABC
Branch
unbork-tx-history
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31672
Build 62840: Build Diffcashtab-tests
Build 62839: arc lint + arc unit

Event Timeline

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 24s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2739: Type '{ xecTxType: XecTxType.Received; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': parsedTokenEntries, appActions
    275 |                 tokenFailedParsings: [],
    276 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 277 |                 parsed: {
        |                 ^^^^^^
    278 |                     xecTxType: XecTxType.Received,
    279 |                     satoshisSent: 546,
    280 |                     stackArray: [


Build cashtab-tests failed with exit code 1

not a good stopping point but have to do kid stuff

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 25s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2339: Property 'AgoraSale' does not exist on type 'typeof XecTxType'.
    935 |                 // For sales of agora partial txs, we assume the amount sold
    936 |                 // goes to a p2pkh address
  > 937 |                 if (xecTxType === XecTxType.AgoraSale) {
        |                                             ^^^^^^^^^
    938 |                     const { type } = cashaddr.getTypeAndHashFromOutputScript(
    939 |                         output.outputScript,
    940 |                     );


Build cashtab-tests failed with exit code 1

getting parseTx tests to pass

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 24s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2739: Type '{ xecTxType: XecTxType.Received; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': appActions, parsedTokenEntries
    275 |                 tokenFailedParsings: [],
    276 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 277 |                 parsed: {
        |                 ^^^^^^
    278 |                     xecTxType: XecTxType.Received,
    279 |                     satoshisSent: 546,
    280 |                     stackArray: [


Build cashtab-tests failed with exit code 1

not a good stopping point but works well in prod without the tx render slowdown problem. Still need more mocks and to get through refactoring the render unit tests

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 25s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2739: Type '{ xecTxType: XecTxType.Received; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': appActions, parsedTokenEntries
    275 |                 tokenFailedParsings: [],
    276 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 277 |                 parsed: {
        |                 ^^^^^^
    278 |                     xecTxType: XecTxType.Received,
    279 |                     satoshisSent: 546,
    280 |                     stackArray: [


Build cashtab-tests failed with exit code 1

21 more tests to update / patch

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 25s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2739: Type '{ xecTxType: XecTxType.Received; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': appActions, parsedTokenEntries
    275 |                 tokenFailedParsings: [],
    276 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 277 |                 parsed: {
        |                 ^^^^^^
    278 |                     xecTxType: XecTxType.Received,
    279 |                     satoshisSent: 546,
    280 |                     stackArray: [


Build cashtab-tests failed with exit code 1

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 25s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2739: Type '{ xecTxType: XecTxType.Received; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': appActions, parsedTokenEntries
    275 |                 tokenFailedParsings: [],
    276 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 277 |                 parsed: {
        |                 ^^^^^^
    278 |                     xecTxType: XecTxType.Received,
    279 |                     satoshisSent: 546,
    280 |                     stackArray: [


Build cashtab-tests failed with exit code 1

Tail of the build log:

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 25s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

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

TS2739: Type '{ xecTxType: XecTxType.Received; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': appActions, parsedTokenEntries
    275 |                 tokenFailedParsings: [],
    276 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 277 |                 parsed: {
        |                 ^^^^^^
    278 |                     xecTxType: XecTxType.Received,
    279 |                     satoshisSent: 546,
    280 |                     stackArray: [


Build cashtab-tests failed with exit code 1

ts lint, updating types, tests

Tail of the build log:

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3335 packages in 27s

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

8 vulnerabilities (2 low, 4 moderate, 2 high)

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.2.12 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/components/Home/Tx/index.tsx
  Line 64:5:  'AgoraTxIcon' is defined but never used  @typescript-eslint/no-unused-vars

src/token-protocols/index.ts
  Line 11:10:  'Token' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman edited the summary of this revision. (Show Details)
bytesofman edited the test plan for this revision. (Show Details)
emack requested changes to this revision.EditedMon, Dec 16, 04:51
emack added a subscriber: emack.

confirm there that load times are significantly improved for wallets with Agora txs

This may be subjective but I'm not seeing a drastic improvement after updating to latest and hard refreshing twice. In some instances prod loaded the home screen and agora pages quicker than the test site, though that could also be the hosting infra playing a part. Tests were done with wallets having agora txs in tx history

cashtab/src/chronik/index.ts
866–872 ↗(On Diff #51601)

you'll need a way to tag all these logic that would be invalidated as soon as you introduce eToken->XEC trades.

901 ↗(On Diff #51601)

as above, need a way to tag these assumptions with a key word so we can just grep for it when we introduce new functions which will break these assumptions, otherwise we're bound to miss one or two in refactors.

907 ↗(On Diff #51601)

should be more explicit otherwise an unexpected value would allow it to enter this IF block.

cashtab/src/components/Home/Tx/index.tsx
847 ↗(On Diff #51601)

Why is a 0 sat agora cancel possible if 0 sat listings are not?

This revision now requires changes to proceed.Mon, Dec 16, 04:51
bytesofman marked 4 inline comments as done.

=== p2pkh instead of !== p2sh

This may be subjective but I'm not seeing a drastic improvement after updating to latest and hard refreshing twice. In some instances prod loaded the home screen and agora pages quicker than the test site, though that could also be the hosting infra playing a part. Tests were done with wallets having agora txs in tx history

may need to receive a tx to get the full benefit here, as there is no forced migration to the new parsed format.

cashtab/src/chronik/index.ts
866–872 ↗(On Diff #51601)

maybe. it's not really practical to try to make design assumptions based on hypothetical future transactions though.

The main guarantor here is the tests. When we need to parse new txs, we also need to make sure the old tx tests still pass.

901 ↗(On Diff #51601)

same as above -- yes, if we had some way of knowing exactly what hypotheticcal future txs might look like. Otherwise, any change here will break tests and force a fix.

907 ↗(On Diff #51601)

good point

cashtab/src/components/Home/Tx/index.tsx
847 ↗(On Diff #51601)

this is a quirk of how the tx works. There is probably a more optimal way to parse this. But, we don't really care about satosohis being sent here. An agora cancel tx only uses dust sat amounts and only involves the creator address for p2pkh.

satoshisSent is itself subjective. It depends on the tx type.

Annoyingly, some parsing will always be guesswork. So it depends in large part about the types of known txs we see and how this particular tx compares to those.

This revision is now accepted and ready to land.Tue, Dec 17, 08:23