From 2bdc6cf19d7b96e930396d7c16a59ca37cc5f3fc Mon Sep 17 00:00:00 2001 From: Kosta Korenkov <7r0ggy@gmail.com> Date: Tue, 28 Jul 2015 09:54:23 +0300 Subject: [PATCH] Make API.createTx accept preselected inputs Add EXTERNAL tx type It will allow clients to specify inputs and outputs when creating proposals Extract _validateOutputs method Use for-loop to simplify and make it fail-fast. Do not validate outputs for EXTERNAL tx type Outputs may be of any kind (e.g. OP_RETURN). It is assumed that outputs are validated by caller. Move output validation to a single place Check tx and estimate fees for EXTERNAL proposal Set amount for external proposals Use changeAddress to determine network Do not shuffle outputs for EXTERNAL txp Fix input selection to respect fees Add more inputs, if we selected exactly the amount to spend, so we avoid 'Insufficient funds for a fee' error Properly handle UTXO with 0 satoshis Minor fixes ignore invalid utxos --- lib/model/txproposal.js | 18 +++-- lib/server.js | 137 ++++++++++++++++++++---------------- test/integration/helpers.js | 31 +++++--- test/integration/server.js | 20 ++++++ test/models/txproposal.js | 29 +++++++- 5 files changed, 160 insertions(+), 75 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index ee6896a..9d7eea5 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -20,6 +20,7 @@ function TxProposal() {}; TxProposal.Types = { SIMPLE: 'simple', MULTIPLEOUTPUTS: 'multiple_outputs', + EXTERNAL: 'external' }; TxProposal.isTypeSupported = function(type) { @@ -49,6 +50,16 @@ TxProposal._create.multiple_outputs = function(txp, opts) { } catch (ex) {} }; +TxProposal._create.external = function(txp, opts) { + txp.setInputs(opts.inputs || []); + txp.outputs = opts.outputs; + txp.outputOrder = _.range(txp.outputs.length + 1); + txp.amount = txp.getTotalAmount(); + try { + txp.network = Bitcore.Address(txp.outputs[0].toAddress).toObject().network; + } catch (ex) {} +}; + TxProposal.create = function(opts) { opts = opts || {}; @@ -243,8 +254,7 @@ TxProposal.prototype.getBitcoreTx = function() { }; TxProposal.prototype.getNetworkName = function() { - var someAddress = this.toAddress || this.outputs[0].toAddress; - return Bitcore.Address(someAddress).toObject().network; + return Bitcore.Address(this.changeAddress.address).toObject().network; }; TxProposal.prototype.getRawTx = function() { @@ -281,11 +291,11 @@ TxProposal.prototype.estimateFee = function() { * @return {Number} total amount of all outputs excluding change output */ TxProposal.prototype.getTotalAmount = function() { - if (this.type == TxProposal.Types.MULTIPLEOUTPUTS) { + if (this.type == TxProposal.Types.MULTIPLEOUTPUTS || this.type == TxProposal.Types.EXTERNAL) { return _.pluck(this.outputs, 'amount') .reduce(function(total, n) { return total + n; - }); + }, 0); } else { return this.amount; } diff --git a/lib/server.js b/lib/server.js index bc3de08..de7d8fa 100644 --- a/lib/server.js +++ b/lib/server.js @@ -827,7 +827,7 @@ WalletService.prototype._getUtxosForAddresses = function(addresses, cb) { var u = _.pick(utxo, ['txid', 'vout', 'address', 'scriptPubKey', 'amount', 'satoshis', 'confirmations']); u.confirmations = u.confirmations || 0; u.locked = false; - u.satoshis = u.satoshis ? +u.satoshis : Utils.strip(u.amount * 1e8); + u.satoshis = _.isNumber(u.satoshis) ? +u.satoshis : Utils.strip(u.amount * 1e8); delete u.amount; return u; }); @@ -1026,8 +1026,43 @@ WalletService.prototype.getFeeLevels = function(opts, cb) { }); }; +WalletService.prototype._checkTxAndEstimateFee = function(txp) { + var bitcoreError; + + var serializationOpts = { + disableIsFullySigned: true + }; + if (!_.startsWith(txp.version, '1.')) { + serializationOpts.disableSmallFees = true; + serializationOpts.disableLargeFees = true; + } + + try { + txp.estimateFee(); + var bitcoreTx = txp.getBitcoreTx(); + bitcoreError = bitcoreTx.getSerializationError(serializationOpts); + if (!bitcoreError) { + txp.fee = bitcoreTx.getFee(); + } + } catch (ex) { + log.error('Error building Bitcore transaction', ex); + return ex; + } + + if (bitcoreError instanceof Bitcore.errors.Transaction.FeeError) + return Errors.INSUFFICIENT_FUNDS_FOR_FEE; + + if (bitcoreError instanceof Bitcore.errors.Transaction.DustOutputs) + return Errors.DUST_AMOUNT; + return bitcoreError; +}; + WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var self = this; + //todo: check inputs are ours and has enough value + if (txp.inputs && txp.inputs.length > 0) { + return cb(self._checkTxAndEstimateFee(txp)); + } function sortUtxos(utxos) { var list = _.map(utxos, function(utxo) { @@ -1086,13 +1121,6 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var inputs = sortUtxos(utxos); var bitcoreTx, bitcoreError; - var serializationOpts = { - disableIsFullySigned: true, - }; - if (!_.startsWith(txp.version, '1.')) { - serializationOpts.disableSmallFees = true; - serializationOpts.disableLargeFees = true; - } while (i < inputs.length) { selected.push(inputs[i]); @@ -1100,28 +1128,14 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { i++; if (total >= txp.getTotalAmount()) { - try { - txp.setInputs(selected); - txp.estimateFee(); - bitcoreTx = txp.getBitcoreTx(); - bitcoreError = bitcoreTx.getSerializationError(serializationOpts); - if (!bitcoreError) { - txp.fee = bitcoreTx.getFee(); - return cb(); - } - } catch (ex) { - log.error('Error building Bitcore transaction', ex); - return cb(ex); + txp.setInputs(selected); + bitcoreError = self._checkTxAndEstimateFee(txp); + if (!bitcoreError) { + return cb(); } } } - if (bitcoreError instanceof Bitcore.errors.Transaction.FeeError) - return cb(Errors.INSUFFICIENT_FUNDS_FOR_FEE); - - if (bitcoreError instanceof Bitcore.errors.Transaction.DustOutputs) - return cb(Errors.DUST_AMOUNT); - return cb(bitcoreError || new Error('Could not select tx inputs')); }); }; @@ -1155,6 +1169,34 @@ WalletService.prototype._canCreateTx = function(copayerId, cb) { }); }; +WalletService.prototype._validateOutputs = function(opts, wallet) { + for (var i = 0; i < opts.outputs.length; i++) { + var output = opts.outputs[i]; + output.valid = false; + + if (!Utils.checkRequired(output, ['toAddress', 'amount'])) { + return new ClientError('Required outputs argument missing'); + } + + var toAddress = {}; + try { + toAddress = new Bitcore.Address(output.toAddress); + } catch (ex) { + return Errors.INVALID_ADDRESS; + } + if (toAddress.network != wallet.getNetworkName()) { + return Errors.INCORRECT_ADDRESS_NETWORK; + } + if (!_.isNumber(output.amount) || _.isNaN(output.amount) || output.amount <= 0) { + return new ClientError('Invalid amount'); + } + if (output.amount < Bitcore.Transaction.DUST_AMOUNT) { + return Errors.DUST_AMOUNT; + } + output.valid = true; + } + return null; +}; WalletService._getProposalHash = function(proposalHeader) { function getOldHash(toAddress, amount, message, payProUrl) { @@ -1178,6 +1220,7 @@ WalletService._getProposalHash = function(proposalHeader) { * @param {string} opts.outputs[].message - A message to attach to this output. * @param {string} opts.message - A message to attach to this transaction. * @param {string} opts.proposalSignature - S(toAddress|amount|message|payProUrl). Used by other copayers to verify the proposal. + * @param {string} opts.inputs - Optional. Inputs for this TX * @param {string} opts.feePerKb - Optional: Use an alternative fee per KB for this TX * @param {string} opts.payProUrl - Optional: Paypro URL for peers to verify TX * @param {string} opts.excludeUnconfirmedUtxos - Optional: Do not use UTXOs of unconfirmed transactions as inputs @@ -1198,17 +1241,6 @@ WalletService.prototype.createTx = function(opts, cb) { if (!Model.TxProposal.isTypeSupported(type)) return cb(new ClientError('Invalid proposal type')); - _.each(opts.outputs, function(output) { - if (!Utils.checkRequired(output, ['toAddress', 'amount'])) { - output.valid = false; - cb(new ClientError('Required outputs argument missing')); - return false; - } - }); - if (_.any(opts.outputs, { - valid: false - })) return; - var feePerKb = opts.feePerKb || Defaults.DEFAULT_FEE_PER_KB; if (feePerKb < Defaults.MIN_FEE_PER_KB || feePerKb > Defaults.MAX_FEE_PER_KB) return cb(new ClientError('Invalid fee per KB value')); @@ -1242,38 +1274,19 @@ WalletService.prototype.createTx = function(opts, cb) { if (err) return cb(err); if (!canCreate) return cb(Errors.TX_CANNOT_CREATE); - _.each(opts.outputs, function(output) { - output.valid = false; - var toAddress = {}; - try { - toAddress = new Bitcore.Address(output.toAddress); - } catch (ex) { - cb(Errors.INVALID_ADDRESS); - return false; - } - if (toAddress.network != wallet.getNetworkName()) { - cb(Errors.INCORRECT_ADDRESS_NETWORK); - return false; + if (type != Model.TxProposal.Types.EXTERNAL) { + var validationError = self._validateOutputs(opts, wallet); + if (validationError) { + return cb(validationError); } - if (!_.isNumber(output.amount) || _.isNaN(output.amount) || output.amount <= 0) { - cb(new ClientError('Invalid amount')); - return false; - } - if (output.amount < Bitcore.Transaction.DUST_AMOUNT) { - cb(Errors.DUST_AMOUNT); - return false; - } - output.valid = true; - }); - if (_.any(opts.outputs, { - valid: false - })) return; + } var txOpts = { type: type, walletId: self.walletId, creatorId: self.copayerId, outputs: opts.outputs, + inputs: opts.inputs, toAddress: opts.toAddress, amount: opts.amount, message: opts.message, diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 9adf561..45deb91 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -215,8 +215,7 @@ helpers.stubUtxos = function(server, wallet, amounts, cb) { }, function(err, addresses) { should.not.exist(err); addresses.should.not.be.empty; - var utxos = _.map([].concat(amounts), function(amount, i) { - var address = addresses[i % addresses.length]; + var utxos = _.compact(_.map([].concat(amounts), function(amount, i) { var confirmations; if (_.isString(amount) && _.startsWith(amount, 'u')) { amount = parseFloat(amount.substring(1)); @@ -224,6 +223,9 @@ helpers.stubUtxos = function(server, wallet, amounts, cb) { } else { confirmations = Math.floor(Math.random() * 100 + 1); } + if (amount <= 0) return null; + + var address = addresses[i % addresses.length]; var scriptPubKey; switch (wallet.addressType) { @@ -239,12 +241,12 @@ helpers.stubUtxos = function(server, wallet, amounts, cb) { return { txid: helpers.randomTXID(), vout: Math.floor(Math.random() * 10 + 1), - satoshis: helpers.toSatoshi(amount).toString(), + satoshis: helpers.toSatoshi(amount), scriptPubKey: scriptPubKey.toBuffer().toString('hex'), address: address.address, - confirmations: confirmations, + confirmations: confirmations }; - }); + })); blockchainExplorer.getUnspentUtxos = function(addresses, cb) { var selected = _.filter(utxos, function(utxo) { return _.contains(addresses, utxo.address); @@ -357,14 +359,27 @@ helpers.createSimpleProposalOpts = function(toAddress, amount, signingKey, opts) return helpers.createProposalOpts(Model.TxProposal.Types.SIMPLE, outputs, signingKey, opts); }; -helpers.createProposalOpts = function(type, outputs, signingKey, moreOpts) { +helpers.createExternalProposalOpts = function(toAddress, amount, signingKey, moreOpts, inputs) { + var outputs = [{ + toAddress: toAddress, + amount: amount, + }]; + if (_.isArray(moreOpts)) { + inputs = moreOpts; + moreOpts = null; + } + return helpers.createProposalOpts(Model.TxProposal.Types.EXTERNAL, outputs, signingKey, moreOpts, inputs); +}; + +helpers.createProposalOpts = function(type, outputs, signingKey, moreOpts, inputs) { _.each(outputs, function(output) { output.amount = helpers.toSatoshi(output.amount); }); var opts = { type: type, - proposalSignature: null + proposalSignature: null, + inputs: inputs || [] }; if (moreOpts) { @@ -384,7 +399,7 @@ helpers.createProposalOpts = function(type, outputs, signingKey, moreOpts) { opts.amount = outputs[0].amount; hash = WalletService._getProposalHash(opts.toAddress, opts.amount, opts.message, opts.payProUrl); - } else if (type == Model.TxProposal.Types.MULTIPLEOUTPUTS) { + } else if (type == Model.TxProposal.Types.MULTIPLEOUTPUTS || type == Model.TxProposal.Types.EXTERNAL) { opts.outputs = outputs; var header = { outputs: outputs, diff --git a/test/integration/server.js b/test/integration/server.js index 4db9d13..7632932 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2166,6 +2166,26 @@ describe('Wallet service', function() { }); }); + it('should be able to create tx with inputs argument', function(done) { + helpers.stubUtxos(server, wallet, [1, 3, 2], function(utxos) { + server._getUtxosForCurrentWallet(function(err, utxos) { + should.not.exist(err); + var inputs = [utxos[0], utxos[2]]; + var txOpts = helpers.createExternalProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 2.5, + TestData.copayers[0].privKey_1H_0, inputs); + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.inputs.length.should.equal(2); + var txids = _.pluck(tx.inputs, 'txid'); + txids.should.contain(utxos[0].txid); + txids.should.contain(utxos[2].txid); + done(); + }); + }); + }); + }); + it('should be able to send max amount', function(done) { helpers.stubUtxos(server, wallet, _.range(1, 10, 0), function() { server.getBalance({}, function(err, balance) { diff --git a/test/models/txproposal.js b/test/models/txproposal.js index 525cc0c..2ea3eed 100644 --- a/test/models/txproposal.js +++ b/test/models/txproposal.js @@ -22,6 +22,13 @@ describe('TXProposal', function() { should.not.exist(txp.toAddress); should.exist(txp.outputs); }); + it('should create an external TxProposal', function() { + var txp = TxProposal.create(aTxpOpts(TxProposal.Types.EXTERNAL)); + should.exist(txp); + should.not.exist(txp.toAddress); + should.exist(txp.outputs); + should.exist(txp.inputs); + }); }); describe('#fromObj', function() { @@ -76,6 +83,15 @@ describe('TXProposal', function() { }); x.getTotalAmount().should.equal(totalOutput); }); + it('should handle external', function() { + var x = TxProposal.fromObj(aTXP(TxProposal.Types.EXTERNAL)); + var totalOutput = 0; + _.each(x.outputs, function(o) { + totalOutput += o.amount + }); + x.getTotalAmount().should.equal(totalOutput); + }); + }); describe('#sign', function() { @@ -135,7 +151,7 @@ var aTxpOpts = function(type) { amount: 50000000, message: 'some message' }; - if (type == TxProposal.Types.MULTIPLEOUTPUTS) { + if (type == TxProposal.Types.MULTIPLEOUTPUTS || type == TxProposal.Types.EXTERNAL) { opts.outputs = [{ toAddress: "18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7", amount: 10000000, @@ -148,6 +164,17 @@ var aTxpOpts = function(type) { delete opts.toAddress; delete opts.amount; } + if (type == TxProposal.Types.EXTERNAL) { + opts.inputs = [{ + "txid": "6ee699846d2d6605f96d20c7cc8230382e5da43342adb11b499bbe73709f06ab", + "vout": 8, + "satoshis": 100000000, + "scriptPubKey": "a914a8a9648754fbda1b6c208ac9d4e252075447f36887", + "address": "3H4pNP6J4PW4NnvdrTg37VvZ7h2QWuAwtA", + "path": "m/2147483647/0/1", + "publicKeys": ["0319008ffe1b3e208f5ebed8f46495c056763f87b07930a7027a92ee477fb0cb0f", "03b5f035af8be40d0db5abb306b7754949ab39032cf99ad177691753b37d101301"] + }]; + } return opts; };