bitcoinjs / bitcoinjs-lib

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

Can't sign P2SH(P2WPKH) transaction built with CoinSelect and TransactionBuilder not working #1154

Closed majestic84 closed 5 years ago

majestic84 commented 5 years ago

I am trying to build a transaction and everything seems to be working well up until the point where I sign the transaction. I keep getting "scripthash not supported" but I am not sure what I am doing wrong. I followed the example from here https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/test/integration/transactions.js#L143 closely, with the main difference being that I am using CoinSelect to choose the utxos for me. I've posted my code below, any help would be greatly appreciated :)

`const bitcoin = require('bitcoinjs-lib') let network = bitcoin.networks.bitcoin

        const getUtxos =
            util
                .promisify(this.network.provider.insight.getUtxos)
                .bind(this.network.provider.insight)
        const utxos = await to(
            getUtxos({addresses: from, minconf: this.network.provider.minConf})
        )

        if (utxos.data.length === 0)
            return({success: false, error: `No confirmed unspent transactions found: ${from}`})

        let jsonUtxos = []
        utxos.data.forEach(utxo => {
            let jsonUtxo = utxo.toJSON()
            jsonUtxo.value = utxo.satoshis
            jsonUtxos.push(jsonUtxo)
        })

        let keyPair = bitcoin.ECPair.fromWIF(key, network)
        const p2wpkh = bitcoin.payments.p2wpkh({ pubkey: keyPair.publicKey, network })
        const p2sh = bitcoin.payments.p2sh({ redeem: p2wpkh, network })

        let amountInSatoshis = this.network.provider.bitcore.Unit.fromBTC(amount).toSatoshis()

        let coinSelect = require('coinselect')
        let feeRate = 110

        let targets = [{
                address: _to,
                value: amountInSatoshis
            }]

        let { inputs, outputs, fee} = coinSelect(jsonUtxos, targets, feeRate)
        if (!inputs || !outputs) return

        let txb = new bitcoin.TransactionBuilder(network)

        inputs.forEach(input => txb.addInput(input.txid, input.vout))
        outputs.forEach(output => {
            if (!output.address) {
                output.address = from
            }
            txb.addOutput(output.address, output.value)
        })

        txb.sign(0, keyPair, p2sh.redeem.output, null, inputs.value) // <-- scripthash not supported

        const tx = txb.build()
        const txid = tx.toHex()`
dabura667 commented 5 years ago

check p2sh right before sign and show me its contents.

the error you describe would only happen if p2sh.redeem.output was a p2sh output and not the redeemscript itself.

majestic84 commented 5 years ago

Thanks for the quick response. Here's the output of p2sh just before signing. This attempt is with a Litecoin transaction. Same code as above, but with the valid network parameters for Litecoin Testnet:

{ network:
   { messagePrefix: '\u0019Litecoin Signed Message:\n',
     bip32: { public: 70711009, private: 70709117 },
     pubKeyHash: 111,
     scriptHash: 58,
     wif: 239 },
  address: [Getter/Setter],
  hash: [Getter/Setter],
  output: [Getter/Setter],
  redeem:
   { network:
      { messagePrefix: '\u0019Litecoin Signed Message:\n',
        bip32: [Object],
        pubKeyHash: 111,
        scriptHash: 58,
        wif: 239 },
     address: [Getter/Setter],
     hash: <Buffer 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2>,
     output: <Buffer 00 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2>,
     pubkey: <Buffer 03 3e 1b 4c 0b 3d 9f e1 20 3a b0 79 51 e1 be 81 30 82 86 96 86 87 f7 a8 36 87 9c 0d 6e 17 31 e4 23>,
     signature: [Getter/Setter],
     input: undefined,
     witness: undefined },
  input: [Getter/Setter],
  witness: [Getter/Setter] }

So p2sh.redeem.output here is a buffer containing 00 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2

I should also mention that here I am trying to send from a BIP39/BIP44 derived address. I derived the first address m/44'/2'/0'/0 and its private key, and am trying to transact with those values. I do have the xprv and the mnemonic phrase, but unless I understood the concept wrong, I should be able to work with the child keys, as this is what my customers will be using.

dabura667 commented 5 years ago

The only thing I can think of as a pot hole you could be falling into is some sort of scope problem with your variables...

Perhaps keyPair, even though you use it to create p2sh there, is a different key than p2sh...

https://github.com/bitcoinjs/bitcoinjs-lib/blob/d67e9758c72e587083e2d05fa802e02c76ca99ca/src/transaction_builder.js#L187

This seems like the only place in the flow of logic where you would fall through.

basically it grabs the hash from your redeemScript (the last 20 bytes) and hash160s your keyPair object again and compares... if they're not equal it doesn't return a pubkeys attribute, then this error triggers:

https://github.com/bitcoinjs/bitcoinjs-lib/blob/d67e9758c72e587083e2d05fa802e02c76ca99ca/src/transaction_builder.js#L269

expanded is the result returned when those two values aren't correct, and the error is thrown when the pubkeys attribute isn't there.

But it says scripthash... so it looks like it's classifying your redeemScript as a p2sh output instead of a p2wpkh output...

could you inspect the script object right before classification here:

https://github.com/bitcoinjs/bitcoinjs-lib/blob/d67e9758c72e587083e2d05fa802e02c76ca99ca/src/transaction_builder.js#L163

and tell me what it says?

majestic84 commented 5 years ago

The value of script just before classify.output(script) is <Buffer 00 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2> and the value of "type" just after it is witnesspubkeyhash

I've been digging a lot on this issue, and since I am using bitpay's insight to get my UTXOs, is it possible my "ins" are malformed? Here is the output of my ins, if that is helpful:

[ { address: 'QT6zg9WQxp5PzoXk2Hx6NzT7ZS4pkd4LiU',
    txid: 'e94ed043eb73fdf1cf27556a2658907e9d860660f359d303cc412e4c0dbbb032',
    vout: 1,
    scriptPubKey: 'a914474d4a554d32b865fbde4be109ec9a20db6d781d87',
    amount: 3,
    value: 300000000 } ]

Here is a standalone function with all the relevant data for this example:

send.js

const explorers = require('litecore-explorers')
const bitcoin = require('bitcoinjs-lib')
const coinSelect = require('coinselect')
const util = require('util')

const send = async () => {
    let litecore = explorers.litecore
    let insight = new explorers.Insight('https://testnet.litecore.io', 'testnet')
    let network = bitcoin.networks.litecoinTestnet
    litecore.Networks.defaultNetwork = litecore.Networks.testnet

    let from = 'QT6zg9WQxp5PzoXk2Hx6NzT7ZS4pkd4LiU'
    let _to = 'mgX1cT4p414nFKm4ZWrWgvKeKGvujkpP7Z'
    let amount = 0.001
    let key = ''

    // Validate the wallets.
    if (!litecore.Address.isValid(from))
        return({success: false, error: `Invalid wallet (from): ${from}`})
    if (!litecore.Address.isValid(_to))
        return({success: false, error: `Invalid wallet (to): ${_to}`})

    // Get unspent transactions
    const getUtxos =
        util
            .promisify(insight.getUtxos)
            .bind(insight)

    let jsonUtxos = []
    await getUtxos({addresses: from, minconf: 0})
        .then(utxos => {
            utxos.forEach(utxo => {
                let jsonUtxo = utxo.toJSON()
                jsonUtxo.value = utxo.satoshis
                jsonUtxos.push(jsonUtxo)
            })
        })

    let keyPair = bitcoin.ECPair.fromWIF(key, network)
    let p2wpkh = bitcoin.payments.p2wpkh({ pubkey: keyPair.publicKey, network })
    let p2sh = bitcoin.payments.p2sh({ redeem: p2wpkh, network })

    let amountInSatoshis = litecore.Unit.fromBTC(amount).toSatoshis()
    let feeRate = 110 // satoshis per byte

    let targets = [{
        address: _to,
        value: amountInSatoshis
    }]

    let { inputs, outputs, fee} = coinSelect(jsonUtxos, targets, feeRate)
    if (!inputs || !outputs) return

    let txb = new bitcoin.TransactionBuilder(network)

    inputs.forEach(input => txb.addInput(input.txid, input.vout))
    outputs.forEach(output => {
        if (!output.address) {
            output.address = from
        }

        txb.addOutput(output.address, output.value)
    })

    txb.sign(0, keyPair, p2sh.redeem.output, null, inputs.value) // fails here. Error: Scripthash not supported

}

module.exports = send

package.json

{
  "name": "",
  "version": "1.0.0",
  "description": "",
  "main": "app.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "MIT",
  "dependencies": {
    "bitcoinjs-lib": "github:majestic84/bitcoinjs-lib",
    "coinselect": "^3.1.11",
    "litecore-explorers": "github:majestic84/litecore-explorers",
    "util": "^0.10.3"
  }
}
dabura667 commented 5 years ago

the value of "type" just after it is witnesspubkeyhash

That makes no sense... the only error in the whole library that is of the format scripthash not supported would be if that classification in that function gave "scripthash"

is it possible my "ins" are malformed?

No, they're fine. the code you showed us only uses txid and vout etc. so anything about the formatting of the code in the upper half is irrelevant.

what are the values of wpkh1 and wpkh2 a few lines down?

majestic84 commented 5 years ago

wpkh1 <Buffer 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2> wpkh2 <Buffer 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2>

Thanks for taking the time to help me with this. I am losing my mind!

Here is the value of "input" just before if (!canSign(input)) throw Error(input.prevOutType + ' not supported') in transaction_builder's prototype.sign method

{ redeemScript: <Buffer 00 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2>,
  redeemScriptType: 'witnesspubkeyhash',
  prevOutType: 'scripthash',
  prevOutScript: <Buffer a9 14 47 4d 4a 55 4d 32 b8 65 fb de 4b e1 09 ec 9a 20 db 6d 78 1d 87>,
  hasWitness: true,
  signScript: <Buffer 76 a9 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2 88 ac>,
  signType: 'witnesspubkeyhash',
  pubkeys:
   [ <Buffer 03 3e 1b 4c 0b 3d 9f e1 20 3a b0 79 51 e1 be 81 30 82 86 96 86 87 f7 a8 36 87 9c 0d 6e 17 31 e4 23> ],
  signatures: [ undefined ] }
dabura667 commented 5 years ago

Could you post the actual stack trace for the "scripthash not supported" error?

majestic84 commented 5 years ago
Error: scripthash not supported
    at TransactionBuilder.sign (C:\Node\ltc\node_modules\bitcoinjs-lib\src\transaction_builder.js:672:32)
    at send (C:\Node\ltc\tx.js:66:13)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

Of course, now I've modified transaction_builder.js, so line 672 there is just this line: if (!canSign(input)) throw Error(input.prevOutType + ' not supported')

dabura667 commented 5 years ago

What is the input object just before that line?

dabura667 commented 5 years ago

canSign might give us some hints if we know what the input is.

function canSign (input) {
  return input.signScript !== undefined &&
    input.signType !== undefined &&
    input.pubkeys !== undefined &&
    input.signatures !== undefined &&
    input.signatures.length === input.pubkeys.length &&
    input.pubkeys.length > 0 &&
    (
      input.hasWitness === false ||
      input.value !== undefined
    )
}
majestic84 commented 5 years ago
{ redeemScript: <Buffer 00 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2>,
  redeemScriptType: 'witnesspubkeyhash',
  prevOutType: 'scripthash',
  prevOutScript: <Buffer a9 14 47 4d 4a 55 4d 32 b8 65 fb de 4b e1 09 ec 9a 20 db 6d 78 1d 87>,
  hasWitness: true,
  signScript: <Buffer 76 a9 14 69 05 44 d3 2c b7 f0 64 cb ee ae a5 1e af 50 69 b9 f2 4d b2 88 ac>,
  signType: 'witnesspubkeyhash',
  pubkeys:
   [ <Buffer 03 3e 1b 4c 0b 3d 9f e1 20 3a b0 79 51 e1 be 81 30 82 86 96 86 87 f7 a8 36 87 9c 0d 6e 17 31 e4 23> ],
  signatures: [ undefined ] }

So here it's clear that the error is being thrown because the prevOutType is "scripthash", but I can't figure out why.

dabura667 commented 5 years ago

input is missing value

that's why.

dabura667 commented 5 years ago

prevOutType is P2SH, that is correct.

dabura667 commented 5 years ago

check the input a bunch of places and see where the input loses input.value...

Maybe insight is giving a bad value for inputs.value???

shouldn't it be inputs[0].value?

majestic84 commented 5 years ago

Wow that's all it was. I cannot thank you enough!

majestic84 commented 5 years ago

So that works great when there is only one UTXO input to satisfy the amount, however when there are more than one it's not able to build the transaction.

I changed the tx building section to the following, which adds up the values of each input, and uses that cumulative value for the .sign method. I checked the value of inputsValue just before signing and it is correct.

let inputsValue = 0
    inputs.forEach(input => {
        inputsValue += input.value
        txb.addInput(input.txid, input.vout)
    })
    outputs.forEach(output => {
        if (!output.address) {
            output.address = from
        }

        txb.addOutput(output.address, output.value)
    })
    txb.sign(0, keyPair, p2sh.redeem.output, null, inputsValue)

    const tx = txb.build() // <- this doesn't complete

Stack:

Error: Transaction is not complete
    at C:\Node\ltc\node_modules\bitcoinjs-lib\src\transaction_builder.js:595:55
    at Array.forEach (<anonymous>)
    at TransactionBuilder.__build (C:\Node\ltc\node_modules\bitcoinjs-lib\src\transaction_builder.js:589:17)
    at TransactionBuilder.build (C:\Node\ltc\node_modules\bitcoinjs-lib\src\transaction_builder.js:574:15)
    at send (C:\Node\ltc\tx.js:77:20)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

transaction_builder line 595: if (!input.prevOutType && !allowIncomplete) throw new Error('Transaction is not complete')

runtime values just before line 595:

in[0]:
input.prevOutType: scripthash
allowIncomplete: false

in[1]:
input.prevOutType: undefined
allowIncomplete: false

TransactionBuilder.prototype.build loops through each of the inputs in the transaction `this.inputs.forEach(function (input, i) {`. It seems in my case, input[0] is complete, and input[1] is empty {}

majestic84 commented 5 years ago

If I do this it seems to work:

txb.sign(0, keyPair, p2sh.redeem.output, null, inputs[0].value)
txb.sign(1, keyPair, p2sh.redeem.output, null, inputs[1].value)

I could handle this with a loop of course, but is it correct to sign for each UTXO input like this?

dabura667 commented 5 years ago

yes. you must loop over the inputs.