Skip to content

Commit

Permalink
Account tracker improvement (#331)
Browse files Browse the repository at this point in the history 
* fix

* test

* test

* prettier

* log

* safelyExecuteWithTimeouttest

* format
  • Loading branch information
@estebanmino
estebanmino authored Feb 4, 2021
1 parent f3d4ddf commit 4381e4d
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 40 deletions.
1 change: 0 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 12 additions & 7 deletions src/assets/AccountTrackerController.ts
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}
Expand All @@ -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 } });
});
Expand Down
33 changes: 11 additions & 22 deletions src/transaction/TransactionController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
validateTransaction,
isSmartContractCode,
handleTransactionFetch,
query,
} from '../util';

const MethodRegistry = require('eth-method-registry');
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 });
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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') {
Expand All @@ -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 };
Expand All @@ -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);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down
50 changes: 50 additions & 0 deletions src/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
83 changes: 83 additions & 0 deletions tests/util.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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');
}
});
});
});
10 changes: 0 additions & 10 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 4381e4d

Please sign in to comment.