From 06b521853dfc7dbddef8550a5637ba7b522b395a Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 10 Jun 2015 16:26:40 -0300 Subject: [PATCH 1/7] test not enough fees --- test/integration/server.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/integration/server.js b/test/integration/server.js index ee38dc2..c1ef9d4 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -214,7 +214,7 @@ helpers.createAddresses = function(server, wallet, main, change, cb) { var storage, blockchainExplorer; -var useMongo = false; +var useMongo = true; function initStorage(cb) { function getDb(cb) { @@ -1322,7 +1322,6 @@ describe('Wallet service', function() { }); }); - it('should fail to create tx with invalid proposal signature', function(done) { helpers.stubUtxos(server, wallet, [100, 200], function() { var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 80, null, 'dummy'); @@ -1408,9 +1407,9 @@ describe('Wallet service', function() { }); }); - it('should fail to create tx when insufficient funds for fee', function(done) { - helpers.stubUtxos(server, wallet, [100], function() { - var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 100, null, TestData.copayers[0].privKey_1H_0); + it.only('should fail to create tx when insufficient funds for fee', function(done) { + helpers.stubUtxos(server, wallet, 0.04822200, function() { + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.04820000, null, TestData.copayers[0].privKey_1H_0); server.createTx(txOpts, function(err, tx) { should.exist(err); err.code.should.equal('INSUFFICIENTFUNDS'); From 9f4ef16bba1afb40667309bf89d423d130fca555 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 10 Jun 2015 17:57:48 -0300 Subject: [PATCH 2/7] test fee behavior --- lib/server.js | 2 ++ test/integration/server.js | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/server.js b/lib/server.js index bdad5ba..cfa7903 100644 --- a/lib/server.js +++ b/lib/server.js @@ -693,6 +693,8 @@ WalletService.prototype.getBalance = function(opts, cb) { WalletService.prototype._selectTxInputs = function(txp, cb) { var self = this; + Bitcore.Transaction.FEE_SECURITY_MARGIN = 1; + self._getUtxos(function(err, utxos) { if (err) return cb(err); diff --git a/test/integration/server.js b/test/integration/server.js index c1ef9d4..c17f4f4 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1407,9 +1407,9 @@ describe('Wallet service', function() { }); }); - it.only('should fail to create tx when insufficient funds for fee', function(done) { - helpers.stubUtxos(server, wallet, 0.04822200, function() { - var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.04820000, null, TestData.copayers[0].privKey_1H_0); + it('should fail to create tx when insufficient funds for fee', function(done) { + helpers.stubUtxos(server, wallet, 0.048222, function() { + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.048200, null, TestData.copayers[0].privKey_1H_0); server.createTx(txOpts, function(err, tx) { should.exist(err); err.code.should.equal('INSUFFICIENTFUNDS'); @@ -1419,6 +1419,18 @@ describe('Wallet service', function() { }); }); + it('should scale fees according to tx size', function(done) { + helpers.stubUtxos(server, wallet, [1, 1, 1, 1], function() { + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 3.5, null, TestData.copayers[0].privKey_1H_0); + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + tx.getBitcoreTx()._estimateSize().should.be.within(1001, 1999); + tx.fee.should.equal(20000); + done(); + }); + }); + }); + it('should fail to create tx for dust amount', function(done) { helpers.stubUtxos(server, wallet, [1], function() { var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.00000001, null, TestData.copayers[0].privKey_1H_0); @@ -2445,7 +2457,7 @@ describe('Wallet service', function() { helpers.createAndJoinWallet(1, 1, function(s, w) { server = s; wallet = w; - helpers.stubUtxos(server, wallet, helpers.toSatoshi(_.range(4)), function() { + helpers.stubUtxos(server, wallet, _.range(4), function() { var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.01, null, TestData.copayers[0].privKey_1H_0); async.eachSeries(_.range(3), function(i, next) { server.createTx(txOpts, function(err, tx) { From 366638b2d4110307d8ffa3d045134d1a71a467cb Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 11 Jun 2015 15:26:34 -0300 Subject: [PATCH 3/7] implement variable fee per kb --- lib/model/txproposal.js | 2 ++ lib/server.js | 12 ++++++++---- test/integration/server.js | 30 ++++++++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index c102339..97ae8fb 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -37,6 +37,7 @@ TxProposal.create = function(opts) { x.outputOrder = _.shuffle(_.range(2)); x.fee = null; x.network = Bitcore.Address(x.toAddress).toObject().network; + x.feePerKb = opts.feePerKb; return x; }; @@ -68,6 +69,7 @@ TxProposal.fromObj = function(obj) { x.outputOrder = obj.outputOrder; x.fee = obj.fee; x.network = obj.network; + x.feePerKb = obj.feePerKb; return x; }; diff --git a/lib/server.js b/lib/server.js index cfa7903..bdc2db8 100644 --- a/lib/server.js +++ b/lib/server.js @@ -693,8 +693,6 @@ WalletService.prototype.getBalance = function(opts, cb) { WalletService.prototype._selectTxInputs = function(txp, cb) { var self = this; - Bitcore.Transaction.FEE_SECURITY_MARGIN = 1; - self._getUtxos(function(err, utxos) { if (err) return cb(err); @@ -730,6 +728,7 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { if (!bitcoreError) { txp.inputPaths = _.pluck(txp.inputs, 'path'); txp.fee = bitcoreTx.getFee(); + $.checkState(txp.fee < 1e8, 'Fees are too high!'); return cb(); } } catch (ex) { @@ -787,7 +786,8 @@ WalletService.prototype._canCreateTx = function(copayerId, cb) { * @param {number} opts.amount - Amount to transfer in satoshi. * @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.payProUrl - Options: Paypro URL for peers to verify 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 * @returns {TxProposal} Transaction proposal. */ WalletService.prototype.createTx = function(opts, cb) { @@ -796,6 +796,10 @@ WalletService.prototype.createTx = function(opts, cb) { if (!Utils.checkRequired(opts, ['toAddress', 'amount', 'proposalSignature'])) return cb(new ClientError('Required argument missing')); + var feePerKb = opts.feePerKb || 10000; + if (!_.contains([1000, 5000, 10000], feePerKb)) + return cb(new ClientError('Invalid fee per KB value')); + self._runLocked(cb, function(cb) { self.getWallet({}, function(err, wallet) { if (err) return cb(err); @@ -827,7 +831,6 @@ WalletService.prototype.createTx = function(opts, cb) { if (opts.amount < Bitcore.Transaction.DUST_AMOUNT) return cb(new ClientError('DUSTAMOUNT', 'Amount below dust threshold')); - var changeAddress = wallet.createAddress(true); var txp = Model.TxProposal.create({ @@ -837,6 +840,7 @@ WalletService.prototype.createTx = function(opts, cb) { amount: opts.amount, message: opts.message, proposalSignature: opts.proposalSignature, + feePerKb: feePerKb, payProUrl: opts.payProUrl, changeAddress: changeAddress, requiredSignatures: wallet.m, diff --git a/test/integration/server.js b/test/integration/server.js index c17f4f4..005029f 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -184,13 +184,15 @@ helpers.stubAddressActivity = function(activeAddresses) { helpers.clientSign = WalletUtils.signTxp; -helpers.createProposalOpts = function(toAddress, amount, message, signingKey) { +helpers.createProposalOpts = function(toAddress, amount, message, signingKey, feePerKb) { var opts = { toAddress: toAddress, amount: helpers.toSatoshi(amount), message: message, proposalSignature: null, }; + if (feePerKb) opts.feePerKb = feePerKb; + var hash = WalletUtils.getProposalHash(opts.toAddress, opts.amount, opts.message); try { opts.proposalSignature = WalletUtils.signMessage(hash, signingKey); @@ -1431,6 +1433,30 @@ describe('Wallet service', function() { }); }); + it('should be possible to use a smaller fee', function(done) { + helpers.stubUtxos(server, wallet, 1, function() { + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.99995, null, TestData.copayers[0].privKey_1H_0); + server.createTx(txOpts, function(err, tx) { + should.exist(err); + err.code.should.equal('INSUFFICIENTFUNDS'); + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.99995, null, TestData.copayers[0].privKey_1H_0, 5000); + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + tx.fee.should.equal(5000); + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey); + // Sign it to make sure Bitcore doesn't complain about the fees + server.signTx({ + txProposalId: tx.id, + signatures: signatures, + }, function(err) { + should.not.exist(err); + done(); + }); + }); + }); + }); + }); + it('should fail to create tx for dust amount', function(done) { helpers.stubUtxos(server, wallet, [1], function() { var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.00000001, null, TestData.copayers[0].privKey_1H_0); @@ -1445,7 +1471,7 @@ describe('Wallet service', function() { it('should fail to create tx that would return change for dust amount', function(done) { helpers.stubUtxos(server, wallet, [1], function() { - var fee = Bitcore.Transaction.FEE_PER_KB / 1e8; + var fee = 10000 / 1e8; var change = 0.00000001; var amount = 1 - fee - change; From 0e0da9974169320d2608fffd1aad1b59b495fc8a Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 11 Jun 2015 16:35:33 -0300 Subject: [PATCH 4/7] v0.0.34 --- package.json | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index ebb42bd..2cda6b0 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "bitcore-wallet-service", "description": "A service for Mutisig HD Bitcoin Wallets", "author": "BitPay Inc", - "version": "0.0.33", + "version": "0.0.34", "keywords": [ "bitcoin", "copay", @@ -19,8 +19,9 @@ }, "dependencies": { "async": "^0.9.0", - "bitcore": "^0.11.6", - "bitcore-wallet-utils": "0.0.13", + "bitcore": "git://github.com/bitpay/bitcore.git#a4ac3f50d300b3f89fad02f9e38fc536ac90abdc", + "bitcore-explorers": "^0.10.3", + "bitcore-wallet-utils": "0.0.14", "body-parser": "^1.11.0", "coveralls": "^2.11.2", "email-validator": "^1.0.1", @@ -61,14 +62,11 @@ "test": "./node_modules/.bin/mocha", "coveralls": "./node_modules/.bin/istanbul cover ./node_modules/mocha/bin/_mocha --report lcovonly -- -R spec && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage" }, - "contributors": [ - { - "name": "Ivan Socolsky", - "email": "ivan@bitpay.com" - }, - { - "name": "Matias Alejo Garcia", - "email": "ematiu@gmail.com" - } - ] + "contributors": [{ + "name": "Ivan Socolsky", + "email": "ivan@bitpay.com" + }, { + "name": "Matias Alejo Garcia", + "email": "ematiu@gmail.com" + }] } From e349a10a67c36228c80e11a955b47ae8361cacfc Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 15 Jun 2015 09:45:43 -0300 Subject: [PATCH 5/7] simplify testing --- lib/server.js | 2 +- test/integration/server.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/server.js b/lib/server.js index bdc2db8..b805aab 100644 --- a/lib/server.js +++ b/lib/server.js @@ -797,7 +797,7 @@ WalletService.prototype.createTx = function(opts, cb) { return cb(new ClientError('Required argument missing')); var feePerKb = opts.feePerKb || 10000; - if (!_.contains([1000, 5000, 10000], feePerKb)) + if (feePerKb > 10000) return cb(new ClientError('Invalid fee per KB value')); self._runLocked(cb, function(cb) { diff --git a/test/integration/server.js b/test/integration/server.js index 005029f..ac54ffa 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -216,7 +216,7 @@ helpers.createAddresses = function(server, wallet, main, change, cb) { var storage, blockchainExplorer; -var useMongo = true; +var useMongo = false; function initStorage(cb) { function getDb(cb) { From e88997337976cdddc38afbf3416793fffbfa866c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 16 Jun 2015 18:05:12 -0300 Subject: [PATCH 6/7] BWU v0.0.15 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2cda6b0..e88810a 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "async": "^0.9.0", "bitcore": "git://github.com/bitpay/bitcore.git#a4ac3f50d300b3f89fad02f9e38fc536ac90abdc", "bitcore-explorers": "^0.10.3", - "bitcore-wallet-utils": "0.0.14", + "bitcore-wallet-utils": "0.0.15", "body-parser": "^1.11.0", "coveralls": "^2.11.2", "email-validator": "^1.0.1", From ba54b01797ba32b32f8e2718b485fe03cd6ba174 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 16 Jun 2015 18:05:26 -0300 Subject: [PATCH 7/7] remove safety check, log error --- lib/server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/server.js b/lib/server.js index b805aab..740107d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -728,10 +728,10 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { if (!bitcoreError) { txp.inputPaths = _.pluck(txp.inputs, 'path'); txp.fee = bitcoreTx.getFee(); - $.checkState(txp.fee < 1e8, 'Fees are too high!'); return cb(); } } catch (ex) { + log.error('Error building Bitcore transaction', ex); return cb(ex); } } @@ -797,7 +797,7 @@ WalletService.prototype.createTx = function(opts, cb) { return cb(new ClientError('Required argument missing')); var feePerKb = opts.feePerKb || 10000; - if (feePerKb > 10000) + if (feePerKb < WalletUtils.MIN_FEE_PER_KB || feePerKb > WalletUtils.MAX_FEE_PER_KB) return cb(new ClientError('Invalid fee per KB value')); self._runLocked(cb, function(cb) {