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.

Sign up for GitHub

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

Jump to bottom

Fix initial native currency TokenRatesController configuration #1497

Merged
merged 1 commit into from
Jul 13, 2023
Merged

Fix initial native currency TokenRatesController configuration #1497

merged 1 commit into from
Jul 13, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 13, 2023 •
edited
Loading

Explanation

The TokenRatesController was always initialized with eth as the default nativeCurrency configuration. This could result in the wrong rates being retrieved if the current network has a different currency symbol.

The constructor has been updated to require a ticker parameter, used to initialize the nativeCurrency configuation for the current network at time of construction.

References

This is tangentially related to #1466

Changelog

@metamask/assets-controllers

  • BREAKING: New required ticker constructor parameter for the TokenRatesController
  • Fixed: Fix bug where token rates were incorrect if initialized with a non-Ethereum selected network.

Checklist

  • I've updated the test suite for new or updated code as appropriate
    • The condition that would fail is no longer possible, so there is no way to add a test case for it. The tests for this controller are very difficult to work with at the moment anyway, which is partly what motivated this change.
  • 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

@Gudahtt Gudahtt marked this pull request as ready for review July 13, 2023 21:54
@Gudahtt Gudahtt requested a review from a team as a code owner July 13, 2023 21:54
mcmire
mcmire approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt
Fix initial native currency TokenRatesController configuration
bd8c9a4 
The `TokenRatesController` was always initialized with `eth` as the
default `nativeCurrency` configuration. This could result in the wrong
rates being retrieved if the current network has a different currency
symbol.

The constructor has been updated to require a `ticker` parameter, used
to initialize the `nativeCurrency` configuation for the current
network at time of construction.
@Gudahtt Gudahtt force-pushed the fix-initial-token-rates-controller-native-currency branch from 9e574d6 to bd8c9a4 Compare July 13, 2023 22:10
@Gudahtt Gudahtt merged commit fd41d1a into main Jul 13, 2023
91 checks passed
@Gudahtt Gudahtt deleted the fix-initial-token-rates-controller-native-currency branch July 13, 2023 22:24
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@Gudahtt @MajorLift
Fix initial native currency TokenRatesController configuration (#1497)
a7192ed 
The `TokenRatesController` was always initialized with `eth` as the
default `nativeCurrency` configuration. This could result in the wrong
rates being retrieved if the current network has a different currency
symbol.

The constructor has been updated to require a `ticker` parameter, used
to initialize the `nativeCurrency` configuation for the current
network at time of construction.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@Gudahtt @MajorLift
Fix initial native currency TokenRatesController configuration (#1497)
2018899 
The `TokenRatesController` was always initialized with `eth` as the
default `nativeCurrency` configuration. This could result in the wrong
rates being retrieved if the current network has a different currency
symbol.

The constructor has been updated to require a `ticker` parameter, used
to initialize the `nativeCurrency` configuation for the current
network at time of construction.
tommasini added a commit that referenced this pull request Mar 7, 2024
@tommasini
cherry pick TokenRatesController configuration #1497
655d7e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcmire mcmire mcmire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gudahtt @mcmire