TransactionBuilder P2SH Multisig order of signatures

src/transaction_builder.js

@@ -76,7 +76,9 @@ TransactionBuilder.fromTransaction = function(transaction) {
         })
 
         hashType = parsed[0].hashType
-        pubKeys = []
+        pubKeys = redeemScript.chunks.slice(1, -2).map(function(pubKey) {
+          return ECPubKey.fromBuffer(pubKey)
+        })
         signatures = parsed.map(function(p) { return p.signature })
 
         break
@@ -191,8 +193,20 @@ TransactionBuilder.prototype.__build = function(allowIncomplete) {
         break
 
       case 'multisig':
+        // we need to put the signatures in the correct order
+        var hash = tx.hashForSignature(index, input.redeemScript, input.hashType)
+        var multisigSignatures = input.pubKeys.map(function(pubKey, i) {
+          var signature = null;
+          input.signatures.forEach(function(_signature) {
+            if (!signature && pubKey.verify(hash, _signature)) {
+              signature = _signature.toScriptSignature(input.hashType)
+            }
+          })
+          return signature;
+        }).filter(function(signature) { return !!signature })
+
         var redeemScript = allowIncomplete ? undefined : input.redeemScript
-        scriptSig = scripts.multisigInput(signatures, redeemScript)
+        scriptSig = scripts.multisigInput(multisigSignatures, redeemScript)
 
         break
 
@@ -223,7 +237,7 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT
   var prevOutScript = this.prevOutScripts[index]
   var prevOutType = this.prevOutTypes[index]
 
-  var scriptType, hash
+  var scriptType, hash, pubKeys
   if (redeemScript) {
     prevOutScript = prevOutScript || scripts.scriptHashOutput(redeemScript.getHash())
     prevOutType = prevOutType || 'scripthash'
@@ -235,6 +249,13 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT
     assert.notEqual(scriptType, 'scripthash', 'RedeemScript can\'t be P2SH')
     assert.notEqual(scriptType, 'nonstandard', 'RedeemScript not supported (nonstandard)')
 
+    // for multisig we need to get the pubKeys from the redeemScript since their order is important
+    if (scriptType === 'multisig') {
+      pubKeys = redeemScript.chunks.slice(1, -2).map(function(pubKey) {
+        return ECPubKey.fromBuffer(pubKey)
+      })
+    }
+
     hash = this.tx.hashForSignature(index, redeemScript, hashType)
 
   } else {
@@ -268,8 +289,13 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT
   assert.deepEqual(input.redeemScript, redeemScript, 'Inconsistent redeemScript')
 
   var signature = privKey.sign(hash)
-  input.pubKeys.push(privKey.pub)
   input.signatures.push(signature)
+
+  if (pubKeys) {
+    input.pubKeys = pubKeys
+  } else {
+    input.pubKeys.push(privKey.pub)
+  }
 }
 
 module.exports = TransactionBuilder

test/transaction_builder.js

@@ -290,5 +290,32 @@ describe('TransactionBuilder', function() {
 
       assert.equal(tx2.toHex(), '0100000001cff58855426469d0ef16442ee9c644c4fb13832467bcbc3173168a7916f0714900000000fd1c01004830450221009c92c1ae1767ac04e424da7f6db045d979b08cde86b1ddba48621d59a109d818022004f5bb21ad72255177270abaeb2d7940ac18f1e5ca1f53db4f3fd1045647a8a8014830450221009418caa5bc18da87b188a180125c0cf06dce6092f75b2d3c01a29493466800fd02206ead65e7ca6e0f17eefe6f78457c084eab59af7c9882be1437de2e7116358eb9014c8752410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b84104c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee51ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a52aeffffffff0110270000000000001976a914751e76e8199196d454941c45d1b3a323f1433bd688ac00000000')
     })
+
+    it('works for the P2SH multisig case with signing out of order', function() {
+      // same test case as above but privKeys are in different order
+      var privKeys = [
+        "91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgww7vXtT",
+        "91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgwmaKkrx"
+      ].map(ECKey.fromWIF)
+      // 2of2
+      var redeemScript = Script.fromASM("OP_2 0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 04c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee51ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a OP_2 OP_CHECKMULTISIG")
+
+      txb.addInput("4971f016798a167331bcbc67248313fbc444c6e92e4416efd06964425588f5cf", 0)
+      txb.addOutput("1BgGZ9tcN4rm9KBzDn7KprQz87SZ26SAMH", 10000)
+      txb.sign(0, privKeys[0], redeemScript)
+
+      var tx = txb.buildIncomplete()
+
+      // in another galaxy...
+      // ... far, far away
+      var txb2 = TransactionBuilder.fromTransaction(tx)
+
+      // [you should] verify that Transaction is what you want...
+      // ... then sign it
+      txb2.sign(0, privKeys[1], redeemScript)
+      var tx2 = txb2.build()
+
+      assert.equal(tx2.toHex(), '0100000001cff58855426469d0ef16442ee9c644c4fb13832467bcbc3173168a7916f0714900000000fd1c01004830450221009c92c1ae1767ac04e424da7f6db045d979b08cde86b1ddba48621d59a109d818022004f5bb21ad72255177270abaeb2d7940ac18f1e5ca1f53db4f3fd1045647a8a8014830450221009418caa5bc18da87b188a180125c0cf06dce6092f75b2d3c01a29493466800fd02206ead65e7ca6e0f17eefe6f78457c084eab59af7c9882be1437de2e7116358eb9014c8752410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b84104c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee51ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a52aeffffffff0110270000000000001976a914751e76e8199196d454941c45d1b3a323f1433bd688ac00000000')
+    })
   })
 })