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

Showing xpub instead of ypub for JoinMarket wallet #558

Closed
huey735 opened this issue Apr 19, 2020 · 12 comments
Closed

Showing xpub instead of ypub for JoinMarket wallet #558

huey735 opened this issue Apr 19, 2020 · 12 comments

Comments

@huey735
Copy link

huey735 commented Apr 19, 2020

Both the gui and the cli show an xpub for (legacy addresses) instead of a ypub (for wrapped segwit)

cli
gui

Images by @zndtoshi and extracted from this thread.

@chris-belcher
Copy link
Contributor

The other bip32 prefixes ypub zpub upub are only used in Electrum I think: https://github.com/spesmilo/electrum-docs/blob/master/xpub_version_bytes.rst

A better solution long term is descriptors, and I remember seeing on the Electrum IRC channel that the Electrum wallet will eventually move over to using descriptors instead of bip32 prefixes. So we probably shouldn't do this, instead we should use descriptors.

@huey735
Copy link
Author

huey735 commented Apr 19, 2020

I'm not sure I understand what you said. These new descriptors would allow for the creation of watch-only wallets too? I'm only familiar with Electrum and Sentinel for the creation of watch-only wallets and both follow the standard you linked to above.

@chris-belcher
Copy link
Contributor

chris-belcher commented Apr 19, 2020

Descriptors or (to give them their full name) Output Descriptors are a way of describing output scripts. Here's a link to an explanation of them: https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md

Descriptors do the same job as xpub version bytes, but are more flexible and extensible. For example they support multisig. I've used descriptors recently in development, they are really good. My point is I've seen Electrum devs also say they are really good and that Electrum will one day move towards using them too. I didn't know about Sentinel, but that's worth noting. So I'd expect long term all watch-only wallets will switch over to using descriptors instead of xpub version bytes.

On the other hand, there wouldn't be anything wrong with using xpub version bytes now, because descriptors might not happen for months/years. It would probably be quite a short PR just requiring a change to the version byte. Probably the best place to look for starting to edit is the cryptoengine.py file.

@openoms
Copy link
Contributor

openoms commented Apr 19, 2020

@huey735 I have been playing with watch only wallets too in Electrum, Bluewallet, SentinelX etc.
Thanks for opening the issue, I would like to not needing to convert the xpubs too.

In the meantime I have used this tool (offline!): https://jlopp.github.io/xpub-converter/ (https://github.com/jlopp/xpub-converter) for a quick workaround.

@chris-belcher descriptors look really good, but indeed it might take some time for that solution to appear in the growing number of usable wallets expecting to just scan a ypub for the specific address type.

@AdamISZ
Copy link
Member

AdamISZ commented Jun 2, 2022

Closing this in favour of #1267

@AdamISZ AdamISZ closed this as completed Jun 2, 2022
@kristapsk
Copy link
Member

I think we should move to show output descriptors instead of ypub/zpub. Some basic support for that was added with #1270, some more work needs to be done here.

@AdamISZ
Copy link
Member

AdamISZ commented Jun 4, 2022

I think we should move to show output descriptors instead of ypub/zpub. Some basic support for that was added with #1270, some more work needs to be done here.

I somewhat agree but ... doesn't that conflict with what you said here? #1267 (comment) i.e. should we just forget y/zpub and work on descriptors or should we put some basic support for the former in while the latter is coming later?

@kristapsk
Copy link
Member

I think we should support both descriptors and xpub/ypub/zpub for importing, but not for exporting.

@AdamISZ
Copy link
Member

AdamISZ commented Jun 4, 2022

I think we should support both descriptors and xpub/ypub/zpub for importing, but not for exporting.

Do you mean 'support for importing' as in support for importing into Joinmarket, i.e. allow someone to enter a zpub into JM and having our software display balances in a watch only wallet? (Sorry for laborious question but the language could be ambiguous).

I guess I can see an argument for it, but it seems a little easier and more useful to do the opposite, because quite a few people like to watch their JM wallet balance on another wallet (and, we don't yet have watch-only wallet support in JM).

@kristapsk
Copy link
Member

Do you mean 'support for importing' as in support for importing into Joinmarket, i.e. allow someone to enter a zpub into JM and having our software display balances in a watch only wallet? (Sorry for laborious question but the language could be ambiguous).

Yes.

it seems a little easier and more useful to do the opposite

Why? I remember @chris-belcher had arguments against ypub/zpub, that output descriptors is the way. And it's easy to just take xpub part from output descriptor. In Qt GUI we likely should have QR code and copy to clipboard support for both plain xpub and output descriptor. Or I didn't understand you here?

we don't yet have watch-only wallet support in JM

I still haven't lost hope for #989 being merged at some point. :)

@BitcoinWukong
Copy link
Contributor

BitcoinWukong commented Jun 7, 2022

Why? I remember @chris-belcher had arguments against ypub/zpub, that output descriptors is the way. And it's easy to just take xpub part from output descriptor. In Qt GUI we likely should have QR code and copy to clipboard support for both plain xpub and output descriptor. Or I didn't understand you here?

I think we can all agree that in an ideal world, all the Bitcoin wallets simply uses output descriptors. But as of today, not many "watch-only wallet apps" support output descriptors. Even though @chris-belcher doesn't like ypub / zpub, he also said #558 (comment):

""" On the other hand, there wouldn't be anything wrong with using xpub version bytes now, because descriptors might not happen for months/years. """

So, for today, if we show ypub/zpub in QT GUI, the user would have a much easier time importing JoinMarket wallet as watch-only to their mobile wallet apps.

In Qt GUI we likely should have QR code and copy to clipboard support for both plain xpub and output descriptor.

I think that would be the ideal solution. Maybe we add an option on the QR page to allow the user to switch between ypub/zpub and output descriptor.
The point is, for today, most mobile wallet apps allow users to import watch-only wallets via ypub/zpub, but not many allow users to import via output descriptors yet.

@kristapsk
Copy link
Member

Maybe we add an option on the QR page to allow the user to switch between ypub/zpub and output descriptor.

Or we can have separate context menu items for both. There is already get_xpub_descriptor() function available in JM after merging #1270.

Mobile wallets I think currently mostly support ypub/zpub (which is Electrum thing), but, for example, Bitcoin Core supports only output descriptors (with descriptor wallets, before that you couldn't import neither xpub nor zpub/ypub in watch only wallet) or importing individual Bitcoin addresses. There is list of wallets supporting descriptors, but I haven't tested it much (apart from Bitcoin Core), for example, for BlueWallet it says "limited support".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants