joinmarket-clientserver icon indicating copy to clipboard operation
joinmarket-clientserver copied to clipboard

Enable importing joinmarket wallet as watch-only to mobile wallets

Open BitcoinWukong opened this issue 2 years ago • 9 comments

Based on https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/1266 and https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/1265.

What changed in this PR

Added a config POLICY.display_ypub_zpub to allow showing xpub in ypub or zpub format.

Both ypub and zpub are BIP standards and are used by many mobile wallets: BlueWallet, Sentinel, etc. We have to show the pub key in ypub or zpub format when we import joinmarket wallet to these mobile wallets in order for them to figure out the correct script to use: p2sh-p2wpkh and p2wpkh respectively.

https://en.bitcoin.it/wiki/BIP_0049

Extended public keys use 0x049d7cb2 to produce a "ypub" prefix, and private keys use 0x049d7878 to produce a "yprv" prefix. Testnet uses 0x044a5262 "upub" and 0x044a4e28 "uprv."

https://en.bitcoin.it/wiki/BIP_0084

Extended public keys use 0x04b24746 to produce a "zpub" prefix, and private keys use 0x04b2430c to produce a "zprv" prefix. Testnet uses 0x045f1cf6 "vpub" and 0x045f18bc "vprv."

Motivation

With the combination of #1265 #1266 and this PR, we can now easily import one mixdepth of the joinmarket wallet as a watch-only wallet to our mobile wallets. (I tested the flow using Sentinel https://play.google.com/store/apps/details?id=com.samourai.sentinel&gl=US)

This way, whenever we want to receive a payment while traveling, we can generate a new wallet address using our mobile wallet. Later on, when we get back to our laptop, we can then start a tumbler process directly without worrying about transferring Bitcoin from other wallets to the joinmakert wallet.

(Eventually, we're going to replace xpub/ypub/zpub with output descriptors, but we're not quite there yet.)

BitcoinWukong avatar May 11 '22 07:05 BitcoinWukong

Concept ACK. Will review / test this and related PRs shortly.

kristapsk avatar May 11 '22 10:05 kristapsk

I agree, could be a very useful addition.

I guess descriptors could also be seen as a big priority.

(To be clear I don't have any plans to work on any of this, short term)

AdamISZ avatar May 11 '22 11:05 AdamISZ

I guess descriptors could also be seen as a big priority.

I already have some code for generating output descriptors in one unfinished PR. That's relatively simple compared to descriptor parsing. So we could add that too. I think here we want to support both simple xpub and descriptors, as different mobile wallets will support different things, with only descriptors this functionality will be limited currently.

kristapsk avatar May 11 '22 12:05 kristapsk

Needs rebase.

(Eventually, we're going to replace xpub/ypub/zpub with output descriptors, but we're not quite there yet.)

Now that #1270 is merged, we can use output descriptors here, but not sure about wallet support, likely we need to support multiple ways for now - xpub, zpub/ypub and descriptors.

kristapsk avatar May 26 '22 07:05 kristapsk

@AdamISZ @kristapsk , all comments addressed, PTAL, thanks!

BitcoinWukong avatar Jun 07 '22 00:06 BitcoinWukong

I've converted this PR back to Draft.

I'm going to enable both options instead: xpub and zpub. So that the user can have the option to either one.

BitcoinWukong avatar Jun 26 '22 21:06 BitcoinWukong

likely we need to support multiple ways for now - xpub, zpub/ypub and descriptors.

Rethinking this - what is bad from UX perspective is multiple choices for GUI user, when he just wants to import pubkey in watchonly wallet. BlueWallet's wallet import screen says "Please enter your seed words, public key, WIF, or anything else you've got", nothing about xpub vs zpub. When provided with xpub, BlueWallet will assume BIP44 and user sees no balance. Maybe just displaying single option and having zpub as default is the right thing to do? Anyone has experinece with importing in various different popular wallets?

kristapsk avatar Oct 06 '22 16:10 kristapsk

When provided with xpub, BlueWallet will assume BIP44 and user sees no balance.

My personal experience is that most mobile wallet does the same thing, and that's my motivation of this PR initially.

Maybe just displaying single option and having zpub as default is the right thing to do? Anyone has experinece with importing in various different popular wallets?

This is my preferred option, but it seems that not everyone agrees with this approach though.

BitcoinWukong avatar Nov 21 '22 06:11 BitcoinWukong

Any chance this will find its way into rpc api?

There seems to be "problems" with the xpubs contained in the /display response. I did not verify it yet, but from reports it seems they represent e.g. m / purpose' / coin_type' / account' / change instead of m / purpose' / coin_type' / account'. Might be done intentionally, so do not see this as a bug report. However, having the xpub/zpub for each mixdepth as distinct property in a response from the api would be awesome. :raised_hands:

theborakompanioni avatar Oct 20 '23 11:10 theborakompanioni