Page MenuHomePhabricator

Re-enable SLPA transactions in CashTab and fix notification bug
ClosedPublic

Authored by bytesofman on Dec 4 2020, 00:01.

Details

Summary

Updated bch-api instance at rest.kingbch.com/v3/ version of bch-api to correct BigNumber issue in hydrateUtxos method.
Re-implemented SLPA txs in CashTab. Also patched BigNumber issue for received SLP txns notifications.

Test Plan
  1. Create an SLPA token with 9 decimal places or ask me for some.
  2. npm start
  3. Send a lot of SLPA tokens back and forth, making sure to check the qty of 0.000000001

Diff Detail

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

Event Timeline

Replace SLP with currency.tokenTicker (SLPA)

deadalnix requested changes to this revision.Dec 4 2020, 01:12
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
web/cashtab/src/components/Send/SendToken.js
7 ↗(On Diff #26183)

Kind of out of scope, but I would strong recommend using relative path in imports as it makes refactoring significantly more difficult.

web/cashtab/src/hooks/useBCH.js
310 ↗(On Diff #26183)

Remove

422 ↗(On Diff #26183)

This is dead code, also a typical instance of the if/return/else pattern.

This revision now requires changes to proceed.Dec 4 2020, 01:12
alcipir requested changes to this revision.Dec 4 2020, 01:18

Warning still showing up telling the user that sending SLPA is disabled.

web/cashtab/src/components/Send/SendToken.js
7 ↗(On Diff #26183)

I believe you meant you strong recommend _against_ using relative path. We could switch to @ aliases: https://arunmichaeldsouza.com/blog/aliasing-module-paths-in-node-js

If I try to send SLPA tokens without BCHA I get this error:

image.png (240×667 px, 22 KB)

web/cashtab/src/hooks/useBCH.js
473–474 ↗(On Diff #26183)

I didn't understand this commented code, what is it for? Since we have git can we get rid of it and keep track of these TODOs elsewhere?

  • Remove debug param and commenting
  • Handle error if user attempts to send SLPA with no BCHA
  • Remove SLPA warning and disabled warning
  • Other corrections requested in comments
deadalnix requested changes to this revision.Dec 4 2020, 16:38

Can you change the diff title? These "Patching foo" is completely redundant. Yes, this is a patch. That tell us nothing about what this does. Failure to be specific is already having an impact on this diff, asyou are throwing in various other stuff that are "geographically" related, as in they happen in the same area of the code, but are semantically unrelated.

This revision now requires changes to proceed.Dec 4 2020, 16:38
alcipir retitled this revision from Patching BigNumber issues to Re-enable SLPA transactions in CashTab and fix notification bug.Dec 5 2020, 03:28

Edited the title and did more extensive tests.
Working fine.

This revision is now accepted and ready to land.Dec 14 2020, 21:43