Page MenuHomePhabricator
Differential D15604

[Cashtab] Create and test new wallet creation and validation functions
ClosedPublic
Actions

Authored by bytesofman on Mar 1 2024, 21:11.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCef572e09b239: [Cashtab] Create and test new wallet creation and validation functions
Summary

T3445

Cashtab's wallet creation needs to be cleaned up before we can get wallet handling into the new standardized state and db management.

Duplicate existing functionality of Cashtab wallet creation engines in a function outside of the useWallet hook. Create a unified isValidCashtabWallet function that will replace isLegacyMigrationRequired and isValidStoredWallet functions.

Create new createCashtabWallet function in wallet/ that matches current Cashtab behavior. Add an integration test to confirm this.

None of the created functions are implemented in this diff.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cashtab-wallets
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 27588
Build 54739: Build Diffcashtab-tests
Build 54738: arc lint + arc unit

Event Timeline

bytesofman created this revision.Mar 1 2024, 21:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 1 2024, 21:11
Harbormaster completed remote builds in B27585: Diff 45827.Mar 1 2024, 21:11
bytesofman updated this revision to Diff 45828.Mar 1 2024, 21:12

lint

teamcity added a comment.Mar 1 2024, 21:13
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated @babel/plugin-proposal-numeric-separator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm WARN deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated @babel/plugin-proposal-private-methods@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-methods instead.
npm WARN deprecated abab@2.0.6: Use your platform's native atob() and btoa() methods instead
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated @babel/plugin-proposal-private-property-in-object@7.21.11: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-property-in-object instead.
npm WARN deprecated domexception@4.0.0: Use your platform's native DOMException instead
npm WARN deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0

added 1722 packages, and audited 1723 packages in 23s

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

found 0 vulnerabilities

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

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

Attempted import error: 'isLegacyMigrationRequired' is not exported from 'utils/cashMethods' (imported as 'isLegacyMigrationRequired').


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 45829.Mar 1 2024, 21:14

back out unrelated changes

Harbormaster completed remote builds in B27586: Diff 45828.Mar 1 2024, 21:14
teamcity added a comment.Mar 1 2024, 21:14
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm WARN deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated @babel/plugin-proposal-numeric-separator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm WARN deprecated @babel/plugin-proposal-private-methods@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-methods instead.
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated @babel/plugin-proposal-private-property-in-object@7.21.11: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-property-in-object instead.
npm WARN deprecated abab@2.0.6: Use your platform's native atob() and btoa() methods instead
npm WARN deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated domexception@4.0.0: Use your platform's native DOMException instead
npm WARN deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0

added 1722 packages, and audited 1723 packages in 23s

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

found 0 vulnerabilities

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

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

Attempted import error: 'isLegacyMigrationRequired' is not exported from 'utils/cashMethods' (imported as 'isLegacyMigrationRequired').


Build cashtab-tests failed with exit code 1
Harbormaster completed remote builds in B27587: Diff 45829.Mar 1 2024, 21:20
bytesofman updated this revision to Diff 45830.Mar 1 2024, 21:50

Add an integration test for importing a wallet that confirms the new wallet creation function matches the existing function

Harbormaster completed remote builds in B27588: Diff 45830.Mar 1 2024, 21:54
bytesofman published this revision for review.Mar 1 2024, 21:57
bytesofman added inline comments.
cashtab/src/components/__tests__/App.test.js
817

We confirm here that the new function createCashtabWallet --- not implemented in this diff -- returns a wallet that matches what we get in Cashtab now.

Difference: createCashtabWallet creates with an empty state field.

In this tests's imported wallet, the wallet.state field is populated (as expected) by API calls made during the wallet load process.

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

Pulled this logic out of existing (and tested) initializeCashtabStateForTests function. Until now, there was no reason to "only get the mocked chronik calls but do not touch localforage".

Behavior here is tested by existing tests for initializeCashtabStateForTests (and that all the other tests pass).

cashtab/src/validation/index.js
696

Right now, Cashtab has two functions that do parts of this

  1. isValidStoredWallet, determine if a wallet from localforage is valid or not
  2. isLegacyMigrationRequired, determine a given wallet from localforage key 'wallet' or localforage key 'savedWallets' needs to be rebuilt from seed in order to match the latest shape expected by Cashtab

This is bad design. We should have only one function. If it fails, we should migrate.

Not ready to implement this yet as will involve changes to how wallet and savedWallet are managed and loaded in useWallet.js. Will be done in a later diff.

cashtab/src/wallet/index.js
103

Longstanding 'to do' that Cashtab wallet shape should make more sense. This is hard to do now because handling migrations is difficult with our current storage management. Will fix that first, then improve the shape of the wallet.

For now, this function just matches what we are already doing in useWallet.js, but is unit tested because it's not part of a hook. We compare it to integration test to confirm this function matches current app behavior.

bytesofman edited the summary of this revision. (Show Details)Mar 1 2024, 21:58
emack accepted this revision.Mar 2 2024, 08:25
This revision is now accepted and ready to land.Mar 2 2024, 08:25
Closed by commit rABCef572e09b239: [Cashtab] Create and test new wallet creation and validation functions (authored by bytesofman). · Explain WhyMar 2 2024, 13:33
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCef572e09b239: [Cashtab] Create and test new wallet creation and validation functions.

Revision Contents
Changeset List

PathSize
cashtab/
src/
components/
__tests__/
74 lines
fixtures/
20 lines
validation/
__tests__/
11 lines
fixtures/
32 lines
235 lines
36 lines
wallet/
__tests__/
122 lines
fixtures/
48 lines
54 lines

Diff 45830

View Options

cashtab/src/components/__tests__/App.test.js

Loading...
View Options

cashtab/src/components/fixtures/helpers.js

Loading...
View Options

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

Loading...
View Options

cashtab/src/validation/fixtures/mocks.js

Loading...
View Options

cashtab/src/validation/fixtures/vectors.js

Loading...
View Options

cashtab/src/validation/index.js

Loading...
View Options

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

Loading...
View Options

cashtab/src/wallet/fixtures/vectors.js

Loading...
View Options

cashtab/src/wallet/index.js

Loading...