Page MenuHomePhabricator

[Cashtab] Extend Cashtab bip21 support to cover multiple addresses and amounts
ClosedPublic

Authored by bytesofman on Mon, Sep 16, 23:38.

Details

Summary

bip21 (extended for ecash) supports multiple addresses and amounts. However Cashtab does not yet support this functionality.

Implement in Cashtab according to the spec (ref: https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/doc/standards/bip21-ecash-additions.md)

Test Plan

npm test

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

go to the send screen and try some bip21 query strings with multiple addrs and amounts

Diff Detail

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

Event Timeline

Failed tests logs:

====== CashTab Unit Tests: <SendXec /> rendered with params in URL bip21 param - an invalid bip21 param shows validation errors but cannot be changed ======
Error: expect(element).toHaveValue(ecash:qp33mh3a7qq7p8yulhnvwty2uq5ynukqcvuxmvzfhm?amount=17&op_return_raw=04007461622263617368746162206d6573736167652077697468206f705f72657475726e5f726177&op_return_raw=04007461622263617368746162206d6573736167652077697468206f705f72657475726e5f726177)

Expected the element to have value:
  ecash:qp33mh3a7qq7p8yulhnvwty2uq5ynukqcvuxmvzfhm?amount=17&op_return_raw=04007461622263617368746162206d6573736167652077697468206f705f72657475726e5f726177&op_return_raw=04007461622263617368746162206d6573736167652077697468206f705f72657475726e5f726177
Received:


Ignored nodes: comments, script, style
<html>
  <head />
  <body>
    <div>
      <div
        class="sc-jxGEyO eNXSvw"
      >
        <div
          class="Toastify"
        />
        <div
          class="sc-gacfCG eIXoZm"
        >
          <div
            class="sc-dEfkYy fjnRqt"
          >
            <div
              class="sc-FQuPU hyajsP"
            >
              <img
                alt="cashtab"
                class="sc-iuDHTM ivXaFh"
                src="test-file-stub"
              />
            </div>
            <div
              class="sc-kNBZmU dLCRIc"
            >
              <select
                class="sc-eopZyb epDJYH"
                data-testid="wallet-select"
                id="wallets"
                name="wallets"
              >
                <option
                  class="sc-eNNmBn gXmKZn"
                  value="Transaction Fixtures"
                >
                  Transaction Fixtures
                </option>
              </select>
              <div
                class="sc-jRuhRL jgRaQB"
              >
                <button
                  aria-label="Copy ecash:qqa9lv3kjd8vq7952p7rq0f6lkpqvlu0cydvxtd70g"
                  class="sc-iQNlJl dQNUoI"
                >
                  <svg
                    title="copy-paste"
                  />
                </button>
                <div
                  class="sc-cpmLhU dyeBXy"
                >
                  <div
                    class="sc-dymIpo gyYfmt"
                  >
                    <input
                      checked=""
                      class="sc-jzgbtB jgfSAg"
                      id="show-hide-balance"
                      name="show-hide-balance"
                      title="show-hide-balance"
                      type="checkbox"
                    />
                    <label
                      class="sc-bnXvFD hBMwJl"
                      for="show-hide-balance"
                    >
                      <span
                        class="sc-gFaPwZ jegjwl"
                        data-off=""
                        data-on=""
                      />
                      <span
                        class="sc-fhYwyz jdgzKk"
                      />
                    </label>
                  </div>
                </div>
              </div>
            </div>
            <div
              class="sc-fihHvN icIKMi"
              title="Wallet Info"
            >
              <div
                class="sc-RbTVP dRTrHs"
                title="Balance in XEC"
              >
                9,513.12
                 
                XEC
                 
              </div>
              <div
                class="sc-hMrMfs ZvGPi"
                title="Balance in Local Currency"
              >
                $
                0.29
                 
                USD
              </div>
              <p
                class="sc-drlKqa dpNjkh"
                title="Price in Local Currency"
              >
                1 
                XEC
                 = 
                0.00003000
                 
                USD
              </p>
            </div>
            <div
              class="sc-erNlkL hlFboo"
            >
              <div
                class="sc-VJcYb Uszwj"
              >
                <div
                  class="sc-cpmLhU dyeBXy"
                >
                  <div
                    class="sc-dymIpo gYUTVy"
                  >
                    <input
                      class="sc-jzgbtB jgfSAg"
                      disabled=""
                      id="Toggle Multisend"
                      name="Toggle Multisend"
                      title="Toggle Multisend"
                      type="checkbox"
                    />
                    <label
                      class="sc-bnXvFD jrCFus"
                      disabled=""
                      for="Toggle Multisend"
                    >
                      <span
                        class="sc-gFaPwZ hktLQl"
                        data-off="Send to one"
                        data-on="Send to many"
                      />
                      <span
                        class="sc-fhYwyz cnLmNg"
                      />
                    </label>
                  </div>
                </div>
              </div>
              <div
                class="sc-jtggT cVUzSi"
              >
                <div
                  class="sc-ipZHIp jSDPgp"
                >
                  <div
                    class="sc-bmyXtO fetwxd"
                  >
                    <div
                      class="sc-dEoRIm bvGLHm"
                    >
                      <div
                     ...
    at toHaveValue (/work/cashtab/src/components/Send/__tests__/SendByUrlParams.test.js:669:52)
    at runWithExpensiveErrorDiagnosticsDisabled (/work/cashtab/node_modules/@testing-library/dom/dist/config.js:47:12)
    at checkCallback (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:124:77)
    at checkRealTimersCallback (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:118:16)
    at Timeout.task [as _onTimeout] (/work/cashtab/node_modules/jsdom/lib/jsdom/browser/Window.js:520:19)
    at listOnTimeout (node:internal/timers:581:17)
    at processTimers (node:internal/timers:519:7)

Each failure log is accessible here:
CashTab Unit Tests: <SendXec /> rendered with params in URL bip21 param - an invalid bip21 param shows validation errors but cannot be changed

complete implementation, add unit tests and integration tests, bump extension version number

back out unrelated changes, improve comments

bytesofman added inline comments.
cashtab/extension/public/manifest.json
5 ↗(On Diff #49695)

we want to update the extension when this lands, as ecashchat wants to use this feature

cashtab/src/components/Send/SendXec.js
966 ↗(On Diff #49695)

If we have multiple outputs from a bip21 query string, we do not render the "amount" field

Instead, we render a summary of the outputs and the total amount sent

Then, at the bottom of the screen before the "Send" button, we render a list of the actual addresses and amounts the tx will send.

This is a tough one to get right with UX. I can see improvements from here. Mb some that could go in this diff but I think it's such a weird problem that mb best left to it's own diff. For example I think the easiest improvement is to render all the outputs and amounts in this space -- however we can't do this trivially because we need to change how the toggle css works in swapping between single and multi send modes. I think cramming that into this diff is too much.

Ideal UX, I think we get rid of the switch for "multi-send". However, the multisend text area is the easiest way to make a multi-output tx manually. It does have users.

bip21 is mostly to support app txs.

cashtab/src/validation/index.js
659 ↗(On Diff #49695)

this is some function now

pretty complicated. development here is primarily test driven.

we have incrementally added several features. mb it should be refactored from the ground-up, however, the test cases are exhaustive, easy-to-maintain, and cover the UX.

This function could be simpler, but not simple. Cashtab supports a wide variety of user input.

emack requested changes to this revision.EditedWed, Sep 18, 02:34
emack added a subscriber: emack.

Desktop and mobile tested all ok via BIP21 url querystring, with different amounts, decimals and addresses

image.png (782×752 px, 99 KB)

Tried to test Extension via eCashChat UAT instance (that's sending the multisend BIP21 query) but that doesn't seem to reflect this diff? I built it locally via npm run extension then reloaded the local chrome extension and verified it to be v3.43.0.

image.png (991×457 px, 72 KB)

This revision now requires changes to proceed.Wed, Sep 18, 02:34

Following chat on tg, issue was on ecc app side. Extension now testing all ok.

image.png (994×498 px, 71 KB)

This revision is now accepted and ready to land.Wed, Sep 18, 04:52