refactor(transaction-pay-controller): read asset data directly from AssetsController - #10461
Merged
Merged
Conversation
matthewwalsh0
force-pushed
the
perf/transaction-pay-direct-assets
branch
from
September 25, 2026 08:53
d19a096 to
1d6026a
Compare
matthewwalsh0
changed the title
matthewwalsh0
force-pushed
the
perf/transaction-pay-direct-assets
branch
from
September 25, 2026 10:51
003db5e to
6872d7c
Compare
Member
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…rom asset state `getTokenInfo` no longer falls back to the `NetworkController` ticker and a hardcoded 18 decimals when a native token is absent from `AssetsController` state, so a chain the wallet has not indexed is reported as unknown instead of resolving on assumed values. Native asset IDs are located by scanning the chain's entries for the one marked `native`, since natives are keyed inconsistently across chains and cannot be derived. ERC-20 IDs remain derived from the chain and address. `buildCaipAssetType` is retained for identifiers sent to third parties, such as Ramps order assets, and documented as such to keep it distinct from lookups against the wallet's own asset state. The asset ID cache is keyed on the metadata snapshot it was read from, so it invalidates itself when `AssetsController` publishes and an asset missing at first lookup resolves once it appears.
Drop type assertions that oxlint reports as unnecessary and rebaseline the suppression counts for the test files this branch rewrites.
…sent An ERC-20 identifier state cannot confirm is no longer returned, matching how a native absent from state already resolved. The derived identifier only ever reached a price lookup for an asset with no metadata, since a balance is unusable without the decimals stored alongside it. Drop the per-snapshot identifier cache. It was discarded whenever AssetsController published, so each entry served a handful of lookups, and what it saved was a string comparison rather than the per-asset keccak256 the legacy projection ran. Read balance decimals from the metadata snapshot already in hand rather than re-entering AssetsController state.
…nd address An asset ID is a static property of its chain and address, so one resolved from any snapshot stays correct and is keyed by chain and lower-cased address rather than scoped to the state it came from. A cached ID is confirmed against the given state before reuse, since an asset AssetsController has not indexed yet is unresolvable however its ID is spelled. That check is a keyed read, so the scan is still skipped. Only resolved IDs are cached, so an asset missing at first lookup is picked up once it appears.
…onstant NATIVE_TOKEN_DECIMALS backed the native token fallback, which no longer exists now that decimals are read from AssetsController state.
…onfig references The package swapped the legacy assets-controllers dependency for accounts-controller, which the repo's generated README dependency graph and tsconfig.lint.json project references are both derived from.
matthewwalsh0
force-pushed
the
perf/transaction-pay-direct-assets
branch
from
September 28, 2026 09:42
ff2ab05 to
3d5bb62
Compare
Member
Author
|
@metamaskbot publish-preview |
…g indexing assets-controller v17 narrows the assetsInfo key to a CAIP-19 template literal, so indexing it with a `for...in` key (typed `string`) produced an implicit `any` under noImplicitAny and an unsafe-member-access lint error. Iterate with Object.entries/Object.keys so the value is read directly rather than through an index operation.
pedronfigueiredo
approved these changes
Sep 29, 2026
Merged
pull Bot
pushed a commit
to Reality2byte/metamask-mobile
that referenced
this pull request
Sep 29, 2026
…etaMask#36889) ## Description Bumps `transaction-pay-controller` to `^30.0.0` ([MetaMask/core#10461](MetaMask/core#10461), released in [MetaMask/core#10565](MetaMask/core#10565)), which reads token metadata, balances, and prices straight from `AssetsController` rather than the legacy projection assembled from five separate controllers. The messenger now delegates `AccountsController:getState` and `AssetsController:getState` in place of the `AccountTrackerController`, `CurrencyRateController`, `TokenBalancesController`, `TokenRatesController`, and `TokensController` reads that backed `AssetsController:getStateForTransactionPay`. It also delegates `AssetsController:stateChange`, which the controller now subscribes to in place of the separate per-controller asset state events. ### Dependencies No resolutions are needed: main already declares the versions the controller requires (`accounts-controller` ^40, `assets-controller` ^17, `ramps-controller` ^26.0.1), so adopting it introduces no duplicate `@metamask` packages. ## Checklist - [x] Typecheck clean (only pre-existing failures from the gitignored generated `termsOfUseContent`) - [x] Messenger action union verified to resolve to its 33 real actions rather than `any` - [x] 701 Pay-related tests pass across 52 suites <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > MetaMask Pay now depends on a single AssetsController surface for balances, rates, and tokens; regressions could affect pay estimates or fiat flows if asset state diverges from the old multi-controller projection. > > **Overview** > Upgrades **`@metamask/transaction-pay-controller`** to **^30.0.0** and aligns the mobile **`TransactionPayController`** messenger with the new asset data path. > > **Messenger delegation** now wires **`AccountsController:getState`** and **`AssetsController:getState`** plus **`AssetsController:stateChange`**, instead of the previous bundle (`AccountTrackerController`, `CurrencyRateController`, `TokenBalancesController`, `TokenRatesController`, `TokensController`, and **`AssetsController:getStateForTransactionPay`**). A unit test asserts those **`AssetsController`** action and event delegations. > > Lockfile changes follow the bumped controller and its transitive deps (e.g. **`assets-controller` ^17**, **`ramps-controller` ^26**). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ab14eb2. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
pull Bot
pushed a commit
to firas9941/metamask-extension
that referenced
this pull request
Sep 30, 2026
…etaMask#46706) ## **Description** Bumps `transaction-pay-controller` to `^30.0.0` ([MetaMask/core#10461](MetaMask/core#10461), released in [MetaMask/core#10565](MetaMask/core#10565)), which reads token metadata, balances, and prices straight from `AssetsController` rather than the legacy projection assembled from five separate controllers. The messenger now delegates `AccountsController:getState` and `AssetsController:getState` in place of the `AccountTrackerController`, `CurrencyRateController`, `TokenBalancesController`, `TokenRatesController`, and `TokensController` reads that backed `AssetsController:getStateForTransactionPay`. The controller now subscribes only to `AssetsController:stateChange` (already delegated), so the `CurrencyRateController`, `TokenRatesController`, and `TokensController` state events are dropped. ### Why `ramps-controller` is bumped Extension was three majors behind, so this also absorbs the v29 Ramps `getQuotes` -> `getQuoteWithFees` rename. `ramps-controller` is bumped to `^26.0.1` and its `^22` resolution removed. Bumping only Pay leaves that resolution forcing it onto `ramps-controller` 22, which has no `getQuoteWithFees`: - At runtime, the fiat strategy's `RampsController:getQuoteWithFees` call fails. - At compile time, the import sits in a `.d.ts`, so `skipLibCheck` resolves it to `any`. That collapses the UI messenger's action union to plain `string`, surfacing only as two `Unused '@ts-expect-error'` errors in `ui-messenger.test.ts`. Ramps 26's own breaking change adds `KycController:getProviderFlowStatus` to `RAMPS_CONTROLLER_REQUIRED_CONTROLLER_ACTIONS`. It is only called on the VBA onboarding path, like the existing `KycController` actions extension already delegates via that constant, so no wiring changes are needed. No other dependencies or resolutions change. Pay declares `accounts-controller` `^40.0.0`, but extension's existing `^39.1.0` resolution is kept: 40.0.0 only drops CommonJS and bumps Node/ES targets, with no API change. ## **Changelog** CHANGELOG entry: null ## **Related issues** Depends on: MetaMask/core#10461, released in MetaMask/core#10565 as 30.0.0 ## **Manual testing steps** 1. Run the extension and open a Pay confirmation (deposit or withdraw). 2. Verify the payment token list shows correct balances and fiat values. 3. Switch the payment token and verify the quote and fees update. 4. Verify a token with a zero balance still appears with the correct metadata. ## **Screenshots/Recordings** N/A — no visual change; this is an internal state-source refactor. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability. - [x] I've included tests if applicable. - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable. - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
This branch was successfully deployed
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
2 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
Token metadata, balances, and fiat rates were read from the legacy per-domain asset controllers (
TokensController,TokenBalancesController,TokenRatesController,CurrencyRateController,AccountTrackerController), with anassetsUnifyStatefeature flag switching some of those reads over to a bespokeAssetsController:getStateForTransactionPayaction. That meant two parallel read paths, a Pay-specific action onAssetsController, and a subscription to four separatestateChangeevents to cover whichever source happened to be live.AssetsControllerunified state, resolving the account viaAccountsController'saccountIdByAddress. The feature-flag branching, the Pay-specificgetStateForTransactionPayaction, and the legacy@metamask/assets-controllersdependency are all no longer used, as is the multi-source subscription, replaced by a singleAssetsController:stateChange.getTokenBalanceexpect raw base units, so balances are shifted by the token's decimals.AssetsControllerbecomes the sole source of token metadata. A token absent from it is reported as unknown rather than being reconstructed from network configuration or assumed from a derived identifier, so an unindexed chain is rejected instead of quoting against an assumed symbol and decimals.Performance
Serving Pay from the legacy shape meant projecting unified state into it, which runs
toChecksumAddress— and therefore keccak256 — once per account and once per asset. That projection is memoised on a single entry keyed by input identity, so the cost is paid in full on the first read of each load and again on every read after the assets pipeline updates. Its own docstring notes this dominates CPU profiles during transaction approval.Reading unified state directly removes that work rather than caching it: Pay no longer builds the legacy projection at all, so no address is checksummed on its behalf. Asset IDs are looked up against the metadata snapshot in hand — a keyed read for ERC-20s, falling back to a scan only for natives and for addresses recovered from calldata in a different case. Because an asset ID is a static property of its chain and address, a resolved ID is cached on those rather than on the state it came from, and confirmed against the current state on reuse so that an asset not yet indexed stays unresolved.
Consumers must delegate
AccountsController:getState,AssetsController:getState, andAssetsController:stateChangeto the Pay messenger, and must have unified asset state populated before using Pay.References
Checklist
Note
High Risk
Breaking integration and payment-path changes: quotes and relay balance checks depend on unified asset indexing and new messenger wiring; misconfigured clients could show wrong balances or fail token resolution.
Overview
Breaking change: Transaction Pay no longer reads token metadata, balances, or fiat rates from the legacy
@metamask/assets-controllersstack or fromAssetsController:getStateForTransactionPay. Everything goes throughAssetsController:getState, with wallet addresses mapped viaAccountsController:getState(accountIdByAddress).The
assetsUnifyStatefeature flag andgetAssetsUnifyStateFeatureare removed. Asset refresh for in-flight Pay transactions now listens only toAssetsController:stateChangeinstead of four separate controller events. Dependencies drop@metamask/assets-controllersand add@metamask/accounts-controller.getTokenBalance,getTokenInfo, andgetTokenFiatRateintoken.tsare rewritten: balances convert human-readableassetsBalanceamounts to raw base units usingassetsInfodecimals; prices come fromassetsPrice(fungible only, stablecoin USD override preserved). Missing assets are treated as unknown/zero rather than inferred from network tickers. CAIP-19 keys are resolved with a small cache and native-token scan logic.Tests and messenger mocks are updated to match the new messenger surface; consumers must wire
AccountsController:getState,AssetsController:getState, andAssetsController:stateChangeand populate unified asset state before Pay runs.Reviewed by Cursor Bugbot for commit 72691e4. Bugbot is set up for automated code reviews on this repo. Configure here.