Skip to content

chore(runway): cherry-pick fix: remove currency rates multichain#22022

Merged
Cal-L merged 1 commit into
release/7.57.2from
cherry-pick-7-57-2-43c399b
Oct 31, 2025
Merged

chore(runway): cherry-pick fix: remove currency rates multichain#22022
Cal-L merged 1 commit into
release/7.57.2from
cherry-pick-7-57-2-43c399b

Conversation

@runway-github

@runway-github runway-github Bot commented Oct 31, 2025

Copy link
Copy Markdown
Contributor

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 the legacy RatesController across Engine, selectors, types, and fixtures, and adds migration 106 to delete its persisted state.

  • Engine:
    • Remove RatesController creation, messenger setup, currency rate sync, and start() call in startPolling.
    • Drop RatesController from context, initialization, and state exposure.
  • Selectors:
    • Delete selectMultichainConversionRate and selectMultichainCoinRates from app/selectors/multichain/multichain.ts and related tests.
  • Store/Migrations:
    • Add migration 106 to remove backgroundState.RatesController with tests.
  • Types/Constants:
    • Remove RatesController types, actions/events, and state from app/core/Engine/types.ts and constants.ts.
  • Tests/Fixtures:
    • Update Engine.test.ts, multichain selector tests, snapshots, and initial-background-state.json to exclude RatesController.

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

43c399b

<!--

Please submit this PR as a draft initially.

Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.

-->

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

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.

- 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 entry: Removed redundant RatesController to optimize API usage

Fixes: <!-- Add issue number if applicable -->

```gherkin
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
```

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<!-- RatesController making redundant API calls alongside
MultichainAssetsRatesController -->

<!-- Single source of rate fetching through
MultichainAssetsRatesController, reducing API overhead -->

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I've included tests if applicable
- [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] 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.

<!-- CURSOR_SUMMARY -->
---

> [!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`.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
da413b8. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@runway-github runway-github Bot requested a review from a team as a code owner October 31, 2025 19:18
@github-actions

Copy link
Copy Markdown
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.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release/7.57.2@7e670ad). Learn more about missing BASE report.

Additional details and impacted files
@@                Coverage Diff                @@
##             release/7.57.2   #22022   +/-   ##
=================================================
  Coverage                  ?   76.78%           
=================================================
  Files                     ?     3445           
  Lines                     ?    85523           
  Branches                  ?    15951           
=================================================
  Hits                      ?    65669           
  Misses                    ?    15421           
  Partials                  ?     4433           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud

Copy link
Copy Markdown

@Cal-L Cal-L left a comment

Copy link
Copy Markdown
Contributor

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 merged commit c1d577f into release/7.57.2 Oct 31, 2025
154 of 161 checks passed
@Cal-L Cal-L deleted the cherry-pick-7-57-2-43c399b branch October 31, 2025 20:06
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 31, 2025
@metamaskbot metamaskbot added the release-7.58.0 Issue or pull request that will be included in release 7.58.0 label Nov 5, 2025
@metamaskbot

Copy link
Copy Markdown
Collaborator

No release label on PR. Adding release label release-7.58.0 on PR, as PR was cherry-picked in branch 7.58.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.58.0 Issue or pull request that will be included in release 7.58.0 size-M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants