Page MenuHomePhabricator

[Cashtab] Refactor address validation to accept prefixless addresses
ClosedPublic

Authored by bytesofman on Dec 16 2021, 23:25.

Details

Summary

T1868

Cashtab address validation has developed incrementally based on shifting demands for address format types, including bitcoincash: format, simpleledger: format, both these legacy formats and the new ecash: + etoken: standard, and only the new standard. Some exchanges have begun using valid ecash: and etoken: addresses without prefixes, so Cashtab should support these too.

Another lingering complication is that the backend only accepts addresses in bitcoincash: or simpleledger: format. So, Cashtab must preserve methods for converting to these formats (even while it does not recognize them as valid user input).

A diff was submitted (D10158) to accomplish this -- but did so by adding another layer of technical debt. At this point, the address validation should be refactored to support ecash formats.

This diff introduces new simplified methods for validating addresses input by the user.

isValidXecAddress - Accepts a valid ecash address with or without the ecash: prefix (checksum must match ecash: prefix
isValidEtokenAddress Accepts a valid etoken address with or without the etoken: prefix (checksum must match etoken: prefix
toLegacyCash - Accepts a valid ecash: address and returns a valid bitcoincash: address
toLegacyToken - Accepts a valid etoken: address and returns a valid simpleledger: address

Validation for send to many was also updated to support these new methods. Previous validation was complicated by the existence of address validation in the parseAddress function. This function has been refactored and simplified.

Some functions were moved from Ticker.js into cashMethods.js as part of this refactor because they are related to each other.

A future diff will remove the now-obsolete isValidCashPrefix and isValidTokenPrefix functions from Ticker.js and ScanQRCode.js. Another future diff will move other functions from Ticker.js that belong in cashMethods.js

Test Plan

npm start

  1. Navigate to Send screen
  2. Enter a valid ecash address with prefix. Confirm no error
  3. Delete the prefix. Confirm no error.
  4. Enter a valid bitcoincash address with prefix. Confirm error.
  5. Enter a valid bitcoincash address without prefix. Confirm error.
  6. Enter a valid legacy BTC address. Confirm error.
  7. Enter a random string. Confirm error.
  8. Perform same tests with amounts in "multi send"
  9. Navigate to Send Token page. Perform steps 2-7 with simpleledger address instead of bitcoincash address.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
address-validation-refactor
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17694
Build 35214: Build Diffcashtab-tests
Build 35213: arc lint + arc unit

Event Timeline

emack requested changes to this revision.Dec 17 2021, 02:34
emack added a subscriber: emack.

Reviewed and tested all ok, just need the additional unit test coverage in cashMethods.test.js below.

web/cashtab/src/utils/__tests__/cashMethods.test.js
218 ↗(On Diff #31442)

Need four additional unit tests in here where you're passing a legacy, bitcoincash:, etoken and prefix-less address into toLegacyCash(), whereby the first 3 should all return an invalid checksum error and the 4th returning the valid bch equivalent.

This revision now requires changes to proceed.Dec 17 2021, 02:34

Added address validation to toLegacyCash and toLegacyToken functions, added unit tests, refactored toLegacyCashArray now that validation is handled in toLegacyCash, updated some unit test results to match new error msgs

emack requested changes to this revision.Dec 18 2021, 03:49
emack added inline comments.
web/cashtab/src/components/Send/Send.js
420 ↗(On Diff #31452)
  • the 5.5 min value should be referenced as a global param in Ticker.js
  • the float validation logic should be in a isValidXecValue() function in validation.js so we can reuse it elsewhere
  • unit tests for isValidXecValue()
This revision now requires changes to proceed.Dec 18 2021, 03:49

New validation function isValidXecSendAmount and unit tests to replace hard coded param in Send.js refactor

This revision is now accepted and ready to land.Dec 22 2021, 07:23