From cb677303b60cb3f192525938e5688e6610437e3c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Dec 2015 15:12:43 -0300 Subject: [PATCH 01/18] fix log message --- lib/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index e34d796..a787a0d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -979,7 +979,7 @@ WalletService.prototype.getBalance = function(opts, cb) { self._computeBytesToSendMax(utxos, function(err, size) { if (err) { - log.error('Could not compute fees needed to transfer max amount', err); + log.error('Could not compute size of send max transaction', err); } balance.totalBytesToSendMax = size || 0; return cb(null, balance); From a838978b3f3b2ab0bc51ac05cf89e10d717ba04c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Dec 2015 15:14:54 -0300 Subject: [PATCH 02/18] return null when totalBytesToSendMax cannot be evaluated (instead of 0) --- lib/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index a787a0d..9786c46 100644 --- a/lib/server.js +++ b/lib/server.js @@ -981,7 +981,7 @@ WalletService.prototype.getBalance = function(opts, cb) { if (err) { log.error('Could not compute size of send max transaction', err); } - balance.totalBytesToSendMax = size || 0; + balance.totalBytesToSendMax = size || null; return cb(null, balance); }); }); From 8c0882bf82ecae7244ac49e6f803160842ac7acd Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Dec 2015 15:56:42 -0300 Subject: [PATCH 03/18] accept address list when fetching utxos for current wallet --- lib/server.js | 69 +++++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/lib/server.js b/lib/server.js index 9786c46..7f3a7c9 100644 --- a/lib/server.js +++ b/lib/server.js @@ -855,44 +855,53 @@ WalletService.prototype._getUtxosForAddresses = function(addresses, cb) { }); }; -WalletService.prototype._getUtxosForCurrentWallet = function(cb) { +WalletService.prototype._getUtxosForCurrentWallet = function(addresses, cb) { var self = this; function utxoKey(utxo) { return utxo.txid + '|' + utxo.vout }; - // Get addresses for this wallet - self.storage.fetchAddresses(self.walletId, function(err, addresses) { - if (err) return cb(err); + async.waterfall([ - var addressStrs = _.pluck(addresses, 'address'); - self._getUtxosForAddresses(addressStrs, function(err, utxos) { - if (err) return cb(err); - if (utxos.length == 0) return cb(null, []); + function(next) { + if (_.isArray(addresses) && addresses.length > 0) { + next(null, addresses); + } else { + self.storage.fetchAddresses(self.walletId, next); + } + }, + function(addresses, next) { + if (addresses.length == 0) return next(null, []); - self.getPendingTxs({}, function(err, txps) { - if (err) return cb(err); + var addressStrs = _.pluck(addresses, 'address'); + self._getUtxosForAddresses(addressStrs, function(err, utxos) { + if (err) return next(err); + if (utxos.length == 0) return next(null, []); - var lockedInputs = _.map(_.flatten(_.pluck(txps, 'inputs')), utxoKey); - var utxoIndex = _.indexBy(utxos, utxoKey); - _.each(lockedInputs, function(input) { - if (utxoIndex[input]) { - utxoIndex[input].locked = true; - } + self.getPendingTxs({}, function(err, txps) { + if (err) return next(err); + + var lockedInputs = _.map(_.flatten(_.pluck(txps, 'inputs')), utxoKey); + var utxoIndex = _.indexBy(utxos, utxoKey); + _.each(lockedInputs, function(input) { + if (utxoIndex[input]) { + utxoIndex[input].locked = true; + } + }); + + // Needed for the clients to sign UTXOs + var addressToPath = _.indexBy(addresses, 'address'); + _.each(utxos, function(utxo) { + utxo.path = addressToPath[utxo.address].path; + utxo.publicKeys = addressToPath[utxo.address].publicKeys; + }); + + return next(null, utxos); }); - - // Needed for the clients to sign UTXOs - var addressToPath = _.indexBy(addresses, 'address'); - _.each(utxos, function(utxo) { - utxo.path = addressToPath[utxo.address].path; - utxo.publicKeys = addressToPath[utxo.address].publicKeys; - }); - - return cb(null, utxos); }); - }); - }); + }, + ], cb); }; /** @@ -907,7 +916,7 @@ WalletService.prototype.getUtxos = function(opts, cb) { opts = opts || {}; if (_.isUndefined(opts.addresses)) { - self._getUtxosForCurrentWallet(cb); + self._getUtxosForCurrentWallet(null, cb); } else { self._getUtxosForAddresses(opts.addresses, cb); } @@ -981,7 +990,7 @@ WalletService.prototype.getBalance = function(opts, cb) { if (err) { log.error('Could not compute size of send max transaction', err); } - balance.totalBytesToSendMax = size || null; + balance.totalBytesToSendMax = _.isNumber(size) ? size : null; return cb(null, balance); }); }); @@ -1600,7 +1609,7 @@ WalletService.prototype._broadcastRawTx = function(network, raw, cb) { bc.broadcast(raw, function(err, txid) { if (err) return cb(err); return cb(null, txid); - }) + }); }; /** From 3874d14f71dbb0865e7fcd640a9dff99718ab78c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 4 Dec 2015 17:17:40 -0300 Subject: [PATCH 04/18] 2 step getBalance --- lib/server.js | 71 ++++++++++++++++++++++++++++++++------ test/integration/server.js | 52 ++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 11 deletions(-) diff --git a/lib/server.js b/lib/server.js index 7f3a7c9..062f54e 100644 --- a/lib/server.js +++ b/lib/server.js @@ -832,7 +832,7 @@ WalletService.prototype._getBlockchainExplorer = function(network) { return this.blockchainExplorer; }; -WalletService.prototype._getUtxosForAddresses = function(addresses, cb) { +WalletService.prototype._getUtxos = function(addresses, cb) { var self = this; if (addresses.length == 0) return cb(null, []); @@ -875,7 +875,7 @@ WalletService.prototype._getUtxosForCurrentWallet = function(addresses, cb) { if (addresses.length == 0) return next(null, []); var addressStrs = _.pluck(addresses, 'address'); - self._getUtxosForAddresses(addressStrs, function(err, utxos) { + self._getUtxos(addressStrs, function(err, utxos) { if (err) return next(err); if (utxos.length == 0) return next(null, []); @@ -918,7 +918,7 @@ WalletService.prototype.getUtxos = function(opts, cb) { if (_.isUndefined(opts.addresses)) { self._getUtxosForCurrentWallet(null, cb); } else { - self._getUtxosForAddresses(opts.addresses, cb); + self._getUtxos(opts.addresses, cb); } }; @@ -958,16 +958,12 @@ WalletService.prototype._computeBytesToSendMax = function(utxos, cb) { }); }; -/** - * Creates a new transaction proposal. - * @param {Object} opts - * @returns {Object} balance - Total amount & locked amount. - */ -WalletService.prototype.getBalance = function(opts, cb) { +WalletService.prototype._getBalanceFromAddresses = function(addresses, cb) { var self = this; - self.getUtxos({}, function(err, utxos) { + self._getUtxosForCurrentWallet(addresses, function(err, utxos) { if (err) return cb(err); + var balance = self._totalizeUtxos(utxos); // Compute balance by address @@ -996,6 +992,59 @@ WalletService.prototype.getBalance = function(opts, cb) { }); }; +var prioritaryAddresses; + +/** + * Get wallet balance. + * @param {Object} opts + * @returns {Object} balance - Total amount & locked amount. + */ +WalletService.prototype.getBalance = function(opts, cb) { + var self = this; + + self.storage.fetchAddresses(self.walletId, function(err, addresses) { + if (err) return cb(err); + self._getBalanceFromAddresses(addresses, function(err, balance) { + if (err) return cb(err); + + // Update cache + var addressIndex = _.indexBy(addresses, 'address'); + prioritaryAddresses = _.map(_.pluck(balance.byAddress, 'address'), function(addrStr) { + return addressIndex[addrStr]; + }); + + return cb(null, balance); + }); + }); +}; + + +/** + * Get wallet balance. + * @param {Object} opts + * @returns {Object} balance - Total amount & locked amount. + */ +WalletService.prototype.getBalance2Steps = function(opts, cb) { + var self = this; + + if (!prioritaryAddresses) return self.getBalance(opts, cb); + + self._getBalanceFromAddresses(prioritaryAddresses, function(err, partialBalance) { + if (err) return cb(err); + cb(null, partialBalance); + setTimeout(function() { + self.getBalance(opts, function(err, fullBalance) { + if (err) return; + if (!_.isEqual(partialBalance, fullBalance)) { + console.log('*** [server.js ln1015] ACTUALIZAR BALANCE!!!!, partialBalance, fullBalance:', partialBalance, fullBalance); // TODO + } + }); + }, 1); + return; + }); +}; + + WalletService.prototype._sampleFeeLevels = function(network, points, cb) { var self = this; @@ -1110,7 +1159,7 @@ WalletService.prototype._selectTxInputs = function(txp, utxosToExclude, cb) { return _.pluck(_.sortBy(list, 'order'), 'utxo'); }; - self.getUtxos({}, function(err, utxos) { + self._getUtxosForCurrentWallet(null, function(err, utxos) { if (err) return cb(err); var excludeIndex = _.reduce(utxosToExclude, function(res, val) { diff --git a/test/integration/server.js b/test/integration/server.js index 61376e0..e87897d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1504,6 +1504,58 @@ describe('Wallet service', function() { }); }); + describe('#getBalance 2 steps', function() { + var server, wallet; + beforeEach(function(done) { + helpers.createAndJoinWallet(1, 1, function(s, w) { + server = s; + wallet = w; + done(); + }); + }); + + it('should get balance', function(done) { + helpers.stubUtxos(server, wallet, [1, 'u2', 3], function() { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(6)); + balance.lockedAmount.should.equal(0); + balance.availableAmount.should.equal(helpers.toSatoshi(6)); + balance.totalBytesToSendMax.should.equal(578); + + balance.totalConfirmedAmount.should.equal(helpers.toSatoshi(4)); + balance.lockedConfirmedAmount.should.equal(0); + balance.availableConfirmedAmount.should.equal(helpers.toSatoshi(4)); + + should.exist(balance.byAddress); + balance.byAddress.length.should.equal(2); + balance.byAddress[0].amount.should.equal(helpers.toSatoshi(4)); + balance.byAddress[1].amount.should.equal(helpers.toSatoshi(2)); + done(); + }); + }); + }); + it.only('should trigger notification when balance of non-prioritary addresses is updated', function(done) { + helpers.stubUtxos(server, wallet, [1, 2], function() { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); + + helpers.stubUtxos(server, wallet, [0.5, 0.6], function() { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + //balance.totalAmount.should.equal(helpers.toSatoshi(1.1)); + //done(); + }); + }); + }); + }); + }); + }); + describe('#getFeeLevels', function() { var server, wallet; beforeEach(function(done) { From 94a376ca33069164f06440f3f2f4b980a3c82ac7 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 9 Dec 2015 15:09:47 -0300 Subject: [PATCH 05/18] store cached data in db --- lib/server.js | 42 ++++++------ lib/storage.js | 36 +++++++++++ test/integration/helpers.js | 111 +++++++++++++++++++------------- test/integration/server.js | 124 ++++++++++++++++++++++++++++++++---- 4 files changed, 238 insertions(+), 75 deletions(-) diff --git a/lib/server.js b/lib/server.js index 062f54e..fc79cf2 100644 --- a/lib/server.js +++ b/lib/server.js @@ -992,8 +992,6 @@ WalletService.prototype._getBalanceFromAddresses = function(addresses, cb) { }); }; -var prioritaryAddresses; - /** * Get wallet balance. * @param {Object} opts @@ -1009,11 +1007,15 @@ WalletService.prototype.getBalance = function(opts, cb) { // Update cache var addressIndex = _.indexBy(addresses, 'address'); - prioritaryAddresses = _.map(_.pluck(balance.byAddress, 'address'), function(addrStr) { + var freshAddresses = _.map(_.pluck(balance.byAddress, 'address'), function(addrStr) { return addressIndex[addrStr]; }); - - return cb(null, balance); + self.storage.storeCacheData(self.walletId, 'freshAddresses', freshAddresses, function(err) { + if (err) { + log.warn('Could not update wallet cache', err); + } + return cb(null, balance); + }); }); }); }; @@ -1027,20 +1029,24 @@ WalletService.prototype.getBalance = function(opts, cb) { WalletService.prototype.getBalance2Steps = function(opts, cb) { var self = this; - if (!prioritaryAddresses) return self.getBalance(opts, cb); - - self._getBalanceFromAddresses(prioritaryAddresses, function(err, partialBalance) { - if (err) return cb(err); - cb(null, partialBalance); - setTimeout(function() { - self.getBalance(opts, function(err, fullBalance) { - if (err) return; - if (!_.isEqual(partialBalance, fullBalance)) { - console.log('*** [server.js ln1015] ACTUALIZAR BALANCE!!!!, partialBalance, fullBalance:', partialBalance, fullBalance); // TODO - } + self.storage.fetchCacheData(self.walletId, 'freshAddresses', function(err, freshAddresses) { + if (err || _.isEmpty(freshAddresses)) { + return self.getBalance(opts, cb); + } else { + self._getBalanceFromAddresses(freshAddresses, function(err, partialBalance) { + if (err) return cb(err); + cb(null, partialBalance); + setTimeout(function() { + self.getBalance(opts, function(err, fullBalance) { + if (err) return; + if (!_.isEqual(partialBalance, fullBalance)) { + self._notify('BalanceUpdated', fullBalance); + } + }); + }, 1); + return; }); - }, 1); - return; + } }); }; diff --git a/lib/storage.js b/lib/storage.js index 59be36c..4f0f760 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -20,6 +20,7 @@ var collections = { COPAYERS_LOOKUP: 'copayers_lookup', PREFERENCES: 'preferences', EMAIL_QUEUE: 'email_queue', + CACHE: 'cache', }; var Storage = function(opts) { @@ -56,6 +57,10 @@ Storage.prototype._createIndexes = function() { this.db.collection(collections.EMAIL_QUEUE).createIndex({ notificationId: 1, }); + this.db.collection(collections.CACHE).createIndex({ + walletId: 1, + key: 1, + }); }; Storage.prototype.connect = function(opts, cb) { @@ -501,6 +506,37 @@ Storage.prototype.fetchEmailByNotification = function(notificationId, cb) { }); }; +Storage.prototype.storeCacheData = function(walletId, key, value, cb) { + var self = this; + + var record = { + walletId: walletId, + key: key, + data: value + }; + self.db.collection(collections.CACHE).update({ + walletId: walletId, + key: key, + }, record, { + w: 1, + upsert: true, + }, cb); +}; + +Storage.prototype.fetchCacheData = function(walletId, key, cb) { + var self = this; + + self.db.collection(collections.CACHE).findOne({ + walletId: walletId, + key: key, + }, function(err, result) { + if (err) return cb(err); + if (!result) return cb(); + + return cb(null, result.data); + }); +}; + Storage.prototype._dump = function(cb, fn) { fn = fn || console.log; cb = cb || function() {}; diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 7e57cd1..6626be6 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -211,52 +211,77 @@ helpers.toSatoshi = function(btc) { } }; -helpers.stubUtxos = function(server, wallet, amounts, cb) { - async.mapSeries(_.range(0, amounts.length > 2 ? 2 : 1), function(i, next) { - server.createAddress({}, next); - }, function(err, addresses) { - should.not.exist(err); - 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; +helpers.stubUtxos = function(server, wallet, amounts, opts, cb) { + if (_.isFunction(opts)) { + cb = opts; + opts = {}; + } + opts = opts || {}; + + if (!helpers._utxos) helpers._utxos = {}; + + async.waterfall([ + + function(next) { + if (opts.addresses) return next(null, [].concat(opts.addresses)); + async.mapSeries(_.range(0, amounts.length > 2 ? 2 : 1), function(i, next) { + server.createAddress({}, next); + }, next); + }, + function(addresses, next) { + 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); + } + if (amount <= 0) return null; + + var address = addresses[i % addresses.length]; + + var scriptPubKey; + switch (wallet.addressType) { + case Constants.SCRIPT_TYPES.P2SH: + scriptPubKey = Bitcore.Script.buildMultisigOut(address.publicKeys, wallet.m).toScriptHashOut(); + break; + case Constants.SCRIPT_TYPES.P2PKH: + scriptPubKey = Bitcore.Script.buildPublicKeyHashOut(address.address); + break; + } + should.exist(scriptPubKey); + + return { + txid: helpers.randomTXID(), + vout: Math.floor(Math.random() * 10 + 1), + satoshis: helpers.toSatoshi(amount), + scriptPubKey: scriptPubKey.toBuffer().toString('hex'), + address: address.address, + confirmations: confirmations + }; + })); + + if (opts.keepUtxos) { + helpers._utxos = helpers._utxos.concat(utxos); } else { - confirmations = Math.floor(Math.random() * 100 + 1); + helpers._utxos = utxos; } - if (amount <= 0) return null; - var address = addresses[i % addresses.length]; - - var scriptPubKey; - switch (wallet.addressType) { - case Constants.SCRIPT_TYPES.P2SH: - scriptPubKey = Bitcore.Script.buildMultisigOut(address.publicKeys, wallet.m).toScriptHashOut(); - break; - case Constants.SCRIPT_TYPES.P2PKH: - scriptPubKey = Bitcore.Script.buildPublicKeyHashOut(address.address); - break; - } - should.exist(scriptPubKey); - - return { - txid: helpers.randomTXID(), - vout: Math.floor(Math.random() * 10 + 1), - satoshis: helpers.toSatoshi(amount), - scriptPubKey: scriptPubKey.toBuffer().toString('hex'), - address: address.address, - confirmations: confirmations + blockchainExplorer.getUnspentUtxos = function(addresses, cb) { + var selected = _.filter(helpers._utxos, function(utxo) { + return _.contains(addresses, utxo.address); + }); + return cb(null, selected); }; - })); - blockchainExplorer.getUnspentUtxos = function(addresses, cb) { - var selected = _.filter(utxos, function(utxo) { - return _.contains(addresses, utxo.address); - }); - return cb(null, selected); - }; - return cb(utxos); + return next(); + }, + ], function(err) { + should.not.exist(err); + return cb(helpers._utxos); }); }; @@ -451,14 +476,14 @@ helpers.createProposalOpts = function(type, outputs, signingKey, moreOpts, input }; helpers.createAddresses = function(server, wallet, main, change, cb) { var clock = sinon.useFakeTimers(Date.now(), 'Date'); - async.map(_.range(main + change), function(i, next) { + async.mapSeries(_.range(main + change), function(i, next) { clock.tick(1000); var address = wallet.createAddress(i >= main); server.storage.storeAddressAndWallet(wallet, address, function(err) { next(err, address); }); }, function(err, addresses) { - if (err) throw new Error('Could not generate addresses'); + should.not.exist(err); clock.restore(); return cb(_.take(addresses, main), _.takeRight(addresses, change)); }); diff --git a/test/integration/server.js b/test/integration/server.js index e87897d..c12e17b 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1504,7 +1504,7 @@ describe('Wallet service', function() { }); }); - describe('#getBalance 2 steps', function() { + describe.only('#getBalance 2 steps', function() { var server, wallet; beforeEach(function(done) { helpers.createAndJoinWallet(1, 1, function(s, w) { @@ -1536,22 +1536,118 @@ describe('Wallet service', function() { }); }); }); - it.only('should trigger notification when balance of non-prioritary addresses is updated', function(done) { - helpers.stubUtxos(server, wallet, [1, 2], function() { - server.getBalance2Steps({}, function(err, balance) { - should.not.exist(err); - should.exist(balance); - balance.totalAmount.should.equal(helpers.toSatoshi(3)); - helpers.stubUtxos(server, wallet, [0.5, 0.6], function() { - server.getBalance2Steps({}, function(err, balance) { - should.not.exist(err); - should.exist(balance); - //balance.totalAmount.should.equal(helpers.toSatoshi(1.1)); - //done(); + it('should trigger notification when balance of non-prioritary addresses is updated', function(done) { + var addresses; + + async.series([ + + function(next) { + helpers.createAddresses(server, wallet, 4, 0, function(addrs) { + addresses = addrs; + helpers.stubUtxos(server, wallet, [1, 2], { + addresses: _.take(addresses, 2), + }, function() { + next(); }); }); - }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); + next(); + }); + }, + function(next) { + helpers.stubUtxos(server, wallet, 0.5, { + addresses: addresses[2], + keepUtxos: true, + }, function() { + next(); + }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); + next(); + }); + }, + function(next) { + setTimeout(next, 100); + }, + function(next) { + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var last = _.last(notifications); + last.type.should.equal('BalanceUpdated'); + var balance = last.data; + balance.totalAmount.should.equal(helpers.toSatoshi(3.5)); + next(); + }); + }, + ], function(err) { + should.not.exist(err); + done(); + }); + }); + + it('should not trigger notification when only balance of prioritary addresses is updated', function(done) { + var addresses; + + async.series([ + + function(next) { + helpers.createAddresses(server, wallet, 4, 0, function(addrs) { + addresses = addrs; + helpers.stubUtxos(server, wallet, [1, 2], { + addresses: _.take(addresses, 2), + }, function() { + next(); + }); + }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); + next(); + }); + }, + function(next) { + helpers.stubUtxos(server, wallet, 0.5, { + addresses: addresses[0], + keepUtxos: true, + }, function() { + next(); + }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3.5)); + next(); + }); + }, + function(next) { + setTimeout(next, 100); + }, + function(next) { + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var last = _.last(notifications); + last.type.should.not.equal('BalanceUpdated'); + next(); + }); + }, + ], function(err) { + should.not.exist(err); + done(); }); }); }); From 2914584d50fa2bc0de1677027ddf5fc858203540 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 9 Dec 2015 15:37:16 -0300 Subject: [PATCH 06/18] test new addresses --- test/integration/server.js | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/integration/server.js b/test/integration/server.js index c12e17b..2d4b380 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1650,6 +1650,64 @@ describe('Wallet service', function() { done(); }); }); + + it('should resolve balance of new addresses immediately', function(done) { + var addresses; + + async.series([ + + function(next) { + helpers.createAddresses(server, wallet, 4, 0, function(addrs) { + addresses = addrs; + helpers.stubUtxos(server, wallet, [1, 2], { + addresses: _.take(addresses, 2), + }, function() { + next(); + }); + }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); + next(); + }); + }, + function(next) { + server.createAddress({}, function(err, addr) { + helpers.stubUtxos(server, wallet, 0.5, { + addresses: addr, + keepUtxos: true, + }, function() { + next(); + }); + }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3.5)); + next(); + }); + }, + function(next) { + setTimeout(next, 100); + }, + function(next) { + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var last = _.last(notifications); + last.type.should.not.equal('BalanceUpdated'); + next(); + }); + }, + ], function(err) { + should.not.exist(err); + done(); + }); + }); }); describe('#getFeeLevels', function() { From 0ab57133fd99d1383316e200eb0801e1269b051f Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 11:43:47 -0300 Subject: [PATCH 07/18] fix tests --- lib/server.js | 43 +++++++++++++++++++++++--- lib/storage.js | 43 ++++++++++++++------------ test/integration/helpers.js | 6 ++-- test/integration/server.js | 61 ++++++++++++++++++++++++++++--------- 4 files changed, 112 insertions(+), 41 deletions(-) diff --git a/lib/server.js b/lib/server.js index fc79cf2..7ea7518 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1007,10 +1007,11 @@ WalletService.prototype.getBalance = function(opts, cb) { // Update cache var addressIndex = _.indexBy(addresses, 'address'); - var freshAddresses = _.map(_.pluck(balance.byAddress, 'address'), function(addrStr) { + var active = _.map(_.pluck(balance.byAddress, 'address'), function(addrStr) { return addressIndex[addrStr]; }); - self.storage.storeCacheData(self.walletId, 'freshAddresses', freshAddresses, function(err) { + + self.storage.storeActiveAddresses(self.walletId, active, function(err) { if (err) { log.warn('Could not update wallet cache', err); } @@ -1021,6 +1022,36 @@ WalletService.prototype.getBalance = function(opts, cb) { }; +WalletService.prototype._getActiveAddresses = function(cb) { + var self = this; + + self.storage.fetchActiveAddresses(self.walletId, function(err, active) { + if (err) { + log.warn('Could not fetch active addresses from cache', err); + return cb(null, []); + } + active = _.pluck(active, 'address'); + + self.storage.fetchAddresses(self.walletId, function(err, allAddresses) { + if (err) return cb(err); + + var now = Math.floor(Date.now() / 1000); + var recent = _.pluck(_.filter(allAddresses, function(address) { + return address.createdOn > (now - 24 * 3600); + }), 'address'); + + var result = _.union(active, recent); + + // HACK: translate result into Address objects, TODO!! should work with strings + var index = _.indexBy(allAddresses, 'address'); + result = _.map(result, function(r) { + return index[r]; + }); + return cb(null, result); + }); + }); +}; + /** * Get wallet balance. * @param {Object} opts @@ -1029,17 +1060,19 @@ WalletService.prototype.getBalance = function(opts, cb) { WalletService.prototype.getBalance2Steps = function(opts, cb) { var self = this; - self.storage.fetchCacheData(self.walletId, 'freshAddresses', function(err, freshAddresses) { - if (err || _.isEmpty(freshAddresses)) { + self._getActiveAddresses(function(err, activeAddresses) { + if (err) return cb(err); + if (_.isEmpty(activeAddresses)) { return self.getBalance(opts, cb); } else { - self._getBalanceFromAddresses(freshAddresses, function(err, partialBalance) { + self._getBalanceFromAddresses(activeAddresses, function(err, partialBalance) { if (err) return cb(err); cb(null, partialBalance); setTimeout(function() { self.getBalance(opts, function(err, fullBalance) { if (err) return; if (!_.isEqual(partialBalance, fullBalance)) { + log.debug('Cache miss: balance in active addresses differs from final balance'); self._notify('BalanceUpdated', fullBalance); } }); diff --git a/lib/storage.js b/lib/storage.js index 4f0f760..b454bac 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -59,6 +59,7 @@ Storage.prototype._createIndexes = function() { }); this.db.collection(collections.CACHE).createIndex({ walletId: 1, + type: 1, key: 1, }); }; @@ -230,7 +231,7 @@ Storage.prototype.fetchLastTxs = function(walletId, creatorId, limit, cb) { Storage.prototype.fetchPendingTxs = function(walletId, cb) { var self = this; - this.db.collection(collections.TXS).find({ + self.db.collection(collections.TXS).find({ walletId: walletId, isPending: true, }).sort({ @@ -506,34 +507,38 @@ Storage.prototype.fetchEmailByNotification = function(notificationId, cb) { }); }; -Storage.prototype.storeCacheData = function(walletId, key, value, cb) { +Storage.prototype.storeActiveAddresses = function(walletId, addresses, cb) { var self = this; - var record = { - walletId: walletId, - key: key, - data: value - }; - self.db.collection(collections.CACHE).update({ - walletId: walletId, - key: key, - }, record, { - w: 1, - upsert: true, + async.each(addresses, function(address, next) { + var record = { + walletId: walletId, + type: 'activeAddresses', + key: address.address, + value: address, + }; + self.db.collection(collections.CACHE).update({ + walletId: record.walletId, + type: record.type, + key: record.key, + }, record, { + w: 1, + upsert: true, + }, next); }, cb); }; -Storage.prototype.fetchCacheData = function(walletId, key, cb) { +Storage.prototype.fetchActiveAddresses = function(walletId, cb) { var self = this; - self.db.collection(collections.CACHE).findOne({ + self.db.collection(collections.CACHE).find({ walletId: walletId, - key: key, - }, function(err, result) { + type: 'activeAddresses', + }).toArray(function(err, result) { if (err) return cb(err); - if (!result) return cb(); + if (!result || _.isEmpty(result)) return cb(null, []); - return cb(null, result.data); + return cb(null, _.pluck(result, 'value')); }); }; diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 6626be6..3aa1b13 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -475,16 +475,16 @@ helpers.createProposalOpts = function(type, outputs, signingKey, moreOpts, input return opts; }; helpers.createAddresses = function(server, wallet, main, change, cb) { - var clock = sinon.useFakeTimers(Date.now(), 'Date'); + // var clock = sinon.useFakeTimers('Date'); async.mapSeries(_.range(main + change), function(i, next) { - clock.tick(1000); + // clock.tick(1000); var address = wallet.createAddress(i >= main); server.storage.storeAddressAndWallet(wallet, address, function(err) { next(err, address); }); }, function(err, addresses) { should.not.exist(err); - clock.restore(); + // clock.restore(); return cb(_.take(addresses, main), _.takeRight(addresses, change)); }); }; diff --git a/test/integration/server.js b/test/integration/server.js index 2d4b380..d527c8c 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1504,15 +1504,20 @@ describe('Wallet service', function() { }); }); - describe.only('#getBalance 2 steps', function() { - var server, wallet; + describe('#getBalance 2 steps', function() { + var server, wallet, clock; beforeEach(function(done) { + clock = sinon.useFakeTimers(Date.now(), 'Date'); + helpers.createAndJoinWallet(1, 1, function(s, w) { server = s; wallet = w; done(); }); }); + afterEach(function() { + clock.restore(); + }); it('should get balance', function(done) { helpers.stubUtxos(server, wallet, [1, 'u2', 3], function() { @@ -1532,21 +1537,28 @@ describe('Wallet service', function() { balance.byAddress.length.should.equal(2); balance.byAddress[0].amount.should.equal(helpers.toSatoshi(4)); balance.byAddress[1].amount.should.equal(helpers.toSatoshi(2)); - done(); + setTimeout(done, 100); }); }); }); it('should trigger notification when balance of non-prioritary addresses is updated', function(done) { - var addresses; + var oldAddrs, newAddrs; async.series([ function(next) { - helpers.createAddresses(server, wallet, 4, 0, function(addrs) { - addresses = addrs; + helpers.createAddresses(server, wallet, 2, 0, function(addrs) { + oldAddrs = addrs; + next(); + }); + }, + function(next) { + clock.tick(7 * 24 * 3600 * 1000); + helpers.createAddresses(server, wallet, 2, 0, function(addrs) { + newAddrs = addrs; helpers.stubUtxos(server, wallet, [1, 2], { - addresses: _.take(addresses, 2), + addresses: [oldAddrs[0], newAddrs[0]], }, function() { next(); }); @@ -1556,13 +1568,24 @@ describe('Wallet service', function() { server.getBalance2Steps({}, function(err, balance) { should.not.exist(err); should.exist(balance); - balance.totalAmount.should.equal(helpers.toSatoshi(3)); + balance.totalAmount.should.equal(helpers.toSatoshi(2)); + next(); + }); + }, + function(next) { + setTimeout(next, 100); + }, + function(next) { + server._getActiveAddresses(function(err, active) { + should.not.exist(err); + should.exist(active); + active.length.should.equal(3); next(); }); }, function(next) { helpers.stubUtxos(server, wallet, 0.5, { - addresses: addresses[2], + addresses: oldAddrs[1], keepUtxos: true, }, function() { next(); @@ -1596,15 +1619,22 @@ describe('Wallet service', function() { }); it('should not trigger notification when only balance of prioritary addresses is updated', function(done) { - var addresses; + var oldAddrs, newAddrs; async.series([ function(next) { - helpers.createAddresses(server, wallet, 4, 0, function(addrs) { - addresses = addrs; + helpers.createAddresses(server, wallet, 2, 0, function(addrs) { + oldAddrs = addrs; + next(); + }); + }, + function(next) { + clock.tick(7 * 24 * 3600 * 1000); + helpers.createAddresses(server, wallet, 2, 0, function(addrs) { + newAddrs = addrs; helpers.stubUtxos(server, wallet, [1, 2], { - addresses: _.take(addresses, 2), + addresses: newAddrs, }, function() { next(); }); @@ -1618,9 +1648,12 @@ describe('Wallet service', function() { next(); }); }, + function(next) { + setTimeout(next, 100); + }, function(next) { helpers.stubUtxos(server, wallet, 0.5, { - addresses: addresses[0], + addresses: newAddrs[0], keepUtxos: true, }, function() { next(); From 9868cd7c34cba3474de7c513c8f0119acc3e0b77 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 12:36:30 -0300 Subject: [PATCH 08/18] fix tests --- lib/common/defaults.js | 3 +++ lib/server.js | 41 ++++++++++++++++++++++++----------------- lib/storage.js | 6 ++++++ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/lib/common/defaults.js b/lib/common/defaults.js index cc8a4ba..52e6d5a 100644 --- a/lib/common/defaults.js +++ b/lib/common/defaults.js @@ -37,4 +37,7 @@ Defaults.FEE_LEVELS = [{ defaultValue: 10000 }]; +// Minimum nb of addresses a wallet must have to start using 2-step balance optimization +Defaults.TWO_STEP_BALANCE_THRESHOLD = 100; + module.exports = Defaults; diff --git a/lib/server.js b/lib/server.js index 7ea7518..fea698f 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1060,26 +1060,33 @@ WalletService.prototype._getActiveAddresses = function(cb) { WalletService.prototype.getBalance2Steps = function(opts, cb) { var self = this; - self._getActiveAddresses(function(err, activeAddresses) { + self.storage.countAddresses(self.walletId, function(err, nbAddresses) { if (err) return cb(err); - if (_.isEmpty(activeAddresses)) { + if (nbAddresses < Defaults.TWO_STEP_BALANCE_THRESHOLD) { return self.getBalance(opts, cb); - } else { - self._getBalanceFromAddresses(activeAddresses, function(err, partialBalance) { - if (err) return cb(err); - cb(null, partialBalance); - setTimeout(function() { - self.getBalance(opts, function(err, fullBalance) { - if (err) return; - if (!_.isEqual(partialBalance, fullBalance)) { - log.debug('Cache miss: balance in active addresses differs from final balance'); - self._notify('BalanceUpdated', fullBalance); - } - }); - }, 1); - return; - }); } + + self._getActiveAddresses(function(err, activeAddresses) { + if (err) return cb(err); + if (_.isEmpty(activeAddresses)) { + return self.getBalance(opts, cb); + } else { + self._getBalanceFromAddresses(activeAddresses, function(err, partialBalance) { + if (err) return cb(err); + cb(null, partialBalance); + setTimeout(function() { + self.getBalance(opts, function(err, fullBalance) { + if (err) return; + if (!_.isEqual(partialBalance, fullBalance)) { + log.debug('Cache miss: balance in active addresses differs from final balance'); + self._notify('BalanceUpdated', fullBalance); + } + }); + }, 1); + return; + }); + } + }); }); }; diff --git a/lib/storage.js b/lib/storage.js index b454bac..0dfc76e 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -389,6 +389,12 @@ Storage.prototype.fetchAddresses = function(walletId, cb) { }); }; +Storage.prototype.countAddresses = function(walletId, cb) { + this.db.collection(collections.ADDRESSES).find({ + walletId: walletId, + }).count(cb); +}; + Storage.prototype.storeAddress = function(address, cb) { var self = this; From 9dcae6f4bc20a00c5ae29cd554163a6da5b08197 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 12:41:40 -0300 Subject: [PATCH 09/18] introduce threshold for nb of addresses --- test/integration/server.js | 53 +++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/test/integration/server.js b/test/integration/server.js index d527c8c..2043d63 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1504,10 +1504,12 @@ describe('Wallet service', function() { }); }); - describe('#getBalance 2 steps', function() { + describe.only('#getBalance 2 steps', function() { var server, wallet, clock; + var _threshold = Defaults.TWO_STEP_BALANCE_THRESHOLD; beforeEach(function(done) { clock = sinon.useFakeTimers(Date.now(), 'Date'); + Defaults.TWO_STEP_BALANCE_THRESHOLD = 0; helpers.createAndJoinWallet(1, 1, function(s, w) { server = s; @@ -1517,6 +1519,7 @@ describe('Wallet service', function() { }); afterEach(function() { clock.restore(); + Defaults.TWO_STEP_BALANCE_THRESHOLD = _threshold; }); it('should get balance', function(done) { @@ -1741,6 +1744,54 @@ describe('Wallet service', function() { done(); }); }); + + it('should not perform 2 steps when nb of addresses below threshold', function(done) { + var oldAddrs, newAddrs; + Defaults.TWO_STEP_BALANCE_THRESHOLD = 5; + + async.series([ + + function(next) { + helpers.createAddresses(server, wallet, 2, 0, function(addrs) { + oldAddrs = addrs; + next(); + }); + }, + function(next) { + clock.tick(7 * 24 * 3600 * 1000); + helpers.createAddresses(server, wallet, 2, 0, function(addrs) { + newAddrs = addrs; + helpers.stubUtxos(server, wallet, [1, 2], { + addresses: [oldAddrs[0], newAddrs[0]], + }, function() { + next(); + }); + }); + }, + function(next) { + server.getBalance2Steps({}, function(err, balance) { + should.not.exist(err); + should.exist(balance); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); + next(); + }); + }, + function(next) { + setTimeout(next, 100); + }, + function(next) { + server.getNotifications({}, function(err, notifications) { + should.not.exist(err); + var last = _.last(notifications); + last.type.should.not.equal('BalanceUpdated'); + next(); + }); + }, + ], function(err) { + should.not.exist(err); + done(); + }); + }); }); describe('#getFeeLevels', function() { From 4e7241e86d7367f2f41d83d963a2e78b68675402 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 12:45:37 -0300 Subject: [PATCH 10/18] add v2/balance endpoint --- lib/expressapp.js | 10 ++++++++++ test/storage.js | 1 + 2 files changed, 11 insertions(+) diff --git a/lib/expressapp.js b/lib/expressapp.js index 6867922..5e8caef 100644 --- a/lib/expressapp.js +++ b/lib/expressapp.js @@ -314,6 +314,7 @@ ExpressApp.prototype.start = function(opts, cb) { }); }); + // DEPRECATED router.get('/v1/balance/', function(req, res) { getServerWithAuth(req, res, function(server) { server.getBalance({}, function(err, balance) { @@ -323,6 +324,15 @@ ExpressApp.prototype.start = function(opts, cb) { }); }); + router.get('/v2/balance/', function(req, res) { + getServerWithAuth(req, res, function(server) { + server.getBalance2Steps({}, function(err, balance) { + if (err) return returnError(err, res, req); + res.json(balance); + }); + }); + }); + // DEPRECATED router.get('/v1/feelevels/', function(req, res) { var opts = {}; diff --git a/test/storage.js b/test/storage.js index f5d4e5c..6b336a7 100644 --- a/test/storage.js +++ b/test/storage.js @@ -77,6 +77,7 @@ describe('Storage', function() { }); }); }); + describe('Copayer lookup', function() { it('should correctly store and fetch copayer lookup', function(done) { var wallet = Model.Wallet.create({ From 56d1562e920c73fb6054929732eb5f6307ec6808 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 12:58:08 -0300 Subject: [PATCH 11/18] express endpoint --- lib/expressapp.js | 13 ++----------- test/expressapp.js | 39 ++++++++++++++++++++++++++++++++++++++ test/integration/server.js | 2 +- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/lib/expressapp.js b/lib/expressapp.js index 5e8caef..60c3639 100644 --- a/lib/expressapp.js +++ b/lib/expressapp.js @@ -314,19 +314,10 @@ ExpressApp.prototype.start = function(opts, cb) { }); }); - // DEPRECATED router.get('/v1/balance/', function(req, res) { getServerWithAuth(req, res, function(server) { - server.getBalance({}, function(err, balance) { - if (err) return returnError(err, res, req); - res.json(balance); - }); - }); - }); - - router.get('/v2/balance/', function(req, res) { - getServerWithAuth(req, res, function(server) { - server.getBalance2Steps({}, function(err, balance) { + var balanceFn = req.query.cache == '1' ? _.bind(server.getBalance2Steps, server) : _.bind(server.getBalance, server); + balanceFn({}, function(err, balance) { if (err) return returnError(err, res, req); res.json(balance); }); diff --git a/test/expressapp.js b/test/expressapp.js index d1310ca..a97f113 100644 --- a/test/expressapp.js +++ b/test/expressapp.js @@ -113,6 +113,45 @@ describe('ExpressApp', function() { }); }); + describe('Balance', function() { + it('should handle cache argument', function(done) { + var server = { + getBalance: sinon.stub().callsArgWith(1, null, {}), + getBalance2Steps: sinon.stub().callsArgWith(1, null, {}), + }; + var TestExpressApp = proxyquire('../lib/expressapp', { + './server': { + initialize: sinon.stub().callsArg(1), + getInstanceWithAuth: sinon.stub().callsArgWith(1, null, server), + } + }); + start(TestExpressApp, function() { + var reqOpts = { + url: testHost + ':' + testPort + config.basePath + '/v1/balance', + headers: { + 'x-identity': 'identity', + 'x-signature': 'signature' + } + }; + request(reqOpts, function(err, res, body) { + should.not.exist(err); + res.statusCode.should.equal(200); + server.getBalance.calledOnce.should.be.true; + server.getBalance2Steps.calledOnce.should.be.false; + + reqOpts.url += '?cache=1'; + request(reqOpts, function(err, res, body) { + should.not.exist(err); + res.statusCode.should.equal(200); + server.getBalance.calledTwice.should.be.false; + server.getBalance2Steps.calledOnce.should.be.true; + done(); + }); + }); + }); + }); + }); + describe('/v1/notifications', function(done) { var server, TestExpressApp, clock; beforeEach(function() { diff --git a/test/integration/server.js b/test/integration/server.js index 2043d63..2c69c22 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1504,7 +1504,7 @@ describe('Wallet service', function() { }); }); - describe.only('#getBalance 2 steps', function() { + describe('#getBalance 2 steps', function() { var server, wallet, clock; var _threshold = Defaults.TWO_STEP_BALANCE_THRESHOLD; beforeEach(function(done) { From cc94b19e8266ce62669c8399131eb88fc958c572 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 13:04:31 -0300 Subject: [PATCH 12/18] rebase --- test/integration/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/server.js b/test/integration/server.js index 2c69c22..a8ffb67 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2516,7 +2516,7 @@ describe('Wallet service', function() { it('should be able to create tx with inputs argument', function(done) { helpers.stubUtxos(server, wallet, [1, 3, 2], function(utxos) { - server._getUtxosForCurrentWallet(function(err, utxos) { + server.getUtxos({}, function(err, utxos) { should.not.exist(err); var inputs = [utxos[0], utxos[2]]; var txOpts = helpers.createExternalProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 2.5, From 072aed5cc1cadde60d98370d326f7f087d2d6392 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 10 Dec 2015 13:27:46 -0300 Subject: [PATCH 13/18] cache address strings only --- lib/server.js | 7 +------ lib/storage.js | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/server.js b/lib/server.js index fea698f..674465a 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1006,11 +1006,7 @@ WalletService.prototype.getBalance = function(opts, cb) { if (err) return cb(err); // Update cache - var addressIndex = _.indexBy(addresses, 'address'); - var active = _.map(_.pluck(balance.byAddress, 'address'), function(addrStr) { - return addressIndex[addrStr]; - }); - + var active = _.pluck(balance.byAddress, 'address') self.storage.storeActiveAddresses(self.walletId, active, function(err) { if (err) { log.warn('Could not update wallet cache', err); @@ -1030,7 +1026,6 @@ WalletService.prototype._getActiveAddresses = function(cb) { log.warn('Could not fetch active addresses from cache', err); return cb(null, []); } - active = _.pluck(active, 'address'); self.storage.fetchAddresses(self.walletId, function(err, allAddresses) { if (err) return cb(err); diff --git a/lib/storage.js b/lib/storage.js index 0dfc76e..5122c5c 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -520,8 +520,8 @@ Storage.prototype.storeActiveAddresses = function(walletId, addresses, cb) { var record = { walletId: walletId, type: 'activeAddresses', - key: address.address, - value: address, + key: address, + value: null, }; self.db.collection(collections.CACHE).update({ walletId: record.walletId, @@ -544,7 +544,7 @@ Storage.prototype.fetchActiveAddresses = function(walletId, cb) { if (err) return cb(err); if (!result || _.isEmpty(result)) return cb(null, []); - return cb(null, _.pluck(result, 'value')); + return cb(null, _.pluck(result, 'key')); }); }; From 82f54f79016510a9581e46dc213a1fb159895fc5 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 11 Dec 2015 16:13:08 -0300 Subject: [PATCH 14/18] remove comment --- lib/server.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index 674465a..7e97009 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1037,7 +1037,6 @@ WalletService.prototype._getActiveAddresses = function(cb) { var result = _.union(active, recent); - // HACK: translate result into Address objects, TODO!! should work with strings var index = _.indexBy(allAddresses, 'address'); result = _.map(result, function(r) { return index[r]; From 87b96d4a8f02e44482aece1d65a499d0e1998052 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 11 Dec 2015 16:32:38 -0300 Subject: [PATCH 15/18] keep getStatus() method, add twoStep param --- lib/expressapp.js | 6 ++++-- lib/server.js | 25 +++++++++++++------------ test/expressapp.js | 11 +++++------ test/integration/server.js | 32 ++++++++++++++++++++++++-------- 4 files changed, 46 insertions(+), 28 deletions(-) diff --git a/lib/expressapp.js b/lib/expressapp.js index 60c3639..c78b1d2 100644 --- a/lib/expressapp.js +++ b/lib/expressapp.js @@ -214,6 +214,7 @@ ExpressApp.prototype.start = function(opts, cb) { getServerWithAuth(req, res, function(server) { var opts = {}; if (req.query.includeExtendedInfo == '1') opts.includeExtendedInfo = true; + if (req.query.twoStep == '1') opts.twoStep = true; server.getStatus(opts, function(err, status) { if (err) return returnError(err, res, req); @@ -316,8 +317,9 @@ ExpressApp.prototype.start = function(opts, cb) { router.get('/v1/balance/', function(req, res) { getServerWithAuth(req, res, function(server) { - var balanceFn = req.query.cache == '1' ? _.bind(server.getBalance2Steps, server) : _.bind(server.getBalance, server); - balanceFn({}, function(err, balance) { + var opts = {}; + if (req.query.twoStep == '1') opts.twoStep = true; + server.getBalance(opts, function(err, balance) { if (err) return returnError(err, res, req); res.json(balance); }); diff --git a/lib/server.js b/lib/server.js index 7e97009..f8cf55e 100644 --- a/lib/server.js +++ b/lib/server.js @@ -263,6 +263,7 @@ WalletService.prototype.getWallet = function(opts, cb) { /** * Retrieves wallet status. * @param {Object} opts + * @param {Object} opts.twoStep[=false] - Optional: use 2-step balance computation for improved performance * @param {Object} opts.includeExtendedInfo - Include PKR info & address managers for wallet & copayers * @returns {Object} status */ @@ -296,7 +297,7 @@ WalletService.prototype.getStatus = function(opts, cb) { }); }, function(next) { - self.getBalance({}, function(err, balance) { + self.getBalance(opts, function(err, balance) { if (err) return next(err); status.balance = balance; next(); @@ -992,12 +993,7 @@ WalletService.prototype._getBalanceFromAddresses = function(addresses, cb) { }); }; -/** - * Get wallet balance. - * @param {Object} opts - * @returns {Object} balance - Total amount & locked amount. - */ -WalletService.prototype.getBalance = function(opts, cb) { +WalletService.prototype._getBalanceOneStep = function(opts, cb) { var self = this; self.storage.fetchAddresses(self.walletId, function(err, addresses) { @@ -1049,27 +1045,33 @@ WalletService.prototype._getActiveAddresses = function(cb) { /** * Get wallet balance. * @param {Object} opts + * @param {Boolean} opts.twoStep[=false] - Optional - Use 2 step balance computation for improved performance * @returns {Object} balance - Total amount & locked amount. */ -WalletService.prototype.getBalance2Steps = function(opts, cb) { +WalletService.prototype.getBalance = function(opts, cb) { var self = this; + opts = opts || {}; + + if (!opts.twoStep) + return self._getBalanceOneStep(opts, cb); + self.storage.countAddresses(self.walletId, function(err, nbAddresses) { if (err) return cb(err); if (nbAddresses < Defaults.TWO_STEP_BALANCE_THRESHOLD) { - return self.getBalance(opts, cb); + return self._getBalanceOneStep(opts, cb); } self._getActiveAddresses(function(err, activeAddresses) { if (err) return cb(err); if (_.isEmpty(activeAddresses)) { - return self.getBalance(opts, cb); + return self._getBalanceOneStep(opts, cb); } else { self._getBalanceFromAddresses(activeAddresses, function(err, partialBalance) { if (err) return cb(err); cb(null, partialBalance); setTimeout(function() { - self.getBalance(opts, function(err, fullBalance) { + self._getBalanceOneStep(opts, function(err, fullBalance) { if (err) return; if (!_.isEqual(partialBalance, fullBalance)) { log.debug('Cache miss: balance in active addresses differs from final balance'); @@ -1084,7 +1086,6 @@ WalletService.prototype.getBalance2Steps = function(opts, cb) { }); }; - WalletService.prototype._sampleFeeLevels = function(network, points, cb) { var self = this; diff --git a/test/expressapp.js b/test/expressapp.js index a97f113..3f20506 100644 --- a/test/expressapp.js +++ b/test/expressapp.js @@ -117,7 +117,6 @@ describe('ExpressApp', function() { it('should handle cache argument', function(done) { var server = { getBalance: sinon.stub().callsArgWith(1, null, {}), - getBalance2Steps: sinon.stub().callsArgWith(1, null, {}), }; var TestExpressApp = proxyquire('../lib/expressapp', { './server': { @@ -136,15 +135,15 @@ describe('ExpressApp', function() { request(reqOpts, function(err, res, body) { should.not.exist(err); res.statusCode.should.equal(200); - server.getBalance.calledOnce.should.be.true; - server.getBalance2Steps.calledOnce.should.be.false; + var args = server.getBalance.getCalls()[0].args[0]; + should.not.exist(args.twoStep); - reqOpts.url += '?cache=1'; + reqOpts.url += '?twoStep=1'; request(reqOpts, function(err, res, body) { should.not.exist(err); res.statusCode.should.equal(200); - server.getBalance.calledTwice.should.be.false; - server.getBalance2Steps.calledOnce.should.be.true; + var args = server.getBalance.getCalls()[1].args[0]; + args.twoStep.should.equal(true); done(); }); }); diff --git a/test/integration/server.js b/test/integration/server.js index a8ffb67..be7ee25 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1524,7 +1524,9 @@ describe('Wallet service', function() { it('should get balance', function(done) { helpers.stubUtxos(server, wallet, [1, 'u2', 3], function() { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(6)); @@ -1568,7 +1570,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(2)); @@ -1595,7 +1599,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(3)); @@ -1644,7 +1650,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(3)); @@ -1663,7 +1671,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(3.5)); @@ -1703,7 +1713,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(3)); @@ -1721,7 +1733,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(3.5)); @@ -1769,7 +1783,9 @@ describe('Wallet service', function() { }); }, function(next) { - server.getBalance2Steps({}, function(err, balance) { + server.getBalance({ + twoStep: true + }, function(err, balance) { should.not.exist(err); should.exist(balance); balance.totalAmount.should.equal(helpers.toSatoshi(3)); From 4198d5c47c45d79b0895081a7efb9914298f536a Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 14 Dec 2015 17:04:37 -0300 Subject: [PATCH 16/18] handle initial conditions (empty cache) --- lib/server.js | 26 ++++++++++++++++++++------ lib/storage.js | 30 +++++++++++++++++++++++++++--- test/integration/server.js | 14 +++++++++----- 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/lib/server.js b/lib/server.js index f8cf55e..2c8d479 100644 --- a/lib/server.js +++ b/lib/server.js @@ -866,8 +866,12 @@ WalletService.prototype._getUtxosForCurrentWallet = function(addresses, cb) { async.waterfall([ function(next) { - if (_.isArray(addresses) && addresses.length > 0) { - next(null, addresses); + if (_.isArray(addresses)) { + if (!_.isEmpty(addresses)) { + next(null, addresses); + } else { + next(null, []); + } } else { self.storage.fetchAddresses(self.walletId, next); } @@ -1002,8 +1006,16 @@ WalletService.prototype._getBalanceOneStep = function(opts, cb) { if (err) return cb(err); // Update cache - var active = _.pluck(balance.byAddress, 'address') - self.storage.storeActiveAddresses(self.walletId, active, function(err) { + async.series([ + + function(next) { + self.storage.cleanActiveAddresses(self.walletId, next); + }, + function(next) { + var active = _.pluck(balance.byAddress, 'address') + self.storage.storeActiveAddresses(self.walletId, active, next); + }, + ], function(err) { if (err) { log.warn('Could not update wallet cache', err); } @@ -1020,9 +1032,11 @@ WalletService.prototype._getActiveAddresses = function(cb) { self.storage.fetchActiveAddresses(self.walletId, function(err, active) { if (err) { log.warn('Could not fetch active addresses from cache', err); - return cb(null, []); + return cb(); } + if (!_.isArray(active)) return cb(); + self.storage.fetchAddresses(self.walletId, function(err, allAddresses) { if (err) return cb(err); @@ -1064,7 +1078,7 @@ WalletService.prototype.getBalance = function(opts, cb) { self._getActiveAddresses(function(err, activeAddresses) { if (err) return cb(err); - if (_.isEmpty(activeAddresses)) { + if (!_.isArray(activeAddresses)) { return self._getBalanceOneStep(opts, cb); } else { self._getBalanceFromAddresses(activeAddresses, function(err, partialBalance) { diff --git a/lib/storage.js b/lib/storage.js index 5122c5c..d159ece 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -513,6 +513,31 @@ Storage.prototype.fetchEmailByNotification = function(notificationId, cb) { }); }; +Storage.prototype.cleanActiveAddresses = function(walletId, cb) { + var self = this; + + async.series([ + + function(next) { + self.db.collection(collections.CACHE).remove({ + walletId: walletId, + type: 'activeAddresses', + }, { + w: 1 + }, next); + }, + function(next) { + self.db.collection(collections.CACHE).insert({ + walletId: walletId, + type: 'activeAddresses', + key: null + }, { + w: 1 + }, next); + }, + ], cb); +}; + Storage.prototype.storeActiveAddresses = function(walletId, addresses, cb) { var self = this; @@ -521,7 +546,6 @@ Storage.prototype.storeActiveAddresses = function(walletId, addresses, cb) { walletId: walletId, type: 'activeAddresses', key: address, - value: null, }; self.db.collection(collections.CACHE).update({ walletId: record.walletId, @@ -542,9 +566,9 @@ Storage.prototype.fetchActiveAddresses = function(walletId, cb) { type: 'activeAddresses', }).toArray(function(err, result) { if (err) return cb(err); - if (!result || _.isEmpty(result)) return cb(null, []); + if (_.isEmpty(result)) return cb(); - return cb(null, _.pluck(result, 'key')); + return cb(null, _.compact(_.pluck(result, 'key'))); }); }; diff --git a/test/integration/server.js b/test/integration/server.js index be7ee25..c0ea169 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1562,10 +1562,14 @@ describe('Wallet service', function() { clock.tick(7 * 24 * 3600 * 1000); helpers.createAddresses(server, wallet, 2, 0, function(addrs) { newAddrs = addrs; - helpers.stubUtxos(server, wallet, [1, 2], { - addresses: [oldAddrs[0], newAddrs[0]], - }, function() { - next(); + server._getActiveAddresses(function(err, active) { + should.not.exist(err); + should.not.exist(active); + helpers.stubUtxos(server, wallet, [1, 2], { + addresses: [oldAddrs[0], newAddrs[0]], + }, function() { + next(); + }); }); }); }, @@ -1575,7 +1579,7 @@ describe('Wallet service', function() { }, function(err, balance) { should.not.exist(err); should.exist(balance); - balance.totalAmount.should.equal(helpers.toSatoshi(2)); + balance.totalAmount.should.equal(helpers.toSatoshi(3)); next(); }); }, From 124742a7923885cc6f35fdd58891fc7d645a7235 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 14 Dec 2015 17:30:28 -0300 Subject: [PATCH 17/18] update active addresses from bc monitor --- lib/blockchainmonitor.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/blockchainmonitor.js b/lib/blockchainmonitor.js index c0d6336..938830b 100644 --- a/lib/blockchainmonitor.js +++ b/lib/blockchainmonitor.js @@ -92,7 +92,7 @@ BlockchainMonitor.prototype._handleTxId = function(data, processIt) { self.storage.fetchTxByHash(data.txid, function(err, txp) { if (err) { - log.error('Could not fetch tx the db'); + log.error('Could not fetch tx from the db'); return; } if (!txp || txp.status != 'accepted') return; @@ -164,13 +164,25 @@ BlockchainMonitor.prototype._handleTxOuts = function(data) { }, walletId: walletId, }); - self._storeAndBroadcastNotification(notification, next); + self._updateActiveAddresses(address, function() { + self._storeAndBroadcastNotification(notification, next); + }); }); }, function(err) { return; }); }; +BlockchainMonitor.prototype._updateActiveAddresses = function(address, cb) { + var self = this; + + self.storage.storeActiveAddresses(address.walletId, address.address, function(err) { + if (err) { + log.warn('Could not update wallet cache', err); + } + return cb(err); + }); +}; BlockchainMonitor.prototype._handleIncommingTx = function(data) { this._handleTxId(data); From 30e3e72263905875a2fd28a9131ae13fe0e6de05 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Mon, 14 Dec 2015 17:33:04 -0300 Subject: [PATCH 18/18] add log message --- lib/server.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/server.js b/lib/server.js index 2c8d479..d60bdab 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1081,6 +1081,7 @@ WalletService.prototype.getBalance = function(opts, cb) { if (!_.isArray(activeAddresses)) { return self._getBalanceOneStep(opts, cb); } else { + log.debug('Requesting partial balance for ' + activeAddresses.length + ' out of ' + nbAddresses + ' addresses'); self._getBalanceFromAddresses(activeAddresses, function(err, partialBalance) { if (err) return cb(err); cb(null, partialBalance);