feat(wallet)!: wire GasFeeController into default initialization#9527
Open
sirtimid wants to merge 3 commits into
Open
feat(wallet)!: wire GasFeeController into default initialization#9527sirtimid wants to merge 3 commits into
GasFeeController into default initialization#9527sirtimid wants to merge 3 commits into
Conversation
sirtimid
added a commit
that referenced
this pull request
Jul 15, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
7 tasks
sirtimid
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Jul 16, 2026
Construct the extension's GasFeeController through @metamask/wallet instead of the local messenger-client-init wiring, integrating MetaMask/core#9527. The wallet now owns GasFeeController as an instanceOptions.gasFeeController slot: it builds getProvider and getCurrentNetworkEIP1559Compatibility from NetworkController itself and lets the controller default onNetworkDidChange and getChainId via its messenger subscription. The extension supplies only the client-specific overrides (interval, clientId, gas API endpoints, and the BSC legacy gas API compatibility check). - Add wallet-init/instance-options/gas-fee-controller.ts and a minimal wallet-init/messengers/gas-fee-controller-messenger.ts (delegates only NetworkController:getState + getNetworkClientById for the chain-id read). - Wire gasFeeController into wallet-init/initialization.ts. - Resolve this.gasFeeController via this.wallet.getInstance('GasFeeController') and drop the GasFeeControllerInit import + init-map entry. - Delete the local confirmations/gas-fee-controller-init and messengers/gas-fee-controller-messenger (+tests) and their entries in messenger-client-init/messengers/index.ts. GasFeeController stays in controller-list.ts. Preview-pin @metamask/wallet and @metamask/gas-fee-controller to the core#9527 preview builds (7ed2770ae) until a wallet release ships it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimid
marked this pull request as ready for review
July 16, 2026 15:14
10 tasks
sirtimid
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Jul 16, 2026
Construct the extension's GasFeeController through @metamask/wallet instead of the local messenger-client-init wiring, integrating MetaMask/core#9527. The wallet now owns GasFeeController as an instanceOptions.gasFeeController slot: it builds getProvider and getCurrentNetworkEIP1559Compatibility from NetworkController itself and lets the controller default onNetworkDidChange and getChainId via its messenger subscription. The extension supplies only the client-specific overrides (interval, clientId, gas API endpoints, and the BSC legacy gas API compatibility check). - Add wallet-init/instance-options/gas-fee-controller.ts and a minimal wallet-init/messengers/gas-fee-controller-messenger.ts (delegates only NetworkController:getState + getNetworkClientById for the chain-id read). - Wire gasFeeController into wallet-init/initialization.ts. - Resolve this.gasFeeController via this.wallet.getInstance('GasFeeController') and drop the GasFeeControllerInit import + init-map entry. - Delete the local confirmations/gas-fee-controller-init and messengers/gas-fee-controller-messenger (+tests) and their entries in messenger-client-init/messengers/index.ts. GasFeeController stays in controller-list.ts. Preview-pin @metamask/wallet and @metamask/gas-fee-controller to the core#9527 preview builds (7ed2770ae) until a wallet release ships it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rekmarks
requested changes
Jul 16, 2026
rekmarks
left a comment
rekmarks
left a comment
Member
There was a problem hiding this comment.
Looks good! Just a couple of things.
Comment on lines
+90
to
+96
| if (index === -1) { | ||
| // A dependency cycle remains. Emit the rest in their current order rather | ||
| // than looping forever; the messenger surfaces a clear "handler has not | ||
| // been registered" error at construction time. | ||
| ordered.push(...remaining); | ||
| break; | ||
| } |
Member
There was a problem hiding this comment.
That error doesn't actually explain what the problem is. Shouldn't we instead throw here and note what the cycle is?
| // The controller substitutes `<chain_id>` with the decimal chain ID per request. | ||
| const DEFAULT_EIP1559_API_ENDPOINT = `${GAS_API_BASE_URL}/networks/<chain_id>/suggestedGasFees`; | ||
| const DEFAULT_LEGACY_API_ENDPOINT = `${GAS_API_BASE_URL}/networks/<chain_id>/gasPrices`; | ||
| const DEFAULT_CLIENT_ID = 'cli'; |
Member
There was a problem hiding this comment.
Should we instead make client id required? It would be unfortunate if someone forgot to add this in the production clients.
Member
Author
There was a problem hiding this comment.
Yes! I made gasFeeController a required instance options key, not just the clientId field ec8eeec
sirtimid
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Jul 20, 2026
Construct the extension's GasFeeController through @metamask/wallet instead of the local messenger-client-init wiring, integrating MetaMask/core#9527. The wallet now owns GasFeeController as an instanceOptions.gasFeeController slot: it builds getProvider and getCurrentNetworkEIP1559Compatibility from NetworkController itself and lets the controller default onNetworkDidChange and getChainId via its messenger subscription. The extension supplies only the client-specific overrides (interval, clientId, gas API endpoints, and the BSC legacy gas API compatibility check). - Add wallet-init/instance-options/gas-fee-controller.ts and a minimal wallet-init/messengers/gas-fee-controller-messenger.ts (delegates only NetworkController:getState + getNetworkClientById for the chain-id read). - Wire gasFeeController into wallet-init/initialization.ts. - Resolve this.gasFeeController via this.wallet.getInstance('GasFeeController') and drop the GasFeeControllerInit import + init-map entry. - Delete the local confirmations/gas-fee-controller-init and messengers/gas-fee-controller-messenger (+tests) and their entries in messenger-client-init/messengers/index.ts. GasFeeController stays in controller-list.ts. Preview-pin @metamask/wallet and @metamask/gas-fee-controller to the core#9527 preview builds (7ed2770ae) until a wallet release ships it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire `GasFeeController` into the wallet's default set as its own `InitializationConfiguration`, with all client-varying values injectable via `instanceOptions.gasFeeController` and platform-agnostic defaults. This resolves the lazy `GasFeeController:fetchGasFeeEstimates` delegation already registered by the wired `TransactionController`. Because `GasFeeController`'s constructor eagerly reads `NetworkController` state, add an optional `dependencies` field to `InitializationConfiguration` and a stable topological sort in `initialize` so dependencies are always constructed first (the default set was otherwise ordered alphabetically). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on #9527: - Make `instanceOptions.gasFeeController.clientId` required and drop the `'cli'` default, so every client (extension, mobile, wallet-cli) must identify itself to the gas API rather than silently inheriting a default. `gasFeeController` is now a required instance-options key (mirroring `networkController`); wallet-cli passes `clientId: 'cli'` explicitly. - Throw a descriptive error naming the members of the cycle when `orderByDependencies` cannot satisfy the declared dependencies, instead of silently deferring to a later "handler not registered" messenger error. Also re-add the `gas-fee-controller` instance rule via the new programmatic `codeowners.ts` generator (#9529) picked up in the rebase onto main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimid
force-pushed
the
sirtimid/wire-gas-fee-controller
branch
from
July 20, 2026 14:27
7ed2770 to
ec8eeec
Compare
matthewwalsh0
requested changes
Jul 20, 2026
| * @returns The configurations ordered so that dependencies come first. | ||
| * @throws If the configurations contain a dependency cycle. | ||
| */ | ||
| export function orderByDependencies( |
Member
There was a problem hiding this comment.
This is technically very cool! But logistically terrifying 😅
Should we ensure we don't migrate packages until they are initialisation-order-agnostic?
We had to do this for TransactionController for example.
I'd hope these dependencies are very rare, as we usually rely on legacy callbacks or dynamic messenger calls, rather than hard dependencies at constructor / initialisation time?
matthewwalsh0
self-requested a review
July 20, 2026 14:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Wires
GasFeeControllerinto@metamask/wallet's default initialization set as its ownInitializationConfigurationundersrc/initialization/instances/gas-fee-controller/, mirroring the most recently merged instances (transaction-controller,network-controller).TransactionControlleris already wired and its messenger delegatesGasFeeController:fetchGasFeeEstimates. Delegation registers a lazy handler, so the wallet boots fine today — but the first transaction flow that needs gas estimates throwsA handler for GasFeeController:fetchGasFeeEstimates has not been registered. WiringGasFeeControlleris the missing piece that lets a wiredTransactionControlleractually estimate gas.@metamask/walletis the shared controller-integration layer thatmetamask-extension,metamask-mobile, and@metamask/wallet-cliall adopt, so every value that differs between clients is an injectableinstanceOptions.gasFeeControllerslot with a platform-agnostic default — nothing is baked to one client.Constructor callbacks
GasFeeControllertakes direct callbacks rather than pure messenger delegation. The instance builds them from the wiredNetworkController:getProvider→NetworkController:getState(selectedNetworkClientId) thenNetworkController:getNetworkClientById(id).providergetCurrentNetworkEIP1559Compatibility→NetworkController:getEIP1559Compatibility(coerced to a definedboolean)onNetworkDidChange/getChainIdare omitted — the constructor already has a messenger-based network-tracking fallback when both are absent (subscribesNetworkController:networkDidChange).Injectable options + per-environment values
EIP1559APIEndpoint.../networks/<chain_id>/suggestedGasFees(dev override →gas.uat-api.cx.metamask.io)legacyAPIEndpoint${GAS_API}/networks/<chain_id>/gasPricesclientId'extension''mobile''cli'(sent asX-Client-Id); injectableinterval10_00015_00015_000; injectablegetCurrentNetworkLegacyGasAPICompatibilitychainId === BSCmainnet || BSC || POLYGON() => false; injectablegetCurrentAccountEIP1559Compatibility() => true() => true; injectableInitialization ordering
GasFeeController's constructor eagerly readsNetworkControllerstate (to build its provider), soNetworkControllermust be constructed first. The default set was previously constructed in the alphabetical order ofObject.values(defaultConfigurations), which happened to satisfy every existing dependency (e.g.network<transaction) — butgas<networkbreaks that, causing aA handler for NetworkController:getState has not been registeredthrow at construction.To fix this generally, this PR adds an optional
dependenciesfield toInitializationConfigurationand a stable topological sort (orderByDependencies) ininitialize.gasFeeControllerdeclaresdependencies: ['NetworkController']. Instances without an unmet dependency keep their relative order, and dependencies outside the set are treated as already satisfied.fetchdecisionGasFeeControllerfetches gas estimates via the globalfetchinside@metamask/gas-fee-controller(it has no injectablefetchoption). This PR takes path (a): accept the globalfetch— no upstream change; works on Node 18+ (wallet-cli daemon), modern browsers, and React Native. The global-fetchusage lives inside the controller package, not inside@metamask/walletcode, and both clients already rely on this today. Adding an injectablefetchoption to@metamask/gas-fee-controlleris tracked as a possible follow-up if the convention is to be enforced strictly.Client adoption PRs
References
packages/gas-fee-controller/src/GasFeeController.tspackages/wallet/src/initialization/instances/transaction-controller/app/scripts/messenger-client-init/confirmations/gas-fee-controller-init.tsapp/core/Engine/controllers/gas-fee-controller/gas-fee-controller-init.tsRelated
GasFeeControllerinto@metamask/walletdefault initialization #9510Checklist
orderByDependenciesunit test).🤖 Generated with Claude Code
Note
Medium Risk
Breaking API for all Wallet consumers and changes core wallet bootstrap order; impact is localized to gas estimation wiring rather than auth or funds movement.
Overview
BREAKING:
@metamask/walletnow bootsGasFeeControllerby default soTransactionControllergas flows (e.g.GasFeeController:fetchGasFeeEstimates) have a real handler instead of failing at runtime.Consumers must pass
instanceOptions.gasFeeControllerwith a requiredclientId(gas APIX-Client-Id); other gas options stay optional with shared defaults (prod gas API URLs, network callbacks wired fromNetworkController).Wallet init no longer relies on implicit alphabetical construction order: configurations can declare
dependencies, andorderByDependenciestopologically sorts them (GasFeeControllerdepends onNetworkController). wallet-cli setsclientId: 'cli'.Reviewed by Cursor Bugbot for commit ec8eeec. Bugbot is set up for automated code reviews on this repo. Configure here.