From 4c2455554540b9d0dd979bad329892279fddd8b9 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 30 Nov 2018 19:21:24 -0330 Subject: [PATCH] Save recent network balances in local storage (#5843) * Use selector for state.metamask.accounts in all cases. * Default to cached balance when selecting metamask accounts * Adds the cached-balances controller * Documentation and small codes fixes for #5843 Co-Authored-By: danjm --- app/scripts/controllers/cached-balances.js | 83 +++++++++++ app/scripts/metamask-controller.js | 9 ++ .../app/controllers/cached-balances-test.js | 137 ++++++++++++++++++ ui/app/app.js | 4 +- ui/app/components/account-menu/index.js | 3 +- ui/app/components/balance-component.js | 4 +- .../confirm-transaction-base.container.js | 3 +- .../create-account/connect-hardware/index.js | 4 +- .../create-account/import-account/json.js | 3 +- .../import-account/private-key.js | 3 +- ui/app/components/send/send.selectors.js | 15 +- .../transaction-view-balance.container.js | 11 +- ui/app/components/wallet-view.js | 2 +- ui/app/conf-tx.js | 3 +- ui/app/selectors.js | 34 +++-- 15 files changed, 290 insertions(+), 28 deletions(-) create mode 100644 app/scripts/controllers/cached-balances.js create mode 100644 test/unit/app/controllers/cached-balances-test.js diff --git a/app/scripts/controllers/cached-balances.js b/app/scripts/controllers/cached-balances.js new file mode 100644 index 000000000..925c45334 --- /dev/null +++ b/app/scripts/controllers/cached-balances.js @@ -0,0 +1,83 @@ +const ObservableStore = require('obs-store') +const extend = require('xtend') + +/** + * @typedef {Object} CachedBalancesOptions + * @property {Object} accountTracker An {@code AccountTracker} reference + * @property {Function} getNetwork A function to get the current network + * @property {Object} initState The initial controller state + */ + +/** + * Background controller responsible for maintaining + * a cache of account balances in local storage + */ +class CachedBalancesController { + /** + * Creates a new controller instance + * + * @param {CachedBalancesOptions} [opts] Controller configuration parameters + */ + constructor (opts = {}) { + const { accountTracker, getNetwork } = opts + + this.accountTracker = accountTracker + this.getNetwork = getNetwork + + const initState = extend({ + cachedBalances: {}, + }, opts.initState) + this.store = new ObservableStore(initState) + + this._registerUpdates() + } + + /** + * Updates the cachedBalances property for the current network. Cached balances will be updated to those in the passed accounts + * if balances in the passed accounts are truthy. + * + * @param {Object} obj The the recently updated accounts object for the current network + * @returns {Promise} + */ + async updateCachedBalances ({ accounts }) { + const network = await this.getNetwork() + const balancesToCache = await this._generateBalancesToCache(accounts, network) + this.store.updateState({ + cachedBalances: balancesToCache, + }) + } + + _generateBalancesToCache (newAccounts, currentNetwork) { + const { cachedBalances } = this.store.getState() + const currentNetworkBalancesToCache = { ...cachedBalances[currentNetwork] } + + Object.keys(newAccounts).forEach(accountID => { + const account = newAccounts[accountID] + + if (account.balance) { + currentNetworkBalancesToCache[accountID] = account.balance + } + }) + const balancesToCache = { + ...cachedBalances, + [currentNetwork]: currentNetworkBalancesToCache, + } + + return balancesToCache + } + + /** + * Sets up listeners and subscriptions which should trigger an update of cached balances. These updates will + * happen when the current account changes. Which happens on block updates, as well as on network and account + * selections. + * + * @private + * + */ + _registerUpdates () { + const update = this.updateCachedBalances.bind(this) + this.accountTracker.store.subscribe(update) + } +} + +module.exports = CachedBalancesController diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 7d6f06f92..fe806e47e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -29,6 +29,7 @@ const ShapeShiftController = require('./controllers/shapeshift') const AddressBookController = require('./controllers/address-book') const InfuraController = require('./controllers/infura') const BlacklistController = require('./controllers/blacklist') +const CachedBalancesController = require('./controllers/cached-balances') const RecentBlocksController = require('./controllers/recent-blocks') const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') @@ -142,6 +143,12 @@ module.exports = class MetamaskController extends EventEmitter { } }) + this.cachedBalancesController = new CachedBalancesController({ + accountTracker: this.accountTracker, + getNetwork: this.networkController.getNetworkState.bind(this.networkController), + initState: initState.CachedBalancesController, + }) + // ensure accountTracker updates balances after network change this.networkController.on('networkDidChange', () => { this.accountTracker._updateAccounts() @@ -241,6 +248,7 @@ module.exports = class MetamaskController extends EventEmitter { ShapeShiftController: this.shapeshiftController.store, NetworkController: this.networkController.store, InfuraController: this.infuraController.store, + CachedBalancesController: this.cachedBalancesController.store, }) this.memStore = new ComposableObservableStore(null, { @@ -248,6 +256,7 @@ module.exports = class MetamaskController extends EventEmitter { AccountTracker: this.accountTracker.store, TxController: this.txController.memStore, BalancesController: this.balancesController.store, + CachedBalancesController: this.cachedBalancesController.store, TokenRatesController: this.tokenRatesController.store, MessageManager: this.messageManager.memStore, PersonalMessageManager: this.personalMessageManager.memStore, diff --git a/test/unit/app/controllers/cached-balances-test.js b/test/unit/app/controllers/cached-balances-test.js new file mode 100644 index 000000000..27aeabba2 --- /dev/null +++ b/test/unit/app/controllers/cached-balances-test.js @@ -0,0 +1,137 @@ +const assert = require('assert') +const sinon = require('sinon') +const CachedBalancesController = require('../../../../app/scripts/controllers/cached-balances') + +describe('CachedBalancesController', () => { + describe('updateCachedBalances', () => { + it('should update the cached balances', async () => { + const controller = new CachedBalancesController({ + getNetwork: () => Promise.resolve(17), + accountTracker: { + store: { + subscribe: () => {}, + }, + }, + initState: { + cachedBalances: 'mockCachedBalances', + }, + }) + + controller._generateBalancesToCache = sinon.stub().callsFake(() => Promise.resolve('mockNewCachedBalances')) + + await controller.updateCachedBalances({ accounts: 'mockAccounts' }) + + assert.equal(controller._generateBalancesToCache.callCount, 1) + assert.deepEqual(controller._generateBalancesToCache.args[0], ['mockAccounts', 17]) + assert.equal(controller.store.getState().cachedBalances, 'mockNewCachedBalances') + }) + }) + + describe('_generateBalancesToCache', () => { + it('should generate updated account balances where the current network was updated', () => { + const controller = new CachedBalancesController({ + accountTracker: { + store: { + subscribe: () => {}, + }, + }, + initState: { + cachedBalances: { + 17: { + a: '0x1', + b: '0x2', + c: '0x3', + }, + 16: { + a: '0xa', + b: '0xb', + c: '0xc', + }, + }, + }, + }) + + const result = controller._generateBalancesToCache({ + a: { balance: '0x4' }, + b: { balance: null }, + c: { balance: '0x5' }, + }, 17) + + assert.deepEqual(result, { + 17: { + a: '0x4', + b: '0x2', + c: '0x5', + }, + 16: { + a: '0xa', + b: '0xb', + c: '0xc', + }, + }) + }) + + it('should generate updated account balances where the a new network was selected', () => { + const controller = new CachedBalancesController({ + accountTracker: { + store: { + subscribe: () => {}, + }, + }, + initState: { + cachedBalances: { + 17: { + a: '0x1', + b: '0x2', + c: '0x3', + }, + }, + }, + }) + + const result = controller._generateBalancesToCache({ + a: { balance: '0x4' }, + b: { balance: null }, + c: { balance: '0x5' }, + }, 16) + + assert.deepEqual(result, { + 17: { + a: '0x1', + b: '0x2', + c: '0x3', + }, + 16: { + a: '0x4', + c: '0x5', + }, + }) + }) + }) + + describe('_registerUpdates', () => { + it('should subscribe to the account tracker with the updateCachedBalances method', async () => { + const subscribeSpy = sinon.spy() + const controller = new CachedBalancesController({ + getNetwork: () => Promise.resolve(17), + accountTracker: { + store: { + subscribe: subscribeSpy, + }, + }, + }) + subscribeSpy.resetHistory() + + const updateCachedBalancesSpy = sinon.spy() + controller.updateCachedBalances = updateCachedBalancesSpy + controller._registerUpdates({ accounts: 'mockAccounts' }) + + assert.equal(subscribeSpy.callCount, 1) + + subscribeSpy.args[0][0]() + + assert.equal(updateCachedBalancesSpy.callCount, 1) + }) + }) + +}) diff --git a/ui/app/app.js b/ui/app/app.js index b3aff1f39..5405f8495 100644 --- a/ui/app/app.js +++ b/ui/app/app.js @@ -7,6 +7,7 @@ const h = require('react-hyperscript') const actions = require('./actions') const classnames = require('classnames') const log = require('loglevel') +const { getMetaMaskAccounts } = require('./selectors') // init const InitializeScreen = require('../../mascara/src/app/first-time').default @@ -275,9 +276,10 @@ function mapStateToProps (state) { loadingMessage, } = appState + const accounts = getMetaMaskAccounts(state) + const { identities, - accounts, address, keyrings, isInitialized, diff --git a/ui/app/components/account-menu/index.js b/ui/app/components/account-menu/index.js index 94eae8d07..e88389096 100644 --- a/ui/app/components/account-menu/index.js +++ b/ui/app/components/account-menu/index.js @@ -13,6 +13,7 @@ const Tooltip = require('../tooltip') import Identicon from '../identicon' import UserPreferencedCurrencyDisplay from '../user-preferenced-currency-display' import { PRIMARY } from '../../constants/common' +import { getMetaMaskAccounts } from '../../selectors' const { SETTINGS_ROUTE, @@ -41,7 +42,7 @@ function mapStateToProps (state) { isAccountMenuOpen: state.metamask.isAccountMenuOpen, keyrings: state.metamask.keyrings, identities: state.metamask.identities, - accounts: state.metamask.accounts, + accounts: getMetaMaskAccounts(state), } } diff --git a/ui/app/components/balance-component.js b/ui/app/components/balance-component.js index 4e2769ee8..78b51449e 100644 --- a/ui/app/components/balance-component.js +++ b/ui/app/components/balance-component.js @@ -6,14 +6,14 @@ import TokenBalance from './token-balance' import Identicon from './identicon' import UserPreferencedCurrencyDisplay from './user-preferenced-currency-display' import { PRIMARY, SECONDARY } from '../constants/common' -const { getNativeCurrency, getAssetImages, conversionRateSelector, getCurrentCurrency} = require('../selectors') +const { getNativeCurrency, getAssetImages, conversionRateSelector, getCurrentCurrency, getMetaMaskAccounts } = require('../selectors') const { formatBalance } = require('../util') module.exports = connect(mapStateToProps)(BalanceComponent) function mapStateToProps (state) { - const accounts = state.metamask.accounts + const accounts = getMetaMaskAccounts(state) const network = state.metamask.network const selectedAddress = state.metamask.selectedAddress || Object.keys(accounts)[0] const account = accounts[selectedAddress] diff --git a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js index 45bf62fb9..626143ac7 100644 --- a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -18,6 +18,7 @@ import { isBalanceSufficient } from '../../send/send.utils' import { conversionGreaterThan } from '../../../conversion-util' import { MIN_GAS_LIMIT_DEC } from '../../send/send.constants' import { addressSlicer, valuesFor } from '../../../util' +import { getMetaMaskAccounts } from '../../../selectors' const casedContractMap = Object.keys(contractMap).reduce((acc, base) => { return { @@ -47,11 +48,11 @@ const mapStateToProps = (state, props) => { } = confirmTransaction const { txParams = {}, lastGasPrice, id: transactionId } = txData const { from: fromAddress, to: txParamsToAddress } = txParams + const accounts = getMetaMaskAccounts(state) const { conversionRate, identities, currentCurrency, - accounts, selectedAddress, selectedAddressTxList, assetImages, diff --git a/ui/app/components/pages/create-account/connect-hardware/index.js b/ui/app/components/pages/create-account/connect-hardware/index.js index 4fe25f629..bd877fd4e 100644 --- a/ui/app/components/pages/create-account/connect-hardware/index.js +++ b/ui/app/components/pages/create-account/connect-hardware/index.js @@ -3,6 +3,7 @@ const PropTypes = require('prop-types') const h = require('react-hyperscript') const connect = require('react-redux').connect const actions = require('../../../../actions') +const { getMetaMaskAccounts } = require('../../../../selectors') const ConnectScreen = require('./connect-screen') const AccountList = require('./account-list') const { DEFAULT_ROUTE } = require('../../../../routes') @@ -224,8 +225,9 @@ ConnectHardwareForm.propTypes = { const mapStateToProps = state => { const { - metamask: { network, selectedAddress, identities = {}, accounts = [] }, + metamask: { network, selectedAddress, identities = {} }, } = state + const accounts = getMetaMaskAccounts(state) const numberOfExistingAccounts = Object.keys(identities).length const { appState: { defaultHdPaths }, diff --git a/ui/app/components/pages/create-account/import-account/json.js b/ui/app/components/pages/create-account/import-account/json.js index 90279bbbd..bb6771e4d 100644 --- a/ui/app/components/pages/create-account/import-account/json.js +++ b/ui/app/components/pages/create-account/import-account/json.js @@ -7,6 +7,7 @@ const connect = require('react-redux').connect const actions = require('../../../../actions') const FileInput = require('react-simple-file-input').default const { DEFAULT_ROUTE } = require('../../../../routes') +const { getMetaMaskAccounts } = require('../../../../selectors') const HELP_LINK = 'https://support.metamask.io/kb/article/7-importing-accounts' import Button from '../../../button' @@ -136,7 +137,7 @@ JsonImportSubview.propTypes = { const mapStateToProps = state => { return { error: state.appState.warning, - firstAddress: Object.keys(state.metamask.accounts)[0], + firstAddress: Object.keys(getMetaMaskAccounts(state))[0], } } diff --git a/ui/app/components/pages/create-account/import-account/private-key.js b/ui/app/components/pages/create-account/import-account/private-key.js index 8db1bfbdd..45068b96e 100644 --- a/ui/app/components/pages/create-account/import-account/private-key.js +++ b/ui/app/components/pages/create-account/import-account/private-key.js @@ -7,6 +7,7 @@ const PropTypes = require('prop-types') const connect = require('react-redux').connect const actions = require('../../../../actions') const { DEFAULT_ROUTE } = require('../../../../routes') +const { getMetaMaskAccounts } = require('../../../../selectors') import Button from '../../../button' PrivateKeyImportView.contextTypes = { @@ -22,7 +23,7 @@ module.exports = compose( function mapStateToProps (state) { return { error: state.appState.warning, - firstAddress: Object.keys(state.metamask.accounts)[0], + firstAddress: Object.keys(getMetaMaskAccounts(state))[0], } } diff --git a/ui/app/components/send/send.selectors.js b/ui/app/components/send/send.selectors.js index eb22a08b7..c217d848e 100644 --- a/ui/app/components/send/send.selectors.js +++ b/ui/app/components/send/send.selectors.js @@ -3,6 +3,9 @@ const abi = require('human-standard-token-abi') const { multiplyCurrencies, } = require('../../conversion-util') +const { + getMetaMaskAccounts, +} = require('../../selectors') const { estimateGasPriceFromRecentBlocks, } = require('./send.utils') @@ -54,10 +57,8 @@ const selectors = { module.exports = selectors function accountsWithSendEtherInfoSelector (state) { - const { - accounts, - identities, - } = state.metamask + const accounts = getMetaMaskAccounts(state) + const { identities } = state.metamask const accountsWithSendEtherInfo = Object.entries(accounts).map(([key, account]) => { return Object.assign({}, account, identities[key]) @@ -72,7 +73,7 @@ function accountsWithSendEtherInfoSelector (state) { // const autoAddTokensThreshold = 1 // const numberOfTransactions = state.metamask.selectedAddressTxList.length -// const numberOfAccounts = Object.keys(state.metamask.accounts).length +// const numberOfAccounts = Object.keys(getMetaMaskAccounts(state)).length // const numberOfTokensAdded = state.metamask.tokens.length // const userPassesThreshold = (numberOfTransactions > autoAddTransactionThreshold) && @@ -155,14 +156,14 @@ function getRecentBlocks (state) { } function getSelectedAccount (state) { - const accounts = state.metamask.accounts + const accounts = getMetaMaskAccounts(state) const selectedAddress = getSelectedAddress(state) return accounts[selectedAddress] } function getSelectedAddress (state) { - const selectedAddress = state.metamask.selectedAddress || Object.keys(state.metamask.accounts)[0] + const selectedAddress = state.metamask.selectedAddress || Object.keys(getMetaMaskAccounts(state))[0] return selectedAddress } diff --git a/ui/app/components/transaction-view-balance/transaction-view-balance.container.js b/ui/app/components/transaction-view-balance/transaction-view-balance.container.js index cb8078ec1..f9f05b0ae 100644 --- a/ui/app/components/transaction-view-balance/transaction-view-balance.container.js +++ b/ui/app/components/transaction-view-balance/transaction-view-balance.container.js @@ -2,12 +2,19 @@ import { connect } from 'react-redux' import { withRouter } from 'react-router-dom' import { compose } from 'recompose' import TransactionViewBalance from './transaction-view-balance.component' -import { getSelectedToken, getSelectedAddress, getNativeCurrency, getSelectedTokenAssetImage } from '../../selectors' +import { + getSelectedToken, + getSelectedAddress, + getNativeCurrency, + getSelectedTokenAssetImage, + getMetaMaskAccounts, +} from '../../selectors' import { showModal } from '../../actions' const mapStateToProps = state => { const selectedAddress = getSelectedAddress(state) - const { metamask: { network, accounts } } = state + const { metamask: { network } } = state + const accounts = getMetaMaskAccounts(state) const account = accounts[selectedAddress] const { balance } = account diff --git a/ui/app/components/wallet-view.js b/ui/app/components/wallet-view.js index e050e0ee6..8ad6637ae 100644 --- a/ui/app/components/wallet-view.js +++ b/ui/app/components/wallet-view.js @@ -38,7 +38,7 @@ function mapStateToProps (state) { network: state.metamask.network, sidebarOpen: state.appState.sidebar.isOpen, identities: state.metamask.identities, - accounts: state.metamask.accounts, + accounts: selectors.getMetaMaskAccounts(state), tokens: state.metamask.tokens, keyrings: state.metamask.keyrings, selectedAddress: selectors.getSelectedAddress(state), diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index 0784a872e..34f5466e2 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -12,6 +12,7 @@ const R = require('ramda') const SignatureRequest = require('./components/signature-request') const Loading = require('./components/loading-screen') const { DEFAULT_ROUTE } = require('./routes') +const { getMetaMaskAccounts } = require('./selectors') module.exports = compose( withRouter, @@ -28,7 +29,7 @@ function mapStateToProps (state) { return { identities: state.metamask.identities, - accounts: state.metamask.accounts, + accounts: getMetaMaskAccounts(state), selectedAddress: state.metamask.selectedAddress, unapprovedTxs: state.metamask.unapprovedTxs, unapprovedMsgs: state.metamask.unapprovedMsgs, diff --git a/ui/app/selectors.js b/ui/app/selectors.js index b518527c9..f99f14aa8 100644 --- a/ui/app/selectors.js +++ b/ui/app/selectors.js @@ -1,9 +1,7 @@ const abi = require('human-standard-token-abi') - import { transactionsSelector, } from './selectors/transactions' - const { multiplyCurrencies, } = require('./conversion-util') @@ -36,12 +34,13 @@ const selectors = { getCurrentViewContext, getTotalUnapprovedCount, preferencesSelector, + getMetaMaskAccounts, } module.exports = selectors function getSelectedAddress (state) { - const selectedAddress = state.metamask.selectedAddress || Object.keys(state.metamask.accounts)[0] + const selectedAddress = state.metamask.selectedAddress || Object.keys(getMetaMaskAccounts(state))[0] return selectedAddress } @@ -53,8 +52,27 @@ function getSelectedIdentity (state) { return identities[selectedAddress] } +function getMetaMaskAccounts (state) { + const currentAccounts = state.metamask.accounts + const cachedBalances = state.metamask.cachedBalances + const selectedAccounts = {} + + Object.keys(currentAccounts).forEach(accountID => { + const account = currentAccounts[accountID] + if (account && account.balance === null || account.balance === undefined) { + selectedAccounts[accountID] = { + ...account, + balance: cachedBalances[accountID], + } + } else { + selectedAccounts[accountID] = account + } + }) + return selectedAccounts +} + function getSelectedAccount (state) { - const accounts = state.metamask.accounts + const accounts = getMetaMaskAccounts(state) const selectedAddress = getSelectedAddress(state) return accounts[selectedAddress] @@ -102,10 +120,8 @@ function getAddressBook (state) { } function accountsWithSendEtherInfoSelector (state) { - const { - accounts, - identities, - } = state.metamask + const accounts = getMetaMaskAccounts(state) + const { identities } = state.metamask const accountsWithSendEtherInfo = Object.entries(accounts).map(([key, account]) => { return Object.assign({}, account, identities[key]) @@ -175,7 +191,7 @@ function autoAddToBetaUI (state) { const autoAddTokensThreshold = 1 const numberOfTransactions = state.metamask.selectedAddressTxList.length - const numberOfAccounts = Object.keys(state.metamask.accounts).length + const numberOfAccounts = Object.keys(getMetaMaskAccounts(state)).length const numberOfTokensAdded = state.metamask.tokens.length const userPassesThreshold = (numberOfTransactions > autoAddTransactionThreshold) &&