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

non-confidential QR code #117

Closed
adam3us opened this issue Jul 17, 2019 · 9 comments
Closed

non-confidential QR code #117

adam3us opened this issue Jul 17, 2019 · 9 comments
Labels
enhancement New feature or request

Comments

@adam3us
Copy link

adam3us commented Jul 17, 2019

It is a somewhat common work flow or payment / donation request model to send people the blockexplorer url for your address.

With liquid CT addresses the JS version removes the public key before sending to the server, which may induce the recipient to send non-confidential contrary to the attempted instructions of the sender.

Not sure what the fix is. eg render QR gif of confidential address client-side in JS if JS is not disabled? Or disable QR code if not? Or show the full confidential address if JS is not disabled? People may still paste the confidential address even without QR code. Or a warning that its non confidential in an alt-text popup?

@shesek
Copy link
Collaborator

shesek commented Jul 20, 2019

Even if the address is rendered client-side, the server will become aware of the blinding pubkey as long as its part of the requested url path. The client-side javascript does strip it off before sending calls to the /address/<address> API endpoint, but the initial request sent to fetch the html already leaked it to the server at this stage...

This could be avoided if the address it put behind a hash fragment (#) instead of in the path.

i.e. https://blockstream.info/liquid/address#<ct-address> instead of https://blockstream.info/liquid/address/<ct-address>

However, noscript users will not be able to use this page (there's no way to access the hash fragment in order to offer a noscript fallback like we currently do).

@shesek
Copy link
Collaborator

shesek commented Jul 20, 2019

Actually... this could work: https://blockstream.info/liquid/address/<non-ct-address>#<ct-address>

@shesek
Copy link
Collaborator

shesek commented Jul 20, 2019

eg render QR gif of confidential address client-side in JS if JS is not disabled?

This is already the case, JavaScript-enabled browsers do client-side rendering of the QR.

Or disable QR code if not? Or show the full confidential address if JS is not disabled?

If we use the /address/<non-ct-address>#<ct-address> format, we could have JS-enabled users render the CT-enabled address, while noscript users get the non-CT one.

People may still paste the confidential address even without QR code.

I'm not sure I understand the concern here

Or a warning that its non confidential in an alt-text popup?

Once we have support for CT addresses, a warning for non-CT addresses sounds like a good idea.

@shesek shesek added the enhancement New feature or request label Jul 20, 2019
@adam3us
Copy link
Author

adam3us commented Jul 20, 2019

People may still paste the confidential address even without QR code.

I'm not sure I understand the concern here

typo it should say may still paste the non-confidential address (even if the QR were disabled for non-CT)

@adam3us
Copy link
Author

adam3us commented Apr 3, 2020

reviewing this I think the assumption for why we were doing this is no longer valid: an older version of elements used to reuse the pubkey.

now that is not the case, neither green nor elements reuse pubkeys, so the pubkey inside the extended confidential address does not correlate.

so we can send it to the server in both JS and noJS cases and make the QR code work, display the full address, and remove the direct/JS to strip the pubkey.

@alessandro-saglimbeni
Copy link

@shesek this was almost fixed and deployed recently.

If I search for a confidential address like https://blockstream.info/liquid/address/VJLGkVvMFXptZv8VWUjeMZvJLWiM1aJF6ukdEwgQvuVuXXMt2bNRsW7JVHbPLzw83WtayVMY7n5yywHB

The address maintains the confidential part, but the Qr actually encodes bitcoin:VJLGkVvMFXptZv8VWUjeMZvJLWiM1aJF6ukdEwgQvuVuXXMt2bNRsW7JVHbPLzw83WtayVMY7n5yywHB

if this address should be a URI (I don't really see the necessity for it, but it's a nice to have) it should prepend liquidnetwork: as per ElementsProject/elements#805

@shesek
Copy link
Collaborator

shesek commented May 7, 2020

see the necessity for it

It allows scanning the QR using the phone's generic QR scanning functionality and have it associate the QR with the locally installed app registered to handle bitcoin: URIs, instead of having to scan it from inside the wallet app.

it should prepend liquidnetwork:

I will change this.

@alessandro-saglimbeni
Copy link

@shesek I think this issue can be closed, it's implemented right?

@shesek
Copy link
Collaborator

shesek commented Nov 9, 2020

That's correct, closing.

@shesek shesek closed this as completed Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants