Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC6c5a3dd84f6a: [Cashtab] [Chronik] [Tx Gen] Parse XEC send value

Summary

As per T2447, this is part of a series of diffs to refactor the transaction generation process.

This diff is a standalone utility function which parses parameters for both one to one and one to many send XEC transactions, aggregates the total send value (in case of one to many) and then checks whether the final send value is above dust.

This diff does not change any existing functionality within the codebase. Once all the utility functions are accepted, then there'll be a final set of diffs to refactor the existing sendXec() function which is getting quite large and cumbersome to maintain.

Test Plan

npm start
npm test
observe the successful execution of new useBCH.test.js unit tests

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emack created this revision.Jun 13 2022, 11:05

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 13 2022, 11:05

emack requested review of this revision.Jun 13 2022, 11:05

Harbormaster completed remote builds in B19375: Diff 33988.Jun 13 2022, 11:09

Could you add a pseudocode description of how you would like the final sendXec() function to work, including each component function and a short description of its inputs and outputs? We have a skeleton in T2447 but I think this should really be its own task. We might come back and edit it a bit as these diffs land and we see more about the best way to get this done.

I'd like to see the context more fleshed out before approving this initial step.

This revision now requires changes to proceed.Jun 13 2022, 22:13

I'd like to see the context more fleshed out before approving this initial step.

See T2503 for overarching context on this refactor initiative

bytesofman requested changes to this revision.Jun 14 2022, 22:47

bytesofman added inline comments.

web/cashtab/src/hooks/__tests__/useBCH.test.js
695 ↗	(On Diff #33988)	We should be able to capture the specific error msg in the unit tests, as we'll want to throw a notification with the specific error msg in the app (not just `console.log` it)
700 ↗	(On Diff #33988)	See above
705 ↗	(On Diff #33988)	Return a specific error msg
710 ↗	(On Diff #33988)	Return a specific error msg
web/cashtab/src/hooks/useBCH.js
1471 ↗	(On Diff #33988)	Situation here where we will have to check for a `null` return value after calling this function. Also, Cashtab doesn't know what caused this error -- only a user reading the `console.log` statement would be able to figure it out. Whatever error causes `sendXec()` to fail should be thrown in a notification. So, we should be capturing the specific errors thrown in this function for the unit test failures.

This revision now requires changes to proceed.Jun 14 2022, 22:47

Throwing specific errors and verified via unit tests

Harbormaster completed remote builds in B19393: Diff 34029.Jun 15 2022, 13:04

bytesofman requested changes to this revision.Jun 15 2022, 23:39

bytesofman added inline comments.

web/cashtab/src/hooks/__tests__/useBCH.test.js
678 ↗	(On Diff #34029)	The parameter `bchValue`, used to pass the single send XEC amount in `Send.js`, is a string. It's okay if this function also works taking a number input, but the unit test should match the app -- i.e. `parseXecSendValue(false, '550', null)...`

This revision now requires changes to proceed.Jun 15 2022, 23:39

bytesofman mentioned this in D11626: [Cashtab] [Chronik] [Tx Gen] Generate encoded OP_RETURN script.Jun 16 2022, 23:29

Changed single XEC input from int to str

Harbormaster completed remote builds in B19397: Diff 34034.Jun 17 2022, 02:39

Pulling down this diff, there is a REMOVE.husky folder.

Make sure that the end result of this diff shows no changes to the husky files.

This revision now requires changes to proceed.Jun 17 2022, 17:01

Removed redundant husky folder

Harbormaster completed remote builds in B19412: Diff 34049.Jun 18 2022, 06:38

Updated husky pre-commit

Make sure that the end result of this diff shows no changes to the husky files.

Done - ready for final review

Harbormaster completed remote builds in B19413: Diff 34050.Jun 18 2022, 06:45

bytesofman requested changes to this revision.Jun 20 2022, 17:27

bytesofman added inline comments.

web/cashtab/src/hooks/useBCH.js
1455 ↗	(On Diff #34050)	dust test can be applied only once to final `value`, instead of applying it two times to `value` in both parts of the `if...else` block