Page MenuHomePhabricator

[Cashtab] create adjustTokenQtyForDecimals function
AbandonedPublic

Authored by kieran709 on Oct 4 2022, 13:58.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

Related to T2690. Created a function, adjustTokenQtyForDecimals, which handles adjusting the token quantity for decimals throughout the app. Replaced logic throughout the app that adjusted token quantity for decimals with the new function, and added unit tests.

Test Plan

cd web/cashtab && npm start
npm test - all tests pass

Using the same wallet on live site, check token balances
in dev env, check that the token balances are the same as live site

in a separate browser & wallet, send token tx to the wallet,
observe that the amount received is the same in both the dev env and the live app

send a tx from the dev env
on live site, check that the amount sent is the same as in the dev env

Diff Detail

Repository
rABC Bitcoin ABC
Branch
adjust-token-decimals-function
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 20405
Build 40483: Build Diffcashtab-tests
Build 40482: arc lint + arc unit

Event Timeline

bytesofman requested changes to this revision.Oct 4 2022, 16:21
  • tokenDecimals should never be a BigNumber
  • the function should accept a string or BigNumber for token quantity
  • don't implement anywhere, just create the function and its unit tests
web/cashtab/src/hooks/useWallet.js
905 ↗(On Diff #35401)

for now, do not implement the function. Only create it and unit tests.

the chronik tx history stack refactors this part of code so we'll need to implement later

web/cashtab/src/utils/cashMethods.js
969 ↗(On Diff #35401)

the input params here should be tokenQty and tokenDecimals

983 ↗(On Diff #35401)

tokenDecimals should specifically be a number between 0 and 9, inclusive

985 ↗(On Diff #35401)

tokenBalance, which should be renamed tokenQty as this function will sometimes be used to parse sent or received amounts, should never be a number. Only a string or BigNumber

web/cashtab/src/utils/chronik.js
322 ↗(On Diff #35401)

for now, do not implement the function. Only create it and unit tests.

This revision now requires changes to proceed.Oct 4 2022, 16:21

Responded to review feedback, added try/catch and new TypeError to catch if invalid params were provided to the function.

bytesofman requested changes to this revision.Oct 7 2022, 18:07

rebase to latest master, getting some weird things here, like the whole parseChronikTx function back in cashMethods.js

This revision now requires changes to proceed.Oct 7 2022, 18:07

Rebased and added test comment

bytesofman requested changes to this revision.Oct 7 2022, 21:22
bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
835 ↗(On Diff #35533)

still seeing this whole function come back in cashMethods.js

Might need to abandon this and try again

This revision now requires changes to proceed.Oct 7 2022, 21:22