diff --git a/lib/common/utils.js b/lib/common/utils.js index 72846bf..a6234df 100644 --- a/lib/common/utils.js +++ b/lib/common/utils.js @@ -23,8 +23,7 @@ Utils.checkRequired = function(obj, args) { * @return {number} */ Utils.strip = function(number) { - return parseFloat(number); - return (parseFloat(number.toPrecision(12))); + return parseFloat(number.toPrecision(12)); } /* TODO: It would be nice to be compatible with bitcoind signmessage. How @@ -67,6 +66,11 @@ Utils.formatAmount = function(satoshis, unit, opts) { maxDecimals: 0, minDecimals: 0, }, + sat: { + toSatoshis: 1, + maxDecimals: 0, + minDecimals: 0, + } }; $.shouldBeNumber(satoshis); @@ -89,13 +93,16 @@ Utils.formatAmount = function(satoshis, unit, opts) { opts = opts || {}; - var u = UNITS[unit]; + var u = _.assign(UNITS[unit], opts); var amount = (satoshis / u.toSatoshis).toFixed(u.maxDecimals); return addSeparators(amount, opts.thousandsSeparator || ',', opts.decimalSeparator || '.', u.minDecimals); }; Utils.formatAmountInBtc = function(amount) { - return Utils.formatAmount(amount, 'btc') + 'btc'; + return Utils.formatAmount(amount, 'btc', { + minDecimals: 8, + maxDecimals: 8, + }) + 'btc'; }; Utils.formatUtxos = function(utxos) { diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 1328810..67a3707 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -158,7 +158,13 @@ TxProposal.prototype._buildTx = function() { }); t.fee(self.fee); - t.change(self.changeAddress.address); + + var totalInputs = _.sum(self.inputs, 'satoshis'); + var totalOutputs = _.sum(self.outputs, 'satoshis'); + + if (totalInputs - totalOutputs - self.fee > 0) { + t.change(self.changeAddress.address); + } // Shuffle outputs for improved privacy if (t.outputs.length > 1) { @@ -173,8 +179,8 @@ TxProposal.prototype._buildTx = function() { }); } - // Validate inputs vs outputs independently of Bitcore - var totalInputs = _.sum(self.inputs, 'satoshis'); + // Validate actual inputs vs outputs independently of Bitcore + var totalInputs = _.sum(t.inputs, 'satoshis'); var totalOutputs = _.sum(t.outputs, 'satoshis'); $.checkState(totalInputs - totalOutputs <= Defaults.MAX_TX_FEE); diff --git a/lib/server.js b/lib/server.js index bfdae8c..aaa0cfd 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1230,7 +1230,11 @@ WalletService.prototype.getFeeLevels = function(opts, cb) { }); }; -WalletService.prototype._checkTxAndEstimateFee = function(txp) { +WalletService.prototype._estimateFee = function(txp) { + txp.estimateFee(); +}; + +WalletService.prototype._checkTx = function(txp) { var bitcoreError; var serializationOpts = { @@ -1241,8 +1245,6 @@ WalletService.prototype._checkTxAndEstimateFee = function(txp) { serializationOpts.disableLargeFees = true; } - txp.estimateFee(); - if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) return Errors.TX_MAX_SIZE_EXCEEDED; @@ -1270,7 +1272,8 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { //todo: check inputs are ours and has enough value if (txp.inputs && txp.inputs.length > 0) { - return cb(self._checkTxAndEstimateFee(txp)); + self._estimateFee(txp); + return cb(self._checkTx(txp)); } var txpAmount = txp.getTotalAmount(); @@ -1331,7 +1334,9 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { log.debug('Considering ' + smallInputs.length + ' small inputs (' + Utils.formatUtxos(smallInputs) + ')'); var total = 0; + var netTotal = 0; var selected = []; + var fee; var error; _.each(smallInputs, function(input, i) { @@ -1342,20 +1347,23 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { return false; } - var inputAmount = input.satoshis - feePerInput; - log.debug('The input contributes ' + Utils.formatAmountInBtc(inputAmount)); + var netInputAmount = input.satoshis - feePerInput; + + log.debug('The input contributes ' + Utils.formatAmountInBtc(netInputAmount)); selected.push(input); - total += inputAmount; + total += input.satoshis; + netTotal += netInputAmount; + var txpSize = baseTxpSize + selected.length * sizePerInput; - var txpFee = baseTxpFee + selected.length * feePerInput; + fee = Math.round(baseTxpFee + selected.length * feePerInput); - log.debug('Tx size: ' + Utils.formatSize(txpSize) + ', Tx fee: ' + Utils.formatAmountInBtc(txpFee)); + log.debug('Tx size: ' + Utils.formatSize(txpSize) + ', Tx fee: ' + Utils.formatAmountInBtc(fee)); - var feeVsAmountRatio = txpFee / txpAmount; - var feeVsSingleInputFeeRatio = txpFee / (baseTxpFee + feePerInput); - var amountVsUtxoRatio = inputAmount / txpAmount; + var feeVsAmountRatio = fee / txpAmount; + var feeVsSingleInputFeeRatio = fee / (baseTxpFee + feePerInput); + var amountVsUtxoRatio = netInputAmount / txpAmount; log.debug('Tx amount/Fee: ' + Utils.formatRatio(feeVsAmountRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR) + ')'); log.debug('Single-input fee/Multi-input fee: ' + Utils.formatRatio(feeVsSingleInputFeeRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) + ')' + ' loses wrt single-input tx: ' + Utils.formatAmountInBtc((selected.length - 1) * feePerInput)); @@ -1380,29 +1388,34 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { } } - log.debug('Cumuled total so far: ' + Utils.formatAmountInBtc(total)); + log.debug('Cumuled total so far: ' + Utils.formatAmountInBtc(total) + ', Net total so far: ' + Utils.formatAmountInBtc(netTotal)); + + if (netTotal >= txpAmount) { + var changeAmount = Math.round(total - txpAmount - fee); - if (total >= txpAmount) { - var changeAmount = total - txpAmount - txpFee; log.debug('Tx change: ', Utils.formatAmountInBtc(changeAmount)); - if (changeAmount <= Bitcore.Transaction.DUST_AMOUNT) { - log.debug('Change (' + Utils.formatAmountInBtc(changeAmount) + ') below dust amount (' + Utils.formatAmountInBtc(Bitcore.Transaction.DUST_AMOUNT) + ')'); - return; + if (changeAmount != 0 && Math.abs(changeAmount) <= Bitcore.Transaction.DUST_AMOUNT) { + log.debug('ABS(Change) (' + Utils.formatAmountInBtc(changeAmount) + ') below dust amount (' + Utils.formatAmountInBtc(Bitcore.Transaction.DUST_AMOUNT) + ')'); + if (changeAmount > 0 && fee < changeAmount) return; + // Either increment or decrement fee to remove change + fee += changeAmount; } return false; } }); - if (total < txpAmount) { - log.debug('Could not reach Txp total (' + Utils.formatAmountInBtc(txpAmount) + '), still missing: ' + Utils.formatAmountInBtc(txpAmount - total)); + if (netTotal < txpAmount) { + log.debug('Could not reach Txp total (' + Utils.formatAmountInBtc(txpAmount) + '), still missing: ' + Utils.formatAmountInBtc(txpAmount - netTotal)); selected = []; if (!_.isEmpty(bigInputs)) { - log.debug('Using big input: ', Utils.formatUtxos(_.first(bigInputs))); var input = _.first(bigInputs); + log.debug('Using big input: ', Utils.formatUtxos(input)); total = input.satoshis; + fee = Math.round(baseTxpFee + feePerInput); + netTotal = total - fee; selected = [input]; } } @@ -1412,7 +1425,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { return cb(error || Errors.INSUFFICIENT_FUNDS_FOR_FEE); } - return cb(null, selected); + return cb(null, selected, fee); }; log.debug('Selecting inputs for a ' + Utils.formatAmountInBtc(txp.getTotalAmount()) + ' txp'); @@ -1443,6 +1456,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { if (!txp.excludeUnconfirmedUtxos) groups.push(0); var inputs = []; + var fee; var selectionError; var i = 0; var lastGroupLength; @@ -1467,7 +1481,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { lastGroupLength = candidateUtxos.length; - select(candidateUtxos, function(err, selected) { + select(candidateUtxos, function(err, selectedInputs, selectedFee) { if (err) { log.debug('No inputs selected on this group: ', err); selectionError = err; @@ -1475,9 +1489,12 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { } selectionError = null; - inputs = selected; + inputs = selectedInputs; + fee = selectedFee; log.debug('Selected inputs from this group: ' + Utils.formatUtxos(inputs)); + log.debug('Fee for this selection: ' + Utils.formatAmountInBtc(fee)); + return next(); }); }, function(err) { @@ -1485,8 +1502,9 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { if (selectionError || _.isEmpty(inputs)) return cb(selectionError || new Error('Could not select tx inputs')); txp.setInputs(inputs); + txp.fee = fee; - var err = self._checkTxAndEstimateFee(txp); + var err = self._checkTx(txp); if (!err) { log.debug('Successfully built transaction. Total fees: ' + Utils.formatAmountInBtc(txp.fee) + ', total change: ' + Utils.formatAmountInBtc(_.sum(txp.inputs, 'satoshis') - txp.fee)); diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 4a573b2..88b9037 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -233,13 +233,13 @@ helpers._parseAmount = function(str) { switch (match[3]) { default: case 'btc': - result.amount = Utils.strip(match[2] * 1e8); + result.amount = Utils.strip(+match[2] * 1e8); break; case 'bit': - result.amount = Utils.strip(match[2] * 1e2); + result.amount = Utils.strip(+match[2] * 1e2); break case 'sat': - result.amount = Utils.strip(match[2]); + result.amount = Utils.strip(+match[2]); break; }; diff --git a/test/integration/server.js b/test/integration/server.js index de1d1df..5eac473 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2381,19 +2381,18 @@ describe('Wallet service', function() { }); }); - it('should fail to create tx that would return change for dust amount', function(done) { + it('should modify fee if tx would return change for dust amount', function(done) { helpers.stubUtxos(server, wallet, [1], function() { - var fee = 4095 / 1e8; // The exact fee of the resulting tx - var change = 100 / 1e8; // Below dust - var amount = 1 - fee - change; + var fee = 4095; // The exact fee of the resulting tx (based exclusively on feePerKB && size) + var change = 100; // Below dust + var amount = (1e8 - fee - change) / 1e8; var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount, TestData.copayers[0].privKey_1H_0, { feePerKb: 10000 }); server.createTxLegacy(txOpts, function(err, tx) { - should.exist(err); - err.code.should.equal('DUST_AMOUNT'); - err.message.should.equal('Amount below dust threshold'); + should.not.exist(err); + tx.fee.should.equal(fee + change); done(); }); }); @@ -3169,16 +3168,19 @@ describe('Wallet service', function() { }); }); - describe('UTXO Selection', function() { + describe.only('UTXO Selection', function() { var server, wallet; beforeEach(function(done) { - log.level = 'debug'; + // log.level = 'debug'; helpers.createAndJoinWallet(2, 3, function(s, w) { server = s; wallet = w; done(); }); }); + afterEach(function() { + log.level = 'info'; + }); it('should select a single utxo if within thresholds relative to tx amount', function(done) { helpers.stubUtxos(server, wallet, [1, '350bit', '100bit', '100bit', '100bit'], function() { @@ -3479,7 +3481,7 @@ describe('Wallet service', function() { }); }); }); - it('should keep adding utxos while change is below dust', function(done) { + it('should correct fee if resulting change would be below dust', function(done) { helpers.stubUtxos(server, wallet, ['200bit', '500sat'], function() { var txOpts = { outputs: [{ @@ -3489,22 +3491,14 @@ describe('Wallet service', function() { feePerKb: 400, }; server.createTx(txOpts, function(err, txp) { - should.exist(err); - err.code.should.equal('DUST_AMOUNT'); - helpers.stubUtxos(server, wallet, ['200bit'].concat(_.times(10, function() { - return '500sat'; - })), function() { - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - txp.inputs[0].satoshis.should.equal(200e2); - (_.sum(txp.inputs, 'satoshis') - txp.outputs[0].amount - txp.fee).should.be.above(Bitcore.Transaction.DUST_AMOUNT); - done(); - }); - }); + should.not.exist(err); + txp.inputs[0].satoshis.should.equal(200e2); + (_.sum(txp.inputs, 'satoshis') - txp.outputs[0].amount - txp.fee).should.equal(0); + done(); }); }); }); - it.skip('should ignore small utxos if fee is higher', function(done) { + it('should ignore small utxos if fee is higher', function(done) { helpers.stubUtxos(server, wallet, [].concat(_.times(10, function() { return '30bit'; })), function() { @@ -3513,9 +3507,10 @@ describe('Wallet service', function() { toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount: 200e2, }], - feePerKb: 30e2, + feePerKb: 50e2, }; server.createTx(txOpts, function(err, txp) { + should.exist(err); err.code.should.equal('INSUFFICIENT_FUNDS_FOR_FEE'); done(); });