From b088183ce9ecf1d289bbbc68ff089a59f5ba7571 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 11 Mar 2015 14:29:13 -0300 Subject: [PATCH 1/3] test locked funds --- test/integration/server.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/integration/server.js b/test/integration/server.js index 4fc094b..0afe5b0 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -984,6 +984,27 @@ describe('Copay server', function() { }); }); + it.only('should fail with different error for insufficient funds and locked funds', function(done) { + helpers.stubUtxos(server, wallet, [10, 10], function() { + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 11, null, TestData.copayers[0].privKey_1H_0); + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + server.getBalance({}, function(err, balance) { + should.not.exist(err); + balance.totalAmount.should.equal(helpers.toSatoshi(20)); + balance.lockedAmount.should.equal(helpers.toSatoshi(20)); + txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 8, null, TestData.copayers[0].privKey_1H_0); + server.createTx(txOpts, function(err, tx) { + should.exist(err); + err.code.should.equal('LOCKEDFUNDS'); + err.message.should.equal('Funds are locked by pending transaction proposals'); + done(); + }); + }); + }); + }); + }); + it('should fail gracefully when bitcore throws exception on raw tx creation', function(done) { helpers.stubUtxos(server, wallet, [10], function() { var bitcoreStub = sinon.stub(Bitcore, 'Transaction'); From b5f6582b777b1cda98702a34221b3d856c4ec46c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 11 Mar 2015 15:04:42 -0300 Subject: [PATCH 2/3] refactor tx inputs selection --- lib/server.js | 98 +++++++++++++++++++------------------- test/integration/server.js | 2 +- 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/lib/server.js b/lib/server.js index b2d2520..014bb22 100644 --- a/lib/server.js +++ b/lib/server.js @@ -464,33 +464,42 @@ WalletService.prototype.getBalance = function(opts, cb) { }); }; +WalletService.prototype._selectTxInputs = function(txp, cb) { + var self = this; -WalletService.prototype._selectUtxos = function(txp, utxos) { - var i = 0; - var total = 0; - var selected = []; - var inputs = _.sortBy(utxos, 'amount'); + self._getUtxos(function(err, utxos) { + if (err) return cb(err); - while (i < inputs.length) { - selected.push(inputs[i]); - total += inputs[i].satoshis; + utxos = _.reject(utxos, { + locked: true + }); - if (total >= txp.amount + Bitcore.Transaction.FEE_PER_KB) { - try { - // Check if there are enough fees - txp.inputs = selected; - var raw = txp.getRawTx(); - return; - } catch (ex) { - if (ex.name != 'bitcore.ErrorTransactionFeeError') { - throw ex.message; + var i = 0; + var total = 0; + var selected = []; + var inputs = _.sortBy(utxos, 'amount'); + + while (i < inputs.length) { + selected.push(inputs[i]); + total += inputs[i].satoshis; + + if (total >= txp.amount + Bitcore.Transaction.FEE_PER_KB) { + try { + // Check if there are enough fees + txp.inputs = selected; + var raw = txp.getRawTx(); + txp.inputPaths = _.pluck(txp.inputs, 'path'); + return cb(); + } catch (ex) { + if (ex.name != 'bitcore.ErrorTransactionFeeError') { + return cb(ex); + } } } - } - i++; - }; - txp.inputs = null; - return; + i++; + }; + return cb(new ClientError('INSUFFICIENTFUNDS', 'Insufficient funds')); + }); }; @@ -534,36 +543,25 @@ WalletService.prototype.createTx = function(opts, cb) { if (opts.amount < Bitcore.Transaction.DUST_AMOUNT) return cb(new ClientError('DUSTAMOUNT', 'Amount below dust threshold')); - self._getUtxos(function(err, utxos) { + + var changeAddress = wallet.createAddress(true); + + var txp = TxProposal.create({ + creatorId: self.copayerId, + toAddress: opts.toAddress, + amount: opts.amount, + message: opts.message, + proposalSignature: opts.proposalSignature, + changeAddress: changeAddress, + requiredSignatures: wallet.m, + requiredRejections: Math.min(wallet.m, wallet.n - wallet.m + 1), + }); + + + self._selectTxInputs(txp, function(err) { if (err) return cb(err); - var changeAddress = wallet.createAddress(true); - - utxos = _.reject(utxos, { - locked: true - }); - - var txp = TxProposal.create({ - creatorId: self.copayerId, - toAddress: opts.toAddress, - amount: opts.amount, - message: opts.message, - proposalSignature: opts.proposalSignature, - changeAddress: changeAddress, - requiredSignatures: wallet.m, - requiredRejections: Math.min(wallet.m, wallet.n - wallet.m + 1), - }); - - try { - self._selectUtxos(txp, utxos); - } catch (ex) { - return cb(new ClientError(ex.toString())); - } - - if (!txp.inputs) - return cb(new ClientError('INSUFFICIENTFUNDS', 'Insufficient funds')); - - txp.inputPaths = _.pluck(txp.inputs, 'path'); + $.checkState(txp.inputs); self.storage.storeAddressAndWallet(wallet, changeAddress, function(err) { if (err) return cb(err); diff --git a/test/integration/server.js b/test/integration/server.js index 0afe5b0..cfd878d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -984,7 +984,7 @@ describe('Copay server', function() { }); }); - it.only('should fail with different error for insufficient funds and locked funds', function(done) { + it('should fail with different error for insufficient funds and locked funds', function(done) { helpers.stubUtxos(server, wallet, [10, 10], function() { var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 11, null, TestData.copayers[0].privKey_1H_0); server.createTx(txOpts, function(err, tx) { From 5d537afc603529f05b0c52c0dafa3dea134e109b Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 11 Mar 2015 17:04:20 -0300 Subject: [PATCH 3/3] improve error detection when building tx --- lib/model/txproposal.js | 6 +++--- lib/server.js | 43 ++++++++++++++++++++++++++------------ test/integration/server.js | 23 ++++++++++++++++++-- test/models/txproposal.js | 2 +- 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index bc6e0a1..ef77ec4 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -86,7 +86,7 @@ TxProposal.prototype._getCurrentSignatures = function() { }); }; -TxProposal.prototype._getBitcoreTx = function() { +TxProposal.prototype.getBitcoreTx = function() { var self = this; var t = new Bitcore.Transaction(); @@ -113,7 +113,7 @@ TxProposal.prototype.getNetworkName = function() { }; TxProposal.prototype.getRawTx = function() { - var t = this._getBitcoreTx(); + var t = this.getBitcoreTx(); return t.uncheckedSerialize(); }; @@ -186,7 +186,7 @@ TxProposal.prototype._addSignaturesToBitcoreTx = function(t, signatures, xpub) { TxProposal.prototype.sign = function(copayerId, signatures, xpub) { try { // Tests signatures are OK - var t = this._getBitcoreTx(); + var t = this.getBitcoreTx(); this._addSignaturesToBitcoreTx(t, signatures, xpub); this.addAction(copayerId, 'accept', null, signatures, xpub); diff --git a/lib/server.js b/lib/server.js index 014bb22..4481849 100644 --- a/lib/server.js +++ b/lib/server.js @@ -421,6 +421,20 @@ WalletService.prototype._getUtxos = function(cb) { }); }; +WalletService.prototype._totalizeUtxos = function(utxos) { + var balance = {}; + balance.totalAmount = Utils.strip(_.reduce(utxos, function(sum, utxo) { + return sum + utxo.satoshis; + }, 0)); + + balance.lockedAmount = Utils.strip(_.reduce(_.filter(utxos, { + locked: true + }), function(sum, utxo) { + return sum + utxo.satoshis; + }, 0)); + return balance; +}; + /** * Creates a new transaction proposal. @@ -433,16 +447,7 @@ WalletService.prototype.getBalance = function(opts, cb) { self._getUtxos(function(err, utxos) { if (err) return cb(err); - var balance = {}; - balance.totalAmount = Utils.strip(_.reduce(utxos, function(sum, utxo) { - return sum + utxo.satoshis; - }, 0)); - - balance.lockedAmount = Utils.strip(_.reduce(_.filter(utxos, { - locked: true - }), function(sum, utxo) { - return sum + utxo.satoshis; - }, 0)); + var balance = self._totalizeUtxos(utxos); // Compute balance by address var byAddress = {}; @@ -470,6 +475,16 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { self._getUtxos(function(err, utxos) { if (err) return cb(err); + var balance = self._totalizeUtxos(utxos); + + var txMinAmount = txp.amount + Bitcore.Transaction.FEE_PER_KB; + if (balance.totalAmount < txMinAmount) + return cb(new ClientError('INSUFFICIENTFUNDS', 'Insufficient funds')); + + if ((balance.totalAmount - balance.lockedAmount) < txMinAmount) + return cb(new ClientError('LOCKEDFUNDS', 'Funds are locked by pending transaction proposals')); + + utxos = _.reject(utxos, { locked: true }); @@ -478,16 +493,17 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { var total = 0; var selected = []; var inputs = _.sortBy(utxos, 'amount'); + var bitcoreTx; while (i < inputs.length) { selected.push(inputs[i]); total += inputs[i].satoshis; - if (total >= txp.amount + Bitcore.Transaction.FEE_PER_KB) { + if (total >= txMinAmount) { try { // Check if there are enough fees txp.inputs = selected; - var raw = txp.getRawTx(); + bitcoreTx = txp.getBitcoreTx(); txp.inputPaths = _.pluck(txp.inputs, 'path'); return cb(); } catch (ex) { @@ -498,7 +514,8 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { } i++; }; - return cb(new ClientError('INSUFFICIENTFUNDS', 'Insufficient funds')); + + return cb(new ClientError('INSUFFICIENTFUNDS', 'Insufficient funds for fee')); }); }; diff --git a/test/integration/server.js b/test/integration/server.js index cfd878d..6b49596 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -18,6 +18,7 @@ var WalletUtils = require('bitcore-wallet-utils'); var Storage = require('../../lib/storage'); var Wallet = require('../../lib/model/wallet'); +var TxProposal = require('../../lib/model/txproposal'); var Address = require('../../lib/model/address'); var Copayer = require('../../lib/model/copayer'); var WalletService = require('../../lib/server'); @@ -1005,6 +1006,25 @@ describe('Copay server', function() { }); }); + it('should fail with insufficient funds if fee is too large', function(done) { + helpers.stubUtxos(server, wallet, 10, function() { + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 9, null, TestData.copayers[0].privKey_1H_0); + + var txpStub = sinon.stub(TxProposal.prototype, 'getBitcoreTx').throws({ + name: 'bitcore.ErrorTransactionFeeError' + }); + + server.createTx(txOpts, function(err, tx) { + should.exist(err); + err.code.should.equal('INSUFFICIENTFUNDS'); + err.message.should.equal('Insufficient funds for fee'); + + txpStub.restore(); + done(); + }); + }); + }); + it('should fail gracefully when bitcore throws exception on raw tx creation', function(done) { helpers.stubUtxos(server, wallet, [10], function() { var bitcoreStub = sinon.stub(Bitcore, 'Transaction'); @@ -1055,8 +1075,7 @@ describe('Copay server', function() { should.exist(tx); var txOpts2 = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 24, null, TestData.copayers[0].privKey_1H_0); server.createTx(txOpts2, function(err, tx) { - err.code.should.equal('INSUFFICIENTFUNDS'); - err.message.should.equal('Insufficient funds'); + err.code.should.equal('LOCKEDFUNDS'); should.not.exist(tx); server.getPendingTxs({}, function(err, txs) { should.not.exist(err); diff --git a/test/models/txproposal.js b/test/models/txproposal.js index 1b646c1..119ab88 100644 --- a/test/models/txproposal.js +++ b/test/models/txproposal.js @@ -19,7 +19,7 @@ describe('TXProposal', function() { describe('#_getBitcoreTx', function() { it('should create a valid bitcore TX', function() { var txp = TXP.fromObj(aTXP()); - var t = txp._getBitcoreTx(); + var t = txp.getBitcoreTx(); should.exist(t); }); });