Skip to content

Commit

Permalink
Remove fee history fallback (#4210)
Browse files Browse the repository at this point in the history
Remove the fee history logic from the `GasFeeController`, meaning if the gas API does not support a network, a standard `eth_gasPrice` call is the only fallback.
  • Loading branch information
matthewwalsh0 committed Apr 27, 2024
1 parent 843a794 commit 9349754
Show file tree
Hide file tree
Showing 14 changed files with 186 additions and 1,422 deletions.
8 changes: 4 additions & 4 deletions packages/gas-fee-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 82.5,
functions: 81,
lines: 88,
statements: 88,
branches: 81.35,
functions: 81.57,
lines: 86.28,
statements: 86.44,
},
},
});
6 changes: 0 additions & 6 deletions packages/gas-fee-controller/src/GasFeeController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import type { Hex } from '@metamask/utils';
import * as sinon from 'sinon';

import determineGasFeeCalculations from './determineGasFeeCalculations';
import fetchGasEstimatesViaEthFeeHistory from './fetchGasEstimatesViaEthFeeHistory';
import {
fetchGasEstimates,
fetchLegacyGasPriceEstimates,
Expand Down Expand Up @@ -341,7 +340,6 @@ describe('GasFeeController', () => {
isLegacyGasAPICompatible: true,
fetchGasEstimates,
fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`,
fetchGasEstimatesViaEthFeeHistory,
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`,
fetchEthGasPriceEstimate,
Expand Down Expand Up @@ -398,7 +396,6 @@ describe('GasFeeController', () => {
isLegacyGasAPICompatible: true,
fetchGasEstimates,
fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`,
fetchGasEstimatesViaEthFeeHistory,
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`,
fetchEthGasPriceEstimate,
Expand Down Expand Up @@ -736,7 +733,6 @@ describe('GasFeeController', () => {
isLegacyGasAPICompatible: true,
fetchGasEstimates,
fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`,
fetchGasEstimatesViaEthFeeHistory,
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`,
fetchEthGasPriceEstimate,
Expand Down Expand Up @@ -881,7 +877,6 @@ describe('GasFeeController', () => {
isLegacyGasAPICompatible: false,
fetchGasEstimates,
fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`,
fetchGasEstimatesViaEthFeeHistory,
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`,
fetchEthGasPriceEstimate,
Expand Down Expand Up @@ -977,7 +972,6 @@ describe('GasFeeController', () => {
fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal(
ChainId.goerli,
)}/suggestedGasFees`,
fetchGasEstimatesViaEthFeeHistory,
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal(
ChainId.goerli,
Expand Down
2 changes: 0 additions & 2 deletions packages/gas-fee-controller/src/GasFeeController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import type { Hex } from '@metamask/utils';
import { v1 as random } from 'uuid';

import determineGasFeeCalculations from './determineGasFeeCalculations';
import fetchGasEstimatesViaEthFeeHistory from './fetchGasEstimatesViaEthFeeHistory';
import {
calculateTimeEstimate,
fetchGasEstimates,
Expand Down Expand Up @@ -466,7 +465,6 @@ export class GasFeeController extends StaticIntervalPollingController<
'<chain_id>',
`${decimalChainId}`,
),
fetchGasEstimatesViaEthFeeHistory,
fetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: this.legacyAPIEndpoint.replace(
'<chain_id>',
Expand Down
159 changes: 31 additions & 128 deletions packages/gas-fee-controller/src/determineGasFeeCalculations.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import determineGasFeeCalculations from './determineGasFeeCalculations';
import fetchGasEstimatesViaEthFeeHistory from './fetchGasEstimatesViaEthFeeHistory';
import {
fetchGasEstimates,
fetchLegacyGasPriceEstimates,
Expand All @@ -15,7 +14,6 @@ import type {
} from './GasFeeController';

jest.mock('./gas-util');
jest.mock('./fetchGasEstimatesViaEthFeeHistory');

const mockedFetchGasEstimates = fetchGasEstimates as jest.Mock<
ReturnType<typeof fetchGasEstimates>,
Expand All @@ -34,11 +32,6 @@ const mockedCalculateTimeEstimate = calculateTimeEstimate as jest.Mock<
ReturnType<typeof calculateTimeEstimate>,
Parameters<typeof calculateTimeEstimate>
>;
const mockedFetchGasEstimatesViaEthFeeHistory =
fetchGasEstimatesViaEthFeeHistory as jest.Mock<
ReturnType<typeof fetchGasEstimatesViaEthFeeHistory>,
Parameters<typeof fetchGasEstimatesViaEthFeeHistory>
>;

const INFURA_API_KEY_MOCK = 'test';

Expand Down Expand Up @@ -126,7 +119,6 @@ describe('determineGasFeeCalculations', () => {
isEIP1559Compatible: false,
isLegacyGasAPICompatible: false,
fetchGasEstimates: mockedFetchGasEstimates,
fetchGasEstimatesViaEthFeeHistory: mockedFetchGasEstimatesViaEthFeeHistory,
fetchGasEstimatesUrl: 'http://doesnt-matter',
fetchLegacyGasPriceEstimates: mockedFetchLegacyGasPriceEstimates,
fetchLegacyGasPriceEstimatesUrl: 'http://doesnt-matter',
Expand Down Expand Up @@ -166,17 +158,10 @@ describe('determineGasFeeCalculations', () => {
});

describe('when nonRPCGasFeeApisDisabled is true', () => {
describe('assuming fetchGasEstimatesViaEthFeeHistory does not throw an error', () => {
it('returns a combination of the fetched fee and time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchGasEstimates();
mockedFetchGasEstimatesViaEthFeeHistory.mockResolvedValue(
gasFeeEstimates,
);
const estimatedGasFeeTimeBounds =
buildMockDataForCalculateTimeEstimate();
mockedCalculateTimeEstimate.mockReturnValue(
estimatedGasFeeTimeBounds,
);
describe('assuming fetchEthGasPriceEstimate does not throw an error', () => {
it('returns the fetched fee estimates and an empty set of time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchEthGasPriceEstimate();
mockedFetchEthGasPriceEstimate.mockResolvedValue(gasFeeEstimates);

const gasFeeCalculations = await determineGasFeeCalculations({
...options,
Expand All @@ -187,37 +172,26 @@ describe('determineGasFeeCalculations', () => {

expect(gasFeeCalculations).toStrictEqual({
gasFeeEstimates,
estimatedGasFeeTimeBounds,
gasEstimateType: 'fee-market',
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'eth_gasPrice',
});
});
});

describe('when fetchGasEstimatesViaEthFeeHistory throws an error', () => {
beforeEach(() => {
mockedFetchGasEstimatesViaEthFeeHistory.mockImplementation(() => {
throw new Error('Some API failure');
describe('when fetchEthGasPriceEstimate throws an error', () => {
it('throws an error that wraps that error', async () => {
mockedFetchEthGasPriceEstimate.mockImplementation(() => {
throw new Error('fetchEthGasPriceEstimate failed');
});
});

describe('assuming fetchEthGasPriceEstimate does not throw an error', () => {
it('returns the fetched fee estimates and an empty set of time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchEthGasPriceEstimate();
mockedFetchEthGasPriceEstimate.mockResolvedValue(gasFeeEstimates);

const gasFeeCalculations = await determineGasFeeCalculations({
...options,
nonRPCGasFeeApisDisabled: true,
});

expect(mockedFetchGasEstimates).toHaveBeenCalledTimes(0);

expect(gasFeeCalculations).toStrictEqual({
gasFeeEstimates,
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'eth_gasPrice',
});
const promise = determineGasFeeCalculations({
...options,
nonRPCGasFeeApisDisabled: true,
});

await expect(promise).rejects.toThrow(
'Gas fee/price estimation failed. Message: fetchEthGasPriceEstimate failed',
);
});
});
});
Expand All @@ -229,103 +203,32 @@ describe('determineGasFeeCalculations', () => {
});
});

describe('assuming neither fetchGasEstimatesViaEthFeeHistory nor calculateTimeEstimate throws errors', () => {
it('returns a combination of the fetched fee and time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchGasEstimates();
mockedFetchGasEstimatesViaEthFeeHistory.mockResolvedValue(
gasFeeEstimates,
);
const estimatedGasFeeTimeBounds =
buildMockDataForCalculateTimeEstimate();
mockedCalculateTimeEstimate.mockReturnValue(
estimatedGasFeeTimeBounds,
);
describe('assuming fetchEthGasPriceEstimate does not throw an error', () => {
it('returns the fetched fee estimates and an empty set of time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchEthGasPriceEstimate();
mockedFetchEthGasPriceEstimate.mockResolvedValue(gasFeeEstimates);

const gasFeeCalculations = await determineGasFeeCalculations(options);

expect(gasFeeCalculations).toStrictEqual({
gasFeeEstimates,
estimatedGasFeeTimeBounds,
gasEstimateType: 'fee-market',
});
});
});

describe('when fetchGasEstimatesViaEthFeeHistory throws an error', () => {
beforeEach(() => {
mockedFetchGasEstimatesViaEthFeeHistory.mockImplementation(() => {
throw new Error('Some API failure');
});
});

describe('assuming fetchEthGasPriceEstimate does not throw an error', () => {
it('returns the fetched fee estimates and an empty set of time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchEthGasPriceEstimate();
mockedFetchEthGasPriceEstimate.mockResolvedValue(gasFeeEstimates);

const gasFeeCalculations = await determineGasFeeCalculations(
options,
);

expect(gasFeeCalculations).toStrictEqual({
gasFeeEstimates,
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'eth_gasPrice',
});
});
});

describe('when fetchEthGasPriceEstimate throws an error', () => {
it('throws an error that wraps that error', async () => {
mockedFetchEthGasPriceEstimate.mockImplementation(() => {
throw new Error('fetchEthGasPriceEstimate failed');
});

const promise = determineGasFeeCalculations(options);

await expect(promise).rejects.toThrow(
'Gas fee/price estimation failed. Message: fetchEthGasPriceEstimate failed',
);
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'eth_gasPrice',
});
});
});

describe('when fetchGasEstimatesViaEthFeeHistory does not throw an error, but calculateTimeEstimate throws an error', () => {
beforeEach(() => {
mockedCalculateTimeEstimate.mockImplementation(() => {
throw new Error('Some API failure');
});
});

describe('assuming fetchEthGasPriceEstimate does not throw an error', () => {
it('returns the fetched fee estimates and an empty set of time estimates', async () => {
const gasFeeEstimates = buildMockDataForFetchEthGasPriceEstimate();
mockedFetchEthGasPriceEstimate.mockResolvedValue(gasFeeEstimates);

const gasFeeCalculations = await determineGasFeeCalculations(
options,
);

expect(gasFeeCalculations).toStrictEqual({
gasFeeEstimates,
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'eth_gasPrice',
});
describe('when fetchEthGasPriceEstimate throws an error', () => {
it('throws an error that wraps that error', async () => {
mockedFetchEthGasPriceEstimate.mockImplementation(() => {
throw new Error('fetchEthGasPriceEstimate failed');
});
});

describe('when fetchEthGasPriceEstimate throws an error', () => {
it('throws an error that wraps that error', async () => {
mockedFetchEthGasPriceEstimate.mockImplementation(() => {
throw new Error('fetchEthGasPriceEstimate failed');
});

const promise = determineGasFeeCalculations(options);
const promise = determineGasFeeCalculations(options);

await expect(promise).rejects.toThrow(
'Gas fee/price estimation failed. Message: fetchEthGasPriceEstimate failed',
);
});
await expect(promise).rejects.toThrow(
'Gas fee/price estimation failed. Message: fetchEthGasPriceEstimate failed',
);
});
});
});
Expand Down
Loading

0 comments on commit 9349754

Please sign in to comment.