Repository: bitcoinjs/bitcoinjs-lib
Pull request #861 (2017-08-23 03:58:46)

tests/txb: add failing p2wsh multisig incomplete fixture
Reported by @junderw (thanks!)

Fixes to be added

junderw: ACK

Patch:
diff --git a/src/transaction_builder.js b/src/transaction_builder.js
index 45d80226..c450bd38 100644
--- a/src/transaction_builder.js
+++ b/src/transaction_builder.js
@@ -14,6 +14,14 @@ var ECPair = require('./ecpair')
 var ECSignature = require('./ecsignature')
 var Transaction = require('./transaction')
 
+function supportedType (type) {
+  return SIGNABLE.indexOf(type) !== -1
+}
+
+function supportedP2SHType (type) {
+  return P2SH.indexOf(type) !== -1
+}
+
 function extractChunks (type, chunks, script) {
   var pubKeys = []
   var signatures = []
@@ -74,7 +82,7 @@ function expandInput (scriptSig, witnessStack) {
     script = redeemScript
   }
 
-  var classifyWitness = bscript.classifyWitness(witnessStack)
+  var classifyWitness = bscript.classifyWitness(witnessStack, true)
   if (classifyWitness === scriptTypes.P2WSH) {
     witnessScript = witnessStack[witnessStack.length - 1]
     witnessScriptType = bscript.classifyOutput(witnessScript)
@@ -82,7 +90,7 @@ function expandInput (scriptSig, witnessStack) {
     if (scriptSig.length === 0) {
       prevOutScript = bscript.witnessScriptHash.output.encode(bcrypto.sha256(witnessScript))
       prevOutType = scriptTypes.P2WSH
-      if (typeof redeemScript !== 'undefined') {
+      if (redeemScript !== undefined) {
         throw new Error('Redeem script given when unnecessary')
       }
       // bare witness
@@ -96,9 +104,10 @@ function expandInput (scriptSig, witnessStack) {
       }
     }
 
-    if (SIGNABLE.indexOf(bscript.classifyOutput(witnessScript)) === -1) {
+    if (!supportedType(bscript.classifyOutput(witnessScript))) {
       throw new Error('unsupported witness script')
     }
+
     script = witnessScript
     scriptType = witnessScriptType
     chunks = witnessStack.slice(0, -1)
@@ -124,7 +133,7 @@ function expandInput (scriptSig, witnessStack) {
     scriptType = scriptTypes.P2PKH
     chunks = witnessStack
   } else if (redeemScript) {
-    if (P2SH.indexOf(redeemScriptType) === -1) {
+    if (!supportedP2SHType(redeemScriptType)) {
       throw new Error('Bad redeemscript!')
     }
 
@@ -390,7 +399,6 @@ function buildStack (type, signatures, pubKeys, allowIncomplete) {
   }
 
   if (!allowIncomplete) throw new Error('Not enough signatures provided')
-
   return []
 }
 
@@ -398,7 +406,8 @@ function buildInput (input, allowIncomplete) {
   var scriptType = input.prevOutType
   var sig = []
   var witness = []
-  if (SIGNABLE.indexOf(scriptType) !== -1) {
+
+  if (supportedType(scriptType)) {
     sig = buildStack(scriptType, input.signatures, input.pubKeys, allowIncomplete)
   }
 
@@ -406,13 +415,14 @@ 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 && !allowIncomplete) {
+    if (!allowIncomplete && !supportedP2SHType(input.redeemScriptType)) {
       throw new Error('Impossible to sign this type')
     }
 
-    if (SIGNABLE.indexOf(input.redeemScriptType) !== -1) {
+    if (supportedType(input.redeemScriptType)) {
       sig = buildStack(input.redeemScriptType, input.signatures, input.pubKeys, allowIncomplete)
     }
+
     // If it wasn't SIGNABLE, it's witness, defer to that
     if (input.redeemScriptType) {
       p2sh = true
@@ -420,18 +430,25 @@ function buildInput (input, allowIncomplete) {
     }
   }
 
-  if (scriptType === bscript.types.P2WPKH) {
+  switch (scriptType) {
     // P2WPKH is a special case of P2PKH
-    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 && !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)
-      scriptType = input.witnessScriptType
-    }
+    case bscript.types.P2WPKH:
+      witness = buildStack(bscript.types.P2PKH, input.signatures, input.pubKeys, allowIncomplete)
+      break
+
+    case bscript.types.P2WSH:
+      // We can remove this check later
+      if (!allowIncomplete && !supportedType(input.witnessScriptType)) {
+        throw new Error('Impossible to sign this type')
+      }
+
+      if (supportedType(input.witnessScriptType)) {
+        witness = buildStack(input.witnessScriptType, input.signatures, input.pubKeys, allowIncomplete)
+        witness.push(input.witnessScript)
+        scriptType = input.witnessScriptType
+      }
+
+      break
   }
 
   // append redeemScript if necessary
@@ -442,7 +459,7 @@ function buildInput (input, allowIncomplete) {
   return {
     type: scriptType,
     script: bscript.compile(sig),
-    witness: witness
+    witness: bscript.toStack(witness)
   }
 }
 
@@ -616,7 +633,7 @@ TransactionBuilder.prototype.__build = function (allowIncomplete) {
 
     // skip if no result
     if (!allowIncomplete) {
-      if (SIGNABLE.indexOf(result.type) === -1 && result.type !== bscript.types.P2WPKH) {
+      if (!supportedType(result.type) && result.type !== bscript.types.P2WPKH) {
         throw new Error(result.type + ' not supported')
       }
     }


Test patch:
diff --git a/test/fixtures/transaction_builder.json b/test/fixtures/transaction_builder.json
index 282e7f0b..afee73bb 100644
--- a/test/fixtures/transaction_builder.json
+++ b/test/fixtures/transaction_builder.json
@@ -1327,11 +1327,68 @@
           }
         ],
         "locktime": 0
+      },
+      {
+        "description": "P2SH(P2WSH(multisig 2-of-2)), incomplete",
+        "network": "testnet",
+        "txHex": "010000000001012915794541ffa77ca795ec7c23ee989a63ccd1a71fab73e1c27ed20c4b6c69a4010000002322002024376a0a9abab599d0e028248d48ebe817bc899efcffa1cd2984d67289daf5afffffffff01b8820100000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac040047304402203b334650f1f13574a1c2edc76421867f7252950968bf0293c8b3ed086ab89e3d0220565cafab0a5044617e94756b948241525b2483a52504e1064d29f641fb18129e010047522102bbbd6eb01efcbe4bd9664b886f26f69de5afcb2e479d72596c8bf21929e352e22102d9c3f7180ef13ec5267723c9c2ffab56a4215241f837502ea8977c8532b9ea1952ae00000000",
+        "incomplete": true,
+        "inputs": [
+          {
+            "txId": "a4696c4b0cd27ec2e173ab1fa7d1cc639a98ee237cec95a77ca7ff4145791529",
+            "vout": 1,
+            "signs": [
+              {
+                "keyPair": "cRAwuVuVSBZMPu7hdrYvMCZ8eevzmkExjFbaBLhqnDdrezxN3nTS",
+                "witnessScript": "OP_2 02bbbd6eb01efcbe4bd9664b886f26f69de5afcb2e479d72596c8bf21929e352e2 02d9c3f7180ef13ec5267723c9c2ffab56a4215241f837502ea8977c8532b9ea19 OP_2 OP_CHECKMULTISIG",
+                "redeemScript": "OP_0 24376a0a9abab599d0e028248d48ebe817bc899efcffa1cd2984d67289daf5af",
+                "value": 100000
+              }
+            ],
+            "prevTxScript": "OP_HASH160 b64f1a3eacc1c8515592a6f10457e8ff90e4db6a OP_EQUAL"
+          }
+        ],
+        "outputs": [
+          {
+            "script": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG",
+            "value": 99000
+          }
+        ]
+      },
+      {
+        "description": "P2SH(P2WSH(multisig 2-of-2))",
+        "network": "testnet",
+        "txHex": "010000000001012915794541ffa77ca795ec7c23ee989a63ccd1a71fab73e1c27ed20c4b6c69a4010000002322002024376a0a9abab599d0e028248d48ebe817bc899efcffa1cd2984d67289daf5afffffffff01b8820100000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac040047304402203b334650f1f13574a1c2edc76421867f7252950968bf0293c8b3ed086ab89e3d0220565cafab0a5044617e94756b948241525b2483a52504e1064d29f641fb18129e0148304502210096e859827fb629b6547658c613f7c8298de151513d74b224560aa8608d521d600220736fb5564322237716ec940de44c67c428198adf5dedfda183c17aa77cd28d640147522102bbbd6eb01efcbe4bd9664b886f26f69de5afcb2e479d72596c8bf21929e352e22102d9c3f7180ef13ec5267723c9c2ffab56a4215241f837502ea8977c8532b9ea1952ae00000000",
+        "incomplete": true,
+        "inputs": [
+          {
+            "txId": "a4696c4b0cd27ec2e173ab1fa7d1cc639a98ee237cec95a77ca7ff4145791529",
+            "vout": 1,
+            "signs": [
+              {
+                "keyPair": "cRAwuVuVSBZMPu7hdrYvMCZ8eevzmkExjFbaBLhqnDdrezxN3nTS",
+                "witnessScript": "OP_2 02bbbd6eb01efcbe4bd9664b886f26f69de5afcb2e479d72596c8bf21929e352e2 02d9c3f7180ef13ec5267723c9c2ffab56a4215241f837502ea8977c8532b9ea19 OP_2 OP_CHECKMULTISIG",
+                "redeemScript": "OP_0 24376a0a9abab599d0e028248d48ebe817bc899efcffa1cd2984d67289daf5af",
+                "value": 100000
+              },
+              {
+                "keyPair": "cTUFsNeVd8TKU4yREN8nMdViNnHyNvCCYVRmRUmkMLgomiMWTiii",
+                "witnessScript": "OP_2 02bbbd6eb01efcbe4bd9664b886f26f69de5afcb2e479d72596c8bf21929e352e2 02d9c3f7180ef13ec5267723c9c2ffab56a4215241f837502ea8977c8532b9ea19 OP_2 OP_CHECKMULTISIG",
+                "redeemScript": "OP_0 24376a0a9abab599d0e028248d48ebe817bc899efcffa1cd2984d67289daf5af",
+                "value": 100000
+              }
+            ],
+            "prevTxScript": "OP_HASH160 b64f1a3eacc1c8515592a6f10457e8ff90e4db6a OP_EQUAL"
+          }
+        ],
+        "outputs": [
+          {
+            "script": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG",
+            "value": 99000
+          }
+        ]
       }
-
-
-    ]
-    ,
+    ],
     "fromTransaction": [
       {
         "description": "Transaction w/ scriptHash(multisig 2-of-2) -> OP_RETURN | 1 OP_0, no signatures",
diff --git a/test/transaction_builder.js b/test/transaction_builder.js
index b09a2feb..6606e386 100644
--- a/test/transaction_builder.js
+++ b/test/transaction_builder.js
@@ -91,7 +91,9 @@ describe('TransactionBuilder', function () {
 
         var tx = Transaction.fromHex(f.txHex)
         var txb = TransactionBuilder.fromTransaction(tx, network)
-        assert.strictEqual(txb.build().toHex(), f.txHex)
+        var txAfter = f.incomplete ? txb.buildIncomplete() : txb.build()
+
+        assert.strictEqual(txAfter.toHex(), f.txHex)
         assert.strictEqual(txb.network, network)
       })
     })
@@ -305,7 +307,8 @@ describe('TransactionBuilder', function () {
     fixtures.valid.build.forEach(function (f) {
       it('builds "' + f.description + '"', function () {
         var txb = construct(f)
-        var tx = txb.build()
+        var tx = f.incomplete ? txb.buildIncomplete() : txb.build()
+
         assert.strictEqual(tx.toHex(), f.txHex)
       })
     })