Page MenuHomePhabricator
Differential D16769

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

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

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC3b47bfb6db78: [Cashtab] Extend Cashtab bip21 support to cover multiple addresses and amounts
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
Branch
cashtab-bip21-multi-addr
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30339
Build 60204: Build Diffcashtab-tests · ecash-lib-tests
Build 60203: arc lint + arc unit

Event Timeline

bytesofman created this revision.Sep 16 2024, 23:38
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 16 2024, 23:38
Harbormaster completed remote builds in B30313: Diff 49664.Sep 16 2024, 23:38
teamcity added a comment.Sep 16 2024, 23:41
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

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

bytesofman updated this revision to Diff 49694.Sep 17 2024, 21:10

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

Harbormaster completed remote builds in B30333: Diff 49694.Sep 17 2024, 21:15
bytesofman updated this revision to Diff 49695.Sep 17 2024, 21:16

back out unrelated changes, improve comments

bytesofman edited the test plan for this revision. (Show Details)Sep 17 2024, 21:18
Harbormaster completed remote builds in B30334: Diff 49695.Sep 17 2024, 21:20
bytesofman published this revision for review.Sep 17 2024, 21:24
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.EditedSep 18 2024, 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.Sep 18 2024, 02:34
emack accepted this revision.Sep 18 2024, 04:52

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.Sep 18 2024, 04:52
bytesofman updated this revision to Diff 49702.Sep 18 2024, 05:00

rebase, squash

Harbormaster completed remote builds in B30339: Diff 49702.Sep 18 2024, 05:04
Closed by commit rABC3b47bfb6db78: [Cashtab] Extend Cashtab bip21 support to cover multiple addresses and amounts (authored by bytesofman). · Explain WhySep 18 2024, 05:04
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC3b47bfb6db78: [Cashtab] Extend Cashtab bip21 support to cover multiple addresses and amounts.

Revision Contents
Changeset List

PathSize
cashtab/
extension/
public/
2 lines
4 lines
2 lines
src/
components/
Send/
229 lines
__tests__/
131 lines
148 lines
validation/
fixtures/
176 lines
128 lines

Diff 49702

View Options

cashtab/extension/public/manifest.json

Loading...
View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/components/Send/SendXec.js

Loading...
View Options

cashtab/src/components/Send/__tests__/SendByUrlParams.test.js

Loading...
View Options

cashtab/src/components/Send/__tests__/SendXec.test.js

Loading...
View Options

cashtab/src/validation/fixtures/vectors.js

Loading...
View Options

cashtab/src/validation/index.js

Loading...