PIVX-Project / PIVX-Android

Android wallet for PIVX
https://www.pivx.org
Other
35 stars 122 forks source link

QR Encode / Decode - Inconsistent and arbitrary public key formatting #1

Closed sabhiram closed 7 years ago

sabhiram commented 7 years ago

I know the project's source is not hosted here as yet, but based on some activity in the #support channel of the pivx slack group, I am creating this issue for book-keeping.

It seems like the QR codes generated by the mobile wallet follow this convention:

pivx:DTT5MBDNgRHgQtY67a9YCVFNNWnsDUara9?label=Receive%20address

While this seems like a good idea, is it really appropriate to add all this meta-data? This will pretty much mean any other app that scans a PIVX mobile wallet generated QR code will not know what it is dealing with unless we establish some sort of PIVX QR code gold standard (which I don't think makes any sense anyway).

Additionally, when one uses a the PIVX paper wallet to generate a QR code, if that is scanned into the app - it will claim that it is invalid since it does not adhere to the above schema.

So this breaks down into two issues:

  1. The QR code scan logic should accept naked public addresses as well as your special marked up ones (today it only accepts the marked up ones).
  2. Perhaps the QR code should omit the extra metadata and we should verify if the public key is a valid pivx one using other logic, this way what we generate would just be DTT5MBDNgRHgQtY67a9YCVFNNWnsDUara9 (for the above case).

1 is an easy enough item to deal with and should be simple to fix. #2 speaks to a larger problem and is something that the architects of this solution should carefully consider. What if I wrote another awesome PIVX app which generated QR codes? Don't we want those to be interoperable?

These are just my 2cents, thoughts welcome!

EvandroZanatta commented 7 years ago

:+1:

furszy commented 7 years ago

Hi, Sorry for the delay, I read this and forget to answer. The qr is following the bitcoin standard specification of BIP21. Is totally compatible with any product implementing the same standard. Please read it. https://github.com/bitcoin/bips/blob/master/bip-0021.mediawiki

EvandroZanatta commented 7 years ago

Then the mistake would be in Paper Wallet? Bitcoin's PaperWallet generates the QR Code without the prefix bitcoin:x1a2c3 But the QR Code generated by Wallet Desktop and mobile, generate with the prefix pivx:x1a2c3

In this case, there are two possible things to do ...

  1. Include a compatibility in the QR Code reader, making it understand a QR Code without the prefix.
  2. Change the PaperWallet, causing the QR Code to be generated with the prefix. But thus, all PIVX applications that generate QR Code, must include the prefix obligatorily.

The decision is up to you. If I need to change something in PaperWallet, I can change it, I'm already making some other improvements as well

Fuzzbawls commented 7 years ago

This is a difference in "standards" per-say. BIP21 sets guidelines for payment request URIs, which are being used in the mobile wallet in places that they should not be.

sabhiram commented 7 years ago

Exactly! Thank you @Fuzzbawls

furszy commented 7 years ago

Issue fixed on v1.2 .