diff --git a/package.json b/package.json index 6c8d1d88021..663dcf411eb 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,6 @@ "eth-sig-util": "^3.0.0", "ethereumjs-util": "^6.1.0", "ethereumjs-wallet": "^0.6.4", - "ethjs-query": "^0.3.8", "human-standard-collectible-abi": "^1.0.2", "human-standard-token-abi": "^2.0.0", "isomorphic-fetch": "^3.0.0", diff --git a/src/assets/AccountTrackerController.ts b/src/assets/AccountTrackerController.ts index 2cd3734936b..d378669fe66 100644 --- a/src/assets/AccountTrackerController.ts +++ b/src/assets/AccountTrackerController.ts @@ -1,8 +1,9 @@ import BaseController, { BaseConfig, BaseState } from '../BaseController'; import PreferencesController from '../user/PreferencesController'; -import { BNToHex, safelyExecute } from '../util'; +import { BNToHex, query, safelyExecuteWithTimeout } from '../util'; -const EthjsQuery = require('ethjs-query'); +const EthQuery = require('eth-query'); +const { Mutex } = require('await-semaphore'); /** * @type AccountInformation @@ -42,7 +43,9 @@ export interface AccountTrackerState extends BaseState { * Controller that tracks information for all accounts in the current keychain */ export class AccountTrackerController extends BaseController { - private ethjsQuery: any; + private ethQuery: any; + + private mutex = new Mutex(); private handle?: NodeJS.Timer; @@ -95,7 +98,7 @@ export class AccountTrackerController extends BaseController { + const releaseLock = await this.mutex.acquire(); interval && this.configure({ interval }, false, false); this.handle && clearTimeout(this.handle); - await safelyExecute(() => this.refresh()); + await this.refresh(); this.handle = setTimeout(() => { + releaseLock(); this.poll(this.config.interval); }, this.config.interval); } @@ -130,8 +135,8 @@ export class AccountTrackerController extends BaseController { - const balance = await this.ethjsQuery.getBalance(address); + await safelyExecuteWithTimeout(async () => { + const balance = await query(this.ethQuery, 'getBalance', [address]); accounts[address] = { balance: BNToHex(balance) }; this.update({ accounts: { ...accounts } }); }); diff --git a/src/transaction/TransactionController.ts b/src/transaction/TransactionController.ts index 404e52fc8c9..33bb86e3040 100644 --- a/src/transaction/TransactionController.ts +++ b/src/transaction/TransactionController.ts @@ -13,6 +13,7 @@ import { validateTransaction, isSmartContractCode, handleTransactionFetch, + query, } from '../util'; const MethodRegistry = require('eth-method-registry'); @@ -240,18 +241,6 @@ export class TransactionController extends BaseController { - return new Promise((resolve, reject) => { - this.ethQuery[method](...args, (error: Error, result: any) => { - if (error) { - reject(error); - return; - } - resolve(result); - }); - }); - } - private async registryLookup(fourBytePrefix: string): Promise { const registryMethod = await this.registry.lookup(fourBytePrefix); const parsedRegistryMethod = this.registry.parse(registryMethod); @@ -490,7 +479,7 @@ export class TransactionController extends BaseController { if (meta.status === 'submitted' && meta.networkID === currentNetworkID) { - const txObj = await this.query('getTransactionByHash', [meta.transactionHash]); + const txObj = await query(this.ethQuery, 'getTransactionByHash', [meta.transactionHash]); /* istanbul ignore else */ if (txObj && txObj.blockNumber) { transactions[index].status = 'confirmed'; @@ -806,7 +795,7 @@ export class TransactionController extends BaseController Promise, logError = fa } } +/** + * Execute and return an asynchronous operation with a timeout + * + * @param operation - Function returning a Promise + * @param logError - Determines if the error should be logged + * @param retry - Function called if an error is caught + * @param timeout - Timeout to fail the operation + * @returns - Promise resolving to the result of the async operation + */ +export async function safelyExecuteWithTimeout(operation: () => Promise, logError = false, timeout = 500) { + try { + return await Promise.race([ + operation(), + new Promise((_, reject) => + setTimeout(() => { + reject(new Error('timeout')); + }, timeout), + ), + ]); + } catch (error) { + /* istanbul ignore next */ + if (logError) { + console.error(error); + } + } +} + /** * Validates a Transaction object for required properties and throws in * the event of any validation error. @@ -445,9 +472,31 @@ export function normalizeEnsName(ensName: string): string | null { return null; } +/** + * Wrapper method to handle EthQuery requests + * + * @param ethQuery - EthQuery object initialized with a provider + * @param method - Method to request + * @param args - Arguments to send + * + * @returns - Promise resolving the request + */ +export function query(ethQuery: any, method: string, args: any[] = []): Promise { + return new Promise((resolve, reject) => { + ethQuery[method](...args, (error: Error, result: any) => { + if (error) { + reject(error); + return; + } + resolve(result); + }); + }); +} + export default { BNToHex, fractionBN, + query, getBuyURL, handleFetch, hexToBN, @@ -455,6 +504,7 @@ export default { isSmartContractCode, normalizeTransaction, safelyExecute, + safelyExecuteWithTimeout, successfulFetch, timeoutFetch, validateTokenToWatch, diff --git a/tests/util.test.ts b/tests/util.test.ts index dfcc20d3099..12a27e6df16 100644 --- a/tests/util.test.ts +++ b/tests/util.test.ts @@ -7,6 +7,46 @@ const { BN } = require('ethereumjs-util'); const SOME_API = 'https://someapi.com'; const SOME_FAILING_API = 'https://somefailingapi.com'; +const HttpProvider = require('ethjs-provider-http'); +const EthQuery = require('eth-query'); + +const mockFlags: { [key: string]: any } = { + estimateGas: null, + gasPrice: null, +}; +const PROVIDER = new HttpProvider('https://ropsten.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035'); + +jest.mock('eth-query', () => + jest.fn().mockImplementation(() => { + return { + estimateGas: (_transaction: any, callback: any) => { + callback(undefined, '0x0'); + }, + gasPrice: (callback: any) => { + if (mockFlags.gasPrice) { + callback(new Error(mockFlags.gasPrice)); + return; + } + callback(undefined, '0x0'); + }, + getBlockByNumber: (_blocknumber: any, _fetchTxs: boolean, callback: any) => { + callback(undefined, { gasLimit: '0x0' }); + }, + getCode: (_to: any, callback: any) => { + callback(undefined, '0x0'); + }, + getTransactionByHash: (_hash: any, callback: any) => { + callback(undefined, { blockNumber: '0x1' }); + }, + getTransactionCount: (_from: any, _to: any, callback: any) => { + callback(undefined, '0x0'); + }, + sendRawTransaction: (_transaction: any, callback: any) => { + callback(undefined, '1337'); + }, + }; + }), +); describe('util', () => { beforeEach(() => { @@ -79,6 +119,31 @@ describe('util', () => { }); }); + describe('safelyExecuteWithTimeout', () => { + it('should swallow errors', async () => { + await util.safelyExecuteWithTimeout(() => { + throw new Error('ahh'); + }); + }); + + it('should resolve', async () => { + const response = await util.safelyExecuteWithTimeout(() => { + return new Promise((res) => setTimeout(() => res('response'), 200)); + }); + expect(response).toEqual('response'); + }); + + it('should timeout', () => { + try { + util.safelyExecuteWithTimeout(() => { + return new Promise((res) => setTimeout(res, 800)); + }); + } catch (e) { + expect(e.message).toContain('timeout'); + } + }); + }); + describe('validateTransaction', () => { it('should throw if no from address', () => { expect(() => util.validateTransaction({} as any)).toThrow(); @@ -577,4 +642,22 @@ describe('util', () => { expect(invalid).toBeNull(); }); }); + + describe('query', () => { + it('should query and resolve', async () => { + const ethQuery = new EthQuery(PROVIDER); + const gasPrice = await util.query(ethQuery, 'gasPrice', []); + expect(gasPrice).toEqual('0x0'); + }); + + it('should query and reject if error', async () => { + const ethQuery = new EthQuery(PROVIDER); + mockFlags.gasPrice = 'Uh oh'; + try { + await util.query(ethQuery, 'gasPrice', []); + } catch (error) { + expect(error.message).toContain('Uh oh'); + } + }); + }); }); diff --git a/yarn.lock b/yarn.lock index e43545c1d8c..bf330f67015 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2808,16 +2808,6 @@ ethjs-query@0.3.7: ethjs-rpc "0.2.0" promise-to-callback "^1.0.0" -ethjs-query@^0.3.8: - version "0.3.8" - resolved "https://registry.yarnpkg.com/ethjs-query/-/ethjs-query-0.3.8.tgz#aa5af02887bdd5f3c78b3256d0f22ffd5d357490" - integrity sha512-/J5JydqrOzU8O7VBOwZKUWXxHDGr46VqNjBCJgBVNNda+tv7Xc8Y2uJc6aMHHVbeN3YOQ7YRElgIc0q1CI02lQ== - dependencies: - babel-runtime "^6.26.0" - ethjs-format "0.2.7" - ethjs-rpc "0.2.0" - promise-to-callback "^1.0.0" - ethjs-rpc@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/ethjs-rpc/-/ethjs-rpc-0.2.0.tgz#3d0011e32cfff156ed6147818c6fb8f801701b4c"