From e835f1fe95496e7759d211f9660988b43f3c08a6 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Wed, 9 Nov 2016 13:01:29 +1100 Subject: [PATCH] TransactionBuilder: add fee safety --- src/transaction_builder.js | 56 +++++++++++++++++++++----- src/types.js | 2 +- test/fixtures/transaction_builder.json | 30 +++++++++----- test/transaction_builder.js | 1 + 4 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/transaction_builder.js b/src/transaction_builder.js index 6f5b47a..109944c 100644 --- a/src/transaction_builder.js +++ b/src/transaction_builder.js @@ -272,7 +272,10 @@ TransactionBuilder.fromTransaction = function (transaction, network) { // Copy inputs transaction.ins.forEach(function (txIn) { - txb.__addInputUnsafe(txIn.hash, txIn.index, txIn.sequence, txIn.script) + txb.__addInputUnsafe(txIn.hash, txIn.index, { + sequence: txIn.sequence, + script: txIn.script + }) }) // fix some things not possible through the public API @@ -288,6 +291,8 @@ TransactionBuilder.prototype.addInput = function (txHash, vout, sequence, prevOu throw new Error('No, this would invalidate signatures') } + var value + // is it a hex string? if (typeof txHash === 'string') { // transaction hashs's are displayed in reverse order, un-reverse it @@ -295,14 +300,21 @@ TransactionBuilder.prototype.addInput = function (txHash, vout, sequence, prevOu // is it a Transaction object? } else if (txHash instanceof Transaction) { - prevOutScript = txHash.outs[vout].script + var txOut = txHash.outs[vout] + prevOutScript = txOut.script + value = txOut.value + txHash = txHash.getHash() } - return this.__addInputUnsafe(txHash, vout, sequence, null, prevOutScript) + return this.__addInputUnsafe(txHash, vout, { + sequence: sequence, + prevOutScript: prevOutScript, + value: value + }) } -TransactionBuilder.prototype.__addInputUnsafe = function (txHash, vout, sequence, scriptSig, prevOutScript) { +TransactionBuilder.prototype.__addInputUnsafe = function (txHash, vout, options) { if (Transaction.isCoinbaseHash(txHash)) { throw new Error('coinbase inputs not supported') } @@ -313,16 +325,21 @@ TransactionBuilder.prototype.__addInputUnsafe = function (txHash, vout, sequence var input = {} // derive what we can from the scriptSig - if (scriptSig) { - input = expandInput(scriptSig) + if (options.script !== undefined) { + input = expandInput(options.script) + } + + // if an input value was given, retain it + if (options.value !== undefined) { + input.value = options.value } // derive what we can from the previous transactions output script - if (!input.prevOutScript && prevOutScript) { + if (!input.prevOutScript && options.prevOutScript) { var prevOutType if (!input.pubKeys && !input.signatures) { - var expanded = expandOutput(prevOutScript) + var expanded = expandOutput(options.prevOutScript) if (expanded.pubKeys) { input.pubKeys = expanded.pubKeys @@ -332,11 +349,11 @@ TransactionBuilder.prototype.__addInputUnsafe = function (txHash, vout, sequence prevOutType = expanded.scriptType } - input.prevOutScript = prevOutScript - input.prevOutType = prevOutType || bscript.classifyOutput(prevOutScript) + input.prevOutScript = options.prevOutScript + input.prevOutType = prevOutType || bscript.classifyOutput(options.prevOutScript) } - var vin = this.tx.addInput(txHash, vout, sequence, scriptSig) + var vin = this.tx.addInput(txHash, vout, options.sequence, options.scriptSig) this.inputs[vin] = input this.prevTxMap[prevTxOut] = true @@ -367,6 +384,9 @@ TransactionBuilder.prototype.__build = function (allowIncomplete) { if (!allowIncomplete) { if (!this.tx.ins.length) throw new Error('Transaction has no inputs') if (!this.tx.outs.length) throw new Error('Transaction has no outputs') + + // do not rely on this, its merely a last resort + if (this.__hasAbsurdFee()) throw new Error('Transaction has absurd fees') } var tx = this.tx.clone() @@ -479,4 +499,18 @@ TransactionBuilder.prototype.__canModifyOutputs = function () { }) } +TransactionBuilder.prototype.__hasAbsurdFee = function () { + // not all inputs will have .value defined + var incoming = this.inputs.reduce(function (a, x) { return a + (x.value >>> 0) }, 0) + + // but all outputs do, and if we have any input value + // we can immediately determine if the outputs are too small + var outgoing = this.tx.outs.reduce(function (a, x) { return a + x.value }, 0) + var fee = incoming - outgoing + + // its not fool-proof, but, it might help somebody + // fee > 0.2BTC + return fee > (0.2 * 1e8) +} + module.exports = TransactionBuilder diff --git a/src/types.js b/src/types.js index 29e53c5..c1e5fb1 100644 --- a/src/types.js +++ b/src/types.js @@ -10,7 +10,7 @@ function BIP32Path (value) { } BIP32Path.toJSON = function () { return 'BIP32 derivation path' } -var SATOSHI_MAX = 2.1 * 1e15 +var SATOSHI_MAX = 21 * 1e14 function Satoshi (value) { return typeforce.UInt53(value) && value <= SATOSHI_MAX } diff --git a/test/fixtures/transaction_builder.json b/test/fixtures/transaction_builder.json index 25ba79f..65402e6 100644 --- a/test/fixtures/transaction_builder.json +++ b/test/fixtures/transaction_builder.json @@ -657,20 +657,33 @@ "inputs": [ { "txId": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - "vout": 0, - "signs": [] + "vout": 0 } ], "outputs": [] }, + { + "exception": "Transaction has absurd fees", + "inputs": [ + { + "txHex": "01000000000100e1f505000000000000000000", + "vout": 0 + } + ], + "outputs": [ + { + "script": "OP_DUP OP_HASH160 ff99e06c1a4ac394b4e1cb3d3a4b2b47749e339a OP_EQUALVERIFY OP_CHECKSIG", + "value": 100000 + } + ] + }, { "description": "Incomplete transaction, nothing assumed", "exception": "Transaction is not complete", "inputs": [ { "txId": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - "vout": 0, - "signs": [] + "vout": 0 } ], "outputs": [ @@ -697,8 +710,7 @@ { "txId": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", "vout": 1, - "prevTxScript": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG", - "signs": [] + "prevTxScript": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG" } ], "outputs": [ @@ -715,13 +727,11 @@ "inputs": [ { "txId": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - "vout": 0, - "signs": [] + "vout": 0 }, { "txId": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - "vout": 0, - "signs": [] + "vout": 0 } ], "outputs": [ diff --git a/test/transaction_builder.js b/test/transaction_builder.js index 97c52ca..b370b06 100644 --- a/test/transaction_builder.js +++ b/test/transaction_builder.js @@ -37,6 +37,7 @@ function construct (f, sign) { if (sign === false) return txb f.inputs.forEach(function (input, index) { + if (!input.signs) return input.signs.forEach(function (sign) { var keyPair = ECPair.fromWIF(sign.keyPair, network) var redeemScript