[Cashtab] Support prefixless addresses with valid checksums
AbandonedPublic
Actions

Authored by bytesofman on Sep 19 2021, 19:08.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Summary

T1868

Cashtab previously enforced the presence of ecash: or etoken: prefixes to scan QR codes or send. This diff changes address validation to support prefixless addresses as long as they have valid checksums for valid eCash prefixes (ecash: or etoken:)

Note: Cashtab does not support sending XEC to an etoken: address or eTokens to an ecash: address.

Test Plan

npm start
Send XEC to an ecash: address after deleting the prefix
Send an eToken tx to an etoken: address after deleting the prefix
Scan a QR code of a prefixless and valid ecash: address
Scan a QR code of a prefixless and valid etoken: address

Note: you can create such QR codes on your phone at https://www.qr-code-generator.com/ and then scan them with your dev machine's webcam through Cashtab

Diff Detail

Repository

rABC Bitcoin ABC

Branch

accept-no-prefix-addr

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16711
Build 33272: Build Diff	cashtab-tests
Build 33271: arc lint + arc unit

Event Timeline

bytesofman created this revision.Sep 19 2021, 19:08

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 19 2021, 19:08

bytesofman requested review of this revision.Sep 19 2021, 19:08

Harbormaster completed remote builds in B16711: Diff 30055.Sep 19 2021, 19:10

I think the address validation deserves a good refactor. There are validation pieces here and there, and now the functions are hard to understand at best. The fact that you have to comment about what the assumptions are at each call site is a hint.
Some examples:

isValidxxxPrefix() returns true if there is no prefix. Cash address and token address prefix validation are now consistent, but the wrong way imo. The fact that isValidTokenPrefix("This is not an address") returns true is a bad API.
The toLegacy() workflow is overcomplicated. You prepare a lot prefix related things before decoding the address, but the decode can fail and is tested against that. You can either rely on it, or use a complete address validation function so you get a useful error to deal with.
Why is parseAddress() taking an isToken parameter ? I would expect this to be a return value, not an input.

Overall the code can be greatly simplified and get a better test coverage by creating a proper address validation function.

This revision now requires changes to proceed.Sep 20 2021, 07:48

Good points. Address validation has been built up incrementally and is not currently in a great state. An additional complication is that the backend will (for now) only accept bitcoincash: or simpleledger: addresses.

I'll do some thinking on how to improve the process.

bytesofman mentioned this in D10687: [Cashtab] Refactor address validation to accept prefixless addresses.Dec 16 2021, 23:25

Joey King <joseph.roy.king@gmail.com> mentioned this in rABCc567533955a8: [Cashtab] Refactor address validation to accept prefixless addresses.Dec 22 2021, 18:12

Superseded by D10687