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

fix: fix RatesController.setCryptoCurrencyList invalid internal field #4572

Merged
merged 8 commits into from
Jul 30, 2024

Conversation

ccharly
Copy link
Contributor

@ccharly ccharly commented Jul 30, 2024

Explanation

We missed to rename that field in that PR: #4242

References

N/A

Changelog

@metamask/assets-controllers

  • FIXED: Fix RatesController.setCryptocurrencyList
    • It was not using the correct field when updating the internal state (fromCurrencies -> cryptocurrencies)

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

@ccharly ccharly self-assigned this Jul 30, 2024
@ccharly ccharly requested a review from a team as a code owner July 30, 2024 12:22
@ccharly
Copy link
Contributor Author

ccharly commented Jul 30, 2024

@metamaskbot publish-previews

gantunesr
gantunesr previously approved these changes Jul 30, 2024
@gantunesr
Copy link
Member

@ccharly maybe we should add a unit test to catch that, the fact that it was passing before shows that the test was lacking

Copy link
Contributor

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.0.0-preview-89bd443",
  "@metamask-previews/address-book-controller": "5.0.0-preview-89bd443",
  "@metamask-previews/announcement-controller": "7.0.0-preview-89bd443",
  "@metamask-previews/approval-controller": "7.0.2-preview-89bd443",
  "@metamask-previews/assets-controllers": "37.0.0-preview-89bd443",
  "@metamask-previews/base-controller": "6.0.2-preview-89bd443",
  "@metamask-previews/build-utils": "3.0.0-preview-89bd443",
  "@metamask-previews/chain-controller": "0.1.1-preview-89bd443",
  "@metamask-previews/composable-controller": "7.0.0-preview-89bd443",
  "@metamask-previews/controller-utils": "11.0.2-preview-89bd443",
  "@metamask-previews/ens-controller": "13.0.1-preview-89bd443",
  "@metamask-previews/eth-json-rpc-provider": "4.1.2-preview-89bd443",
  "@metamask-previews/gas-fee-controller": "19.0.1-preview-89bd443",
  "@metamask-previews/json-rpc-engine": "9.0.2-preview-89bd443",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.2-preview-89bd443",
  "@metamask-previews/keyring-controller": "17.1.2-preview-89bd443",
  "@metamask-previews/logging-controller": "5.0.0-preview-89bd443",
  "@metamask-previews/message-manager": "10.0.2-preview-89bd443",
  "@metamask-previews/name-controller": "8.0.0-preview-89bd443",
  "@metamask-previews/network-controller": "20.1.0-preview-89bd443",
  "@metamask-previews/notification-controller": "6.0.0-preview-89bd443",
  "@metamask-previews/notification-services-controller": "0.2.0-preview-89bd443",
  "@metamask-previews/permission-controller": "11.0.0-preview-89bd443",
  "@metamask-previews/permission-log-controller": "3.0.0-preview-89bd443",
  "@metamask-previews/phishing-controller": "10.1.1-preview-89bd443",
  "@metamask-previews/polling-controller": "9.0.1-preview-89bd443",
  "@metamask-previews/preferences-controller": "13.0.1-preview-89bd443",
  "@metamask-previews/profile-sync-controller": "0.2.0-preview-89bd443",
  "@metamask-previews/queued-request-controller": "4.0.0-preview-89bd443",
  "@metamask-previews/rate-limit-controller": "6.0.0-preview-89bd443",
  "@metamask-previews/selected-network-controller": "17.0.0-preview-89bd443",
  "@metamask-previews/signature-controller": "18.0.1-preview-89bd443",
  "@metamask-previews/transaction-controller": "35.1.0-preview-89bd443",
  "@metamask-previews/user-operation-controller": "14.0.1-preview-89bd443"
}

@ccharly ccharly force-pushed the fix/assets-rates-controller-setCryptocurrencyList branch from 50a1dd8 to 31fdc1d Compare July 30, 2024 15:07
@ccharly ccharly force-pushed the fix/assets-rates-controller-setCryptocurrencyList branch from 31fdc1d to 9be3b6b Compare July 30, 2024 15:16
Copy link
Member

@gantunesr gantunesr left a comment

Choose a reason for hiding this comment

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

Nice, LGTM. Thanks for correcting my mistake

@danroc danroc merged commit 882be32 into main Jul 30, 2024
116 checks passed
@danroc danroc deleted the fix/assets-rates-controller-setCryptocurrencyList branch July 30, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants