server/asset/btc: require zpub, but offer xpub conversion #1255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is running on dex-test.ssgen.io, on top of #1239. I registered fresh there with it using a vpub. The
|
buck54321
reviewed
Oct 26, 2021
There was a problem hiding this comment.
Seems fine, but harness doesn't work anymore.
failed to create fee addresser for asset btc: provided extended key (prefix "tpub") not for p2wpkh, equivalent is "vpub5SLqN2bLY4WeZJ9SmNJHsyzqVKreTXD4ZnPC22MugDNcjhKX5xNX9QiQWcE4SSRzVWyHWUihpKRT7hckDGNzVc69wSX2JPcfGeNiT5c2XZy"
chappjc
force-pushed
the
xpub-conversion
branch
from
e9a0b6a to
b709abe
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a server operator to accept BTC for the registration fee, it is currently necessary to provide an extended public key with the "xpub" prefix. This is because the
btcutil/hdkeychainpackage only recognizes such xpub strings as being from a given network, otherwise the net/version bytes are unknown. However, the derived fee addresses are P2WPKH.The potential trouble is that wallets that generate native segwit addresses tend to return "zpub"-prefixed extended public key encodings in following BIP-84 (vpub for testnet and regnet). I have verified that Electrum does this if the wallet was created as native segwit, and Exodus actually exports both an xpub and zpub that do not seem to correspond to the same extended key (two separate derivation paths) making it potentially ill-advised to use the xpub for segwit addresses!
Prior to this PR, an operator with a wallet that provides a zpub would have to convert to an xpub. This is not hard (see here), but it is confusing and unnecessary since the dex software can do it. Further, if they got an xpub from a non-segwit wallet or non-segwit path, it's possibly they will have to go to some trouble to claim funds sent to segwit addresses derived from this extended key.
To address this, this PR now requires the key to be with a "zpub" prefix. Also, if an "xpub" is provided, it will convert it to "zpub" and return an error with this string so the operator is aware that segwit addresses will be generated if they decide to use that zpub.