Page MenuHomePhabricator

[Cashtab] Improve multisend validation
ClosedPublic

Authored by bytesofman on Dec 13 2023, 18:36.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABCe9366b58c957: [Cashtab] Improve multisend validation
Summary

Improve validation functions supporting xec multisend.

  • Do not .split() more than once
  • Do not accept decimal markers other than '.'
  • Throw error if more than one comma per line in expected csv input
  • Do not accept non-digit characters for value input (except '.')

Note: I did not add a check against very large values, as it is still safe in JS to multiple the total supply of XEC by 100. So, I think it's overkill to check for this every time.

Test Plan

npm test

Diff Detail

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

Event Timeline

all of the snapshots is a bit much even though jest recommends including them. In this diff -- you can see that a css class has changed, which is consistent with changing a css rule on a shared component.

cashtab/src/components/Common/EnhancedInputs.js
115 ↗(On Diff #43608)

UX goal here is for all error msgs to fit on one line

PiRK added a subscriber: PiRK.
PiRK added inline comments.
cashtab/src/utils/validation.js
505–519 ↗(On Diff #43609)

Depending on how much you like complicated regular expressions, it could be possible to do both checks in one regex: https://stackoverflow.com/a/12643073/4494781

^([0-9]+([.][0-9]*)?|[.][0-9]+)$
>>> import re
>>> rg = "^([0-9]+([.][0-9]*)?|[.][0-9]+)$"
>>>
>>> re.match(rg, "123a34") != None
False
>>> re.match(rg, "123.34") != None
True
>>> re.match(rg, ".34") != None
True
>>> re.match(rg, "123.") != None
True
>>> re.match(rg, "123.1.2") != None
False
>>> re.match(rg, "123") != None
True

or if you don't care to match "123." this one is simpler:

^([0-9]*[.])?[0-9]+$
This revision is now accepted and ready to land.Dec 14 2023, 08:06
cashtab/src/utils/validation.js
505–519 ↗(On Diff #43609)

/^([0-9]+([.][0-9]*)?|[.][0-9]+)$/.test('123.12345')

> true

seems to work, but in this case we want to reject values with more than 2 decimal places.

Since parseFloat is the method eventually applied to the input to build the tx, I think for now best to use it in the validation check as well.

This revision was automatically updated to reflect the committed changes.