From 60d9b9a1a77c65199b2df8b05d11d41bcbb73cf3 Mon Sep 17 00:00:00 2001 From: Derek Brans Date: Wed, 26 Jun 2024 08:48:55 -0400 Subject: [PATCH 1/3] put back default options removed by 2b1841c --- .../src/GasFeeController.test.ts | 260 ++++-------------- 1 file changed, 53 insertions(+), 207 deletions(-) diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index 84d25a03dc8..e4d68dd1f4d 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -734,6 +734,12 @@ describe('GasFeeController', () => { describe('fetchGasFeeEstimates', () => { describe('when on any network supporting legacy gas estimation api', () => { + const getDefaultOptions = () => ({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), + }); const mockDetermineGasFeeCalculations = buildMockGasFeeStateLegacy(); beforeEach(() => { @@ -744,10 +750,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), + ...getDefaultOptions(), networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -788,12 +791,7 @@ describe('GasFeeController', () => { }); it('should update the state with a fetched set of estimates', async () => { - await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), - }); + await setupGasFeeController(getDefaultOptions()); await gasFeeController.fetchGasFeeEstimates(); @@ -803,12 +801,7 @@ describe('GasFeeController', () => { }); it('should return the same data that it puts into state', async () => { - await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), - }); + await setupGasFeeController(getDefaultOptions()); const estimateData = await gasFeeController.fetchGasFeeEstimates(); @@ -817,10 +810,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a number input', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue(1), }); @@ -835,10 +825,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a hexstring input', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue('0x1'), }); @@ -853,10 +840,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when nonRPCGasFeeApisDisabled is true', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), + ...getDefaultOptions(), state: { ...buildMockGasFeeStateEthGasPrice(), nonRPCGasFeeApisDisabled: true, @@ -874,10 +858,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when nonRPCGasFeeApisDisabled is false', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), + ...getDefaultOptions(), state: { ...buildMockGasFeeStateEthGasPrice(), nonRPCGasFeeApisDisabled: false, @@ -895,10 +876,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a numeric string input', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue('1'), }); @@ -913,6 +891,9 @@ describe('GasFeeController', () => { }); describe('when on any network supporting EIP-1559', () => { + const getDefaultOptions = () => ({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + }); const mockDetermineGasFeeCalculations = buildMockGasFeeStateFeeMarket(); beforeEach(() => { @@ -923,7 +904,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + ...getDefaultOptions(), networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -964,9 +945,7 @@ describe('GasFeeController', () => { }); it('should update the state with a fetched set of estimates', async () => { - await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - }); + await setupGasFeeController(getDefaultOptions()); await gasFeeController.fetchGasFeeEstimates(); @@ -976,9 +955,7 @@ describe('GasFeeController', () => { }); it('should return the same data that it puts into state', async () => { - await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - }); + await setupGasFeeController(getDefaultOptions()); const estimateData = await gasFeeController.fetchGasFeeEstimates(); @@ -987,7 +964,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations with a URL that contains the chain ID', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue('0x1'), }); @@ -1001,6 +978,31 @@ describe('GasFeeController', () => { }); }); describe('when passed a networkClientId in options object', () => { + const getDefaultOptions = () => ({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, + }); const mockDetermineGasFeeCalculations = buildMockGasFeeStateFeeMarket(); beforeEach(() => { @@ -1011,29 +1013,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, + ...getDefaultOptions(), clientId: '99999', }); @@ -1068,29 +1048,7 @@ describe('GasFeeController', () => { describe("the chainId of the networkClientId matches the globally selected network's chainId", () => { it('should update the globally selected network state with a fetched set of estimates', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue(ChainId.goerli), onNetworkDidChange: jest.fn(), }); @@ -1106,29 +1064,7 @@ describe('GasFeeController', () => { it('should update the gasFeeEstimatesByChainId state with a fetched set of estimates', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue(ChainId.goerli), onNetworkDidChange: jest.fn(), }); @@ -1146,29 +1082,7 @@ describe('GasFeeController', () => { describe("the chainId of the networkClientId does not match the globally selected network's chainId", () => { it('should not update the globally selected network state with a fetched set of estimates', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue(ChainId.mainnet), onNetworkDidChange: jest.fn(), }); @@ -1186,29 +1100,7 @@ describe('GasFeeController', () => { it('should update the gasFeeEstimatesByChainId state with a fetched set of estimates', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, + ...getDefaultOptions(), getChainId: jest.fn().mockReturnValue(ChainId.mainnet), onNetworkDidChange: jest.fn(), }); @@ -1224,31 +1116,7 @@ describe('GasFeeController', () => { }); it('should return the same data that it puts into state', async () => { - await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, - }); + await setupGasFeeController(getDefaultOptions()); const estimateData = await gasFeeController.fetchGasFeeEstimates({ networkClientId: 'sepolia', @@ -1259,29 +1127,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations with a URL that contains the chain ID', async () => { await setupGasFeeController({ - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, + ...getDefaultOptions(), }); await gasFeeController.fetchGasFeeEstimates({ From 284ae063b7e56062f066563aa9da2a319170c421 Mon Sep 17 00:00:00 2001 From: Derek Brans Date: Mon, 10 Jun 2024 19:57:05 -0400 Subject: [PATCH 2/3] Revert "fix: use hardcoded Infura gas API urls (#4068)" This reverts commit 850461d323fc304baaffa94bfa5bee196761ddb2. merge conflicts: - determineGasFeeCalculations.ts - GasFeeController.ts - GasFeeController.test.ts --- packages/gas-fee-controller/package.json | 1 + .../src/GasFeeController.test.ts | 91 +++++++------ .../src/GasFeeController.ts | 23 ++-- .../src/determineGasFeeCalculations.test.ts | 3 - .../src/determineGasFeeCalculations.ts | 13 +- .../gas-fee-controller/src/gas-util.test.ts | 126 ++++++------------ packages/gas-fee-controller/src/gas-util.ts | 45 ++----- yarn.lock | 1 + 8 files changed, 117 insertions(+), 186 deletions(-) diff --git a/packages/gas-fee-controller/package.json b/packages/gas-fee-controller/package.json index 61c1093a9dc..d72cc271f72 100644 --- a/packages/gas-fee-controller/package.json +++ b/packages/gas-fee-controller/package.json @@ -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", diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index e4d68dd1f4d..c4a29de35ba 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -23,11 +23,7 @@ import { fetchEthGasPriceEstimate, calculateTimeEstimate, } from './gas-util'; -import { - GAS_API_BASE_URL, - GAS_ESTIMATE_TYPES, - GasFeeController, -} from './GasFeeController'; +import { GAS_ESTIMATE_TYPES, GasFeeController } from './GasFeeController'; import type { GasFeeState, GasFeeStateChange, @@ -228,12 +224,13 @@ describe('GasFeeController', () => { * GasFeeController. * @param options.getCurrentNetworkLegacyGasAPICompatibility - Sets * getCurrentNetworkLegacyGasAPICompatibility on the GasFeeController. + * @param options.legacyAPIEndpoint - Sets legacyAPIEndpoint on the GasFeeController. + * @param options.EIP1559APIEndpoint - Sets EIP1559APIEndpoint on the GasFeeController. * @param options.clientId - Sets clientId on the GasFeeController. * @param options.networkControllerState - State object to initialize * NetworkController with. * @param options.interval - The polling interval. * @param options.state - The initial GasFeeController state - * @param options.infuraAPIKey - The Infura API key. * @param options.initializeNetworkProvider - Whether to instruct the * NetworkController to initialize its provider. */ @@ -242,7 +239,8 @@ describe('GasFeeController', () => { getCurrentNetworkLegacyGasAPICompatibility = jest .fn() .mockReturnValue(false), - infuraAPIKey = 'INFURA_API_KEY', + legacyAPIEndpoint = 'http://legacy.endpoint/', + EIP1559APIEndpoint = 'http://eip-1559.endpoint/', clientId, getChainId, onNetworkDidChange, @@ -255,11 +253,13 @@ describe('GasFeeController', () => { onNetworkDidChange?: jest.Mock; getIsEIP1559Compatible?: jest.Mock>; getCurrentNetworkLegacyGasAPICompatibility?: jest.Mock; + legacyAPIEndpoint?: string; + // eslint-disable-next-line @typescript-eslint/naming-convention + EIP1559APIEndpoint?: string; clientId?: string; networkControllerState?: Partial; state?: GasFeeState; interval?: number; - infuraAPIKey?: string; initializeNetworkProvider?: boolean; } = {}) { const controllerMessenger = getControllerMessenger(); @@ -277,10 +277,11 @@ describe('GasFeeController', () => { messenger, getCurrentNetworkLegacyGasAPICompatibility, getCurrentNetworkEIP1559Compatibility: getIsEIP1559Compatible, // change this for networkDetails.state.networkDetails.isEIP1559Compatible ??? + legacyAPIEndpoint, + EIP1559APIEndpoint, state, clientId, interval, - infuraAPIKey, }); } @@ -334,6 +335,8 @@ describe('GasFeeController', () => { getCurrentNetworkLegacyGasAPICompatibility: jest .fn() .mockReturnValue(true), + legacyAPIEndpoint: 'https://some-legacy-endpoint/', + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -361,14 +364,14 @@ describe('GasFeeController', () => { isEIP1559Compatible: false, isLegacyGasAPICompatible: true, fetchGasEstimates, - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`, + fetchGasEstimatesUrl: 'https://some-eip-1559-endpoint/1337', fetchLegacyGasPriceEstimates, - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: + 'https://some-legacy-endpoint/1337', fetchEthGasPriceEstimate, calculateTimeEstimate, clientId: '99999', ethQuery: expect.any(EthQuery), - infuraAPIKey: expect.any(String), nonRPCGasFeeApisDisabled: false, }); }); @@ -398,6 +401,8 @@ describe('GasFeeController', () => { getCurrentNetworkLegacyGasAPICompatibility: jest .fn() .mockReturnValue(true), + legacyAPIEndpoint: 'https://some-legacy-endpoint/', + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -427,14 +432,14 @@ describe('GasFeeController', () => { isEIP1559Compatible: false, isLegacyGasAPICompatible: true, fetchGasEstimates, - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`, + fetchGasEstimatesUrl: 'https://some-eip-1559-endpoint/1337', fetchLegacyGasPriceEstimates, - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: + 'https://some-legacy-endpoint/1337', fetchEthGasPriceEstimate, calculateTimeEstimate, clientId: '99999', ethQuery: expect.any(EthQuery), - infuraAPIKey: expect.any(String), nonRPCGasFeeApisDisabled: false, }); }); @@ -751,6 +756,8 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ ...getDefaultOptions(), + legacyAPIEndpoint: 'https://some-legacy-endpoint/', + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -778,14 +785,13 @@ describe('GasFeeController', () => { isEIP1559Compatible: false, isLegacyGasAPICompatible: true, fetchGasEstimates, - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`, + fetchGasEstimatesUrl: 'https://some-eip-1559-endpoint/1337', fetchLegacyGasPriceEstimates, - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: 'https://some-legacy-endpoint/1337', fetchEthGasPriceEstimate, calculateTimeEstimate, clientId: '99999', ethQuery: expect.any(EthQuery), - infuraAPIKey: expect.any(String), nonRPCGasFeeApisDisabled: false, }); }); @@ -811,6 +817,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a number input', async () => { await setupGasFeeController({ ...getDefaultOptions(), + legacyAPIEndpoint: 'http://legacy.endpoint/', getChainId: jest.fn().mockReturnValue(1), }); @@ -818,7 +825,7 @@ describe('GasFeeController', () => { expect(mockedDetermineGasFeeCalculations).toHaveBeenCalledWith( expect.objectContaining({ - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: 'http://legacy.endpoint/1', }), ); }); @@ -826,6 +833,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a hexstring input', async () => { await setupGasFeeController({ ...getDefaultOptions(), + legacyAPIEndpoint: 'http://legacy.endpoint/', getChainId: jest.fn().mockReturnValue('0x1'), }); @@ -833,7 +841,7 @@ describe('GasFeeController', () => { expect(mockedDetermineGasFeeCalculations).toHaveBeenCalledWith( expect.objectContaining({ - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: 'http://legacy.endpoint/1', }), ); }); @@ -877,6 +885,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a numeric string input', async () => { await setupGasFeeController({ ...getDefaultOptions(), + legacyAPIEndpoint: 'http://legacy.endpoint/', getChainId: jest.fn().mockReturnValue('1'), }); @@ -884,7 +893,7 @@ describe('GasFeeController', () => { expect(mockedDetermineGasFeeCalculations).toHaveBeenCalledWith( expect.objectContaining({ - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: 'http://legacy.endpoint/1', }), ); }); @@ -905,6 +914,8 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ ...getDefaultOptions(), + legacyAPIEndpoint: 'https://some-legacy-endpoint/', + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -932,14 +943,13 @@ describe('GasFeeController', () => { isEIP1559Compatible: true, isLegacyGasAPICompatible: false, fetchGasEstimates, - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/suggestedGasFees`, + fetchGasEstimatesUrl: 'https://some-eip-1559-endpoint/1337', fetchLegacyGasPriceEstimates, - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/1337/gasPrices`, + fetchLegacyGasPriceEstimatesUrl: 'https://some-legacy-endpoint/1337', fetchEthGasPriceEstimate, calculateTimeEstimate, clientId: '99999', ethQuery: expect.any(EthQuery), - infuraAPIKey: expect.any(String), nonRPCGasFeeApisDisabled: false, }); }); @@ -965,6 +975,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations with a URL that contains the chain ID', async () => { await setupGasFeeController({ ...getDefaultOptions(), + EIP1559APIEndpoint: 'http://eip-1559.endpoint/', getChainId: jest.fn().mockReturnValue('0x1'), }); @@ -972,7 +983,7 @@ describe('GasFeeController', () => { expect(mockedDetermineGasFeeCalculations).toHaveBeenCalledWith( expect.objectContaining({ - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/1/suggestedGasFees`, + fetchGasEstimatesUrl: 'http://eip-1559.endpoint/1', }), ); }); @@ -1014,6 +1025,8 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ ...getDefaultOptions(), + legacyAPIEndpoint: 'https://some-legacy-endpoint/', + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', clientId: '99999', }); @@ -1027,20 +1040,17 @@ describe('GasFeeController', () => { fetchGasEstimates, // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal( - ChainId.goerli, - )}/suggestedGasFees`, + fetchGasEstimatesUrl: 'https://some-eip-1559-endpoint/5', fetchLegacyGasPriceEstimates, // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - fetchLegacyGasPriceEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal( + fetchLegacyGasPriceEstimatesUrl: `https://some-legacy-endpoint/${convertHexToDecimal( ChainId.goerli, - )}/gasPrices`, + )}`, fetchEthGasPriceEstimate, calculateTimeEstimate, clientId: '99999', ethQuery: expect.any(EthQuery), - infuraAPIKey: expect.any(String), nonRPCGasFeeApisDisabled: false, }); }); @@ -1128,6 +1138,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations with a URL that contains the chain ID', async () => { await setupGasFeeController({ ...getDefaultOptions(), + EIP1559APIEndpoint: 'http://eip-1559.endpoint/', }); await gasFeeController.fetchGasFeeEstimates({ @@ -1138,9 +1149,9 @@ describe('GasFeeController', () => { expect.objectContaining({ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal( + fetchGasEstimatesUrl: `http://eip-1559.endpoint/${convertHexToDecimal( ChainId.sepolia, - )}/suggestedGasFees`, + )}`, }), ); }); @@ -1155,6 +1166,8 @@ describe('GasFeeController', () => { getCurrentNetworkLegacyGasAPICompatibility: jest .fn() .mockReturnValue(true), + legacyAPIEndpoint: 'https://some-legacy-endpoint/', + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', networkControllerState: { networksMetadata: { goerli: { @@ -1182,9 +1195,9 @@ describe('GasFeeController', () => { expect.objectContaining({ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal( + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( ChainId.goerli, - )}/suggestedGasFees`, + )}`, }), ); await clock.tickAsync(pollingInterval / 2); @@ -1195,9 +1208,9 @@ describe('GasFeeController', () => { expect.objectContaining({ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal( + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( ChainId.goerli, - )}/suggestedGasFees`, + )}`, }), ); expect( @@ -1210,9 +1223,9 @@ describe('GasFeeController', () => { expect.objectContaining({ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - fetchGasEstimatesUrl: `${GAS_API_BASE_URL}/networks/${convertHexToDecimal( + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( ChainId.sepolia, - )}/suggestedGasFees`, + )}`, }), ); }); diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index f6bc4e0a1c5..9e7b30515ef 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -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`; // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention @@ -282,8 +282,6 @@ export class GasFeeController extends StaticIntervalPollingController< private readonly getCurrentAccountEIP1559Compatibility; - private readonly infuraAPIKey: string; - private currentChainId; private ethQuery?: EthQuery; @@ -309,9 +307,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, @@ -323,8 +323,9 @@ export class GasFeeController extends StaticIntervalPollingController< getCurrentNetworkLegacyGasAPICompatibility, getProvider, onNetworkDidChange, + legacyAPIEndpoint = LEGACY_GAS_PRICES_API_URL, + EIP1559APIEndpoint, clientId, - infuraAPIKey, }: { interval?: number; messenger: GasFeeMessenger; @@ -335,8 +336,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, @@ -354,10 +357,9 @@ export class GasFeeController extends StaticIntervalPollingController< this.getCurrentAccountEIP1559Compatibility = getCurrentAccountEIP1559Compatibility; this.#getProvider = getProvider; - this.EIP1559APIEndpoint = `${GAS_API_BASE_URL}/networks//suggestedGasFees`; - this.legacyAPIEndpoint = `${GAS_API_BASE_URL}/networks//gasPrices`; + this.EIP1559APIEndpoint = EIP1559APIEndpoint; + this.legacyAPIEndpoint = legacyAPIEndpoint; this.clientId = clientId; - this.infuraAPIKey = infuraAPIKey; this.ethQuery = new EthQuery(this.#getProvider()); @@ -487,7 +489,6 @@ export class GasFeeController extends StaticIntervalPollingController< calculateTimeEstimate, clientId: this.clientId, ethQuery, - infuraAPIKey: this.infuraAPIKey, nonRPCGasFeeApisDisabled: this.state.nonRPCGasFeeApisDisabled, }); diff --git a/packages/gas-fee-controller/src/determineGasFeeCalculations.test.ts b/packages/gas-fee-controller/src/determineGasFeeCalculations.test.ts index aee4f05b8a7..a8df42bc14c 100644 --- a/packages/gas-fee-controller/src/determineGasFeeCalculations.test.ts +++ b/packages/gas-fee-controller/src/determineGasFeeCalculations.test.ts @@ -33,8 +33,6 @@ const mockedCalculateTimeEstimate = calculateTimeEstimate as jest.Mock< Parameters >; -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 @@ -126,7 +124,6 @@ describe('determineGasFeeCalculations', () => { calculateTimeEstimate: mockedCalculateTimeEstimate, clientId: 'some-client-id', ethQuery: {}, - infuraAPIKey: INFURA_API_KEY_MOCK, }; describe('when isEIP1559Compatible is true', () => { diff --git a/packages/gas-fee-controller/src/determineGasFeeCalculations.ts b/packages/gas-fee-controller/src/determineGasFeeCalculations.ts index 72697cc4f54..732540a7276 100644 --- a/packages/gas-fee-controller/src/determineGasFeeCalculations.ts +++ b/packages/gas-fee-controller/src/determineGasFeeCalculations.ts @@ -12,13 +12,11 @@ type DetermineGasFeeCalculationsRequest = { isLegacyGasAPICompatible: boolean; fetchGasEstimates: ( url: string, - infuraAPIKey: string, clientId?: string, ) => Promise; fetchGasEstimatesUrl: string; fetchLegacyGasPriceEstimates: ( url: string, - infuraAPIKey: string, clientId?: string, ) => Promise; fetchLegacyGasPriceEstimatesUrl: string; @@ -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; }; @@ -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. */ @@ -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; @@ -158,13 +149,11 @@ async function getEstimatesUsingLegacyEndpoint( const { fetchLegacyGasPriceEstimates, fetchLegacyGasPriceEstimatesUrl, - infuraAPIKey, clientId, } = request; const estimates = await fetchLegacyGasPriceEstimates( fetchLegacyGasPriceEstimatesUrl, - infuraAPIKey, clientId, ); diff --git a/packages/gas-fee-controller/src/gas-util.test.ts b/packages/gas-fee-controller/src/gas-util.test.ts index d6829e7fc78..baea4369289 100644 --- a/packages/gas-fee-controller/src/gas-util.test.ts +++ b/packages/gas-fee-controller/src/gas-util.test.ts @@ -1,4 +1,4 @@ -import { handleFetch } from '@metamask/controller-utils'; +import nock from 'nock'; import { fetchLegacyGasPriceEstimates, @@ -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: { @@ -73,45 +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', () => { 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 () => { @@ -137,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(); }); }); diff --git a/packages/gas-fee-controller/src/gas-util.ts b/packages/gas-fee-controller/src/gas-util.ts index 8a7c863f2f1..17a242ac8be 100644 --- a/packages/gas-fee-controller/src/gas-util.ts +++ b/packages/gas-fee-controller/src/gas-util.ts @@ -33,19 +33,17 @@ export function normalizeGWEIDecimalNumbers(n: string | number) { * Fetch gas estimates from the given URL. * * @param url - The gas estimate URL. - * @param infuraAPIKey - The Infura API key used for infura API requests. * @param clientId - The client ID used to identify to the API who is asking for estimates. * @returns The gas estimates. */ export async function fetchGasEstimates( url: string, - infuraAPIKey: string, clientId?: string, ): Promise { - const infuraAuthToken = buildInfuraAuthToken(infuraAPIKey); - const estimates = await handleFetch(url, { - headers: getHeaders(infuraAuthToken, clientId), - }); + const estimates = await handleFetch( + url, + clientId ? { headers: makeClientIdHeader(clientId) } : undefined, + ); return { low: { ...estimates.low, @@ -89,22 +87,22 @@ export async function fetchGasEstimates( * high values from that API. * * @param url - The URL to fetch gas price estimates from. - * @param infuraAPIKey - The Infura API key used for infura API requests. * @param clientId - The client ID used to identify to the API who is asking for estimates. * @returns The gas price estimates. */ export async function fetchLegacyGasPriceEstimates( url: string, - infuraAPIKey: string, clientId?: string, ): Promise { - const infuraAuthToken = buildInfuraAuthToken(infuraAPIKey); const result = await handleFetch(url, { referrer: url, referrerPolicy: 'no-referrer-when-downgrade', method: 'GET', mode: 'cors', - headers: getHeaders(infuraAuthToken, clientId), + headers: { + 'Content-Type': 'application/json', + ...(clientId && makeClientIdHeader(clientId)), + }, }); return { low: result.SafeGasPrice, @@ -193,30 +191,3 @@ export function calculateTimeEstimate( upperTimeBound, }; } - -/** - * Build an infura auth token from the given API key and secret. - * - * @param infuraAPIKey - The Infura API key. - * @returns The base64 encoded auth token. - */ -function buildInfuraAuthToken(infuraAPIKey: string) { - // We intentionally leave the password empty, as Infura does not require one - return Buffer.from(`${infuraAPIKey}:`).toString('base64'); -} - -/** - * Get the headers for a request to the gas fee API. - * - * @param infuraAuthToken - The Infura auth token to use for the request. - * @param clientId - The client ID used to identify to the API who is asking for estimates. - * @returns The headers for the request. - */ -function getHeaders(infuraAuthToken: string, clientId?: string) { - return { - 'Content-Type': 'application/json', - Authorization: `Basic ${infuraAuthToken}`, - // Only add the clientId header if clientId is a non-empty string - ...(clientId?.trim() ? makeClientIdHeader(clientId) : {}), - }; -} diff --git a/yarn.lock b/yarn.lock index 7d3f5cedb5a..90734457147 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2960,6 +2960,7 @@ __metadata: 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 From e5857dc96e286b40c0876e8e34f69d9103749932 Mon Sep 17 00:00:00 2001 From: Derek Brans Date: Tue, 11 Jun 2024 08:45:28 -0400 Subject: [PATCH 3/3] update jest config --- packages/gas-fee-controller/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gas-fee-controller/jest.config.js b/packages/gas-fee-controller/jest.config.js index b165bc85627..14cf684c6a7 100644 --- a/packages/gas-fee-controller/jest.config.js +++ b/packages/gas-fee-controller/jest.config.js @@ -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, },