Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixes for incoming transactions #7043

Merged
merged 4 commits into from
Aug 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 26 additions & 12 deletions app/scripts/controllers/incoming-transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const ObservableStore = require('obs-store')
const log = require('loglevel')
const BN = require('bn.js')
const createId = require('../lib/random-id')
const { bnToHex } = require('../lib/util')
const { bnToHex, fetchWithTimeout } = require('../lib/util')
const {
MAINNET_CODE,
ROPSTEN_CODE,
Expand All @@ -14,11 +14,14 @@ const {
MAINNET,
} = require('./network/enums')
const networkTypeToIdMap = {
[ROPSTEN]: ROPSTEN_CODE,
[RINKEBY]: RINKEYBY_CODE,
[KOVAN]: KOVAN_CODE,
[MAINNET]: MAINNET_CODE,
[ROPSTEN]: String(ROPSTEN_CODE),
[RINKEBY]: String(RINKEYBY_CODE),
[KOVAN]: String(KOVAN_CODE),
[MAINNET]: String(MAINNET_CODE),
}
const fetch = fetchWithTimeout({
timeout: 30000,
})

class IncomingTransactionsController {

Expand All @@ -33,6 +36,15 @@ class IncomingTransactionsController {
this.preferencesController = preferencesController
this.getCurrentNetwork = () => networkController.getProviderConfig().type

this._onLatestBlock = async (newBlockNumberHex) => {
const selectedAddress = this.preferencesController.getSelectedAddress()
const newBlockNumberDec = parseInt(newBlockNumberHex, 16)
await this._update({
address: selectedAddress,
newBlockNumberDec,
})
}

const initState = Object.assign({
incomingTransactions: {},
incomingTxLastFetchedBlocksByNetwork: {
Expand All @@ -51,20 +63,22 @@ class IncomingTransactionsController {
networkType: newType,
})
})
this.blockTracker.on('latest', async (newBlockNumberHex) => {
const address = this.preferencesController.getSelectedAddress()
await this._update({
address,
newBlockNumberDec: parseInt(newBlockNumberHex, 16),
})
})
this.preferencesController.store.subscribe(async ({ selectedAddress }) => {
await this._update({
address: selectedAddress,
})
})
}

start () {
this.blockTracker.removeListener('latest', this._onLatestBlock)
this.blockTracker.addListener('latest', this._onLatestBlock)
}

stop () {
this.blockTracker.removeListener('latest', this._onLatestBlock)
}

async _update ({ address, newBlockNumberDec, networkType } = {}) {
try {
const dataForUpdate = await this._getDataForUpdate({ address, newBlockNumberDec, networkType })
Expand Down
24 changes: 24 additions & 0 deletions app/scripts/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,29 @@ function removeListeners (listeners, emitter) {
})
}

function fetchWithTimeout ({ timeout = 120000 } = {}) {
return async function _fetch (url, opts) {
const abortController = new AbortController()
const abortSignal = abortController.signal
const f = fetch(url, {
...opts,
signal: abortSignal,
})

const timer = setTimeout(() => abortController.abort(), timeout)

try {
const res = await f
clearTimeout(timer)
return res
} catch (e) {
clearTimeout(timer)
throw e
}
}
}


module.exports = {
removeListeners,
applyListeners,
Expand All @@ -154,4 +177,5 @@ module.exports = {
hexToBn,
bnToHex,
BnMultiplyByFraction,
fetchWithTimeout,
}
2 changes: 2 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,10 @@ module.exports = class MetamaskController extends EventEmitter {
this.on('controllerConnectionChanged', (activeControllerConnections) => {
if (activeControllerConnections > 0) {
this.accountTracker.start()
this.incomingTransactionsController.start()
} else {
this.accountTracker.stop()
this.incomingTransactionsController.stop()
}
})

Expand Down
39 changes: 27 additions & 12 deletions test/unit/app/controllers/incoming-transactions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ describe('IncomingTransactionsController', () => {
}

const MOCK_BLOCKTRACKER = {
on: sinon.spy(),
addListener: sinon.spy(),
removeListener: sinon.spy(),
testProperty: 'fakeBlockTracker',
getCurrentBlock: () => '0xa',
}
Expand Down Expand Up @@ -95,17 +96,6 @@ describe('IncomingTransactionsController', () => {
})

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], {
address: '0x0101',
newBlockNumberDec: 2748,
})
})

it('should set the store to a provided initial state', () => {
Expand All @@ -120,6 +110,31 @@ describe('IncomingTransactionsController', () => {
})
})

describe('#start', () => {
it('should set up a listener for the latest block', () => {
const incomingTransactionsController = new IncomingTransactionsController({
blockTracker: MOCK_BLOCKTRACKER,
networkController: MOCK_NETWORK_CONTROLLER,
preferencesController: MOCK_PREFERENCES_CONTROLLER,
initState: {},
})
sinon.spy(incomingTransactionsController, '_update')

incomingTransactionsController.start()

assert(incomingTransactionsController.blockTracker.addListener.calledOnce)
assert.equal(incomingTransactionsController.blockTracker.addListener.getCall(0).args[0], 'latest')
const blockTrackerListenerCallback = incomingTransactionsController.blockTracker.addListener.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], {
address: '0x0101',
newBlockNumberDec: 2748,
})
})
})

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({
Expand Down
33 changes: 7 additions & 26 deletions ui/app/components/app/transaction-list/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,15 @@
}

&__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;

&__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;
@media screen and (max-width: $break-small) {
padding: 8px 0 8px 16px;
}
}

Expand Down
5 changes: 4 additions & 1 deletion ui/app/selectors/transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ import txHelper from '../../lib/tx-helper'
export const shapeShiftTxListSelector = state => state.metamask.shapeShiftTxList

export const incomingTxListSelector = state => {
const network = state.metamask.network
const selectedAddress = state.metamask.selectedAddress
return Object.values(state.metamask.incomingTransactions)
.filter(({ txParams }) => txParams.to === selectedAddress)
.filter(({ metamaskNetworkId, txParams }) => (
txParams.to === selectedAddress && metamaskNetworkId === network
))
}
export const unapprovedMsgsSelector = state => state.metamask.unapprovedMsgs
export const selectedAddressTxListSelector = state => state.metamask.selectedAddressTxList
Expand Down