KomodoPlatform / komodo-wallet-mobile

KomodoWallet Mobile codebase
https://komodoplatform.com
MIT License
21 stars 33 forks source link

Remove prefix from QR code scans #121

Closed takenagain closed 5 months ago

takenagain commented 6 months ago

Fixes #120 by removing the prefix from the address when it is scanned in.

Before: image

After: image

smk762 commented 6 months ago

Thanks! I've tested QR codes for addresses with prefix for BLK, LTC and BCH, and can confirm the prefix is removed allowing a successful spend in most situations. Confirmed the bch prefix from coins file being removed has no negative impact ( https://github.com/KomodoPlatform/coins/blob/master/coins#L1372)

I did encounter one issue though, using the QR code example from below as provided by a community member via https://coincards.com/ca/all-gift-cards/ image

Scanning this code also adds a suffix indicating the amount to be paid as shown in the image below image

As this is related to this PR, but outside the original scope, I can create a new issue for this unless you prefer to continue in this PR.

For the suffix, redaction could be the simplest option, but it would be a nice UX enhancement to have the ?amount= value also populate the Amount field in the app send form when present. I'm not sure if there is an industry standard with respect to the suffixes, though I'd assume most would follow the obvious convention (? to indicate beginning of suffix, amount to indicate value payable). To cover all bases we could do

if suffix_params:
    if 'amount' in suffix_params:
        populate_amount_field(suffix_params['amount'])
    else:
        redact_suffix()          
CharlVS commented 6 months ago

@smk762 We could pivot this to be regex-based, which would then cover all current and future issues instead of having to add a handle for each new one.

Can you provide a regex that's safe to use for address sanitation?

smk762 commented 6 months ago

Can you provide a regex that's safe to use for address sanitation?

For Address: (?<=:)(.*?)(?=\?)

For Amount: (?<=amount=)(.*?)(?=&|$)

You can use the second one for any other anticipated query param like fields for memo etc. if needed - just change amount to the desired key.

takenagain commented 6 months ago

Thanks for the regex, @smk762. The Amount field should now be populated if there is an ?amount= parameter.

I noticed the error below when scanning bitcoin:1M5m1DuGw...4oREzpCX from BTC-Segwit and testing potential changes to PaymentUriInfo. Should an exception be made for Segwit pairs when scanning QR codes @smk762, @CharlVS?

image