Page MenuHomePhabricator
Differential D17140

[Cashtab] More ts implementation as prep for wallet upgrade
ClosedPublic
Actions

Authored by bytesofman on Nov 15 2024, 06:01.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCc4b245224f2c: [Cashtab] More ts implementation as prep for wallet upgrade
Summary

Convert useWallet and dependencies to ts. Patch bugs discovered in ts implementation.

Ended up being quite a large diff.

I don't think there is much point in splitting it up, since everything is interdependent, and the integration tests confirm expected behavior. If something goes wrong, we can revert it all.

Summary of changes

  • New types for legacy wallets. Complicated implementation as these overlap in various ways.
  • Update wallet migration and storage functions to work with the types.
  • Patch some bugs discovered during ts implementation, for example parseInt() being used on something that was already a number in useWallet.js.
  • Some cleanup on wallet loading and migration in useWallet.js brought on by issues discovered in ts implementation
  • Major update of mocked wallets -- all tx history is now ChronikClient and type checked; all legacy wallets are appropriate legacy types.

Impact

  • Makes future wallet upgrades easier, esp if we want to support other cryptocurrencies like Doge
  • We need to add publickey into the wallet as it is needed for agora methods, cumbersome to always calculate it with Ecc every time we need it.
  • Now that ChronikClient has stable types, important to get Cashtab fully converted to ts
Test Plan

npm test

this diff is live at https://cashtab-local-dev.netlify.app/

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pubkey-in-wallet
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31174
Build 61848: Build Diffcashtab-tests
Build 61847: arc lint + arc unit

Event Timeline

bytesofman created this revision.Nov 15 2024, 06:01
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 15 2024, 06:01
Harbormaster completed remote builds in B31149: Diff 50867.Nov 15 2024, 06:04
teamcity added a comment.Nov 15 2024, 06:04
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-lib@0.2.1 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

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.1.1 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

added 1485 packages, and audited 3331 packages in 25s

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

3 vulnerabilities (2 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> cashtab@2.53.7 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/chronik/index.ts
  Line 210:13:  Forbidden non-null assertion  @typescript-eslint/no-non-null-assertion
  Line 210:31:  Forbidden non-null assertion  @typescript-eslint/no-non-null-assertion

src/wallet/useWallet.ts
  Line 57:5:  'MsgBlockClient' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 50875.Nov 15 2024, 18:12

Fixing types, more ts conversion, tests passing

Harbormaster completed remote builds in B31154: Diff 50875.Nov 15 2024, 18:33
teamcity added a comment.Nov 15 2024, 18:33
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

/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 1s

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

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.1.1 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

added 1485 packages, and audited 3331 packages in 25s

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

3 vulnerabilities (2 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

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

TS2769: No overload matches this call.
  Overload 1 of 4, '(iterable?: Iterable<readonly [string, string]> | null | undefined): Map<string, string>', gave the following error.
    Argument of type 'LegacyTokenState[] | [string, string][]' is not assignable to parameter of type 'Iterable<readonly [string, string]> | null | undefined'.
      Type 'LegacyTokenState[]' is not assignable to type 'Iterable<readonly [string, string]>'.
        The types returned by '[Symbol.iterator]().next(...)' are incompatible between these types.
          Type 'IteratorResult<LegacyTokenState, any>' is not assignable to type 'IteratorResult<readonly [string, string], any>'.
            Type 'IteratorYieldResult<LegacyTokenState>' is not assignable to type 'IteratorResult<readonly [string, string], any>'.
              Type 'IteratorYieldResult<LegacyTokenState>' is not assignable to type 'IteratorYieldResult<readonly [string, string]>'.
                Type 'LegacyTokenState' is not assignable to type 'readonly [string, string]'.
  Overload 2 of 4, '(entries?: readonly (readonly [string, string])[] | null | undefined): Map<string, string>', gave the following error.
    Argument of type 'LegacyTokenState[] | [string, string][]' is not assignable to parameter of type 'readonly (readonly [string, string])[] | null | undefined'.
      Type 'LegacyTokenState[]' is not assignable to type 'readonly (readonly [string, string])[]'.
        Type 'LegacyTokenState' is not assignable to type 'readonly [string, string]'.
    127 |         state: {
    128 |             ...storedCashtabWallet.state,
  > 129 |             tokens: new Map(storedCashtabWallet.state.tokens),
        |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    130 |         },
    131 |     };
    132 | };


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 50877.Nov 15 2024, 18:37

ts lint

Harbormaster completed remote builds in B31156: Diff 50877.Nov 15 2024, 18:40
teamcity added a comment.Nov 15 2024, 18:40
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

> ecash-lib@0.2.1 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 1s

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

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.1.1 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

added 1485 packages, and audited 3331 packages in 24s

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

3 vulnerabilities (2 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> cashtab@2.53.7 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/chronik/index.ts
  Line 210:13:  Forbidden non-null assertion  @typescript-eslint/no-non-null-assertion
  Line 210:31:  Forbidden non-null assertion  @typescript-eslint/no-non-null-assertion

src/helpers/index.ts
  Line 16:5:  'LegacyCashtabWallet_Pre_2_1_0' is defined but never used  @typescript-eslint/no-unused-vars
  Line 17:5:  'LegacyCashtabWallet_Pre_2_9_0' is defined but never used  @typescript-eslint/no-unused-vars

src/wallet/useWallet.ts
  Line 57:5:  'MsgBlockClient' 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)Nov 15 2024, 18:43
bytesofman edited the test plan for this revision. (Show Details)
bytesofman updated this revision to Diff 50878.Nov 15 2024, 18:49

lint

Harbormaster completed remote builds in B31157: Diff 50878.Nov 15 2024, 18:53
bytesofman published this revision for review.Nov 15 2024, 18:57
bytesofman edited the summary of this revision. (Show Details)
emack requested changes to this revision.Nov 16 2024, 12:01
emack added a subscriber: emack.
emack added inline comments.
cashtab/package-lock.json
47 ↗(On Diff #50878)

react-toastify (already in package.json) provides its own type definitions, so based on this it seems like you don't need @types/react-toastify?

cashtab/src/alias/index.ts
77–79 ↗(On Diff #50878)

Is this a ts linting thing that forces you to split this out?

cashtab/src/wallet/index.ts
304 ↗(On Diff #50878)
This revision now requires changes to proceed.Nov 16 2024, 12:01
bytesofman updated this revision to Diff 50896.Nov 16 2024, 13:54
bytesofman marked 3 inline comments as done.

rebase, remove not-needed dep, back out unrelated change to alias query function, fix comment

cashtab/package-lock.json
47 ↗(On Diff #50878)

nice good catch. this was installed as a first attempt to fix a ts lint error, but it was actually fixed by importing the type from the dist.

cashtab/src/alias/index.ts
77–79 ↗(On Diff #50878)

no, reverted.

Harbormaster completed remote builds in B31174: Diff 50896.Nov 16 2024, 13:58
emack accepted this revision.Nov 16 2024, 14:01
This revision is now accepted and ready to land.Nov 16 2024, 14:01
Closed by commit rABCc4b245224f2c: [Cashtab] More ts implementation as prep for wallet upgrade (authored by bytesofman). · Explain WhyNov 16 2024, 14:22
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCc4b245224f2c: [Cashtab] More ts implementation as prep for wallet upgrade.

Revision Contents
Changeset List

PathSize
cashtab/
src/
alias/
index.ts
index.js
33 lines
chronik/
index.ts
index.js
241 lines
components/
Agora/
Collection/
3 lines
App/
fixtures/
mocks.ts
mocks.js
1259 lines
Nfts/
__tests__/
5 lines
config/
8 lines
CashtabState.ts
CashtabState.js
17 lines
helpers/
219 lines
validation/
__tests__/
2 lines
wallet/
__tests__/
5 lines
60 lines
useWallet.ts
useWallet.js
445 lines

Diff 50896

View Options

cashtab/src/alias/index.js

Loading...
View Options

cashtab/src/alias/index.ts

Loading...
View Options

cashtab/src/chronik/index.js

Loading...
View Options

cashtab/src/chronik/index.ts

Loading...
View Options

cashtab/src/components/Agora/Collection/index.test.js

Loading...
View Options

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

Loading...
View Options

cashtab/src/components/App/fixtures/mocks.ts

Loading...
View Options

cashtab/src/components/Nfts/__tests__/index.test.js

Loading...
View Options

cashtab/src/config/CashtabSettings.ts

Loading...
View Options

cashtab/src/config/CashtabState.js

Loading...
View Options

cashtab/src/config/CashtabState.ts

Loading...
View Options

cashtab/src/helpers/index.js

Loading...
View Options

cashtab/src/helpers/index.ts

Loading...
View Options

cashtab/src/validation/__tests__/index.test.js

Loading...
View Options

cashtab/src/wallet/__tests__/useWallet.test.js

Loading...
View Options

cashtab/src/wallet/index.ts

Loading...
View Options

cashtab/src/wallet/useWallet.js

Loading...
View Options

cashtab/src/wallet/useWallet.ts

Loading...