From 9e0735efd9684e019493c6b38009b8892f639dc6 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Dec 2014 16:56:48 -0300 Subject: [PATCH 1/6] simple refactor on wallet balance to support caches --- js/models/Wallet.js | 121 +++++++++++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 46 deletions(-) diff --git a/js/models/Wallet.js b/js/models/Wallet.js index ebdd90d96..136a91fed 100644 --- a/js/models/Wallet.js +++ b/js/models/Wallet.js @@ -113,6 +113,8 @@ function Wallet(opts) { //one nonce for oneself, and then one nonce for each copayer this.network.setHexNonce(opts.networkNonce); this.network.setHexNonces(opts.networkNonces); + + this.cache = {}; } inherits(Wallet, events.EventEmitter); @@ -2076,6 +2078,7 @@ Wallet.estimatedFee = function(unspentCount) { return parseInt(estimatedSizeKb * bitcore.TransactionBuilder.FEE_PER_1000B_SAT); }; + /** * @callback {getBalanceCallback} * @param {string=} err - an error, if any @@ -2084,45 +2087,58 @@ Wallet.estimatedFee = function(unspentCount) { * @param {number} safeBalance - total number of satoshis in UTXOs that are not part of any TxProposal * @param {number} safeUnspentCount - total number of safe unspent Outputs that make this balance. */ + /** - * @desc Returns the balances for all addresses in Satoshis + * computeBalance + * + * @param safeUnspent + * @param unspent * @param {getBalanceCallback} cb */ -Wallet.prototype.getBalance = function(cb) { +Wallet.prototype.computeBalance = function(safeUnspent, unspent, cb) { var balance = 0; var safeBalance = 0; var balanceByAddr = {}; var COIN = coinUtil.COIN; + for (var i = 0; i < unspent.length; i++) { + var u = unspent[i]; + var amt = u.amount * COIN; + balance += amt; + balanceByAddr[u.address] = (balanceByAddr[u.address] || 0) + amt; + } + + // we multiply and divide by BIT to avoid rounding errors when adding + for (var a in balanceByAddr) { + balanceByAddr[a] = parseInt(balanceByAddr[a].toFixed(0), 10); + } + + balance = parseInt(balance.toFixed(0), 10); + + var safeUnspentCount = safeUnspent.length; + + for (var i = 0; i < safeUnspentCount; i++) { + var u = safeUnspent[i]; + var amt = u.amount * COIN; + safeBalance += amt; + } + + safeBalance = parseInt(safeBalance.toFixed(0), 10); + return cb(null, balance, balanceByAddr, safeBalance, safeUnspentCount); +}; + +/** + * @desc Returns the balances for all addresses in Satoshis + * @param {getBalanceCallback} cb + */ +Wallet.prototype.getBalance = function(cb) { + var self = this; + this.getUnspent(function(err, safeUnspent, unspent) { if (err) { return cb(err); } - - for (var i = 0; i < unspent.length; i++) { - var u = unspent[i]; - var amt = u.amount * COIN; - balance += amt; - balanceByAddr[u.address] = (balanceByAddr[u.address] || 0) + amt; - } - - // we multiply and divide by BIT to avoid rounding errors when adding - for (var a in balanceByAddr) { - balanceByAddr[a] = parseInt(balanceByAddr[a].toFixed(0), 10); - } - - balance = parseInt(balance.toFixed(0), 10); - - var safeUnspentCount = safeUnspent.length; - - for (var i = 0; i < safeUnspentCount; i++) { - var u = safeUnspent[i]; - var amt = u.amount * COIN; - safeBalance += amt; - } - - safeBalance = parseInt(safeBalance.toFixed(0), 10); - return cb(null, balance, balanceByAddr, safeBalance, safeUnspentCount); + self.computeBalance(safeUnspent, unspent, cb); }); }; @@ -2149,33 +2165,48 @@ Wallet.prototype.maxRejectCount = function() { * @param {getUnspentCallback} cb */ -// TODO: Can we add cache to getUnspent? Wallet.prototype.getUnspent = function(cb) { var self = this; + var addresses = this.getAddresses(); - - log.debug('Wallet ' + this.getName() + ': Getting unspents from ' + addresses.length + ' addresses'); this.blockchain.getUnspent(addresses, function(err, unspentList) { - - if (err) { + if (err) return cb(err); - } - var safeUnspendList = []; - var uu = self.txProposals.getUsedUnspent(self.maxRejectCount()); - - for (var i in unspentList) { - var u = unspentList[i]; - var name = u.txid + ',' + u.vout; - if (!uu[name] && (self.spendUnconfirmed || u.confirmations >= 1)) - safeUnspendList.push(u); - } - - return cb(null, safeUnspendList, unspentList); + self.cache.unspent = unspentList; + return self.computeUnspent(unspentList, cb); }); }; +/** + * + * @callback getUnspentCallback + * @param {string} error + * @param {Object[]} safeUnspendList + * @param {Object[]} unspentList + */ +/** + * computeUnspent + * + * @param unspentList List of unprocessed utxos. + * @param {getUnspentCallback} cb + */ +Wallet.prototype.computeUnspent = function(unspentList, cb) { + var self = this; + + var safeUnspendList = []; + var uu = this.txProposals.getUsedUnspent(this.maxRejectCount()); + + _.each(unspentList, function(u){ + var name = u.txid + ',' + u.vout; + if (!uu[name] && (self.spendUnconfirmed || u.confirmations >= 1)) + safeUnspendList.push(u); + }); + + return cb(null, safeUnspendList, unspentList); +}; + /** * spend * @@ -2227,8 +2258,6 @@ Wallet.prototype.spend = function(opts, cb) { preconditions.checkArgument(amountSat, 'no amount'); preconditions.checkArgument(toAddress, 'no address'); - // TODO no bajar los unspends de vuelta... - // this.getUnspent(function(err, safeUnspent) { if (err) { log.info(err); From 0544f757a1493dc6cf35c13536f8d8b967cb9afb Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Dec 2014 18:31:48 -0300 Subject: [PATCH 2/6] unspend cache! --- js/models/Wallet.js | 79 ++++++++++++++++++++++++----------- js/services/balanceService.js | 15 ++++--- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/js/models/Wallet.js b/js/models/Wallet.js index 136a91fed..1c4adf15a 100644 --- a/js/models/Wallet.js +++ b/js/models/Wallet.js @@ -98,7 +98,6 @@ function Wallet(opts) { this.syncedTimestamp = opts.syncedTimestamp || 0; this.lastMessageFrom = {}; - this.paymentRequestsCache = {}; var networkName = Wallet.obtainNetworkName(opts); @@ -114,7 +113,9 @@ function Wallet(opts) { this.network.setHexNonce(opts.networkNonce); this.network.setHexNonces(opts.networkNonces); - this.cache = {}; + this.cache = { + paymentRequests: {}, + }; } inherits(Wallet, events.EventEmitter); @@ -257,26 +258,27 @@ Wallet.prototype.seedCopayer = function(pubKey) { }; -Wallet.prototype._newAddresses = function(dontUpdateUx) { - this.subscribeToAddresses(); - this.emitAndKeepAlive('newAddresses', dontUpdateUx); -}; - - /** * @desc Handles an 'indexes' message. * * Processes the data using {@link HDParams#fromList} and merges it with the * {@link Wallet#publicKeyRing}. * - * @param {string} senderId - the sender id * @param {Object} data - the data recived, {@see HDParams#fromList} */ -Wallet.prototype._onIndexes = function(senderId, data) { - var inIndexes = HDParams.fromList(data.indexes); +Wallet.prototype._onIndexes = function(indexes, fromTxProposal) { + preconditions.checkArgument(indexes); + var inIndexes = HDParams.fromList(indexes); var hasChanged = this.publicKeyRing.mergeIndexes(inIndexes); if (hasChanged) { - this._newAddresses(); + + // If the new indexes come from TX proposals (change address) + // that addresses should be new. No need to clean cache. + if (!fromTxProposal) + this.clearUnspentCache(); + + this.subscribeToAddresses(); + this.emitAndKeepAlive('newAddresses'); } }; @@ -519,6 +521,10 @@ Wallet.prototype._onTxProposal = function(senderId, data) { preconditions.checkArgument(data.txProposal); var self = this; + if (data.indexes) { + this._onIndexes(data.indexes, true); + } + try { var incomingTx = self._txProposalFromUntrustedObj(data.txProposal, Wallet.builderOpts); var incomingNtxid = incomingTx.getId(); @@ -728,7 +734,7 @@ Wallet.prototype._onData = function(senderId, data, ts) { this._onSignature(senderId, data); break; case 'indexes': - this._onIndexes(senderId, data); + this._onIndexes(data.indexes); break; case 'addressbook': this._onAddressBook(senderId, data); @@ -888,6 +894,7 @@ Wallet.prototype._setupBlockchainHandlers = function() { log.debug('Setting Blockchain listeners for', this.getName()); self.blockchain.on('reconnect', function(attempts) { log.debug('Wallet:' + self.id + 'blockchain reconnect event'); + self.clearUnspentCache(); self.emitAndKeepAlive('insightReconnected'); }); @@ -899,12 +906,19 @@ Wallet.prototype._setupBlockchainHandlers = function() { self.blockchain.on('tx', function(tx) { log.debug('Wallet:' + self.id + ' blockchain tx event'); var addresses = self.getAddresses(); + // This should always be >=0 if (_.indexOf(addresses, tx.address) >= 0) { + self.clearUnspentCache(); self.emitAndKeepAlive('tx', tx.address, self.addressIsChange(tx.address)); } }); if (!self.spendUnconfirmed) { + + // TODO HERE should only clean utxos if there are some wallet + // transactions waiting for confirmation (ie confirmation < min confirmation) + self.clearUnspentCache(); + self.blockchain.on('block', self.emitAndKeepAlive.bind(self, 'balanceUpdated')); } } @@ -1206,10 +1220,12 @@ Wallet.prototype.sendAllTxProposals = function(recipients, sinceTs) { */ Wallet.prototype.sendTxProposal = function(ntxid, recipients) { preconditions.checkArgument(ntxid); + var indexes = HDParams.serialize(this.publicKeyRing.indexes); this._sendToPeers(recipients, { type: 'txProposal', txProposal: this.txProposals.get(ntxid).toObjTrim(), walletId: this.id, + indexes: indexes, }); }; @@ -1357,7 +1373,10 @@ Wallet.prototype.getName = function() { * @return {string[]} a list of all the addresses generated so far for the wallet */ Wallet.prototype._doGenerateAddress = function(isChange) { - return this.publicKeyRing.generateAddress(isChange, this.publicKey); + var addr = this.publicKeyRing.generateAddress(isChange, this.publicKey); + this.subscribeToAddresses(); + this.emitAndKeepAlive('newAddresses'); + return addr; }; /** @@ -1368,7 +1387,6 @@ Wallet.prototype._doGenerateAddress = function(isChange) { Wallet.prototype.generateAddress = function(isChange) { var addr = this._doGenerateAddress(isChange); this.sendIndexes(); - this._newAddresses(); return addr; }; @@ -1668,8 +1686,8 @@ Wallet.prototype.fetchPaymentRequest = function(options, cb) { preconditions.checkArgument(options.url.indexOf('http') == 0, 'Bad PayPro URL given:' + options.url); var self = this; - if (self.paymentRequestsCache[options.url]) - return cb(null, self.paymentRequestsCache[options.url]); + if (self.cache.paymentRequests[options.url]) + return cb(null, self.cache.paymentRequests[options.url]); this.httpUtil.request({ method: 'GET', @@ -1691,7 +1709,7 @@ Wallet.prototype.fetchPaymentRequest = function(options, cb) { log.debug('PayPro request data', merchantData); - self.paymentRequestsCache[options.url] = merchantData; + self.cache.paymentRequests[options.url] = merchantData; return cb(err, merchantData); }) .error(function(data, status) { @@ -2156,6 +2174,12 @@ Wallet.prototype.maxRejectCount = function() { return this.totalCopayers - this.requiredCopayers; }; + +Wallet.prototype.clearUnspentCache = function() { + log.debug('Cleaning unspent cache'); + this.cache.unspent = null; +}; + /** * @callback getUnspentCallback * @desc Get a list of unspent transaction outputs @@ -2168,6 +2192,13 @@ Wallet.prototype.maxRejectCount = function() { Wallet.prototype.getUnspent = function(cb) { var self = this; + + if (self.cache.unspent != null) { + log.debug('Wallet ' + this.getName() + ': Get unspent cache hit'); + return self.computeUnspent(self.cache.unspent, cb); + return + } + var addresses = this.getAddresses(); log.debug('Wallet ' + this.getName() + ': Getting unspents from ' + addresses.length + ' addresses'); this.blockchain.getUnspent(addresses, function(err, unspentList) { @@ -2175,7 +2206,7 @@ Wallet.prototype.getUnspent = function(cb) { return cb(err); self.cache.unspent = unspentList; - return self.computeUnspent(unspentList, cb); + return self.computeUnspent(self.cache.unspent, cb); }); }; @@ -2198,7 +2229,7 @@ Wallet.prototype.computeUnspent = function(unspentList, cb) { var safeUnspendList = []; var uu = this.txProposals.getUsedUnspent(this.maxRejectCount()); - _.each(unspentList, function(u){ + _.each(unspentList, function(u) { var name = u.txid + ',' + u.vout; if (!uu[name] && (self.spendUnconfirmed || u.confirmations >= 1)) safeUnspendList.push(u); @@ -2283,9 +2314,6 @@ Wallet.prototype.spend = function(opts, cb) { } log.debug('TXP Added: ', ntxid); - - self.sendIndexes(); - // Needs only one signature? Broadcast it! if (!self.requiresMultipleSignatures()) return self.issueTx(ntxid, cb); @@ -2395,7 +2423,6 @@ Wallet.prototype._createTxProposal = function(toAddress, amountSat, comment, utx signWith: keys, }); - console.log('[Wallet.js.2303]'); //TODO return txp; }; @@ -2420,7 +2447,9 @@ Wallet.prototype.updateIndexes = function(callback) { async.parallel(tasks, function(err) { if (err) callback(err); log.debug('Wallet:' + self.id + ' Indexes updated'); - self._newAddresses(); + this.clearUnspentCache(); + this.subscribeToAddresses(); + this.emitAndKeepAlive('newAddresses'); callback(); }); }; diff --git a/js/services/balanceService.js b/js/services/balanceService.js index 31c91ef26..3c09cff32 100644 --- a/js/services/balanceService.js +++ b/js/services/balanceService.js @@ -2,10 +2,11 @@ var bitcore = require('bitcore'); angular.module('copayApp.services') - .factory('balanceService', function($rootScope, $filter, rateService) { + .factory('balanceService', function($rootScope, $filter, $timeout, rateService) { var root = {}; var _balanceCache = {}; root.clearBalanceCache = function(w) { + w.clearUnspentCache(); delete _balanceCache[w.getId()]; }; @@ -48,7 +49,7 @@ angular.module('copayApp.services') r.totalBalanceAlternative = $filter('noFractionNumber')(totalBalanceAlternative, 2); r.lockedBalanceAlternative = $filter('noFractionNumber')(lockedBalanceAlternative, 2); r.alternativeConversionRate = $filter('noFractionNumber')(alternativeConversionRate, 2); - + r.alternativeBalanceAvailable = true; r.alternativeIsoCode = w.settings.alternativeIsoCode; }; @@ -63,7 +64,7 @@ angular.module('copayApp.services') w = w || $rootScope.wallet; if (!w || !w.isComplete()) return; - copay.logger.debug('Updating balance of:', w.getName(), isFocused); + copay.logger.debug('Updating balance of:', w.getName(), isFocused); var wid = w.getId(); @@ -80,13 +81,17 @@ angular.module('copayApp.services') root._fetchBalance(w, function(err, res) { if (err) throw err; - w.balanceInfo=_balanceCache[wid] = res; + w.balanceInfo = _balanceCache[wid] = res; w.balanceInfo.updating = false; if (isFocused) { $rootScope.updatingBalance = false; } - if (cb) cb(); + // we alwalys calltimeout because if balance is cached, we are still on the same + // execution path + if (cb) $timeout(function() { + return cb(); + }, 1); }); }; From a99e37bbdcbfd52815414ea9127ebd20be75d573 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Dec 2014 18:39:38 -0300 Subject: [PATCH 3/6] add wrapper function to onIndexes --- js/models/Wallet.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/js/models/Wallet.js b/js/models/Wallet.js index 1c4adf15a..70b45e637 100644 --- a/js/models/Wallet.js +++ b/js/models/Wallet.js @@ -266,7 +266,7 @@ Wallet.prototype.seedCopayer = function(pubKey) { * * @param {Object} data - the data recived, {@see HDParams#fromList} */ -Wallet.prototype._onIndexes = function(indexes, fromTxProposal) { +Wallet.prototype._doOnIndexes = function(indexes, fromTxProposal) { preconditions.checkArgument(indexes); var inIndexes = HDParams.fromList(indexes); var hasChanged = this.publicKeyRing.mergeIndexes(inIndexes); @@ -282,6 +282,11 @@ Wallet.prototype._onIndexes = function(indexes, fromTxProposal) { } }; + +Wallet.prototype._onIndexes = function(senderId, data) { + return this._doOnIndexes(data.indexes); +}; + /** * @desc * Changes wallet settings. The settings format is: From e4ebb535ca3ebbb96f903f2908fb71a88d4ac34f Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Dec 2014 19:07:54 -0300 Subject: [PATCH 4/6] fix tests --- js/models/Wallet.js | 6 +++--- js/util/HTTP.js | 1 + test/Wallet.js | 5 +++-- test/util.http.js | 8 ++++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/js/models/Wallet.js b/js/models/Wallet.js index 70b45e637..e83128774 100644 --- a/js/models/Wallet.js +++ b/js/models/Wallet.js @@ -2452,9 +2452,9 @@ Wallet.prototype.updateIndexes = function(callback) { async.parallel(tasks, function(err) { if (err) callback(err); log.debug('Wallet:' + self.id + ' Indexes updated'); - this.clearUnspentCache(); - this.subscribeToAddresses(); - this.emitAndKeepAlive('newAddresses'); + self.clearUnspentCache(); + self.subscribeToAddresses(); + self.emitAndKeepAlive('newAddresses'); callback(); }); }; diff --git a/js/util/HTTP.js b/js/util/HTTP.js index e900c5334..0c184fb8a 100644 --- a/js/util/HTTP.js +++ b/js/util/HTTP.js @@ -18,6 +18,7 @@ module.exports = { _success: function() {; }, _error: function(_, err) { + console.trace(err); throw err; } }; diff --git a/test/Wallet.js b/test/Wallet.js index 3e2c687a7..18446fe2d 100644 --- a/test/Wallet.js +++ b/test/Wallet.js @@ -687,6 +687,7 @@ describe('Wallet model', function() { amount: u } })); + w.clearUnspentCache(); w.getBalance(function(err, balance, balanceByAddr, safeBalance) { balance.should.equal(c.balance); done(); @@ -821,7 +822,7 @@ describe('Wallet model', function() { should.exist(id); status.should.equal(Wallet.TX_PROPOSAL_SENT); w.sendTxProposal.calledOnce.should.equal(true); - w.sendIndexes.calledOnce.should.equal(true); + w.sendIndexes.calledOnce.should.equal(false); done(); }); }); @@ -861,7 +862,7 @@ describe('Wallet model', function() { }, function(err, id, status) { err.should.equal('error'); w.sendTxProposal.calledOnce.should.equal(false); - w.sendIndexes.calledOnce.should.equal(true); + w.sendIndexes.calledOnce.should.equal(false); done(); }); }); diff --git a/test/util.http.js b/test/util.http.js index 18d7801dc..65453d1fe 100644 --- a/test/util.http.js +++ b/test/util.http.js @@ -75,15 +75,15 @@ describe('http utils', function() { done(); }); }); - it('should get with default error', function() { + it('should get with default error', function(done) { xhr.error = 1; - var ret = httpUtils.request({ xhr: xhr, method: 'GET', url: 'http://test', - }); - ret._error.should.throw() + }).error(function(){ + done(); + }) }); }); From e7d5bb6ef942685b63201a12a2cafab1bfd6b095 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Dec 2014 19:10:22 -0300 Subject: [PATCH 5/6] rm log --- js/controllers/copayers.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/js/controllers/copayers.js b/js/controllers/copayers.js index a6b38989c..5fdf0434e 100644 --- a/js/controllers/copayers.js +++ b/js/controllers/copayers.js @@ -2,9 +2,6 @@ angular.module('copayApp.controllers').controller('CopayersController', function($scope, $rootScope, $timeout, go) { - - console.log('[copayers.js.5]'); //TODO - $scope.init = function() { var w = $rootScope.wallet; $rootScope.title = 'Waiting copayers for ' + $rootScope.wallet.getName(); From e370644861d5b7446f3e5e2c87bd879c954b23aa Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Dec 2014 19:41:50 -0300 Subject: [PATCH 6/6] fix args --- js/models/Wallet.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/models/Wallet.js b/js/models/Wallet.js index e83128774..1b503e61d 100644 --- a/js/models/Wallet.js +++ b/js/models/Wallet.js @@ -527,7 +527,7 @@ Wallet.prototype._onTxProposal = function(senderId, data) { var self = this; if (data.indexes) { - this._onIndexes(data.indexes, true); + this._doOnIndexes(data.indexes, true); } try {