search

bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.69k stars 2.11k forks source link

Questions regarding SegWit, PSBT and RBF #1616

Closed AndreasGassmann closed 4 years ago

AndreasGassmann commented 4 years ago

Hi all. In our library, which is targeted towards offline signing, we currently only support legacy bitcoin addresses and are using bitgo-utxo-lib.

I now want to improve our library and add segwit support. This library here seems to be more up to date, so I'm planning on switching.

My goal is the following:

Would this be considered the best practice for a bitcoin wallet at the moment?

While trying to implement this, I ran into a couple of issues:

How to deal with xPub, yPub and zPub

I usually use https://iancoleman.io/bip39/ as a reference. I noticed that when I select the "BIP44", "BIP49" and "BIP84" tabs, the the extended public keys have a different prefix (xPub, yPub, zPub for mainnet and tPub, uPub, vPub for testnet).

At first I ignored it, but when I sent the xPub to blockbook to get my UTXOs, I got an empty array back with my xPub, but it worked with the zPub key.

I read some discussions around this (https://github.com/bitcoinjs/bitcoinjs-lib/pull/927, https://github.com/bitcoinjs/bitcoinjs-lib/issues/1334, https://github.com/trezor/connect/issues/98), but I'm not sure what the current state is. It was mentioned that it was not in the BIP49 standard, but now it seems to be there: https://github.com/bitcoin/bips/blob/master/bip-0049.mediawiki#extended-key-version

So what is the proper way to handle this?

PSBT signing support

We currently get the UTXOs from an API on an online device, select the inputs we want and then send this in a custom JSON structure to our signer. The signer takes the values and creates a transaction using the TransactionBuilder and signs them, completely offline.

It seems to me that the best way going forward would be to use the PSBT standard to do that. I read the BIP174, but it's not clear to me what different cases need to be handled by the signer.

In our online wallet, when preparing the transaction, we will only have segwit addresses as the origin. But our signer should be able to also handle PSBTs that come from Electrum/Coldcard/etc. So using the example here, would I be able to sign every possible valid transaction that can be given to the signer via PSBT? (Legacy addresses, segwit addresses, custom scripts, timelock, etc). Or is there some additional work required to handle all the different cases?

RBF (Replace by fee)

It seems that this was requested by a couple of people, but a long time ago the decision was made to not include it in this library https://github.com/bitcoinjs/bitcoinjs-lib/issues/521.

Are there any plans to reconsider this? As far as I've seen, this seems like one of the features people see as a standard in bitcoin wallets.

If this is not planned, are there any other projects/examples that handle this and are compatible with this library?

Thanks a lot for taking the time to answer. I spent days reading about all of these issues, but it's hard to find concrete and up-to-date answers because the ecosystem is constantly evolving. I hope you can shed some light on this so I know what I should try to implement 😃.

BTW: The library and apps that will use this are completely open source, so once I have it working I would be willing to create some example scripts that can be added to this library so other people can learn from them as well.

junderw commented 4 years ago

I'll respond in a bit.

Thanks for your questions.

junderw commented 4 years ago

How to deal with xPub, yPub and zPub

That will have to be done with your app's logic. Then convert then into xpubs. Currently bitcoinjs-lib only accepts xpub. You can do this by using bs58check to decode, check the first 4 bytes (version) to see if it matches zpub ypub or xpub's version bytes, mark down which one it was, and encode it using the xpub version bytes.

PSBT signing support

We have implemented all of the signer responsibilities. So if you manage the private keys using us, it's as simple as running one of the sign methods.

For hardware support it's a little more tricky, as you need to extrapolate some extra info from the PSBT and hand it over to the hardware wallet directly... See this issue for some ideas on how to implement: #1517

RBF (Replace by fee)

PSBT has the setInputSequence method, which you can use to set all inputs to 0xfffffffd and you will be RBF.

RBF is not too difficult, but it is not consensus code... so including it in this library is not logical.

However, currently RBF is defined as "Any transaction where all input sequences are equal to or less than MAX_VALUE - 2" so it will be easy to create a small helper function in your app, especially if you have no other uses for the sequence value.

AndreasGassmann commented 4 years ago

Thank you for your response. I'll look into it and let you know how it goes.

sidhujag commented 4 years ago

You can use https://github.com/Anderson-Juhasc/bip84 we solved this problem (analogous to xpub/ypub/zpub) for anyone who is interested using my work here to consume bip84 which will work with legacy/bech32 address types: https://github.com/syscoin/syscoinjs-lib

Note it will also work with Bitcoin and assumes a blockbook backend as well. Motivation was to do kind of like web3js for Bitcoin type environments. We are actually integrating this into a fork of Metamask so it can call out metamask once you request signing a transaction which again can be applicable to Bitcoin.

This is where it will derive based on bip84 vs bip44 https://github.com/syscoin/syscoinjs-lib/blob/master/utils.js#L72 where you pass in the bip number depending on if you are doing legacy or bech32 (bip84) where it will use ZPub/ZPrv otherwise will use XPub/XPrv and testnet is always YPub/YPrv, but note the library needs this pull request accepted for it to work OOTB: https://github.com/Anderson-Juhasc/bip84/pull/5

My library syscoinjslib also deals with RBF you can see it from the test fixtures how RBF would work which does what @junderw says by setting the sequence flag: https://github.com/syscoin/syscoinjs-lib/blob/master/test/fixtures/index.js#L815

Some of the nifty features we do are pertaining to just Bitcoin should you wish to use:

1) Use BigNumber to deal with satoshi's to avoid rounding errors or 53 digit precision issues in JS, all thoughout our stack into the coinselect library 2) Optimize fees further as a post processing step by analyzing change addresses and using a two algo strategy using blackjack (fill with no change first) and fallback to accumulative (ordered descending from highest value utxo to fund transactions with change addresses). 3) Flexible fees by passing in fee rate (likely should be called from blockbook's estimate fee API) to get fee rate based on confirmation speed desired. 4) Change address strategy (default is to always use change for quantum resistance and little bit of privacy (doesn't randomize change address output index currently but certainly can). Pulls in latest free change and recv index from blockbook utxo call and updates is as transactions are sent or are being funded. 5) Uses this library (bitcoinjs-lib) for psbt and HD signing and HD key management built-in. AES256 encrypted (to password) local storage of seeds for persistence on web clients (disabled automatically for nodejs server side deployment).

Anyways all the work we do is with Bitcoin compatibility in mind, this should all work with Bitcoin blockbook and transaction funding, whilst using XPub/YPub/Zpub abstracting the complexity in an SDK design with HD change address strategy built-in. If you think the code will work I would be happy to help in any way you need. Forgive my coding style as I am more of a C/C++ guy but PR's are welcome. Hopefully it can help you.

junderw commented 4 years ago

Use BigNumber to deal with satoshi's to avoid rounding errors or 53 digit precision issues in JS, all thoughout our stack into the coinselect library

This is not needed for Bitcoin since value is always dealt with using integers, and the largest possible UTXO value is 23.3% the size of MAX_SAFE_INTEGER.

> (21 * 1e6 * 1e8) / Number.MAX_SAFE_INTEGER
0.2331468351712829
sidhujag commented 4 years ago

Use BigNumber to deal with satoshi's to avoid rounding errors or 53 digit precision issues in JS, all thoughout our stack into the coinselect library

This is not needed for Bitcoin since value is always dealt with using integers, and the largest possible UTXO value is 23.3% the size of MAX_SAFE_INTEGER.

> (21 * 1e6 * 1e8) / Number.MAX_SAFE_INTEGER
0.2331468351712829

Yup, agreed unless you are doing some calculations with sums and stuff. I put it in there because we have an asset layer which can extend to int64 but also incase people build statistics for whatever reason into it.

AndreasGassmann commented 4 years ago

@sidhujag Thanks for sharing your code. I still didn't have time to look into this, but if I have any issues while implementing it, I'll also look at your code.

We have our own library with a shared interface between all coins we support. This allows us to easily integrate them into our apps, without having any protocol specific code in our apps.

AndreasGassmann commented 4 years ago

So I finally had time to look into this. I'm very close, but there are still a few things missing.

That will have to be done with your app's logic. Then convert then into xpubs. Currently bitcoinjs-lib only accepts xpub. You can do this by using bs58check to decode, check the first 4 bytes (version) to see if it matches zpub ypub or xpub's version bytes, mark down which one it was, and encode it using the xpub version bytes.

That works.

PSBT has the setInputSequence method, which you can use to set all inputs to 0xfffffffd and you will be RBF.

Got that working as well, thanks.

We have implemented all of the signer responsibilities. So if you manage the private keys using us, it's as simple as running one of the sign methods. For hardware support it's a little more tricky, as you need to extrapolate some extra info from the PSBT and hand it over to the hardware wallet directly... See this issue for some ideas on how to implement: #1517

This is where I still have some problems. We are only using bitcoinjs, no hardware wallets involved.

I fail to construct a standard segwit => segwit transaction and broadcast it to the network. Our use-case is that we have an online part with only the extended public key, and an offline part that has the extended private key. We do have access to the full mnemonic on offline signer, but I'm not sure if that is needed (ideally it is not and we only use the extended private key).

When I try that, the node rejects my transaction with the following error:

Error validating transaction: Error running script for input 0 referencing 2b714ec0d0b7f0d98de877430551b47d8ed2e20af8aed4b11e12aaf3aafb0cec at 0: nonempty scriptsig in witness transaction.

Preparing transaction

public async prepareTransactionFromExtendedPublicKey(
    extendedPublicKey: string,
    offset: number,
    recipients: string[],
    values: string[],
    fee: string
  ): Promise<any> {

    // Some logic to select the right inputs and outputs and store them in the "transaction" object

    const psbt = new bitcoinJS.Psbt()
    const keyPair = bitcoinJS.bip32.fromBase58(new ExtendedPublicKey(extendedPublicKey).toXpub())
    const p2sh = bitcoinJS.payments.p2sh({
      redeem: bitcoinJS.payments.p2wpkh({ pubkey: keyPair.publicKey, network: this.options.network.extras.network }),
      network: this.options.network.extras.network
    })

    const p2shOutput = p2sh.output
    const p2shRedeem = p2sh.redeem

    if (!p2shOutput) {
      throw new Error('no p2shOutput')
    }
    if (!p2shRedeem) {
      throw new Error('no p2shRedeem')
    }

    transaction.outs.forEach((out) => {
      psbt.addOutput({ address: out.recipient, value: parseInt(out.value) })
    })

    const RBF: boolean = true // Replace by fee, should be passed by the user
    transaction.ins.forEach((tx) => {
      // const childNode = keyPair.derivePath(tx.derivationPath!)
      psbt.addInput({
        hash: tx.txId,
        index: tx.vout,
        redeemScript: p2shRedeem.output,
        sequence: RBF ? 0xfffffffd : undefined, // Needs to be at least 2 below max int value to be RBF
        witnessUtxo: {
          script: p2shOutput,
          value: parseInt(tx.value)
        }
        // bip32Derivation: [
        //   {
        //     masterFingerprint: keyPair.fingerprint,
        //     pubkey: childNode.publicKey,
        //     path: tx.derivationPath!
        //   }
        // ]
      })
    })
    const tx: RawBitcoinSegwitTransaction = {
      psbt: psbt.toBase64()
    }

    return tx
}

Signing method

  public async signWithExtendedPrivateKey(extendedPrivateKey: string, transaction: any): Promise<string> {
    const keyPair = bitcoinJS.bip32.fromBase58(extendedPrivateKey, this.options.network.extras.network)

    const psbt = bitcoinJS.Psbt.fromBase64(transaction.psbt)

    psbt.signAllInputs(keyPair)
    psbt.finalizeAllInputs()

    return psbt.toBase64()
  }

Transaction: 02000000000101ec0cfbaaf3aa121eb1d4aef80ae2d28e7db451054377e88dd9f0b7d0c04e712b00000000171600149cb04e7e6c551c3d9124e5ff79105d87dbc9de4bfdffffff02d0070000000000001600147dcad5c0346d8e29f7e0d110ddd5d1721494ace9d60d0000000000001600145805b8830c2822d4c89f5f2dafa349679e4cdf5d0248304502210089525b82d5126782ba304436947ddf810d3b83f51b000e0f656c8bae09d1c33502204deeeffe37e8ffe22eb3a8a784a1372ef77166ecc31ff74df3fe44b1203a78cf012102ff3694e6f5ae01195ab8e4e717d06d7faffaf7f759d62e400c8a5c564f3e5c0700000000

There are a few things I don't understand yet about PSBT:

Thanks again for your help!

junderw commented 4 years ago

After a quick glance, you're using the wrong index for addInput.

It is not the index of some input, it is the index within the tx pointed to by hash of the output being used/spent in this input.

"I want to spend the "index"th output of the tx at "hash"" is what you're saying with the info.

junderw commented 4 years ago

index starts at 0

AndreasGassmann commented 4 years ago

Ok, that makes sense. I actually did use the vout value as well during testing. But in this specific case, both the indexes resulted in 0, so I don't think this is the problem.

This is what the blockbook API returns for my zpub key:

[
  {
    "txid": "2b714ec0d0b7f0d98de877430551b47d8ed2e20af8aed4b11e12aaf3aafb0cec",
    "vout": 0,
    "value": "7542",
    "height": 643770,
    "confirmations": 8401,
    "address": "bc1q0h9dtsp5dk8znalq6ygdm4w3wg2fft8fkpnmxu",
    "path": "m/84'/0'/0'/0/0"
  }
]

EDIT: I also updated the code above to use tx.vout instead of the foreach index, so people don't copy paste the wrong code.

junderw commented 4 years ago
  • It seems that the inputs do not have a derivation path. So how does the sign method know how to sign the inputs? Do I have to use the signAllInputsHD method for that? And if yes, what should the bip32Derivation object look like? I took a look at the tests, but there the derivation path was a full derivation path starting from the mnemonic/bip32 root seed. So how can this be done with the extended private key only?

If you are using an HD key you MUST include bip32Derivation and you MUST use a signHD method. Since an HD key fills all the interfaces for a non-HD key, when you use signAllInputs by passing the root HD private key, you are literally signing it with the root m key ONLY no derivation is taking place. This is your problem. Alternatively, you can derive the key in YOUR logic and pass the derived* key into each input that is related to that derivation.

  • What does "finalising" an input actually do? If we want to support multisig, we probably don't have to finalize the inputs and instead do that on the combiner/online part of the flow? (in the first version we only want to support signing of multisigs, not preparing multisig transactions. So we could for example get such a multisig transaction from electrum or specter).

Finalizing takes all the PSBTInput info and uses it to create the finalScriptSig and finalScriptWitness attributes, then deletes all unneeded info for that input. This step was added exactly for the use case you describe; If I pass out a multisig PSBT, and I get back 5 PSBTs each with one different signature each, I can merge the PSBTs easily if the signatures are separated out into their own logical section (partialSig), but if someone tried to make an attempt at "creating an incomplete scriptSig / witness" before I get the PSBTs, then I will have a hard time merging them if one of the signers has a bad implementation.

So finalizing step being an explicit separate step is to logically separate the actions of data gathering and final input creation. It makes the protocol MUCH more flexible.

  • How can I get information out of the PSBT class? This is probably related to #1559. I would like to display all the information on the offline signer so people know what they are actually signing.

https://github.com/bitcoinjs/bitcoinjs-lib/blob/533d6c2e6d0aa4111f7948b1c12003cf6ef83137/ts_src/psbt.ts#L166-L186

psbt.txInputs[0].sequence // this will give the sequence of the first input.

You can get the tx info from here. As far as the other info, you can access it directly from the psbt.data.inputs (ie. value from witnessUtxo etc. exactly how you input the data you can access it)

AndreasGassmann commented 4 years ago

Thanks so much for your help. I was now able to do a transaction now, but I still had to hardcode some of the things. For example, the "path" in the bip32Derivation object was m/84'/0'/0'/0/0, but because I don't have the root key in the sign method, but rather the xprv, that obviously didn't work (Only the 0/0 path would have to be applied). But when I tried to change the path in the bip32Derivation object to 0/0, then it didn't work at all (it seemed to internally replace it with m/0?)

So what I now had to do was use the mnemonic in the sign method to get the root key. Then the derivation path was correctly applied and everything worked.

Is it somehow possible to work only with the xpub/xprv keys without accessing the root key?

Alternatively, you can derive the key in YOUR logic and pass the derived key into each input that is related to that derivation.

What is the default bip32Derivation format? Is it always the full path like m/84'/0'/0'/0/0, or can it be a partial path like 0/0? Because I would like to stay within the standard. And if the second one is not a standard and I want to have our own logic to derive the keys myself, where could I store the partial derivation path in the PSBT?

Thanks a lot for your help. I think I'm done for today, but I can hopefully finish the proof of concept over the next few days.

junderw commented 4 years ago

There is no good reason to store only a derived xprv.

You should have a mnemonic, which means you have the root.

bip32Derivation is defined in PSBT BIP174

  1. root key fingerprint
  2. The path from root to the key
  3. The pubkey of that derived key
sidhujag commented 4 years ago

jfyi you should look at my code if you need help, its all there. Let me know if you can't figure it out still. Its bip44/84 compliant as well.