Repository: bitcoinjs/bitcoinjs-lib (js)

Issue:
buildIncomplete fails for unsigned P2SH inputs
I am trying to write test cases for sending an unsigned transaction P2SH transaction to my service, but I'm unable to get the hex because the `buildIncomplete` function fails with a strange error. See below for bug repro:

```
var b = require('bitcoinjs-lib');
var inp = Buffer.from('010000000173120703f67318aef51f7251272a6816d3f7523bb25e34b136d80be959391c100000000000ffffffff0100c817a80400000017a91471a8ec07ff69c6c4fee489184c462a9b1b9237488700000000', 'hex') // arbitrary P2SH input
var inpTx = b.Transaction.fromBuffer(inp)

var txb = new b.TransactionBuilder(b.networks.testnet)
txb.addInput(inpTx, 0)
txb.addOutput(b.address.toOutputScript('2NAkqp5xffoomp5RLBcakuGpZ12GU4twdz4', b.networks.testnet), 1e8) // arbitrary output

txb.buildIncomplete()
```

produces the error

```
Error: Impossible to sign this type
    at buildInput (/Users/alex/dev/bitcoinjs-lib/src/transaction_builder.js:408:13)
    at /Users/alex/dev/bitcoinjs-lib/src/transaction_builder.js:614:18
    at Array.forEach (native)
    at TransactionBuilder.__build (/Users/alex/dev/bitcoinjs-lib/src/transaction_builder.js:611:15)
    at TransactionBuilder.buildIncomplete (/Users/alex/dev/bitcoinjs-lib/src/transaction_builder.js:600:15)
    at repl:1:5
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
    at ContextifyScript.Script.runInThisContext (vm.js:21:12)
    at REPLServer.defaultEval (repl.js:346:29)

```

Based on reading transaction_builder.js it looks like the `buildInput` function assumes the input's `redeemScriptType` would have been set. I would write up a PR but don't know enough about bitcoinjs-lib to know if this is a simple fix or requires a deeper reworking of the txn build/sign flow. Suggestions appreciated!

Pull request description:
Re #833: fix bug when dealing with incomplete P2SH/P2WSH inputs.
Should be able to deal with incomplete P2SH/P2WSH inputs when allowIncomplete is set. Incorporates suggestion from @Melvillian and does similar for P2WSH, adding test cases for p2sh/p2wsh/p2wpkh.

Fixes #833 

Patch:
diff --git a/src/transaction_builder.js b/src/transaction_builder.js
index 3e282bbb..45d80226 100644
--- a/src/transaction_builder.js
+++ b/src/transaction_builder.js
@@ -406,15 +406,18 @@ function buildInput (input, allowIncomplete) {
   if (scriptType === bscript.types.P2SH) {
     // We can remove this error later when we have a guarantee prepareInput
     // rejects unsignable scripts - it MUST be signable at this point.
-    if (P2SH.indexOf(input.redeemScriptType) === -1) {
+    if (P2SH.indexOf(input.redeemScriptType) === -1 && !allowIncomplete) {
       throw new Error('Impossible to sign this type')
     }
-    p2sh = true
+
     if (SIGNABLE.indexOf(input.redeemScriptType) !== -1) {
       sig = buildStack(input.redeemScriptType, input.signatures, input.pubKeys, allowIncomplete)
     }
     // If it wasn't SIGNABLE, it's witness, defer to that
-    scriptType = input.redeemScriptType
+    if (input.redeemScriptType) {
+      p2sh = true
+      scriptType = input.redeemScriptType
+    }
   }
 
   if (scriptType === bscript.types.P2WPKH) {
@@ -422,16 +425,13 @@ function buildInput (input, allowIncomplete) {
     witness = buildStack(bscript.types.P2PKH, input.signatures, input.pubKeys, allowIncomplete)
   } else if (scriptType === bscript.types.P2WSH) {
     // We can remove this check later
-    if (SIGNABLE.indexOf(input.witnessScriptType) !== -1) {
+    if (SIGNABLE.indexOf(input.witnessScriptType) === -1 && !allowIncomplete) {
+      throw new Error('Impossible to sign this type')
+    } else if (SIGNABLE.indexOf(input.witnessScriptType) !== -1) {
       witness = buildStack(input.witnessScriptType, input.signatures, input.pubKeys, allowIncomplete)
       witness.push(input.witnessScript)
-    } else {
-      // We can remove this error later when we have a guarantee prepareInput
-      // rejects unsignble scripts - it MUST be signable at this point.
-      throw new Error()
+      scriptType = input.witnessScriptType
     }
-
-    scriptType = input.witnessScriptType
   }
 
   // append redeemScript if necessary
@@ -578,7 +578,6 @@ TransactionBuilder.prototype.__addInputUnsafe = function (txHash, vout, options)
   var vin = this.tx.addInput(txHash, vout, options.sequence, options.scriptSig)
   this.inputs[vin] = input
   this.prevTxMap[prevTxOut] = vin
-
   return vin
 }
 


Test patch:
diff --git a/test/integration/bip32.js b/test/integration/bip32.js
index dd700c94..c6973437 100644
--- a/test/integration/bip32.js
+++ b/test/integration/bip32.js
@@ -49,6 +49,23 @@ describe('bitcoinjs-lib (BIP32)', function () {
     assert.equal(child2.getAddress(), '12Tyvr1U8A3ped6zwMEU5M8cx3G38sP5Au')
   })
 
+  it('can create a BIP49, bitcoin testnet, account 0, external address', function () {
+    var mnemonic = 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'
+    var seed = bip39.mnemonicToSeed(mnemonic)
+    var root = bitcoin.HDNode.fromSeedBuffer(seed)
+
+    var path = "m/49'/1'/0'/0/0"
+    var child = root.derivePath(path)
+
+    var keyhash = bitcoin.crypto.hash160(child.getPublicKeyBuffer())
+    var scriptSig = bitcoin.script.witnessPubKeyHash.output.encode(keyhash)
+    var addressBytes = bitcoin.crypto.hash160(scriptSig)
+    var outputScript = bitcoin.script.scriptHash.output.encode(addressBytes)
+    var address = bitcoin.address.fromOutputScript(outputScript, bitcoin.networks.testnet)
+
+    assert.equal(address, '2Mww8dCYPUpKHofjgcXcBCEGmniw9CoaiD2')
+  })
+
   it('can recover a BIP32 parent private key from the parent public key, and a derived, non-hardened child private key', function () {
     function recoverParent (master, child) {
       assert(!master.keyPair.d, 'You already have the parent private key')
diff --git a/test/transaction_builder.js b/test/transaction_builder.js
index d75ef1cb..b09a2feb 100644
--- a/test/transaction_builder.js
+++ b/test/transaction_builder.js
@@ -362,10 +362,42 @@ describe('TransactionBuilder', function () {
       var randomTx = Transaction.fromHex(randomTxData)
       var tx = new TransactionBuilder()
       tx.addInput(randomTx, 0)
-      tx.addOutput(baddress.toOutputScript(randomAddress), 1000)
+      tx.addOutput(randomAddress, 1000)
       tx = tx.buildIncomplete()
       assert(tx)
     })
+
+    it('for incomplete P2SH with 0 signatures', function () {
+      var inp = Buffer.from('010000000173120703f67318aef51f7251272a6816d3f7523bb25e34b136d80be959391c100000000000ffffffff0100c817a80400000017a91471a8ec07ff69c6c4fee489184c462a9b1b9237488700000000', 'hex') // arbitrary P2SH input
+      var inpTx = Transaction.fromBuffer(inp)
+
+      var txb = new TransactionBuilder(NETWORKS.testnet)
+      txb.addInput(inpTx, 0)
+      txb.addOutput('2NAkqp5xffoomp5RLBcakuGpZ12GU4twdz4', 1e8) // arbitrary output
+
+      txb.buildIncomplete()
+    })
+
+    it('for incomplete P2WPKH with 0 signatures', function () {
+      var inp = Buffer.from('010000000173120703f67318aef51f7251272a6816d3f7523bb25e34b136d80be959391c100000000000ffffffff0100c817a8040000001600141a15805e1f4040c9f68ccc887fca2e63547d794b00000000', 'hex')
+      var inpTx = Transaction.fromBuffer(inp)
+
+      var txb = new TransactionBuilder(NETWORKS.testnet)
+      txb.addInput(inpTx, 0)
+      txb.addOutput('2NAkqp5xffoomp5RLBcakuGpZ12GU4twdz4', 1e8) // arbitrary output
+
+      txb.buildIncomplete()
+    })
+
+    it('for incomplete P2WSH with 0 signatures', function () {
+      var inpTx = Transaction.fromBuffer(Buffer.from('010000000173120703f67318aef51f7251272a6816d3f7523bb25e34b136d80be959391c100000000000ffffffff0100c817a80400000022002072df76fcc0b231b94bdf7d8c25d7eef4716597818d211e19ade7813bff7a250200000000', 'hex'))
+
+      var txb = new TransactionBuilder(NETWORKS.testnet)
+      txb.addInput(inpTx, 0)
+      txb.addOutput('2NAkqp5xffoomp5RLBcakuGpZ12GU4twdz4', 1e8) // arbitrary output
+
+      txb.buildIncomplete()
+    })
   })
 
   describe('multisig', function () {