Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QR Code scanner #9913

Closed
luke-jr opened this issue Mar 4, 2017 · 49 comments
Closed

QR Code scanner #9913

luke-jr opened this issue Mar 4, 2017 · 49 comments
Labels

Comments

@luke-jr
Copy link
Member

luke-jr commented Mar 4, 2017

Core is currently missing any way to scan QR Codes. We should add this.

@TheBlueMatt
Copy link
Contributor

TheBlueMatt commented Mar 4, 2017 via email

@luke-jr
Copy link
Member Author

luke-jr commented Mar 4, 2017

FWIW, I started (and I think did most of the hard parts?) this in my qrcode branch, but I don't intend to finish it now since I don't really have a way to test it... so if someone else wants to complete it, feel free (but mention here so we don't duplicate work)

Do other desktop wallets have that?

At least Electrum does.

How many people really want to scan a QR code on their desktop?

There's also no reason to assume Core is running only on a desktop...

@jonasschnelli
Copy link
Contributor

The Digital Bitbox Desktop App (based on this project, same build system, QT/C++11) has a QRCode scanner in case someone wants to extend this:
https://github.com/digitalbitbox/dbb-app/blob/e7ce27112a4d44ba0b821e84f1178f6590caa6ec/src/qt/qrcodescanner.cpp

From what I can tell is that to get the dependencies right (deterministic gitian builds!) this can be very hard.

Maybe it's simpler to have a third party scanning app that execute a bitcoin: link (which then opens in Bitcoin-Qt).

@laanwj
Copy link
Member

laanwj commented Mar 4, 2017

Qt has built-in support for cameras now? I was about to go on a rant that this would involve a lot of platform specific code, but wow, that kind of changes everything.

@laanwj laanwj added the GUI label Mar 4, 2017
@jonasschnelli
Copy link
Contributor

QtMultimedia has built in camera support.
The complicated part is to get the all-static deterministic binary on linux.
And why not provide a third party application that is not part of the "important" binary? It could scan and pass it over the bitcoin: URL scheme into Bitcoin-Qt.

@laanwj
Copy link
Member

laanwj commented Mar 4, 2017

The complicated part is to get the all-static deterministic binary on linux.

Ouch, indeed. Would need at least dynamic loading of the underlying C libraries used for accessing the cameras, so that there is only an optional dependency.

And why not provide a third party application that is not part of the "important" binary? It could scan and pass it over the bitcoin: URL scheme into Bitcoin-Qt.

I tend to agree that this use case is 100% implementable with an external utility, without any modifications to our current code. This is what bitcoin: URL support was designed for. I guess the only thing missing from a user point of view is a way to launch the utility from Bitcoin Core's GUI.

@luke-jr
Copy link
Member Author

luke-jr commented Mar 4, 2017

Qt does indeed have camera support, but I was using ZBar, which does both the camera and QR Code stuff.

Deterministic binary can be "no QR scanning supported" for the initial implementation if that's difficult for some reason. AFAIK cameras just use a normal file descriptor, so I don't see why it'd be a huge problem...

A third-party application won't integrate nicely... users want a QR Scan button on the Send tab :)

@SoraKohaku
Copy link

nice for next development i hope :)

as i see phone only can do that. not for desktop. but if it's added will be nice when shopping online. If that great put all shop online with QR :)

@luke-jr
Copy link
Member Author

luke-jr commented Mar 4, 2017

Hm, QR scan the display? Not what I had in mind, but interesting idea nonetheless...

@SoraKohaku
Copy link

i still learn for vc programming but i know vb.net. if possible. i wanna make Bitcoin Core great(used vc of course). my idea can converted here?~permission needed

@luke-jr
Copy link
Member Author

luke-jr commented Mar 4, 2017

@FndNur1Labs Bitcoin Core is C++, not Visual . While we don't care what IDE you use, you'll have a much easier time with something GCC/LLVM-based and/or on Linux. But this is off-topic here, so if you have further questions/comments along those lines, please bring it up in the #bitcoin-core-dev IRC channel instead.

@laanwj
Copy link
Member

laanwj commented Mar 4, 2017

A third-party application won't integrate nicely... users want a QR Scan button on the Send tab :)

There's no reason why a QR scan button in the send tab couldn't invoke an external utility. This is what many of the phone apps do too. They just launch whatever QR scanner the user has installed.

@SoraKohaku
Copy link

@luke-jr ok sir and @laanwj so no support for desktop. thank you for information

@benma
Copy link

benma commented Mar 9, 2017

@TheBlueMatt

Do other desktop wallets have that? How many people really want to scan a QR code on their desktop?

It is handy when you want to send some funds to your mobile wallet. I use the atrocious qtqr tool to scan the mobile wallet's QR code.

@laanwj
Copy link
Member

laanwj commented Mar 9, 2017

I use the atrocious qtqr tool to scan the mobile wallet's QR code.

What is so atrocious about it?

@benma
Copy link

benma commented Mar 9, 2017

The UI is surprisingly bad. For example, when the scanner recognizes the QR code, the video stream keeps going with the QR code marked with a green rectangle. To see its contents / perform an action, you have to click it. It is irritating doing that and holding the phone steady at the same time.

A sane QR code reader (bascially any other I have seen) automatically continues when it recognizes the code, which is the only thing a user reasonably wants.

Once it is decoded, it is shown in a dialog with two buttons: Edit/OK. A first time user clicks 'OK', expecting something to happen, but the dialog disappears and nothing happens. Repeat the clumsy scanning. You are supposed to copy/paste the contents from the dialog box (or hit edit and get it from the textarea used to encode QR codes).

Usual features as opening an encoded url or performing registered actions don't exist (edit: it might exist, but it is not easy to discover how it works). Copying is the only thing you can do. That isn't so bad, but then it should simply do that or give you a button to do it instead of manually selecting and copying it.

@laanwj
Copy link
Member

laanwj commented Mar 9, 2017

You could try contributing to the project to increase its usefulness. At least reporting these issues. Just like Bitcoin Core it's open source.

If the go-to QR scanner for Ubuntu is "atrocious", I'd say much is to be gained by improving that, also for non-bitcoin users.

@benma
Copy link

benma commented Mar 9, 2017

I have considered that but haven't gotten around it it (also put off by LaunchPad, to be honest). Now that I typed it, I'll see what I can do.

Edit:
https://bugs.launchpad.net/qr-tools/+bug/1671481
https://bugs.launchpad.net/qr-tools/+bug/1671482
https://bugs.launchpad.net/qr-tools/+bug/1671484

@laanwj
Copy link
Member

laanwj commented Mar 9, 2017

Thanks!

@SoraKohaku
Copy link

@laanwj that only ubuntu. is can do by Debian OS?Or just ubuntu....

@mariodian
Copy link
Contributor

Isn't it the common practice to put a sticker over laptop's camera for privacy reasons? If that's the case (and empirically it is) then this feature wouldn't make much sense.

However, scanning the display for a QR code is an interesting idea. I even remember needing this feature once and spent around half an hour looking for an app which can do that.

@laanwj
Copy link
Member

laanwj commented Apr 21, 2017

Isn't it the common practice to put a sticker over laptop's camera for privacy reasons?

Yep.

However, scanning the display for a QR code is an interesting idea.

That's indeed a quite interesting idea. Would be easily possible on many OSes at the moment. Though on the long run it might be complicated by the ongoing initiative to sandbox applications so that they can't 'spy' on others.

@jonasschnelli
Copy link
Contributor

However, scanning the display for a QR code is an interesting idea.

Well,... if you have a QRCode scanner installed, all you need is to uncover your laptops camera and use something that reflects well (mirroring cellphone back).
If it's a bitcoin URI, it will launch/trigger Bitcoin-Qt.

@laanwj
Copy link
Member

laanwj commented Apr 21, 2017

something that reflects well (mirroring cellphone back).

I didn't know - QR codes are mirror-invariant?

@mariodian
Copy link
Contributor

I didn't know - QR codes are mirror-invariant?

Some apps can handle it. I tested it on iOS with CrazyScan and Copay and it works. Other app (Scan - QR Code and Barcode Reader) couldn't read it.

@Sjors
Copy link
Member

Sjors commented Mar 16, 2018

Concept ACK. There are situations where I don't want to use a mobile mobile wallet for payments in meat-space. I've had to use clumsy workarounds several times now to get a Bitcoin address of off someone sitting right next to me.

To add to the list of "desktop" wallets that support scanning QR code: blockchain.info web wallet

If deterministic builds are an issue, I would prefer dropping Linux support (or using a differently library) for that feature over making an external utility. Ideally though I'd like to have my cake and eat it too :-)

@Sjors
Copy link
Member

Sjors commented Sep 23, 2019

This may also be relevant for:

@icota
Copy link
Contributor

icota commented Sep 30, 2019

I have used qzxing for scanning QR and it integrates very nicely with Qt (both C++ and QML). It's a lighter dependency as well because it only does image processing on top of QtMultimedia which, in turn, handles the camera(s) in a platform-independent fashion (tested this on Linux, macOS, Android). Like others have commented the real problem is introducing a new dependency in an orderly fashion and getting a deterministic binary built.

I think launching an external app to scan the QR is an attack vector. It is safer and better UX to have this integrated.

@promag
Copy link
Contributor

promag commented Sep 30, 2019

I think qzxing build system isn't very mature right? It doesn't even allow to only build qrcode support?

@promag
Copy link
Contributor

promag commented Sep 30, 2019

@icota right, but that's not a problem, let's improve it, I think @ftylitak doesn't mind.

@ftylitak
Copy link

Hello all, I am the maintainer of QZXing and I am open to any change as long as it is useful.

If you like to describe the change needed, I would gladly get involved with it.

@icota
Copy link
Contributor

icota commented Oct 1, 2019

Hi @ftylitak, I guess the first question is how hard would it be to make it possible to configure QZXing to build a static library with QR decode support only? Thanks for helping.

@ftylitak
Copy link

ftylitak commented Oct 1, 2019

Hello @icota,

the answer is that it is quite easy to do such a modification so I will make first sample and get back to you withinn the comming days (if not today).

@laanwj
Copy link
Member

laanwj commented Oct 2, 2019

If deterministic builds are an issue, I would prefer dropping Linux support (or using a differently library) for that feature over making an external utility. Ideally though I'd like to have my cake and eat it too :-)

Yes, I think it's fine to have it disabled in the release builds for Linux.

It's impossible to do this on Linux with the current way we package executables there for widest distribution support. E.g. when statically linking Qt. The tree of dependencies requires for webcam input on Linux is huge, there's no way we can (nor want to) statically link Linux' whole multimedia system.

In principle, using Flatpak as a distribution medium could solve this as it allows dynamically linking against the system Qt, which can be assumed to have the multimedia library. Also, distro-built Bitcoin packages, as well as manually built ones, won't suffer from this problem.

In any case, just targeting MacOSX and Windows binaries for this is fine in the initial release, no need to increase the scope to Linux cross-distro compatibility nightmares.

@promag
Copy link
Contributor

promag commented Oct 2, 2019

If we add qzxing then we can drop qrencode.

@ftylitak
Copy link

ftylitak commented Oct 2, 2019

An update from QZXing side.

I have created a (Work-In-Progress) pull request in QZXing that when completed, will provide more freedom to select the library parts needed to be built. ftylitak/qzxing#141

@promag (or any other), would you like to take a look at the approach? Any input is welcomed.

@icota
Copy link
Contributor

icota commented Nov 4, 2019

Thanks for the conditional build @ftylitak. If the project is otherwise stable could you do an official release? This would be a starting point to adding it as an optional Bitcoin Core dependency and potentially getting rid of qrencode.

@ftylitak
Copy link

ftylitak commented Nov 4, 2019

@icota indeed I long wanted to make an official release. There is a specific bug which holds me back that might be affecting you as well.

Since you mentioned the qrencode library I understand that you are interested in the Qr encoding part as well, correct? Currently in QZXing when encoding QR codes, the output image is correct for input strings that do not exceed the 106 characters with Low error correction mode.

Is this bug a blocking point for you?

This input will be useful for me to make proper prioritization.

@promag
Copy link
Contributor

promag commented Nov 4, 2019

@ftylitak is the fix hard (is there an issue)? If not I think we can wait.

@Sjors
Copy link
Member

Sjors commented Nov 4, 2019

We use BIP21 URIs to communicate an address + amount + arbitrary label. The UI enforces a limit of 255 characters (MAX_URI_LENGTH). The BIP does not impose a maximum, but it refers to RFC3986 which recommends 255 (although it's not clear if that recommendation includes parameters).

Here's a maximum lenght example on macOS (testnet):
Schermafbeelding 2019-11-04 om 17 28 49

@ftylitak
Copy link

ftylitak commented Nov 4, 2019

Ok, it is confirmed that if the example of @Sjors is decoded and re-encoded, the encoded image is wrong.

So, I will put an effort to fix the issue in the coming days and get back to you.

@Sjors
Copy link
Member

Sjors commented Nov 4, 2019

Thanks. In the future we might want to use QR code to transfer partially signed transactions (PSBT) to hardware wallets with a camera. Those might be even bigger, though we can worry about that later.

@andronoob
Copy link

@Sjors

In the future we might want to use QR code to transfer partially signed transactions (PSBT) to hardware wallets with a camera.

Data capacity of one single QR code is too small (3KB). To avoid hitting such limit, Electrum has already done something that made me feel uncomfortable.

@andronoob
Copy link

To be frank, I think PSBT lacks something like chunked encoding/sequence number/checksum/fountain code.

@ftylitak
Copy link

Greetings,

the Qr code encoding in QZXing is now fixed. You can freely give it a try and see if it works for you case.

In the coming days I will do the official release though the code is already available on the master branch.

@Rspigler
Copy link
Contributor

Rspigler commented Jan 7, 2021

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

I am setting a bounty for this issue (QR code scanner) for $300 to be paid in Bitcoin.

The bounty will be split between devs (author and substantial reviews) as I see most fair.

Thank you!

Robert Spigler

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEf4WKHNGE9pWzsby0UsewL8eQ8/AFAl/2w2cACgkQUsewL8eQ
8/BDZBAAqpfQW3iVqJUY9+5uYSSoFT6Yh0mSWVGScpCqwGWkQNTJlU+XH8vgTJIr
rFsoeLcAieMrA0KREf0WrB+3sfQAOJmBuxfzMJiI+3J2Yr7R//PEuYp0lMyKx6F5
cpqxHJkDyKz0nEdCEcb1nCSuy/f+xflmD5+4fKxGdIVY55NDj5R6Qp2TMUXIs+Np
ojPwRBx3WXlr1eW7ugFKn6vWOan8hsAnJEmQpJTlH8huyOVlumoya1e+alzCfpL5
lspOXzr3jdGyAbXjwMO8U99Xanz/3VWq5U6CZKJ5vU3OMDsVGeGE7e5BeAlW9RVz
yTCPfxov4h4w5EzgwrhA5X4HjhbHF0G+oJ7/gzmUbeBUlbFZH7B2M9TSEgpxY6cc
yfjr4YcmUofCO6rOWozYmKHv2Xp9CAZ5wBJwADoonKiraPn9DpeavUoPQhF3eWBF
yu1uxZClrWD5Fzl8HLdLKVsYEHuOQQGOCcJa5w6jhSqWmgW28qynfDe2ATNMdpd4
4LbEaTJ2AwUHTutxRn78D4jXqsdfN02Z2TzaSkoFM2NTqEQBp5F+LqhvM+W5wDMT
L/ydIC6vdANG0HuLCMPHDb8nAEgrWGC7mgq5SlYrm+vrbEt9zpTe4TBj5QWpgj24
BhfPVYmKqnZ2yIxq8qqLf1iGAiTLX8vEir+HLC1rfDk4i9zWLBw=
=8Yb3
-----END PGP SIGNATURE-----

@Bosch-0
Copy link

Bosch-0 commented Apr 1, 2022

I am setting a bounty for this issue (QR code scanner) for $300 to be paid in Bitcoin.

@Rspigler https://bitcoinbounties.org/

@fanquake
Copy link
Member

Moved too bitcoin-core/gui#670.

@bitcoin bitcoin locked and limited conversation to collaborators Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests