Details

Reviewers

majcosta
alcipir
deadalnix

Group Reviewers

Restricted Project

Commits

rABC6b87d08fce28: Re-enable SLPA transactions in CashTab and fix notification bug

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

Create an SLPA token with 9 decimal places or ask me for some.
npm start
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

bytesofman created this revision.Dec 4 2020, 00:01

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 4 2020, 00:01

bytesofman requested review of this revision.Dec 4 2020, 00:01

Harbormaster completed remote builds in B14301: Diff 26182.Dec 4 2020, 00:01

Replace SLP with currency.tokenTicker (SLPA)

Harbormaster completed remote builds in B14302: Diff 26183.Dec 4 2020, 00:04

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

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

alcipir added inline comments.Dec 4 2020, 01:21

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:

alcipir added inline comments.Dec 4 2020, 01:30

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

Harbormaster completed remote builds in B14306: Diff 26187.Dec 4 2020, 03:54

Removing more comments

Harbormaster completed remote builds in B14307: Diff 26188.Dec 4 2020, 04:16

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.

alcipir accepted this revision.Dec 5 2020, 03:35

Renamed

Harbormaster completed remote builds in B14407: Diff 26371.Dec 14 2020, 19:00

deadalnix accepted this revision.Dec 14 2020, 21:43

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

Closed by commit rABC6b87d08fce28: Re-enable SLPA transactions in CashTab and fix notification bug (authored by Joey King <joseph.roy.king@gmail.com>). · Explain WhyDec 14 2020, 21:45

This revision was automatically updated to reflect the committed changes.

Joey King <joseph.roy.king@gmail.com> added a commit: rABC6b87d08fce28: Re-enable SLPA transactions in CashTab and fix notification bug.