fix(multichain): fix showFiat option for test assets #26224

ccharly · 2024-07-30T12:04:23Z

Description

Since the introduction of the new Bitcoin accounts in the account-list-menu, we introduced a regression regarding the "Show conversion in testnets" flag (for both Bitcoin and Ethereum accounts).

This PR fixes that.

Related issues

Fixes:

https://github.com/MetaMask/accounts-planning/issues/545

Manual testing steps

yarn start:flask
Enable Bitcoin support (Settings > Experimental > Enable "Add a new Bitcoin*" flags
Creates mainnet + testnet Bitcoin accounts
Check that the "Show conversion in tesnets" flag is off
Open the account menu list (click the header top bar, on the currently selected account)
You should not see any USD prices except for mainnet accounts (you must be on a mainnet EVM networks to see that for your EVM account)
Enables "Show conversion in tesnets" flag
Repeat step 5
You should now see USD prices also for testnet accounts/test assets

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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.

github-actions · 2024-07-30T12:04:40Z

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.

…-item balance displays

metamaskbot · 2024-07-31T10:57:08Z

Builds ready [3631381]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (199 ± 202 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	124	89	19	9
		domContentLoaded	9	55	21	14	7
		load	40	1467	199	420	202
		domInteractive	9	55	21	15	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -190 Bytes (-0.00%)
common: 410 Bytes (0.01%)

…st-item

…fix tests balances and snapshots

…al + fix snapshots

…al and account-list-item tests

…eraction state

…ccount state + snapshot

…r advanced-tab

codecov · 2024-07-31T17:30:58Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.96%. Comparing base (cd30d78) to head (24e70a6).
Report is 27 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26224      +/-   ##
===========================================
+ Coverage    69.96%   69.96%   +0.01%     
===========================================
  Files         1410     1411       +1     
  Lines        49915    49990      +75     
  Branches     13802    13806       +4     
===========================================
+ Hits         34919    34975      +56     
- Misses       14996    15015      +19

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

metamaskbot · 2024-07-31T17:43:00Z

Builds ready [bdbdc12]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (173 ± 173 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	284	126	47	22
		domContentLoaded	10	139	36	31	15
		load	41	1737	173	361	173
		domInteractive	10	139	36	31	15

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -117 Bytes (-0.00%)
common: 356 Bytes (0.00%)

…ouldShowFiat

metamaskbot · 2024-07-31T18:49:16Z

Builds ready [05dacfc]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (205 ± 200 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	63	323	109	53	25
		domContentLoaded	9	66	20	14	7
		load	41	1486	205	417	200
		domInteractive	9	66	20	14	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -117 Bytes (-0.00%)
common: 452 Bytes (0.01%)

ccharly · 2024-07-31T19:20:38Z

...ponents/multichain/connect-accounts-modal/__snapshots__/connect-accounts-modal.test.tsx.snap

@@ -354,17 +354,17 @@ exports[`Connect More Accounts Modal should render correctly 1`] = `
                      <div
                        class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
                        data-testid="first-currency-display"
-                        title="$0.00 USD"
+                        title="0 ETH"


I assume this snapshot was wrong for quite some time now.

The providerConfig on mock-state.json here is 0x5 (goerli), which is a testnet.

Test assets/accounts should not show any fiat values unless showFiatInTestnets is set to true (but it's currently false here)

You will see a bunch of snapshots changes in this PR, because of this ⬆️.

ccharly · 2024-07-31T19:35:20Z

ui/components/multichain/pages/connections/__snapshots__/connections.test.tsx.snap

@@ -293,17 +293,17 @@ exports[`Connections Content should render correctly 1`] = `
                    <div
                      class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
                      data-testid="first-currency-display"
-                      title="$880.18 USD"


I did not track why the fiat balance was actually this low in the original snapshot.

However, based on this account's balance, the actual balance should be:

0x346ba7725f412cbfdb # Hex (WEI) # Convert to decimal 966987986469506500000 # Decimal (WEI) # Convert to ETH 966.9879864695065 # Decimal (ETH)

So here again, I guess the snapshot was wrong too. Also, the same comment applies here, so no more fiat.

shared/constants/network.ts

danroc · 2024-08-01T08:55:12Z

I think we shouldn't truncate the unit, WDYT?

ccharly · 2024-08-01T09:01:35Z

I think we should not truncate no, but that's not something new, it seems to behave the same for Ethereum:

So I believe it should be fixed separately, is that ok for you?

ui/selectors/multichain.ts

Co-authored-by: Daniel Rocha <daniel.rocha@consensys.net>

sonarcloud · 2024-08-01T09:37:12Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-08-01T10:05:33Z

Builds ready [24e70a6]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (62 ± 6 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	65	121	92	14	7
		domContentLoaded	10	33	19	8	4
		load	42	84	62	12	6
		domInteractive	10	33	19	8	4

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -117 Bytes (-0.00%)
common: 296 Bytes (0.00%)

fix(multichain): fix showFiat option for test assets

1e52ba4

ccharly added type-bug team-accounts labels Jul 30, 2024

ccharly self-assigned this Jul 30, 2024

ccharly requested a review from a team as a code owner July 30, 2024 12:04

montelaidev mentioned this pull request Jul 30, 2024

Version v12.1.0 cherrypick 43b8031 #26158

Merged

7 tasks

ccharly added 5 commits July 31, 2024 12:23

feat(multichain): add getMultichainIsTestnet + use it for accout-list…

9fb4733

…-item balance displays

fix(multichain): use account in getMultichainIsMainnet

bb7f238

refactor(multichain): remove old comment

44b5e49

feat: add more missing TEST_NETWORK_IDS

3631381

chore: remove unused logs

693d24a

ccharly added 2 commits July 31, 2024 13:28

fix(multichain): checks for getMultichainShouldShowFiat in account-li…

3056b77

…st-item

fix(multichain): always use account.balance for EVM accounts

536ed9b

ccharly requested a review from a team as a code owner July 31, 2024 14:54

ccharly added 14 commits July 31, 2024 17:08

test(multichain): use mainnet provider config in account-list-item + …

0f41494

…fix tests balances and snapshots

test(multichain): use mainnet provider config in connect-accounts-mod…

33e365d

…al + fix snapshots

chore: lint

4eb1225

test: enable showFiatInTestnets in mock-state.json

7aa1c11

test: no longer uses mainnet provider config for connect-accounts-mod…

ea15201

…al and account-list-item tests

test: fix ui/components/multichain/pages/send state + snapshot

abacc02

test: fix ui/components/multichain/pages/connections snapshot

aefb465

test: fix ui/components/app/user-preferenced-currency-input state

09720cb

test: fix ui/components/app/user-preferenced-token-input state

00a120d

test: fix ui/components/ui/token-input state

8216bc2

test: fix ui/pages/confirmations/components/confirm/info/contract-int…

39db594

…eraction state

test: fix ui/pages/confirmations/confirm-send-ether state

da414d1

test: fix ui/pages/confirmations/confirmation/templates/remove-snap-a…

a248e9e

…ccount state + snapshot

test: add test for "Show conversion on testnets" + update snapshot fo…

ef336a0

…r advanced-tab

ccharly force-pushed the fix/show-fiat-for-test-assets branch from edfb49b to ef336a0 Compare July 31, 2024 15:10

ccharly added 2 commits July 31, 2024 19:09

test: fix e2e account-token-list fiat test

ed47113

test: revert back to showFiatInTestnet=false + re-update snapshots

bdbdc12

feat(multichain): adapt logic for mainnet/testnet for getMultichainSh…

05dacfc

…ouldShowFiat

ccharly commented Jul 31, 2024

View reviewed changes

danroc reviewed Aug 1, 2024

View reviewed changes

shared/constants/network.ts Outdated Show resolved Hide resolved

danroc reviewed Aug 1, 2024

View reviewed changes

ui/selectors/multichain.ts Outdated Show resolved Hide resolved

ccharly and others added 2 commits August 1, 2024 11:33

chore: typo in ui/selectors/multichain.ts

a18b2b9

Co-authored-by: Daniel Rocha <daniel.rocha@consensys.net>

refactor: revert newly added TEST_NETWORK_IDS

24e70a6

danroc approved these changes Aug 1, 2024

View reviewed changes

matthewwalsh0 approved these changes Aug 1, 2024

View reviewed changes

ccharly merged commit f3383ac into develop Aug 1, 2024
75 checks passed

ccharly deleted the fix/show-fiat-for-test-assets branch August 1, 2024 10:30

github-actions bot locked and limited conversation to collaborators Aug 1, 2024

metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 1, 2024

gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(multichain): fix showFiat option for test assets #26224

fix(multichain): fix showFiat option for test assets #26224

ccharly commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024

metamaskbot commented Jul 31, 2024

codecov bot commented Jul 31, 2024 •

edited

Loading

metamaskbot commented Jul 31, 2024

metamaskbot commented Jul 31, 2024

ccharly Jul 31, 2024

ccharly Jul 31, 2024

danroc commented Aug 1, 2024

ccharly commented Aug 1, 2024

sonarcloud bot commented Aug 1, 2024

metamaskbot commented Aug 1, 2024

fix(multichain): fix showFiat option for test assets #26224

fix(multichain): fix showFiat option for test assets #26224

Conversation

ccharly commented Jul 30, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 30, 2024

metamaskbot commented Jul 31, 2024

codecov bot commented Jul 31, 2024 • edited Loading

Codecov Report

metamaskbot commented Jul 31, 2024

metamaskbot commented Jul 31, 2024

ccharly Jul 31, 2024

Choose a reason for hiding this comment

ccharly Jul 31, 2024

Choose a reason for hiding this comment

danroc commented Aug 1, 2024

ccharly commented Aug 1, 2024

sonarcloud bot commented Aug 1, 2024

Quality Gate passed

metamaskbot commented Aug 1, 2024

ccharly commented Jul 30, 2024 •

edited

Loading

codecov bot commented Jul 31, 2024 •

edited

Loading