Page MenuHomePhabricator

[Cashtab] Move cashtabCache to cashtabState
ClosedPublic

Authored by bytesofman on Feb 27 2024, 23:51.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCb82890879305: [Cashtab] Move cashtabCache to cashtabState
Summary

The existing cashtabCache is poorly tested and, based on observing debug logs in prod Cashtab, it is possibly not of predictable shape.

The shape of the existing cashtabCache, an object with tokenId for keys, is bad for its use case and also difficult to migrate and test. Since there are possibly bugs in the existing cache, do not migrate -- instead start with a new cache.

Since we are starting with a new one, do it the right way. Token cache should be a map storing genesisInfo by tokenId.

We can't store a map in localforage, so add helper functions to convert cashtabCache to and from JSON.

Update existing mocks for new shape.

Add new tests to confirm migration and expected behavior.

There is a lot going on in this diff but I do not think it can be split into steps. Perhaps we could migrate to the map before we move cashtabCache to cashtabState -- but it is the consistent organization of this value in cashtabState with standardized getters and setters that makes this migration practical. Perhaps we could move to cashtabState and then migrate -- but the existing validation functions and implementation are not functioning properly, and this could raise unrelated errors.

Big diff. But ,

  1. It's tested
  2. We get a lot of improvement
  3. This will make it easier to migrate to in-node chronik-client
Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
add-cache-to-cashtabstate
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 27528
Build 54619: Build Diffcashtab-tests
Build 54618: arc lint + arc unit

Event Timeline

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
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-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 rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
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 @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 sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec 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 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 21s

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

found 0 vulnerabilities

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

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

Attempted import error: 'defaultCashtabCache' is not exported from 'config/cashtabCache' (imported as 'defaultCashtabCache').


Build cashtab-tests failed with exit code 1

TODO

  • cache is currently broken, potentially worth a diff to just nuke it. At the same time, could just do that here.
  • token info in cache should be a map by tokenId and genesisInfo, stored at key tokens
  • migration function should handle this upgrade

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
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-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 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-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 abab@2.0.6: Use your platform's native atob() and btoa() methods 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 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 domexception@4.0.0: Use your platform's native DOMException instead
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0

added 1722 packages, and audited 1723 packages in 22s

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

found 0 vulnerabilities

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

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

Attempted import error: 'defaultCashtabCache' is not exported from 'config/cashtabCache' (imported as 'defaultCashtabCache').


Build cashtab-tests failed with exit code 1

lots of progress but still more to go

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
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-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 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 @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 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 abab@2.0.6: Use your platform's native atob() and btoa() methods 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.4.6 build
> node scripts/build.js

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

Attempted import error: 'defaultCashtabCache' is not exported from 'config/cashtabCache' (imported as 'defaultCashtabCache').


Build cashtab-tests failed with exit code 1

Working through bugs, tests pass

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

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

Attempted import error: 'returnGetTokenInfoChronikPromise' is not exported from 'chronik' (imported as 'returnGetTokenInfoChronikPromise').


Build cashtab-tests failed with exit code 1

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
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-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 abab@2.0.6: Use your platform's native atob() and btoa() methods 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-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 @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 sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec 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 22s

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

found 0 vulnerabilities

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

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

Attempted import error: 'returnGetTokenInfoChronikPromise' is not exported from 'chronik' (imported as 'returnGetTokenInfoChronikPromise').


Build cashtab-tests failed with exit code 1
bytesofman edited the test plan for this revision. (Show Details)

Document impacted chronik functions

Do not update token cache on websocket receives

add migration integration test

remove debug log, remove promise.all around one promise

bytesofman edited the summary of this revision. (Show Details)
bytesofman added inline comments.
cashtab/src/chronik/fixtures/chronikUtxos.js
6980 ↗(On Diff #45742)

tokenId is not in chronik-client's genesisInfo

We were adding it due to the difficulties of accessing object keys in the old non-map way of caching.

Now we need to remove from mocks for tests to work, as we do not expect it

12403 ↗(On Diff #45742)

we convert the old mocks for the tokens key of cashtabCache to the new Map format for use in existing unit tests

cashtab/src/chronik/index.js
242 ↗(On Diff #45742)

Add documentation to all undocumented functions as they are updated

In general, the way Cashtab manages data in these functions should be overhauled, and this will be relatively simple to do with existing integration tests. However, imo this should be done after migrating to in-node chronik-client, as we will then have a more stable data structure.

cashtab/src/components/Home/Home.js
147 ↗(On Diff #45742)

now all three of the params formerly used by this component are in cashtabState

cashtab/src/hooks/useWallet.js
235 ↗(On Diff #45742)

would be confusing that we always have to remember to convert this particular key to JSON before storing. this is why we are moving all state updates to this one function, updateCashtabState

emack requested changes to this revision.Feb 29 2024, 12:46
emack added a subscriber: emack.
  1. I can see the reasoning for getting rid of token ID is because it's not in chronik-client's genesisInfo, but why are the other properties grouped together in index [1]? Isn't it simpler to have them as individual props? Is there a performance/caching consideration here?

image.png (365×473 px, 41 KB)

  1. Do these console warnings upon load need to be sorted? I did some manual tests given how large this diff is and the potential for edge migration tests. Tested all ok though.

image.png (91×397 px, 13 KB)

image.png (138×386 px, 21 KB)

This revision now requires changes to proceed.Feb 29 2024, 12:46

I can see the reasoning for getting rid of token ID is because it's not in chronik-client's genesisInfo, but why are the other properties grouped together in index [1]? Isn't it simpler to have them as individual props? Is there a performance/caching consideration here?

You are looking at the JSON-stored version here when inspecting localforage. console.log the cashtabState version to see the Map.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map

So, they aren't really in index[1]. This is just how they appear in the keyvalue array we use to store the Map in JSON. In practice, they are all at the tokenId key, and are accessed by tokens.get(tokenId). We can check if they exist with tokens.has(tokenId). The use of a map allows us to get rid of all the "hey do we have this key in the giant object of keys that are tokenIds? also make sure before you add a new token that it isn't in Object.keys(giant cumbersome object)`

Do these console warnings upon load need to be sorted? I did some manual tests given how large this diff is and the potential for edge migration tests. Tested all ok though.

checking this out

Do these console warnings upon load need to be sorted? I did some manual tests given how large this diff is and the potential for edge migration tests. Tested all ok though.

these are both bugs. good catch, fixed.

patch proptypes and useeffect bugs, version bump, rebase on updated settings

re: the useEffect syntax, this is unrelated to this diff and fixed here: D15580

Failed tests logs:

====== CashTab Unit Tests: <App /> Setting "ABSOLUTE MINIMUM fees" settings will reduce fees to absolute min ======
TestingLibraryElementError: Unable to find an element by: [data-testid="settings-minFeeSends-switch"]

Ignored nodes: comments, script, style
<body>
  <div
    class="ant-notification ant-notification-topRight css-dev-only-do-not-override-1rqnfsa ant-notification-stack ant-notification-stack-expanded"
    style="right: 0px; top: 24px;"
  >
    <div
      class="ant-notification-notice-wrapper"
      style="transform: translate3d(0, 0, 0);"
    >
      <div
        class="ant-notification-notice ant-notification-notice-success ant-notification-notice-closable"
      >
        <div
          class="ant-notification-notice-content"
        >
          <div
            class="ant-notification-notice-with-icon"
            role="alert"
          >
            <span
              class="ant-notification-notice-icon"
            >
              <div
                class="ant-image css-dev-only-do-not-override-1rqnfsa"
                style="width: 24px; height: 24px;"
              >
                <img
                  class="ant-image-img css-dev-only-do-not-override-1rqnfsa"
                  height="24px"
                  src="test-file-stub"
                  style="height: 24px;"
                  width="24px"
                />
              </div>
            </span>
            <div
              class="ant-notification-notice-message"
            >
              Success
            </div>
            <div
              class="ant-notification-notice-description"
            >
              <a
                href="https://explorer.e.cash/tx/7eb806a83c48b0ab38b5af10aaa7452d4648f2c0d41975343ada9f4aa8255bd8"
                rel="noopener noreferrer"
                target="_blank"
              >
                Transaction successful. Click to view in block explorer.
              </a>
            </div>
          </div>
        </div>
        <a
          class="ant-notification-notice-close"
          tabindex="0"
        >
          <span
            class="ant-notification-notice-close-x"
          >
            <span
              aria-label="close"
              class="anticon anticon-close ant-notification-notice-close-icon"
              role="img"
            >
              <svg
                aria-hidden="true"
                data-icon="close"
                fill="currentColor"
                fill-rule="evenodd"
                focusable="false"
                height="1em"
                viewBox="64 64 896 896"
                width="1em"
              >
                <path
                  d="M799.86 166.31c.02 0 .04.02.08.06l57.69 57.7c.04.03.05.05.06.08a.12.12 0 010 .06c0 .03-.02.05-.06.09L569.93 512l287.7 287.7c.04.04.05.06.06.09a.12.12 0 010 .07c0 .02-.02.04-.06.08l-57.7 57.69c-.03.04-.05.05-.07.06a.12.12 0 01-.07 0c-.03 0-.05-.02-.09-.06L512 569.93l-287.7 287.7c-.04.04-.06.05-.09.06a.12.12 0 01-.07 0c-.02 0-.04-.02-.08-.06l-57.69-57.7c-.04-.03-.05-.05-.06-.07a.12.12 0 010-.07c0-.03.02-.05.06-.09L454.07 512l-287.7-287.7c-.04-.04-.05-.06-.06-.09a.12.12 0 010-.07c0-.02.02-.04.06-.08l57.7-57.69c.03-.04.05-.05.07-.06a.12.12 0 01.07 0c.03 0 .05.02.09.06L512 454.07l287.7-287.7c.04-.04.06-.05.09-.06a.12.12 0 01.07 0z"
                />
              </svg>
            </span>
          </span>
        </a>
      </div>
    </div>
  </div>
  <div>
    <div
      class="ant-spin-nested-loading css-dev-only-do-not-override-1rqnfsa"
    >
      <div>
        <div
          aria-busy="true"
          aria-live="polite"
          class="ant-spin ant-spin-spinning css-dev-only-do-not-override-1rqnfsa"
        >
          <span
            aria-label="loading"
            class="anticon anticon-loading anticon-spin cashLoadingIcon ant-spin-dot"
            role="img"
          >
            <svg
              aria-hidden="true"
              data-icon="loading"
              fill="currentColor"
              focusable="false"
              height="1em"
              viewBox="0 0 1024 1024"
              width="1em"
            >
              <path
                d="M988 548c-19.9 0-36-16.1-36-36 0-59.4-11.6-117-34.6-171.3a440.45 440.45 0 00-94.3-139.9 437.71 437.71 0 00-139.9-94.3C629 83.6 571.4 72 512 72c-19.9 0-36-16.1-36-36s16.1-36 36-36c69.1 0 136.2 13.5 199.3 40.3C772.3 66 827 103 874 150c47 47 83.9 101.8 109.7 162.7 26.7 63.1 40.2 130.2 40.2 199.3.1 19.9-16 36-35.9 36z"
              />
            </svg>
          </span>
        </div>
      </div>
      <div
        class="ant-spin-container ant-spin-blur"
      >
        <div
          class="sc-iBEsjs hYXNZl"
        >
          <div
            class="sc-RcBXQ krXqvz"
          >
            <div
              class="sc-iSDuPN cdWENt"
            >
              <div
                class="sc-chbbiW kmKhFr"
              >
                <div
                  class="sc-fZwumE jmbiQP"
                >
                  <img
                    alt="cashtab"
                    class="sc-fQejPQ iclGva"
                    src="test-file-stub"
                  />
                  <div
                    class="sc-jXQZqI jNpDSO"
                  [...
    at Object.getElementError (/work/cashtab/node_modules/@testing-library/dom/dist/config.js:37:19)
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:76:38
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:52:17
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:95:19
    at Object.getByTestId (/work/cashtab/src/components/__tests__/App.test.js:581:42)

Each failure log is accessible here:
CashTab Unit Tests: <App /> Setting "ABSOLUTE MINIMUM fees" settings will reduce fees to absolute min

update test mocks to use token instead of tx

cashtab/src/hooks/useWallet.js
947 ↗(On Diff #45758)

can test this by sending a token that is not in a wallet at https://cashtab-local-dev.netlify.app/, which has this diff deployed

This revision is now accepted and ready to land.Mar 1 2024, 11:48