Page MenuHomePhabricator
Differential D17916

[ecash-lib] Fix erroneously deleting EccDummy from tests
ClosedPublic
Actions

Authored by tobias_ruck on Wed, Apr 9, 19:55.

Details

Reviewers
bytesofman
Fabien
Group Reviewers
Restricted Project
Commits
rABC382cb8d58187: [ecash-lib] Fix erroneously deleting EccDummy from tests
Summary

Diff D17640 introduced a subtle and undetected bug: It ran the tx size estimation within tests using real signatures instead of EccDummy. However, the "leftover" calculation always uses EccDummy.

Since Schnorr signatures are fixed length, this was undetected. But for ECDSA signatures, the tests fail, as "real" signatures can have a different length than dummy signatures used for leftover calculation.

This diff restores the old behavoir, and additionally tests size estimation using ECDSA signatures to ensure we cover behavoir this in our tests.

Test Plan

npm run integration-tests

Diff Detail

Event Timeline

tobias_ruck created this revision.Wed, Apr 9, 19:55
Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Apr 9, 19:55
tobias_ruck edited the summary of this revision. (Show Details)Wed, Apr 9, 19:56
Harbormaster completed remote builds in B32932: Diff 53434.Wed, Apr 9, 20:00
teamcity added a comment.Wed, Apr 9, 20:00
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <Token /> available actions rendered We can list an SLP1 fungible token ======
TestingLibraryElementError: Unable to find an accessible element with the role "button" and name `/List Vespene Gas/`

Here are the accessible roles:

  region:

  Name "Notifications Alt+T":
  <section
    aria-atomic="false"
    aria-label="Notifications Alt+T"
    aria-live="polite"
    aria-relevant="additions text"
    class="Toastify"
  />

  --------------------------------------------------
  combobox:

  Name "":
  <select
    class="sc-crNyjn eVzJws"
    data-testid="wallet-select"
    id="wallets"
    name="wallets"
  />

  --------------------------------------------------
  option:

  Name "Token Test":
  <option
    class="sc-cpHetk jodErs"
    value="Token Test"
  />

  --------------------------------------------------
  button:

  Name "Copy ecash:qqq9f9z3uhpzkxrgdjkd7dxuuey7tmpmugpmnw0kue":
  <button
    aria-label="Copy ecash:qqq9f9z3uhpzkxrgdjkd7dxuuey7tmpmugpmnw0kue"
    class="sc-cJSrbW kZNvLw"
  />

  Name "icon for 20a0b9337a78603c6681ed2bc541593375535dcd9979196620ce71f233f2f6f8":
  <button
    class="sc-hzNEM eBXvVM"
  />

  Name "Click for more info about this token type":
  <button
    aria-label="Click for more info about this token type"
    class="sc-cJSrbW kZNvLw"
  />

  Name "Copy Token ID":
  <button
    aria-label="Copy Token ID"
    class="sc-cJSrbW kZNvLw"
  />

  Name "Click for more info about agora partial sales":
  <button
    aria-label="Click for more info about agora partial sales"
    class="sc-cJSrbW kZNvLw"
  />

  Name "Transactions":
  <button
    class="sc-hycgNl cDcbOV"
  />

  Name "Send Screen":
  <button
    aria-label="Send Screen"
    class="sc-hycgNl cDcbOV"
  />

  Name "Tokens":
  <button
    aria-label="Tokens"
    class="sc-hycgNl cDcbOV"
  />

  Name "Receive":
  <button
    aria-label="Receive"
    class="sc-hycgNl cDcbOV"
  />

  Name "Wallet Backup":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Wallets":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Listed NFTs":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Agora":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Contacts":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Airdrop":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Rewards":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Sign & Verify":
  <button
    class="sc-chAAoq fqJHzP"
  />

  Name "Settings":
  <button
    class="sc-chAAoq fqJHzP"
  />

  --------------------------------------------------
  link:

  Name "XECX":
  <a
    href="#/token/c67bf5c2b6d91cfb46a5c1772582eff80d88686887be10aa63b0945479cf4ed4"
  />

  Name "FIRMA":
  <a
    href="#/token/0387947fd575db4fb19a3e322f635dec37fd192b5941625b66bc4b2c3008cbf0"
  />

  Name "20a...6f8":
  <a
    href="https://explorer.e.cash/tx/20a0b9337a78603c6681ed2bc541593375535dcd9979196620ce71f233f2f6f8"
    rel="noopener noreferrer"
    target="_blank"
  />

  Name "simple.wikipedia.org/wiki/StarCraft#Gameplay":
  <a
    href="https://simple.wikipedia.org/wiki/StarCraft#Gameplay"
    rel="noreferrer"
    target="_blank"
  />

  --------------------------------------------------
  paragraph:

  Name "Price in Local Currency":
  <p
    class="sc-AnqlK dBmDGi"
    title="Price in Local Currency"
  />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  Name "":
  <p />

  --------------------------------------------------
  img:

  Name "icon for 20a0b9337a78603c6681ed2bc541593375535dcd9979196620ce71f233f2f6f8":
  <img
    alt="icon for 20a0b9337a78603c6681ed2bc541593375535dcd9979196620ce71f233f2f6f8"
    height="128"
    src="https://icons.etokens.cash/128/20a0b9337a78603c6681ed2bc541593375535dcd9979196620ce71f233f2f6f8.png"
    width="128"
  />

  Name "cashtab":
  <img
    alt="cashtab"
    class="sc-cCbXAZ jOUSPN"
    src="test-file-stub"
  />

  --------------------------------------------------
  heading:

  Name "Vespene Gas":
  <h2 />

  --------------------------------------------------

Ignored nodes: comments, script, style
<body>
  <div>
    <div
      class="sc-keFjpB fuvsSf"
    >
      <section
        aria-atomic="false"
        aria-label="Notifications Alt+T"
        aria-live="polite"
        aria-relevant="additions text"
        class="Toastify"
      />
      <div
        class="sc-jWojfa gOMhPW"
      >
        <div
          class="sc-kVrTmx dSIcDd"
        >
          <div
            class="sc-ivVeuv hQBWoJ"
          >
            <img
              alt="cashtab"
              class="sc-cCbXAZ jOUSPN"
              src="test-file-stub"
            />
          </div>
          <div
            class="sc-cmIlrE ckvYmi"
          >
            <div
              class="sc-jkCMRl gxKbjs"
            >
              <select
                class="sc-crNyjn eVzJws"
                data-testid="wallet-select"
                id="wallets"
                name="wallets"
              >
                <option
                  class="sc-cpHetk jodErs"
                  value="Token Test"
                >
                  Token Test
                </option>
              </select>
              <div
                class="sc-bqjOQT kauMuv"
              >
                <button
                  aria-label="Copy ecash:qqq9f9z3uhpzkxrgdjkd7dxuuey7tmpmugpmnw0kue"
                  class="sc-cJSrbW kZNvLw"
                >
                  <svg
                    title="copy-paste"
                  />
                </button>
                <div
                  class="sc-eerKOB cvXwwv"
                >
                  <div
                    class="sc-emmjRN bSWkUh"
                  >
                    <input
                      checked=""
                      class="sc-gFaPwZ iPJMNd"
                      id="show-hide-balance"
                      name="show-hide-balance"
                      title="show-hide-balance"
                      type="checkbox"
                    />
                    <label
                      class="sc-cpmLhU iUeRKn"
                      for="show-hide-balance"
                    >
                      <span
                        class="sc-dymIpo fPSvEF"
                        data-off=""
                        data-on=""
                      />
                      <span
                        class="sc-bnXvFD cpOMDW"
                      />
                    </label>
                  </div>
                </div>
              </div>
            </div>
            <div
              class="sc-gleUXh fSfkBQ"
              title="Wallet Info"
            >
              <div
                class="sc-nrwXf HaEYz"
              >
                <div
                  class="sc-eitiEO isWDMZ"
                  title="Balance XEC"
                >
                  9,970.81
                   
                  XEC
                </div>
                <div
                  class="sc-eitiEO gaHDhC"
                  title="Balance XECX"
                >
                  10,000.00
                   
                  <a
                    href="#/token/c67bf5c2b6d91cfb46a5c1772582eff80d88686887be10aa63b0945479cf4ed4"
                  >
                    XECX
                  </a>
                </div>
                <div
                  class="sc-eitiEO gaHDhC"
                  title="Balance FIRMA"
                >
                  100.0000
                   
                  <a
                    href="#/token/0387947fd575db4fb19a3e322f635dec37fd192b5941625b66bc4b2c3008cbf0"
                  >
                    FIRMA
                  </a>
                </div>
              </div>
              <div
                class="sc-bhlBdH gcIcEt"
                title="Balance in Local Currency"
              >
                $
                100.60
                 
                USD
              </div>
              <p
                class="sc-AnqlK dBmDGi"
                title="Price in Local Currency"
              >
                1 
                XEC
                 = 
                0.00003000
                 
                USD
              </p>
            </div>
          </div>
          <div
            class="sc-dTLGrV eQoClh"
          >
            <div
              class="sc-jKVCRD kVWwbS"
            >
              <div
                class="sc-fcdeBU chTKnN"
                title="Token Stats"
              >
                <div
                  class="sc-RcBXQ kXyqpW"
                >
                  <button
                    class="sc-hzNEM eBXvVM"
                  >
                    <img
                      alt="icon for 20a0b9337a78603c6681ed2bc541593375535dcd9979196620ce71f233f2f6f8"
                      height="128"
      ...
    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.getByRole (/work/cashtab/src/components/Etokens/__tests__/TokenActions.test.js:272:35)

Each failure log is accessible here:
CashTab Unit Tests: <Token /> available actions rendered We can list an SLP1 fungible token

tobias_ruck updated this revision to Diff 53473.Mon, Apr 14, 14:15

rerun CI

Harbormaster completed remote builds in B32958: Diff 53473.Mon, Apr 14, 14:24
tobias_ruck updated this revision to Diff 53474.Mon, Apr 14, 14:45

increase test dummy sig coverage using ecdsaSign

Harbormaster completed remote builds in B32959: Diff 53474.Mon, Apr 14, 14:52
tobias_ruck published this revision for review.Mon, Apr 14, 16:27
tobias_ruck edited the summary of this revision. (Show Details)
bytesofman accepted this revision.Mon, Apr 14, 16:42

But for ECDSA signatures, the tests fail, as "real" signatures can have a different length than dummy signatures used for leftover calculation.

Does this mean we can't accurately estimate tx size of ECDSA -signed txs with dummy signatures?

This revision is now accepted and ready to land.Mon, Apr 14, 16:42
Fabien added a subscriber: Fabien.Mon, Apr 14, 18:30
In D17916#408501, @bytesofman wrote:

But for ECDSA signatures, the tests fail, as "real" signatures can have a different length than dummy signatures used for leftover calculation.

Does this mean we can't accurately estimate tx size of ECDSA -signed txs with dummy signatures?

ECDSA signatures are DER encoded, and will vary in size. Usually they are 71-73 bytes but occasionally be can smaller than that.

Fabien accepted this revision.Mon, Apr 14, 18:33
Closed by commit rABC382cb8d58187: [ecash-lib] Fix erroneously deleting EccDummy from tests (authored by tobias_ruck). · Explain WhyThu, Apr 17, 13:53
This revision was automatically updated to reflect the committed changes.
tobias_ruck added a commit: rABC382cb8d58187: [ecash-lib] Fix erroneously deleting EccDummy from tests.

Revision Contents
Changeset List

PathSize
modules/
ecash-lib/
tests/
48 lines

Diff 53434

View Options

modules/ecash-lib/tests/txBuilder.test.ts

Loading...