From fe8a0febce561abecde71142f05a574360304470 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Mon, 12 Aug 2019 09:57:16 -0230 Subject: [PATCH 01/15] Adds incoming transactions controller --- .../controllers/incoming-transactions.js | 181 ++++++++++++++++++ app/scripts/metamask-controller.js | 11 ++ 2 files changed, 192 insertions(+) create mode 100644 app/scripts/controllers/incoming-transactions.js diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js new file mode 100644 index 000000000000..499b48e67200 --- /dev/null +++ b/app/scripts/controllers/incoming-transactions.js @@ -0,0 +1,181 @@ +const ObservableStore = require('obs-store') +const extend = require('xtend') +const EthQuery = require('eth-query') +const log = require('loglevel') +const BN = require('bn.js') +const createId = require('../lib/random-id') +const { bnToHex } = require('../lib/util') +const { + MAINNET_CODE, + ROPSTEN_CODE, + RINKEYBY_CODE, + KOVAN_CODE, + ROPSTEN, + RINKEBY, + KOVAN, + MAINNET, +} = require('./network/enums') +const networkTypeToIdMap = { + [ROPSTEN]: ROPSTEN_CODE, + [RINKEBY]: RINKEYBY_CODE, + [KOVAN]: KOVAN_CODE, + [MAINNET]: MAINNET_CODE, +} + +class IncomingTransactionsController { + + constructor (opts = {}) { + const { + blockTracker, + provider, + networkController, + getSelectedAddress, + } = opts + this.blockTracker = blockTracker + this.ethQuery = new EthQuery(provider) + this.getSelectedAddress = getSelectedAddress + this.getCurrentNetwork = () => networkController.getProviderConfig().type + + const initState = extend({ + incomingTransactions: {}, + incomingTxlastFetchedBlocksByNetwork: { + [ROPSTEN]: null, + [RINKEBY]: null, + [KOVAN]: null, + [MAINNET]: null, + }, + }, opts.initState) + this.store = new ObservableStore(initState) + + const changeListener = async ({ newBlockNumberHex, networkType }) => { + try { + const { + incomingTransactions: currentIncomingTxs, + incomingTxlastFetchedBlocksByNetwork: currentBlocksByNetwork, + } = this.store.getState() + + const address = this.getSelectedAddress() + const network = networkType || this.getCurrentNetwork() + const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] + const blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberHex + + const { latestIncomingTxBlockNumber, txs } = await this.fetchAll(address, blockToFetchFrom, network) + + const newLatestBlockHashByNetwork = parseInt(latestIncomingTxBlockNumber, 10) > parseInt(blockToFetchFrom, 10) + ? latestIncomingTxBlockNumber + : blockToFetchFrom + + const newIncomingTransactions = { + ...currentIncomingTxs, + } + txs.forEach(tx => { newIncomingTransactions[tx.hash] = tx }) + + this.store.updateState({ + incomingTxlastFetchedBlocksByNetwork: { + ...currentBlocksByNetwork, + [network]: newLatestBlockHashByNetwork, + }, + incomingTransactions: newIncomingTransactions, + }) + } catch (err) { + log.error(err) + } + } + + const blockListener = async (newBlockNumberHex) => { + changeListener({ newBlockNumberHex: parseInt(newBlockNumberHex, 16) }) + } + const networkListener = async (newType) => { + changeListener({ networkType: newType }) + } + + networkController.on('networkDidChange', networkListener) + this.blockTracker.on('latest', blockListener) + } + + async fetchAll (address, fromBlock, networkType) { + let etherscanSubdomain = 'api' + const currentNetworkID = networkTypeToIdMap[networkType] + const supportedNetworkTypes = [ROPSTEN, RINKEBY, KOVAN, MAINNET] + + if (supportedNetworkTypes.indexOf(networkType) === -1) { + return + } + + if (networkType !== 'mainnet') { + etherscanSubdomain = `api-${networkType}` + } + const apiUrl = `https://${etherscanSubdomain}.etherscan.io` + + if (!apiUrl) { + return + } + let url = `${apiUrl}/api?module=account&action=txlist&address=${address}&tag=latest&page=1` + + if (fromBlock) { + url += `&startBlock=${parseInt(fromBlock, 10)}` + } + const response = await fetch(url) + const parsedResponse = await response.json() + + if (parsedResponse.status !== '0' && parsedResponse.result.length > 0) { + const remoteTxList = {} + const remoteTxs = [] + parsedResponse.result.forEach((tx) => { + if (!remoteTxList[tx.hash]) { + remoteTxs.push(this.normalizeTxFromEtherscan(tx, currentNetworkID)) + remoteTxList[tx.hash] = 1 + } + }) + + const allTxs = [ ...remoteTxs ] + allTxs.sort((a, b) => (a.time < b.time ? -1 : 1)) + + let latestIncomingTxBlockNumber + allTxs.forEach((tx) => { + if (tx.txParams.to && tx.txParams.to.toLowerCase() === address.toLowerCase()) { + if ( + tx.blockNumber && + (!latestIncomingTxBlockNumber || + parseInt(latestIncomingTxBlockNumber, 10) < parseInt(tx.blockNumber, 10)) + ) { + latestIncomingTxBlockNumber = tx.blockNumber + } + } + }) + this.store.updateState({ transactions: allTxs }) + return { + latestIncomingTxBlockNumber, + txs: allTxs, + } + } + return { + latestIncomingTxBlockNumber: '0', + txs: [], + } + } + + normalizeTxFromEtherscan (txMeta, currentNetworkID) { + const time = parseInt(txMeta.timeStamp, 10) * 1000 + const status = txMeta.isError === '0' ? 'confirmed' : 'failed' + return { + blockNumber: txMeta.blockNumber, + id: createId(), + metamaskNetworkId: currentNetworkID, + status, + time, + txParams: { + from: txMeta.from, + gas: bnToHex(new BN(txMeta.gas)), + gasPrice: bnToHex(new BN(txMeta.gasPrice)), + nonce: bnToHex(new BN(txMeta.nonce)), + to: txMeta.to, + value: bnToHex(new BN(txMeta.value)), + }, + hash: txMeta.hash, + transactionCategory: 'incoming', + } + } +} + +module.exports = IncomingTransactionsController diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e9299d5f82af..0dbdc8aa1adb 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -30,6 +30,7 @@ const InfuraController = require('./controllers/infura') const CachedBalancesController = require('./controllers/cached-balances') const OnboardingController = require('./controllers/onboarding') const RecentBlocksController = require('./controllers/recent-blocks') +const IncomingTransactionsController = require('./controllers/incoming-transactions') const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') const TypedMessageManager = require('./lib/typed-message-manager') @@ -137,6 +138,14 @@ module.exports = class MetamaskController extends EventEmitter { networkController: this.networkController, }) + this.incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: this.blockTracker, + provider: this.provider, + networkController: this.networkController, + getSelectedAddress: this.preferencesController.getSelectedAddress.bind(this.preferencesController), + initState: initState.IncomingTransactionsController, + }) + // account tracker watches balances, nonces, and any code at their address. this.accountTracker = new AccountTracker({ provider: this.provider, @@ -270,6 +279,7 @@ module.exports = class MetamaskController extends EventEmitter { CachedBalancesController: this.cachedBalancesController.store, OnboardingController: this.onboardingController.store, ProviderApprovalController: this.providerApprovalController.store, + IncomingTransactionsController: this.incomingTransactionsController.store, }) this.memStore = new ComposableObservableStore(null, { @@ -294,6 +304,7 @@ module.exports = class MetamaskController extends EventEmitter { // ProviderApprovalController ProviderApprovalController: this.providerApprovalController.store, ProviderApprovalControllerMemStore: this.providerApprovalController.memStore, + IncomingTransactionsController: this.incomingTransactionsController.store, }) this.memStore.subscribe(this.sendUpdate.bind(this)) } From 2ebbedc48e7841cc2159377eca65411028e0d8ab Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Mon, 12 Aug 2019 09:57:27 -0230 Subject: [PATCH 02/15] Adds UX for incoming transactions --- app/_locales/en/messages.json | 3 ++ .../transaction-list-item.component.js | 5 ++- .../transaction-list-item.container.js | 12 ++++-- .../app/transaction-list/index.scss | 33 ++++++++++++---- .../transaction-list.component.js | 38 ++++++++++++++++--- .../transaction-list.container.js | 11 +++++- ui/app/helpers/constants/transactions.js | 1 + ui/app/helpers/utils/transactions.util.js | 5 +++ 8 files changed, 89 insertions(+), 19 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 2b957129d669..52a0d17a0c75 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1280,6 +1280,9 @@ "receive": { "message": "Receive" }, + "received": { + "message": "Received" + }, "recents": { "message": "Recents" }, diff --git a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js index 8bdb6a313be6..f4f8d97b27bb 100644 --- a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js +++ b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js @@ -36,6 +36,7 @@ export default class TransactionListItem extends PureComponent { rpcPrefs: PropTypes.object, data: PropTypes.string, getContractMethodData: PropTypes.func, + isDeposit: PropTypes.bool, } static defaultProps = { @@ -117,7 +118,7 @@ export default class TransactionListItem extends PureComponent { } renderPrimaryCurrency () { - const { token, primaryTransaction: { txParams: { data } = {} } = {}, value } = this.props + const { token, primaryTransaction: { txParams: { data } = {} } = {}, value, isDeposit } = this.props return token ? ( @@ -132,7 +133,7 @@ export default class TransactionListItem extends PureComponent { className="transaction-list-item__amount transaction-list-item__amount--primary" value={value} type={PRIMARY} - prefix="-" + prefix={isDeposit ? '' : '-'} /> ) } diff --git a/ui/app/components/app/transaction-list-item/transaction-list-item.container.js b/ui/app/components/app/transaction-list-item/transaction-list-item.container.js index 1675958aa9f2..27b9e2608c10 100644 --- a/ui/app/components/app/transaction-list-item/transaction-list-item.container.js +++ b/ui/app/components/app/transaction-list-item/transaction-list-item.container.js @@ -21,8 +21,10 @@ const mapStateToProps = (state, ownProps) => { const { showFiatInTestnets } = preferencesSelector(state) const isMainnet = getIsMainnet(state) const { transactionGroup: { primaryTransaction } = {} } = ownProps - const { txParams: { gas: gasLimit, gasPrice, data } = {} } = primaryTransaction - const selectedAccountBalance = accounts[getSelectedAddress(state)].balance + const { txParams: { gas: gasLimit, gasPrice, data, to } = {} } = primaryTransaction + const selectedAddress = getSelectedAddress(state) + const selectedAccountBalance = accounts[selectedAddress].balance + const isDeposit = selectedAddress === to const selectRpcInfo = frequentRpcListDetail.find(rpcInfo => rpcInfo.rpcUrl === provider.rpcTarget) const { rpcPrefs } = selectRpcInfo || {} @@ -42,6 +44,7 @@ const mapStateToProps = (state, ownProps) => { selectedAccountBalance, hasEnoughCancelGas, rpcPrefs, + isDeposit, } } @@ -68,12 +71,13 @@ const mapDispatchToProps = dispatch => { const mergeProps = (stateProps, dispatchProps, ownProps) => { const { transactionGroup: { primaryTransaction, initialTransaction } = {} } = ownProps + const { isDeposit } = stateProps const { retryTransaction, ...restDispatchProps } = dispatchProps - const { txParams: { nonce, data } = {}, time } = initialTransaction + const { txParams: { nonce, data } = {}, time = 0 } = initialTransaction const { txParams: { value } = {} } = primaryTransaction const tokenData = data && getTokenData(data) - const nonceAndDate = nonce ? `#${hexToDecimal(nonce)} - ${formatDate(time)}` : formatDate(time) + const nonceAndDate = nonce && !isDeposit ? `#${hexToDecimal(nonce)} - ${formatDate(time)}` : formatDate(time) return { ...stateProps, diff --git a/ui/app/components/app/transaction-list/index.scss b/ui/app/components/app/transaction-list/index.scss index 42eddd31e3a9..7535137e2169 100644 --- a/ui/app/components/app/transaction-list/index.scss +++ b/ui/app/components/app/transaction-list/index.scss @@ -11,15 +11,34 @@ } &__header { - flex: 0 0 auto; - font-size: 14px; - line-height: 20px; - color: $Grey-400; border-bottom: 1px solid $Grey-100; - padding: 8px 0 8px 20px; - @media screen and (max-width: $break-small) { - padding: 8px 0 8px 16px; + &__tabs { + display: flex; + } + + &__tab, + &__tab--selected { + flex: 0 0 auto; + font-size: 14px; + line-height: 20px; + color: $Grey-400; + padding: 8px 0 8px 20px; + cursor: pointer; + + &:hover { + font-weight: bold; + } + + @media screen and (max-width: $break-small) { + padding: 8px 0 8px 16px; + } + } + + &__tab--selected { + font-weight: bold; + color: $Blue-400; + cursor: auto; } } diff --git a/ui/app/components/app/transaction-list/transaction-list.component.js b/ui/app/components/app/transaction-list/transaction-list.component.js index 157e7200b413..0573c24fa60d 100644 --- a/ui/app/components/app/transaction-list/transaction-list.component.js +++ b/ui/app/components/app/transaction-list/transaction-list.component.js @@ -1,5 +1,6 @@ import React, { PureComponent } from 'react' import PropTypes from 'prop-types' +import classnames from 'classnames' import TransactionListItem from '../transaction-list-item' import ShapeShiftTransactionListItem from '../shift-list-item' import { TRANSACTION_TYPE_SHAPESHIFT } from '../../../helpers/constants/transactions' @@ -22,6 +23,11 @@ export default class TransactionList extends PureComponent { selectedToken: PropTypes.object, updateNetworkNonce: PropTypes.func, assetImages: PropTypes.object, + incomingTransactions: PropTypes.array, + } + + state = { + selectedTab: 'history', } componentDidMount () { @@ -51,9 +57,13 @@ export default class TransactionList extends PureComponent { renderTransactions () { const { t } = this.context - const { pendingTransactions = [], completedTransactions = [] } = this.props + const { selectedTab } = this.state + const { pendingTransactions = [], completedTransactions = [], incomingTransactions = [] } = this.props const pendingLength = pendingTransactions.length + const selectedTabIsIncoming = selectedTab === 'incoming' + const primaryTransactionsToRender = selectedTabIsIncoming ? incomingTransactions : completedTransactions + return (
{ @@ -72,11 +82,29 @@ export default class TransactionList extends PureComponent { }
- { t('history') } +
+
this.setState({ selectedTab: 'history' })} + className={classnames({ + 'transaction-list__header__tab--selected': selectedTab === 'history', + 'transaction-list__header__tab': selectedTab !== 'history', + })}> + { t('history') } +
+
|
+
this.setState({ selectedTab: 'incoming' })} + className={classnames({ + 'transaction-list__header__tab--selected': selectedTabIsIncoming, + 'transaction-list__header__tab': selectedTab !== 'incoming', + })}> + { t('received') } +
+
{ - completedTransactions.length > 0 - ? completedTransactions.map((transactionGroup, index) => ( + primaryTransactionsToRender.length > 0 + ? primaryTransactionsToRender.map((transactionGroup, index) => ( this.renderTransaction(transactionGroup, index) )) : this.renderEmpty() @@ -90,7 +118,7 @@ export default class TransactionList extends PureComponent { const { selectedToken, assetImages } = this.props const { transactions = [] } = transactionGroup - return transactions[0].key === TRANSACTION_TYPE_SHAPESHIFT + return transactions[0] && transactions[0].key === TRANSACTION_TYPE_SHAPESHIFT ? ( { + const selectedAddress = getSelectedAddress(state) + const _incomingTransactions = Object.values(state.metamask.incomingTransactions) + .filter(({ txParams }) => txParams.to === selectedAddress) + return { completedTransactions: nonceSortedCompletedTransactionsSelector(state), pendingTransactions: nonceSortedPendingTransactionsSelector(state), selectedToken: selectedTokenSelector(state), - selectedAddress: getSelectedAddress(state), + selectedAddress, assetImages: getAssetImages(state), + incomingTransactions: _incomingTransactions.map(incomingTx => ({ + transactions: _incomingTransactions, + primaryTransaction: incomingTx, + initialTransaction: incomingTx, + })), } } diff --git a/ui/app/helpers/constants/transactions.js b/ui/app/helpers/constants/transactions.js index d0a819b9b238..e91e56ddc4ca 100644 --- a/ui/app/helpers/constants/transactions.js +++ b/ui/app/helpers/constants/transactions.js @@ -20,5 +20,6 @@ export const TRANSFER_FROM_ACTION_KEY = 'transferFrom' export const SIGNATURE_REQUEST_KEY = 'signatureRequest' export const CONTRACT_INTERACTION_KEY = 'contractInteraction' export const CANCEL_ATTEMPT_ACTION_KEY = 'cancelAttempt' +export const DEPOSIT_TRANSACTION_KEY = 'deposit' export const TRANSACTION_TYPE_SHAPESHIFT = 'shapeshift' diff --git a/ui/app/helpers/utils/transactions.util.js b/ui/app/helpers/utils/transactions.util.js index b65bda5b2dd1..cb347ffaaf1b 100644 --- a/ui/app/helpers/utils/transactions.util.js +++ b/ui/app/helpers/utils/transactions.util.js @@ -21,6 +21,7 @@ import { SIGNATURE_REQUEST_KEY, CONTRACT_INTERACTION_KEY, CANCEL_ATTEMPT_ACTION_KEY, + DEPOSIT_TRANSACTION_KEY, } from '../constants/transactions' import log from 'loglevel' @@ -124,6 +125,10 @@ export function isTokenMethodAction (transactionCategory) { export function getTransactionActionKey (transaction) { const { msgParams, type, transactionCategory } = transaction + if (transactionCategory === 'incoming') { + return DEPOSIT_TRANSACTION_KEY + } + if (type === 'cancel') { return CANCEL_ATTEMPT_ACTION_KEY } From b8c6a026a1b24da07edbabb4758aadce4c60550e Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Mon, 12 Aug 2019 13:33:26 -0230 Subject: [PATCH 03/15] Update fixture data for integration tests to accomodate incoming tx addition --- development/states/confirm-sig-requests.json | 1 + development/states/currency-localization.json | 1 + development/states/send-new-ui.json | 1 + development/states/tx-list-items.json | 1 + 4 files changed, 4 insertions(+) diff --git a/development/states/confirm-sig-requests.json b/development/states/confirm-sig-requests.json index 16199f48fa4b..ae7f3454de41 100644 --- a/development/states/confirm-sig-requests.json +++ b/development/states/confirm-sig-requests.json @@ -63,6 +63,7 @@ ], "tokens": [], "transactions": {}, + "incomingTransactions": {}, "selectedAddressTxList": [], "unapprovedTxs": {}, "unapprovedMsgs": { diff --git a/development/states/currency-localization.json b/development/states/currency-localization.json index 9d5f771c2d7f..dff527f5aca1 100644 --- a/development/states/currency-localization.json +++ b/development/states/currency-localization.json @@ -64,6 +64,7 @@ ], "tokens": [], "transactions": {}, + "incomingTransactions": {}, "selectedAddressTxList": [], "unapprovedMsgs": {}, "unapprovedMsgCount": 0, diff --git a/development/states/send-new-ui.json b/development/states/send-new-ui.json index bcfc762210ee..69b4b0568cf3 100644 --- a/development/states/send-new-ui.json +++ b/development/states/send-new-ui.json @@ -28,6 +28,7 @@ "conversionRate": 1200.88200327, "conversionDate": 1489013762, "noActiveNotices": true, + "incomingTransactions": {}, "frequentRpcList": [], "network": "3", "accounts": { diff --git a/development/states/tx-list-items.json b/development/states/tx-list-items.json index fd60003ba158..08d1cf263751 100644 --- a/development/states/tx-list-items.json +++ b/development/states/tx-list-items.json @@ -64,6 +64,7 @@ ], "tokens": [], "transactions": {}, + "incomingTransactions": {}, "selectedAddressTxList": [ { "err": { From efc7af13fc60689b458647de807f69de91912207 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Wed, 14 Aug 2019 14:48:40 -0230 Subject: [PATCH 04/15] Improve lastFetchBlockByCurrentNetwork logic in incoming-transactions controller --- .../controllers/incoming-transactions.js | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 499b48e67200..4c02c4f7a5f0 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -47,7 +47,7 @@ class IncomingTransactionsController { }, opts.initState) this.store = new ObservableStore(initState) - const changeListener = async ({ newBlockNumberHex, networkType }) => { + const changeListener = async ({ newBlockNumberDec, networkType } = {}) => { try { const { incomingTransactions: currentIncomingTxs, @@ -57,14 +57,11 @@ class IncomingTransactionsController { const address = this.getSelectedAddress() const network = networkType || this.getCurrentNetwork() const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] - const blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberHex - + const blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberDec const { latestIncomingTxBlockNumber, txs } = await this.fetchAll(address, blockToFetchFrom, network) - - const newLatestBlockHashByNetwork = parseInt(latestIncomingTxBlockNumber, 10) > parseInt(blockToFetchFrom, 10) - ? latestIncomingTxBlockNumber - : blockToFetchFrom - + const newLatestBlockHashByNetwork = latestIncomingTxBlockNumber + ? parseInt(latestIncomingTxBlockNumber, 10) + 1 + : blockToFetchFrom + 1 const newIncomingTransactions = { ...currentIncomingTxs, } @@ -82,10 +79,10 @@ class IncomingTransactionsController { } } - const blockListener = async (newBlockNumberHex) => { - changeListener({ newBlockNumberHex: parseInt(newBlockNumberHex, 16) }) + const blockListener = (newBlockNumberHex) => { + changeListener({ newBlockNumberDec: parseInt(newBlockNumberHex, 16) }) } - const networkListener = async (newType) => { + const networkListener = (newType) => { changeListener({ networkType: newType }) } @@ -150,7 +147,7 @@ class IncomingTransactionsController { } } return { - latestIncomingTxBlockNumber: '0', + latestIncomingTxBlockNumber: null, txs: [], } } From 45a24a129450589033318a698fe772248cc1e94e Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Wed, 14 Aug 2019 14:52:31 -0230 Subject: [PATCH 05/15] Refactor incoming tx UX to only use the single tx list / tab --- .../transaction-list.component.js | 40 +++---------------- .../transaction-list.container.js | 11 +---- ui/app/selectors/transactions.js | 37 +++++++++++------ 3 files changed, 31 insertions(+), 57 deletions(-) diff --git a/ui/app/components/app/transaction-list/transaction-list.component.js b/ui/app/components/app/transaction-list/transaction-list.component.js index 0573c24fa60d..ea604ade8f82 100644 --- a/ui/app/components/app/transaction-list/transaction-list.component.js +++ b/ui/app/components/app/transaction-list/transaction-list.component.js @@ -1,6 +1,5 @@ import React, { PureComponent } from 'react' import PropTypes from 'prop-types' -import classnames from 'classnames' import TransactionListItem from '../transaction-list-item' import ShapeShiftTransactionListItem from '../shift-list-item' import { TRANSACTION_TYPE_SHAPESHIFT } from '../../../helpers/constants/transactions' @@ -23,11 +22,6 @@ export default class TransactionList extends PureComponent { selectedToken: PropTypes.object, updateNetworkNonce: PropTypes.func, assetImages: PropTypes.object, - incomingTransactions: PropTypes.array, - } - - state = { - selectedTab: 'history', } componentDidMount () { @@ -57,13 +51,9 @@ export default class TransactionList extends PureComponent { renderTransactions () { const { t } = this.context - const { selectedTab } = this.state - const { pendingTransactions = [], completedTransactions = [], incomingTransactions = [] } = this.props + const { pendingTransactions = [], completedTransactions = [] } = this.props const pendingLength = pendingTransactions.length - const selectedTabIsIncoming = selectedTab === 'incoming' - const primaryTransactionsToRender = selectedTabIsIncoming ? incomingTransactions : completedTransactions - return (
{ @@ -82,29 +72,11 @@ export default class TransactionList extends PureComponent { }
-
-
this.setState({ selectedTab: 'history' })} - className={classnames({ - 'transaction-list__header__tab--selected': selectedTab === 'history', - 'transaction-list__header__tab': selectedTab !== 'history', - })}> - { t('history') } -
-
|
-
this.setState({ selectedTab: 'incoming' })} - className={classnames({ - 'transaction-list__header__tab--selected': selectedTabIsIncoming, - 'transaction-list__header__tab': selectedTab !== 'incoming', - })}> - { t('received') } -
-
+ { t('history') }
{ - primaryTransactionsToRender.length > 0 - ? primaryTransactionsToRender.map((transactionGroup, index) => ( + completedTransactions.length > 0 + ? completedTransactions.map((transactionGroup, index) => ( this.renderTransaction(transactionGroup, index) )) : this.renderEmpty() @@ -118,7 +90,7 @@ export default class TransactionList extends PureComponent { const { selectedToken, assetImages } = this.props const { transactions = [] } = transactionGroup - return transactions[0] && transactions[0].key === TRANSACTION_TYPE_SHAPESHIFT + return transactions[0].key === TRANSACTION_TYPE_SHAPESHIFT ? ( ) } -} +} \ No newline at end of file diff --git a/ui/app/components/app/transaction-list/transaction-list.container.js b/ui/app/components/app/transaction-list/transaction-list.container.js index 08dae939db77..67a24588b6ac 100644 --- a/ui/app/components/app/transaction-list/transaction-list.container.js +++ b/ui/app/components/app/transaction-list/transaction-list.container.js @@ -11,21 +11,12 @@ import { selectedTokenSelector } from '../../../selectors/tokens' import { updateNetworkNonce } from '../../../store/actions' const mapStateToProps = state => { - const selectedAddress = getSelectedAddress(state) - const _incomingTransactions = Object.values(state.metamask.incomingTransactions) - .filter(({ txParams }) => txParams.to === selectedAddress) - return { completedTransactions: nonceSortedCompletedTransactionsSelector(state), pendingTransactions: nonceSortedPendingTransactionsSelector(state), selectedToken: selectedTokenSelector(state), - selectedAddress, + selectedAddress: getSelectedAddress(state), assetImages: getAssetImages(state), - incomingTransactions: _incomingTransactions.map(incomingTx => ({ - transactions: _incomingTransactions, - primaryTransaction: incomingTx, - initialTransaction: incomingTx, - })), } } diff --git a/ui/app/selectors/transactions.js b/ui/app/selectors/transactions.js index b1d27b333e87..3eed1b075228 100644 --- a/ui/app/selectors/transactions.js +++ b/ui/app/selectors/transactions.js @@ -10,11 +10,16 @@ import { TRANSACTION_TYPE_RETRY, } from '../../../app/scripts/controllers/transactions/enums' import { hexToDecimal } from '../helpers/utils/conversions.util' - import { selectedTokenAddressSelector } from './tokens' import txHelper from '../../lib/tx-helper' export const shapeShiftTxListSelector = state => state.metamask.shapeShiftTxList + +export const incomingTxListSelector = state => { + const selectedAddress = state.metamask.selectedAddress + return Object.values(state.metamask.incomingTransactions) + .filter(({ txParams }) => txParams.to === selectedAddress) +} export const unapprovedMsgsSelector = state => state.metamask.unapprovedMsgs export const selectedAddressTxListSelector = state => state.metamask.selectedAddressTxList export const unapprovedPersonalMsgsSelector = state => state.metamask.unapprovedPersonalMsgs @@ -55,9 +60,10 @@ export const transactionsSelector = createSelector( selectedTokenAddressSelector, unapprovedMessagesSelector, shapeShiftTxListSelector, + incomingTxListSelector, selectedAddressTxListSelector, - (selectedTokenAddress, unapprovedMessages = [], shapeShiftTxList = [], transactions = []) => { - const txsToRender = transactions.concat(unapprovedMessages, shapeShiftTxList) + (selectedTokenAddress, unapprovedMessages = [], shapeShiftTxList = [], incomingTxList = [], transactions = []) => { + const txsToRender = transactions.concat(unapprovedMessages, shapeShiftTxList, incomingTxList) return selectedTokenAddress ? txsToRender @@ -158,17 +164,18 @@ const insertTransactionGroupByTime = (transactionGroups, transactionGroup) => { } /** - * @name mergeShapeshiftTransactionGroups + * @name mergeNonNonceTransactionGroups * @private - * @description Inserts (mutates) shapeshift transactionGroups into an array of nonce-ordered - * transactionGroups by time. Shapeshift transactionGroups need to be sorted by time within the list - * of transactions as they do not have nonces. + * @description Inserts (mutates) transactionGroups that are not to be ordered by nonce into an array + * of nonce-ordered transactionGroups by time. Shapeshift transactionGroups need to be sorted by time + * within the list of transactions as they do not have nonces. * @param {transactionGroup[]} orderedTransactionGroups - Array of transactionGroups ordered by * nonce. - * @param {transactionGroup[]} shapeshiftTransactionGroups - Array of shapeshift transactionGroups + * @param {transactionGroup[]} nonNonceTransactionGroups - Array of transactionGroups not intended to be ordered by nonce, + * but intended to be ordered by timestamp */ -const mergeShapeshiftTransactionGroups = (orderedTransactionGroups, shapeshiftTransactionGroups) => { - shapeshiftTransactionGroups.forEach(shapeshiftGroup => { +const mergeNonNonceTransactionGroups = (orderedTransactionGroups, nonNonceTransactionGroups) => { + nonNonceTransactionGroups.forEach(shapeshiftGroup => { insertTransactionGroupByTime(orderedTransactionGroups, shapeshiftGroup) }) } @@ -183,13 +190,14 @@ export const nonceSortedTransactionsSelector = createSelector( (transactions = []) => { const unapprovedTransactionGroups = [] const shapeshiftTransactionGroups = [] + const incomingTransactionGroups = [] const orderedNonces = [] const nonceToTransactionsMap = {} transactions.forEach(transaction => { - const { txParams: { nonce } = {}, status, type, time: txTime, key } = transaction + const { txParams: { nonce } = {}, status, type, time: txTime, key, transactionCategory } = transaction - if (typeof nonce === 'undefined') { + if (typeof nonce === 'undefined' || transactionCategory === 'incoming') { const transactionGroup = { transactions: [transaction], initialTransaction: transaction, @@ -200,6 +208,8 @@ export const nonceSortedTransactionsSelector = createSelector( if (key === 'shapeshift') { shapeshiftTransactionGroups.push(transactionGroup) + } else if (transactionCategory === 'incoming') { + incomingTransactionGroups.push(transactionGroup) } else { insertTransactionGroupByTime(unapprovedTransactionGroups, transactionGroup) } @@ -245,7 +255,8 @@ export const nonceSortedTransactionsSelector = createSelector( }) const orderedTransactionGroups = orderedNonces.map(nonce => nonceToTransactionsMap[nonce]) - mergeShapeshiftTransactionGroups(orderedTransactionGroups, shapeshiftTransactionGroups) + mergeNonNonceTransactionGroups(orderedTransactionGroups, shapeshiftTransactionGroups) + mergeNonNonceTransactionGroups(orderedTransactionGroups, incomingTransactionGroups) return unapprovedTransactionGroups.concat(orderedTransactionGroups) } ) From b87b992df33a47816ff8d630a34ed3d9b74a15d2 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Thu, 15 Aug 2019 09:23:29 -0230 Subject: [PATCH 06/15] Cleanup/refactor incoming-transactions.js --- .../controllers/incoming-transactions.js | 159 +++++++++++------- app/scripts/metamask-controller.js | 1 - .../transaction-list.component.js | 2 +- ui/app/selectors/transactions.js | 2 +- 4 files changed, 100 insertions(+), 64 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 4c02c4f7a5f0..77e081ac114b 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -1,6 +1,5 @@ const ObservableStore = require('obs-store') const extend = require('xtend') -const EthQuery = require('eth-query') const log = require('loglevel') const BN = require('bn.js') const createId = require('../lib/random-id') @@ -27,12 +26,11 @@ class IncomingTransactionsController { constructor (opts = {}) { const { blockTracker, - provider, networkController, getSelectedAddress, } = opts this.blockTracker = blockTracker - this.ethQuery = new EthQuery(provider) + this.networkController = networkController this.getSelectedAddress = getSelectedAddress this.getCurrentNetwork = () => networkController.getProviderConfig().type @@ -47,50 +45,88 @@ class IncomingTransactionsController { }, opts.initState) this.store = new ObservableStore(initState) - const changeListener = async ({ newBlockNumberDec, networkType } = {}) => { - try { - const { - incomingTransactions: currentIncomingTxs, - incomingTxlastFetchedBlocksByNetwork: currentBlocksByNetwork, - } = this.store.getState() - - const address = this.getSelectedAddress() - const network = networkType || this.getCurrentNetwork() - const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] - const blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberDec - const { latestIncomingTxBlockNumber, txs } = await this.fetchAll(address, blockToFetchFrom, network) - const newLatestBlockHashByNetwork = latestIncomingTxBlockNumber - ? parseInt(latestIncomingTxBlockNumber, 10) + 1 - : blockToFetchFrom + 1 - const newIncomingTransactions = { - ...currentIncomingTxs, - } - txs.forEach(tx => { newIncomingTransactions[tx.hash] = tx }) - - this.store.updateState({ - incomingTxlastFetchedBlocksByNetwork: { - ...currentBlocksByNetwork, - [network]: newLatestBlockHashByNetwork, - }, - incomingTransactions: newIncomingTransactions, - }) - } catch (err) { - log.error(err) - } + this.networkController.on('networkDidChange', (newType) => { + this._update({ networkType: newType }) + }) + this.blockTracker.on('latest', (newBlockNumberHex) => { + this._update({ newBlockNumberDec: parseInt(newBlockNumberHex, 16) }) + }) + } + + async _update ({ newBlockNumberDec, networkType } = {}) { + try { + const dataForUpdate = await this._getDataForUpdate({ newBlockNumberDec, networkType }) + this._updateStateWithNewTxData(dataForUpdate) + } catch (err) { + log.error(err) } + } + + async _getDataForUpdate ({ newBlockNumberDec, networkType } = {}) { + try { + const { + incomingTransactions: currentIncomingTxs, + incomingTxlastFetchedBlocksByNetwork: currentBlocksByNetwork, + } = this.store.getState() + + const address = this.getSelectedAddress() + const network = networkType || this.getCurrentNetwork() + const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] + let blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberDec + if (blockToFetchFrom === undefined) { + blockToFetchFrom = parseInt(this.blockTracker.getCurrentBlock(), 16) + } - const blockListener = (newBlockNumberHex) => { - changeListener({ newBlockNumberDec: parseInt(newBlockNumberHex, 16) }) + const { latestIncomingTxBlockNumber, txs: newTxs } = await this._fetchAll(address, blockToFetchFrom, network) + + return { + latestIncomingTxBlockNumber, + newTxs, + currentIncomingTxs, + currentBlocksByNetwork, + fetchedBlockNumber: blockToFetchFrom, + network, + } + } catch (err) { + log.error(err) } - const networkListener = (newType) => { - changeListener({ networkType: newType }) + } + + async _updateStateWithNewTxData ({ + latestIncomingTxBlockNumber, + newTxs, + currentIncomingTxs, + currentBlocksByNetwork, + fetchedBlockNumber, + network, + }) { + const newLatestBlockHashByNetwork = latestIncomingTxBlockNumber + ? parseInt(latestIncomingTxBlockNumber, 10) + 1 + : fetchedBlockNumber + 1 + const newIncomingTransactions = { + ...currentIncomingTxs, } + newTxs.forEach(tx => { newIncomingTransactions[tx.hash] = tx }) - networkController.on('networkDidChange', networkListener) - this.blockTracker.on('latest', blockListener) + this.store.updateState({ + incomingTxlastFetchedBlocksByNetwork: { + ...currentBlocksByNetwork, + [network]: newLatestBlockHashByNetwork, + }, + incomingTransactions: newIncomingTransactions, + }) + } + + async _fetchAll (address, fromBlock, networkType) { + try { + const fetchedTxResponse = await this._fetchTxs(address, fromBlock, networkType) + return this._processTxFetchResponse(fetchedTxResponse) + } catch (err) { + log.error(err) + } } - async fetchAll (address, fromBlock, networkType) { + async _fetchTxs (address, fromBlock, networkType) { let etherscanSubdomain = 'api' const currentNetworkID = networkTypeToIdMap[networkType] const supportedNetworkTypes = [ROPSTEN, RINKEBY, KOVAN, MAINNET] @@ -103,10 +139,6 @@ class IncomingTransactionsController { etherscanSubdomain = `api-${networkType}` } const apiUrl = `https://${etherscanSubdomain}.etherscan.io` - - if (!apiUrl) { - return - } let url = `${apiUrl}/api?module=account&action=txlist&address=${address}&tag=latest&page=1` if (fromBlock) { @@ -115,35 +147,40 @@ class IncomingTransactionsController { const response = await fetch(url) const parsedResponse = await response.json() - if (parsedResponse.status !== '0' && parsedResponse.result.length > 0) { + return { + ...parsedResponse, + address, + currentNetworkID, + } + } + + _processTxFetchResponse ({ status, result, address, currentNetworkID }) { + if (status !== '0' && result.length > 0) { const remoteTxList = {} const remoteTxs = [] - parsedResponse.result.forEach((tx) => { + result.forEach((tx) => { if (!remoteTxList[tx.hash]) { - remoteTxs.push(this.normalizeTxFromEtherscan(tx, currentNetworkID)) + remoteTxs.push(this._normalizeTxFromEtherscan(tx, currentNetworkID)) remoteTxList[tx.hash] = 1 } }) - const allTxs = [ ...remoteTxs ] - allTxs.sort((a, b) => (a.time < b.time ? -1 : 1)) + const incomingTxs = remoteTxs.filter(tx => tx.txParams.to && tx.txParams.to.toLowerCase() === address.toLowerCase()) + incomingTxs.sort((a, b) => (a.time < b.time ? -1 : 1)) let latestIncomingTxBlockNumber - allTxs.forEach((tx) => { - if (tx.txParams.to && tx.txParams.to.toLowerCase() === address.toLowerCase()) { - if ( - tx.blockNumber && - (!latestIncomingTxBlockNumber || - parseInt(latestIncomingTxBlockNumber, 10) < parseInt(tx.blockNumber, 10)) - ) { - latestIncomingTxBlockNumber = tx.blockNumber - } + incomingTxs.forEach((tx) => { + if ( + tx.blockNumber && + (!latestIncomingTxBlockNumber || + parseInt(latestIncomingTxBlockNumber, 10) < parseInt(tx.blockNumber, 10)) + ) { + latestIncomingTxBlockNumber = tx.blockNumber } }) - this.store.updateState({ transactions: allTxs }) return { latestIncomingTxBlockNumber, - txs: allTxs, + txs: incomingTxs, } } return { @@ -152,7 +189,7 @@ class IncomingTransactionsController { } } - normalizeTxFromEtherscan (txMeta, currentNetworkID) { + _normalizeTxFromEtherscan (txMeta, currentNetworkID) { const time = parseInt(txMeta.timeStamp, 10) * 1000 const status = txMeta.isError === '0' ? 'confirmed' : 'failed' return { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 0dbdc8aa1adb..4de6f450674f 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -140,7 +140,6 @@ module.exports = class MetamaskController extends EventEmitter { this.incomingTransactionsController = new IncomingTransactionsController({ blockTracker: this.blockTracker, - provider: this.provider, networkController: this.networkController, getSelectedAddress: this.preferencesController.getSelectedAddress.bind(this.preferencesController), initState: initState.IncomingTransactionsController, diff --git a/ui/app/components/app/transaction-list/transaction-list.component.js b/ui/app/components/app/transaction-list/transaction-list.component.js index ea604ade8f82..157e7200b413 100644 --- a/ui/app/components/app/transaction-list/transaction-list.component.js +++ b/ui/app/components/app/transaction-list/transaction-list.component.js @@ -126,4 +126,4 @@ export default class TransactionList extends PureComponent {
) } -} \ No newline at end of file +} diff --git a/ui/app/selectors/transactions.js b/ui/app/selectors/transactions.js index 3eed1b075228..5450978a66bc 100644 --- a/ui/app/selectors/transactions.js +++ b/ui/app/selectors/transactions.js @@ -166,7 +166,7 @@ const insertTransactionGroupByTime = (transactionGroups, transactionGroup) => { /** * @name mergeNonNonceTransactionGroups * @private - * @description Inserts (mutates) transactionGroups that are not to be ordered by nonce into an array + * @description Inserts (mutates) transactionGroups that are not to be ordered by nonce into an array * of nonce-ordered transactionGroups by time. Shapeshift transactionGroups need to be sorted by time * within the list of transactions as they do not have nonces. * @param {transactionGroup[]} orderedTransactionGroups - Array of transactionGroups ordered by From 7f7a19fa96f2ddb9430198a66dba3c47d35da5ec Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Thu, 15 Aug 2019 13:16:03 -0230 Subject: [PATCH 07/15] Add tests for the incoming transactions controller --- .../controllers/incoming-transactions-test.js | 621 ++++++++++++++++++ 1 file changed, 621 insertions(+) create mode 100644 test/unit/app/controllers/incoming-transactions-test.js diff --git a/test/unit/app/controllers/incoming-transactions-test.js b/test/unit/app/controllers/incoming-transactions-test.js new file mode 100644 index 000000000000..80fd0b48981f --- /dev/null +++ b/test/unit/app/controllers/incoming-transactions-test.js @@ -0,0 +1,621 @@ +const assert = require('assert') +const sinon = require('sinon') +const proxyquire = require('proxyquire') +const IncomingTransactionsController = proxyquire('../../../../app/scripts/controllers/incoming-transactions', { + '../lib/random-id': () => 54321, +}) + +const { + ROPSTEN, + RINKEBY, + KOVAN, + MAINNET, +} = require('../../../../app/scripts/controllers/network/enums') + +describe('IncomingTransactionsController', () => { + const EMPTY_INIT_STATE = { + incomingTransactions: {}, + incomingTxlastFetchedBlocksByNetwork: { + [ROPSTEN]: null, + [RINKEBY]: null, + [KOVAN]: null, + [MAINNET]: null, + }, + } + + const NON_EMPTY_INIT_STATE = { + incomingTransactions: { + '0x123456': { id: 777 }, + }, + incomingTxlastFetchedBlocksByNetwork: { + [ROPSTEN]: 1, + [RINKEBY]: 2, + [KOVAN]: 3, + [MAINNET]: 4, + }, + } + + const NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE = { + incomingTransactions: { + '0x123456': { id: 777 }, + }, + incomingTxlastFetchedBlocksByNetwork: { + [ROPSTEN]: 1, + [RINKEBY]: 2, + [KOVAN]: 3, + [MAINNET]: 4, + FAKE_NETWORK: 1111, + }, + } + + const MOCK_BLOCKTRACKER = { + on: sinon.spy(), + testProperty: 'fakeBlockTracker', + getCurrentBlock: () => '0xa', + } + + const MOCK_NETWORK_CONTROLLER = { + getProviderConfig: () => ({ type: 'FAKE_NETWORK' }), + on: sinon.spy(), + } + + describe('constructor', () => { + it('should set up correct store, listeners and properties in the constructor', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: 'fakeGetSelectedAddress', + initState: {}, + }) + sinon.spy(incomingTransactionsController, '_update') + + assert.deepEqual(incomingTransactionsController.blockTracker, MOCK_BLOCKTRACKER) + assert.deepEqual(incomingTransactionsController.networkController, MOCK_NETWORK_CONTROLLER) + assert.equal(incomingTransactionsController.getSelectedAddress, 'fakeGetSelectedAddress') + assert.equal(incomingTransactionsController.getCurrentNetwork(), 'FAKE_NETWORK') + + assert.deepEqual(incomingTransactionsController.store.getState(), EMPTY_INIT_STATE) + + assert(incomingTransactionsController.networkController.on.calledOnce) + assert.equal(incomingTransactionsController.networkController.on.getCall(0).args[0], 'networkDidChange') + const networkControllerListenerCallback = incomingTransactionsController.networkController.on.getCall(0).args[1] + assert.equal(incomingTransactionsController._update.callCount, 0) + networkControllerListenerCallback('testNetworkType') + assert.equal(incomingTransactionsController._update.callCount, 1) + assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], { networkType: 'testNetworkType' }) + + incomingTransactionsController._update.resetHistory() + + assert(incomingTransactionsController.blockTracker.on.calledOnce) + assert.equal(incomingTransactionsController.blockTracker.on.getCall(0).args[0], 'latest') + const blockTrackerListenerCallback = incomingTransactionsController.blockTracker.on.getCall(0).args[1] + assert.equal(incomingTransactionsController._update.callCount, 0) + blockTrackerListenerCallback('0xabc') + assert.equal(incomingTransactionsController._update.callCount, 1) + assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], { newBlockNumberDec: 2748 }) + }) + + it('should set the store to a provided initial state', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: 'fakeGetSelectedAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + assert.deepEqual(incomingTransactionsController.store.getState(), NON_EMPTY_INIT_STATE) + }) + }) + + describe('_getDataForUpdate', () => { + it('should call fetchAll with the correct params when passed a new block number and the current network has no stored block', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + incomingTransactionsController._fetchAll = sinon.spy() + + await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) + + assert(incomingTransactionsController._fetchAll.calledOnce) + + assert.deepEqual(incomingTransactionsController._fetchAll.getCall(0).args, [ + 'fakeAddress', 999, 'FAKE_NETWORK', + ]) + }) + + it('should call fetchAll with the correct params when passed a new block number but the current network has a stored block', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, + }) + incomingTransactionsController._fetchAll = sinon.spy() + + await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) + + assert(incomingTransactionsController._fetchAll.calledOnce) + + assert.deepEqual(incomingTransactionsController._fetchAll.getCall(0).args, [ + 'fakeAddress', 1111, 'FAKE_NETWORK', + ]) + }) + + it('should call fetchAll with the correct params when passed a new network type but no block info exists', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, + }) + incomingTransactionsController._fetchAll = sinon.spy() + + await incomingTransactionsController._getDataForUpdate({ networkType: 'NEW_FAKE_NETWORK' }) + + assert(incomingTransactionsController._fetchAll.calledOnce) + + assert.deepEqual(incomingTransactionsController._fetchAll.getCall(0).args, [ + 'fakeAddress', 10, 'NEW_FAKE_NETWORK', + ]) + }) + + it('should call fetchAll with the correct params when passed a new block number but the current network has a stored block', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, + }) + incomingTransactionsController._fetchAll = sinon.spy() + + await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) + + assert(incomingTransactionsController._fetchAll.calledOnce) + + assert.deepEqual(incomingTransactionsController._fetchAll.getCall(0).args, [ + 'fakeAddress', 1111, 'FAKE_NETWORK', + ]) + }) + + it('should return the expected data', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, + }) + incomingTransactionsController._fetchAll = sinon.stub().returns({ + latestIncomingTxBlockNumber: 444, + txs: [{ id: 555 }], + }) + + const result = await incomingTransactionsController._getDataForUpdate({ networkType: 'FAKE_NETWORK' }) + + assert.deepEqual(result, { + latestIncomingTxBlockNumber: 444, + newTxs: [{ id: 555 }], + currentIncomingTxs: { + '0x123456': { id: 777 }, + }, + currentBlocksByNetwork: { + [ROPSTEN]: 1, + [RINKEBY]: 2, + [KOVAN]: 3, + [MAINNET]: 4, + FAKE_NETWORK: 1111, + }, + fetchedBlockNumber: 1111, + network: 'FAKE_NETWORK', + }) + }) + }) + + describe('_updateStateWithNewTxData', () => { + const MOCK_INPUT_WITHOUT_LASTEST = { + newTxs: [{ id: 555, hash: '0xfff' }], + currentIncomingTxs: { + '0x123456': { id: 777, hash: '0x123456' }, + }, + currentBlocksByNetwork: { + [ROPSTEN]: 1, + [RINKEBY]: 2, + [KOVAN]: 3, + [MAINNET]: 4, + FAKE_NETWORK: 1111, + }, + fetchedBlockNumber: 1111, + network: 'FAKE_NETWORK', + } + + const MOCK_INPUT_WITH_LASTEST = { + ...MOCK_INPUT_WITHOUT_LASTEST, + latestIncomingTxBlockNumber: 444, + } + + it('should update state with correct blockhash and transactions when passed a truthy latestIncomingTxBlockNumber', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + sinon.spy(incomingTransactionsController.store, 'updateState') + + await incomingTransactionsController._updateStateWithNewTxData(MOCK_INPUT_WITH_LASTEST) + + assert(incomingTransactionsController.store.updateState.calledOnce) + + assert.deepEqual(incomingTransactionsController.store.updateState.getCall(0).args[0], { + incomingTxlastFetchedBlocksByNetwork: { + ...MOCK_INPUT_WITH_LASTEST.currentBlocksByNetwork, + 'FAKE_NETWORK': 445, + }, + incomingTransactions: { + '0x123456': { id: 777, hash: '0x123456' }, + '0xfff': { id: 555, hash: '0xfff' }, + }, + }) + }) + + it('should update state with correct blockhash and transactions when passed a falsy latestIncomingTxBlockNumber', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + sinon.spy(incomingTransactionsController.store, 'updateState') + + await incomingTransactionsController._updateStateWithNewTxData(MOCK_INPUT_WITHOUT_LASTEST) + + assert(incomingTransactionsController.store.updateState.calledOnce) + + assert.deepEqual(incomingTransactionsController.store.updateState.getCall(0).args[0], { + incomingTxlastFetchedBlocksByNetwork: { + ...MOCK_INPUT_WITH_LASTEST.currentBlocksByNetwork, + 'FAKE_NETWORK': 1112, + }, + incomingTransactions: { + '0x123456': { id: 777, hash: '0x123456' }, + '0xfff': { id: 555, hash: '0xfff' }, + }, + }) + }) + }) + + describe('_fetchTxs', () => { + const mockFetch = sinon.stub().returns(Promise.resolve({ + json: () => Promise.resolve({ someKey: 'someValue' }), + })) + let tempFetch + beforeEach(() => { + tempFetch = global.fetch + global.fetch = mockFetch + }) + + afterEach(() => { + global.fetch = tempFetch + mockFetch.resetHistory() + }) + + it('should call fetch with the expected url when passed an address, block number and supported network', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + await incomingTransactionsController._fetchTxs('0xfakeaddress', '789', ROPSTEN) + + assert(mockFetch.calledOnce) + assert.equal(mockFetch.getCall(0).args[0], `https://api-${ROPSTEN}.etherscan.io/api?module=account&action=txlist&address=0xfakeaddress&tag=latest&page=1&startBlock=789`) + }) + + it('should call fetch with the expected url when passed an address, block number and MAINNET', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + await incomingTransactionsController._fetchTxs('0xfakeaddress', '789', MAINNET) + + assert(mockFetch.calledOnce) + assert.equal(mockFetch.getCall(0).args[0], `https://api.etherscan.io/api?module=account&action=txlist&address=0xfakeaddress&tag=latest&page=1&startBlock=789`) + }) + + it('should call fetch with the expected url when passed an address and supported network, but a falsy block number', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + await incomingTransactionsController._fetchTxs('0xfakeaddress', null, ROPSTEN) + + assert(mockFetch.calledOnce) + assert.equal(mockFetch.getCall(0).args[0], `https://api-${ROPSTEN}.etherscan.io/api?module=account&action=txlist&address=0xfakeaddress&tag=latest&page=1`) + }) + + it('should not fetch and return undefined when passed an unsported network', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + const result = await incomingTransactionsController._fetchTxs('0xfakeaddress', null, 'UNSUPPORTED_NETWORK') + + assert(mockFetch.notCalled) + assert.equal(result, undefined) + }) + + it('should return the results from the fetch call, plus the address and currentNetworkID, when passed an address, block number and supported network', async () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + const result = await incomingTransactionsController._fetchTxs('0xfakeaddress', '789', ROPSTEN) + + assert(mockFetch.calledOnce) + assert.deepEqual(result, { + someKey: 'someValue', + address: '0xfakeaddress', + currentNetworkID: 3, + }) + }) + }) + + describe('_processTxFetchResponse', () => { + it('should return a null block number and empty tx array if status is 0', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + const result = incomingTransactionsController._processTxFetchResponse({ + status: '0', + result: [{ id: 1 }], + address: '0xfakeaddress', + }) + + assert.deepEqual(result, { + latestIncomingTxBlockNumber: null, + txs: [], + }) + }) + + it('should return a null block number and empty tx array if the passed result array is empty', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + const result = incomingTransactionsController._processTxFetchResponse({ + status: '1', + result: [], + address: '0xfakeaddress', + }) + + assert.deepEqual(result, { + latestIncomingTxBlockNumber: null, + txs: [], + }) + }) + + it('should return the expected block number and tx list when passed data from a successful fetch', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + incomingTransactionsController._normalizeTxFromEtherscan = (tx, currentNetworkID) => ({ + ...tx, + currentNetworkID, + normalized: true, + }) + + const result = incomingTransactionsController._processTxFetchResponse({ + status: '1', + address: '0xfakeaddress', + currentNetworkID: 'FAKE_NETWORK', + result: [ + { + hash: '0xabc123', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5000, + time: 10, + }, + { + hash: '0xabc123', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5000, + time: 10, + }, + { + hash: '0xabc1234', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5000, + time: 9, + }, + { + hash: '0xabc12345', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5001, + time: 11, + }, + { + hash: '0xabc123456', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5001, + time: 12, + }, + { + hash: '0xabc1234567', + txParams: { + to: '0xanotherFakeaddress', + }, + blockNumber: 5002, + time: 13, + }, + ], + }) + + assert.deepEqual(result, { + latestIncomingTxBlockNumber: 5001, + txs: [ + { + hash: '0xabc1234', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5000, + time: 9, + normalized: true, + currentNetworkID: 'FAKE_NETWORK', + }, + { + hash: '0xabc123', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5000, + time: 10, + normalized: true, + currentNetworkID: 'FAKE_NETWORK', + }, + { + hash: '0xabc12345', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5001, + time: 11, + normalized: true, + currentNetworkID: 'FAKE_NETWORK', + }, + { + hash: '0xabc123456', + txParams: { + to: '0xfakeaddress', + }, + blockNumber: 5001, + time: 12, + normalized: true, + currentNetworkID: 'FAKE_NETWORK', + }, + ], + }) + }) + }) + + describe('_normalizeTxFromEtherscan', () => { + it('should return the expected data when the tx is in error', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + const result = incomingTransactionsController._normalizeTxFromEtherscan({ + timeStamp: '4444', + isError: '1', + blockNumber: 333, + from: '0xa', + gas: '11', + gasPrice: '12', + nonce: '13', + to: '0xe', + value: '15', + hash: '0xg', + }, 'FAKE_NETWORK') + + assert.deepEqual(result, { + blockNumber: 333, + id: 54321, + metamaskNetworkId: 'FAKE_NETWORK', + status: 'failed', + time: 4444000, + txParams: { + from: '0xa', + gas: '0xb', + gasPrice: '0xc', + nonce: '0xd', + to: '0xe', + value: '0xf', + }, + hash: '0xg', + transactionCategory: 'incoming', + }) + }) + }) + + describe('_normalizeTxFromEtherscan', () => { + it('should return the expected data when the tx is not in error', () => { + const incomingTransactionsController = new IncomingTransactionsController({ + blockTracker: MOCK_BLOCKTRACKER, + networkController: MOCK_NETWORK_CONTROLLER, + getSelectedAddress: () => 'fakeAddress', + initState: NON_EMPTY_INIT_STATE, + }) + + const result = incomingTransactionsController._normalizeTxFromEtherscan({ + timeStamp: '4444', + isError: '0', + blockNumber: 333, + from: '0xa', + gas: '11', + gasPrice: '12', + nonce: '13', + to: '0xe', + value: '15', + hash: '0xg', + }, 'FAKE_NETWORK') + + assert.deepEqual(result, { + blockNumber: 333, + id: 54321, + metamaskNetworkId: 'FAKE_NETWORK', + status: 'confirmed', + time: 4444000, + txParams: { + from: '0xa', + gas: '0xb', + gasPrice: '0xc', + nonce: '0xd', + to: '0xe', + value: '0xf', + }, + hash: '0xg', + transactionCategory: 'incoming', + }) + }) + }) +}) From f56f369d7e31cfefb2729df7549e7770cbb2e3dd Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Thu, 15 Aug 2019 14:55:35 -0230 Subject: [PATCH 08/15] Unit test fixes for received transactions --- .../controllers/incoming-transactions.js | 46 +++++++++---------- test/data/mock-state.json | 1 + .../controllers/incoming-transactions-test.js | 12 ++--- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 77e081ac114b..cd5fe213f261 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -63,32 +63,28 @@ class IncomingTransactionsController { } async _getDataForUpdate ({ newBlockNumberDec, networkType } = {}) { - try { - const { - incomingTransactions: currentIncomingTxs, - incomingTxlastFetchedBlocksByNetwork: currentBlocksByNetwork, - } = this.store.getState() - - const address = this.getSelectedAddress() - const network = networkType || this.getCurrentNetwork() - const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] - let blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberDec - if (blockToFetchFrom === undefined) { - blockToFetchFrom = parseInt(this.blockTracker.getCurrentBlock(), 16) - } + const { + incomingTransactions: currentIncomingTxs, + incomingTxlastFetchedBlocksByNetwork: currentBlocksByNetwork, + } = this.store.getState() + + const address = this.getSelectedAddress() + const network = networkType || this.getCurrentNetwork() + const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] + let blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberDec + if (blockToFetchFrom === undefined) { + blockToFetchFrom = parseInt(this.blockTracker.getCurrentBlock(), 16) + } - const { latestIncomingTxBlockNumber, txs: newTxs } = await this._fetchAll(address, blockToFetchFrom, network) + const { latestIncomingTxBlockNumber, txs: newTxs } = await this._fetchAll(address, blockToFetchFrom, network) - return { - latestIncomingTxBlockNumber, - newTxs, - currentIncomingTxs, - currentBlocksByNetwork, - fetchedBlockNumber: blockToFetchFrom, - network, - } - } catch (err) { - log.error(err) + return { + latestIncomingTxBlockNumber, + newTxs, + currentIncomingTxs, + currentBlocksByNetwork, + fetchedBlockNumber: blockToFetchFrom, + network, } } @@ -132,7 +128,7 @@ class IncomingTransactionsController { const supportedNetworkTypes = [ROPSTEN, RINKEBY, KOVAN, MAINNET] if (supportedNetworkTypes.indexOf(networkType) === -1) { - return + return {} } if (networkType !== 'mainnet') { diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 122945ec1422..ed25b0abe058 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -12,6 +12,7 @@ } }, "cachedBalances": {}, + "incomingTransactions": {}, "unapprovedTxs": { "8393540981007587": { "id": 8393540981007587, diff --git a/test/unit/app/controllers/incoming-transactions-test.js b/test/unit/app/controllers/incoming-transactions-test.js index 80fd0b48981f..fbde1aa49fed 100644 --- a/test/unit/app/controllers/incoming-transactions-test.js +++ b/test/unit/app/controllers/incoming-transactions-test.js @@ -115,7 +115,7 @@ describe('IncomingTransactionsController', () => { getSelectedAddress: () => 'fakeAddress', initState: NON_EMPTY_INIT_STATE, }) - incomingTransactionsController._fetchAll = sinon.spy() + incomingTransactionsController._fetchAll = sinon.stub().returns({}) await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) @@ -133,7 +133,7 @@ describe('IncomingTransactionsController', () => { getSelectedAddress: () => 'fakeAddress', initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) - incomingTransactionsController._fetchAll = sinon.spy() + incomingTransactionsController._fetchAll = sinon.stub().returns({}) await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) @@ -151,7 +151,7 @@ describe('IncomingTransactionsController', () => { getSelectedAddress: () => 'fakeAddress', initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) - incomingTransactionsController._fetchAll = sinon.spy() + incomingTransactionsController._fetchAll = sinon.stub().returns({}) await incomingTransactionsController._getDataForUpdate({ networkType: 'NEW_FAKE_NETWORK' }) @@ -169,7 +169,7 @@ describe('IncomingTransactionsController', () => { getSelectedAddress: () => 'fakeAddress', initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) - incomingTransactionsController._fetchAll = sinon.spy() + incomingTransactionsController._fetchAll = sinon.stub().returns({}) await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) @@ -343,7 +343,7 @@ describe('IncomingTransactionsController', () => { assert.equal(mockFetch.getCall(0).args[0], `https://api-${ROPSTEN}.etherscan.io/api?module=account&action=txlist&address=0xfakeaddress&tag=latest&page=1`) }) - it('should not fetch and return undefined when passed an unsported network', async () => { + it('should not fetch and return an empty object when passed an unsported network', async () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, @@ -354,7 +354,7 @@ describe('IncomingTransactionsController', () => { const result = await incomingTransactionsController._fetchTxs('0xfakeaddress', null, 'UNSUPPORTED_NETWORK') assert(mockFetch.notCalled) - assert.equal(result, undefined) + assert.deepEqual(result, {}) }) it('should return the results from the fetch call, plus the address and currentNetworkID, when passed an address, block number and supported network', async () => { From 6ce7f74ec4c2cd1796a9e727d82f58bf32531c47 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 14:26:37 -0230 Subject: [PATCH 09/15] Remove unused received i18n message --- app/_locales/en/messages.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 52a0d17a0c75..2b957129d669 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1280,9 +1280,6 @@ "receive": { "message": "Receive" }, - "received": { - "message": "Received" - }, "recents": { "message": "Recents" }, From e84f6ba3ae449f93735b15ce7be178b4cf70a117 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 14:29:38 -0230 Subject: [PATCH 10/15] Use return value from IncomingTransactionsController#_update in handlers --- app/scripts/controllers/incoming-transactions.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index cd5fe213f261..876dd5addeb3 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -45,18 +45,18 @@ class IncomingTransactionsController { }, opts.initState) this.store = new ObservableStore(initState) - this.networkController.on('networkDidChange', (newType) => { - this._update({ networkType: newType }) + this.networkController.on('networkDidChange', async (newType) => { + await this._update({ networkType: newType }) }) - this.blockTracker.on('latest', (newBlockNumberHex) => { - this._update({ newBlockNumberDec: parseInt(newBlockNumberHex, 16) }) + this.blockTracker.on('latest', async (newBlockNumberHex) => { + await this._update({ newBlockNumberDec: parseInt(newBlockNumberHex, 16) }) }) } async _update ({ newBlockNumberDec, networkType } = {}) { try { const dataForUpdate = await this._getDataForUpdate({ newBlockNumberDec, networkType }) - this._updateStateWithNewTxData(dataForUpdate) + await this._updateStateWithNewTxData(dataForUpdate) } catch (err) { log.error(err) } From d0da125e53a7ef1677c49294bd3a83fbd685efdb Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 14:29:52 -0230 Subject: [PATCH 11/15] Rename incomingTxLastFetchedBlocksByNetwork --- app/scripts/controllers/incoming-transactions.js | 6 +++--- .../unit/app/controllers/incoming-transactions-test.js | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 876dd5addeb3..70c4e072159b 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -36,7 +36,7 @@ class IncomingTransactionsController { const initState = extend({ incomingTransactions: {}, - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { [ROPSTEN]: null, [RINKEBY]: null, [KOVAN]: null, @@ -65,7 +65,7 @@ class IncomingTransactionsController { async _getDataForUpdate ({ newBlockNumberDec, networkType } = {}) { const { incomingTransactions: currentIncomingTxs, - incomingTxlastFetchedBlocksByNetwork: currentBlocksByNetwork, + incomingTxLastFetchedBlocksByNetwork: currentBlocksByNetwork, } = this.store.getState() const address = this.getSelectedAddress() @@ -105,7 +105,7 @@ class IncomingTransactionsController { newTxs.forEach(tx => { newIncomingTransactions[tx.hash] = tx }) this.store.updateState({ - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { ...currentBlocksByNetwork, [network]: newLatestBlockHashByNetwork, }, diff --git a/test/unit/app/controllers/incoming-transactions-test.js b/test/unit/app/controllers/incoming-transactions-test.js index fbde1aa49fed..3a8552ef5af1 100644 --- a/test/unit/app/controllers/incoming-transactions-test.js +++ b/test/unit/app/controllers/incoming-transactions-test.js @@ -15,7 +15,7 @@ const { describe('IncomingTransactionsController', () => { const EMPTY_INIT_STATE = { incomingTransactions: {}, - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { [ROPSTEN]: null, [RINKEBY]: null, [KOVAN]: null, @@ -27,7 +27,7 @@ describe('IncomingTransactionsController', () => { incomingTransactions: { '0x123456': { id: 777 }, }, - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { [ROPSTEN]: 1, [RINKEBY]: 2, [KOVAN]: 3, @@ -39,7 +39,7 @@ describe('IncomingTransactionsController', () => { incomingTransactions: { '0x123456': { id: 777 }, }, - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { [ROPSTEN]: 1, [RINKEBY]: 2, [KOVAN]: 3, @@ -249,7 +249,7 @@ describe('IncomingTransactionsController', () => { assert(incomingTransactionsController.store.updateState.calledOnce) assert.deepEqual(incomingTransactionsController.store.updateState.getCall(0).args[0], { - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { ...MOCK_INPUT_WITH_LASTEST.currentBlocksByNetwork, 'FAKE_NETWORK': 445, }, @@ -274,7 +274,7 @@ describe('IncomingTransactionsController', () => { assert(incomingTransactionsController.store.updateState.calledOnce) assert.deepEqual(incomingTransactionsController.store.updateState.getCall(0).args[0], { - incomingTxlastFetchedBlocksByNetwork: { + incomingTxLastFetchedBlocksByNetwork: { ...MOCK_INPUT_WITH_LASTEST.currentBlocksByNetwork, 'FAKE_NETWORK': 1112, }, From 850c0cc211e0770fb5c3faa9ca15b9c2f28fbc86 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 14:31:46 -0230 Subject: [PATCH 12/15] Use MAINNET enum in place of string literal --- app/scripts/controllers/incoming-transactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 70c4e072159b..e07b0a4929cb 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -131,7 +131,7 @@ class IncomingTransactionsController { return {} } - if (networkType !== 'mainnet') { + if (networkType !== MAINNET) { etherscanSubdomain = `api-${networkType}` } const apiUrl = `https://${etherscanSubdomain}.etherscan.io` From aab817c3749a7c9e65f23852fdf168a71ad7ab95 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 14:34:40 -0230 Subject: [PATCH 13/15] Initialize latestIncomingTxBlockNumber in IncomingTransactionsController --- app/scripts/controllers/incoming-transactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index e07b0a4929cb..f188cf57badc 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -164,7 +164,7 @@ class IncomingTransactionsController { const incomingTxs = remoteTxs.filter(tx => tx.txParams.to && tx.txParams.to.toLowerCase() === address.toLowerCase()) incomingTxs.sort((a, b) => (a.time < b.time ? -1 : 1)) - let latestIncomingTxBlockNumber + let latestIncomingTxBlockNumber = null incomingTxs.forEach((tx) => { if ( tx.blockNumber && From 076041d85743a7f5dbe34bbce3b1678ab604b6f3 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 14:39:14 -0230 Subject: [PATCH 14/15] Remove xtend usage from IncomingTransactionsController --- app/scripts/controllers/incoming-transactions.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index f188cf57badc..33640ae447f5 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -1,5 +1,4 @@ const ObservableStore = require('obs-store') -const extend = require('xtend') const log = require('loglevel') const BN = require('bn.js') const createId = require('../lib/random-id') @@ -34,7 +33,7 @@ class IncomingTransactionsController { this.getSelectedAddress = getSelectedAddress this.getCurrentNetwork = () => networkController.getProviderConfig().type - const initState = extend({ + const initState = Object.assign({ incomingTransactions: {}, incomingTxLastFetchedBlocksByNetwork: { [ROPSTEN]: null, From a7eba59d5557efb926ba749747bd8cf2b8268351 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 16 Aug 2019 15:08:08 -0230 Subject: [PATCH 15/15] Update incoming txs when address changes --- .../controllers/incoming-transactions.js | 28 +++++-- app/scripts/metamask-controller.js | 2 +- .../controllers/incoming-transactions-test.js | 75 ++++++++++++------- 3 files changed, 67 insertions(+), 38 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 33640ae447f5..4b431442726c 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -26,11 +26,11 @@ class IncomingTransactionsController { const { blockTracker, networkController, - getSelectedAddress, + preferencesController, } = opts this.blockTracker = blockTracker this.networkController = networkController - this.getSelectedAddress = getSelectedAddress + this.preferencesController = preferencesController this.getCurrentNetwork = () => networkController.getProviderConfig().type const initState = Object.assign({ @@ -45,29 +45,41 @@ class IncomingTransactionsController { this.store = new ObservableStore(initState) this.networkController.on('networkDidChange', async (newType) => { - await this._update({ networkType: newType }) + const address = this.preferencesController.getSelectedAddress() + await this._update({ + address, + networkType: newType, + }) }) this.blockTracker.on('latest', async (newBlockNumberHex) => { - await this._update({ newBlockNumberDec: parseInt(newBlockNumberHex, 16) }) + const address = this.preferencesController.getSelectedAddress() + await this._update({ + address, + newBlockNumberDec: parseInt(newBlockNumberHex, 16), + }) + }) + this.preferencesController.store.subscribe(async ({ selectedAddress }) => { + await this._update({ + address: selectedAddress, + }) }) } - async _update ({ newBlockNumberDec, networkType } = {}) { + async _update ({ address, newBlockNumberDec, networkType } = {}) { try { - const dataForUpdate = await this._getDataForUpdate({ newBlockNumberDec, networkType }) + const dataForUpdate = await this._getDataForUpdate({ address, newBlockNumberDec, networkType }) await this._updateStateWithNewTxData(dataForUpdate) } catch (err) { log.error(err) } } - async _getDataForUpdate ({ newBlockNumberDec, networkType } = {}) { + async _getDataForUpdate ({ address, newBlockNumberDec, networkType } = {}) { const { incomingTransactions: currentIncomingTxs, incomingTxLastFetchedBlocksByNetwork: currentBlocksByNetwork, } = this.store.getState() - const address = this.getSelectedAddress() const network = networkType || this.getCurrentNetwork() const lastFetchBlockByCurrentNetwork = currentBlocksByNetwork[network] let blockToFetchFrom = lastFetchBlockByCurrentNetwork || newBlockNumberDec diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 4de6f450674f..14fa143f438a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -141,7 +141,7 @@ module.exports = class MetamaskController extends EventEmitter { this.incomingTransactionsController = new IncomingTransactionsController({ blockTracker: this.blockTracker, networkController: this.networkController, - getSelectedAddress: this.preferencesController.getSelectedAddress.bind(this.preferencesController), + preferencesController: this.preferencesController, initState: initState.IncomingTransactionsController, }) diff --git a/test/unit/app/controllers/incoming-transactions-test.js b/test/unit/app/controllers/incoming-transactions-test.js index 3a8552ef5af1..923da7de9fee 100644 --- a/test/unit/app/controllers/incoming-transactions-test.js +++ b/test/unit/app/controllers/incoming-transactions-test.js @@ -59,19 +59,26 @@ describe('IncomingTransactionsController', () => { on: sinon.spy(), } + const MOCK_PREFERENCES_CONTROLLER = { + getSelectedAddress: sinon.stub().returns('0x0101'), + store: { + subscribe: sinon.spy(), + }, + } + describe('constructor', () => { it('should set up correct store, listeners and properties in the constructor', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: 'fakeGetSelectedAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: {}, }) sinon.spy(incomingTransactionsController, '_update') assert.deepEqual(incomingTransactionsController.blockTracker, MOCK_BLOCKTRACKER) assert.deepEqual(incomingTransactionsController.networkController, MOCK_NETWORK_CONTROLLER) - assert.equal(incomingTransactionsController.getSelectedAddress, 'fakeGetSelectedAddress') + assert.equal(incomingTransactionsController.preferencesController, MOCK_PREFERENCES_CONTROLLER) assert.equal(incomingTransactionsController.getCurrentNetwork(), 'FAKE_NETWORK') assert.deepEqual(incomingTransactionsController.store.getState(), EMPTY_INIT_STATE) @@ -82,7 +89,10 @@ describe('IncomingTransactionsController', () => { assert.equal(incomingTransactionsController._update.callCount, 0) networkControllerListenerCallback('testNetworkType') assert.equal(incomingTransactionsController._update.callCount, 1) - assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], { networkType: 'testNetworkType' }) + assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], { + address: '0x0101', + networkType: 'testNetworkType', + }) incomingTransactionsController._update.resetHistory() @@ -92,14 +102,17 @@ describe('IncomingTransactionsController', () => { assert.equal(incomingTransactionsController._update.callCount, 0) blockTrackerListenerCallback('0xabc') assert.equal(incomingTransactionsController._update.callCount, 1) - assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], { newBlockNumberDec: 2748 }) + assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], { + address: '0x0101', + newBlockNumberDec: 2748, + }) }) it('should set the store to a provided initial state', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: 'fakeGetSelectedAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -112,12 +125,12 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) incomingTransactionsController._fetchAll = sinon.stub().returns({}) - await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) + await incomingTransactionsController._getDataForUpdate({ address: 'fakeAddress', newBlockNumberDec: 999 }) assert(incomingTransactionsController._fetchAll.calledOnce) @@ -130,12 +143,12 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) incomingTransactionsController._fetchAll = sinon.stub().returns({}) - await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) + await incomingTransactionsController._getDataForUpdate({ address: 'fakeAddress', newBlockNumberDec: 999 }) assert(incomingTransactionsController._fetchAll.calledOnce) @@ -148,12 +161,15 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) incomingTransactionsController._fetchAll = sinon.stub().returns({}) - await incomingTransactionsController._getDataForUpdate({ networkType: 'NEW_FAKE_NETWORK' }) + await incomingTransactionsController._getDataForUpdate({ + address: 'fakeAddress', + networkType: 'NEW_FAKE_NETWORK', + }) assert(incomingTransactionsController._fetchAll.calledOnce) @@ -166,12 +182,12 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) incomingTransactionsController._fetchAll = sinon.stub().returns({}) - await incomingTransactionsController._getDataForUpdate({ newBlockNumberDec: 999 }) + await incomingTransactionsController._getDataForUpdate({ address: 'fakeAddress', newBlockNumberDec: 999 }) assert(incomingTransactionsController._fetchAll.calledOnce) @@ -184,7 +200,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE_WITH_FAKE_NETWORK_STATE, }) incomingTransactionsController._fetchAll = sinon.stub().returns({ @@ -192,7 +208,10 @@ describe('IncomingTransactionsController', () => { txs: [{ id: 555 }], }) - const result = await incomingTransactionsController._getDataForUpdate({ networkType: 'FAKE_NETWORK' }) + const result = await incomingTransactionsController._getDataForUpdate({ + address: 'fakeAddress', + networkType: 'FAKE_NETWORK', + }) assert.deepEqual(result, { latestIncomingTxBlockNumber: 444, @@ -239,7 +258,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) sinon.spy(incomingTransactionsController.store, 'updateState') @@ -264,7 +283,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) sinon.spy(incomingTransactionsController.store, 'updateState') @@ -305,7 +324,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -319,7 +338,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -333,7 +352,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -347,7 +366,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -361,7 +380,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -381,7 +400,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -401,7 +420,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -421,7 +440,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -540,7 +559,7 @@ describe('IncomingTransactionsController', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, }) @@ -575,14 +594,12 @@ describe('IncomingTransactionsController', () => { transactionCategory: 'incoming', }) }) - }) - describe('_normalizeTxFromEtherscan', () => { it('should return the expected data when the tx is not in error', () => { const incomingTransactionsController = new IncomingTransactionsController({ blockTracker: MOCK_BLOCKTRACKER, networkController: MOCK_NETWORK_CONTROLLER, - getSelectedAddress: () => 'fakeAddress', + preferencesController: MOCK_PREFERENCES_CONTROLLER, initState: NON_EMPTY_INIT_STATE, })