Account tracker improvement

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",

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<AccountTrackerConfig, AccountTrackerState> {
-  private ethjsQuery: any;
+  private ethQuery: any;
+
+  private mutex = new Mutex();
 
   private handle?: NodeJS.Timer;
 
@@ -95,7 +98,7 @@ export class AccountTrackerController extends BaseController<AccountTrackerConfi
    * @param provider - Provider used to create a new underlying EthQuery instance
    */
   set provider(provider: any) {
-    this.ethjsQuery = new EthjsQuery(provider);
+    this.ethQuery = new EthQuery(provider);
   }
 
   /**
@@ -115,10 +118,12 @@ export class AccountTrackerController extends BaseController<AccountTrackerConfi
    * @param interval - Polling interval trigger a 'refresh'
    */
   async poll(interval?: number): Promise<void> {
+    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<AccountTrackerConfi
     this.syncAccounts();
     const { accounts } = this.state;
     for (const address in accounts) {
-      await safelyExecute(async () => {
-        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 } });
       });

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<TransactionConfig, Tra
     this.hub.emit(`${transactionMeta.id}:finished`, transactionMeta);
   }
 
-  private query(method: string, args: any[] = []): Promise<any> {
-    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<MethodData> {
     const registryMethod = await this.registry.lookup(fourBytePrefix);
     const parsedRegistryMethod = this.registry.parse(registryMethod);
@@ -490,7 +479,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
 
     try {
       transactionMeta.status = 'approved';
-      transactionMeta.transaction.nonce = await this.query('getTransactionCount', [from, 'pending']);
+      transactionMeta.transaction.nonce = await query(this.ethQuery, 'getTransactionCount', [from, 'pending']);
       transactionMeta.transaction.chainId = parseInt(currentChainId, undefined);
 
       const ethTransaction = new Transaction({ ...transactionMeta.transaction });
@@ -501,7 +490,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
 
       transactionMeta.rawTransaction = rawTransaction;
       this.updateTransaction(transactionMeta);
-      const transactionHash = await this.query('sendRawTransaction', [rawTransaction]);
+      const transactionHash = await query(this.ethQuery, 'sendRawTransaction', [rawTransaction]);
       transactionMeta.transactionHash = transactionHash;
       transactionMeta.status = 'submitted';
       this.updateTransaction(transactionMeta);
@@ -560,7 +549,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
 
     await this.sign(ethTransaction, transactionMeta.transaction.from);
     const rawTransaction = bufferToHex(ethTransaction.serialize());
-    await this.query('sendRawTransaction', [rawTransaction]);
+    await query(this.ethQuery, 'sendRawTransaction', [rawTransaction]);
     transactionMeta.status = 'cancelled';
     this.hub.emit(`${transactionMeta.id}:finished`, transactionMeta);
   }
@@ -590,7 +579,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
     const ethTransaction = new Transaction({ ...transactionMeta.transaction, gasPrice });
     await this.sign(ethTransaction, transactionMeta.transaction.from);
     const rawTransaction = bufferToHex(ethTransaction.serialize());
-    const transactionHash = await this.query('sendRawTransaction', [rawTransaction]);
+    const transactionHash = await query(this.ethQuery, 'sendRawTransaction', [rawTransaction]);
     const newTransactionMeta = {
       ...transactionMeta,
       id: random(),
@@ -614,9 +603,9 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
    */
   async estimateGas(transaction: Transaction) {
     const estimatedTransaction = { ...transaction };
-    const { gasLimit } = await this.query('getBlockByNumber', ['latest', false]);
+    const { gasLimit } = await query(this.ethQuery, 'getBlockByNumber', ['latest', false]);
     const { gas, gasPrice: providedGasPrice, to, value, data } = estimatedTransaction;
-    const gasPrice = typeof providedGasPrice === 'undefined' ? await this.query('gasPrice') : providedGasPrice;
+    const gasPrice = typeof providedGasPrice === 'undefined' ? await query(this.ethQuery, 'gasPrice') : providedGasPrice;
 
     // 1. If gas is already defined on the transaction, use it
     if (typeof gas !== 'undefined') {
@@ -625,7 +614,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
 
     // 2. If to is not defined or this is not a contract address, and there is no data use 0x5208 / 21000
     /* istanbul ignore next */
-    const code = to ? await this.query('getCode', [to]) : undefined;
+    const code = to ? await query(this.ethQuery, 'getCode', [to]) : undefined;
     /* istanbul ignore next */
     if (!to || (to && !data && (!code || code === '0x'))) {
       return { gas: '0x5208', gasPrice };
@@ -636,7 +625,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
     estimatedTransaction.value = typeof value === 'undefined' ? '0x0' : /* istanbul ignore next */ value;
     const gasLimitBN = hexToBN(gasLimit);
     estimatedTransaction.gas = BNToHex(fractionBN(gasLimitBN, 19, 20));
-    const gasHex = await this.query('estimateGas', [estimatedTransaction]);
+    const gasHex = await query(this.ethQuery, 'estimateGas', [estimatedTransaction]);
 
     // 4. Pad estimated gas without exceeding the most recent block gasLimit
     const gasBN = hexToBN(gasHex);
@@ -686,7 +675,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
       Promise.all(
         transactions.map(async (meta, index) => {
           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<TransactionConfig, Tra
       if (tx.toSmartContract === undefined) {
         // If not `to` is a contract deploy, if not `data` is send eth
         if (tx.transaction.to && (!tx.transaction.data || tx.transaction.data !== '0x')) {
-          const code = await this.query('getCode', [tx.transaction.to]);
+          const code = await query(this.ethQuery, 'getCode', [tx.transaction.to]);
           tx.toSmartContract = isSmartContractCode(code);
         } else {
           tx.toSmartContract = false;

src/util.ts

@@ -215,6 +215,33 @@ export async function safelyExecute(operation: () => Promise<any>, 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<any>, logError = false, timeout = 500) {
+  try {
+    return await Promise.race([
+      operation(),
+      new Promise<void>((_, 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,16 +472,39 @@ 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<any> {
+  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,
   hexToText,
   isSmartContractCode,
   normalizeTransaction,
   safelyExecute,
+  safelyExecuteWithTimeout,
   successfulFetch,
   timeoutFetch,
   validateTokenToWatch,

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');
+      }
+    });
+  });
 });

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"