Skip to content
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

feat: fetch token rates across accounts #4759

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

bergeron
Copy link
Contributor

@bergeron bergeron commented Oct 3, 2024

Explanation

The TokenRatesController used to only fetch prices for tokens on the currently selected account. With this change, it will fetch prices for tokens across all accounts.

This will power UI features like showing portfolio $ values in the account picker, and avoids needing to re-fetch prices when switching accounts which causes some UI lag.

References

Changelog

@metamask/assets-controllers

  • CHANGED: TokenRatesController now fetches prices for tokens across all accounts

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 highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@bergeron bergeron requested a review from a team as a code owner October 3, 2024 21:34
@bergeron
Copy link
Contributor Author

bergeron commented Oct 3, 2024

@metamaskbot publish-preview

Copy link
Contributor

github-actions bot commented Oct 3, 2024

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "18.2.2-preview-9d54fee",
  "@metamask-previews/address-book-controller": "6.0.1-preview-9d54fee",
  "@metamask-previews/announcement-controller": "7.0.1-preview-9d54fee",
  "@metamask-previews/approval-controller": "7.1.0-preview-9d54fee",
  "@metamask-previews/assets-controllers": "38.1.0-preview-9d54fee",
  "@metamask-previews/base-controller": "7.0.1-preview-9d54fee",
  "@metamask-previews/build-utils": "3.0.1-preview-9d54fee",
  "@metamask-previews/chain-controller": "0.1.3-preview-9d54fee",
  "@metamask-previews/composable-controller": "9.0.1-preview-9d54fee",
  "@metamask-previews/controller-utils": "11.3.0-preview-9d54fee",
  "@metamask-previews/ens-controller": "14.0.1-preview-9d54fee",
  "@metamask-previews/eth-json-rpc-provider": "4.1.4-preview-9d54fee",
  "@metamask-previews/gas-fee-controller": "20.0.1-preview-9d54fee",
  "@metamask-previews/json-rpc-engine": "9.0.3-preview-9d54fee",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.3-preview-9d54fee",
  "@metamask-previews/keyring-controller": "17.2.2-preview-9d54fee",
  "@metamask-previews/logging-controller": "6.0.1-preview-9d54fee",
  "@metamask-previews/message-manager": "10.1.1-preview-9d54fee",
  "@metamask-previews/name-controller": "8.0.1-preview-9d54fee",
  "@metamask-previews/network-controller": "21.0.1-preview-9d54fee",
  "@metamask-previews/notification-controller": "7.0.0-preview-9d54fee",
  "@metamask-previews/notification-services-controller": "0.8.2-preview-9d54fee",
  "@metamask-previews/permission-controller": "11.0.2-preview-9d54fee",
  "@metamask-previews/permission-log-controller": "3.0.1-preview-9d54fee",
  "@metamask-previews/phishing-controller": "12.0.3-preview-9d54fee",
  "@metamask-previews/polling-controller": "10.0.1-preview-9d54fee",
  "@metamask-previews/preferences-controller": "13.0.3-preview-9d54fee",
  "@metamask-previews/profile-sync-controller": "0.9.5-preview-9d54fee",
  "@metamask-previews/queued-request-controller": "5.1.0-preview-9d54fee",
  "@metamask-previews/rate-limit-controller": "6.0.1-preview-9d54fee",
  "@metamask-previews/selected-network-controller": "18.0.1-preview-9d54fee",
  "@metamask-previews/signature-controller": "19.1.0-preview-9d54fee",
  "@metamask-previews/transaction-controller": "37.2.0-preview-9d54fee",
  "@metamask-previews/user-operation-controller": "15.0.1-preview-9d54fee"
}

Copy link
Contributor

@gambinish gambinish left a comment

Choose a reason for hiding this comment

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

✅ Looks good to me.

Resolved my frontend PR to this and it seems to address the race condition nicely.

Went through it on the debugger and didn't run into a state where we didn't have enough market data to properly render & sort the list by descending fiat balance 👍

@bergeron bergeron merged commit 3b7b88c into main Oct 4, 2024
116 checks passed
@bergeron bergeron deleted the brian/token-rates-across-accounts branch October 4, 2024 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants