fix(multichain): use accounts{Added,Removed} to fetch/clear balances #25884

ccharly · 2024-07-17T10:04:34Z

Description

Fixing the balances fetching that happens a bit too frequently.

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/527

Manual testing steps

Use yarn start:flask
Enable Bitcoin support: Settings > Experimental > Bitcoin toggles
Create Bitcoin accounts (mainnet, testnet or both)
You should see your balances as usual

Extra steps:

You can check your Network tabs on your dev-tools and look for blockchair requests
You should see 1 request (for each Bitcoin accounts) upon account creation (to fetch the initial balance)
You should NOT see any new requests when you're adding new Ethereum/HW accounts or when selecting a new accounts (basically any changes to the AccountsController should not trigger new fetches)
You MIGHT see new requests after some time (around 10min, which is the average block time of Bitcoin), to refresh the balance

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-17T10:04:47Z

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.

…ultichainAccounts callback

metamaskbot · 2024-07-18T09:23:33Z

Builds ready [bd3dda9]

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 (299 ± 256 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	292	108	46	22
		domContentLoaded	10	48	23	10	5
		load	44	1641	299	532	256
		domInteractive	10	48	23	10	5

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

background: 2.42 KiB (0.07%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2024-07-18T09:29:38Z

Builds ready [bd3dda9]

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 (299 ± 256 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	292	108	46	22
		domContentLoaded	10	48	23	10	5
		load	44	1641	299	532	256
		domInteractive	10	48	23	10	5

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

background: 2.42 KiB (0.07%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

…cesController

…ntroller logic

codecov · 2024-07-18T11:43:38Z

Codecov Report

Attention: Patch coverage is 88.60759% with 9 lines in your changes missing coverage. Please review.

Project coverage is 69.69%. Comparing base (2d416bf) to head (9637c8e).
Report is 92 commits behind head on develop.

Files	Patch %	Lines
app/scripts/lib/accounts/BalancesController.ts	87.80%	5 Missing ⚠️
ui/store/actions.ts	0.00%	2 Missing ⚠️
app/scripts/lib/accounts/BalancesTracker.ts	96.88%	1 Missing ⚠️
...ltichain/create-btc-account/create-btc-account.tsx	66.67%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25884      +/-   ##
===========================================
- Coverage    69.77%   69.69%   -0.09%     
===========================================
  Files         1398     1401       +3     
  Lines        49165    49577     +412     
  Branches     13574    13701     +127     
===========================================
+ Hits         34304    34548     +244     
- Misses       14861    15029     +168

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

metamaskbot · 2024-07-18T11:54:44Z

Builds ready [7008a68]

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 (63 ± 8 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	66	131	92	19	9
		domContentLoaded	8	56	27	16	8
		load	37	105	63	18	8
		domInteractive	8	56	27	16	8

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

background: 2.58 KiB (0.08%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

…hen fetching balances

…eBtcAccount)

…nces

metamaskbot · 2024-07-18T17:30:51Z

Builds ready [1e805b2]

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 (222 ± 228 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	128	93	17	8
		domContentLoaded	9	53	26	13	6
		load	48	1723	222	475	228
		domInteractive	9	53	26	13	6

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

background: 2.66 KiB (0.08%)
ui: 41 Bytes (0.00%)
common: 240 Bytes (0.00%)

app/scripts/lib/accounts/BalancesController.ts

app/scripts/lib/accounts/BalancesTracker.ts

app/scripts/lib/accounts/BalancesController.ts

…time)

…ce after account creation

sonarcloud · 2024-07-19T11:53:48Z

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-07-19T12:19:35Z

Builds ready [9637c8e]

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 (174 ± 194 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	67	157	112	30	15
		domContentLoaded	10	74	31	19	9
		load	44	1933	174	404	194
		domInteractive	10	74	31	19	9

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

background: 2.76 KiB (0.08%)
ui: 117 Bytes (0.00%)
common: 240 Bytes (0.00%)

metamaskbot · 2024-08-30T12:17:43Z

Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.3.0), as PR was cherry-picked in branch 12.2.0.

fix(multichain): use accounts{Added,Removed} to fetch/clear balances

262616b

metamaskbot added the team-accounts label Jul 17, 2024

ccharly force-pushed the fix/prevent-too-many-fethes-in-BalancesController branch 2 times, most recently from 758e8a8 to b4189dc Compare July 18, 2024 08:13

ccharly added 5 commits July 18, 2024 10:42

feat(multichain): now tracks non-EVM balances to relax excessive fetches

9e959df

test(multichain): fix + add more tests for BalancesController

17a2e96

test(multichain): add tests for BalancesTracker

00cc6be

chore: lint

d9c01bb

refactor(multichain): use action rather than AccountsController.listM…

bd3dda9

…ultichainAccounts callback

ccharly force-pushed the fix/prevent-too-many-fethes-in-BalancesController branch from b4189dc to bd3dda9 Compare July 18, 2024 08:43

ccharly added 3 commits July 18, 2024 13:14

refactor(multichain): add public BalancesController.updateBalance method

ad712af

fix(multichain): only tracks non-EVM accounts when constructing Balan…

c8e21df

…cesController

test(multichain): fix metamask-controller.test.js with new BalancesCo…

7008a68

…ntroller logic

ccharly marked this pull request as ready for review July 18, 2024 11:57

ccharly requested a review from a team as a code owner July 18, 2024 11:57

ccharly added 5 commits July 18, 2024 16:40

fix(multichain): do not subscribe to AccountsController:stateChange w…

06c946b

…hen fetching balances

feat(multichain): force fetch balance upon account creation (in Creat…

502872e

…eBtcAccount)

feat(multichain): update fetch info when fetching single account bala…

f973430

…nces

fix(multichain): fix BalancesController initial state

a3bd765

fix(multichain): fix btc balances tests

1e805b2

ccharly force-pushed the fix/prevent-too-many-fethes-in-BalancesController branch from 2739b48 to 1e805b2 Compare July 18, 2024 16:54

danroc reviewed Jul 19, 2024

View reviewed changes

app/scripts/lib/accounts/BalancesController.ts Outdated Show resolved Hide resolved

danroc reviewed Jul 19, 2024

View reviewed changes

app/scripts/lib/accounts/BalancesTracker.ts Outdated Show resolved Hide resolved

danroc reviewed Jul 19, 2024

View reviewed changes

app/scripts/lib/accounts/BalancesController.ts Outdated Show resolved Hide resolved

ccharly added 5 commits July 19, 2024 13:10

feat(multichain): add BALANCES_UPDATE_TIME (lower than average block …

ecd1b6a

…time)

refactor(multichain): use Promise.allSettled when fecthing all balances

175abe5

refactor(multichain): use named function for BalancesTracker's callback

3dba8f7

test(multichain): remove use of it.only

5be1e46

refactor(btc): prevent error from bubbling up when fetching BTC balan…

9637c8e

…ce after account creation

danroc approved these changes Jul 19, 2024

View reviewed changes

montelaidev approved these changes Jul 19, 2024

View reviewed changes

ccharly merged commit 8574f94 into develop Jul 19, 2024
85 checks passed

ccharly deleted the fix/prevent-too-many-fethes-in-BalancesController branch July 19, 2024 14:02

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

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

metamaskbot added release-12.2.0 Issue or pull request that will be included in release 12.2.0 and removed release-12.3.0 Issue or pull request that will be included in release 12.3.0 labels Aug 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(multichain): use accounts{Added,Removed} to fetch/clear balances #25884

fix(multichain): use accounts{Added,Removed} to fetch/clear balances #25884

ccharly commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024

metamaskbot commented Jul 18, 2024

metamaskbot commented Jul 18, 2024

codecov bot commented Jul 18, 2024 •

edited

Loading

metamaskbot commented Jul 18, 2024

metamaskbot commented Jul 18, 2024

sonarcloud bot commented Jul 19, 2024

metamaskbot commented Jul 19, 2024

metamaskbot commented Aug 30, 2024

fix(multichain): use accounts{Added,Removed} to fetch/clear balances #25884

fix(multichain): use accounts{Added,Removed} to fetch/clear balances #25884

Conversation

ccharly commented Jul 17, 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 17, 2024

metamaskbot commented Jul 18, 2024

metamaskbot commented Jul 18, 2024

codecov bot commented Jul 18, 2024 • edited Loading

Codecov Report

metamaskbot commented Jul 18, 2024

metamaskbot commented Jul 18, 2024

sonarcloud bot commented Jul 19, 2024

Quality Gate passed

metamaskbot commented Jul 19, 2024

metamaskbot commented Aug 30, 2024

ccharly commented Jul 17, 2024 •

edited

Loading

codecov bot commented Jul 18, 2024 •

edited

Loading