From 6c0d2cce6859ab5c5cb67ea8b49969c5621c9065 Mon Sep 17 00:00:00 2001 From: Ruben de Vries Date: Tue, 3 Feb 2015 14:01:16 +0100 Subject: [PATCH 1/2] add failing test case for when P2SH multisig is signed in a different order than the order of the pubKeys in the redeemScript --- test/transaction_builder.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/transaction_builder.js b/test/transaction_builder.js index 8e2b528db..ba108440e 100644 --- a/test/transaction_builder.js +++ b/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') + }) }) }) From bbbc8a21d54b0bb2a74baf5d047181f5f7e6c166 Mon Sep 17 00:00:00 2001 From: Ruben de Vries Date: Tue, 3 Feb 2015 15:30:09 +0100 Subject: [PATCH 2/2] now ordering signatures based on pubKeys for P2SH multisig --- src/transaction_builder.js | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/transaction_builder.js b/src/transaction_builder.js index 71a46ecbc..6ca4b089a 100644 --- a/src/transaction_builder.js +++ b/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