From 9f1e3181e678edfa4bb84597757132594c6b4864 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 21 Jul 2026 13:08:28 +0300 Subject: [PATCH 1/6] fix(gas-fee-controller): make GasFeeController initialization-order-agnostic Defer the constructor's NetworkController and provider reads until the first gas fee fetch. The constructor no longer eagerly calls NetworkController:getState, NetworkController:getNetworkClientById, the getChainId option, or the getProvider option; the provider (EthQuery) and current chain ID are now resolved lazily on first use and memoized, kept fresh by the networkDidChange handler. This lets the controller be constructed before NetworkController is ready, removing the need for dependency ordering when wiring it into the shared @metamask/wallet init set. The constructor signature and gas fee fetching behavior are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/gas-fee-controller/CHANGELOG.md | 2 + .../src/GasFeeController.test.ts | 189 ++++++++++++++++++ .../src/GasFeeController.ts | 45 +++-- 3 files changed, 221 insertions(+), 15 deletions(-) diff --git a/packages/gas-fee-controller/CHANGELOG.md b/packages/gas-fee-controller/CHANGELOG.md index 293f717bbaf..9505844fe3c 100644 --- a/packages/gas-fee-controller/CHANGELOG.md +++ b/packages/gas-fee-controller/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Defer the `GasFeeController` constructor's `NetworkController` and provider reads until the first gas fee fetch ([#9563](https://github.com/MetaMask/core/pull/9563)) + - The constructor no longer eagerly calls `NetworkController:getState`, `NetworkController:getNetworkClientById`, the `getChainId` option, or the `getProvider` option; the provider (`EthQuery`) and the current chain ID are now resolved lazily on first use. This makes the controller initialization-order-agnostic, so it can be constructed before `NetworkController` is ready. The constructor signature and gas fee fetching behavior are unchanged. - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) ## [26.2.4] diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index 594330af428..09b1eefcedf 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -19,6 +19,7 @@ import type { import type { Hex } from '@metamask/utils'; import nock from 'nock'; +import { flushPromises } from '../../../tests/helpers'; import { buildCustomNetworkConfiguration, buildCustomRpcEndpoint, @@ -366,6 +367,110 @@ describe('GasFeeController', () => { it('should set the name of the controller to GasFeeController', () => { expect(gasFeeController.name).toBe(name); }); + + describe('initialization-order independence', () => { + /** + * Builds a messenger whose NetworkController action handlers throw if + * called, so that a test can assert the constructor never reads from the + * NetworkController. + * + * @returns The messenger along with spies for its handlers. + */ + const getMessengerWithThrowingNetworkHandlers = (): { + messenger: ReturnType; + getState: jest.Mock; + getNetworkClientById: jest.Mock; + } => { + const rootMessenger = getRootMessenger(); + const getState = jest.fn(() => { + throw new Error('NetworkController:getState should not be called'); + }); + const getNetworkClientById = jest.fn(() => { + throw new Error( + 'NetworkController:getNetworkClientById should not be called', + ); + }); + rootMessenger.registerActionHandler( + 'NetworkController:getState', + getState, + ); + rootMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + getNetworkClientById, + ); + return { + messenger: getGasFeeControllerMessenger(rootMessenger), + getState, + getNetworkClientById, + }; + }; + + it('does not read the chain ID or provider from the network when constructed without getChainId/onNetworkDidChange', () => { + const { messenger, getState, getNetworkClientById } = + getMessengerWithThrowingNetworkHandlers(); + const getProvider = jest.fn(() => { + throw new Error('getProvider should not be called'); + }); + + let controller: GasFeeController | undefined; + expect(() => { + controller = new GasFeeController({ + messenger, + getProvider, + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(false), + getCurrentNetworkEIP1559Compatibility: jest + .fn() + .mockResolvedValue(true), + EIP1559APIEndpoint: 'http://eip-1559.endpoint/', + }); + }).not.toThrow(); + + expect(getState).not.toHaveBeenCalled(); + expect(getNetworkClientById).not.toHaveBeenCalled(); + expect(getProvider).not.toHaveBeenCalled(); + + controller?.destroy(); + }); + + it('does not read the chain ID or provider from the network when constructed with getChainId/onNetworkDidChange', () => { + const { messenger, getState, getNetworkClientById } = + getMessengerWithThrowingNetworkHandlers(); + const getProvider = jest.fn(() => { + throw new Error('getProvider should not be called'); + }); + const getChainId = jest.fn(() => { + throw new Error('getChainId should not be called'); + }); + const onNetworkDidChange = jest.fn(); + + let controller: GasFeeController | undefined; + expect(() => { + controller = new GasFeeController({ + messenger, + getProvider, + getChainId, + onNetworkDidChange, + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(false), + getCurrentNetworkEIP1559Compatibility: jest + .fn() + .mockResolvedValue(true), + EIP1559APIEndpoint: 'http://eip-1559.endpoint/', + }); + }).not.toThrow(); + + expect(getState).not.toHaveBeenCalled(); + expect(getNetworkClientById).not.toHaveBeenCalled(); + expect(getProvider).not.toHaveBeenCalled(); + expect(getChainId).not.toHaveBeenCalled(); + expect(onNetworkDidChange).toHaveBeenCalledTimes(1); + + controller?.destroy(); + }); + }); }); describe('getGasFeeEstimatesAndStartPolling', () => { @@ -1294,6 +1399,90 @@ describe('GasFeeController', () => { }); }); + describe('when the selected network changes', () => { + it('resets the eth query and updates the chain ID when notified via the onNetworkDidChange callback', async () => { + let networkDidChangeListener: + | ((networkControllerState: NetworkState) => Promise) + | undefined; + const onNetworkDidChange = jest.fn((listener) => { + networkDidChangeListener = listener; + }); + await setupGasFeeController({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', + getChainId: jest.fn().mockReturnValue(ChainId.mainnet), + onNetworkDidChange, + }); + + await gasFeeController.fetchGasFeeEstimates(); + expect(mockedDetermineGasFeeCalculations).toHaveBeenLastCalledWith( + expect.objectContaining({ + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( + ChainId.mainnet, + )}`, + }), + ); + + // Simulate the network switching to Sepolia. + await networkDidChangeListener?.({ + selectedNetworkClientId: 'sepolia', + } as NetworkState); + + await gasFeeController.fetchGasFeeEstimates(); + expect(mockedDetermineGasFeeCalculations).toHaveBeenLastCalledWith( + expect.objectContaining({ + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( + ChainId.sepolia, + )}`, + }), + ); + }); + + it('resets the eth query and updates the chain ID when notified via NetworkController:networkDidChange', async () => { + const rootMessenger = getRootMessenger(); + networkController = await setupNetworkController({ + rootMessenger, + state: {}, + initializeProvider: false, + }); + gasFeeController = new GasFeeController({ + getProvider: jest.fn(), + messenger: getGasFeeControllerMessenger(rootMessenger), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(false), + getCurrentNetworkEIP1559Compatibility: jest + .fn() + .mockResolvedValue(true), + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', + }); + + await gasFeeController.fetchGasFeeEstimates(); + expect(mockedDetermineGasFeeCalculations).toHaveBeenLastCalledWith( + expect.objectContaining({ + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( + ChainId.mainnet, + )}`, + }), + ); + + // Simulate the network switching to Sepolia. + rootMessenger.publish('NetworkController:networkDidChange', { + selectedNetworkClientId: 'sepolia', + } as NetworkState); + await flushPromises(); + + await gasFeeController.fetchGasFeeEstimates(); + expect(mockedDetermineGasFeeCalculations).toHaveBeenLastCalledWith( + expect.objectContaining({ + fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal( + ChainId.sepolia, + )}`, + }), + ); + }); + }); + describe('metadata', () => { beforeEach(async () => { await setupGasFeeController(); diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index bacebdd786f..4b4e2a5828e 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -323,7 +323,7 @@ export class GasFeeController extends StaticIntervalPollingController ProviderProxy; + readonly #getChainId?: () => Hex; + /** * Creates a GasFeeController instance. * @@ -401,28 +403,19 @@ export class GasFeeController extends StaticIntervalPollingController { await this.#onNetworkControllerDidChange(networkControllerState); }); } else { - const { selectedNetworkClientId } = this.messenger.call( - 'NetworkController:getState', - ); - this.currentChainId = this.messenger.call( - 'NetworkController:getNetworkClientById', - selectedNetworkClientId, - ).configuration.chainId; this.messenger.subscribe( 'NetworkController:networkDidChange', // TODO: Either fix this lint violation or explain why it's necessary to ignore. @@ -520,12 +513,12 @@ export class GasFeeController extends StaticIntervalPollingController { - if (this.currentChainId === chainId) { + if (this.#getCurrentChainId() === chainId) { state.gasFeeEstimates = gasFeeCalculations.gasFeeEstimates; state.estimatedGasFeeTimeBounds = gasFeeCalculations.estimatedGasFeeTimeBounds; @@ -683,13 +676,35 @@ export class GasFeeController extends StaticIntervalPollingController { state.nonRPCGasFeeApisDisabled = false; From 6f7e4a6015524ee7a7be6461060d36bf4c58f956 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 21 Jul 2026 13:10:11 +0300 Subject: [PATCH 2/6] docs(gas-fee-controller): point changelog entry at the correct PR link Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/gas-fee-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gas-fee-controller/CHANGELOG.md b/packages/gas-fee-controller/CHANGELOG.md index 9505844fe3c..311c7e40059 100644 --- a/packages/gas-fee-controller/CHANGELOG.md +++ b/packages/gas-fee-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Defer the `GasFeeController` constructor's `NetworkController` and provider reads until the first gas fee fetch ([#9563](https://github.com/MetaMask/core/pull/9563)) +- Defer the `GasFeeController` constructor's `NetworkController` and provider reads until the first gas fee fetch ([#9569](https://github.com/MetaMask/core/pull/9569)) - The constructor no longer eagerly calls `NetworkController:getState`, `NetworkController:getNetworkClientById`, the `getChainId` option, or the `getProvider` option; the provider (`EthQuery`) and the current chain ID are now resolved lazily on first use. This makes the controller initialization-order-agnostic, so it can be constructed before `NetworkController` is ready. The constructor signature and gas fee fetching behavior are unchanged. - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) From 9de7108915200907e2d32d92ff174ed81fd44cef Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 21 Jul 2026 13:12:29 +0300 Subject: [PATCH 3/6] docs(gas-fee-controller): condense changelog entry Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/gas-fee-controller/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/gas-fee-controller/CHANGELOG.md b/packages/gas-fee-controller/CHANGELOG.md index 311c7e40059..4d2f6ad9657 100644 --- a/packages/gas-fee-controller/CHANGELOG.md +++ b/packages/gas-fee-controller/CHANGELOG.md @@ -9,8 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Defer the `GasFeeController` constructor's `NetworkController` and provider reads until the first gas fee fetch ([#9569](https://github.com/MetaMask/core/pull/9569)) - - The constructor no longer eagerly calls `NetworkController:getState`, `NetworkController:getNetworkClientById`, the `getChainId` option, or the `getProvider` option; the provider (`EthQuery`) and the current chain ID are now resolved lazily on first use. This makes the controller initialization-order-agnostic, so it can be constructed before `NetworkController` is ready. The constructor signature and gas fee fetching behavior are unchanged. +- Defer the `GasFeeController` constructor's `NetworkController` and provider reads to the first gas fee fetch, making the controller initialization-order-agnostic; the constructor signature and fetching behavior are unchanged ([#9569](https://github.com/MetaMask/core/pull/9569)) - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) ## [26.2.4] From cdbca6285160a12c68dde88459b62c99552f94e2 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 21 Jul 2026 13:39:59 +0300 Subject: [PATCH 4/6] refactor(gas-fee-controller): tidy lazy chain-ID resolution and cover eth query rebuild Resolve the current chain ID into a local before entering the update() recipe so the Immer producer stays side-effect free, and extract #getChainIdForNetworkClient to dedupe the network-client chain-ID lookup shared by the network-change handler and the lazy resolver. Add a test asserting the provider is read once, the eth query is cached, then rebuilt from the provider after a network change (mutation-verified), and reuse setupGasFeeController for the messenger-event test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/GasFeeController.test.ts | 64 +++++++++++++------ .../src/GasFeeController.ts | 14 ++-- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index 09b1eefcedf..415a067f8f7 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -15,6 +15,7 @@ import { NetworkController, NetworkStatus } from '@metamask/network-controller'; import type { NetworkControllerMessenger, NetworkState, + ProviderProxy, } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import nock from 'nock'; @@ -279,6 +280,7 @@ describe('GasFeeController', () => { * * @param options - The options. * @param options.getChainId - Sets getChainId on the GasFeeController. + * @param options.getProvider - Sets getProvider on the GasFeeController. * @param options.onNetworkDidChange - A function for registering an event handler for the * @param options.getIsEIP1559Compatible - Sets getCurrentNetworkEIP1559Compatibility on the * GasFeeController. @@ -293,6 +295,7 @@ describe('GasFeeController', () => { * @param options.state - The initial GasFeeController state * @param options.initializeNetworkProvider - Whether to instruct the * NetworkController to initialize its provider. + * @returns The root messenger, so tests can publish network events to it. */ async function setupGasFeeController({ getIsEIP1559Compatible = jest.fn().mockResolvedValue(true), @@ -303,6 +306,7 @@ describe('GasFeeController', () => { EIP1559APIEndpoint = 'http://eip-1559.endpoint/', clientId, getChainId, + getProvider = jest.fn(), onNetworkDidChange, networkControllerState = {}, state, @@ -310,6 +314,7 @@ describe('GasFeeController', () => { initializeNetworkProvider = true, }: { getChainId?: jest.Mock; + getProvider?: jest.Mock; onNetworkDidChange?: jest.Mock; getIsEIP1559Compatible?: jest.Mock>; getCurrentNetworkLegacyGasAPICompatibility?: jest.Mock; @@ -329,7 +334,7 @@ describe('GasFeeController', () => { }); const restrictedMessenger = getGasFeeControllerMessenger(rootMessenger); gasFeeController = new GasFeeController({ - getProvider: jest.fn(), + getProvider, getChainId, onNetworkDidChange, messenger: restrictedMessenger, @@ -341,6 +346,7 @@ describe('GasFeeController', () => { clientId, interval, }); + return { rootMessenger }; } beforeEach(() => { @@ -1400,7 +1406,7 @@ describe('GasFeeController', () => { }); describe('when the selected network changes', () => { - it('resets the eth query and updates the chain ID when notified via the onNetworkDidChange callback', async () => { + it('updates the chain ID used for the next fetch when notified via the onNetworkDidChange callback', async () => { let networkDidChangeListener: | ((networkControllerState: NetworkState) => Promise) | undefined; @@ -1438,23 +1444,10 @@ describe('GasFeeController', () => { ); }); - it('resets the eth query and updates the chain ID when notified via NetworkController:networkDidChange', async () => { - const rootMessenger = getRootMessenger(); - networkController = await setupNetworkController({ - rootMessenger, - state: {}, - initializeProvider: false, - }); - gasFeeController = new GasFeeController({ - getProvider: jest.fn(), - messenger: getGasFeeControllerMessenger(rootMessenger), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(false), - getCurrentNetworkEIP1559Compatibility: jest - .fn() - .mockResolvedValue(true), + it('updates the chain ID used for the next fetch when notified via NetworkController:networkDidChange', async () => { + const { rootMessenger } = await setupGasFeeController({ EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', + initializeNetworkProvider: false, }); await gasFeeController.fetchGasFeeEstimates(); @@ -1481,6 +1474,41 @@ describe('GasFeeController', () => { }), ); }); + + it('reads the provider once, caches the eth query, then rebuilds it from the provider after a network change', async () => { + const provider1 = { id: 1 } as unknown as ProviderProxy; + const provider2 = { id: 2 } as unknown as ProviderProxy; + const getProvider = jest + .fn() + .mockReturnValueOnce(provider1) + .mockReturnValueOnce(provider2); + const { rootMessenger } = await setupGasFeeController({ + getProvider, + EIP1559APIEndpoint: 'https://some-eip-1559-endpoint/', + initializeNetworkProvider: false, + }); + + // The provider is read lazily on the first fetch, and the resulting eth + // query is cached across subsequent fetches. + await gasFeeController.fetchGasFeeEstimates(); + await gasFeeController.fetchGasFeeEstimates(); + expect(getProvider).toHaveBeenCalledTimes(1); + const ethQueryBeforeChange = + mockedDetermineGasFeeCalculations.mock.lastCall?.[0].ethQuery; + + // Simulate the network switching to Sepolia. + rootMessenger.publish('NetworkController:networkDidChange', { + selectedNetworkClientId: 'sepolia', + } as NetworkState); + await flushPromises(); + + // The next fetch rebuilds the eth query from the provider. + await gasFeeController.fetchGasFeeEstimates(); + expect(getProvider).toHaveBeenCalledTimes(2); + const ethQueryAfterChange = + mockedDetermineGasFeeCalculations.mock.lastCall?.[0].ethQuery; + expect(ethQueryAfterChange).not.toBe(ethQueryBeforeChange); + }); }); describe('metadata', () => { diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index 4b4e2a5828e..65936ec7881 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -549,8 +549,9 @@ export class GasFeeController extends StaticIntervalPollingController { - if (this.#getCurrentChainId() === chainId) { + if (currentChainId === chainId) { state.gasFeeEstimates = gasFeeCalculations.gasFeeEstimates; state.estimatedGasFeeTimeBounds = gasFeeCalculations.estimatedGasFeeTimeBounds; @@ -670,10 +671,7 @@ export class GasFeeController extends StaticIntervalPollingController Date: Tue, 21 Jul 2026 13:47:02 +0300 Subject: [PATCH 5/6] fix: lint --- packages/gas-fee-controller/src/GasFeeController.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index 65936ec7881..940dce00529 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -671,7 +671,9 @@ export class GasFeeController extends StaticIntervalPollingController Date: Tue, 21 Jul 2026 16:29:55 +0300 Subject: [PATCH 6/6] fix: prune stale gas-fee-controller lint suppressions The order-agnostic refactor removed two no-restricted-syntax violations in GasFeeController.ts, leaving stale suppression counts that fail lint:eslint in CI. Co-Authored-By: Claude Opus 4.8 (1M context) --- eslint-suppressions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 46131ea3fee..d40257bc6df 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1100,7 +1100,7 @@ "count": 1 }, "no-restricted-syntax": { - "count": 16 + "count": 14 } }, "packages/gas-fee-controller/src/gas-util.ts": {