joinmarket-clientserver
joinmarket-clientserver copied to clipboard
Enable importing joinmarket wallet as watch-only to mobile wallets
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.)
Concept ACK. Will review / test this and related PRs shortly.
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)
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.
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.
@AdamISZ @kristapsk , all comments addressed, PTAL, thanks!
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.
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?
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.
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: