Skip to content

Commit 24518b2

Browse files
authored
chore: remove hardcoded NFT chain allowlist from NftDetectionController (#8180)
## Explanation ### Summary As part of the NFT provider migration plan, this PR removes the hardcoded `supportedNftDetectionNetworks` allowlist from `NftDetectionController` and delegates chain support validation to the server instead. > **Deployment dependency:** This PR depends on backend changes deployed to production in [`va-mmcx-nft-api#221`](consensys-vertical-apps/va-mmcx-nft-api#221). **Do not merge this PR until those backend changes are live in production.** The server must return an empty array (instead of an error) for unsupported chains before the client-side chain gating can be safely removed. ### Changes - **Remove `supportedNftDetectionNetworks`**: Drops the static `Set<Hex>` that previously allowed only 8 specific chains (Mainnet, BSC, Polygon, Avalanche, Linea, Base, Sei, Monad) through NFT detection. Detection now passes all provided chain IDs directly to the API. - **Remove chain filtering in `detectNfts`**: The pre-API filter that dropped unsupported chains before calling `#getOwnerNfts` has been removed. `chainIds` is now passed through unmodified, allowing the server to decide what to return. - **Add non-EVM guard in `#getOwnerNfts`**: Chains that convert to decimal `'0'` (non-EVM/invalid chains) are filtered out before making the API call, and if all chains are non-EVM, the method returns `{ tokens: [], continuation: null }` early — avoiding a pointless API round-trip. - **Fix `continuation` type**: Updated `ReservoirResponse.continuation` from `string | undefined` to `string | null` to match the updated server response shape. ### Motivation Keeping a hardcoded chain allowlist in the Core package tightly coupled backend and frontend release cycles — adding support for a new chain required coordinated changes in both places simultaneously. The backend change in [`va-mmcx-nft-api#221`](consensys-vertical-apps/va-mmcx-nft-api#221) updates the NFT API to return `[]` instead of an error for unsupported chains. With that in place, the Core package no longer needs to know which chains are supported, enabling each side to evolve independently going forward. ### Testing - All existing `NftDetectionController` tests should continue to pass. - Verify that NFT detection on previously unsupported chains gracefully returns no results rather than throwing. - Verify that non-EVM chains short-circuit before hitting the API. ### Merge checklist - [ ] [`va-mmcx-nft-api#221`](consensys-vertical-apps/va-mmcx-nft-api#221) is merged and deployed to production - [ ] NFT detection verified end-to-end against production API on at least one previously unsupported chain <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References https://consensyssoftware.atlassian.net/browse/ASSETS-2899 <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes NFT auto-detection behavior by delegating chain support to the backend and altering pagination typing; incorrect backend behavior or unexpected chain IDs could impact NFT detection results. > > **Overview** > **NFT detection no longer hardcodes a supported-chain allowlist.** `NftDetectionController.detectNfts` now passes all provided `chainIds` through to the NFT API instead of filtering to a fixed set of EVM chains, relying on the server to return empty results for unsupported networks. > > **Non-EVM guardrails and response typing were updated.** `#getOwnerNfts` filters out chain IDs that convert to decimal `0` and returns an empty `{ tokens: [], continuation: null }` without making an API call when only non-EVM chains are provided, and `ReservoirResponse.continuation` is widened to `string | null` to match the API shape. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 33e9745. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 5470e31 commit 24518b2

File tree

2 files changed

+20
-26
lines changed

2 files changed

+20
-26
lines changed

packages/assets-controllers/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4343
- **BREAKING:** Standardize names of `AccountTrackerController` messenger action types ([#8164](https://github.com/MetaMask/core/pull/8164))
4444
- All existing types for messenger actions have been renamed so they include `Controller` (e.g. `AccountTrackerUpdateNativeBalancesAction` -> `AccountTrackerControllerUpdateNativeBalancesAction`). You will need to update imports appropriately.
4545
- This change only affects the types. The action type strings themselves have not changed, so you do not need to update the list of actions you pass when initializing `AccountTrackerController` messengers.
46+
- Remove hardcoded `supportedNftDetectionNetworks` chain allowlist from `NftDetectionController`; the NFT API now returns an empty array for unsupported chains instead of an error, so chain gating is no longer needed in the client ([#8180](https://github.com/MetaMask/core/pull/8180))
47+
- `NftDetectionController` now skips the NFT API call entirely when all provided chain IDs are non-EVM (decimal `0`), returning an empty result immediately ([#8180](https://github.com/MetaMask/core/pull/8180))
48+
- Update `ReservoirResponse.continuation` type from `string` to `string | null` to match the NFT API response shape ([#8180](https://github.com/MetaMask/core/pull/8180))
4649

4750
### Fixed
4851

packages/assets-controllers/src/NftDetectionController.ts

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,6 @@ export type NftDetectionControllerMessenger = Messenger<
6363
AllowedEvents
6464
>;
6565

66-
/**
67-
* Set of supported networks for NFT detection.
68-
*/
69-
const supportedNftDetectionNetworks: Set<Hex> = new Set([
70-
'0x1', // Mainnet
71-
'0x38', // BSC
72-
'0x89', // Polygon
73-
'0xa86a', // Avalanche
74-
'0xe708', // Linea Mainnet
75-
'0x2105', // Base
76-
'0x531', // Sei
77-
'0x8f', // Monad
78-
]);
79-
8066
/**
8167
* @type ApiNft
8268
*
@@ -188,7 +174,7 @@ export type ApiNftCreator = {
188174

189175
export type ReservoirResponse = {
190176
tokens: TokensResponse[];
191-
continuation?: string;
177+
continuation?: string | null;
192178
};
193179

194180
export type TokensResponse = {
@@ -540,8 +526,21 @@ export class NftDetectionController extends BaseController<
540526
const convertedChainIds = chainIds.map((chainId) =>
541527
convertHexToDecimal(chainId).toString(),
542528
);
529+
530+
const filteredChainIds = convertedChainIds.filter(
531+
(chainId) => chainId !== '0',
532+
);
533+
534+
// Avoid making the API call for non-EVM chains
535+
if (filteredChainIds.length === 0) {
536+
return {
537+
tokens: [],
538+
continuation: null,
539+
};
540+
}
541+
543542
const url = this.#getOwnerNftApi({
544-
chainIds: convertedChainIds,
543+
chainIds: filteredChainIds,
545544
address,
546545
next: cursor,
547546
});
@@ -575,12 +574,8 @@ export class NftDetectionController extends BaseController<
575574
options?.userAddress ??
576575
this.messenger.call('AccountsController:getSelectedAccount').address;
577576

578-
// filter out unsupported chainIds
579-
const supportedChainIds = chainIds.filter((chainId) =>
580-
supportedNftDetectionNetworks.has(chainId),
581-
);
582577
/* istanbul ignore if */
583-
if (supportedChainIds.length === 0 || this.#disabled) {
578+
if (chainIds.length === 0 || this.#disabled) {
584579
return;
585580
}
586581
/* istanbul ignore else */
@@ -611,11 +606,7 @@ export class NftDetectionController extends BaseController<
611606
let resultNftApi: ReservoirResponse;
612607
try {
613608
do {
614-
resultNftApi = await this.#getOwnerNfts(
615-
userAddress,
616-
supportedChainIds,
617-
next,
618-
);
609+
resultNftApi = await this.#getOwnerNfts(userAddress, chainIds, next);
619610
apiNfts = resultNftApi.tokens.filter(
620611
(elm) =>
621612
elm.token.isSpam === false &&

0 commit comments

Comments
 (0)