Skip to content

feat: fetch asset metadata on search #31258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 91 commits into from
Mar 28, 2025
Merged

Conversation

micaelae
Copy link
Member

@micaelae micaelae commented Mar 25, 2025

Description

This changes adds a useAssetMetadata hook that fetches asset details if it is not found in existing token lists. I've also added more unit test coverage for the asset-picker token filtering

See Copilot summary for more details

Open in GitHub Codespaces

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MMS-2094

Manual testing steps

  1. Select solana as active network
  2. Go to Bridge page
  3. Get the address of a new pump.fun coin
  4. Paste address into the asset picker's search bar
  5. The token should appear on the list and be selectable
  6. After filling out quote params, quotes should be fetched

Screenshots/Recordings

Before

No asset is shown

After

Screen.Recording.2025-03-25.at.5.29.08.PM.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new hook (useAssetMetadata) to fetch asset details that aren’t available in the local token lists and improves token filtering in the asset picker. Key changes include refactoring multichain balance processing to use a robust CAIP parser, updating the tokens filtering logic with enhanced image URL fallbacks, and integrating a debounced search with cancellation in the asset picker modal.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ui/hooks/useMultichainBalances.ts Refactored CAIP parsing and adjusted asset property types to improve balance handling.
ui/hooks/bridge/useTokensWithFiltering.ts Updated token filtering logic and image URL fallback for native assets.
ui/components/multichain/asset-picker-amount/asset-picker-modal/hooks/useAssetMetadata.ts Added new hook to fetch metadata for tokens not in the local list with an external service check.
ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx Integrated debounced search input, aborting previous fetches and using asset metadata when available.
shared/lib/asset-utils.ts Updated helper functions to generate asset IDs and corresponding image URLs.
Test files Extended coverage to validate asset metadata fetching and asset picker search behavior.
Comments suppressed due to low confidence (4)

ui/hooks/useMultichainBalances.ts:30

  • The 'address' property type has been changed from a CAIP template literal to a generic string. Please verify that downstream consumers correctly handle this updated format.
address: string;

ui/hooks/bridge/useTokensWithFiltering.ts:203

  • Ensure that the 'isNative' property is consistently set across token objects, as the updated condition now relies on it for correctly identifying native tokens.
if (isNativeAddress(token.address) || token.isNative) {

ui/components/multichain/asset-picker-amount/asset-picker-modal/hooks/useAssetMetadata.ts:50

  • [nitpick] The metadata fetch condition requires the trimmed search query to be longer than 30 characters; please confirm that this threshold is intended to avoid skipping valid queries.
if (allowExternalServices && shouldFetchMetadata && trimmedSearchQuery.length > 30) {

ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx:157

  • [nitpick] Ensure that the debouncedSetSearchQuery function is properly memoized or stable across renders to prevent unnecessary effect calls.
useEffect(() => { debouncedSetSearchQuery(searchQuery); }, [searchQuery, debouncedSetSearchQuery]);

darkwing
darkwing previously approved these changes Mar 27, 2025
Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked as expected! Awesome work @micaelae !

Suggestion:

If a token's info is being searched for, show a spinner or some type of feedback message so the user knows that work is happening.

Base automatically changed from mms2102-bridge-migration to main March 27, 2025 21:07
@micaelae micaelae dismissed stale reviews from darkwing, GustavoRSSilva, and ghgoodreau March 27, 2025 21:07

The base branch was changed.

@micaelae micaelae requested review from a team as code owners March 27, 2025 21:07
@micaelae micaelae requested a review from BboyStatix March 27, 2025 21:08
@micaelae micaelae removed request for a team and BboyStatix March 27, 2025 21:17
@micaelae micaelae enabled auto-merge March 27, 2025 21:34
@GustavoRSSilva GustavoRSSilva requested a review from Copilot March 27, 2025 21:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds support for fetching asset metadata during token search and improves token filtering and rendering across multiple chains. Key changes include the introduction of a new hook (useAssetMetadata) to fetch missing asset details, updates to token filtering logic and image URL resolution in the bridge and asset picker components, and corresponding unit and end-to-end test updates.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ui/hooks/useMultichainBalances.ts Adjusts asset data structure by replacing the CAIP-formatted address with a generic string and adding an assetId field via parseCaipAssetType.
ui/hooks/bridge/useTokensWithFiltering.ts Enhances image URL fallback logic and token native address checks with added conditions and external image map usage.
ui/components/multichain/asset-picker-amount/asset-picker-modal/hooks/useAssetMetadata.ts Introduces a new hook for asynchronously fetching asset metadata based on search input.
ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx Incorporates debounced search input and aborting of previous metadata fetch requests to improve responsiveness.
shared/lib/asset-utils.ts & Tests Implements asset ID conversion, image URL generation, and metadata fetching logic with extended test coverage.

@metamaskbot
Copy link
Collaborator

Builds ready [dd3baa7]
UI Startup Metrics (1212 ± 65 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1212106413486512551314
load10609211220651177985
domContentLoaded10549171211651173981
domInteractive16135051527
firstPaint78686121840923698
backgroundConnect106495910
firstReactRender20155162131
getState13439868
initialActions001000
loadScripts83871597862879944
setupStore8426378
WebpackHomeuiStartup943777121983954973
load80462996166842914
domContentLoaded79861595366836903
domInteractive15124271333
firstPaint44557957337824856
backgroundConnect16115381539
firstReactRender14122831325
getState6415278
initialActions001000
loadScripts79560594465835893
setupStore7416279
FirefoxBrowserifyHomeuiStartup13521188188814213791732
load12171055175213812531568
domContentLoaded12171054175113812531567
domInteractive9636167258895
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect23176572436
firstReactRender22195042427
getState6321278
initialActions001001
loadScripts11951036171713612301503
setupStore6429357
WebpackHomeuiStartup9888291573163916980
load8667261362143812901
domContentLoaded8657261362143811900
domInteractive119371842715289
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect201386102435
firstReactRender19162721924
getState94791079
initialActions001001
loadScripts8487131340140799944
setupStore95681179
Bundle size diffs
  • background: -69.92 KiB (-1.18%)
  • ui: 3.86 KiB (0.06%)
  • common: 69.92 KiB (0.74%)

@micaelae micaelae added this pull request to the merge queue Mar 28, 2025
Merged via the queue into main with commit 12505e7 Mar 28, 2025
150 checks passed
@micaelae micaelae deleted the mms2094-token-metadata-on-paste branch March 28, 2025 16:09
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2025
@metamaskbot metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Mar 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.17.0 Issue or pull request that will be included in release 12.17.0 team-swaps-and-bridge For issues with Swaps or Bridging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants