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

patch([email protected]): revert "use hardcoded Infura gas API urls" (#4068) #4403

Draft
wants to merge 2 commits into
base: base/patch/extension-gas-api-endpoint
Choose a base branch
from
Draft
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 packages/gas-fee-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module.exports = merge(baseConfig, {
coverageThreshold: {
global: {
branches: 81.35,
functions: 81.57,
functions: 80.55,
lines: 86.28,
statements: 86.44,
},
Expand Down
1 change: 1 addition & 0 deletions packages/gas-fee-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"deepmerge": "^4.2.2",
"jest": "^27.5.1",
"jest-when": "^3.4.2",
"nock": "^13.3.1",
"sinon": "^9.2.4",
"ts-jest": "^27.1.4",
"typedoc": "^0.24.8",
Expand Down
106 changes: 67 additions & 39 deletions packages/gas-fee-controller/src/GasFeeController.test.ts

Large diffs are not rendered by default.

23 changes: 12 additions & 11 deletions packages/gas-fee-controller/src/GasFeeController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ import { v1 as random } from 'uuid';

import determineGasFeeCalculations from './determineGasFeeCalculations';
import {
calculateTimeEstimate,
fetchGasEstimates,
fetchLegacyGasPriceEstimates,
fetchEthGasPriceEstimate,
calculateTimeEstimate,
} from './gas-util';

export const GAS_API_BASE_URL = 'https://gas.api.infura.io';
export const LEGACY_GAS_PRICES_API_URL = `https://api.metaswap.codefi.network/gasPrices`;

export type unknownString = 'unknown';

Expand Down Expand Up @@ -278,8 +278,6 @@ export class GasFeeController extends StaticIntervalPollingController<

private readonly getCurrentAccountEIP1559Compatibility;

private readonly infuraAPIKey: string;

private currentChainId;

private ethQuery?: EthQuery;
Expand All @@ -305,9 +303,11 @@ export class GasFeeController extends StaticIntervalPollingController<
* @param options.getProvider - Returns a network provider for the current network.
* @param options.onNetworkDidChange - A function for registering an event handler for the
* network state change event.
* @param options.legacyAPIEndpoint - The legacy gas price API URL. This option is primarily for
* testing purposes.
* @param options.EIP1559APIEndpoint - The EIP-1559 gas price API URL.
* @param options.clientId - The client ID used to identify to the gas estimation API who is
* asking for estimates.
* @param options.infuraAPIKey - The Infura API key used for infura API requests.
*/
constructor({
interval = 15000,
Expand All @@ -319,8 +319,9 @@ export class GasFeeController extends StaticIntervalPollingController<
getCurrentNetworkLegacyGasAPICompatibility,
getProvider,
onNetworkDidChange,
legacyAPIEndpoint = LEGACY_GAS_PRICES_API_URL,
EIP1559APIEndpoint,
clientId,
infuraAPIKey,
}: {
interval?: number;
messenger: GasFeeMessenger;
Expand All @@ -331,8 +332,10 @@ export class GasFeeController extends StaticIntervalPollingController<
getChainId?: () => Hex;
getProvider: () => ProviderProxy;
onNetworkDidChange?: (listener: (state: NetworkState) => void) => void;
legacyAPIEndpoint?: string;
// eslint-disable-next-line @typescript-eslint/naming-convention
EIP1559APIEndpoint: string;
clientId?: string;
infuraAPIKey: string;
}) {
super({
name,
Expand All @@ -350,10 +353,9 @@ export class GasFeeController extends StaticIntervalPollingController<
this.getCurrentAccountEIP1559Compatibility =
getCurrentAccountEIP1559Compatibility;
this.#getProvider = getProvider;
this.EIP1559APIEndpoint = `${GAS_API_BASE_URL}/networks/<chain_id>/suggestedGasFees`;
this.legacyAPIEndpoint = `${GAS_API_BASE_URL}/networks/<chain_id>/gasPrices`;
this.EIP1559APIEndpoint = EIP1559APIEndpoint;
this.legacyAPIEndpoint = legacyAPIEndpoint;
this.clientId = clientId;
this.infuraAPIKey = infuraAPIKey;

this.ethQuery = new EthQuery(this.#getProvider());

Expand Down Expand Up @@ -475,7 +477,6 @@ export class GasFeeController extends StaticIntervalPollingController<
calculateTimeEstimate,
clientId: this.clientId,
ethQuery,
infuraAPIKey: this.infuraAPIKey,
nonRPCGasFeeApisDisabled: this.state.nonRPCGasFeeApisDisabled,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ const mockedCalculateTimeEstimate = calculateTimeEstimate as jest.Mock<
Parameters<typeof calculateTimeEstimate>
>;

const INFURA_API_KEY_MOCK = 'test';

/**
* Builds mock data for the `fetchGasEstimates` function. All of the data here is filled in to make
* the gas fee estimation code function in a way that represents a reasonably happy path; it does
Expand Down Expand Up @@ -126,7 +124,6 @@ describe('determineGasFeeCalculations', () => {
calculateTimeEstimate: mockedCalculateTimeEstimate,
clientId: 'some-client-id',
ethQuery: {},
infuraAPIKey: INFURA_API_KEY_MOCK,
};

describe('when isEIP1559Compatible is true', () => {
Expand Down
13 changes: 1 addition & 12 deletions packages/gas-fee-controller/src/determineGasFeeCalculations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ type DetermineGasFeeCalculationsRequest = {
isLegacyGasAPICompatible: boolean;
fetchGasEstimates: (
url: string,
infuraAPIKey: string,
clientId?: string,
) => Promise<GasFeeEstimates>;
fetchGasEstimatesUrl: string;
fetchLegacyGasPriceEstimates: (
url: string,
infuraAPIKey: string,
clientId?: string,
) => Promise<LegacyGasPriceEstimate>;
fetchLegacyGasPriceEstimatesUrl: string;
Expand All @@ -34,7 +32,6 @@ type DetermineGasFeeCalculationsRequest = {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ethQuery: any;
infuraAPIKey: string;
nonRPCGasFeeApisDisabled?: boolean;
};

Expand All @@ -60,7 +57,6 @@ type DetermineGasFeeCalculationsRequest = {
* @param args.calculateTimeEstimate - A function that determine time estimate bounds.
* @param args.clientId - An identifier that an API can use to know who is asking for estimates.
* @param args.ethQuery - An EthQuery instance we can use to talk to Ethereum directly.
* @param args.infuraAPIKey - Infura API key to use for requests to Infura.
* @param args.nonRPCGasFeeApisDisabled - Whether to disable requests to the legacyAPIEndpoint and the EIP1559APIEndpoint
* @returns The gas fee calculations.
*/
Expand Down Expand Up @@ -120,16 +116,11 @@ async function getEstimatesUsingFeeMarketEndpoint(
const {
fetchGasEstimates,
fetchGasEstimatesUrl,
infuraAPIKey,
clientId,
calculateTimeEstimate,
} = request;

const estimates = await fetchGasEstimates(
fetchGasEstimatesUrl,
infuraAPIKey,
clientId,
);
const estimates = await fetchGasEstimates(fetchGasEstimatesUrl, clientId);

const { suggestedMaxPriorityFeePerGas, suggestedMaxFeePerGas } =
estimates.medium;
Expand Down Expand Up @@ -158,13 +149,11 @@ async function getEstimatesUsingLegacyEndpoint(
const {
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl,
infuraAPIKey,
clientId,
} = request;

const estimates = await fetchLegacyGasPriceEstimates(
fetchLegacyGasPriceEstimatesUrl,
infuraAPIKey,
clientId,
);

Expand Down
130 changes: 42 additions & 88 deletions packages/gas-fee-controller/src/gas-util.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { handleFetch } from '@metamask/controller-utils';
import nock from 'nock';

import {
fetchLegacyGasPriceEstimates,
Expand All @@ -8,14 +8,6 @@ import {
} from './gas-util';
import type { GasFeeEstimates } from './GasFeeController';

jest.mock('@metamask/controller-utils', () => {
return {
...jest.requireActual('@metamask/controller-utils'),
handleFetch: jest.fn(),
};
});
const handleFetchMock = jest.mocked(handleFetch);

const mockEIP1559ApiResponses: GasFeeEstimates[] = [
{
low: {
Expand Down Expand Up @@ -73,49 +65,27 @@ const mockEIP1559ApiResponses: GasFeeEstimates[] = [
},
];

const INFURA_API_KEY_MOCK = 'test';
const INFURA_AUTH_TOKEN_MOCK = 'dGVzdDo=';
const INFURA_GAS_API_URL_MOCK = 'https://gas.api.infura.io';

describe('gas utils', () => {
beforeEach(() => {
jest.resetAllMocks();
});

describe('fetchGasEstimates', () => {
it('should fetch external gasFeeEstimates when data is valid', async () => {
handleFetchMock.mockResolvedValue(mockEIP1559ApiResponses[0]);
const result = await fetchGasEstimates(
INFURA_GAS_API_URL_MOCK,
INFURA_API_KEY_MOCK,
);

expect(handleFetchMock).toHaveBeenCalledTimes(1);
expect(handleFetchMock).toHaveBeenCalledWith(INFURA_GAS_API_URL_MOCK, {
headers: expect.objectContaining({
Authorization: `Basic ${INFURA_AUTH_TOKEN_MOCK}`,
}),
});
const scope = nock('https://not-a-real-url/')
.get(/.+/u)
.reply(200, mockEIP1559ApiResponses[0])
.persist();
const result = await fetchGasEstimates('https://not-a-real-url/');
expect(result).toMatchObject(mockEIP1559ApiResponses[0]);
scope.done();
});

it('should fetch external gasFeeEstimates with client id header when clientId arg is added', async () => {
const clientIdMock = 'test';
handleFetchMock.mockResolvedValue(mockEIP1559ApiResponses[0]);
const result = await fetchGasEstimates(
INFURA_GAS_API_URL_MOCK,
INFURA_API_KEY_MOCK,
clientIdMock,
);

expect(handleFetchMock).toHaveBeenCalledTimes(1);
expect(handleFetchMock).toHaveBeenCalledWith(INFURA_GAS_API_URL_MOCK, {
headers: expect.objectContaining({
Authorization: `Basic ${INFURA_AUTH_TOKEN_MOCK}`,
'X-Client-Id': clientIdMock,
}),
});
const scope = nock('https://not-a-real-url/')
.matchHeader('x-client-id', 'test')
.get(/.+/u)
.reply(200, mockEIP1559ApiResponses[0])
.persist();
const result = await fetchGasEstimates('https://not-a-real-url/', 'test');
expect(result).toMatchObject(mockEIP1559ApiResponses[0]);
scope.done();
});

it('should fetch and normalize external gasFeeEstimates when data is has an invalid number of decimals', async () => {
Expand All @@ -141,73 +111,57 @@ describe('gas utils', () => {
estimatedBaseFee: '32.000000017',
};

handleFetchMock.mockResolvedValue(mockEIP1559ApiResponses[1]);
const result = await fetchGasEstimates(
INFURA_GAS_API_URL_MOCK,
INFURA_API_KEY_MOCK,
);
const scope = nock('https://not-a-real-url/')
.get(/.+/u)
.reply(200, mockEIP1559ApiResponses[1])
.persist();
const result = await fetchGasEstimates('https://not-a-real-url/');
expect(result).toMatchObject(expectedResult);
scope.done();
});
});

describe('fetchLegacyGasPriceEstimates', () => {
it('should fetch external gasPrices and return high/medium/low', async () => {
handleFetchMock.mockResolvedValue({
SafeGasPrice: '22',
ProposeGasPrice: '25',
FastGasPrice: '30',
});
const scope = nock('https://not-a-real-url/')
.get(/.+/u)
.reply(200, {
SafeGasPrice: '22',
ProposeGasPrice: '25',
FastGasPrice: '30',
})
.persist();
const result = await fetchLegacyGasPriceEstimates(
INFURA_GAS_API_URL_MOCK,
INFURA_API_KEY_MOCK,
);

expect(handleFetchMock).toHaveBeenCalledTimes(1);
expect(handleFetchMock).toHaveBeenCalledWith(
INFURA_GAS_API_URL_MOCK,

expect.objectContaining({
headers: expect.objectContaining({
Authorization: `Basic ${INFURA_AUTH_TOKEN_MOCK}`,
}),
}),
'https://not-a-real-url/',
);
expect(result).toMatchObject({
high: '30',
medium: '25',
low: '22',
});
scope.done();
});

it('should fetch external gasPrices with client id header when clientId arg is passed', async () => {
const clientIdMock = 'test';
handleFetchMock.mockResolvedValue({
SafeGasPrice: '22',
ProposeGasPrice: '25',
FastGasPrice: '30',
});
const scope = nock('https://not-a-real-url/')
.matchHeader('x-client-id', 'test')
.get(/.+/u)
.reply(200, {
SafeGasPrice: '22',
ProposeGasPrice: '25',
FastGasPrice: '30',
})
.persist();
const result = await fetchLegacyGasPriceEstimates(
INFURA_GAS_API_URL_MOCK,
INFURA_API_KEY_MOCK,
clientIdMock,
);

expect(handleFetchMock).toHaveBeenCalledTimes(1);
expect(handleFetchMock).toHaveBeenCalledWith(
INFURA_GAS_API_URL_MOCK,

expect.objectContaining({
headers: expect.objectContaining({
Authorization: `Basic ${INFURA_AUTH_TOKEN_MOCK}`,
'X-Client-Id': clientIdMock,
}),
}),
'https://not-a-real-url/',
'test',
);
expect(result).toMatchObject({
high: '30',
medium: '25',
low: '22',
});
scope.done();
});
});

Expand Down
Loading