From baaa6e62c7eb49dd97c03e33f00fbea4e675b611 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 26 Feb 2016 16:05:27 -0300 Subject: [PATCH 01/32] refactor integration test code --- test/integration/server.js | 767 ++++++++++++++++++++----------------- 1 file changed, 405 insertions(+), 362 deletions(-) diff --git a/test/integration/server.js b/test/integration/server.js index 2009952..0f58fc1 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -239,346 +239,350 @@ describe('Wallet service', function() { }); describe('#joinWallet', function() { - var server, walletId; - beforeEach(function(done) { - server = new WalletService(); - var walletOpts = { - name: 'my wallet', - m: 1, - n: 2, - pubKey: TestData.keyPair.pub, - }; - server.createWallet(walletOpts, function(err, wId) { - should.not.exist(err); - walletId = wId; - should.exist(walletId); - done(); - }); - }); + describe('New clients', function() { - it('should join existing wallet', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - customData: 'dummy custom data', + var server, walletId; + beforeEach(function(done) { + server = new WalletService(); + var walletOpts = { + name: 'my wallet', + m: 1, + n: 2, + pubKey: TestData.keyPair.pub, + }; + server.createWallet(walletOpts, function(err, wId) { + should.not.exist(err); + walletId = wId; + should.exist(walletId); + done(); + }); }); - server.joinWallet(copayerOpts, function(err, result) { - should.not.exist(err); - var copayerId = result.copayerId; - helpers.getAuthServer(copayerId, function(server) { - server.getWallet({}, function(err, wallet) { - wallet.id.should.equal(walletId); - wallet.copayers.length.should.equal(1); - var copayer = wallet.copayers[0]; - copayer.name.should.equal('me'); - copayer.id.should.equal(copayerId); - copayer.customData.should.equal('dummy custom data'); - server.getNotifications({}, function(err, notifications) { - should.not.exist(err); - var notif = _.find(notifications, { - type: 'NewCopayer' - }); - should.exist(notif); - notif.data.walletId.should.equal(walletId); - notif.data.copayerId.should.equal(copayerId); - notif.data.copayerName.should.equal('me'); - notif = _.find(notifications, { - type: 'WalletComplete' + it('should join existing wallet', function(done) { + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + customData: 'dummy custom data', + }); + server.joinWallet(copayerOpts, function(err, result) { + should.not.exist(err); + var copayerId = result.copayerId; + helpers.getAuthServer(copayerId, function(server) { + server.getWallet({}, function(err, wallet) { + wallet.id.should.equal(walletId); + wallet.copayers.length.should.equal(1); + var copayer = wallet.copayers[0]; + copayer.name.should.equal('me'); + copayer.id.should.equal(copayerId); + copayer.customData.should.equal('dummy custom data'); + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var notif = _.find(notifications, { + type: 'NewCopayer' + }); + should.exist(notif); + notif.data.walletId.should.equal(walletId); + notif.data.copayerId.should.equal(copayerId); + notif.data.copayerName.should.equal('me'); + + notif = _.find(notifications, { + type: 'WalletComplete' + }); + should.not.exist(notif); + done(); }); - should.not.exist(notif); - done(); }); }); }); }); - }); - it('should fail to join with no name', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: '', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - }); - server.joinWallet(copayerOpts, function(err, result) { - should.not.exist(result); - should.exist(err); - err.message.should.contain('name'); - done(); - }); - }); - - it('should fail to join non-existent wallet', function(done) { - var copayerOpts = { - walletId: '123', - name: 'me', - xPubKey: 'dummy', - requestPubKey: 'dummy', - copayerSignature: 'dummy', - }; - server.joinWallet(copayerOpts, function(err) { - should.exist(err); - done(); - }); - }); - - it('should fail to join full wallet', function(done) { - helpers.createAndJoinWallet(1, 1, function(s, wallet) { + it('should fail to join with no name', function(done) { var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: wallet.id, - name: 'me', - xPubKey: TestData.copayers[1].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[1].pubKey_1H_0, - }); - server.joinWallet(copayerOpts, function(err) { - should.exist(err); - err.code.should.equal('WALLET_FULL'); - err.message.should.equal('Wallet full'); - done(); - }); - }); - }); - - it('should return copayer in wallet error before full wallet', function(done) { - helpers.createAndJoinWallet(1, 1, function(s, wallet) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: wallet.id, - name: 'me', + walletId: walletId, + name: '', xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, requestPubKey: TestData.copayers[0].pubKey_1H_0, }); - server.joinWallet(copayerOpts, function(err) { + server.joinWallet(copayerOpts, function(err, result) { + should.not.exist(result); should.exist(err); - err.code.should.equal('COPAYER_IN_WALLET'); + err.message.should.contain('name'); done(); }); }); - }); - it('should fail to re-join wallet', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - }); - server.joinWallet(copayerOpts, function(err) { - should.not.exist(err); - server.joinWallet(copayerOpts, function(err) { - should.exist(err); - err.code.should.equal('COPAYER_IN_WALLET'); - err.message.should.equal('Copayer already in wallet'); - done(); - }); - }); - }); - - it('should be able to get wallet info without actually joining', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - customData: 'dummy custom data', - dryRun: true, - }); - server.joinWallet(copayerOpts, function(err, result) { - should.not.exist(err); - should.exist(result); - should.not.exist(result.copayerId); - result.wallet.id.should.equal(walletId); - result.wallet.m.should.equal(1); - result.wallet.n.should.equal(2); - result.wallet.copayers.should.be.empty; - server.storage.fetchWallet(walletId, function(err, wallet) { - should.not.exist(err); - wallet.id.should.equal(walletId); - wallet.copayers.should.be.empty; - done(); - }); - }); - }); - - it('should fail to join two wallets with same xPubKey', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - }); - server.joinWallet(copayerOpts, function(err) { - should.not.exist(err); - - var walletOpts = { - name: 'my other wallet', - m: 1, - n: 1, - pubKey: TestData.keyPair.pub, + it('should fail to join non-existent wallet', function(done) { + var copayerOpts = { + walletId: '123', + name: 'me', + xPubKey: 'dummy', + requestPubKey: 'dummy', + copayerSignature: 'dummy', }; - server.createWallet(walletOpts, function(err, walletId) { - should.not.exist(err); - copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, + server.joinWallet(copayerOpts, function(err) { + should.exist(err); + done(); + }); + }); + + it('should fail to join full wallet', function(done) { + helpers.createAndJoinWallet(1, 1, function(s, wallet) { + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: wallet.id, + name: 'me', + xPubKey: TestData.copayers[1].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[1].pubKey_1H_0, + }); + server.joinWallet(copayerOpts, function(err) { + should.exist(err); + err.code.should.equal('WALLET_FULL'); + err.message.should.equal('Wallet full'); + done(); + }); + }); + }); + + it('should return copayer in wallet error before full wallet', function(done) { + helpers.createAndJoinWallet(1, 1, function(s, wallet) { + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: wallet.id, name: 'me', xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, requestPubKey: TestData.copayers[0].pubKey_1H_0, }); server.joinWallet(copayerOpts, function(err) { should.exist(err); - err.code.should.equal('COPAYER_REGISTERED'); - err.message.should.equal('Copayer ID already registered on server'); + err.code.should.equal('COPAYER_IN_WALLET'); done(); }); }); }); - }); - it('should fail to join with bad formated signature', function(done) { - var copayerOpts = { - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - copayerSignature: 'bad sign', - }; - server.joinWallet(copayerOpts, function(err) { - err.message.should.equal('Bad request'); - done(); - }); - }); - - it('should fail to join with invalid xPubKey', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: 'copayer 1', - xPubKey: 'invalid', - requestPubKey: TestData.copayers[0].pubKey_1H_0, - }); - server.joinWallet(copayerOpts, function(err, result) { - should.not.exist(result); - should.exist(err); - err.message.should.contain('extended public key'); - done(); - }); - }); - - it('should fail to join with null signature', function(done) { - var copayerOpts = { - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - }; - server.joinWallet(copayerOpts, function(err) { - should.exist(err); - err.message.should.contain('argument missing'); - done(); - }); - }); - - it('should fail to join with wrong signature', function(done) { - var copayerOpts = helpers.getSignedCopayerOpts({ - walletId: walletId, - name: 'me', - xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, - requestPubKey: TestData.copayers[0].pubKey_1H_0, - }); - copayerOpts.name = 'me2'; - server.joinWallet(copayerOpts, function(err) { - err.message.should.equal('Bad request'); - done(); - }); - }); - - it('should set pkr and status = complete on last copayer joining (2-3)', function(done) { - helpers.createAndJoinWallet(2, 3, function(server) { - server.getWallet({}, function(err, wallet) { - should.not.exist(err); - wallet.status.should.equal('complete'); - wallet.publicKeyRing.length.should.equal(3); - server.getNotifications({}, function(err, notifications) { - should.not.exist(err); - var notif = _.find(notifications, { - type: 'WalletComplete' - }); - should.exist(notif); - notif.data.walletId.should.equal(wallet.id); - done(); - }); - }); - }); - }); - - it('should not notify WalletComplete if 1-of-1', function(done) { - helpers.createAndJoinWallet(1, 1, function(server) { - server.getNotifications({}, function(err, notifications) { - should.not.exist(err); - var notif = _.find(notifications, { - type: 'WalletComplete' - }); - should.not.exist(notif); - done(); - }); - }); - }); - }); - - describe('#joinWallet new/legacy clients', function() { - var server; - beforeEach(function() { - server = new WalletService(); - }); - - it('should fail to join legacy wallet from new client', function(done) { - var walletOpts = { - name: 'my wallet', - m: 1, - n: 2, - pubKey: TestData.keyPair.pub, - supportBIP44AndP2PKH: false, - }; - server.createWallet(walletOpts, function(err, walletId) { - should.not.exist(err); - should.exist(walletId); + it('should fail to re-join wallet', function(done) { var copayerOpts = helpers.getSignedCopayerOpts({ walletId: walletId, name: 'me', xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, requestPubKey: TestData.copayers[0].pubKey_1H_0, }); - server.joinWallet(copayerOpts, function(err, result) { - should.exist(err); - err.message.should.contain('The wallet you are trying to join was created with an older version of the client app'); - done(); + server.joinWallet(copayerOpts, function(err) { + should.not.exist(err); + server.joinWallet(copayerOpts, function(err) { + should.exist(err); + err.code.should.equal('COPAYER_IN_WALLET'); + err.message.should.equal('Copayer already in wallet'); + done(); + }); }); }); - }); - it('should fail to join new wallet from legacy client', function(done) { - var walletOpts = { - name: 'my wallet', - m: 1, - n: 2, - pubKey: TestData.keyPair.pub, - }; - server.createWallet(walletOpts, function(err, walletId) { - should.not.exist(err); - should.exist(walletId); + + it('should be able to get wallet info without actually joining', function(done) { var copayerOpts = helpers.getSignedCopayerOpts({ walletId: walletId, name: 'me', - xPubKey: TestData.copayers[0].xPubKey_45H, + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, requestPubKey: TestData.copayers[0].pubKey_1H_0, - supportBIP44AndP2PKH: false, + customData: 'dummy custom data', + dryRun: true, }); server.joinWallet(copayerOpts, function(err, result) { - should.exist(err); - err.code.should.equal('UPGRADE_NEEDED'); + should.not.exist(err); + should.exist(result); + should.not.exist(result.copayerId); + result.wallet.id.should.equal(walletId); + result.wallet.m.should.equal(1); + result.wallet.n.should.equal(2); + result.wallet.copayers.should.be.empty; + server.storage.fetchWallet(walletId, function(err, wallet) { + should.not.exist(err); + wallet.id.should.equal(walletId); + wallet.copayers.should.be.empty; + done(); + }); + }); + }); + + it('should fail to join two wallets with same xPubKey', function(done) { + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + }); + server.joinWallet(copayerOpts, function(err) { + should.not.exist(err); + + var walletOpts = { + name: 'my other wallet', + m: 1, + n: 1, + pubKey: TestData.keyPair.pub, + }; + server.createWallet(walletOpts, function(err, walletId) { + should.not.exist(err); + copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + }); + server.joinWallet(copayerOpts, function(err) { + should.exist(err); + err.code.should.equal('COPAYER_REGISTERED'); + err.message.should.equal('Copayer ID already registered on server'); + done(); + }); + }); + }); + }); + + it('should fail to join with bad formated signature', function(done) { + var copayerOpts = { + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + copayerSignature: 'bad sign', + }; + server.joinWallet(copayerOpts, function(err) { + err.message.should.equal('Bad request'); done(); }); }); + + it('should fail to join with invalid xPubKey', function(done) { + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'copayer 1', + xPubKey: 'invalid', + requestPubKey: TestData.copayers[0].pubKey_1H_0, + }); + server.joinWallet(copayerOpts, function(err, result) { + should.not.exist(result); + should.exist(err); + err.message.should.contain('extended public key'); + done(); + }); + }); + + it('should fail to join with null signature', function(done) { + var copayerOpts = { + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + }; + server.joinWallet(copayerOpts, function(err) { + should.exist(err); + err.message.should.contain('argument missing'); + done(); + }); + }); + + it('should fail to join with wrong signature', function(done) { + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + }); + copayerOpts.name = 'me2'; + server.joinWallet(copayerOpts, function(err) { + err.message.should.equal('Bad request'); + done(); + }); + }); + + it('should set pkr and status = complete on last copayer joining (2-3)', function(done) { + helpers.createAndJoinWallet(2, 3, function(server) { + server.getWallet({}, function(err, wallet) { + should.not.exist(err); + wallet.status.should.equal('complete'); + wallet.publicKeyRing.length.should.equal(3); + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var notif = _.find(notifications, { + type: 'WalletComplete' + }); + should.exist(notif); + notif.data.walletId.should.equal(wallet.id); + done(); + }); + }); + }); + }); + + it('should not notify WalletComplete if 1-of-1', function(done) { + helpers.createAndJoinWallet(1, 1, function(server) { + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var notif = _.find(notifications, { + type: 'WalletComplete' + }); + should.not.exist(notif); + done(); + }); + }); + }); + }); + + describe('Interaction new/legacy clients', function() { + var server; + beforeEach(function() { + server = new WalletService(); + }); + + it('should fail to join legacy wallet from new client', function(done) { + var walletOpts = { + name: 'my wallet', + m: 1, + n: 2, + pubKey: TestData.keyPair.pub, + supportBIP44AndP2PKH: false, + }; + server.createWallet(walletOpts, function(err, walletId) { + should.not.exist(err); + should.exist(walletId); + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_44H_0H_0H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + }); + server.joinWallet(copayerOpts, function(err, result) { + should.exist(err); + err.message.should.contain('The wallet you are trying to join was created with an older version of the client app'); + done(); + }); + }); + }); + + it('should fail to join new wallet from legacy client', function(done) { + var walletOpts = { + name: 'my wallet', + m: 1, + n: 2, + pubKey: TestData.keyPair.pub, + }; + server.createWallet(walletOpts, function(err, walletId) { + should.not.exist(err); + should.exist(walletId); + var copayerOpts = helpers.getSignedCopayerOpts({ + walletId: walletId, + name: 'me', + xPubKey: TestData.copayers[0].xPubKey_45H, + requestPubKey: TestData.copayers[0].pubKey_1H_0, + supportBIP44AndP2PKH: false, + }); + server.joinWallet(copayerOpts, function(err, result) { + should.exist(err); + err.code.should.equal('UPGRADE_NEEDED'); + done(); + }); + }); + }); }); }); @@ -3084,83 +3088,124 @@ describe('Wallet service', function() { }); }); }); - }); - }); - describe('#createTx backoff time', function(done) { - var server, wallet, txid; + describe('Backoff time', function(done) { + var server, wallet, txid; - beforeEach(function(done) { - helpers.createAndJoinWallet(2, 2, function(s, w) { - server = s; - wallet = w; - helpers.stubUtxos(server, wallet, _.range(2, 6), function() { + beforeEach(function(done) { + helpers.createAndJoinWallet(2, 2, function(s, w) { + server = s; + wallet = w; + helpers.stubUtxos(server, wallet, _.range(2, 6), function() { + done(); + }); + }); + }); + + it('should follow backoff time after consecutive rejections', function(done) { + async.series([ + + function(next) { + async.each(_.range(3), function(i, next) { + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); + server.createTxLegacy(txOpts, function(err, tx) { + should.not.exist(err); + server.rejectTx({ + txProposalId: tx.id, + reason: 'some reason', + }, next); + }); + }, + next); + }, + function(next) { + // Allow a 4th tx + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); + server.createTxLegacy(txOpts, function(err, tx) { + server.rejectTx({ + txProposalId: tx.id, + reason: 'some reason', + }, next); + }); + }, + function(next) { + // Do not allow before backoff time + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); + server.createTxLegacy(txOpts, function(err, tx) { + should.exist(err); + err.code.should.equal('TX_CANNOT_CREATE'); + next(); + }); + }, + function(next) { + var clock = sinon.useFakeTimers(Date.now() + (Defaults.BACKOFF_TIME + 2) * 60 * 1000, 'Date'); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); + server.createTxLegacy(txOpts, function(err, tx) { + clock.restore(); + server.rejectTx({ + txProposalId: tx.id, + reason: 'some reason', + }, next); + }); + }, + function(next) { + // Do not allow a 5th tx before backoff time + var clock = sinon.useFakeTimers(Date.now() + (Defaults.BACKOFF_TIME + 2) * 60 * 1000 + 1, 'Date'); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); + server.createTxLegacy(txOpts, function(err, tx) { + clock.restore(); + should.exist(err); + err.code.should.equal('TX_CANNOT_CREATE'); + next(); + }); + }, + ], function(err) { + should.not.exist(err); done(); }); }); }); - it('should follow backoff time after consecutive rejections', function(done) { - async.series([ + describe('UTXO selection', function() { + var server, wallet; + beforeEach(function(done) { + helpers.createAndJoinWallet(2, 3, function(s, w) { + server = s; + wallet = w; + done(); + }); + }); - function(next) { - async.each(_.range(3), function(i, next) { - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); - server.createTxLegacy(txOpts, function(err, tx) { - should.not.exist(err); - server.rejectTx({ - txProposalId: tx.id, - reason: 'some reason', - }, next); - }); - }, - next); - }, - function(next) { - // Allow a 4th tx - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); - server.createTxLegacy(txOpts, function(err, tx) { - server.rejectTx({ - txProposalId: tx.id, - reason: 'some reason', - }, next); + it('should create a tx', function(done) { + helpers.stubUtxos(server, wallet, [1, 2], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 0.8 * 1e8, + }], + message: 'some message', + customData: 'some custom data', + }; + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.walletM.should.equal(2); + tx.walletN.should.equal(3); + tx.requiredRejections.should.equal(2); + tx.requiredSignatures.should.equal(2); + tx.isAccepted().should.equal.false; + tx.isRejected().should.equal.false; + tx.isPending().should.equal.true; + tx.isTemporary().should.equal.true; + tx.amount.should.equal(helpers.toSatoshi(0.8)); + server.getPendingTxs({}, function(err, txs) { + should.not.exist(err); + txs.should.be.empty; + done(); + }); }); - }, - function(next) { - // Do not allow before backoff time - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); - server.createTxLegacy(txOpts, function(err, tx) { - should.exist(err); - err.code.should.equal('TX_CANNOT_CREATE'); - next(); - }); - }, - function(next) { - var clock = sinon.useFakeTimers(Date.now() + (Defaults.BACKOFF_TIME + 2) * 60 * 1000, 'Date'); - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); - server.createTxLegacy(txOpts, function(err, tx) { - clock.restore(); - server.rejectTx({ - txProposalId: tx.id, - reason: 'some reason', - }, next); - }); - }, - function(next) { - // Do not allow a 5th tx before backoff time - var clock = sinon.useFakeTimers(Date.now() + (Defaults.BACKOFF_TIME + 2) * 60 * 1000 + 1, 'Date'); - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 1, TestData.copayers[0].privKey_1H_0); - server.createTxLegacy(txOpts, function(err, tx) { - clock.restore(); - should.exist(err); - err.code.should.equal('TX_CANNOT_CREATE'); - next(); - }); - }, - ], function(err) { - should.not.exist(err); - done(); + }); }); }); }); @@ -5023,7 +5068,6 @@ describe('Wallet service', function() { done(); }); }); - }); describe('#scan', function() { @@ -5686,5 +5730,4 @@ describe('Wallet service', function() { }); }); }); - }); From dbba3acfa8682811f146dc16cc66582abca05e7f Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 29 Feb 2016 18:17:14 -0300 Subject: [PATCH 02/32] new input selection algorithm --- lib/model/txproposal.js | 7 +- lib/server.js | 171 +++++++++++++++++++++++++++++++++++- test/integration/helpers.js | 17 ++-- test/integration/server.js | 72 +++++++++++++++ 4 files changed, 255 insertions(+), 12 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 0498702..1328810 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -219,13 +219,16 @@ TxProposal.prototype.getRawTx = function() { return t.uncheckedSerialize(); }; +TxProposal.prototype.getEstimatedSizeForSingleInput = function() { + return this.requiredSignatures * 72 + this.walletN * 36 + 44; +}; + 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; - var inputSize = walletM * 72 + this.walletN * 36 + 44; + var inputSize = this.getEstimatedSizeForSingleInput(); var outputSize = 34; var nbInputs = this.inputs.length; var nbOutputs = (_.isArray(this.outputs) ? Math.max(1, this.outputs.length) : 1) + 1; diff --git a/lib/server.js b/lib/server.js index b2b1759..49e2d89 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1347,6 +1347,171 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { }); }; + +var UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; + +WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { + var self = this; + + //todo: check inputs are ours and has enough value + if (txp.inputs && txp.inputs.length > 0) { + return cb(self._checkTxAndEstimateFee(txp)); + } + + function excludeUtxos(utxos) { + var excludeIndex = _.reduce(utxosToExclude, function(res, val) { + res[val] = val; + return res; + }, {}); + + return _.reject(utxos, function(utxo) { + return excludeIndex[utxo.txid + ":" + utxo.vout]; + }); + }; + + function partitionUtxos(utxos) { + return _.groupBy(utxos, function(utxo) { + if (utxo.confirmations == 0) return '0' + if (utxo.confirmations < 6) return '<6'; + return '6+'; + }); + }; + + function select(utxos) { + var txpAmount = txp.getTotalAmount(); + var i = 0; + var total = 0; + var selected = []; + + console.log('*** [server.js ln1362] ----------------------- select for amount of:', txpAmount); // TODO + + // TODO: fix for when fee is specified instead of feePerKb + var feePerInput = txp.getEstimatedSizeForSingleInput() * txp.feePerKb / 1000.; + + console.log('*** [server.js ln1375] feePerInput:', feePerInput); // TODO + + var partitions = _.partition(utxos, function(utxo) { + return utxo.satoshis > txpAmount * UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + }); + + var bigInputs = _.sortBy(partitions[0], 'satoshis'); + var smallInputs = _.sortBy(partitions[1], function(utxo) { + return -utxo.satoshis; + }); + + console.log('*** [server.js ln1386] bigInputs:', _.pluck(bigInputs, 'satoshis')); // TODO + console.log('*** [server.js ln1386] smallInputs:', _.pluck(smallInputs, 'satoshis')); // TODO + + + _.each(smallInputs, function(input) { + if (input.satoshis < feePerInput) return false; + selected.push(input); + + console.log('*** [server.js ln1380] input:', input.satoshis, ' aporta ->>> ', input.satoshis - feePerInput); // TODO + + total += input.satoshis - feePerInput; + if (total >= txpAmount) return false; + }); + + console.log('*** [server.js ln1400] total, txpAmount:', total, txpAmount); // TODO + + if (total < txpAmount) { + console.log('*** [server.js ln1401] no alcanzó:'); // TODO + + selected = []; + if (!_.isEmpty(bigInputs)) { + console.log('*** [server.js ln1405] pero hay bigInputs!:', _.first(bigInputs).satoshis); // TODO + + selected = [_.first(bigInputs)]; + } + } + return selected; + }; + + self._getUtxosForCurrentWallet(null, function(err, utxos) { + if (err) return cb(err); + + utxos = excludeUtxos(utxos); + + var totalAmount; + var availableAmount; + + var balance = self._totalizeUtxos(utxos); + if (txp.excludeUnconfirmedUtxos) { + totalAmount = balance.totalConfirmedAmount; + availableAmount = balance.availableConfirmedAmount; + } else { + totalAmount = balance.totalAmount; + availableAmount = balance.availableAmount; + } + + if (totalAmount < txp.getTotalAmount()) return cb(Errors.INSUFFICIENT_FUNDS); + if (availableAmount < txp.getTotalAmount()) return cb(Errors.LOCKED_FUNDS); + + // Prepare UTXOs list + utxos = _.reject(utxos, 'locked'); + if (txp.excludeUnconfirmedUtxos) { + utxos = _.filter(utxos, 'confirmations'); + } + + var inputs = []; + var groups = [6, 1, 0]; + var lastGroupLength; + _.each(groups, function(group) { + var candidateUtxos = _.filter(utxos, function(utxo) { + return utxo.confirmations >= group; + }); + + // If this group does not have any new elements, skip it + if (lastGroupLength === candidateUtxos.length) return; + lastGroupLength = candidateUtxos.length; + + console.log('*** [server.js ln1415] group >=', group, '\n', _.map(candidateUtxos, function(u) { + return _.pick(u, 'satoshis', 'confirmations') + })); // TODO + + inputs = select(candidateUtxos); + + console.log('*** [server.js ln1418] inputs:', _.pluck(inputs, 'satoshis')); // TODO + + if (!_.isEmpty(inputs)) return false; + }); + + if (_.isEmpty(inputs)) return cb(Errors.INSUFFICIENT_FUNDS); + + txp.setInputs(inputs); + if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) + return cb(Errors.TX_MAX_SIZE_EXCEEDED); + + var bitcoreError = self._checkTxAndEstimateFee(txp); + return cb(bitcoreError); + + // var i = 0; + // var total = 0; + // var selected = []; + + // var bitcoreTx, bitcoreError; + + // function select() { + // if (i >= inputs.length) return cb(bitcoreError || new Error('Could not select tx inputs')); + + // var input = inputs[i++]; + // selected.push(input); + // total += input.satoshis; + // if (total >= txp.getTotalAmount()) { + // txp.setInputs(selected); + // bitcoreError = self._checkTxAndEstimateFee(txp); + // if (!bitcoreError) return cb(); + // if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) + // return cb(Errors.TX_MAX_SIZE_EXCEEDED); + // } + // setTimeout(select, 0); + // }; + + // select(); + }); +}; + WalletService.prototype._canCreateTx = function(cb) { var self = this; self.storage.fetchLastTxs(self.walletId, self.copayerId, 5 + Defaults.BACKOFF_OFFSET, function(err, txs) { @@ -1527,7 +1692,7 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { txp.version = '1.0.1'; } - self._selectTxInputs(txp, opts.utxosToExclude, function(err) { + self._selectTxInputs2(txp, opts.utxosToExclude, function(err) { if (err) return cb(err); $.checkState(txp.inputs); @@ -1610,7 +1775,7 @@ WalletService.prototype.createTx = function(opts, cb) { var txp = Model.TxProposal.create(txOpts); - self._selectTxInputs(txp, opts.utxosToExclude, function(err) { + self._selectTxInputs2(txp, opts.utxosToExclude, function(err) { if (err) return cb(err); self.storage.storeAddressAndWallet(wallet, txp.changeAddress, function(err) { @@ -2213,7 +2378,7 @@ WalletService.prototype.getTxHistory = function(opts, cb) { } amount = Math.abs(amount); - if (action == 'sent' || action == 'moved') { + if (action == 'sent' || xaction == 'moved') { var firstExternalOutput = _.find(outputs, { isMine: false }); diff --git a/test/integration/helpers.js b/test/integration/helpers.js index e612662..09192e8 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -233,12 +233,15 @@ helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { addresses.should.not.be.empty; var utxos = _.compact(_.map([].concat(amounts), function(amount, i) { - var confirmations; - if (_.isString(amount) && _.startsWith(amount, 'u')) { - amount = parseFloat(amount.substring(1)); - confirmations = 0; - } else { - confirmations = Math.floor(Math.random() * 100 + 1); + var confirmations = _.random(6, 100); + if (_.isString(amount)) { + if (_.startsWith(amount, 'u')) { + amount = parseFloat(amount.substring(1)); + confirmations = 0; + } else if (_.startsWith(amount, '<6')) { + amount = parseFloat(amount.substring(2)); + confirmations = _.random(1, 5); + } } if (amount <= 0) return null; @@ -257,7 +260,7 @@ helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { return { txid: helpers.randomTXID(), - vout: Math.floor(Math.random() * 10 + 1), + vout: _.random(0, 10), satoshis: helpers.toSatoshi(amount), scriptPubKey: scriptPubKey.toBuffer().toString('hex'), address: address.address, diff --git a/test/integration/server.js b/test/integration/server.js index 0f58fc1..1effa87 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -5730,4 +5730,76 @@ describe('Wallet service', function() { }); }); }); + + describe('UTXO Selection', function() { + var server, wallet; + beforeEach(function(done) { + helpers.createAndJoinWallet(2, 3, function(s, w) { + server = s; + wallet = w; + done(); + }); + }); + + it.skip('should select a single utxo if within thresholds relative to tx amount', function(done) {}); + + it('should select smaller utxos if within max fee constraints', function(done) { + helpers.stubUtxos(server, wallet, [1, 0.0001, 0.0001, 0.0001], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 20000, + }], + feePerKb: 1000, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + done(); + }); + }); + }); + it('should select smallest big utxo if small utxos are insufficient', function(done) { + helpers.stubUtxos(server, wallet, [3, 1, 2, 0.0001, 0.0001, 0.0001], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 30000, + }], + feePerKb: 1000, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + done(); + }); + }); + }); + it.skip('should select smallest big utxo if small utxos exceed maximum fee', function(done) {}); + it.only('should ignore utxos not contributing enough to cover increase in fee', function(done) { + helpers.stubUtxos(server, wallet, [0.0001, 0.0001, 0.0001], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 20000, + }], + feePerKb: 8000, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(3); + txOpts.feePerKb = 12000; + server.createTx(txOpts, function(err, txp) { + should.exist(err); + should.not.exist(txp); + done(); + }); + }); + }); + }); + }); + }); From 328f6250be905b39f72b771131ff0196bca590dd Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 1 Mar 2016 12:51:59 -0300 Subject: [PATCH 03/32] improve utxo mocking --- lib/model/txproposal_legacy.js | 9 ++++-- test/integration/helpers.js | 52 +++++++++++++++++++++++++--------- test/integration/server.js | 2 +- 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/lib/model/txproposal_legacy.js b/lib/model/txproposal_legacy.js index a2af008..da39d8f 100644 --- a/lib/model/txproposal_legacy.js +++ b/lib/model/txproposal_legacy.js @@ -273,16 +273,19 @@ TxProposal.prototype.getRawTx = function() { return t.uncheckedSerialize(); }; +TxProposal.prototype.getEstimatedSizeForSingleInput = function() { + return this.requiredSignatures * 72 + this.walletN * 36 + 44; +}; + 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; - var inputSize = walletM * 72 + this.walletN * 36 + 44; + var inputSize = this.getEstimatedSizeForSingleInput(); var outputSize = 34; var nbInputs = this.inputs.length; - var nbOutputs = (_.isArray(this.outputs) ? this.outputs.length : 1) + 1; + var nbOutputs = (_.isArray(this.outputs) ? Math.max(1, this.outputs.length) : 1) + 1; var size = overhead + inputSize * nbInputs + outputSize * nbOutputs; diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 09192e8..1d43e8e 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -212,6 +212,40 @@ helpers.toSatoshi = function(btc) { } }; +helpers._parseAmount = function(str) { + var result = { + amount: +0, + confirmations: _.random(6, 100), + }; + + if (_.isNumber(str)) str = str.toString(); + + var re = /^((?:\d+c)|u)?\s*([\d\.]+)\s*(btc|bit|sat)?$/; + var match = str.match(re); + + if (!match) throw new Error('Could not parse amount ' + str); + + if (match[1]) { + if (match[1] == 'u') result.confirmations = 0; + if (_.endsWith(match[1], 'c')) result.confirmations = +match[1].slice(0, -1); + } + + switch (match[3]) { + default: + case 'btc': + result.amount = Utils.strip(match[2] * 1e8); + break; + case 'bit': + result.amount = Utils.strip(match[2] * 1e6); + break + case 'sat': + result.amount = Utils.strip(match[2]); + break; + }; + + return result; +}; + helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { if (_.isFunction(opts)) { cb = opts; @@ -233,17 +267,9 @@ helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { addresses.should.not.be.empty; var utxos = _.compact(_.map([].concat(amounts), function(amount, i) { - var confirmations = _.random(6, 100); - if (_.isString(amount)) { - if (_.startsWith(amount, 'u')) { - amount = parseFloat(amount.substring(1)); - confirmations = 0; - } else if (_.startsWith(amount, '<6')) { - amount = parseFloat(amount.substring(2)); - confirmations = _.random(1, 5); - } - } - if (amount <= 0) return null; + var parsed = helpers._parseAmount(amount); + console.log('*** [helpers.js ln270] amount, result:', amount, parsed); // TODO + if (parsed.amount <= 0) return null; var address = addresses[i % addresses.length]; @@ -261,10 +287,10 @@ helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { return { txid: helpers.randomTXID(), vout: _.random(0, 10), - satoshis: helpers.toSatoshi(amount), + satoshis: parsed.amount, scriptPubKey: scriptPubKey.toBuffer().toString('hex'), address: address.address, - confirmations: confirmations + confirmations: parsed.confirmations, }; })); diff --git a/test/integration/server.js b/test/integration/server.js index 1effa87..0fcba03 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -5778,7 +5778,7 @@ describe('Wallet service', function() { }); }); it.skip('should select smallest big utxo if small utxos exceed maximum fee', function(done) {}); - it.only('should ignore utxos not contributing enough to cover increase in fee', function(done) { + it('should ignore utxos not contributing enough to cover increase in fee', function(done) { helpers.stubUtxos(server, wallet, [0.0001, 0.0001, 0.0001], function() { var txOpts = { outputs: [{ From cd820178457316e9e961c7779d5886b5d9f6227d Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 1 Mar 2016 14:54:02 -0300 Subject: [PATCH 04/32] fix tests --- lib/server.js | 4 ++-- test/integration/server.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/server.js b/lib/server.js index 49e2d89..24b8342 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1477,7 +1477,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { if (!_.isEmpty(inputs)) return false; }); - if (_.isEmpty(inputs)) return cb(Errors.INSUFFICIENT_FUNDS); + if (_.isEmpty(inputs)) return cb(Errors.INSUFFICIENT_FUNDS_FOR_FEE); txp.setInputs(inputs); if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) @@ -2378,7 +2378,7 @@ WalletService.prototype.getTxHistory = function(opts, cb) { } amount = Math.abs(amount); - if (action == 'sent' || xaction == 'moved') { + if (action == 'sent' || action == 'moved') { var firstExternalOutput = _.find(outputs, { isMine: false }); diff --git a/test/integration/server.js b/test/integration/server.js index 0fcba03..f2a7007 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2355,7 +2355,7 @@ describe('Wallet service', function() { it('should fail to create a tx exceeding max size in kb', function(done) { helpers.stubUtxos(server, wallet, _.range(1, 10, 0), function() { - var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 9, TestData.copayers[0].privKey_1H_0); + var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 8, TestData.copayers[0].privKey_1H_0); var _oldDefault = Defaults.MAX_TX_SIZE_IN_KB; Defaults.MAX_TX_SIZE_IN_KB = 1; server.createTxLegacy(txOpts, function(err, tx) { @@ -5778,6 +5778,7 @@ describe('Wallet service', function() { }); }); it.skip('should select smallest big utxo if small utxos exceed maximum fee', function(done) {}); + it.skip('should not fail with tx exceeded max size if there is at least 1 big input', function(done) {}); it('should ignore utxos not contributing enough to cover increase in fee', function(done) { helpers.stubUtxos(server, wallet, [0.0001, 0.0001, 0.0001], function() { var txOpts = { From fdd5255e59e76da30424baa616953ffbb2841a91 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 1 Mar 2016 17:40:07 -0300 Subject: [PATCH 05/32] add stop conditions for small utxos --- lib/server.js | 17 ++++++++++- test/integration/helpers.js | 4 +-- test/integration/server.js | 60 ++++++++++++++++++++++++++++++++----- 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/lib/server.js b/lib/server.js index 24b8342..88a1c41 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1349,6 +1349,8 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; +var UTXO_SELECTION_MAX_FEE_FACTOR_FOR_SMALL_UTXOS = 2; +var UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR_FOR_SMALL_UTXOS = 0.002; WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var self = this; @@ -1386,6 +1388,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { console.log('*** [server.js ln1362] ----------------------- select for amount of:', txpAmount); // TODO // TODO: fix for when fee is specified instead of feePerKb + var baseTxpFee = txp.getEstimatedSize() * txp.feePerKb / 1000.; var feePerInput = txp.getEstimatedSizeForSingleInput() * txp.feePerKb / 1000.; console.log('*** [server.js ln1375] feePerInput:', feePerInput); // TODO @@ -1407,7 +1410,19 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { if (input.satoshis < feePerInput) return false; selected.push(input); - console.log('*** [server.js ln1380] input:', input.satoshis, ' aporta ->>> ', input.satoshis - feePerInput); // TODO + var txpFee = baseTxpFee + (feePerInput * selected.length); + + var amountVsFeeRatio = txpFee / txpAmount; + var singleInputFeeVsFeeRatio = txpFee / (baseTxpFee + feePerInput); + + console.log('*** [server.js ln1402] txpFee, amountVsFeeRatio, singleInputFeeVsFeeRatio:', txpFee, amountVsFeeRatio, singleInputFeeVsFeeRatio); // TODO + + + if (amountVsFeeRatio > UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR_FOR_SMALL_UTXOS && + (_.isEmpty(bigInputs) || (singleInputFeeVsFeeRatio > UTXO_SELECTION_MAX_FEE_FACTOR_FOR_SMALL_UTXOS))) + return false; + + console.log('*** [server.js ln1380] input:', input.satoshis, ' aporta:', input.satoshis - feePerInput); // TODO total += input.satoshis - feePerInput; if (total >= txpAmount) return false; diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 1d43e8e..4a573b2 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -236,7 +236,7 @@ helpers._parseAmount = function(str) { result.amount = Utils.strip(match[2] * 1e8); break; case 'bit': - result.amount = Utils.strip(match[2] * 1e6); + result.amount = Utils.strip(match[2] * 1e2); break case 'sat': result.amount = Utils.strip(match[2]); @@ -268,7 +268,7 @@ helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { var utxos = _.compact(_.map([].concat(amounts), function(amount, i) { var parsed = helpers._parseAmount(amount); - console.log('*** [helpers.js ln270] amount, result:', amount, parsed); // TODO + if (parsed.amount <= 0) return null; var address = addresses[i % addresses.length]; diff --git a/test/integration/server.js b/test/integration/server.js index f2a7007..94529f8 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -5741,10 +5741,8 @@ describe('Wallet service', function() { }); }); - it.skip('should select a single utxo if within thresholds relative to tx amount', function(done) {}); - - it('should select smaller utxos if within max fee constraints', function(done) { - helpers.stubUtxos(server, wallet, [1, 0.0001, 0.0001, 0.0001], function() { + 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() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', @@ -5755,12 +5753,36 @@ describe('Wallet service', function() { server.createTx(txOpts, function(err, txp) { should.not.exist(err); should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(35000); + + done(); + }); + }); + }); + + it('should select smaller utxos if within max fee constraints', function(done) { + helpers.stubUtxos(server, wallet, [1, '100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 20000, + }], + feePerKb: 1000, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(3); + _.all(txp.inputs, function(input) { + return input == 10000; + }); done(); }); }); }); it('should select smallest big utxo if small utxos are insufficient', function(done) { - helpers.stubUtxos(server, wallet, [3, 1, 2, 0.0001, 0.0001, 0.0001], function() { + helpers.stubUtxos(server, wallet, [3, 1, 2, '100bit', '100bit', '100bit'], function() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', @@ -5777,10 +5799,34 @@ describe('Wallet service', function() { }); }); }); - it.skip('should select smallest big utxo if small utxos exceed maximum fee', function(done) {}); + it.only('should select smallest big utxo if small utxos exceed maximum fee', function(done) { + helpers.stubUtxos(server, wallet, [3, 1, 2].concat(_.times(20, function() { + return '1000bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 12000e2, + }], + feePerKb: 20e2, + }; + server.createTx(txOpts, function(err, txp) { + console.log('*** [server.js ln5790] err:', err); // TODO + + should.not.exist(err); + should.exist(txp); + console.log('*** [server.js ln5792] txp.fee:', txp.fee); // TODO + + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + + done(); + }); + }); + }); it.skip('should not fail with tx exceeded max size if there is at least 1 big input', function(done) {}); it('should ignore utxos not contributing enough to cover increase in fee', function(done) { - helpers.stubUtxos(server, wallet, [0.0001, 0.0001, 0.0001], function() { + helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', From a8cd4fe2c69dad862febb6d37e831119b9f5dfcb Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 2 Mar 2016 11:26:07 -0300 Subject: [PATCH 06/32] improve test cases --- lib/common/defaults.js | 5 +++ lib/server.js | 67 ++++++++++++++++++++++++++------------ test/integration/server.js | 54 ++++++++++++++++++++++++++---- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/lib/common/defaults.js b/lib/common/defaults.js index 362363b..a2f5f85 100644 --- a/lib/common/defaults.js +++ b/lib/common/defaults.js @@ -49,4 +49,9 @@ Defaults.FIAT_RATE_MAX_LOOK_BACK_TIME = 120; // In minutes Defaults.HISTORY_LIMIT = 100; +Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; +Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR = 5; +Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR = 0.002; +Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = 0.1; + module.exports = Defaults; diff --git a/lib/server.js b/lib/server.js index 88a1c41..2c9a822 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1348,10 +1348,6 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { }; -var UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; -var UTXO_SELECTION_MAX_FEE_FACTOR_FOR_SMALL_UTXOS = 2; -var UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR_FOR_SMALL_UTXOS = 0.002; - WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var self = this; @@ -1388,13 +1384,15 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { console.log('*** [server.js ln1362] ----------------------- select for amount of:', txpAmount); // TODO // TODO: fix for when fee is specified instead of feePerKb - var baseTxpFee = txp.getEstimatedSize() * txp.feePerKb / 1000.; - var feePerInput = txp.getEstimatedSizeForSingleInput() * txp.feePerKb / 1000.; + var baseTxpSize = txp.getEstimatedSize(); + var baseTxpFee = baseTxpSize * txp.feePerKb / 1000.; + var sizePerInput = txp.getEstimatedSizeForSingleInput(); + var feePerInput = sizePerInput * txp.feePerKb / 1000.; console.log('*** [server.js ln1375] feePerInput:', feePerInput); // TODO var partitions = _.partition(utxos, function(utxo) { - return utxo.satoshis > txpAmount * UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + return utxo.satoshis > txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; }); var bigInputs = _.sortBy(partitions[0], 'satoshis'); @@ -1406,40 +1404,69 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { console.log('*** [server.js ln1386] smallInputs:', _.pluck(smallInputs, 'satoshis')); // TODO - _.each(smallInputs, function(input) { + _.each(smallInputs, function(input, i) { + console.log('****************************** [server.js ln1395] INPUT #', i); // TODO + if (input.satoshis < feePerInput) return false; + + var inputAmount = input.satoshis - feePerInput; + console.log('*** [server.js ln1398] inputAmount:', inputAmount); // TODO + selected.push(input); - var txpFee = baseTxpFee + (feePerInput * selected.length); + var txpSize = baseTxpSize + selected.length * sizePerInput; + var txpFee = baseTxpFee + selected.length * feePerInput; var amountVsFeeRatio = txpFee / txpAmount; var singleInputFeeVsFeeRatio = txpFee / (baseTxpFee + feePerInput); + var amountVsUtxoRatio = inputAmount / txpAmount; - console.log('*** [server.js ln1402] txpFee, amountVsFeeRatio, singleInputFeeVsFeeRatio:', txpFee, amountVsFeeRatio, singleInputFeeVsFeeRatio); // TODO + console.log('*** [server.js ln1402] txpSize, txpFee', txpSize, txpFee); // TODO + console.log('*** [server.js ln1403] amountVsFeeRatio, singleInputFeeVsFeeRatio, amountVsUtxoRatio:', amountVsFeeRatio, singleInputFeeVsFeeRatio, amountVsUtxoRatio); // TODO + if (!_.isEmpty(bigInputs)) { + if ((amountVsFeeRatio > Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR && + singleInputFeeVsFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { + console.log('*** [server.js ln1413] breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); // TODO + return false; + } - if (amountVsFeeRatio > UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR_FOR_SMALL_UTXOS && - (_.isEmpty(bigInputs) || (singleInputFeeVsFeeRatio > UTXO_SELECTION_MAX_FEE_FACTOR_FOR_SMALL_UTXOS))) - return false; + if (amountVsUtxoRatio < Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) { + console.log('*** [server.js ln1418] breaking because utxo is too small compared to tx amount'); // TODO + return false; + } + + if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { + console.log('*** [server.js ln1423] breaking because tx size is too big'); // TODO + + return false; + } + } console.log('*** [server.js ln1380] input:', input.satoshis, ' aporta:', input.satoshis - feePerInput); // TODO - total += input.satoshis - feePerInput; + total += inputAmount; + console.log('*** [server.js ln1421] total:', total); // TODO + if (total >= txpAmount) return false; }); - console.log('*** [server.js ln1400] total, txpAmount:', total, txpAmount); // TODO - if (total < txpAmount) { - console.log('*** [server.js ln1401] no alcanzó:'); // TODO + console.log('*** [server.js ln1401] could not reach txp total, still missing:', txpAmount - total); // TODO selected = []; if (!_.isEmpty(bigInputs)) { - console.log('*** [server.js ln1405] pero hay bigInputs!:', _.first(bigInputs).satoshis); // TODO - - selected = [_.first(bigInputs)]; + console.log('*** [server.js ln1405] using big inputs!:', _.first(bigInputs).satoshis); // TODO + var input = _.first(bigInputs); + total = input.satoshis; + selected = [input]; } } + + if (!_.isEmpty(selected)) { + console.log('*** [server.js ln1400] SUCCESS! locked overhead:', total - txpAmount, ' ratio to txp:', (total - txpAmount) / txpAmount); // TODO + } + return selected; }; diff --git a/test/integration/server.js b/test/integration/server.js index 94529f8..0039270 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -5761,14 +5761,14 @@ describe('Wallet service', function() { }); }); - it('should select smaller utxos if within max fee constraints', function(done) { - helpers.stubUtxos(server, wallet, [1, '100bit', '100bit', '100bit'], function() { + it('should select smaller utxos if within fee constraints', function(done) { + helpers.stubUtxos(server, wallet, [1, '800bit', '800bit', '800bit'], function() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 20000, + amount: 2000e2, }], - feePerKb: 1000, + feePerKb: 10e2, }; server.createTx(txOpts, function(err, txp) { should.not.exist(err); @@ -5799,7 +5799,7 @@ describe('Wallet service', function() { }); }); }); - it.only('should select smallest big utxo if small utxos exceed maximum fee', function(done) { + it('should select smallest big utxo if small utxos exceed maximum fee', function(done) { helpers.stubUtxos(server, wallet, [3, 1, 2].concat(_.times(20, function() { return '1000bit'; })), function() { @@ -5824,7 +5824,49 @@ describe('Wallet service', function() { }); }); }); - it.skip('should not fail with tx exceeded max size if there is at least 1 big input', function(done) {}); + it('should select smallest big utxo if small utxos are below accepted ratio of txp amount', function(done) { + helpers.stubUtxos(server, wallet, [9, 1, 1, 0.5, 0.2, 0.2, 0.2], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 3e8, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(9e8); + done(); + }); + }); + }); + it('should not fail with tx exceeded max size if there is at least 1 big input', function(done) { + var _old1 = Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR; + var _old2 = Defaults.MAX_TX_SIZE_IN_KB; + Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = 0.0001; + Defaults.MAX_TX_SIZE_IN_KB = 3; + + helpers.stubUtxos(server, wallet, [100].concat(_.range(1, 20, 0)), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 15e8, + }], + feePerKb: 120e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(100e8); + Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = _old1; + Defaults.MAX_TX_SIZE_IN_KB = _old2; + done(); + }); + }); + }); it('should ignore utxos not contributing enough to cover increase in fee', function(done) { helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { var txOpts = { From 62c0fb06efe40f8227c8406b668d8f8e5e75c8f5 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 2 Mar 2016 16:44:38 -0300 Subject: [PATCH 07/32] test utxo confirmation priority --- lib/server.js | 9 +++- test/integration/server.js | 94 +++++++++++++++++++++++++++++++++----- 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/lib/server.js b/lib/server.js index 2c9a822..718e276 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1376,7 +1376,6 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { }; function select(utxos) { - var txpAmount = txp.getTotalAmount(); var i = 0; var total = 0; var selected = []; @@ -1384,12 +1383,18 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { console.log('*** [server.js ln1362] ----------------------- select for amount of:', txpAmount); // TODO // TODO: fix for when fee is specified instead of feePerKb + var txpAmount = txp.getTotalAmount(); var baseTxpSize = txp.getEstimatedSize(); var baseTxpFee = baseTxpSize * txp.feePerKb / 1000.; var sizePerInput = txp.getEstimatedSizeForSingleInput(); var feePerInput = sizePerInput * txp.feePerKb / 1000.; - console.log('*** [server.js ln1375] feePerInput:', feePerInput); // TODO + var totalValueInUtxos = _.sum(utxos, 'satoshis') - baseTxpFee - (utxos.length * feePerInput); + if (totalValueInUtxos < txpAmount) { + console.log('*** [server.js ln1380] value in all utxos:', totalValueInUtxos, ' is inusfficient to cover for txp amount:', txpAmount); // TODO + + return false; + } var partitions = _.partition(utxos, function(utxo) { return utxo.satoshis > txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; diff --git a/test/integration/server.js b/test/integration/server.js index 0039270..bc1c2b2 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -5746,9 +5746,9 @@ describe('Wallet service', function() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 20000, + amount: 200e2, }], - feePerKb: 1000, + feePerKb: 10e2, }; server.createTx(txOpts, function(err, txp) { should.not.exist(err); @@ -5775,7 +5775,7 @@ describe('Wallet service', function() { should.exist(txp); txp.inputs.length.should.equal(3); _.all(txp.inputs, function(input) { - return input == 10000; + return input == 100e2; }); done(); }); @@ -5786,9 +5786,9 @@ describe('Wallet service', function() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 30000, + amount: 300e2, }], - feePerKb: 1000, + feePerKb: 10e2, }; server.createTx(txOpts, function(err, txp) { should.not.exist(err); @@ -5811,12 +5811,8 @@ describe('Wallet service', function() { feePerKb: 20e2, }; server.createTx(txOpts, function(err, txp) { - console.log('*** [server.js ln5790] err:', err); // TODO - should.not.exist(err); should.exist(txp); - console.log('*** [server.js ln5792] txp.fee:', txp.fee); // TODO - txp.inputs.length.should.equal(1); txp.inputs[0].satoshis.should.equal(1e8); @@ -5872,15 +5868,15 @@ describe('Wallet service', function() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 20000, + amount: 200e2, }], - feePerKb: 8000, + feePerKb: 80e2, }; server.createTx(txOpts, function(err, txp) { should.not.exist(err); should.exist(txp); txp.inputs.length.should.equal(3); - txOpts.feePerKb = 12000; + txOpts.feePerKb = 120e2; server.createTx(txOpts, function(err, txp) { should.exist(err); should.not.exist(txp); @@ -5889,6 +5885,80 @@ describe('Wallet service', function() { }); }); }); + it('should fail to select utxos if not enough to cover tx amount', function(done) { + helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 400e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.exist(err); + should.not.exist(txp); + err.code.should.equal('INSUFFICIENT_FUNDS'); + done(); + }); + }); + }); + it('should fail to select utxos if not enough to cover fees', function(done) { + helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 299e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.exist(err); + should.not.exist(txp); + err.code.should.equal('INSUFFICIENT_FUNDS_FOR_FEE'); + done(); + }); + }); + }); + it('should prefer a higher fee (breaking all limits) if inputs have 6+ confirmations', function(done) { + helpers.stubUtxos(server, wallet, ['2c 2000bit'].concat(_.times(20, function() { + return '100bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 1500e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + _.all(txp.inputs, function(input) { + return input == 100e2; + }); + done(); + }); + }); + }); + it('should select unconfirmed utxos if not enough confirmed utxos', function(done) { + helpers.stubUtxos(server, wallet, ['u 1btc', '0.5btc'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 0.8e8, + }], + feePerKb: 100e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + done(); + }); + }); + }); + }); }); From c90d5bfed47b4b5cfe329a02b8286b663bc63786 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 2 Mar 2016 17:33:44 -0300 Subject: [PATCH 08/32] proper log messages --- lib/server.js | 120 +++++++++++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 56 deletions(-) diff --git a/lib/server.js b/lib/server.js index 718e276..dc6e2e4 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1375,13 +1375,28 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { }); }; + function formatAmount(amount) { + return Utils.formatAmount(amount, 'btc') + 'btc'; + }; + + function formatInputs(inputs) { + if (_.isEmpty(inputs)) return 'none'; + return _.map([].concat(inputs), function(i) { + var amount = formatAmount(i.satoshis); + var confirmations = i.confirmations ? i.confirmations + 'c' : 'u'; + return amount + '/' + confirmations; + }).join(', '); + }; + + function formatRatio(ratio) { + return (ratio * 100.).toFixed(4) + '%'; + }; + + function formatSize(size) { + return (size / 1000.).toFixed(4) + 'kB'; + }; + function select(utxos) { - var i = 0; - var total = 0; - var selected = []; - - console.log('*** [server.js ln1362] ----------------------- select for amount of:', txpAmount); // TODO - // TODO: fix for when fee is specified instead of feePerKb var txpAmount = txp.getTotalAmount(); var baseTxpSize = txp.getEstimatedSize(); @@ -1391,13 +1406,15 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var totalValueInUtxos = _.sum(utxos, 'satoshis') - baseTxpFee - (utxos.length * feePerInput); if (totalValueInUtxos < txpAmount) { - console.log('*** [server.js ln1380] value in all utxos:', totalValueInUtxos, ' is inusfficient to cover for txp amount:', txpAmount); // TODO - + log.debug('Value in all utxos (' + formatAmount(totalValueInUtxos) + ') is inusufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); // TODO return false; } + var bigInputThreshold = txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + log.debug('Big input threshold ' + formatAmount(bigInputThreshold)); + var partitions = _.partition(utxos, function(utxo) { - return utxo.satoshis > txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + return utxo.satoshis > bigInputThreshold; }); var bigInputs = _.sortBy(partitions[0], 'satoshis'); @@ -1405,17 +1422,22 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return -utxo.satoshis; }); - console.log('*** [server.js ln1386] bigInputs:', _.pluck(bigInputs, 'satoshis')); // TODO - console.log('*** [server.js ln1386] smallInputs:', _.pluck(smallInputs, 'satoshis')); // TODO + log.debug('Considering ' + bigInputs.length + ' big inputs (' + formatInputs(bigInputs) + ')'); + log.debug('Considering ' + smallInputs.length + ' small inputs (' + formatInputs(smallInputs) + ')'); + var total = 0; + var selected = []; _.each(smallInputs, function(input, i) { - console.log('****************************** [server.js ln1395] INPUT #', i); // TODO + log.debug('Input #' + i + ': ' + formatInputs(input)); - if (input.satoshis < feePerInput) return false; + if (input.satoshis < feePerInput) { + log.debug('The input does not cover the extra fees (' + formatAmount(feePerInput) + ')'); + return false; + } var inputAmount = input.satoshis - feePerInput; - console.log('*** [server.js ln1398] inputAmount:', inputAmount); // TODO + log.debug('The input contributes ' + formatAmount(inputAmount)); selected.push(input); @@ -1426,42 +1448,41 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var singleInputFeeVsFeeRatio = txpFee / (baseTxpFee + feePerInput); var amountVsUtxoRatio = inputAmount / txpAmount; - console.log('*** [server.js ln1402] txpSize, txpFee', txpSize, txpFee); // TODO - console.log('*** [server.js ln1403] amountVsFeeRatio, singleInputFeeVsFeeRatio, amountVsUtxoRatio:', amountVsFeeRatio, singleInputFeeVsFeeRatio, amountVsUtxoRatio); // TODO + log.debug('Tx amount/Fee: ' + formatRatio(amountVsFeeRatio) + ' (max: ' + formatRatio(Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR) + ')'); + log.debug('Single-input fee/Multi-input fee: ' + formatRatio(singleInputFeeVsFeeRatio) + ' (max: ' + formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) + ')' + ' loses wrt single-input tx: ' + formatAmount((selected.length - 1) * feePerInput)); + log.debug('Tx amount/input amount:' + formatRatio(amountVsUtxoRatio) + ' (min: ' + formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); if (!_.isEmpty(bigInputs)) { if ((amountVsFeeRatio > Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR && singleInputFeeVsFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { - console.log('*** [server.js ln1413] breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); // TODO + log.debug('Breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); return false; } if (amountVsUtxoRatio < Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) { - console.log('*** [server.js ln1418] breaking because utxo is too small compared to tx amount'); // TODO + log.debug('Breaking because utxo is too small compared to tx amount'); return false; } if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { - console.log('*** [server.js ln1423] breaking because tx size is too big'); // TODO + log.debug('Breaking because tx size is too big (' + formatSize(txpSize) + ')'); // TODO return false; } } - console.log('*** [server.js ln1380] input:', input.satoshis, ' aporta:', input.satoshis - feePerInput); // TODO - total += inputAmount; - console.log('*** [server.js ln1421] total:', total); // TODO + log.debug('Cumuled total so far: ' + formatAmount(total)); if (total >= txpAmount) return false; }); if (total < txpAmount) { - console.log('*** [server.js ln1401] could not reach txp total, still missing:', txpAmount - total); // TODO + log.debug('Could not reach Txp total (' + formatAmount(txpAmount) + '), still missing: ' + formatAmount(txpAmount - total)); selected = []; if (!_.isEmpty(bigInputs)) { - console.log('*** [server.js ln1405] using big inputs!:', _.first(bigInputs).satoshis); // TODO + log.debug('Using big input: ', formatInputs(_.first(bigInputs))); var input = _.first(bigInputs); total = input.satoshis; selected = [input]; @@ -1469,12 +1490,17 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { } if (!_.isEmpty(selected)) { - console.log('*** [server.js ln1400] SUCCESS! locked overhead:', total - txpAmount, ' ratio to txp:', (total - txpAmount) / txpAmount); // TODO + var lockedOverhead = total - txpAmount; + log.debug('SUCCESS! Total locked: ' + formatAmount(total) + ', overhead: ' + formatAmount(lockedOverhead) + ' (' + formatRatio(lockedOverhead / txpAmount) + ')'); + } else { + log.debug('Could not find enough funds within this utxso subset'); } return selected; }; + log.debug('Selecting inputs for a ' + formatAmount(txp.getTotalAmount()) + ' txp'); + self._getUtxosForCurrentWallet(null, function(err, utxos) { if (err) return cb(err); @@ -1501,6 +1527,8 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { utxos = _.filter(utxos, 'confirmations'); } + log.debug('Considering ' + utxos.length + ' utxos (' + formatInputs(utxos) + ')'); + var inputs = []; var groups = [6, 1, 0]; var lastGroupLength; @@ -1509,17 +1537,21 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return utxo.confirmations >= group; }); - // If this group does not have any new elements, skip it - if (lastGroupLength === candidateUtxos.length) return; - lastGroupLength = candidateUtxos.length; + log.debug('Group >= ' + group); - console.log('*** [server.js ln1415] group >=', group, '\n', _.map(candidateUtxos, function(u) { - return _.pick(u, 'satoshis', 'confirmations') - })); // TODO + // If this group does not have any new elements, skip it + if (lastGroupLength === candidateUtxos.length) { + log.debug('This group is identical to the one already explored'); + return; + } + + log.debug('Candidate utxos: ' + formatInputs(candidateUtxos)); + + lastGroupLength = candidateUtxos.length; inputs = select(candidateUtxos); - console.log('*** [server.js ln1418] inputs:', _.pluck(inputs, 'satoshis')); // TODO + log.debug('Selected inputs from this group: ' + formatInputs(inputs)); if (!_.isEmpty(inputs)) return false; }); @@ -1532,30 +1564,6 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var bitcoreError = self._checkTxAndEstimateFee(txp); return cb(bitcoreError); - - // var i = 0; - // var total = 0; - // var selected = []; - - // var bitcoreTx, bitcoreError; - - // function select() { - // if (i >= inputs.length) return cb(bitcoreError || new Error('Could not select tx inputs')); - - // var input = inputs[i++]; - // selected.push(input); - // total += input.satoshis; - // if (total >= txp.getTotalAmount()) { - // txp.setInputs(selected); - // bitcoreError = self._checkTxAndEstimateFee(txp); - // if (!bitcoreError) return cb(); - // if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) - // return cb(Errors.TX_MAX_SIZE_EXCEEDED); - // } - // setTimeout(select, 0); - // }; - - // select(); }); }; From 485b98de86f7016d96368c65b15c92f768281ba1 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 3 Mar 2016 10:15:42 -0300 Subject: [PATCH 09/32] fix big input threshold computation --- lib/server.js | 12 +++++++++--- test/integration/server.js | 26 ++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/server.js b/lib/server.js index dc6e2e4..81acde4 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1410,7 +1410,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return false; } - var bigInputThreshold = txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + var bigInputThreshold = txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR + (baseTxpFee + feePerInput); log.debug('Big input threshold ' + formatAmount(bigInputThreshold)); var partitions = _.partition(utxos, function(utxo) { @@ -1562,8 +1562,14 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) return cb(Errors.TX_MAX_SIZE_EXCEEDED); - var bitcoreError = self._checkTxAndEstimateFee(txp); - return cb(bitcoreError); + var err = self._checkTxAndEstimateFee(txp); + if (!err) { + log.debug('Successfully built transaction. Total fees: ', formatAmount(txp.fee)); + } else { + log.warn('Error building transaction', err); + } + + return cb(err); }); }; diff --git a/test/integration/server.js b/test/integration/server.js index bc1c2b2..2dded8c 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -5760,7 +5760,6 @@ describe('Wallet service', function() { }); }); }); - it('should select smaller utxos if within fee constraints', function(done) { helpers.stubUtxos(server, wallet, [1, '800bit', '800bit', '800bit'], function() { var txOpts = { @@ -5799,6 +5798,30 @@ describe('Wallet service', function() { }); }); }); + it('should account for fee when selecting smallest big utxo', function(done) { + // log.level = 'debug'; + var _old = Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; + // The 605 bits input cannot be selected even if it is > 2 * tx amount + // because it cannot cover for fee on its own. + helpers.stubUtxos(server, wallet, [1, '605bit', '100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 300e2, + }], + feePerKb: 1200e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = _old; + done(); + }); + }); + }); it('should select smallest big utxo if small utxos exceed maximum fee', function(done) { helpers.stubUtxos(server, wallet, [3, 1, 2].concat(_.times(20, function() { return '1000bit'; @@ -5958,7 +5981,6 @@ describe('Wallet service', function() { }); }); }); - }); }); From 2581f488fa6628a08346d31a8e14de6a7d63d768 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Mar 2016 17:10:48 -0300 Subject: [PATCH 10/32] improve error handling --- lib/server.js | 53 ++++++++++++++++++++++++-------------- test/integration/server.js | 6 +++-- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/lib/server.js b/lib/server.js index 81acde4..7d4e569 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1243,6 +1243,9 @@ WalletService.prototype._checkTxAndEstimateFee = function(txp) { txp.estimateFee(); + if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) + return Errors.TX_MAX_SIZE_EXCEEDED; + try { var bitcoreTx = txp.getBitcoreTx(); bitcoreError = bitcoreTx.getSerializationError(serializationOpts); @@ -1337,8 +1340,6 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { txp.setInputs(selected); bitcoreError = self._checkTxAndEstimateFee(txp); if (!bitcoreError) return cb(); - if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) - return cb(Errors.TX_MAX_SIZE_EXCEEDED); } setTimeout(select, 0); }; @@ -1404,10 +1405,15 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var sizePerInput = txp.getEstimatedSizeForSingleInput(); var feePerInput = sizePerInput * txp.feePerKb / 1000.; - var totalValueInUtxos = _.sum(utxos, 'satoshis') - baseTxpFee - (utxos.length * feePerInput); + var totalValueInUtxos = _.sum(utxos, 'satoshis'); + var netValueInUtxos = totalValueInUtxos - baseTxpFee - (utxos.length * feePerInput); if (totalValueInUtxos < txpAmount) { - log.debug('Value in all utxos (' + formatAmount(totalValueInUtxos) + ') is inusufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); // TODO - return false; + log.debug('Total value in all utxos (' + formatAmount(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); // TODO + return Errors.INSUFFICIENT_FUNDS; + } + if (netValueInUtxos < txpAmount) { + log.debug('Value after fees in all utxos (' + formatAmount(netValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); // TODO + return Errors.INSUFFICIENT_FUNDS_FOR_FEE; } var bigInputThreshold = txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR + (baseTxpFee + feePerInput); @@ -1427,6 +1433,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var total = 0; var selected = []; + var error; _.each(smallInputs, function(input, i) { log.debug('Input #' + i + ': ' + formatInputs(input)); @@ -1444,6 +1451,8 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var txpSize = baseTxpSize + selected.length * sizePerInput; var txpFee = baseTxpFee + selected.length * feePerInput; + log.debug('Tx size: ' + formatSize(txpSize) + ', Tx fee: ' + formatAmount(txpFee)); + var amountVsFeeRatio = txpFee / txpAmount; var singleInputFeeVsFeeRatio = txpFee / (baseTxpFee + feePerInput); var amountVsUtxoRatio = inputAmount / txpAmount; @@ -1452,6 +1461,12 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { log.debug('Single-input fee/Multi-input fee: ' + formatRatio(singleInputFeeVsFeeRatio) + ' (max: ' + formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) + ')' + ' loses wrt single-input tx: ' + formatAmount((selected.length - 1) * feePerInput)); log.debug('Tx amount/input amount:' + formatRatio(amountVsUtxoRatio) + ' (min: ' + formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); + if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { + log.debug('Breaking because tx size (' + formatSize(txpSize) + ') is too big (max: ' + formatSize(Defaults.MAX_TX_SIZE_IN_KB * 1000.) + ')'); // TODO + error = Errors.TX_MAX_SIZE_EXCEEDED; + return false; + } + if (!_.isEmpty(bigInputs)) { if ((amountVsFeeRatio > Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR && singleInputFeeVsFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { @@ -1463,12 +1478,6 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { log.debug('Breaking because utxo is too small compared to tx amount'); return false; } - - if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { - log.debug('Breaking because tx size is too big (' + formatSize(txpSize) + ')'); // TODO - - return false; - } } total += inputAmount; @@ -1489,11 +1498,9 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { } } - if (!_.isEmpty(selected)) { - var lockedOverhead = total - txpAmount; - log.debug('SUCCESS! Total locked: ' + formatAmount(total) + ', overhead: ' + formatAmount(lockedOverhead) + ' (' + formatRatio(lockedOverhead / txpAmount) + ')'); - } else { - log.debug('Could not find enough funds within this utxso subset'); + if (_.isEmpty(selected)) { + log.debug('Could not find enough funds within this utxo subset'); + return error || Errors.INSUFFICIENT_FUNDS_FOR_FEE; } return selected; @@ -1531,6 +1538,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var inputs = []; var groups = [6, 1, 0]; + var error; var lastGroupLength; _.each(groups, function(group) { var candidateUtxos = _.filter(utxos, function(utxo) { @@ -1549,20 +1557,25 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { lastGroupLength = candidateUtxos.length; - inputs = select(candidateUtxos); + var result = select(candidateUtxos); + if (result && !_.isArray(result)) { + error = result; + } else { + inputs = result; + error = null; + } log.debug('Selected inputs from this group: ' + formatInputs(inputs)); if (!_.isEmpty(inputs)) return false; }); - if (_.isEmpty(inputs)) return cb(Errors.INSUFFICIENT_FUNDS_FOR_FEE); + if (error) return cb(error); txp.setInputs(inputs); - if (txp.getEstimatedSize() / 1000 > Defaults.MAX_TX_SIZE_IN_KB) - return cb(Errors.TX_MAX_SIZE_EXCEEDED); var err = self._checkTxAndEstimateFee(txp); + if (!err) { log.debug('Successfully built transaction. Total fees: ', formatAmount(txp.fee)); } else { diff --git a/test/integration/server.js b/test/integration/server.js index 2dded8c..507d10d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2132,6 +2132,7 @@ describe('Wallet service', function() { }); it('should use unconfirmed utxos only when no more confirmed utxos are available', function(done) { + // log.level = 'debug'; helpers.stubUtxos(server, wallet, [1.3, 'u0.5', 'u0.1', 1.2], function(utxos) { var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 2.55, TestData.copayers[0].privKey_1H_0, { message: 'some message' @@ -2354,10 +2355,11 @@ describe('Wallet service', function() { }); it('should fail to create a tx exceeding max size in kb', function(done) { + log.level = 'debug'; + var _oldDefault = Defaults.MAX_TX_SIZE_IN_KB; + Defaults.MAX_TX_SIZE_IN_KB = 1; helpers.stubUtxos(server, wallet, _.range(1, 10, 0), function() { var txOpts = helpers.createSimpleProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 8, TestData.copayers[0].privKey_1H_0); - var _oldDefault = Defaults.MAX_TX_SIZE_IN_KB; - Defaults.MAX_TX_SIZE_IN_KB = 1; server.createTxLegacy(txOpts, function(err, tx) { should.exist(err); err.code.should.equal('TX_MAX_SIZE_EXCEEDED'); From 60f6300fbce4b15b42aae14e545891d4c8a8aa81 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Mar 2016 17:53:16 -0300 Subject: [PATCH 11/32] rebased --- lib/server.js | 7 +++---- test/integration/server.js | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/server.js b/lib/server.js index 7d4e569..4240776 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1398,7 +1398,6 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { }; function select(utxos) { - // TODO: fix for when fee is specified instead of feePerKb var txpAmount = txp.getTotalAmount(); var baseTxpSize = txp.getEstimatedSize(); var baseTxpFee = baseTxpSize * txp.feePerKb / 1000.; @@ -1408,11 +1407,11 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var totalValueInUtxos = _.sum(utxos, 'satoshis'); var netValueInUtxos = totalValueInUtxos - baseTxpFee - (utxos.length * feePerInput); if (totalValueInUtxos < txpAmount) { - log.debug('Total value in all utxos (' + formatAmount(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); // TODO + log.debug('Total value in all utxos (' + formatAmount(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); return Errors.INSUFFICIENT_FUNDS; } if (netValueInUtxos < txpAmount) { - log.debug('Value after fees in all utxos (' + formatAmount(netValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); // TODO + log.debug('Value after fees in all utxos (' + formatAmount(netValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); return Errors.INSUFFICIENT_FUNDS_FOR_FEE; } @@ -1462,7 +1461,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { log.debug('Tx amount/input amount:' + formatRatio(amountVsUtxoRatio) + ' (min: ' + formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { - log.debug('Breaking because tx size (' + formatSize(txpSize) + ') is too big (max: ' + formatSize(Defaults.MAX_TX_SIZE_IN_KB * 1000.) + ')'); // TODO + log.debug('Breaking because tx size (' + formatSize(txpSize) + ') is too big (max: ' + formatSize(Defaults.MAX_TX_SIZE_IN_KB * 1000.) + ')'); error = Errors.TX_MAX_SIZE_EXCEEDED; return false; } diff --git a/test/integration/server.js b/test/integration/server.js index 507d10d..0b05fdb 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2355,7 +2355,7 @@ describe('Wallet service', function() { }); it('should fail to create a tx exceeding max size in kb', function(done) { - log.level = 'debug'; + // log.level = 'debug'; var _oldDefault = Defaults.MAX_TX_SIZE_IN_KB; Defaults.MAX_TX_SIZE_IN_KB = 1; helpers.stubUtxos(server, wallet, _.range(1, 10, 0), function() { @@ -3186,8 +3186,7 @@ describe('Wallet service', function() { toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', amount: 0.8 * 1e8, }], - message: 'some message', - customData: 'some custom data', + feePerKb: 100e2, }; server.createTx(txOpts, function(err, tx) { should.not.exist(err); From d3faad0639b4cd83a8d436a084c8eefcacb30e85 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 7 Mar 2016 12:30:10 -0300 Subject: [PATCH 12/32] make select fn async --- lib/server.js | 72 +++--- test/integration/server.js | 509 +++++++++++++++++-------------------- 2 files changed, 277 insertions(+), 304 deletions(-) diff --git a/lib/server.js b/lib/server.js index 4240776..cc78fa0 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1397,7 +1397,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return (size / 1000.).toFixed(4) + 'kB'; }; - function select(utxos) { + function select(utxos, cb) { var txpAmount = txp.getTotalAmount(); var baseTxpSize = txp.getEstimatedSize(); var baseTxpFee = baseTxpSize * txp.feePerKb / 1000.; @@ -1408,11 +1408,11 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var netValueInUtxos = totalValueInUtxos - baseTxpFee - (utxos.length * feePerInput); if (totalValueInUtxos < txpAmount) { log.debug('Total value in all utxos (' + formatAmount(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); - return Errors.INSUFFICIENT_FUNDS; + return cb(Errors.INSUFFICIENT_FUNDS); } if (netValueInUtxos < txpAmount) { log.debug('Value after fees in all utxos (' + formatAmount(netValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); - return Errors.INSUFFICIENT_FUNDS_FOR_FEE; + return cb(Errors.INSUFFICIENT_FUNDS_FOR_FEE); } var bigInputThreshold = txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR + (baseTxpFee + feePerInput); @@ -1499,10 +1499,10 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { if (_.isEmpty(selected)) { log.debug('Could not find enough funds within this utxo subset'); - return error || Errors.INSUFFICIENT_FUNDS_FOR_FEE; + return cb(error || Errors.INSUFFICIENT_FUNDS_FOR_FEE); } - return selected; + return cb(null, selected); }; log.debug('Selecting inputs for a ' + formatAmount(txp.getTotalAmount()) + ' txp'); @@ -1535,11 +1535,18 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { log.debug('Considering ' + utxos.length + ' utxos (' + formatInputs(utxos) + ')'); + var groups = [6, 1]; + if (!txp.excludeUnconfirmedUtxos) groups.push(0); + var inputs = []; - var groups = [6, 1, 0]; - var error; + var selectionError; + var i = 0; var lastGroupLength; - _.each(groups, function(group) { + async.whilst(function() { + return i < groups.length && _.isEmpty(inputs); + }, function(next) { + var group = groups[i++]; + var candidateUtxos = _.filter(utxos, function(utxo) { return utxo.confirmations >= group; }); @@ -1549,39 +1556,42 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { // If this group does not have any new elements, skip it if (lastGroupLength === candidateUtxos.length) { log.debug('This group is identical to the one already explored'); - return; + return next(); } log.debug('Candidate utxos: ' + formatInputs(candidateUtxos)); lastGroupLength = candidateUtxos.length; - var result = select(candidateUtxos); - if (result && !_.isArray(result)) { - error = result; + select(candidateUtxos, function(err, selected) { + if (err) { + log.debug('No inputs selected on this group: ', err); + selectionError = err; + return next(); + } + + selectionError = null; + inputs = selected; + + log.debug('Selected inputs from this group: ' + formatInputs(inputs)); + return next(); + }); + }, function(err) { + if (err) return cb(err); + if (selectionError || _.isEmpty(inputs)) return cb(selectionError || new Error('Could not select tx inputs')); + + txp.setInputs(inputs); + + var err = self._checkTxAndEstimateFee(txp); + + if (!err) { + log.debug('Successfully built transaction. Total fees: ', formatAmount(txp.fee)); } else { - inputs = result; - error = null; + log.warn('Error building transaction', err); } - log.debug('Selected inputs from this group: ' + formatInputs(inputs)); - - if (!_.isEmpty(inputs)) return false; + return cb(err); }); - - if (error) return cb(error); - - txp.setInputs(inputs); - - var err = self._checkTxAndEstimateFee(txp); - - if (!err) { - log.debug('Successfully built transaction. Total fees: ', formatAmount(txp.fee)); - } else { - log.warn('Error building transaction', err); - } - - return cb(err); }); }; diff --git a/test/integration/server.js b/test/integration/server.js index 0b05fdb..3cb0053 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3169,7 +3169,7 @@ describe('Wallet service', function() { }); }); - describe('UTXO selection', function() { + describe('UTXO Selection', function() { var server, wallet; beforeEach(function(done) { helpers.createAndJoinWallet(2, 3, function(s, w) { @@ -3179,32 +3179,248 @@ describe('Wallet service', function() { }); }); - it('should create a tx', function(done) { - helpers.stubUtxos(server, wallet, [1, 2], function() { + 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() { var txOpts = { outputs: [{ toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 0.8 * 1e8, + amount: 200e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + console.log('*** [server.js ln3193] err:', err); // TODO + + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(35000); + + done(); + }); + }); + }); + it('should select smaller utxos if within fee constraints', function(done) { + helpers.stubUtxos(server, wallet, [1, '800bit', '800bit', '800bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 2000e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(3); + _.all(txp.inputs, function(input) { + return input == 100e2; + }); + done(); + }); + }); + }); + it('should select smallest big utxo if small utxos are insufficient', function(done) { + helpers.stubUtxos(server, wallet, [3, 1, 2, '100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 300e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + done(); + }); + }); + }); + it('should account for fee when selecting smallest big utxo', function(done) { + // log.level = 'debug'; + var _old = Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; + Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; + // The 605 bits input cannot be selected even if it is > 2 * tx amount + // because it cannot cover for fee on its own. + helpers.stubUtxos(server, wallet, [1, '605bit', '100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 300e2, + }], + feePerKb: 1200e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = _old; + done(); + }); + }); + }); + it('should select smallest big utxo if small utxos exceed maximum fee', function(done) { + helpers.stubUtxos(server, wallet, [3, 1, 2].concat(_.times(20, function() { + return '1000bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 12000e2, + }], + feePerKb: 20e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + + done(); + }); + }); + }); + it('should select smallest big utxo if small utxos are below accepted ratio of txp amount', function(done) { + helpers.stubUtxos(server, wallet, [9, 1, 1, 0.5, 0.2, 0.2, 0.2], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 3e8, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(9e8); + done(); + }); + }); + }); + it('should not fail with tx exceeded max size if there is at least 1 big input', function(done) { + var _old1 = Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR; + var _old2 = Defaults.MAX_TX_SIZE_IN_KB; + Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = 0.0001; + Defaults.MAX_TX_SIZE_IN_KB = 3; + + helpers.stubUtxos(server, wallet, [100].concat(_.range(1, 20, 0)), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 15e8, + }], + feePerKb: 120e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(100e8); + Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = _old1; + Defaults.MAX_TX_SIZE_IN_KB = _old2; + done(); + }); + }); + }); + it('should ignore utxos not contributing enough to cover increase in fee', function(done) { + helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 200e2, + }], + feePerKb: 80e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(3); + txOpts.feePerKb = 120e2; + server.createTx(txOpts, function(err, txp) { + should.exist(err); + should.not.exist(txp); + done(); + }); + }); + }); + }); + it('should fail to select utxos if not enough to cover tx amount', function(done) { + helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 400e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.exist(err); + should.not.exist(txp); + err.code.should.equal('INSUFFICIENT_FUNDS'); + done(); + }); + }); + }); + it('should fail to select utxos if not enough to cover fees', function(done) { + helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 299e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.exist(err); + should.not.exist(txp); + err.code.should.equal('INSUFFICIENT_FUNDS_FOR_FEE'); + done(); + }); + }); + }); + it('should prefer a higher fee (breaking all limits) if inputs have 6+ confirmations', function(done) { + helpers.stubUtxos(server, wallet, ['2c 2000bit'].concat(_.times(20, function() { + return '100bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 1500e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + _.all(txp.inputs, function(input) { + return input == 100e2; + }); + done(); + }); + }); + }); + it('should select unconfirmed utxos if not enough confirmed utxos', function(done) { + // log.level = 'debug'; + helpers.stubUtxos(server, wallet, ['u 1btc', '0.5btc'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 0.8e8, }], feePerKb: 100e2, }; - server.createTx(txOpts, function(err, tx) { + server.createTx(txOpts, function(err, txp) { + console.log('*** [server.js ln3417] err:', err); // TODO + should.not.exist(err); - should.exist(tx); - tx.walletM.should.equal(2); - tx.walletN.should.equal(3); - tx.requiredRejections.should.equal(2); - tx.requiredSignatures.should.equal(2); - tx.isAccepted().should.equal.false; - tx.isRejected().should.equal.false; - tx.isPending().should.equal.true; - tx.isTemporary().should.equal.true; - tx.amount.should.equal(helpers.toSatoshi(0.8)); - server.getPendingTxs({}, function(err, txs) { - should.not.exist(err); - txs.should.be.empty; - done(); - }); + should.exist(txp); + txp.inputs.length.should.equal(1); + txp.inputs[0].satoshis.should.equal(1e8); + done(); }); }); }); @@ -5731,257 +5947,4 @@ describe('Wallet service', function() { }); }); }); - - describe('UTXO Selection', function() { - var server, wallet; - beforeEach(function(done) { - helpers.createAndJoinWallet(2, 3, function(s, w) { - server = s; - wallet = w; - done(); - }); - }); - - 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() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 200e2, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(35000); - - done(); - }); - }); - }); - it('should select smaller utxos if within fee constraints', function(done) { - helpers.stubUtxos(server, wallet, [1, '800bit', '800bit', '800bit'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 2000e2, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(3); - _.all(txp.inputs, function(input) { - return input == 100e2; - }); - done(); - }); - }); - }); - it('should select smallest big utxo if small utxos are insufficient', function(done) { - helpers.stubUtxos(server, wallet, [3, 1, 2, '100bit', '100bit', '100bit'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 300e2, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(1e8); - done(); - }); - }); - }); - it('should account for fee when selecting smallest big utxo', function(done) { - // log.level = 'debug'; - var _old = Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR; - Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; - // The 605 bits input cannot be selected even if it is > 2 * tx amount - // because it cannot cover for fee on its own. - helpers.stubUtxos(server, wallet, [1, '605bit', '100bit', '100bit', '100bit'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 300e2, - }], - feePerKb: 1200e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(1e8); - Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = _old; - done(); - }); - }); - }); - it('should select smallest big utxo if small utxos exceed maximum fee', function(done) { - helpers.stubUtxos(server, wallet, [3, 1, 2].concat(_.times(20, function() { - return '1000bit'; - })), function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 12000e2, - }], - feePerKb: 20e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(1e8); - - done(); - }); - }); - }); - it('should select smallest big utxo if small utxos are below accepted ratio of txp amount', function(done) { - helpers.stubUtxos(server, wallet, [9, 1, 1, 0.5, 0.2, 0.2, 0.2], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 3e8, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(9e8); - done(); - }); - }); - }); - it('should not fail with tx exceeded max size if there is at least 1 big input', function(done) { - var _old1 = Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR; - var _old2 = Defaults.MAX_TX_SIZE_IN_KB; - Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = 0.0001; - Defaults.MAX_TX_SIZE_IN_KB = 3; - - helpers.stubUtxos(server, wallet, [100].concat(_.range(1, 20, 0)), function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 15e8, - }], - feePerKb: 120e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(100e8); - Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = _old1; - Defaults.MAX_TX_SIZE_IN_KB = _old2; - done(); - }); - }); - }); - it('should ignore utxos not contributing enough to cover increase in fee', function(done) { - helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 200e2, - }], - feePerKb: 80e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(3); - txOpts.feePerKb = 120e2; - server.createTx(txOpts, function(err, txp) { - should.exist(err); - should.not.exist(txp); - done(); - }); - }); - }); - }); - it('should fail to select utxos if not enough to cover tx amount', function(done) { - helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 400e2, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.exist(err); - should.not.exist(txp); - err.code.should.equal('INSUFFICIENT_FUNDS'); - done(); - }); - }); - }); - it('should fail to select utxos if not enough to cover fees', function(done) { - helpers.stubUtxos(server, wallet, ['100bit', '100bit', '100bit'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 299e2, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.exist(err); - should.not.exist(txp); - err.code.should.equal('INSUFFICIENT_FUNDS_FOR_FEE'); - done(); - }); - }); - }); - it('should prefer a higher fee (breaking all limits) if inputs have 6+ confirmations', function(done) { - helpers.stubUtxos(server, wallet, ['2c 2000bit'].concat(_.times(20, function() { - return '100bit'; - })), function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 1500e2, - }], - feePerKb: 10e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - _.all(txp.inputs, function(input) { - return input == 100e2; - }); - done(); - }); - }); - }); - it('should select unconfirmed utxos if not enough confirmed utxos', function(done) { - helpers.stubUtxos(server, wallet, ['u 1btc', '0.5btc'], function() { - var txOpts = { - outputs: [{ - toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', - amount: 0.8e8, - }], - feePerKb: 100e2, - }; - server.createTx(txOpts, function(err, txp) { - should.not.exist(err); - should.exist(txp); - txp.inputs.length.should.equal(1); - txp.inputs[0].satoshis.should.equal(1e8); - done(); - }); - }); - }); - }); - }); From 504b52d69598c18e787577f57c661fbc148f6628 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 7 Mar 2016 13:00:53 -0300 Subject: [PATCH 13/32] compare both utxo selection algorithms --- lib/common/utils.js | 20 ++++++++ lib/server.js | 102 ++++++++++++++++++++----------------- test/integration/server.js | 4 +- 3 files changed, 76 insertions(+), 50 deletions(-) diff --git a/lib/common/utils.js b/lib/common/utils.js index 01f1420..bd749fd 100644 --- a/lib/common/utils.js +++ b/lib/common/utils.js @@ -93,5 +93,25 @@ Utils.formatAmount = function(satoshis, unit, opts) { return addSeparators(amount, opts.thousandsSeparator || ',', opts.decimalSeparator || '.', u.minDecimals); }; +Utils.formatAmountInBtc = function(amount) { + return Utils.formatAmount(amount, 'btc') + 'btc'; +}; + +Utils.formatUtxos = function(utxos) { + if (_.isEmpty(utxos)) return 'none'; + return _.map([].concat(utxos), function(i) { + var amount = Utils.formatAmountInBtc(i.satoshis); + var confirmations = i.confirmations ? i.confirmations + 'c' : 'u'; + return amount + '/' + confirmations; + }).join(', '); +}; + +Utils.formatRatio = function(ratio) { + return (ratio * 100.).toFixed(4) + '%'; +}; + +Utils.formatSize = function(size) { + return (size / 1000.).toFixed(4) + 'kB'; +}; module.exports = Utils; diff --git a/lib/server.js b/lib/server.js index cc78fa0..6b7abeb 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1376,27 +1376,6 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { }); }; - function formatAmount(amount) { - return Utils.formatAmount(amount, 'btc') + 'btc'; - }; - - function formatInputs(inputs) { - if (_.isEmpty(inputs)) return 'none'; - return _.map([].concat(inputs), function(i) { - var amount = formatAmount(i.satoshis); - var confirmations = i.confirmations ? i.confirmations + 'c' : 'u'; - return amount + '/' + confirmations; - }).join(', '); - }; - - function formatRatio(ratio) { - return (ratio * 100.).toFixed(4) + '%'; - }; - - function formatSize(size) { - return (size / 1000.).toFixed(4) + 'kB'; - }; - function select(utxos, cb) { var txpAmount = txp.getTotalAmount(); var baseTxpSize = txp.getEstimatedSize(); @@ -1407,16 +1386,16 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var totalValueInUtxos = _.sum(utxos, 'satoshis'); var netValueInUtxos = totalValueInUtxos - baseTxpFee - (utxos.length * feePerInput); if (totalValueInUtxos < txpAmount) { - log.debug('Total value in all utxos (' + formatAmount(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); + log.debug('Total value in all utxos (' + Utils.formatAmountInBtc(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + Utils.formatAmountInBtc(txpAmount) + ')'); return cb(Errors.INSUFFICIENT_FUNDS); } if (netValueInUtxos < txpAmount) { - log.debug('Value after fees in all utxos (' + formatAmount(netValueInUtxos) + ') is insufficient to cover for txp amount (' + formatAmount(txpAmount) + ')'); + log.debug('Value after fees in all utxos (' + Utils.formatAmountInBtc(netValueInUtxos) + ') is insufficient to cover for txp amount (' + Utils.formatAmountInBtc(txpAmount) + ')'); return cb(Errors.INSUFFICIENT_FUNDS_FOR_FEE); } var bigInputThreshold = txpAmount * Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR + (baseTxpFee + feePerInput); - log.debug('Big input threshold ' + formatAmount(bigInputThreshold)); + log.debug('Big input threshold ' + Utils.formatAmountInBtc(bigInputThreshold)); var partitions = _.partition(utxos, function(utxo) { return utxo.satoshis > bigInputThreshold; @@ -1427,41 +1406,41 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return -utxo.satoshis; }); - log.debug('Considering ' + bigInputs.length + ' big inputs (' + formatInputs(bigInputs) + ')'); - log.debug('Considering ' + smallInputs.length + ' small inputs (' + formatInputs(smallInputs) + ')'); + log.debug('Considering ' + bigInputs.length + ' big inputs (' + Utils.formatUtxos(bigInputs) + ')'); + log.debug('Considering ' + smallInputs.length + ' small inputs (' + Utils.formatUtxos(smallInputs) + ')'); var total = 0; var selected = []; var error; _.each(smallInputs, function(input, i) { - log.debug('Input #' + i + ': ' + formatInputs(input)); + log.debug('Input #' + i + ': ' + Utils.formatUtxos(input)); if (input.satoshis < feePerInput) { - log.debug('The input does not cover the extra fees (' + formatAmount(feePerInput) + ')'); + log.debug('The input does not cover the extra fees (' + Utils.formatAmountInBtc(feePerInput) + ')'); return false; } var inputAmount = input.satoshis - feePerInput; - log.debug('The input contributes ' + formatAmount(inputAmount)); + log.debug('The input contributes ' + Utils.formatAmountInBtc(inputAmount)); selected.push(input); var txpSize = baseTxpSize + selected.length * sizePerInput; var txpFee = baseTxpFee + selected.length * feePerInput; - log.debug('Tx size: ' + formatSize(txpSize) + ', Tx fee: ' + formatAmount(txpFee)); + log.debug('Tx size: ' + Utils.formatSize(txpSize) + ', Tx fee: ' + Utils.formatAmountInBtc(txpFee)); var amountVsFeeRatio = txpFee / txpAmount; var singleInputFeeVsFeeRatio = txpFee / (baseTxpFee + feePerInput); var amountVsUtxoRatio = inputAmount / txpAmount; - log.debug('Tx amount/Fee: ' + formatRatio(amountVsFeeRatio) + ' (max: ' + formatRatio(Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR) + ')'); - log.debug('Single-input fee/Multi-input fee: ' + formatRatio(singleInputFeeVsFeeRatio) + ' (max: ' + formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) + ')' + ' loses wrt single-input tx: ' + formatAmount((selected.length - 1) * feePerInput)); - log.debug('Tx amount/input amount:' + formatRatio(amountVsUtxoRatio) + ' (min: ' + formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); + log.debug('Tx amount/Fee: ' + Utils.formatRatio(amountVsFeeRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR) + ')'); + log.debug('Single-input fee/Multi-input fee: ' + Utils.formatRatio(singleInputFeeVsFeeRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) + ')' + ' loses wrt single-input tx: ' + Utils.formatAmountInBtc((selected.length - 1) * feePerInput)); + log.debug('Tx amount/input amount:' + Utils.formatRatio(amountVsUtxoRatio) + ' (min: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { - log.debug('Breaking because tx size (' + formatSize(txpSize) + ') is too big (max: ' + formatSize(Defaults.MAX_TX_SIZE_IN_KB * 1000.) + ')'); + log.debug('Breaking because tx size (' + Utils.formatSize(txpSize) + ') is too big (max: ' + Utils.formatSize(Defaults.MAX_TX_SIZE_IN_KB * 1000.) + ')'); error = Errors.TX_MAX_SIZE_EXCEEDED; return false; } @@ -1480,17 +1459,17 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { } total += inputAmount; - log.debug('Cumuled total so far: ' + formatAmount(total)); + log.debug('Cumuled total so far: ' + Utils.formatAmountInBtc(total)); if (total >= txpAmount) return false; }); if (total < txpAmount) { - log.debug('Could not reach Txp total (' + formatAmount(txpAmount) + '), still missing: ' + formatAmount(txpAmount - total)); + log.debug('Could not reach Txp total (' + Utils.formatAmountInBtc(txpAmount) + '), still missing: ' + Utils.formatAmountInBtc(txpAmount - total)); selected = []; if (!_.isEmpty(bigInputs)) { - log.debug('Using big input: ', formatInputs(_.first(bigInputs))); + log.debug('Using big input: ', Utils.formatUtxos(_.first(bigInputs))); var input = _.first(bigInputs); total = input.satoshis; selected = [input]; @@ -1505,7 +1484,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return cb(null, selected); }; - log.debug('Selecting inputs for a ' + formatAmount(txp.getTotalAmount()) + ' txp'); + log.debug('Selecting inputs for a ' + Utils.formatAmountInBtc(txp.getTotalAmount()) + ' txp'); self._getUtxosForCurrentWallet(null, function(err, utxos) { if (err) return cb(err); @@ -1533,7 +1512,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { utxos = _.filter(utxos, 'confirmations'); } - log.debug('Considering ' + utxos.length + ' utxos (' + formatInputs(utxos) + ')'); + log.debug('Considering ' + utxos.length + ' utxos (' + Utils.formatUtxos(utxos) + ')'); var groups = [6, 1]; if (!txp.excludeUnconfirmedUtxos) groups.push(0); @@ -1559,7 +1538,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { return next(); } - log.debug('Candidate utxos: ' + formatInputs(candidateUtxos)); + log.debug('Candidate utxos: ' + Utils.formatUtxos(candidateUtxos)); lastGroupLength = candidateUtxos.length; @@ -1573,7 +1552,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { selectionError = null; inputs = selected; - log.debug('Selected inputs from this group: ' + formatInputs(inputs)); + log.debug('Selected inputs from this group: ' + Utils.formatUtxos(inputs)); return next(); }); }, function(err) { @@ -1585,7 +1564,7 @@ WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { var err = self._checkTxAndEstimateFee(txp); if (!err) { - log.debug('Successfully built transaction. Total fees: ', formatAmount(txp.fee)); + log.debug('Successfully built transaction. Total fees: ', Utils.formatAmountInBtc(txp.fee)); } else { log.warn('Error building transaction', err); } @@ -1775,7 +1754,7 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { txp.version = '1.0.1'; } - self._selectTxInputs2(txp, opts.utxosToExclude, function(err) { + self._selectTxInputs(txp, opts.utxosToExclude, function(err) { if (err) return cb(err); $.checkState(txp.inputs); @@ -1817,6 +1796,22 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { WalletService.prototype.createTx = function(opts, cb) { var self = this; + function logStatistics(prefix, txp) { + var totalAmount = txp.getTotalAmount(); + var inputs = _.sortBy(_.map(txp.inputs, function(input) { + return _.pick(input, 'satoshis', 'confirmations'); + }), 'satoshis'); + var totalLocked = _.sum(inputs, 'satoshis'); + var overhead = totalLocked - totalAmount; + + log.info(prefix, 'TXP ID: ' + txp.id); + log.info(prefix, 'Total amount: ' + Utils.formatAmountInBtc(totalAmount)); + log.info(prefix, 'Fee: ' + Utils.formatAmountInBtc(txp.fee) + ' (per KB: ' + Utils.formatAmountInBtc(txp.feePerKb) + ')'); + log.info(prefix, 'Exclude unconfirmed: ' + txp.excludeUnconfirmedUtxos); + log.info(prefix, 'Total locked: ' + Utils.formatAmountInBtc(totalLocked) + ', Overhead: ' + Utils.formatAmountInBtc(overhead) + ' (' + Utils.formatRatio(overhead / totalAmount) + ')'); + log.info(prefix, 'Inputs: ', Utils.formatUtxos(inputs)); + }; + if (!Utils.checkRequired(opts, ['outputs', 'feePerKb'])) return cb(new ClientError('Required argument missing')); @@ -1859,14 +1854,27 @@ WalletService.prototype.createTx = function(opts, cb) { var txp = Model.TxProposal.create(txOpts); self._selectTxInputs2(txp, opts.utxosToExclude, function(err) { - if (err) return cb(err); + if (err) { + log.error('Could not select inputs using new algorithm', err); + } else { + logStatistics('NEW_UTXO_SEL', txp); + } - self.storage.storeAddressAndWallet(wallet, txp.changeAddress, function(err) { + txp.setInputs([]); + txp.fee = null; + + self._selectTxInputs(txp, opts.utxosToExclude, function(err) { if (err) return cb(err); - self.storage.storeTx(wallet.id, txp, function(err) { + logStatistics('OLD_UTXO_SEL', txp); + + self.storage.storeAddressAndWallet(wallet, txp.changeAddress, function(err) { if (err) return cb(err); - return cb(null, txp); + + self.storage.storeTx(wallet.id, txp, function(err) { + if (err) return cb(err); + return cb(null, txp); + }); }); }); }); diff --git a/test/integration/server.js b/test/integration/server.js index 3cb0053..0121975 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3169,7 +3169,7 @@ describe('Wallet service', function() { }); }); - describe('UTXO Selection', function() { + describe.skip('UTXO Selection', function() { var server, wallet; beforeEach(function(done) { helpers.createAndJoinWallet(2, 3, function(s, w) { @@ -3189,8 +3189,6 @@ describe('Wallet service', function() { feePerKb: 10e2, }; server.createTx(txOpts, function(err, txp) { - console.log('*** [server.js ln3193] err:', err); // TODO - should.not.exist(err); should.exist(txp); txp.inputs.length.should.equal(1); From 680516da70328a8008daf66c318a97487b006370 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 7 Mar 2016 13:04:18 -0300 Subject: [PATCH 14/32] replace old utxo selection algo --- lib/server.js | 123 ++----------------------------------- test/integration/server.js | 2 +- 2 files changed, 6 insertions(+), 119 deletions(-) diff --git a/lib/server.js b/lib/server.js index 6b7abeb..be9f80a 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1267,90 +1267,6 @@ WalletService.prototype._checkTxAndEstimateFee = function(txp) { WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var self = this; - //todo: check inputs are ours and has enough value - if (txp.inputs && txp.inputs.length > 0) { - return cb(self._checkTxAndEstimateFee(txp)); - } - - function sortUtxos(utxos) { - var list = _.map(utxos, function(utxo) { - var order; - if (utxo.confirmations == 0) { - order = 0; - } else if (utxo.confirmations < 6) { - order = -1; - } else { - order = -2; - } - return { - order: order, - utxo: utxo - }; - }); - return _.pluck(_.sortBy(list, 'order'), 'utxo'); - }; - - self._getUtxosForCurrentWallet(null, function(err, utxos) { - if (err) return cb(err); - - var excludeIndex = _.reduce(utxosToExclude, function(res, val) { - res[val] = val; - return res; - }, {}); - - utxos = _.reject(utxos, function(utxo) { - return excludeIndex[utxo.txid + ":" + utxo.vout]; - }); - - var totalAmount; - var availableAmount; - - var balance = self._totalizeUtxos(utxos); - if (txp.excludeUnconfirmedUtxos) { - totalAmount = balance.totalConfirmedAmount; - availableAmount = balance.availableConfirmedAmount; - } else { - totalAmount = balance.totalAmount; - availableAmount = balance.availableAmount; - } - - if (totalAmount < txp.getTotalAmount()) return cb(Errors.INSUFFICIENT_FUNDS); - if (availableAmount < txp.getTotalAmount()) return cb(Errors.LOCKED_FUNDS); - - // Prepare UTXOs list - utxos = _.reject(utxos, 'locked'); - if (txp.excludeUnconfirmedUtxos) { - utxos = _.filter(utxos, 'confirmations'); - } - - var i = 0; - var total = 0; - var selected = []; - var inputs = sortUtxos(utxos); - - var bitcoreTx, bitcoreError; - - function select() { - if (i >= inputs.length) return cb(bitcoreError || new Error('Could not select tx inputs')); - - var input = inputs[i++]; - selected.push(input); - total += input.satoshis; - if (total >= txp.getTotalAmount()) { - txp.setInputs(selected); - bitcoreError = self._checkTxAndEstimateFee(txp); - if (!bitcoreError) return cb(); - } - setTimeout(select, 0); - }; - - select(); - }); -}; - - -WalletService.prototype._selectTxInputs2 = function(txp, utxosToExclude, cb) { - var self = this; //todo: check inputs are ours and has enough value if (txp.inputs && txp.inputs.length > 0) { @@ -1796,22 +1712,6 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { WalletService.prototype.createTx = function(opts, cb) { var self = this; - function logStatistics(prefix, txp) { - var totalAmount = txp.getTotalAmount(); - var inputs = _.sortBy(_.map(txp.inputs, function(input) { - return _.pick(input, 'satoshis', 'confirmations'); - }), 'satoshis'); - var totalLocked = _.sum(inputs, 'satoshis'); - var overhead = totalLocked - totalAmount; - - log.info(prefix, 'TXP ID: ' + txp.id); - log.info(prefix, 'Total amount: ' + Utils.formatAmountInBtc(totalAmount)); - log.info(prefix, 'Fee: ' + Utils.formatAmountInBtc(txp.fee) + ' (per KB: ' + Utils.formatAmountInBtc(txp.feePerKb) + ')'); - log.info(prefix, 'Exclude unconfirmed: ' + txp.excludeUnconfirmedUtxos); - log.info(prefix, 'Total locked: ' + Utils.formatAmountInBtc(totalLocked) + ', Overhead: ' + Utils.formatAmountInBtc(overhead) + ' (' + Utils.formatRatio(overhead / totalAmount) + ')'); - log.info(prefix, 'Inputs: ', Utils.formatUtxos(inputs)); - }; - if (!Utils.checkRequired(opts, ['outputs', 'feePerKb'])) return cb(new ClientError('Required argument missing')); @@ -1853,28 +1753,15 @@ WalletService.prototype.createTx = function(opts, cb) { var txp = Model.TxProposal.create(txOpts); - self._selectTxInputs2(txp, opts.utxosToExclude, function(err) { - if (err) { - log.error('Could not select inputs using new algorithm', err); - } else { - logStatistics('NEW_UTXO_SEL', txp); - } + self._selectTxInputs(txp, opts.utxosToExclude, function(err) { + if (err) return cb(err); - txp.setInputs([]); - txp.fee = null; - - self._selectTxInputs(txp, opts.utxosToExclude, function(err) { + self.storage.storeAddressAndWallet(wallet, txp.changeAddress, function(err) { if (err) return cb(err); - logStatistics('OLD_UTXO_SEL', txp); - - self.storage.storeAddressAndWallet(wallet, txp.changeAddress, function(err) { + self.storage.storeTx(wallet.id, txp, function(err) { if (err) return cb(err); - - self.storage.storeTx(wallet.id, txp, function(err) { - if (err) return cb(err); - return cb(null, txp); - }); + return cb(null, txp); }); }); }); diff --git a/test/integration/server.js b/test/integration/server.js index 0121975..e12ddcb 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3169,7 +3169,7 @@ describe('Wallet service', function() { }); }); - describe.skip('UTXO Selection', function() { + describe('UTXO Selection', function() { var server, wallet; beforeEach(function(done) { helpers.createAndJoinWallet(2, 3, function(s, w) { From cbde3233df8a634d976db3c8c29c21a93f1662ae Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 7 Mar 2016 15:18:32 -0300 Subject: [PATCH 15/32] handle lots of small inputs causing the total balance after fees to decrease --- lib/server.js | 28 +++++++++++++++++----------- test/integration/server.js | 21 +++++++++++++++++++-- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/server.js b/lib/server.js index be9f80a..7acdd0f 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1273,6 +1273,12 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { return cb(self._checkTxAndEstimateFee(txp)); } + var txpAmount = txp.getTotalAmount(); + var baseTxpSize = txp.getEstimatedSize(); + var baseTxpFee = baseTxpSize * txp.feePerKb / 1000.; + var sizePerInput = txp.getEstimatedSizeForSingleInput(); + var feePerInput = sizePerInput * txp.feePerKb / 1000.; + function excludeUtxos(utxos) { var excludeIndex = _.reduce(utxosToExclude, function(res, val) { res[val] = val; @@ -1284,6 +1290,15 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { }); }; + function sanitizeUtxos(utxos) { + return _.filter(utxos, function(utxo) { + if (utxo.locked) return false; + if (utxo.satoshis <= feePerInput) return false; + if (txp.excludeUnconfirmedUtxos && !utxo.confirmations) return false; + return true; + }); + }; + function partitionUtxos(utxos) { return _.groupBy(utxos, function(utxo) { if (utxo.confirmations == 0) return '0' @@ -1293,14 +1308,9 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { }; function select(utxos, cb) { - var txpAmount = txp.getTotalAmount(); - var baseTxpSize = txp.getEstimatedSize(); - var baseTxpFee = baseTxpSize * txp.feePerKb / 1000.; - var sizePerInput = txp.getEstimatedSizeForSingleInput(); - var feePerInput = sizePerInput * txp.feePerKb / 1000.; - var totalValueInUtxos = _.sum(utxos, 'satoshis'); var netValueInUtxos = totalValueInUtxos - baseTxpFee - (utxos.length * feePerInput); + if (totalValueInUtxos < txpAmount) { log.debug('Total value in all utxos (' + Utils.formatAmountInBtc(totalValueInUtxos) + ') is insufficient to cover for txp amount (' + Utils.formatAmountInBtc(txpAmount) + ')'); return cb(Errors.INSUFFICIENT_FUNDS); @@ -1422,11 +1432,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { if (totalAmount < txp.getTotalAmount()) return cb(Errors.INSUFFICIENT_FUNDS); if (availableAmount < txp.getTotalAmount()) return cb(Errors.LOCKED_FUNDS); - // Prepare UTXOs list - utxos = _.reject(utxos, 'locked'); - if (txp.excludeUnconfirmedUtxos) { - utxos = _.filter(utxos, 'confirmations'); - } + utxos = sanitizeUtxos(utxos); log.debug('Considering ' + utxos.length + ' utxos (' + Utils.formatUtxos(utxos) + ')'); diff --git a/test/integration/server.js b/test/integration/server.js index e12ddcb..1b78eed 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3412,8 +3412,6 @@ describe('Wallet service', function() { feePerKb: 100e2, }; server.createTx(txOpts, function(err, txp) { - console.log('*** [server.js ln3417] err:', err); // TODO - should.not.exist(err); should.exist(txp); txp.inputs.length.should.equal(1); @@ -3422,6 +3420,25 @@ describe('Wallet service', function() { }); }); }); + it('should ignore utxos too small to pay for fee', function(done) { + helpers.stubUtxos(server, wallet, ['1c200bit', '200bit'].concat(_.times(20, function() { + return '1bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 200e2, + }], + feePerKb: 90e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(2); + done(); + }); + }); + }); }); }); From 08cb603a5097db79c11581fb0edb7a029c5ef9dc Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 7 Mar 2016 15:44:40 -0300 Subject: [PATCH 16/32] merge utxo exclusion into sanitize fn --- lib/server.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/server.js b/lib/server.js index 7acdd0f..c56b490 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1279,22 +1279,17 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var sizePerInput = txp.getEstimatedSizeForSingleInput(); var feePerInput = sizePerInput * txp.feePerKb / 1000.; - function excludeUtxos(utxos) { + function sanitizeUtxos(utxos) { var excludeIndex = _.reduce(utxosToExclude, function(res, val) { res[val] = val; return res; }, {}); - return _.reject(utxos, function(utxo) { - return excludeIndex[utxo.txid + ":" + utxo.vout]; - }); - }; - - function sanitizeUtxos(utxos) { return _.filter(utxos, function(utxo) { if (utxo.locked) return false; if (utxo.satoshis <= feePerInput) return false; if (txp.excludeUnconfirmedUtxos && !utxo.confirmations) return false; + if (excludeIndex[utxo.txid + ":" + utxo.vout]) return false; return true; }); }; @@ -1415,8 +1410,6 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { self._getUtxosForCurrentWallet(null, function(err, utxos) { if (err) return cb(err); - utxos = excludeUtxos(utxos); - var totalAmount; var availableAmount; From 37d27cec40803336556c465020be48bc98212a14 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 7 Mar 2016 09:59:15 -0300 Subject: [PATCH 17/32] create compound index for walletId, createdOn. drop old index --- lib/storage.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/storage.js b/lib/storage.js index c77e3a4..3011cbb 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -50,7 +50,8 @@ Storage.prototype._createIndexes = function() { id: 1, }); this.db.collection(collections.ADDRESSES).createIndex({ - walletId: 1 + walletId: 1, + createdOn: 1, }); this.db.collection(collections.ADDRESSES).createIndex({ address: 1, @@ -63,6 +64,11 @@ Storage.prototype._createIndexes = function() { type: 1, key: 1, }); + + + this.db.collection(collections.ADDRESSES).dropIndex({ + walletId: 1 + }); }; Storage.prototype.connect = function(opts, cb) { From 722a1d4ae79d1566d3d76556220f3215e5894a3d Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Mon, 29 Feb 2016 12:47:30 -0300 Subject: [PATCH 18/32] add version check --- config.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config.js b/config.js index c0479c3..6a7c531 100644 --- a/config.js +++ b/config.js @@ -42,7 +42,8 @@ var config = { }, testnet: { provider: 'insight', - url: 'https://test-insight.bitpay.com:443', + //url: 'https://test-insight.bitpay.com:443', + url: 'http://localhost:3001', // Multiple servers (in priority order) // url: ['http://a.b.c', 'https://test-insight.bitpay.com:443'], }, From 69d099020ff9671ac0b40ca1d7d5cd5194939a11 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Tue, 8 Mar 2016 09:28:34 -0300 Subject: [PATCH 19/32] add tests --- test/integration/server.js | 60 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/integration/server.js b/test/integration/server.js index 1b78eed..4b6f18d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3172,6 +3172,7 @@ describe('Wallet service', function() { describe('UTXO Selection', function() { var server, wallet; beforeEach(function(done) { + log.level = 'debug'; helpers.createAndJoinWallet(2, 3, function(s, w) { server = s; wallet = w; @@ -3198,6 +3199,26 @@ describe('Wallet service', function() { }); }); }); + it('should select a confirmed utxos if within thresholds relative to tx amount', function(done) { + helpers.stubUtxos(server, wallet, [1, 'u 350bit', '100bit', '100bit', '100bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 200e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(3); + txp.inputs[0].satoshis.should.equal(10000); + + done(); + }); + }); + }); + it('should select smaller utxos if within fee constraints', function(done) { helpers.stubUtxos(server, wallet, [1, '800bit', '800bit', '800bit'], function() { var txOpts = { @@ -3439,6 +3460,45 @@ describe('Wallet service', function() { }); }); }); + it('should use small utxos if fee is low', function(done) { + helpers.stubUtxos(server, wallet, [].concat(_.times(10, function() { + return '30bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 200e2, + }], + feePerKb: 10e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + txp.inputs.length.should.equal(8); + done(); + }); + }); + }); + it.only('should ignore small utxos if fee is higher', function(done) { + helpers.stubUtxos(server, wallet, [].concat(_.times(10, function() { + return '30bit'; + })), function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 200e2, + }], + feePerKb: 30e2, + }; + server.createTx(txOpts, function(err, txp) { + err.code.should.equal('INSUFFICIENT_FUNDS_FOR_FEE'); + done(); + }); + }); + }); + + + }); }); From 72cf236bd655427a7782e03f2e6e0614e5c5cde2 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 8 Mar 2016 09:29:57 -0300 Subject: [PATCH 20/32] fix variable names --- lib/common/defaults.js | 2 +- lib/server.js | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/common/defaults.js b/lib/common/defaults.js index a2f5f85..9b0f468 100644 --- a/lib/common/defaults.js +++ b/lib/common/defaults.js @@ -51,7 +51,7 @@ Defaults.HISTORY_LIMIT = 100; Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR = 5; -Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR = 0.002; +Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR = 0.002; Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = 0.1; module.exports = Defaults; diff --git a/lib/server.js b/lib/server.js index c56b490..e77f6e3 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1352,12 +1352,12 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { log.debug('Tx size: ' + Utils.formatSize(txpSize) + ', Tx fee: ' + Utils.formatAmountInBtc(txpFee)); - var amountVsFeeRatio = txpFee / txpAmount; - var singleInputFeeVsFeeRatio = txpFee / (baseTxpFee + feePerInput); + var feeVsAmountRatio = txpFee / txpAmount; + var feeVsSingleInputFeeRatio = txpFee / (baseTxpFee + feePerInput); var amountVsUtxoRatio = inputAmount / txpAmount; - log.debug('Tx amount/Fee: ' + Utils.formatRatio(amountVsFeeRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR) + ')'); - log.debug('Single-input fee/Multi-input fee: ' + Utils.formatRatio(singleInputFeeVsFeeRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) + ')' + ' loses wrt single-input tx: ' + Utils.formatAmountInBtc((selected.length - 1) * feePerInput)); + 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)); log.debug('Tx amount/input amount:' + Utils.formatRatio(amountVsUtxoRatio) + ' (min: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { @@ -1367,8 +1367,8 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { } if (!_.isEmpty(bigInputs)) { - if ((amountVsFeeRatio > Defaults.UTXO_SELECTION_MAX_TX_AMOUNT_VS_FEE_FACTOR && - singleInputFeeVsFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { + if ((feeVsAmountRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR && + feeVsSingleInputFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { log.debug('Breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); return false; } From 9ed48f1e77a1c971c6cd23f625c0cae19440ebc2 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 8 Mar 2016 10:55:32 -0300 Subject: [PATCH 21/32] keep adding utxos to raise change above dust --- lib/common/utils.js | 1 + lib/server.js | 16 +++++++++++++--- test/integration/server.js | 31 ++++++++++++++++++++++++++++--- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/lib/common/utils.js b/lib/common/utils.js index bd749fd..72846bf 100644 --- a/lib/common/utils.js +++ b/lib/common/utils.js @@ -23,6 +23,7 @@ Utils.checkRequired = function(obj, args) { * @return {number} */ Utils.strip = function(number) { + return parseFloat(number); return (parseFloat(number.toPrecision(12))); } diff --git a/lib/server.js b/lib/server.js index e77f6e3..bfdae8c 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1347,6 +1347,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { selected.push(input); + total += inputAmount; var txpSize = baseTxpSize + selected.length * sizePerInput; var txpFee = baseTxpFee + selected.length * feePerInput; @@ -1379,10 +1380,19 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { } } - total += inputAmount; log.debug('Cumuled total so far: ' + Utils.formatAmountInBtc(total)); - if (total >= txpAmount) return false; + 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; + } + + return false; + } }); if (total < txpAmount) { @@ -1479,7 +1489,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var err = self._checkTxAndEstimateFee(txp); if (!err) { - log.debug('Successfully built transaction. Total fees: ', Utils.formatAmountInBtc(txp.fee)); + log.debug('Successfully built transaction. Total fees: ' + Utils.formatAmountInBtc(txp.fee) + ', total change: ' + Utils.formatAmountInBtc(_.sum(txp.inputs, 'satoshis') - txp.fee)); } else { log.warn('Error building transaction', err); } diff --git a/test/integration/server.js b/test/integration/server.js index 4b6f18d..de1d1df 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3199,7 +3199,7 @@ describe('Wallet service', function() { }); }); }); - it('should select a confirmed utxos if within thresholds relative to tx amount', function(done) { + it('should select a confirmed utxos if within thresholds relative to tx amount', function(done) { helpers.stubUtxos(server, wallet, [1, 'u 350bit', '100bit', '100bit', '100bit'], function() { var txOpts = { outputs: [{ @@ -3218,7 +3218,7 @@ describe('Wallet service', function() { }); }); }); - + it('should select smaller utxos if within fee constraints', function(done) { helpers.stubUtxos(server, wallet, [1, '800bit', '800bit', '800bit'], function() { var txOpts = { @@ -3479,7 +3479,32 @@ describe('Wallet service', function() { }); }); }); - it.only('should ignore small utxos if fee is higher', function(done) { + it('should keep adding utxos while change is below dust', function(done) { + helpers.stubUtxos(server, wallet, ['200bit', '500sat'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 200e2, + }], + 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(); + }); + }); + }); + }); + }); + it.skip('should ignore small utxos if fee is higher', function(done) { helpers.stubUtxos(server, wallet, [].concat(_.times(10, function() { return '30bit'; })), function() { From c01d1568bb7fdb1bc638a98888025dbcf8550374 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 8 Mar 2016 15:24:58 -0300 Subject: [PATCH 22/32] adapt fee to avoid change below dust --- lib/common/utils.js | 15 +++++--- lib/model/txproposal.js | 12 +++++-- lib/server.js | 68 +++++++++++++++++++++++-------------- test/integration/helpers.js | 6 ++-- test/integration/server.js | 43 +++++++++++------------ 5 files changed, 85 insertions(+), 59 deletions(-) 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(); }); From 49791bcfdf4b2da14405a00b2845a43c686ed6fa Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 8 Mar 2016 15:28:49 -0300 Subject: [PATCH 23/32] remove redundant check for very small utxos --- lib/server.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/server.js b/lib/server.js index aaa0cfd..67b0824 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1342,11 +1342,6 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { _.each(smallInputs, function(input, i) { log.debug('Input #' + i + ': ' + Utils.formatUtxos(input)); - if (input.satoshis < feePerInput) { - log.debug('The input does not cover the extra fees (' + Utils.formatAmountInBtc(feePerInput) + ')'); - return false; - } - var netInputAmount = input.satoshis - feePerInput; log.debug('The input contributes ' + Utils.formatAmountInBtc(netInputAmount)); From c3ee9e9b93a8164b5eacacb63173ccb264f32ef3 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 8 Mar 2016 15:47:31 -0300 Subject: [PATCH 24/32] shuffle inputs --- lib/server.js | 2 +- test/integration/server.js | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/server.js b/lib/server.js index 67b0824..76bad80 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1496,7 +1496,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { if (err) return cb(err); if (selectionError || _.isEmpty(inputs)) return cb(selectionError || new Error('Could not select tx inputs')); - txp.setInputs(inputs); + txp.setInputs(_.shuffle(inputs)); txp.fee = fee; var err = self._checkTx(txp); diff --git a/test/integration/server.js b/test/integration/server.js index 5eac473..6736d92 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3168,7 +3168,7 @@ describe('Wallet service', function() { }); }); - describe.only('UTXO Selection', function() { + describe('UTXO Selection', function() { var server, wallet; beforeEach(function(done) { // log.level = 'debug'; @@ -3201,6 +3201,29 @@ describe('Wallet service', function() { }); }); }); + it('should return inputs in random order', function(done) { + // NOTE: this test has a chance of failing of 1 in 1'073'741'824 :P + helpers.stubUtxos(server, wallet, _.range(1, 31), function(utxos) { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: _.sum(utxos, 'satoshis') - 0.5e8, + }], + feePerKb: 100e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + var amounts = _.pluck(txp.inputs, 'satoshis'); + amounts.length.should.equal(30); + _.all(amounts, function(amount, i) { + if (i == 0) return true; + return amount < amounts[i - 1]; + }).should.be.false; + done(); + }); + }); + }); it('should select a confirmed utxos if within thresholds relative to tx amount', function(done) { helpers.stubUtxos(server, wallet, [1, 'u 350bit', '100bit', '100bit', '100bit'], function() { var txOpts = { @@ -3492,7 +3515,6 @@ describe('Wallet service', 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.equal(0); done(); }); From 5afc74d4f68d42ed063eb357d0a0b0acfb226b04 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 9 Mar 2016 10:18:13 -0300 Subject: [PATCH 25/32] revert breaking conditions for small inputs to improve tracing --- lib/server.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/server.js b/lib/server.js index 76bad80..2907fea 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1371,14 +1371,14 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { } if (!_.isEmpty(bigInputs)) { - if ((feeVsAmountRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR && - feeVsSingleInputFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { - log.debug('Breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); + if (amountVsUtxoRatio < Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) { + log.debug('Breaking because utxo is too small compared to tx amount'); return false; } - if (amountVsUtxoRatio < Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) { - log.debug('Breaking because utxo is too small compared to tx amount'); + if ((feeVsAmountRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR && + feeVsSingleInputFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { + log.debug('Breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); return false; } } From f976637b1458747a760f9751e5deb0d1955ea8e7 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 9 Mar 2016 10:45:37 -0300 Subject: [PATCH 26/32] improve code readability --- lib/common/defaults.js | 13 +++++++++++-- lib/server.js | 17 +++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/common/defaults.js b/lib/common/defaults.js index 9b0f468..8b8de41 100644 --- a/lib/common/defaults.js +++ b/lib/common/defaults.js @@ -49,9 +49,18 @@ Defaults.FIAT_RATE_MAX_LOOK_BACK_TIME = 120; // In minutes Defaults.HISTORY_LIMIT = 100; +// The maximum amount of an UTXO to be considered too big to be used in the tx before exploring smaller +// alternatives (proportinal to tx amount). Defaults.UTXO_SELECTION_MAX_SINGLE_UTXO_FACTOR = 2; -Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR = 5; -Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR = 0.002; + +// The minimum amount an UTXO need to contribute proportional to tx amount. Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR = 0.1; +// The maximum threshold to consider fees non-significant in relation to tx amount. +Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR = 0.05; + +// The maximum amount to pay for using small inputs instead of one big input +// when fees are significant (proportional to how much we would pay for using that big input only). +Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR = 5; + module.exports = Defaults; diff --git a/lib/server.js b/lib/server.js index 2907fea..e20a1a7 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1357,12 +1357,10 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { log.debug('Tx size: ' + Utils.formatSize(txpSize) + ', Tx fee: ' + Utils.formatAmountInBtc(fee)); 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)); - log.debug('Tx amount/input amount:' + Utils.formatRatio(amountVsUtxoRatio) + ' (min: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); + log.debug('Fee/Tx amount: ' + Utils.formatRatio(feeVsAmountRatio) + ' (max: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR) + ')'); + log.debug('Tx amount/Input amount:' + Utils.formatRatio(amountVsUtxoRatio) + ' (min: ' + Utils.formatRatio(Defaults.UTXO_SELECTION_MIN_TX_AMOUNT_VS_UTXO_FACTOR) + ')'); if (txpSize / 1000. > Defaults.MAX_TX_SIZE_IN_KB) { log.debug('Breaking because tx size (' + Utils.formatSize(txpSize) + ') is too big (max: ' + Utils.formatSize(Defaults.MAX_TX_SIZE_IN_KB * 1000.) + ')'); @@ -1376,10 +1374,13 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { return false; } - if ((feeVsAmountRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR && - feeVsSingleInputFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR)) { - log.debug('Breaking because fee exceeded fee/amount ratio and fee is too expensive compared to single input tx'); - return false; + if (feeVsAmountRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_TX_AMOUNT_FACTOR) { + var feeVsSingleInputFeeRatio = fee / (baseTxpFee + feePerInput); + log.debug('Fee/Single-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)); + if (feeVsSingleInputFeeRatio > Defaults.UTXO_SELECTION_MAX_FEE_VS_SINGLE_UTXO_FEE_FACTOR) { + log.debug('Breaking because fee is too significant compared to tx amount and it is too expensive compared to using single input'); + return false; + } } } From ee7d3bad7fd00d3449705a642daf7bb84a243adf Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Mar 2016 11:47:16 -0300 Subject: [PATCH 27/32] allow absolute fee when specifying inputs --- lib/model/txproposal.js | 6 ++++-- lib/server.js | 27 ++++++++++++++++++--------- test/integration/helpers.js | 1 + test/integration/server.js | 24 ++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 67a3707..f6dd382 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -33,7 +33,6 @@ TxProposal.create = function(opts) { x.message = opts.message; x.payProUrl = opts.payProUrl; x.changeAddress = opts.changeAddress; - x.setInputs(opts.inputs); x.outputs = _.map(opts.outputs, function(output) { return _.pick(output, ['amount', 'toAddress', 'message']); }); @@ -44,7 +43,6 @@ TxProposal.create = function(opts) { x.requiredRejections = Math.min(x.walletM, x.walletN - x.walletM + 1), x.status = 'temporary'; x.actions = []; - x.fee = null; x.feePerKb = opts.feePerKb; x.excludeUnconfirmedUtxos = opts.excludeUnconfirmedUtxos; @@ -59,6 +57,9 @@ TxProposal.create = function(opts) { } catch (ex) {} $.checkState(_.contains(_.values(Constants.NETWORKS), x.network)); + x.setInputs(opts.inputs); + x.fee = opts.fee; + return x; }; @@ -137,6 +138,7 @@ TxProposal.prototype._buildTx = function() { switch (self.addressType) { case Constants.SCRIPT_TYPES.P2SH: _.each(self.inputs, function(i) { + $.checkState(i.publicKeys, 'Inputs should include public keys'); t.from(i, i.publicKeys, self.requiredSignatures); }); break; diff --git a/lib/server.js b/lib/server.js index e20a1a7..d8026aa 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1270,9 +1270,10 @@ WalletService.prototype._checkTx = function(txp) { WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { var self = this; - //todo: check inputs are ours and has enough value - if (txp.inputs && txp.inputs.length > 0) { - self._estimateFee(txp); + //todo: check inputs are ours and have enough value + if (txp.inputs && !_.isEmpty(txp.inputs)) { + if (!_.isNumber(txp.fee)) + self._estimateFee(txp); return cb(self._checkTx(txp)); } @@ -1725,21 +1726,28 @@ WalletService.prototype.createTxLegacy = function(opts, cb) { * @param {number} opts.outputs[].amount - Amount to transfer in satoshi. * @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 - The fee per kB to use for this TX. + * @param {number} opts.feePerKb - The fee per kB to use for this TX. * @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. + * @param {Array} opts.inputs - Optional. Inputs for this TX + * @param {number} opts.fee - Optional. The fee to use for this TX (used only when opts.inputs is specified). * @returns {TxProposal} Transaction proposal. */ WalletService.prototype.createTx = function(opts, cb) { var self = this; - if (!Utils.checkRequired(opts, ['outputs', 'feePerKb'])) + if (!Utils.checkRequired(opts, ['outputs'])) return cb(new ClientError('Required argument missing')); - if (opts.feePerKb < Defaults.MIN_FEE_PER_KB || opts.feePerKb > Defaults.MAX_FEE_PER_KB) - return cb(new ClientError('Invalid fee per KB')); + // feePerKb is required unless inputs & fee are specified + if (!_.isNumber(opts.feePerKb) && !(opts.inputs && _.isNumber(opts.fee))) + return cb(new ClientError('Required argument missing')); + + if (_.isNumber(opts.feePerKb)) { + 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) { @@ -1760,7 +1768,6 @@ WalletService.prototype.createTx = function(opts, cb) { var txOpts = { walletId: self.walletId, creatorId: self.copayerId, - inputs: opts.inputs, outputs: opts.outputs, message: opts.message, changeAddress: wallet.createAddress(true), @@ -1772,6 +1779,8 @@ WalletService.prototype.createTx = function(opts, cb) { validateOutputs: !opts.validateOutputs, addressType: wallet.addressType, customData: opts.customData, + inputs: opts.inputs, + fee: opts.inputs && !_.isNumber(opts.feePerKb) ? opts.fee : null, }; var txp = Model.TxProposal.create(txOpts); diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 88b9037..e53d38b 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -291,6 +291,7 @@ helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { scriptPubKey: scriptPubKey.toBuffer().toString('hex'), address: address.address, confirmations: parsed.confirmations, + publicKeys: address.publicKeys, }; })); diff --git a/test/integration/server.js b/test/integration/server.js index 6736d92..45e2334 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3089,6 +3089,30 @@ describe('Wallet service', function() { }); }); }); + + it('should be able to specify inputs & absolute fee', function(done) { + helpers.stubUtxos(server, wallet, [1, 2], function(utxos) { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 0.8e8, + }], + inputs: utxos, + fee: 1000e2, + }; + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.amount.should.equal(helpers.toSatoshi(0.8)); + should.not.exist(tx.feePerKb); + tx.fee.should.equal(1000e2); + var t = tx.getBitcoreTx(); + t.getFee().should.equal(1000e2); + t.getChangeOutput().satoshis.should.equal(3e8 - 0.8e8 - 1000e2); + done(); + }); + }); + }); }); describe('Backoff time', function(done) { From 8865d42ec3fd654121878f8b4057a8b9c80e47f3 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Mar 2016 12:32:16 -0300 Subject: [PATCH 28/32] improve fee computation for P2PKH wallets --- lib/model/txproposal.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index f6dd382..2697c4c 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -228,7 +228,13 @@ TxProposal.prototype.getRawTx = function() { }; TxProposal.prototype.getEstimatedSizeForSingleInput = function() { - return this.requiredSignatures * 72 + this.walletN * 36 + 44; + switch (this.addressType) { + case Constants.SCRIPT_TYPES.P2PKH: + return 147; + default: + case Constants.SCRIPT_TYPES.P2SH: + return this.requiredSignatures * 72 + this.walletN * 36 + 44; + } }; TxProposal.prototype.getEstimatedSize = function() { From 8ce304fd4e7f686b5484b7bb4677fc551871cd8e Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Mar 2016 12:34:52 -0300 Subject: [PATCH 29/32] reduce safety margin to 2% --- lib/model/txproposal.js | 2 +- test/models/txproposal.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 2697c4c..6f5d782 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -239,7 +239,7 @@ TxProposal.prototype.getEstimatedSizeForSingleInput = 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 safetyMargin = 0.02; var overhead = 4 + 4 + 9 + 9; var inputSize = this.getEstimatedSizeForSingleInput(); diff --git a/test/models/txproposal.js b/test/models/txproposal.js index 28d0cb7..6a8e35a 100644 --- a/test/models/txproposal.js +++ b/test/models/txproposal.js @@ -56,7 +56,7 @@ describe('TxProposal', function() { describe('#getEstimatedSize', function() { it('should return estimated size in bytes', function() { var x = TxProposal.fromObj(aTXP()); - x.getEstimatedSize().should.equal(407); + x.getEstimatedSize().should.equal(396); }); }); From 6f7488fb59668a6132821a8f86e13b8dd7389594 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 11 Mar 2016 10:47:25 -0300 Subject: [PATCH 30/32] default to public insight server on testnet --- config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config.js b/config.js index 6a7c531..58b9711 100644 --- a/config.js +++ b/config.js @@ -42,8 +42,8 @@ var config = { }, testnet: { provider: 'insight', - //url: 'https://test-insight.bitpay.com:443', - url: 'http://localhost:3001', + url: 'https://test-insight.bitpay.com:443', + // url: 'http://localhost:3001', // Multiple servers (in priority order) // url: ['http://a.b.c', 'https://test-insight.bitpay.com:443'], }, From 10b4ff3d15c4066a8e52d4aa743546a2ea37f009 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 11 Mar 2016 11:13:29 -0300 Subject: [PATCH 31/32] fix netTotal bug + test --- lib/server.js | 2 +- test/integration/server.js | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/server.js b/lib/server.js index d8026aa..517b973 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1335,7 +1335,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { log.debug('Considering ' + smallInputs.length + ' small inputs (' + Utils.formatUtxos(smallInputs) + ')'); var total = 0; - var netTotal = 0; + var netTotal = -baseTxpFee; var selected = []; var fee; var error; diff --git a/test/integration/server.js b/test/integration/server.js index 45e2334..8acd72a 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3562,9 +3562,22 @@ describe('Wallet service', function() { }); }); }); - - - + it('should always select inputs as long as there are sufficient funds', function(done) { + helpers.stubUtxos(server, wallet, [80, '50bit', '50bit', '50bit', '50bit', '50bit'], function() { + var txOpts = { + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 101e2, + }], + feePerKb: 100e2, + }; + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + done(); + }); + }); + }); }); }); From a4ced0320f1d2d7e62651c7b4cb6a9a2e2ee51d2 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 11 Mar 2016 11:47:57 -0300 Subject: [PATCH 32/32] refactor fee correction on change below dust --- lib/server.js | 8 +++----- test/integration/server.js | 2 ++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/server.js b/lib/server.js index 517b973..1eefa57 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1389,13 +1389,11 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { if (netTotal >= txpAmount) { var changeAmount = Math.round(total - txpAmount - fee); - log.debug('Tx change: ', Utils.formatAmountInBtc(changeAmount)); - 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 + if (changeAmount > 0 && changeAmount <= Bitcore.Transaction.DUST_AMOUNT) { + log.debug('Change below dust amount (' + Utils.formatAmountInBtc(Bitcore.Transaction.DUST_AMOUNT) + ')'); + // Remove dust change by incrementing fee fee += changeAmount; } diff --git a/test/integration/server.js b/test/integration/server.js index 8acd72a..c9f1832 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3540,6 +3540,8 @@ describe('Wallet service', function() { server.createTx(txOpts, function(err, txp) { should.not.exist(err); (_.sum(txp.inputs, 'satoshis') - txp.outputs[0].amount - txp.fee).should.equal(0); + var changeOutput = txp.getBitcoreTx().getChangeOutput(); + should.not.exist(changeOutput); done(); }); });