Skip to content

Conversation

@salimtb
Copy link
Contributor

@salimtb salimtb commented Oct 31, 2025

Description

This PR removes the RatesController to reduce redundant CryptoCompare API calls in the multichain architecture.

Reason for the change

The RatesController was introduced for fetching cryptocurrency conversion rates (primarily for BTC and SOL) to support non-EVM chains. However, this resulted in duplicate API calls to CryptoCompare since we already have MultichainAssetsRatesController and TokenRatesController handling rate fetching functionality. This redundancy increased API usage unnecessarily.

Improvement/solution

  • Removed RatesController initialization and configuration from the Engine
  • Deleted RatesController messenger setup and related controller initialization files
  • Removed multichain selectors that depended on RatesController state (selectMultichainConversionRate, selectMultichainCoinRates)
  • Cleaned up type definitions and state management to remove RatesController references
  • Updated test snapshots and initial background state

The existing MultichainAssetsRatesController and TokenRatesController will continue to handle all rate-fetching needs without the additional overhead of RatesController.

Changelog

CHANGELOG entry: Removed redundant RatesController to optimize API usage

Related issues

Fixes:

Manual testing steps

Feature: Multichain rate fetching

  Scenario: User views portfolio balance with non-EVM accounts
    Given user has Bitcoin and Solana accounts configured
    And user is viewing their portfolio

    When the app fetches cryptocurrency rates
    Then rates should be fetched only through MultichainAssetsRatesController
    And no redundant API calls should be made to CryptoCompare
    And account balances should display with correct fiat conversions

  Scenario: User switches between EVM and non-EVM accounts
    Given user has both EVM and non-EVM accounts
    And user is viewing their wallet

    When user switches between different account types
    Then appropriate conversion rates should display for each account
    And rate updates should continue to work correctly

Screenshots/Recordings

Before

After

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.

Note

Removes RatesController across Engine, messengers, selectors, and types; adds migration 106 to purge its persisted state; updates tests and fixtures.

  • Engine/Core:
    • Remove RatesController initialization, context exposure, polling, and state-change subscriptions in Engine.ts and constants.ts.
    • Drop rates-controller-init and its tests; delete rates-controller-messenger and references in messengers/index.ts.
  • Selectors:
    • Remove selectMultichainConversionRate and selectMultichainCoinRates; update multichain.ts and related tests accordingly.
  • Types:
    • Strip RatesController types and entries from types.ts (controllers, state, actions/events, controller names).
  • Migration:
    • Add migration 106 to remove backgroundState.RatesController; register in migrations/index.ts with tests.
  • Tests/Fixtures:
    • Update snapshots and initial-background-state.json to omit RatesController.
    • Adjust Engine.test.ts assertions to no longer expect RatesController.

Written by Cursor Bugbot for commit da413b8. This will update automatically on new commits. Configure here.

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@github-actions github-actions bot added size-L and removed size-M labels Oct 31, 2025
@salimtb salimtb marked this pull request as ready for review October 31, 2025 11:07
@salimtb salimtb requested a review from a team as a code owner October 31, 2025 11:07
@sahar-fehri
Copy link
Contributor

sahar-fehri commented Oct 31, 2025

I think we should also remove useCurrencyRatePolling(chainParams) from the PollingProvide?

@salimtb
Copy link
Contributor Author

salimtb commented Oct 31, 2025

Should we also remove useCurrencyRatePolling(chainParams) from the PollingProvide?

i think this one is needed for evm since it's using the CurrencyRatesController and not RatesController

@salimtb salimtb changed the title fix: remove currency rates multichain fix: cp-7.58.0 remove currency rates multichain Oct 31, 2025
@sonarqubecloud
Copy link

@salimtb salimtb changed the title fix: cp-7.58.0 remove currency rates multichain fix: cp-7.57.2 remove currency rates multichain Oct 31, 2025
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@Cal-L Cal-L added this pull request to the merge queue Oct 31, 2025
Merged via the queue into main with commit 43c399b Oct 31, 2025
150 of 152 checks passed
@Cal-L Cal-L deleted the fix/remove-currency-rates branch October 31, 2025 17:28
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2025
@metamaskbot metamaskbot added the release-7.59.0 Issue or pull request that will be included in release 7.59.0 label Oct 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.59.0 Issue or pull request that will be included in release 7.59.0 size-L team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants