Skip to content

fix: cp-8.3.0 convert pay-flow token amounts to USD instead of relabeling#33321

Merged
OGPoyraz merged 3 commits into
mainfrom
ogp/mmpay-currency-fix-follow-up
Jul 15, 2026
Merged

fix: cp-8.3.0 convert pay-flow token amounts to USD instead of relabeling#33321
OGPoyraz merged 3 commits into
mainfrom
ogp/mmpay-currency-fix-follow-up

Conversation

@OGPoyraz

@OGPoyraz OGPoyraz commented Jul 15, 2026

Copy link
Copy Markdown
Member

Description

Follow-up to #32631.

That PR fixed the label on pay-flow token amounts. It didn't fix the value: asset.fiat.balance comes from the assets-controller in the user's preferred currency (e.g. EUR), and Intl.NumberFormat({ currency: 'USD' }) only changes the symbol — it doesn't convert. Result: EUR-preferred users saw their EUR magnitude next to a $ symbol.

This PR converts the value instead of relabeling it, and encapsulates the pay-currency policy in a new hook.

Changes

  • New useAssetFiatFormatter hook in hooks/pay/. Owns: reading pay currency, reading currency rates, re-scaling by (native→USD) / (native→preferred), Intl formatting. Returns undefined when the target currency can't be safely computed (missing rate, missing chain config).
  • useAccountTokens becomes currency-agnostic. Drops all pay-flow imports; calls useAssetFiatFormatter and forwards its output to balanceInSelectedCurrency. Public API and all 9 consumers unchanged.
  • Zero balances short-circuit the conversion (0 is currency-invariant) so the enrichment $0 placeholder always renders.

Changelog

CHANGELOG entry: Fixed a bug that caused pay-flow token amounts (Perps, Predict, Money Account, mUSD) to be shown with a USD symbol but the user's preferred-currency numeric value, when the user's preferred currency was not USD.

Related issues

Fixes: N/A (follow-up to #32631)
Refs: #32631

Manual testing steps

Feature: Pay-flow confirmations convert token amounts to USD, not just relabel

  Background:
    Given the user has set their preferred fiat currency to EUR

  Scenario: Pay With modal shows USD-converted numeric values
    Given the user opens a Perps / Predict / Money / mUSD conversion confirmation
    When the user opens the "Pay with" modal
    Then every row shows a "$" prefix
    And the number is the USD equivalent (not the EUR magnitude)

  Scenario: musdClaim keeps the user's preferred currency
    Given the user opens an mUSD claim confirmation
    Then token amounts render in EUR

  Scenario: Regular Send flow is unaffected
    Given the user opens the regular Send flow
    Then token amounts render in EUR

  Scenario: Preferred currency already USD
    Given the user has set their preferred currency to USD
    When the user opens any pay-flow confirmation
    Then amounts render exactly as before this PR

  Scenario: Missing USD rate for a chain
    Given a token on a chain whose usdConversionRate has not loaded
    When the user opens a pay-flow confirmation
    Then that row shows no fiat value (rather than a wrong-currency value)

Screenshots/Recordings

Before

N/A — visible symptom is the $ prefix on a preferred-currency numeric value. Device recording will be attached with QA build.

After

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-07-15.at.12.51.12.mov

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

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

Medium Risk
Changes how fiat amounts are computed and sorted in send/pay token lists; wrong rates or eligibility bugs could show incorrect USD values or hide balances, though missing-rate cases are explicitly guarded.

Overview
Fixes pay-flow token rows showing a USD symbol while still using the preferred-currency magnitude from asset.fiat.balance by computing EVM fiat as balance × rate (via useTokenFiatRates with the active display currency) instead of formatting the assets-controller fiat field directly.

Adds useAssetFiatFormatter, which picks pay-flow USD vs user preferred currency (through useTransactionPayCurrency) and handles Intl currency formatting only; rate selection stays in useTokenFiatRates.

useAccountTokens delegates display formatting to that hook, builds rate requests only for EVM hex-address assets (non-EVM still uses fiat.balance), sorts by the same derived fiat shown in the UI, and keeps $0 visible for zero balances when rates are missing while hiding non-zero rows without a rate.

Reviewed by Cursor Bugbot for commit 1ad7b83. Bugbot is set up for automated code reviews on this repo. Configure here.

@metamask-ci metamask-ci Bot added the team-confirmations Push issues to confirmations team label Jul 15, 2026
@metamask-ci

metamask-ci Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

…relabeling

Follow-up to #32631. `useAccountTokens` was formatting `asset.fiat.balance`
with `currency: 'USD'` while the numeric balance still came from the
assets-controller selector computed with the user's preferred fiat rate. For
non-USD preferences, values kept their EUR/GBP/etc. magnitude but rendered
with a $ symbol.

Extract the pay-currency policy into a new `useAssetFiatFormatter` hook in
`hooks/pay/`. It owns the rate lookup, the (native->USD) / (native->preferred)
re-scaling, the Intl formatting, and the missing-rate fallback.
`useAccountTokens` becomes currency-agnostic and just calls the formatter.

Hides fiat (returns undefined) rather than mislabeling when either
conversion rate is unavailable, matching the existing testnet-hidden
fallback. Preserves identity when preferred currency is already USD.
@OGPoyraz OGPoyraz force-pushed the ogp/mmpay-currency-fix-follow-up branch from 0aee094 to 458c305 Compare July 15, 2026 09:50
@github-actions github-actions Bot added size-L and removed size-M labels Jul 15, 2026
@OGPoyraz OGPoyraz marked this pull request as ready for review July 15, 2026 09:55
@OGPoyraz OGPoyraz requested a review from a team as a code owner July 15, 2026 09:55
@OGPoyraz OGPoyraz changed the title fix(confirmations): convert pay-flow token amounts to USD instead of relabeling fix: cp-8.3.0 convert pay-flow token amounts to USD instead of relabeling Jul 15, 2026
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jul 15, 2026
@matthewwalsh0 matthewwalsh0 self-requested a review July 15, 2026 11:04

payAmount =
preferredRate && usdRate
? preferredAmount.multipliedBy(usdRate).dividedBy(preferredRate)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have this logic in the pay controller, and elsewhere in the UI.

I recall assets used to have balanceUsd which would have helped here.

But could we at least re-use useTokenFiatRates which was my attempt to define this rate derivation once?

Plus crucially it factors USD stable-coins which we don't want to convert at all.

@OGPoyraz OGPoyraz Jul 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the heads up Matthew

Unfortunately balanceUsd is not a property for asset entity anymore.
Screenshot 2026-07-15 at 14 33 43

But I leverage using useTokenFiatRates for locking down the pay with token list with USD.

Here is the recording for latest version, settled EUR for preferred currency, send flow has no regression on EVM + nonEVM tokens and kept EUR meanwhile pay with token list locked to USD:

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-07-15.at.15.10.09.mov

@OGPoyraz OGPoyraz requested a review from matthewwalsh0 July 15, 2026 12:15

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 98e590d. Configure here.

Comment thread app/components/Views/confirmations/hooks/send/useAccountTokens.ts
Comment thread app/components/Views/confirmations/hooks/send/useAccountTokens.ts Outdated
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations, SmokeNetworkExpansion, SmokeMoney, SmokeStake, SmokePredictions, SmokeSwap
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR refactors useAccountTokens and introduces useAssetFiatFormatter:

  1. SmokeConfirmations: useAccountTokens is used in send flows (useSendTokens), pay flows (usePayTokenAccountBalance, useTransactionPayAvailableTokens), and custom-amount-info component. The fiat calculation logic changed from using asset.fiat.balance to balance * rate for EVM assets, which affects token balance display in send/pay confirmation screens. Solana SPL token handling was also updated with isEvmRateEligible.

  2. SmokeNetworkExpansion: The refactoring explicitly adds Solana-aware logic (isEvmRateEligible, isNonEvmChainId) to handle non-EVM assets differently. Solana token fiat display in confirmations is affected. Per tag description, Solana flows also require SmokeConfirmations.

  3. SmokeMoney: useMoneyEarnableTokens and useTransactionPayAvailableTokens use useAccountTokens, affecting the token selection in card/ramps flows.

  4. SmokeStake: useMusdConversionTokens (Earn) uses useAccountTokens, affecting lending/staking token display.

  5. SmokePredictions: usePredictDefaultPaymentToken uses useAccountTokens, affecting prediction market payment token selection.

  6. SmokeSwap: The pay flow hooks (useTransactionPayAvailableTokens, useTransactionPayMetrics) that use useAccountTokens are part of swap/bridge flows. Per tag descriptions, SmokeSwap also requires SmokeConfirmations.

The changes are a refactoring (not new features), but the fiat derivation logic change (balance × rate vs. stored fiat.balance) could cause subtle display differences in token lists across all these flows.

Performance Test Selection:
The changes are a refactoring of fiat formatting and token rate derivation hooks used in confirmation/send/pay flows. While these hooks are called during asset list rendering, the changes are internal logic refactoring (extracting formatting to a new hook, changing rate source) rather than changes to rendering architecture, data fetching patterns, or list virtualization. No performance-sensitive paths like app launch, onboarding, login, or asset loading lists are directly impacted in a way that would change measured performance metrics. No performance spec files were changed.

View GitHub Actions results

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts 0/99 0/172 0/366

AI-detected flaky patterns

app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts

  • J3 — Missing jest.resetAllMocks() — mock implementations bleed between tests (high)
    • jest.clearAllMocks() only clears call counts, instances, and results — it does NOT reset mock implementations set via mockImplementation() or mockReturnValue(). Several tests in this file (e.g. in the 'accountOverride', 'enrichTokenRequests', and 'edge cases' describe blocks) call mockUseSelector.mockImplementation(...) directly inside the test body. Because the beforeEach only calls clearAllMocks(), those custom implementations persist into subsequent tests that do not set their own mockUseSelector implementation. This causes order-dependent failures: a test that relies on the default mockUseSelector behaviour will silently use a previous test's implementation instead. The fix is to add jest.resetAllMocks() (which resets implementations) alongside or instead of clearAllMocks(), or to explicitly set a default mockUseSelector implementation in beforeEach so every test starts from a known state.
    • Suggested fix in app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts:
      -describe('useAccountTokens', () => {
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -    useTransactionAccountOverrideMock.mockReturnValue(undefined);
      -    mockSelectAssetsBySelectedAccountGroup.mockReturnValue(mockAssets as any);
      -    mockSelectCurrentCurrency.mockReturnValue('USD');
      -    // ...
      +describe('useAccountTokens', () => {
      +  beforeEach(() => {
      +    jest.resetAllMocks(); // resets implementations AND clears call counts
      +    useTransactionAccountOverrideMock.mockReturnValue(undefined);
      +    mockSelectAssetsBySelectedAccountGroup.mockReturnValue(mockAssets as any);
      +    mockSelectCurrentCurrency.mockReturnValue('USD');
      +    // ... (keep all other existing beforeEach setup unchanged)

This check is informational only and does not block merging.

@sonarqubecloud

Copy link
Copy Markdown

@OGPoyraz OGPoyraz enabled auto-merge July 15, 2026 14:02
@OGPoyraz OGPoyraz added this pull request to the merge queue Jul 15, 2026
Merged via the queue into main with commit 2ef73f3 Jul 15, 2026
278 of 280 checks passed
@OGPoyraz OGPoyraz deleted the ogp/mmpay-currency-fix-follow-up branch July 15, 2026 14:29
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 15, 2026
@metamask-ci metamask-ci Bot added the release-8.4.0 Issue or pull request that will be included in release 8.4.0 label Jul 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-8.4.0 Issue or pull request that will be included in release 8.4.0 risk:medium AI analysis: medium risk size-L team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants