From 37f87e47937f81fd962de43256fd17838206b20c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Mar 2016 10:25:12 -0300 Subject: [PATCH 1/4] accept feeLevel on createTx --- lib/model/txproposal.js | 2 ++ lib/server.js | 2 ++ test/integration/server.js | 2 ++ 3 files changed, 6 insertions(+) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index af4f764..a286927 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -46,6 +46,7 @@ TxProposal.create = function(opts) { x.actions = []; x.fee = opts.fee; x.feePerKb = opts.feePerKb; + x.feeLevel = opts.feeLevel; x.excludeUnconfirmedUtxos = opts.excludeUnconfirmedUtxos; x.addressType = opts.addressType || (x.walletN > 1 ? Constants.SCRIPT_TYPES.P2SH : Constants.SCRIPT_TYPES.P2PKH); @@ -95,6 +96,7 @@ TxProposal.fromObj = function(obj) { x.outputOrder = obj.outputOrder; x.fee = obj.fee; x.feePerKb = obj.feePerKb; + x.feeLevel = obj.feeLevel; x.excludeUnconfirmedUtxos = obj.excludeUnconfirmedUtxos; x.addressType = obj.addressType; x.customData = obj.customData; diff --git a/lib/server.js b/lib/server.js index c7c70a4..f651bba 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1546,6 +1546,7 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { * @param {Array} opts.inputs - Optional. Inputs for this TX * @param {string} opts.fee - Optional. Use an alternative fee for this TX (mutually exclusive with feePerKb) * @param {string} opts.feePerKb - Optional. Use an alternative fee per KB for this TX (mutually exclusive with fee) + * @param {string} opts.feeLevel - Optional. Specify the fee level used to compute feePerKb for this txp. * @param {string} opts.payProUrl - Optional. Paypro URL for peers to verify TX * @param {string} opts.excludeUnconfirmedUtxos[=false] - Optional. Do not use UTXOs of unconfirmed transactions as inputs * @param {string} opts.validateOutputs[=true] - Optional. Perform validation on outputs. @@ -1595,6 +1596,7 @@ WalletService.prototype.createTx = function(opts, cb) { changeAddress: wallet.createAddress(true), fee: opts.fee, feePerKb: opts.feePerKb, + feeLevel: opts.feeLevel, payProUrl: opts.payProUrl, walletM: wallet.m, walletN: wallet.n, diff --git a/test/integration/server.js b/test/integration/server.js index 7aa5166..58cd249 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2718,6 +2718,7 @@ describe('Wallet service', function() { }], message: 'some message', customData: 'some custom data', + feeLevel: 'priority', }; server.createTx(txOpts, function(err, tx) { should.not.exist(err); @@ -2731,6 +2732,7 @@ describe('Wallet service', function() { tx.isPending().should.equal.true; tx.isTemporary().should.equal.true; tx.amount.should.equal(helpers.toSatoshi(0.8)); + tx.feeLevel.should.equal('priority'); server.getPendingTxs({}, function(err, txs) { should.not.exist(err); txs.should.be.empty; From 446f35426fd5c0756fe54193b22a23f2e54fbdbf Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Mar 2016 10:30:05 -0300 Subject: [PATCH 2/4] remove explicit final fee for tx creation --- lib/model/txproposal.js | 2 +- lib/server.js | 21 +++------------ test/integration/server.js | 55 -------------------------------------- 3 files changed, 5 insertions(+), 73 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index a286927..1ae7538 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -44,7 +44,7 @@ TxProposal.create = function(opts) { x.requiredRejections = Math.min(x.walletM, x.walletN - x.walletM + 1), x.status = 'temporary'; x.actions = []; - x.fee = opts.fee; + x.fee = null; x.feePerKb = opts.feePerKb; x.feeLevel = opts.feeLevel; x.excludeUnconfirmedUtxos = opts.excludeUnconfirmedUtxos; diff --git a/lib/server.js b/lib/server.js index f651bba..b72998e 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1223,9 +1223,7 @@ WalletService.prototype._checkTxAndEstimateFee = function(txp) { serializationOpts.disableLargeFees = true; } - if (_.isNumber(txp.feePerKb)) { - txp.estimateFee(); - } + txp.estimateFee(); try { var bitcoreTx = txp.getBitcoreTx(); @@ -1544,7 +1542,6 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { * @param {string} opts.outputs[].message - A message to attach to this output. * @param {string} opts.message - A message to attach to this transaction. * @param {Array} opts.inputs - Optional. Inputs for this TX - * @param {string} opts.fee - Optional. Use an alternative fee for this TX (mutually exclusive with feePerKb) * @param {string} opts.feePerKb - Optional. Use an alternative fee per KB for this TX (mutually exclusive with fee) * @param {string} opts.feeLevel - Optional. Specify the fee level used to compute feePerKb for this txp. * @param {string} opts.payProUrl - Optional. Paypro URL for peers to verify TX @@ -1558,18 +1555,9 @@ WalletService.prototype.createTx = function(opts, cb) { if (!Utils.checkRequired(opts, ['outputs'])) return cb(new ClientError('Required argument missing')); - if (_.isNumber(opts.fee)) { - if (_.isNumber(opts.feePerKb)) - return cb(new ClientError('Cannot sepcify both fee and feePerKb arguments')); - opts.feePerKb = null; - if (opts.fee < Defaults.MIN_TX_FEE || opts.fee > Defaults.MAX_TX_FEE) - return cb(new ClientError('Invalid fee')); - } else { - opts.fee = null; - opts.feePerKb = opts.feePerKb || Defaults.DEFAULT_FEE_PER_KB; - if (opts.feePerKb < Defaults.MIN_FEE_PER_KB || opts.feePerKb > Defaults.MAX_FEE_PER_KB) - return cb(new ClientError('Invalid fee per KB')); - } + opts.feePerKb = opts.feePerKb || Defaults.DEFAULT_FEE_PER_KB; + if (opts.feePerKb < Defaults.MIN_FEE_PER_KB || opts.feePerKb > Defaults.MAX_FEE_PER_KB) + return cb(new ClientError('Invalid fee per KB')); self._runLocked(cb, function(cb) { self.getWallet({}, function(err, wallet) { @@ -1594,7 +1582,6 @@ WalletService.prototype.createTx = function(opts, cb) { outputs: opts.outputs, message: opts.message, changeAddress: wallet.createAddress(true), - fee: opts.fee, feePerKb: opts.feePerKb, feeLevel: opts.feeLevel, payProUrl: opts.payProUrl, diff --git a/test/integration/server.js b/test/integration/server.js index 58cd249..5bca205 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2742,61 +2742,6 @@ describe('Wallet service', function() { }); }); - it('should be able to specify the final fee', function(done) { - helpers.stubUtxos(server, wallet, [1, 2], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 0.8 * 1e8, - }], - fee: 123400, - }; - server.createTx(txOpts, function(err, tx) { - should.not.exist(err); - should.exist(tx); - tx.fee.should.equal(123400); - var t = tx.getBitcoreTx(); - t.getFee().should.equal(123400); - done(); - }); - }); - }); - - it('should not be able to specify both final fee & fee per kb', function(done) { - helpers.stubUtxos(server, wallet, [1, 2], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 0.8 * 1e8, - }], - fee: 123400, - feePerKb: 123400, - }; - server.createTx(txOpts, function(err, tx) { - should.exist(err); - err.message.should.contain('fee'); - done(); - }); - }); - }); - - it('should check explicit fee to be below max', function(done) { - helpers.stubUtxos(server, wallet, [1, 2], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 0.8 * 1e8, - }], - fee: 1e8, - }; - server.createTx(txOpts, function(err, tx) { - should.exist(err); - err.message.should.contain('fee'); - done(); - }); - }); - }); - it('should be able to publish a temporary tx proposal', function(done) { helpers.stubUtxos(server, wallet, [1, 2], function() { var txOpts = { From 682bf7fdd24140aa988f19785279d970b651fb18 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Mar 2016 11:06:16 -0300 Subject: [PATCH 3/4] make feePerKb required --- lib/common/defaults.js | 3 ++- lib/model/txproposal_legacy.js | 2 +- lib/server.js | 5 ++-- test/integration/server.js | 44 +++++++++++++++++++++++++++++----- 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/lib/common/defaults.js b/lib/common/defaults.js index d21c079..362363b 100644 --- a/lib/common/defaults.js +++ b/lib/common/defaults.js @@ -2,7 +2,6 @@ var Defaults = {}; -Defaults.DEFAULT_FEE_PER_KB = 10000; Defaults.MIN_FEE_PER_KB = 0; Defaults.MAX_FEE_PER_KB = 1000000; Defaults.MIN_TX_FEE = 0; @@ -39,6 +38,8 @@ Defaults.FEE_LEVELS = [{ defaultValue: 25000 }]; +Defaults.DEFAULT_FEE_PER_KB = Defaults.FEE_LEVELS[1].defaultValue; + // Minimum nb of addresses a wallet must have to start using 2-step balance optimization Defaults.TWO_STEP_BALANCE_THRESHOLD = 100; diff --git a/lib/model/txproposal_legacy.js b/lib/model/txproposal_legacy.js index 09952ec..a2af008 100644 --- a/lib/model/txproposal_legacy.js +++ b/lib/model/txproposal_legacy.js @@ -133,8 +133,8 @@ TxProposal.fromObj = function(obj) { return TxProposalAction.fromObj(action); }); x.outputOrder = obj.outputOrder; - x.fee = obj.fee; x.network = obj.network; + x.fee = obj.fee; x.feePerKb = obj.feePerKb; x.excludeUnconfirmedUtxos = obj.excludeUnconfirmedUtxos; x.proposalSignaturePubKey = obj.proposalSignaturePubKey; diff --git a/lib/server.js b/lib/server.js index b72998e..d91f8d1 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1542,7 +1542,7 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { * @param {string} opts.outputs[].message - A message to attach to this output. * @param {string} opts.message - A message to attach to this transaction. * @param {Array} opts.inputs - Optional. Inputs for this TX - * @param {string} opts.feePerKb - Optional. Use an alternative fee per KB for this TX (mutually exclusive with fee) + * @param {string} opts.feePerKb - The fee per kB to use for this TX. * @param {string} opts.feeLevel - Optional. Specify the fee level used to compute feePerKb for this txp. * @param {string} opts.payProUrl - Optional. Paypro URL for peers to verify TX * @param {string} opts.excludeUnconfirmedUtxos[=false] - Optional. Do not use UTXOs of unconfirmed transactions as inputs @@ -1552,10 +1552,9 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { WalletService.prototype.createTx = function(opts, cb) { var self = this; - if (!Utils.checkRequired(opts, ['outputs'])) + if (!Utils.checkRequired(opts, ['outputs', 'feePerKb'])) return cb(new ClientError('Required argument missing')); - opts.feePerKb = opts.feePerKb || Defaults.DEFAULT_FEE_PER_KB; if (opts.feePerKb < Defaults.MIN_FEE_PER_KB || opts.feePerKb > Defaults.MAX_FEE_PER_KB) return cb(new ClientError('Invalid fee per KB')); diff --git a/test/integration/server.js b/test/integration/server.js index 5bca205..15d2f57 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2038,10 +2038,26 @@ describe('Wallet service', function() { }); }); + it('should assume default feePerKb for "normal" level when none is specified', function(done) { + helpers.stubUtxos(server, wallet, [100, 200], function() { + var txOpts = helpers.createProposalOptsLegacy('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 80, 'some message', TestData.copayers[0].privKey_1H_0); + server.createTxLegacy(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.feePerKb.should.equal(_.find(Defaults.FEE_LEVELS, { + name: 'normal' + }).defaultValue); + done(); + }); + }); + }); + it('should support creating a tx with no change address', function(done) { helpers.stubUtxos(server, wallet, [1, 2], function() { var max = 3 - (7200 / 1e8); // Fees for this tx at 100bits/kB = 7200 sat - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max, TestData.copayers[0].privKey_1H_0, { + feePerKb: 100e2 + }); server.createTxLegacy(txOpts, function(err, txp) { should.not.exist(err); should.exist(txp); @@ -2360,7 +2376,9 @@ describe('Wallet service', function() { var fee = 4100 / 1e8; // The exact fee of the resulting tx var amount = 1 - fee; - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount, TestData.copayers[0].privKey_1H_0, { + feePerKb: 100e2 + }); server.createTxLegacy(txOpts, function(err, tx) { should.not.exist(err); should.exist(tx); @@ -2507,7 +2525,8 @@ describe('Wallet service', function() { message: 'message #2' }]; var txOpts = helpers.createProposalOpts(Model.TxProposalLegacy.Types.MULTIPLEOUTPUTS, outputs, TestData.copayers[0].privKey_1H_0, { - message: 'some message' + message: 'some message', + feePerKb: 100e2, }); server.createTxLegacy(txOpts, function(err, txp) { should.not.exist(err); @@ -2587,7 +2606,9 @@ describe('Wallet service', function() { balance.totalBytesToSendConfirmedMax.should.equal(2896); var fee = parseInt((balance.totalBytesToSendMax * 10000 / 1000).toFixed(0)); var max = balance.availableAmount - fee; - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max / 1e8, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max / 1e8, TestData.copayers[0].privKey_1H_0, { + feePerKb: 100e2, + }); server.createTxLegacy(txOpts, function(err, tx) { should.not.exist(err); should.exist(tx); @@ -2652,7 +2673,9 @@ describe('Wallet service', function() { balance.totalBytesToSendConfirmedMax.should.equal(720); var fee = parseInt((balance.totalBytesToSendConfirmedMax * 10000 / 1000).toFixed(0)); var max = balance.availableConfirmedAmount - fee; - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max / 1e8, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', max / 1e8, TestData.copayers[0].privKey_1H_0, { + feePerKb: 100e2, + }); server.createTxLegacy(txOpts, function(err, tx) { should.not.exist(err); should.exist(tx); @@ -2718,6 +2741,7 @@ describe('Wallet service', function() { }], message: 'some message', customData: 'some custom data', + feePerKb: 123e2, feeLevel: 'priority', }; server.createTx(txOpts, function(err, tx) { @@ -2732,7 +2756,7 @@ describe('Wallet service', function() { tx.isPending().should.equal.true; tx.isTemporary().should.equal.true; tx.amount.should.equal(helpers.toSatoshi(0.8)); - tx.feeLevel.should.equal('priority'); + tx.feePerKb.should.equal(123e2); server.getPendingTxs({}, function(err, txs) { should.not.exist(err); txs.should.be.empty; @@ -2749,6 +2773,7 @@ describe('Wallet service', function() { toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount: 0.8 * 1e8, }], + feePerKb: 100e2, message: 'some message', customData: 'some custom data', }; @@ -2776,6 +2801,7 @@ describe('Wallet service', function() { toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount: 0.8 * 1e8, }], + feePerKb: 100e2, message: 'some message', }; server.createTx(txOpts, function(err, txp) { @@ -2819,6 +2845,7 @@ describe('Wallet service', function() { toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount: 0.8 * 1e8, }], + feePerKb: 100e2, message: 'some message', }; server.createTx(txOpts, function(err, txp) { @@ -2843,6 +2870,7 @@ describe('Wallet service', function() { toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount: 0.8 * 1e8, }], + feePerKb: 100e2, message: 'some message', }; server.createTx(txOpts, function(err, txp) { @@ -2885,6 +2913,7 @@ describe('Wallet service', function() { amount: 0.8 * 1e8, }], message: 'some message', + feePerKb: 100e2, }; server.createTx(txOpts, function(err, txp) { should.not.exist(err); @@ -2920,6 +2949,7 @@ describe('Wallet service', function() { amount: 0.8 * 1e8, }], message: 'some message', + feePerKb: 100e2, }; async.waterfall([ @@ -2989,6 +3019,7 @@ describe('Wallet service', function() { }], message: 'some message', customData: 'some custom data', + feePerKb: 100e2, }; server.createTx(txOpts, function(err, txp) { should.not.exist(err); @@ -3655,6 +3686,7 @@ describe('Wallet service', function() { amount: 9e8, }], message: 'some message', + feePerKb: 100e2, }; helpers.createAndPublishTx(server, txOpts, TestData.copayers[0].privKey_1H_0, function(txp) { should.exist(txp); From f878594d408ac6d96b782ea3e94da132f56e8be0 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Mar 2016 11:07:17 -0300 Subject: [PATCH 4/4] rm feeLevel --- lib/model/txproposal.js | 2 -- lib/server.js | 2 -- test/integration/server.js | 1 - 3 files changed, 5 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 1ae7538..0498702 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -46,7 +46,6 @@ TxProposal.create = function(opts) { x.actions = []; x.fee = null; x.feePerKb = opts.feePerKb; - x.feeLevel = opts.feeLevel; x.excludeUnconfirmedUtxos = opts.excludeUnconfirmedUtxos; x.addressType = opts.addressType || (x.walletN > 1 ? Constants.SCRIPT_TYPES.P2SH : Constants.SCRIPT_TYPES.P2PKH); @@ -96,7 +95,6 @@ TxProposal.fromObj = function(obj) { x.outputOrder = obj.outputOrder; x.fee = obj.fee; x.feePerKb = obj.feePerKb; - x.feeLevel = obj.feeLevel; x.excludeUnconfirmedUtxos = obj.excludeUnconfirmedUtxos; x.addressType = obj.addressType; x.customData = obj.customData; diff --git a/lib/server.js b/lib/server.js index d91f8d1..9300722 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1543,7 +1543,6 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { * @param {string} opts.message - A message to attach to this transaction. * @param {Array} opts.inputs - Optional. Inputs for this TX * @param {string} opts.feePerKb - The fee per kB to use for this TX. - * @param {string} opts.feeLevel - Optional. Specify the fee level used to compute feePerKb for this txp. * @param {string} opts.payProUrl - Optional. Paypro URL for peers to verify TX * @param {string} opts.excludeUnconfirmedUtxos[=false] - Optional. Do not use UTXOs of unconfirmed transactions as inputs * @param {string} opts.validateOutputs[=true] - Optional. Perform validation on outputs. @@ -1582,7 +1581,6 @@ WalletService.prototype.createTx = function(opts, cb) { message: opts.message, changeAddress: wallet.createAddress(true), feePerKb: opts.feePerKb, - feeLevel: opts.feeLevel, payProUrl: opts.payProUrl, walletM: wallet.m, walletN: wallet.n, diff --git a/test/integration/server.js b/test/integration/server.js index 15d2f57..89685a9 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2742,7 +2742,6 @@ describe('Wallet service', function() { message: 'some message', customData: 'some custom data', feePerKb: 123e2, - feeLevel: 'priority', }; server.createTx(txOpts, function(err, tx) { should.not.exist(err);