refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088

digiwand · 2024-09-12T06:01:43Z

Description

move fetchErc20Decimals method to utils/token
use fetchErc20Decimals for other instances calling getTokenStandardAndDetails. This will set decimals to 18 if no decimals were retrieved in the response. Previously, it used 0 instead of 18 in these cases.
memoize method to avoid making additional requests for the same token - performance improvement
create tests for memorized fetchErc20Decimals method

Related issues

Fixes: #24374

Manual testing steps

Test token values and fiat conversions on confirmation pages

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-09-12T06:01:56Z

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.

…emoize

- we currently have a patch with custom non-hex string

…emoize

metamaskbot · 2024-09-12T20:56:38Z

Builds ready [dba0ba3]

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, 10, 11, 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 (1681 ± 89 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	177	2100	1616	376	180
		domContentLoaded	1399	2090	1659	178	85
		load	1440	2102	1681	185	89
		domInteractive	23	57	32	10	5

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 1.07 KiB (0.01%)
common: 0 Bytes (0.00%)

…emoize

metamaskbot · 2024-09-13T12:25:14Z

Builds ready [5602fd9]

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, 10, 11, 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 (1702 ± 110 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	220	2281	1611	385	185
		domContentLoaded	1444	2202	1687	221	106
		load	1452	2288	1702	230	110
		domInteractive	14	71	33	14	7

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 1.07 KiB (0.01%)
common: 0 Bytes (0.00%)

codecov · 2024-09-13T12:25:23Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.95%. Comparing base (9459903) to head (a7c315d).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #27088      +/-   ##
===========================================
- Coverage    69.95%   69.95%   -0.00%     
===========================================
  Files         1441     1443       +2     
  Lines        50099    50095       -4     
  Branches     14007    14005       -2     
===========================================
- Hits         35046    35043       -3     
+ Misses       15053    15052       -1

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

matthewwalsh0 · 2024-09-14T19:52:09Z

ui/pages/confirmations/constants/token.ts

Minor, given there is very little in index.ts currently, do we warrant this level of modularity?

questionable, so updated this to share ui/pages/confirmations/utils/token.ts

…emoize

metamaskbot · 2024-09-16T14:17:13Z

Builds ready [a7c315d]

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, 10, 11, 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 (1545 ± 75 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1346	1975	1556	159	76
		domContentLoaded	1339	1922	1529	148	71
		load	1347	1931	1545	156	75
		domInteractive	14	175	40	36	17

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 491 Bytes (0.01%)
common: 0 Bytes (0.00%)

This reverts commit 4f8e366.

digiwand · 2024-09-18T10:27:20Z

oops, accidentally merged a commit. Reverted after. May I get a reapproval please? @matthewwalsh0

sonarcloud · 2024-09-18T10:28:52Z

Quality Gate passed

Issues
0 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-09-18T10:58:55Z

Builds ready [99cfb20]

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, 10, 11, 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 (1895 ± 107 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	241	2640	1501	734	353
		domContentLoaded	1613	2613	1866	224	107
		load	1626	2620	1895	222	107
		domInteractive	19	60	38	10	5

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 491 Bytes (0.01%)
common: 0 Bytes (0.00%)

jpuri

👍

refactor: mv fetchErc20Decimals to utils/token

b8fec4a

github-actions bot added the team-confirmations Push issues to confirmations team label Sep 12, 2024

digiwand added 6 commits September 12, 2024 10:27

fix: lint needs EOL

d9913ab

refactor: use fetchErc20Decimals in permit simulation and dataTree

171c4a1

refactor: memoize fetchErc20Decimals

eff0525

fix: lint add EOF line

a83c8af

test: memoized fetchErc20Decimals util

557d8a4

test:fix: fetchErc20Decimals test phrasing

fb228be

digiwand changed the title ~~refactor: mv fetchErc20Decimals to utils/token~~ refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances Sep 12, 2024

digiwand added 5 commits September 12, 2024 17:34

fix:test: uncomment needed afterEach code

282a8a6

fix: yarn lint:fix

6f9b3c3

fix:test: useBalanceChanges memoized fetchErc20Decimals

68fa1c5

fix:test: mv useBalanceChanges memoized clear cache

7c4afaf

Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…

d07cc85

…emoize

digiwand marked this pull request as ready for review September 12, 2024 13:58

digiwand requested a review from a team as a code owner September 12, 2024 13:58

digiwand added 4 commits September 12, 2024 20:16

fix: allow string with hex token addresses

595975e

- we currently have a patch with custom non-hex string

fix:test: memoize fetchErc20Decimals for confirm test

6ab6f3b

fix: lint utils/token comment

09a9fee

Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…

dba0ba3

…emoize

Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…

5602fd9

…emoize

matthewwalsh0 previously approved these changes Sep 14, 2024

View reviewed changes

refactor: rm new utils/token.ts

913a79a

digiwand dismissed matthewwalsh0’s stale review via 913a79a September 16, 2024 09:57

Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…

a7c315d

…emoize

digiwand requested review from matthewwalsh0 and jpuri September 16, 2024 15:23

matthewwalsh0 previously approved these changes Sep 17, 2024

View reviewed changes

fix: cherry-pick PermitSimulationValueDisplay code

4f8e366

digiwand dismissed matthewwalsh0’s stale review via 4f8e366 September 18, 2024 10:01

Revert "fix: cherry-pick PermitSimulationValueDisplay code"

99cfb20

This reverts commit 4f8e366.

jpuri approved these changes Sep 18, 2024

View reviewed changes

digiwand requested a review from matthewwalsh0 September 19, 2024 07:53

matthewwalsh0 approved these changes Sep 19, 2024

View reviewed changes

digiwand merged commit 88664bb into develop Sep 19, 2024
78 checks passed

digiwand deleted the refactor-token-decimals-fetch-logic-and-memoize branch September 19, 2024 07:55

github-actions bot locked and limited conversation to collaborators Sep 19, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 19, 2024

metamaskbot added release-12.5.0 Issue or pull request that will be included in release 12.5.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088

refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088

digiwand commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024

metamaskbot commented Sep 12, 2024

metamaskbot commented Sep 13, 2024

codecov bot commented Sep 13, 2024 •

edited

Loading

matthewwalsh0 Sep 14, 2024

digiwand Sep 16, 2024

metamaskbot commented Sep 16, 2024

digiwand commented Sep 18, 2024

sonarcloud bot commented Sep 18, 2024

metamaskbot commented Sep 18, 2024

jpuri left a comment

refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088

refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088

Conversation

digiwand commented Sep 12, 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 Sep 12, 2024

metamaskbot commented Sep 12, 2024

metamaskbot commented Sep 13, 2024

codecov bot commented Sep 13, 2024 • edited Loading

Codecov Report

matthewwalsh0 Sep 14, 2024

Choose a reason for hiding this comment

digiwand Sep 16, 2024

Choose a reason for hiding this comment

metamaskbot commented Sep 16, 2024

digiwand commented Sep 18, 2024

sonarcloud bot commented Sep 18, 2024

Quality Gate passed

metamaskbot commented Sep 18, 2024

jpuri left a comment

Choose a reason for hiding this comment

digiwand commented Sep 12, 2024 •

edited

Loading

codecov bot commented Sep 13, 2024 •

edited

Loading