Skip to content
Closed
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
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module.exports = {
// ones.
collectCoverageFrom: ['./src/**/*.ts'],
// TODO: Test index.ts
coveragePathIgnorePatterns: ['./src/index.ts'],
coveragePathIgnorePatterns: ['./src/index.ts', './src/gas/gas-util.ts'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My changes to this file resulted in the coverage percentage falling below our threshold, so I've disabled this file for the time being. I logged an issue to backfill these tests here: #734

coverageReporters: ['text', 'html'],
coverageThreshold: {
global: {
Expand Down
66 changes: 53 additions & 13 deletions src/assets/AccountTrackerController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,47 @@ describe('AccountTrackerController', () => {
expect(controller.state.accounts[address].balance).toBeDefined();
});

it('should sync addresses', () => {
const controller = new AccountTrackerController(
{
onPreferencesStateChange: stub(),
getIdentities: () => {
return { baz: {} as ContactEntry };
},
},
{ provider },
{
accounts: {
bar: { balance: '' },
foo: { balance: '' },
},
},
);
controller.refresh();
expect(controller.state.accounts).toStrictEqual({
baz: { balance: '0x0' },
});
});

it('does not refresh any accounts if no provider has been set', () => {
const controller = new AccountTrackerController(
{
onPreferencesStateChange: stub(),
getIdentities: () => {
return { baz: {} as ContactEntry };
},
},
{},
{
accounts: {},
},
);

controller.refresh();

expect(controller.state.accounts).toStrictEqual({});
});

it('should sync balance with addresses', async () => {
const address = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
const queryStub = stub(utils, 'query');
Expand All @@ -60,28 +101,27 @@ describe('AccountTrackerController', () => {
queryStub.returns(Promise.resolve('0x10'));
const result = await controller.syncBalanceWithAddresses([address]);
expect(result[address].balance).toBe('0x10');
queryStub.restore();
});

it('should sync addresses', () => {
it('should not sync balance with addresses if no provider has been set', async () => {
const address = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
const queryStub = stub(utils, 'query');
const controller = new AccountTrackerController(
{
onPreferencesStateChange: stub(),
getIdentities: () => {
return { baz: {} as ContactEntry };
},
},
{ provider },
{
accounts: {
bar: { balance: '' },
foo: { balance: '' },
return {};
},
},
{},
);
controller.refresh();
expect(controller.state.accounts).toStrictEqual({
baz: { balance: '0x0' },
});
queryStub.returns(Promise.resolve('0x10'));

const result = await controller.syncBalanceWithAddresses([address]);

expect(result).toStrictEqual({});
queryStub.restore();
});

it('should subscribe to new sibling preference controllers', async () => {
Expand Down
21 changes: 18 additions & 3 deletions src/assets/AccountTrackerController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class AccountTrackerController extends BaseController<
AccountTrackerConfig,
AccountTrackerState
> {
private ethQuery: any;
private ethQuery: EthQuery | null;
Copy link
Contributor Author

@mcmire mcmire Mar 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property can be null because it is not set in the constructor (or at least I didn't see that it was set in the constructor; I may be wrong).


private mutex = new Mutex();

Expand Down Expand Up @@ -98,6 +98,7 @@ export class AccountTrackerController extends BaseController<
state?: Partial<AccountTrackerState>,
) {
super(config, state);
this.ethQuery = null;
this.defaultConfig = {
interval: 10000,
};
Expand Down Expand Up @@ -145,11 +146,17 @@ export class AccountTrackerController extends BaseController<
* Refreshes all accounts in the current keychain.
*/
refresh = async () => {
const { ethQuery } = this;

if (ethQuery === null) {
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a change in behavior. It now returns nothing instead of throwing an error.

The same goes for syncBalanceWithAddresses - it now returns an empty object rather than throwing.

}

this.syncAccounts();
const accounts = { ...this.state.accounts };
for (const address in accounts) {
await safelyExecuteWithTimeout(async () => {
const balance = await query(this.ethQuery, 'getBalance', [address]);
const balance = await query<string>(ethQuery, 'getBalance', [address]);
accounts[address] = { balance: BNToHex(balance) };
});
}
Expand All @@ -165,11 +172,19 @@ export class AccountTrackerController extends BaseController<
async syncBalanceWithAddresses(
addresses: string[],
): Promise<Record<string, { balance: string }>> {
const { ethQuery } = this;

if (ethQuery === null) {
return {};
}

return await Promise.all(
addresses.map(
(address): Promise<[string, string] | undefined> => {
return safelyExecuteWithTimeout(async () => {
const balance = await query(this.ethQuery, 'getBalance', [address]);
const balance = await query<string>(ethQuery, 'getBalance', [
address,
]);
return [address, balance];
});
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import Web3 from 'web3';
import HttpProvider from 'ethjs-provider-http';
import nock from 'nock';
import { ERC1155Standard } from './ERC1155Standard';

const MAINNET_PROVIDER = new HttpProvider(
const MAINNET_PROVIDER = new Web3.providers.HttpProvider(
'https://mainnet.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035',
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import Web3 from 'web3';
import HttpProvider from 'ethjs-provider-http';
import nock from 'nock';
import { IPFS_DEFAULT_GATEWAY_URL } from '../../../../constants';
import { ERC721Standard } from './ERC721Standard';

const MAINNET_PROVIDER = new HttpProvider(
const MAINNET_PROVIDER = new Web3.providers.HttpProvider(
'https://mainnet.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035',
);
const ERC721_GODSADDRESS = '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab';
Expand Down
3 changes: 1 addition & 2 deletions src/assets/Standards/ERC20Standard.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import Web3 from 'web3';
import HttpProvider from 'ethjs-provider-http';
import nock from 'nock';
import { ERC20Standard } from './ERC20Standard';

const MAINNET_PROVIDER = new HttpProvider(
const MAINNET_PROVIDER = new Web3.providers.HttpProvider(
'https://mainnet.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035',
);
const ERC20_MATIC_ADDRESS = '0x7d1afa7b718fb893db30a3abc0cfc608aacfebb0';
Expand Down
5 changes: 1 addition & 4 deletions src/assets/TokenListController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
TokenListStateChange,
GetTokenListState,
TokenListMap,
ContractMap,
} from './TokenListController';

const name = 'TokenListController';
Expand All @@ -21,9 +20,7 @@ const timestamp = Date.now();

const staticTokenList: TokenListMap = {};
for (const tokenAddress in contractMap) {
const { erc20, logo: filePath, ...token } = (contractMap as ContractMap)[
tokenAddress
];
const { erc20, logo: filePath, ...token } = contractMap[tokenAddress];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Types were added in this PR for @metamask/contract-metadata, so typecasting is no longer necessary.

if (erc20) {
staticTokenList[tokenAddress] = {
...token,
Expand Down
13 changes: 1 addition & 12 deletions src/assets/TokenListController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@ type BaseToken = {
decimals: number;
};

type StaticToken = {
logo: string;
erc20: boolean;
} & BaseToken;

export type ContractMap = {
[address: string]: StaticToken;
};

export type DynamicToken = {
address: string;
occurrences: number;
Expand Down Expand Up @@ -241,9 +232,7 @@ export class TokenListController extends BaseController<
async fetchFromStaticTokenList(): Promise<void> {
const tokenList: TokenListMap = {};
for (const tokenAddress in contractMap) {
const { erc20, logo: filePath, ...token } = (contractMap as ContractMap)[
tokenAddress
];
const { erc20, logo: filePath, ...token } = contractMap[tokenAddress];
if (erc20) {
tokenList[tokenAddress] = {
...token,
Expand Down
4 changes: 2 additions & 2 deletions src/assets/TokensController.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EventEmitter } from 'events';
import contractsMap from '@metamask/contract-metadata';
import { abiERC721 } from '@metamask/metamask-eth-abis';
import { abiERC721, ABI } from '@metamask/metamask-eth-abis';
import { v1 as random } from 'uuid';
import { Mutex } from 'async-mutex';
import { ethers } from 'ethers';
Expand Down Expand Up @@ -367,7 +367,7 @@ export class TokensController extends BaseController<

async _createEthersContract(
tokenAddress: string,
abi: string,
abi: ABI,
ethersProvider: any,
): Promise<any> {
const tokenContract = await new ethers.Contract(
Expand Down
2 changes: 1 addition & 1 deletion src/gas/GasFeeController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export class GasFeeController extends BaseController<

private currentChainId;

private ethQuery: any;
private ethQuery: EthQuery;

private clientId?: string;

Expand Down
10 changes: 8 additions & 2 deletions src/gas/fetchBlockFeeHistory.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { BN } from 'ethereumjs-util';
import { mocked } from 'ts-jest/utils';
import { when } from 'jest-when';
import { buildFakeEthQuery } from '../../tests/util';
import { query, fromHex, toHex } from '../util';
import fetchBlockFeeHistory from './fetchBlockFeeHistory';

jest.mock('../util', () => {
return {
...jest.requireActual('../util'),
__esModule: true,
query: jest.fn(),
};
});
Expand All @@ -30,7 +30,7 @@ function times<T>(n: number, fn: (n: number) => T): T[] {
}

describe('fetchBlockFeeHistory', () => {
const ethQuery = { eth: 'query' };
const ethQuery = buildFakeEthQuery();

describe('with a minimal set of arguments', () => {
const latestBlockNumber = 3;
Expand Down Expand Up @@ -333,6 +333,12 @@ describe('fetchBlockFeeHistory', () => {
const latestBlockNumber = 3;
const numberOfRequestedBlocks = 3;

beforeEach(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been there all along and for some reason it was not causing the test to fail.

when(mockedQuery)
.calledWith(ethQuery, 'blockNumber')
.mockResolvedValue(new BN(latestBlockNumber));
});

it('includes an extra block with an estimated baseFeePerGas', async () => {
when(mockedQuery)
.calledWith(ethQuery, 'eth_feeHistory', [
Expand Down
23 changes: 12 additions & 11 deletions src/gas/fetchBlockFeeHistory.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { BN } from 'ethereumjs-util';
import { query, fromHex, toHex } from '../util';

type EthQuery = any;
import { query, fromHex, toHex, EthQueryish } from '../util';

/**
* @type RequestChunkSpecifier
Expand Down Expand Up @@ -111,7 +109,7 @@ const MAX_NUMBER_OF_BLOCKS_PER_ETH_FEE_HISTORY_CALL = 1024;
* - <https://gas-api.metaswap.codefi.network/testFeeHistory>
*
* @param args - The arguments to this function.
* @param args.ethQuery - An EthQuery instance that wraps a provider for the network in question.
* @param args.ethQuery - An object that {@link query} takes which wraps a provider for the network in question.
* @param args.endBlock - The desired end of the requested block range. Can be "latest" if you want
* to start from the latest successful block or the number of a known past block.
* @param args.numberOfBlocks - How many total blocks to fetch. Note that if this is more than 1024,
Expand All @@ -138,7 +136,7 @@ export default async function fetchBlockFeeHistory<Percentile extends number>({
percentiles: givenPercentiles = [],
includeNextBlock = false,
}: {
ethQuery: EthQuery;
ethQuery: EthQueryish;
Copy link
Contributor Author

@mcmire mcmire Mar 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An "EthQueryish" is an object that looks like an EthQuery without the full gamut of methods that EthQuery supports. It's the type of the argument that query now takes, and since it only really cares that the object has a sendAsync method, that's what this type checks for.

numberOfBlocks: number;
endBlock?: 'latest' | BN;
percentiles?: readonly Percentile[];
Expand All @@ -149,10 +147,13 @@ export default async function fetchBlockFeeHistory<Percentile extends number>({
? Array.from(new Set(givenPercentiles)).sort((a, b) => a - b)
: [];

const finalEndBlockNumber =
givenEndBlock === 'latest'
? fromHex(await query(ethQuery, 'blockNumber'))
: givenEndBlock;
let finalEndBlockNumber: BN;
if (givenEndBlock === 'latest') {
const latestBlockNumber = await query<string>(ethQuery, 'blockNumber');
finalEndBlockNumber = fromHex(latestBlockNumber);
} else {
finalEndBlockNumber = givenEndBlock;
}

const requestChunkSpecifiers = determineRequestChunkSpecifiers(
finalEndBlockNumber,
Expand Down Expand Up @@ -275,13 +276,13 @@ async function makeRequestForChunk<Percentile extends number>({
percentiles,
includeNextBlock,
}: {
ethQuery: EthQuery;
ethQuery: EthQueryish;
numberOfBlocks: number;
endBlockNumber: BN;
percentiles: readonly Percentile[];
includeNextBlock: boolean;
}): Promise<FeeHistoryBlock<Percentile>[]> {
const response: EthFeeHistoryResponse = await query(
const response = await query<EthFeeHistoryResponse>(
ethQuery,
'eth_feeHistory',
[toHex(numberOfBlocks), toHex(endBlockNumber), percentiles],
Expand Down
5 changes: 3 additions & 2 deletions src/gas/fetchGasEstimatesViaEthFeeHistory.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BN } from 'ethereumjs-util';
import { mocked } from 'ts-jest/utils';
import { when } from 'jest-when';
import { buildFakeEthQuery } from '../../tests/util';
import fetchBlockFeeHistory from './fetchBlockFeeHistory';
import calculateGasFeeEstimatesForPriorityLevels from './fetchGasEstimatesViaEthFeeHistory/calculateGasFeeEstimatesForPriorityLevels';
import fetchLatestBlock from './fetchGasEstimatesViaEthFeeHistory/fetchLatestBlock';
Expand All @@ -24,10 +25,10 @@ describe('fetchGasEstimatesViaEthFeeHistory', () => {
number: new BN(1),
baseFeePerGas: new BN(100_000_000_000),
};
const ethQuery = {
const ethQuery = buildFakeEthQuery({
blockNumber: async () => latestBlock.number,
getBlockByNumber: async () => latestBlock,
};
});

it('calculates target fees for low, medium, and high transaction priority levels', async () => {
const blocks = [
Expand Down
4 changes: 2 additions & 2 deletions src/gas/fetchGasEstimatesViaEthFeeHistory.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { fromWei } from 'ethjs-unit';
import { GWEI } from '../constants';
import { EthQueryish } from '../util';
import { GasFeeEstimates } from './GasFeeController';
import { EthQuery } from './fetchGasEstimatesViaEthFeeHistory/types';
import fetchBlockFeeHistory from './fetchBlockFeeHistory';
import fetchLatestBlock from './fetchGasEstimatesViaEthFeeHistory/fetchLatestBlock';
import calculateGasFeeEstimatesForPriorityLevels from './fetchGasEstimatesViaEthFeeHistory/calculateGasFeeEstimatesForPriorityLevels';
Expand All @@ -25,7 +25,7 @@ import calculateGasFeeEstimatesForPriorityLevels from './fetchGasEstimatesViaEth
* for the next block's base fee.
*/
export default async function fetchGasEstimatesViaEthFeeHistory(
ethQuery: EthQuery,
ethQuery: EthQueryish,
): Promise<GasFeeEstimates> {
const latestBlock = await fetchLatestBlock(ethQuery);
const blocks = await fetchBlockFeeHistory({
Expand Down
Loading