Support for BIP49 YPUB/UPUB #927

plasticlobster · 2017-11-17T23:58:48Z

The only thing missing from bitcoinjs-lib to support BIP49 is the network type designator. This simple change allows the HD wallet to support BIP49 YPUB's (or testnet UPUB's). This allows for the import of pure segwit HD wallets.

See also: https://github.com/bitcoin/bips/blob/master/bip-0049.mediawiki

Understanding that BIP49 is draft, you may reject this change, but I've made this change for a project and figured submitting a PR would be an appropriate offer to allow you to implement this simple functionality.

dcousens · 2017-11-18T00:04:01Z

From reading that BIP, I don't see the network constants you've specified...

Instead, the only difference I saw was that the purpose constant was 49 not 44?

To derive a public key from the root account, this BIP uses the same account-structure as defined in BIP 44, but only uses a different purpose value to indicate the different transaction serialization method.

plasticlobster · 2017-11-18T00:32:41Z

@dcousens

My address generation happens outside of bitcoinjs-lib, and I'm not sure if there are more implications to fully supporting an implementation of BIP49 as a result.

The use case was for an electrum wallet. See:
http://docs.electrum.org/en/latest/seedphrase.html#list-of-reserved-numbers
and
https://github.com/spesmilo/electrum/blob/0a648e2b1cdb7852d80c0626c62984ef01c1c7e9/lib/bitcoin.py#L50

The usage of these constants (which translate to ypub and upub) allow for auto-detection of a bip49-style wallet. When using a bip32-style xpub, it's impossible to determine whether the wallet is bip32 or bip49. While the constants do not appear in a BIP, they are the constants currently in use by wallet providers like electrum.

dcousens · 2017-11-28T04:46:47Z

Is this related to #934 ?

afk11 · 2017-12-15T14:10:15Z

src/hdnode.js

@@ -64,7 +64,9 @@ HDNode.fromBase58 = function (string, networks) {
  if (Array.isArray(networks)) {
    network = networks.filter(function (x) {
      return version === x.bip32.private ||
-             version === x.bip32.public
+             version === x.bip32.public ||
+             version === x.bip49.private ||


I think these semantics should be opt-in. Imagine someone supported Bitcoin Cash, and imported keys. There's not currently a bitcoin-cash network in the library, so they're using the bitcoin network for that. Imagine they didn't read the release notes, and allowed users to import a P2SH(P2WPKH) address, and funded that on Bitcoin Cash.. They're unspendable without talking to a miner.

I think this feature should be opt in, so a new function bip44TypefromBase58 or something that forces developers to think about what they are doing.

@afk11 I don't understand, what about these new BIP49 constants would trap users into creating unspendable addresses?

Isn't the risk that any P2SH(P2WPKH) scripts are unspendable by BCash nodes?
Nothing to do with the BIP49 constants?

Applications might accept stuff they shouldn't when forks like bitcoin cash happen, and developers start using bitcoin.networks.bitcoin as a bitcoin cash network. It's mostly that the feature should only be used via specialized encoding/decoding/factory functions. That way, developers have to consider the difference between xpub only, and the format that uses different scripts.

afk11

https://github.com/plasticlobster/bitcoinjs-lib/blob/8f39b516946805c6b4640d1df0fdd33a0ebedba7/src/hdnode.js#L129
This should be updated to use script type

Should also be opt in, and not usable by people who expect fromBase58 to only parse normal BIP32 keys.

afk11 · 2017-12-15T14:26:38Z

In general I think the proposal has its merits, it's nice to support what some of the major wallets are doing. I have some comments on the PR and proposal in general.

Generally, concept ACK once it's opt in.

Worth mentioning that the situation with these new prefixes isn't perfect, since some prefixes can have two script types. The specification just seems to be the ML post and electrum website, so it's not clear what the official spec actually is outside of specific prefixes.

I think we can introduce this as limited support for such versioned BIP32 wallets, and see what other prefixes people request. Maybe the config parameters could go into bip32ScriptVersions, as a new property on the network? Minor nit though

dcousens

LGTM, waiting on @afk11

dcousens · 2018-01-17T01:16:19Z

Shouldn't getAddress be changed too? (even if it is being removed in 4.0.0)

dcousens · 2018-01-17T01:21:05Z

@dabura667 @junderw where does zpub fit into this?

dabura667 · 2018-01-17T14:26:34Z

@dcousens Honestly... I think maybe this is a little bit early.

I would really like to see a BIP from @ecdsa before we merge this.

Also, I think this is insufficient.

Electrum also has extra version bytes for WIF private keys. We should probably include those if we include this.

zpub/prv are for native P2WPKH.

Currently, ECPair and HDNode have a network attribute which refers to a network in networks.

I think they should also have an attribute for "scriptType" which will tell us what to use when using toWIF or toBase58Check respectively. So we can look at the scriptType, and use that to guide us as to what to fetch from the network object.

I also dislike calling this bip49... perhaps keep all the legacy network structure and add a new key:

ie.

  bitcoin: {
    messagePrefix: '\x18Bitcoin Signed Message:\n',
    bech32: 'bc',
    bip32: {
      public: 0x0488b21e,
      private: 0x0488ade4
    },
    pubKeyHash: 0x00,
    scriptHash: 0x05,
    wif: 0x80,
    versionBytes: {
      ECPair: {
        'P2PKH': {
          wif: 0x80
        },
        'P2SH(P2WPKH)': {
          wif: 0x82
        },
        'P2WPKH': {
          wif: 0x81
        }
      },
      HDNode: {
        'P2PKH': {
          public: 0x0488b21e,
          private: 0x0488ade4
        },
        'P2SH(P2WPKH)': {
          public: 0x049d7cb2,
          private: 0x049d7878
        },
        'P2WPKH': {
          public: 0x04b24746,
          private: 0x04b2430c
        }
      }
    }
  },

(WIF values weren't in wiki, but they are in the bitcoin.py.
https://github.com/spesmilo/electrum/blob/0a648e2b1cdb7852d80c0626c62984ef01c1c7e9/lib/bitcoin.py#L483
The function for encoding WIF keys is right below it and you see they are adding those numbers to 128, which is essentially ORing them.)

dcousens · 2018-01-18T00:36:06Z

What a mess.

junderw · 2018-01-18T03:28:47Z

Yup. It is.

Big old mess.

But I have been thinking for a while that now we have different ways to use private keys, we need a state similar to compressed that can store the script type. The way this is represented when exported or imported can be added later when BIPs are written.

What do you think? @dcousens

dcousens · 2018-01-18T04:16:38Z

@junderw it feels like the version bytes are becoming useless as network information, and instead, acting as flags for other meta information.

Maybe we should support non-strict fromBase58 decoding? Sigh.

dcousens · 2018-01-18T04:20:55Z

Can we simply tell these muppets to please stop abusing the BIP32 standard and instead write a BIP that stores this information somewhere else?

junderw · 2018-01-18T04:21:38Z

IMO we have bech32, why not use it? lol.

junderw · 2018-01-18T04:22:14Z

hrp could be p2shp2wpkh or something

junderw · 2018-01-18T04:22:49Z

xpubp2shp2wpkh1sdfouofgsdfijo9df8ehosf8feojosihfhe8fh

etc etc

dcousens · 2018-01-18T04:22:51Z

@junderw I have an idea, lets write a BIP that captures the path information AND the address type(s).

(semi serious, it might take less time than writing sane APIs for this mess)

junderw · 2018-01-18T04:23:38Z

While that comic is true.

Character encoding has a clear winner.

If you don't use UTF-8 you are wrong.

dcousens · 2018-01-18T04:38:53Z

Why is this necessary anyway? Who is actually passing around these base58 strings rather than a mnemonic?

ecdsa · 2018-01-18T04:58:59Z

@dabura667 we will submit a BIP

dcousens · 2018-01-18T05:10:47Z

@ecdsa are you open to engaging in a discussion about a more generic standard e.g like the above?

junderw · 2018-01-18T06:26:36Z

Why is the distinction necessary anyway?

Mainly for xpubs. A lot of people pass around xpubs for watch-only and merchant services.

dcousens · 2018-01-18T06:30:02Z

Mainly for xpubs. A lot of people pass around xpubs for watch-only and merchant services.

Everyone read the Security and Implications part of BIP32, right?
Why has this become a thing?

We even have an example of how to take advantage of this weakness in our integration tests.

ecdsa · 2018-01-18T09:34:20Z

@ecdsa are you open to engaging in a discussion about a more generic standard e.g like the above?

can you be more specific?

junderw · 2018-01-18T09:56:45Z

Why has this become a thing?

Have you ever used Electrum? Electrum made it a thing since before BIP32 was even a thing.

The rationale is that you can't export individual private keys from Electrum, so it's not a problem. (iirc)

mautematico · 2018-02-04T02:23:33Z

Why is this necessary anyway? Who is actually passing around these base58 strings rather than a mnemonic?

To create watch-only wallets for internal (but secure) use; just like I am trying to do right now.

The rationale is that you can't export individual private keys from Electrum, so it's not a problem. (iirc)
Yes, you can export individual private keys from electrum.

Wallet >> Private Keys >> Export

dabura667 · 2018-02-04T04:37:08Z

watch-only wallets for internal (but secure) use

Watch-only wallets are easy to hack: Just replace the xpub with a hacker's xpub. Now everyone will send money to the hacker and not you.

mautematico · 2018-02-04T18:57:54Z

Watch-only wallets are easy to hack: Just replace the xpub with a hacker's xpub. Now everyone will send money to the hacker and not you.

That is pointless; exactly the same can be done regardless if you are using xpub or not. So no "xpub hack" here.

dabura667 · 2018-02-04T23:58:51Z

This PR is not complete, that’s why it’s not working.

clarkmoody · 2018-02-08T23:34:08Z

We also have the SLIP-32 proposal out there, which encodes the derivation path within the serialized xpub data payload.

plasticlobster · 2018-02-09T05:41:28Z

@mautematico You're right. getAddress would need modification to get the proper address, but you can do it as follows:

const Bitcoin = require('bitcoinjs-lib');

Bitcoin.address.fromOutputScript(
  Bitcoin.script.scriptHash.output.encode(
    Bitcoin.crypto.hash160(
      Bitcoin.script.witnessPubKeyHash.output.encode(
        Bitcoin.crypto.hash160(
          Bitcoin.HDNode.fromBase58('ypub6XMTwf6NSvfzYYgVgdNWRNfMTiQt4rSjZbEk8qoCnBGhUD2rsgZ2A8pexgzaGLKgySZxqxrctDpAVU8QtfxqfX8QUAhtFmGFUFx9B51TVg8', Bitcoin.networks.bitcoin)
          .derive(0)
          .derive(27)
          .getPublicKeyBuffer()
        )
      )
    )
  ),
  Bitcoin.networks.bitcoin
)

Obviously a change like this would have to be implemented in getAddress to make this easier, but I don't even want to pretend I understand the coding requirements of this project enough to propose that change.

dcousens · 2018-02-12T04:13:01Z

For context on 4.0.0, the above code becomes:

const bitcoin = require('bitcoinjs-lib')
const pubKey = bitcoin.HDNode.fromBase58('ypub6XMTwf6NSvfzYYgVgdNWRNfMTiQt4rSjZbEk8qoCnBGhUD2rsgZ2A8pexgzaGLKgySZxqxrctDpAVU8QtfxqfX8QUAhtFmGFUFx9B51TVg8')
  .derive(0)
  .derive(27)
  .getPublicKeyBuffer()

const { address } = bitcoin.payments.p2sh({
  redeem: bitcoin.payments.p2wpk({
    pubkey: pubKey
  })
})

dcousens · 2018-02-12T04:24:34Z

ping @afk11

afk11 · 2018-03-03T23:46:46Z

I think the PR needs to address generating the scriptPubKey/address for it to be complete, first of all.

It'd really be nice for it to work with the other BIP's that do this custom HD prefix to indicate an address. There's BIP49, and BIP84, each has custom HD prefixes implementing the same logic, which is constructing a HD key capable of producing a P2SH|P2WPKH address, or P2WPKH respectively. Electrum takes this a step further, and defines 5 prefix/scriptTypes for which they do this.

I've been thinking about the missing address generation part of this PR: could we add an optional parameter to the hdkey constructor, a closure accepting a public or private key, and returns a {scriptPubKey:, rs:, ws:, address: } when called? We can add the optional parameter to the generation methods as well. Derivations could pass on the closure to children as well.

hdkey.getAddress could then check if the closure was set, and call it if so. If it wasn't set, just return P2PKH. A default closure of P2PKH could be how we make this work, actually. Maybe we want to add another function to hdkey, that returns {scriptPubKey:, rs:, ws:, address: }, since getAddress exclusively returns the address. This information would be really handy for signing time!

That way, we can inject different kinds of closures to work with all the types that electrum supports (http://docs.electrum.org/en/latest/seedphrase.html):

P2PKH (xprv)
P2SH|P2PKH (xprv - conflicts)
P2SH|P2WPKH (yprv)
P2SH|P2WSH|P2PKH (Yprv)
P2WPKH (zprv)
P2WSH|P2PKH (Zprv)

It'd be nice to support the 5 (or more) serialization formats as well. One issue I have with the PR is that the deserialization will succeed even for people who don't want the feature. It'd be nice if people could control which prefix/scriptTypes are active during a deserialization? Could we find a way to pass in the extra prefixes/scriptTypes/closures for consideration in a call

Maybe the prefixes/scriptTypes could be defined in the network, then a selection of these can be passed into deserialize/serialize methods?

Whew.. this one's been on my mind anyway, overall the feature is nice to have if it's future proof against script-types, and developers need to opt into the behavior somehow!

clarkmoody · 2018-03-03T23:49:13Z

We're putting these registered HD prefix types in SLIP-0132, to pull the reference out of any one implementation's docs. https://github.com/satoshilabs/slips/blob/master/slip-0132.md

…

-Clark

On Sat, Mar 3, 2018 at 5:46 PM, Thomas Kerin ***@***.***> wrote: I think the PR needs to address generating the scriptPubKey/address for it to be complete, first of all. It'd really be nice for it to work with the other BIP's that do this custom HD prefix to indicate an address. There's BIP49, and BIP84, each has custom HD prefixes implementing the same logic, which is constructing a HD key capable of producing a P2SH|P2WPKH address, or P2WPKH respectively. Electrum takes this a step further, and defines 5 prefix/scriptTypes for which they do this. I've been thinking about the missing address generation part of this PR: could we add an optional parameter to the hdkey constructor, a closure accepting a public or private key, and returns a {scriptPubKey:, rs:, ws:, address: } when called? We can add the optional parameter to the generation methods as well. Derivations could pass on the closure to children as well. hdkey.getAddress could then check if the closure was set, and call it if so. If it wasn't set, just return P2PKH. A default closure of P2PKH could be how we make this work, actually. Maybe we want to add another function to hdkey, that returns {scriptPubKey:, rs:, ws:, address: }, since getAddress exclusively returns the address. This information would be really handy for signing time! That way, we can inject different kinds of closures to work with all the types that electrum supports (http://docs.electrum.org/en/ latest/seedphrase.html): - P2PKH (xprv) - P2SH|P2PKH (xprv) - P2SH|P2WPKH (yprv) - P2WSH|P2PKH (Yprv) - P2WPKH (zprv) - P2WSH ???? (this doesn't say which type, just p2wsh) It'd be nice to support the 5 (or more) serialization formats as well. One issue I have with the PR is that the deserialization will succeed even for people who don't want the feature. It'd be nice if people could control which prefix/scriptTypes are active during a deserialization? Could we find a way to pass in the extra prefixes/scriptTypes/closures for consideration in a call Maybe the prefixes/scriptTypes could be defined in the network, then a selection of these can be passed into deserialize/serialize methods? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#927 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABfXMZ_xDzySJB0QEK-Sd5huwKc06vjGks5taytugaJpZM4QixnQ> .

afk11 · 2018-03-04T18:58:46Z

I took a quick look at getting getAddress to work, but a mocked getAddress test fails, since keyPair.getAddress was overwritten and now the results aren't identical.

Makes me wonder if keyPair should have getAddress, but anyways. I'll comment the test for now since the addresses will vary going forward!

#1014 - allows 'extra' prefixes to be provided by developers, or allows regular behavior of using the networks bip32 prefix.

dcousens · 2018-04-12T04:15:05Z

Related to bitcoin/bips#673 ?
Not HD though...

liangfenxiaodao · 2018-04-30T00:33:15Z

@mautematico may I have one question?

for this code:

Bitcoin.HDNode.fromBase58('ypub6XMTwf6NSvfzYYgVgdNWRNfMTiQt4rSjZbEk8qoCnBGhUD2rsgZ2A8pexgzaGLKgySZxqxrctDpAVU8QtfxqfX8QUAhtFmGFUFx9B51TVg8', Bitcoin.networks.bitcoin)
    .derive(0)
    .derive(27)
    .getAddress()

I'd like to know what's the meaning of derive(0) and then derive(27), could you please explain a little bit?

Thank you very much.
Jacky.

mautematico · 2018-04-30T02:43:18Z

@jackylimel sure,

have you read about bip32 derivation paths? They let you generate multiple public-private key pairs from a single initial master public-private pair (ypub & ypriv).

twice .derive(0).derive(0) is the same as .derivePath("m/0/0")

https://bitcoin.stackexchange.com/questions/56561/bip32-derivation-path
http://bip32.org/

Overtorment · 2018-07-08T14:22:44Z

for anyone waiting for ypub support in bitcoinjs, heres a quick workaround I borrowed from #966

    // bitcoinjs does not support ypub yet, so we just convert it from xpub
    const b58 = require('bs58check')
    let data = b58.decode(xpub)
    data = data.slice(4)
    data = Buffer.concat([Buffer.from('049d7cb2','hex'), data])
    return b58.encode(data);

sneurlax · 2018-07-09T23:48:59Z

@Overtorment, thanks for that. This should be the zpub equivalent workaround:

let data = b58.decode(xpub)
data = data.slice(4);
data = Buffer.concat([Buffer.from('04b24746','hex'), data]); // see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-September/014907.html for tenative (non-BIP-official) version bytes
return b58.encode(data);

EDIT: SLIP-0132 is a better source than the email linked above until it's in an actual BIP

dcousens · 2018-07-19T09:33:24Z

What do I do here?
With the removal of getAddress...

Thoughts?

youssefgh · 2019-08-23T16:27:56Z

@dcousens i believe that "YPUB/UPUB" is now a part of BIP49. See Extended Key Version
What do you think ?

Support for BIP49 YPUB/UPUB

8f39b51

dcousens added the feature label Nov 28, 2017

dcousens requested a review from afk11 November 28, 2017 04:47

afk11 reviewed Dec 15, 2017

View reviewed changes

afk11 suggested changes Dec 15, 2017

View reviewed changes

karelbilek mentioned this pull request Dec 18, 2017

Different bip32 prefixes for various output scripts #934

Closed

dcousens mentioned this pull request Dec 23, 2017

Trezor generated xPub - 'Invalid network version' #966

Closed

dcousens self-assigned this Jan 17, 2018

dcousens approved these changes Jan 17, 2018

View reviewed changes

dcousens modified the milestones: 4.0.0, 3.4.0 Jan 17, 2018

This was referenced Jan 25, 2018

Support for extended LTC-keys in xprv/xpub format #996

Closed

API for 4.0.0 #508

Closed

dcousens removed this from the 3.4.0 milestone May 14, 2018

dcousens closed this Jul 23, 2018

yuradmt mentioned this pull request Jan 19, 2020

How to generate a BIP32 xpub from a mnemonic for segwit accounts? #1535

Closed

AndreasGassmann mentioned this pull request Sep 6, 2020

Questions regarding SegWit, PSBT and RBF #1616

Closed

junderw mentioned this pull request Aug 21, 2023

What's the proper and latest way to generate child public keys out of an extended pub key? #1238

Closed

Support for BIP49 YPUB/UPUB #927

Support for BIP49 YPUB/UPUB #927

Conversation

plasticlobster commented Nov 17, 2017

dcousens commented Nov 18, 2017

plasticlobster commented Nov 18, 2017

dcousens commented Nov 28, 2017 • edited

afk11 Dec 15, 2017 • edited

Choose a reason for hiding this comment

dcousens Jan 17, 2018

Choose a reason for hiding this comment

dcousens Jan 17, 2018 • edited

Choose a reason for hiding this comment

afk11 Mar 3, 2018

Choose a reason for hiding this comment

afk11 left a comment

Choose a reason for hiding this comment

afk11 commented Dec 15, 2017 • edited

dcousens left a comment

Choose a reason for hiding this comment

dcousens commented Jan 17, 2018 • edited

dcousens commented Jan 17, 2018

dabura667 commented Jan 17, 2018

dcousens commented Jan 18, 2018

junderw commented Jan 18, 2018

dcousens commented Jan 18, 2018 • edited

dcousens commented Jan 18, 2018 • edited

junderw commented Jan 18, 2018

junderw commented Jan 18, 2018

junderw commented Jan 18, 2018

dcousens commented Jan 18, 2018 • edited

junderw commented Jan 18, 2018

dcousens commented Jan 18, 2018 • edited

ecdsa commented Jan 18, 2018

dcousens commented Jan 18, 2018

junderw commented Jan 18, 2018

dcousens commented Jan 18, 2018 • edited

ecdsa commented Jan 18, 2018

junderw commented Jan 18, 2018

mautematico commented Feb 4, 2018 • edited

dabura667 commented Feb 4, 2018

mautematico commented Feb 4, 2018 • edited

dabura667 commented Feb 4, 2018

clarkmoody commented Feb 8, 2018

plasticlobster commented Feb 9, 2018

dcousens commented Feb 12, 2018 • edited

dcousens commented Feb 12, 2018

afk11 commented Mar 3, 2018 • edited

clarkmoody commented Mar 3, 2018 via email

afk11 commented Mar 4, 2018 • edited

dcousens commented Apr 12, 2018 • edited

liangfenxiaodao commented Apr 30, 2018

mautematico commented Apr 30, 2018

Overtorment commented Jul 8, 2018

sneurlax commented Jul 9, 2018 • edited

dcousens commented Jul 19, 2018

youssefgh commented Aug 23, 2019

dcousens commented Nov 28, 2017 •

edited

afk11 Dec 15, 2017 •

edited

dcousens Jan 17, 2018 •

edited

afk11 commented Dec 15, 2017 •

edited

dcousens commented Jan 17, 2018 •

edited

dcousens commented Jan 18, 2018 •

edited

dcousens commented Jan 18, 2018 •

edited

dcousens commented Jan 18, 2018 •

edited

dcousens commented Jan 18, 2018 •

edited

dcousens commented Jan 18, 2018 •

edited

mautematico commented Feb 4, 2018 •

edited

mautematico commented Feb 4, 2018 •

edited

dcousens commented Feb 12, 2018 •

edited

afk11 commented Mar 3, 2018 •

edited

afk11 commented Mar 4, 2018 •

edited

dcousens commented Apr 12, 2018 •

edited

sneurlax commented Jul 9, 2018 •

edited