Page MenuHomePhabricator
Differential D15676

[Cashtab] Consolidate send xec validation functions
ClosedPublic
Actions

Authored by bytesofman on Mar 11 2024, 17:22.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABC996e154ec774: [Cashtab] Consolidate send xec validation functions
Summary

Cashtab send xec validation is ambiguous. We have one function, shouldRejectAmountInput, that specifically validates the user input of the SendXec field. We have another function, isValidXecSendAmount, that validates XEC send amounts for multi-send or param passing input. The rules in both cases are the same, so we should have one function.

Consolidate to a single function, isValidXecSendAmount, and implement everywhere we validate xec send amounts. Implement the new fiatToSatoshis function to replace legacy fiatToCrypto, a dependency of legacy validation approach.

Test Plan

npm test

Diff Detail

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

Event Timeline

bytesofman created this revision.Mar 11 2024, 17:22
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 11 2024, 17:22
Harbormaster completed remote builds in B27795: Diff 46098.Mar 11 2024, 17:29
bytesofman published this revision for review.Mar 11 2024, 17:30
PiRK accepted this revision.EditedMar 12 2024, 14:20
PiRK added a subscriber: PiRK.

Having the function return a True or a truthy string feels a bit weird and kind of violates the principle of least astonishment imo. But I can't really think of a more concise way of doing, and you documented the behavior in the doxygen comment.

No strong opinion on this, but extracting the error via an empty object passed as a param feels a little bit more familiar / less surprising to me, but maybe that's my C++ bias.
https://stackoverflow.com/a/3175838/4494781

var error = {};
if (!isLegal("something", error)) {
    alert(error.val);
}

On the other hand, this makes the isValidMultiSendUserInput and isValidXecSendAmount functions consistent, so that's a good point.

This revision is now accepted and ready to land.Mar 12 2024, 14:20
Closed by commit rABC996e154ec774: [Cashtab] Consolidate send xec validation functions (authored by bytesofman). · Explain WhyMar 12 2024, 14:30
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC996e154ec774: [Cashtab] Consolidate send xec validation functions.

Revision Contents
Changeset List

PathSize
cashtab/
4 lines
2 lines
src/
components/
Common/
2 lines
Send/
12 lines
44 lines
__tests__/
56 lines
validation/
__tests__/
462 lines
fixtures/
351 lines
194 lines

Diff 46131

View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/components/Common/EnhancedInputs.js

Loading...
View Options

cashtab/src/components/Send/SendToken.js

Loading...
View Options

cashtab/src/components/Send/SendXec.js

Loading...
View Options

cashtab/src/components/Send/__tests__/SendXec.test.js

Loading...
View Options

cashtab/src/validation/__tests__/index.test.js

Loading...
View Options

cashtab/src/validation/fixtures/vectors.js

Loading...
View Options

cashtab/src/validation/index.js

Loading...