MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.66k stars 4.78k forks source link

[Bug]: Fiat values continue to show despite "Show conversion on test networks" (Advanced Settings) being disabled #24945

Open digiwand opened 1 month ago

digiwand commented 1 month ago

Describe the bug

Fiat values continue to show despite "Show conversion on test networks" (Advanced Settings) being disabled

Discovered in https://github.com/MetaMask/metamask-extension/pull/24854#issuecomment-2136464701

Expected behavior

Fiat conversion values should not show on test networks when "Show conversion on test networks" (Advanced Settings) being disabled

Screenshots/Recordings

334642377-bd2eec03-02da-4f56-a2b0-5a3f39abbef9

Steps to reproduce

  1. Turn off "Show conversion on test networks" setting in Advanced Settings
  2. Use L2 test network
  3. Go to test dapp
  4. Click on "Create a token"
  5. Observe fiat value is shown

Error messages or log output

No response

Version

11.18.0

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response

sleepytanya commented 1 month ago

Develop, Settings->Advanced->Show conversion on test networks disabled - for certain test networks fat values are shown. Sepolia, BNB Chain testnets - no conversion:

Screenshot 2024-06-01 at 14 19 15

Arbitrum Sepolia, Polygon Amoy, Avalanche Fuji - fiat values displayed:

Screenshot 2024-06-01 at 14 16 27
digiwand commented 1 month ago

thanks @sleepytanya for the screenshots and additional cases above!

I've tested and also confirmed this issue exists for native testnets.

It appears a few places on confirmation pages rely only on the useCurrencyRateCheck (Security & Privacy > "Show balance and token price checker" setting value) where we display fiat values. However, we are expecting the display of the fiat value also to include showFiatInTestnets (Advanced > "Show conversion on test networks" setting value) in the logic.

I believe we'll want to use the getShouldShowFiat for these instances which takes both the useCurrencyRateCheck and showFiatInTestnets checks, along with other conditions.

export function getShouldShowFiat(state) {
  const isMainNet = getIsMainnet(state);
  const isLineaMainNet = getIsLineaMainnet(state);
  const isCustomNetwork = getIsCustomNetwork(state);
  const conversionRate = getConversionRate(state);
  const useCurrencyRateCheck = getUseCurrencyRateCheck(state);
  const { showFiatInTestnets } = getPreferences(state);
  return Boolean(
    (isMainNet || isLineaMainNet || isCustomNetwork || showFiatInTestnets) &&
      useCurrencyRateCheck &&
      conversionRate,
  );
}
digiwand commented 1 month ago

unassigning myself to refine and reprioritize the issue