From 1c744cfa5abf2aa8340005f20faa26974b793bcb Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 12:09:31 +1000 Subject: [PATCH 1/8] TxBuilder: add failing test for non-zero vin inputs --- test/fixtures/transaction_builder.json | 20 ++++++++++++++++++++ test/transaction_builder.js | 1 + 2 files changed, 21 insertions(+) diff --git a/test/fixtures/transaction_builder.json b/test/fixtures/transaction_builder.json index c2444ea..6795c48 100644 --- a/test/fixtures/transaction_builder.json +++ b/test/fixtures/transaction_builder.json @@ -63,6 +63,26 @@ "value": 10000 } ] + }, + { + "description": "Transaction w/ non-zero vin inputs", + "txid": "7d9b699f26765fdfdd598223a952a6e129f8c159e2e05e911af822ee743fa745", + "txhex": "0100000001ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff010000006a47304402205c80bbb5125b35d5e5a8324b1336832d29a6fc004859c8a9ff6bef47ba7fc348022018612216e57a521b2c4543f1f4fd738a76814c37c074e88adfe12464fff31cf901210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac00000000", + "inputs": [ + { + "index": 1, + "prevTx": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "privKeys": [ + "KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU73sVHnoWn" + ] + } + ], + "outputs": [ + { + "script": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG", + "value": 10000 + } + ] } ] }, diff --git a/test/transaction_builder.js b/test/transaction_builder.js index d6ab2f0..7f16dbc 100644 --- a/test/transaction_builder.js +++ b/test/transaction_builder.js @@ -182,6 +182,7 @@ describe('TransactionBuilder', function() { var tx = txb.build() assert.equal(tx.getId(), f.txid) + assert.equal(tx.toHex(), f.txhex) }) }) From e2357c09ce74f30676e129e15ef16adbfe0493c1 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 12:10:02 +1000 Subject: [PATCH 2/8] TxBuilder: fix invalid txin.index usage for non-zero vin inputs --- src/transaction_builder.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transaction_builder.js b/src/transaction_builder.js index 05c0591..c4f18da 100644 --- a/src/transaction_builder.js +++ b/src/transaction_builder.js @@ -30,7 +30,7 @@ TransactionBuilder.fromTransaction = function(transaction) { }) // Extract/add signatures - transaction.ins.forEach(function(txin) { + transaction.ins.forEach(function(txin, i) { // Ignore empty scripts if (txin.script.buffer.length === 0) return @@ -86,7 +86,7 @@ TransactionBuilder.fromTransaction = function(transaction) { assert(false, scriptType + ' not supported') } - txb.signatures[txin.index] = { + txb.signatures[i] = { hashType: hashType, pubKeys: pubKeys, redeemScript: redeemScript, From aa80bde815eaf1b669e16013171759e1b80076de Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 12:22:55 +1000 Subject: [PATCH 3/8] TxBuilder: avoid unnecessary recalculation of prevOutScript data --- src/transaction_builder.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/transaction_builder.js b/src/transaction_builder.js index c4f18da..42a77bf 100644 --- a/src/transaction_builder.js +++ b/src/transaction_builder.js @@ -231,13 +231,18 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT } else { prevOutScript = prevOutScript || privKey.pub.getAddress().toOutputScript() - scriptType = prevOutType || 'pubkeyhash' + prevOutType = prevOutType || 'pubkeyhash' - assert.notEqual(scriptType, 'scripthash', 'PrevOutScript requires redeemScript') + assert.notEqual(prevOutType, 'scripthash', 'PrevOutScript is P2SH, missing redeemScript') + + scriptType = prevOutType hash = this.tx.hashForSignature(index, prevOutScript, hashType) } + this.prevOutScripts[index] = prevOutScript + this.prevOutTypes[index] = prevOutType + if (!(index in this.signatures)) { this.signatures[index] = { hashType: hashType, From b3438c5ef2636711011afe481650670dc4f2d273 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 12:51:29 +1000 Subject: [PATCH 4/8] Transaction: throw if sequence is not a number --- src/transaction.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/transaction.js b/src/transaction.js index a9a0899..ffb9522 100644 --- a/src/transaction.js +++ b/src/transaction.js @@ -52,6 +52,7 @@ Transaction.prototype.addInput = function(tx, index, sequence) { assert(Buffer.isBuffer(hash), 'Expected Transaction, txId or txHash, got ' + tx) assert.equal(hash.length, 32, 'Expected hash length of 32, got ' + hash.length) assert.equal(typeof index, 'number', 'Expected number index, got ' + index) + assert.equal(typeof sequence, 'number', 'Expected number sequence, got ' + sequence) // Add the input and return the input's index return (this.ins.push({ From e5618bb8bd0c69ea9d5677d4a6fc4a3bf8698a49 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 12:52:05 +1000 Subject: [PATCH 5/8] TxBuilder: add test for missing redeemScript if P2SH --- test/transaction_builder.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/transaction_builder.js b/test/transaction_builder.js index 7f16dbc..fb2ae67 100644 --- a/test/transaction_builder.js +++ b/test/transaction_builder.js @@ -110,6 +110,18 @@ describe('TransactionBuilder', function() { }) }) + describe('when redeemScript is undefined', function() { + it('throws if prevOutScript is P2SH', function() { + var privScriptP2SH = scripts.scriptHashOutput(privScript.getHash()) + + txb.addInput(prevTxHash, 0, undefined, privScriptP2SH) + + assert.throws(function() { + txb.sign(0, privKey) + }, /PrevOutScript is P2SH, missing redeemScript/) + }) + }) + describe('when redeemScript is defined', function() { it('assumes scriptHash', function() { txb.addInput(prevTxHash, 0) From 8d5ef2dd063cf0d9671734b9df3391e42aeb2021 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 13:02:26 +1000 Subject: [PATCH 6/8] TxBuilder: limit signatures depending on scriptType --- src/transaction_builder.js | 2 ++ test/transaction_builder.js | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/src/transaction_builder.js b/src/transaction_builder.js index 42a77bf..fc6a419 100644 --- a/src/transaction_builder.js +++ b/src/transaction_builder.js @@ -251,6 +251,8 @@ TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashT scriptType: scriptType, signatures: [] } + } else { + assert.equal(scriptType, 'multisig', scriptType + ' doesn\'t support multiple signatures') } var input = this.signatures[index] diff --git a/test/transaction_builder.js b/test/transaction_builder.js index fb2ae67..974f687 100644 --- a/test/transaction_builder.js +++ b/test/transaction_builder.js @@ -110,6 +110,15 @@ describe('TransactionBuilder', function() { }) }) + it('throws if scriptType doesn\'t support multiple signatures', function() { + txb.addInput(prevTxHash, 0) + txb.sign(0, privKey) + + assert.throws(function() { + txb.sign(0, privKey) + }, /pubkeyhash doesn\'t support multiple signatures/) + }) + describe('when redeemScript is undefined', function() { it('throws if prevOutScript is P2SH', function() { var privScriptP2SH = scripts.scriptHashOutput(privScript.getHash()) From e1479b6fa54a9c33c9c7cbd0f7378603d1643461 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 13:02:02 +1000 Subject: [PATCH 7/8] scripts: add error for >n signatures with multisig --- src/scripts.js | 10 +++++++--- test/fixtures/scripts.json | 13 +++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/scripts.js b/src/scripts.js index 661285a..88e3e61 100644 --- a/src/scripts.js +++ b/src/scripts.js @@ -238,9 +238,13 @@ function multisigInput(signatures, scriptPubKey) { if (scriptPubKey) { assert(isMultisigOutput.call(scriptPubKey)) - var m = scriptPubKey.chunks[0] - var k = m - (opcodes.OP_1 - 1) - assert(k <= signatures.length, 'Not enough signatures provided') + var mOp = scriptPubKey.chunks[0] + var nOp = scriptPubKey.chunks[scriptPubKey.chunks.length - 2] + var m = mOp - (opcodes.OP_1 - 1) + var n = nOp - (opcodes.OP_1 - 1) + + assert(signatures.length >= m, 'Not enough signatures provided') + assert(signatures.length <= n, 'Too many signatures provided') } return Script.fromChunks([].concat(opcodes.OP_0, signatures)) diff --git a/test/fixtures/scripts.json b/test/fixtures/scripts.json index 666ea59..e264a4a 100644 --- a/test/fixtures/scripts.json +++ b/test/fixtures/scripts.json @@ -101,6 +101,19 @@ "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf75939446f6ca2801" ], "scriptPubKey": false + }, + { + "exception": "Too many signatures provided", + "pubKeys": [ + "02359c6e3f04cefbf089cf1d6670dc47c3fb4df68e2bad1fa5a369f9ce4b42bbd1", + "0395a9d84d47d524548f79f435758c01faec5da2b7e551d3b8c995b7e06326ae4a" + ], + "signatures": [ + "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf75939446f6ca2801", + "3045022100ef253c1faa39e65115872519e5f0a33bbecf430c0f35cf562beabbad4da24d8d02201742be8ee49812a73adea3007c9641ce6725c32cd44ddb8e3a3af460015d140501", + "3045022100ef253c1faa39e65115872519e5f0a33bbecf430c0f35cf562beabbad4da24d8d02201742be8ee49812a73adea3007c9641ce6725c32cd44ddb8e3a3af460015d140501" + ], + "scriptPubKey": false } ] } From 1d0fd3e9a473108ce55fd9eb5a9726593d04caa2 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 30 Aug 2014 14:35:46 +1000 Subject: [PATCH 8/8] TxBuilder: add more failing cases and error handling --- src/transaction_builder.js | 6 +++++- test/fixtures/transaction_builder.json | 10 ++++++++++ test/transaction_builder.js | 10 ++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/transaction_builder.js b/src/transaction_builder.js index fc6a419..c0acb09 100644 --- a/src/transaction_builder.js +++ b/src/transaction_builder.js @@ -34,6 +34,10 @@ TransactionBuilder.fromTransaction = function(transaction) { // Ignore empty scripts if (txin.script.buffer.length === 0) return + assert(!Array.prototype.every.call(txin.hash, function(x) { + return x === 0 + }), 'coinbase inputs not supported') + var redeemScript var scriptSig = txin.script var scriptType = scripts.classifyInput(scriptSig) @@ -83,7 +87,7 @@ TransactionBuilder.fromTransaction = function(transaction) { break default: - assert(false, scriptType + ' not supported') + assert(false, scriptType + ' inputs not supported') } txb.signatures[i] = { diff --git a/test/fixtures/transaction_builder.json b/test/fixtures/transaction_builder.json index 6795c48..a44078b 100644 --- a/test/fixtures/transaction_builder.json +++ b/test/fixtures/transaction_builder.json @@ -167,6 +167,16 @@ } ] } + ], + "fromTransaction": [ + { + "exception": "coinbase inputs not supported", + "hex":"01000000010000000000000000000000000000000000000000000000000000000000000000000000006b483045022100a3b254e1c10b5d039f36c05f323995d6e5a367d98dd78a13d5bbc3991b35720e022022fccea3897d594de0689601fbd486588d5bfa6915be2386db0397ee9a6e80b601210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac00000000" + }, + { + "exception": "nonstandard inputs not supported", + "hex": "0100000001ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0000000023aa206fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d619000000000087ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac00000000" + } ] } } diff --git a/test/transaction_builder.js b/test/transaction_builder.js index 974f687..0bc1032 100644 --- a/test/transaction_builder.js +++ b/test/transaction_builder.js @@ -256,6 +256,16 @@ describe('TransactionBuilder', function() { }) }) + fixtures.invalid.fromTransaction.forEach(function(f,i) { + it('throws on ' + f.exception, function() { + var tx = Transaction.fromHex(f.hex) + + assert.throws(function() { + TransactionBuilder.fromTransaction(tx) + }, new RegExp(f.exception)) + }) + }) + it('works for the P2SH multisig case', function() { var privKeys = [ "91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgwmaKkrx",