From 344fc525e0952be4621aad5777088c18844f3edb Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 27 Jul 2015 08:49:54 -0300 Subject: [PATCH 1/4] add fee estimation method to txp --- lib/model/txproposal.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 1e7d776..15812df 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -169,6 +169,25 @@ TxProposal.prototype.getRawTx = function() { return t.uncheckedSerialize(); }; +TxProposal.prototype.estimateFee = function(walletN) { + // Note: found empirically based on all multisig P2SH inputs and within m & n allowed limits. + var safetyMargin = 0.05; + + var walletM = this.requiredSignatures; + + var overhead = 4 + 4 + 9 + 9; + var inputSize = walletM * 72 + walletN * 36 + 44; + var outputSize = 34; + var nbInputs = this.inputs.length; + var nbOutputs = (_.isArray(this.outputs) ? this.outputs.length : 1) + 1; + + var size = overhead + inputSize * nbInputs + outputSize * nbOutputs; + + var fee = this.feePerKb * (size * (1 + safetyMargin)) / 1000; + + // Round up to nearest bit + this.fee = parseInt((Math.ceil(fee / 100) * 100).toFixed(0)); +}; /** * getTotalAmount From 5b3671b0799f7fed22b755f283efef23be73c61c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 27 Jul 2015 09:00:37 -0300 Subject: [PATCH 2/4] fix tests --- lib/model/txproposal.js | 6 ++++-- lib/server.js | 9 ++++++--- test/integration/server.js | 33 +++++++++++++++++++-------------- test/models/txproposal.js | 2 ++ 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 15812df..3209246 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -64,6 +64,7 @@ TxProposal.create = function(opts) { x.inputPaths = []; x.requiredSignatures = opts.requiredSignatures; x.requiredRejections = opts.requiredRejections; + x.walletN = opts.walletN; x.status = 'pending'; x.actions = []; x.fee = null; @@ -100,6 +101,7 @@ TxProposal.fromObj = function(obj) { x.inputs = obj.inputs; x.requiredSignatures = obj.requiredSignatures; x.requiredRejections = obj.requiredRejections; + x.walletN = obj.walletN; x.status = obj.status; x.txid = obj.txid; x.broadcastedOn = obj.broadcastedOn; @@ -169,14 +171,14 @@ TxProposal.prototype.getRawTx = function() { return t.uncheckedSerialize(); }; -TxProposal.prototype.estimateFee = function(walletN) { +TxProposal.prototype.estimateFee = function() { // Note: found empirically based on all multisig P2SH inputs and within m & n allowed limits. var safetyMargin = 0.05; var walletM = this.requiredSignatures; var overhead = 4 + 4 + 9 + 9; - var inputSize = walletM * 72 + walletN * 36 + 44; + var inputSize = walletM * 72 + this.walletN * 36 + 44; var outputSize = 34; var nbInputs = this.inputs.length; var nbOutputs = (_.isArray(this.outputs) ? this.outputs.length : 1) + 1; diff --git a/lib/server.js b/lib/server.js index 49dd387..6672769 100644 --- a/lib/server.js +++ b/lib/server.js @@ -796,7 +796,7 @@ WalletService.prototype.getFeeLevels = function(opts, cb) { var samplePoints = _.uniq(_.pluck(levels, 'nbBlocks')); self._sampleFeeLevels(network, samplePoints, function(err, feeSamples) { var values = _.map(levels, function(level) { - var result = { + var result = { level: level.name, }; if (err || feeSamples[level.nbBlocks] < 0) { @@ -875,9 +875,12 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { if (total >= txp.getTotalAmount()) { try { txp.setInputs(selected); + txp.estimateFee(); bitcoreTx = txp.getBitcoreTx(); bitcoreError = bitcoreTx.getSerializationError({ disableIsFullySigned: true, + disableSmallFees: true, + disableLargeFees: true, }); if (!bitcoreError) { txp.fee = bitcoreTx.getFee(); @@ -901,7 +904,6 @@ WalletService.prototype._selectTxInputs = function(txp, cb) { }); }; - WalletService.prototype._canCreateTx = function(copayerId, cb) { var self = this; self.storage.fetchLastTxs(self.walletId, copayerId, 5 + WalletService.backoffOffset, function(err, txs) { @@ -972,7 +974,7 @@ WalletService.prototype.createTx = function(opts, cb) { valid: false })) return; - var feePerKb = opts.feePerKb || 10000; + var feePerKb = opts.feePerKb || WalletUtils.DEFAULT_FEE_PER_KB; if (feePerKb < WalletUtils.MIN_FEE_PER_KB || feePerKb > WalletUtils.MAX_FEE_PER_KB) return cb(new ClientError('Invalid fee per KB value')); @@ -1047,6 +1049,7 @@ WalletService.prototype.createTx = function(opts, cb) { payProUrl: opts.payProUrl, requiredSignatures: wallet.m, requiredRejections: Math.min(wallet.m, wallet.n - wallet.m + 1), + walletN: wallet.n, excludeUnconfirmedUtxos: !!opts.excludeUnconfirmedUtxos, }); diff --git a/test/integration/server.js b/test/integration/server.js index 9109053..6162f2d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1516,10 +1516,10 @@ describe('Wallet service', function() { })); fees.priority.feePerKB.should.equal(50000); should.not.exist(fees.priority.nbBlocks); - + fees.normal.feePerKB.should.equal(18000); fees.normal.nbBlocks.should.equal(4); - + fees.economy.feePerKB.should.equal(0); fees.economy.nbBlocks.should.equal(12); done(); @@ -1613,7 +1613,8 @@ describe('Wallet service', function() { tx.isAccepted().should.equal.false; tx.isRejected().should.equal.false; tx.amount.should.equal(helpers.toSatoshi(80)); - tx.fee.should.equal(Bitcore.Transaction.FEE_PER_KB); + var estimatedFee = WalletUtils.DEFAULT_FEE_PER_KB * 400 / 1000; // fully signed tx should have about 400 bytes + tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); server.getPendingTxs({}, function(err, txs) { should.not.exist(err); txs.length.should.equal(1); @@ -1836,8 +1837,8 @@ describe('Wallet service', function() { var txOpts = helpers.createSimpleProposalOpts('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); + var estimatedFee = WalletUtils.DEFAULT_FEE_PER_KB * 1300 / 1000; // fully signed tx should have about 1300 bytes + tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); done(); }); }); @@ -1845,16 +1846,18 @@ describe('Wallet service', function() { it('should be possible to use a smaller fee', function(done) { helpers.stubUtxos(server, wallet, 1, function() { - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.99995, null, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 0.99995, null, TestData.copayers[0].privKey_1H_0, 80000); server.createTx(txOpts, function(err, tx) { should.exist(err); err.code.should.equal('INSUFFICIENTFUNDS'); var txOpts = helpers.createSimpleProposalOpts('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); + var estimatedFee = 5000 * 400 / 1000; // fully signed tx should have about 400 bytes + tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); + // Sign it to make sure Bitcore doesn't complain about the fees + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: tx.id, signatures: signatures, @@ -1881,11 +1884,11 @@ 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 = 10000 / 1e8; - var change = 0.00000001; + var fee = 4095 / 1e8; // The exact fee of the resulting tx + var change = 100 / 1e8; // Below dust var amount = 1 - fee - change; - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount, null, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount, null, TestData.copayers[0].privKey_1H_0, 10000); server.createTx(txOpts, function(err, tx) { should.exist(err); err.code.should.equal('DUSTAMOUNT'); @@ -1918,7 +1921,7 @@ describe('Wallet service', function() { it('should create tx with 0 change output', function(done) { helpers.stubUtxos(server, wallet, [1], function() { - var fee = Bitcore.Transaction.FEE_PER_KB / 1e8; + var fee = 4100 / 1e8; // The exact fee of the resulting tx var amount = 1 - fee; var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount, null, TestData.copayers[0].privKey_1H_0); @@ -2097,7 +2100,8 @@ describe('Wallet service', function() { should.not.exist(err); should.exist(tx); tx.amount.should.equal(max); - tx.fee.should.equal(3 * 10000); + var estimatedFee = 2900 * 10000 / 1000; // estimated size = 2900 bytes + tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); server.getBalance({}, function(err, balance) { should.not.exist(err); balance.lockedAmount.should.equal(helpers.toSatoshi(9)); @@ -2124,7 +2128,8 @@ describe('Wallet service', function() { should.not.exist(err); should.exist(tx); tx.amount.should.equal(max); - tx.fee.should.equal(2 * 2000); + var estimatedFee = 1650 * 2000 / 1000; // estimated size = 1650 bytes + tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); server.getBalance({}, function(err, balance) { should.not.exist(err); balance.lockedAmount.should.equal(helpers.toSatoshi(9)); diff --git a/test/models/txproposal.js b/test/models/txproposal.js index 4d46a97..9de4a3a 100644 --- a/test/models/txproposal.js +++ b/test/models/txproposal.js @@ -197,9 +197,11 @@ var aTXP = function(type) { "inputPaths": ["m/2147483647/0/1"], "requiredSignatures": 2, "requiredRejections": 1, + "walletN": 2, "status": "pending", "actions": [], "outputOrder": [0, 1], + "fee": 10000, }; if (type == TxProposal.Types.MULTIPLEOUTPUTS) { txp.outputs = [{ From 26aef25be69b078a88a1fb2a067938327b3b65ff Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 27 Jul 2015 12:19:27 -0300 Subject: [PATCH 3/4] compute total bytes to send max --- lib/model/txproposal.js | 11 ++++++++--- lib/server.js | 27 +++++++++++++-------------- test/integration/server.js | 25 +++++++++++++++---------- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 3209246..5a05b00 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -171,10 +171,9 @@ TxProposal.prototype.getRawTx = function() { return t.uncheckedSerialize(); }; -TxProposal.prototype.estimateFee = function() { +TxProposal.prototype.getEstimatedSize = function() { // Note: found empirically based on all multisig P2SH inputs and within m & n allowed limits. var safetyMargin = 0.05; - var walletM = this.requiredSignatures; var overhead = 4 + 4 + 9 + 9; @@ -185,7 +184,13 @@ TxProposal.prototype.estimateFee = function() { var size = overhead + inputSize * nbInputs + outputSize * nbOutputs; - var fee = this.feePerKb * (size * (1 + safetyMargin)) / 1000; + return parseInt((size * (1 + safetyMargin)).toFixed(0)); +}; + +TxProposal.prototype.estimateFee = function() { + + var size = this.getEstimatedSize(); + var fee = this.feePerKb * size / 1000; // Round up to nearest bit this.fee = parseInt((Math.ceil(fee / 100) * 100).toFixed(0)); diff --git a/lib/server.js b/lib/server.js index 6672769..e4218bf 100644 --- a/lib/server.js +++ b/lib/server.js @@ -692,7 +692,7 @@ WalletService.prototype._totalizeUtxos = function(utxos) { }; -WalletService.prototype._computeKbToSendMax = function(utxos, amount, cb) { +WalletService.prototype._computeBytesToSendMax = function(utxos, cb) { var self = this; var unlockedUtxos = _.reject(utxos, 'locked'); @@ -701,17 +701,16 @@ WalletService.prototype._computeKbToSendMax = function(utxos, amount, cb) { self.getWallet({}, function(err, wallet) { if (err) return cb(err); - var t = WalletUtils.newBitcoreTransaction(); - try { - _.each(unlockedUtxos, function(i) { - t.from(i, i.publicKeys, wallet.m); - }); - t.to(utxos[0].address, amount); - var sizeInKb = Math.ceil(t._estimateSize() / 1000); - return cb(null, sizeInKb); - } catch (ex) { - return cb(ex); - } + var txp = Model.TxProposal.create({ + walletId: self.walletId, + requiredSignatures: wallet.m, + walletN: wallet.n, + }); + txp.inputs = unlockedUtxos; + + var size = txp.getEstimatedSize(); + + return cb(null, size); }); }; @@ -744,11 +743,11 @@ WalletService.prototype.getBalance = function(opts, cb) { balance.byAddress = _.values(byAddress); - self._computeKbToSendMax(utxos, balance.availableAmount, function(err, sizeInKb) { + self._computeBytesToSendMax(utxos, function(err, sizeInBytes) { if (err) { log.error('Could not compute fees needed to transfer max amount', err); } - balance.totalKbToSendMax = sizeInKb || 0; + balance.totalBytesToSendMax = sizeInBytes || 0; return cb(null, balance); }); }); diff --git a/test/integration/server.js b/test/integration/server.js index 6162f2d..6770ae0 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1370,7 +1370,7 @@ describe('Wallet service', function() { balance.totalAmount.should.equal(helpers.toSatoshi(6)); balance.lockedAmount.should.equal(0); balance.availableAmount.should.equal(helpers.toSatoshi(6)); - balance.totalKbToSendMax.should.equal(1); + balance.totalBytesToSendMax.should.equal(578); balance.totalConfirmedAmount.should.equal(helpers.toSatoshi(4)); balance.lockedConfirmedAmount.should.equal(0); @@ -1396,7 +1396,7 @@ describe('Wallet service', function() { balance.totalAmount.should.equal(0); balance.lockedAmount.should.equal(0); balance.availableAmount.should.equal(0); - balance.totalKbToSendMax.should.equal(0); + balance.totalBytesToSendMax.should.equal(0); should.exist(balance.byAddress); balance.byAddress.length.should.equal(0); done(); @@ -1412,7 +1412,7 @@ describe('Wallet service', function() { balance.totalAmount.should.equal(0); balance.lockedAmount.should.equal(0); balance.availableAmount.should.equal(0); - balance.totalKbToSendMax.should.equal(0); + balance.totalBytesToSendMax.should.equal(0); should.exist(balance.byAddress); balance.byAddress.length.should.equal(0); done(); @@ -1440,7 +1440,7 @@ describe('Wallet service', function() { should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(9)); balance.lockedAmount.should.equal(0); - balance.totalKbToSendMax.should.equal(2); + balance.totalBytesToSendMax.should.equal(1535); done(); }); }); @@ -2093,18 +2093,21 @@ describe('Wallet service', function() { balance.totalAmount.should.equal(helpers.toSatoshi(9)); balance.lockedAmount.should.equal(0); balance.availableAmount.should.equal(helpers.toSatoshi(9)); - balance.totalKbToSendMax.should.equal(3); - var max = (balance.totalAmount - balance.lockedAmount) - (balance.totalKbToSendMax * 10000); + balance.totalBytesToSendMax.should.equal(2896); + var sizeInKB = balance.totalBytesToSendMax / 1000; + var fee = parseInt((Math.ceil(sizeInKB * 10000 / 100) * 100).toFixed(0)); + var max = balance.availableAmount - fee; var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max / 1e8, null, TestData.copayers[0].privKey_1H_0); server.createTx(txOpts, function(err, tx) { should.not.exist(err); should.exist(tx); tx.amount.should.equal(max); - var estimatedFee = 2900 * 10000 / 1000; // estimated size = 2900 bytes + var estimatedFee = 2896 * 10000 / 1000; tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); server.getBalance({}, function(err, balance) { should.not.exist(err); balance.lockedAmount.should.equal(helpers.toSatoshi(9)); + balance.availableAmount.should.equal(0); done(); }); }); @@ -2121,14 +2124,16 @@ describe('Wallet service', function() { balance.totalAmount.should.equal(helpers.toSatoshi(9)); balance.lockedAmount.should.equal(helpers.toSatoshi(4)); balance.availableAmount.should.equal(helpers.toSatoshi(5)); - balance.totalKbToSendMax.should.equal(2); - var max = (balance.totalAmount - balance.lockedAmount) - (balance.totalKbToSendMax * 2000); + balance.totalBytesToSendMax.should.equal(1653); + var sizeInKB = balance.totalBytesToSendMax / 1000; + var fee = parseInt((Math.ceil(sizeInKB * 2000 / 100) * 100).toFixed(0)); + var max = balance.availableAmount - fee; var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max / 1e8, null, TestData.copayers[0].privKey_1H_0, 2000); server.createTx(txOpts, function(err, tx) { should.not.exist(err); should.exist(tx); tx.amount.should.equal(max); - var estimatedFee = 1650 * 2000 / 1000; // estimated size = 1650 bytes + var estimatedFee = 1653 * 2000 / 1000; tx.fee.should.be.within(0.9 * estimatedFee, 1.1 * estimatedFee); server.getBalance({}, function(err, balance) { should.not.exist(err); From 1a72bb9c18e0d123d1bd33c2948c79a85a4e2dcd Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 27 Jul 2015 12:38:12 -0300 Subject: [PATCH 4/4] BWU v0.0.23 --- lib/server.js | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/server.js b/lib/server.js index e4218bf..e79553b 100644 --- a/lib/server.js +++ b/lib/server.js @@ -743,11 +743,11 @@ WalletService.prototype.getBalance = function(opts, cb) { balance.byAddress = _.values(byAddress); - self._computeBytesToSendMax(utxos, function(err, sizeInBytes) { + self._computeBytesToSendMax(utxos, function(err, size) { if (err) { log.error('Could not compute fees needed to transfer max amount', err); } - balance.totalBytesToSendMax = sizeInBytes || 0; + balance.totalBytesToSendMax = size || 0; return cb(null, balance); }); }); diff --git a/package.json b/package.json index 4e837c3..00ae69c 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.46", + "version": "0.0.47", "keywords": [ "bitcoin", "copay", @@ -20,7 +20,7 @@ "dependencies": { "async": "^0.9.0", "bitcore": "^0.12.9", - "bitcore-wallet-utils": "^0.0.22", + "bitcore-wallet-utils": "^0.0.23", "body-parser": "^1.11.0", "coveralls": "^2.11.2", "email-validator": "^1.0.1",