feat: assets unify balance and traces - #44978
Merged
Merged
Conversation
…bled Use package balance helpers for unified account-group fiat totals, gate the account-group balance UI behind assetsUnifyState, and remove the local balance-utils polyfill. Co-authored-by: Cursor <cursoragent@cursor.com>
…acesEnabled Wire controller init through traceAsControllerCallback and only emit traces when assetsUnifyState is enabled with tracesEnabled true.
Contributor
|
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. |
salimtb
changed the title
Contributor
✨ Files requiring CODEOWNER review ✨💎 @MetaMask/metamask-assets (2 files, +40 -2)
|
Contributor
Builds ready [bf902e6]
⚡ Performance Benchmarks (Total: 🟢 15 pass · 🟡 7 warn · 🔴 2 fail)
Bundle size diffs
|
Contributor
Builds ready [1445496] [reused from bf902e6]
⚡ Performance Benchmarks (Total: 🟢 15 pass · 🟡 7 warn · 🔴 2 fail)
Bundle size diffs
|
|
Contributor
Builds ready [8c4904f] [reused from bf902e6]
⚡ Performance Benchmarks (Total: 🟢 15 pass · 🟡 7 warn · 🔴 2 fail)
Bundle size diffs
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 8c4904f. Configure here.
| ); | ||
| const selectedGroupBalance = isAssetsUnifyStateEnabled | ||
| ? unifiedSelectedGroupBalance | ||
| : legacySelectedGroupBalance; |
Contributor
There was a problem hiding this comment.
Dual balance selectors always both run
Medium Severity
AccountGroupBalance always subscribes to both selectBalanceBySelectedAccountGroup and selectUnifiedBalanceBySelectedAccountGroup, so both aggregations recompute on assets/account-tree updates even though only one result is used. selectBalanceBySelectedAccountGroup already switches on the unify flag via selectBalanceForAllWallets, so the unused path still pays for full all-wallets aggregation (or the unified per-group path) on every update.
Reviewed by Cursor Bugbot for commit 8c4904f. Configure here.
Contributor
Author
There was a problem hiding this comment.
this will be cleaned once the FF is removed
Contributor
There was a problem hiding this comment.
yeh... feature was shipped for some time now, maybe can scope out this clean up :)
Prithpal-Sooriya
approved these changes
Jul 29, 2026
metamaskbot
added
the
release-13.43.0
Contributor
🧪 Validation RunVerdict: Note Trial run of the MetaMask evidence skills — The callback handed to Ran to completion (exit 0) — read the output, no verdict assertedClaim under test: the Sentry trace path this PR adds is off by default and costs nothing while the flag is off $ bash -c B=f87e3bc8db833aea5da91ab48e3a90d702fbbb19; H=8c4904f63a66685b490f0fb015e2dc45e0497be6
F=app/scripts/messenger-client-init/assets/assets-controller-init.ts
git fetch -q --depth=1 origin $B
echo "== production files this PR touches (tests excluded) =="
git diff --stat $B $H -- "*.ts" "*.tsx" ":(exclude)*.test.*" ":(exclude)test/*"
echo
echo "== the trace callback handed to AssetsController =="
git show $H:$F | cat -n | sed -n 130,142p
echo
echo "== what that callback re-reads on every invocation =="
git show $H:$F | cat -n | sed -n 102,121p
echo
echo "== the one value this same file caches across calls =="
git show $H:$F | cat -n | sed -n 28p
git show $H:$F | cat -n | sed -n 150,161p
echo
echo "== every messenger read in this file, by line =="
git grep -nE "initMessenger[.]call[(]" $H -- $F
== production files this PR touches (tests excluded) ==
.../assets/assets-controller-init.ts | 63 +++++++
.../assets/assets-controller-messenger.ts | 5 +-
.../lib/assets-unify-state/remote-feature-flag.ts | 26 +++
.../account-group-balance.tsx | 13 +-
ui/selectors/assets-unify-state/feature-flags.ts | 1 +
ui/selectors/assets.balance-utils.ts | 189 ---------------------
ui/selectors/assets.ts | 147 ++++++++++++++--
7 files changed, 236 insertions(+), 208 deletions(-)
== the trace callback handed to AssetsController ==
130 function createAssetsControllerTrace(
131 initMessenger: AssetsControllerInitMessenger,
132 ): ControllerTraceCallback {
133 return <Result>(
134 req: ControllerTraceRequest,
135 fn?: (ctx?: ControllerTraceContext) => Result,
136 ): Promise<Result> => {
137 if (!isAssetsControllerTracesEnabled(initMessenger)) {
138 return Promise.resolve(fn?.() as Result);
139 }
140 return traceAsControllerCallback(req, fn);
141 };
142 }
== what that callback re-reads on every invocation ==
102 function isAssetsControllerTracesEnabled(
103 initMessenger: AssetsControllerInitMessenger,
104 ): boolean {
105 try {
106 if (!getIsAssetsUnifiedStateIncludedInBuild()) {
107 return false;
108 }
109 const { remoteFeatureFlags } = initMessenger.call(
110 'RemoteFeatureFlagController:getState',
111 );
112 return isAssetsUnifyStateTracesEnabled(
113 remoteFeatureFlags?.[ASSETS_UNIFY_STATE_FLAG] as
114 | AssetsUnifyStateFeatureFlag
115 | undefined,
116 ASSETS_UNIFY_STATE_VERSION_1,
117 );
118 } catch {
119 return false;
120 }
121 }
== the one value this same file caches across calls ==
28 let apiClient: AssetsControllerOptions['queryApiClient'] | null = null;
150 function getApiClient(
151 initMessenger: AssetsControllerInitMessenger,
152 ): AssetsControllerOptions['queryApiClient'] {
153 if (!apiClient) {
154 apiClient = createApiPlatformClient({
155 clientProduct: 'metamask-extension',
156 clientVersion: process.env.METAMASK_VERSION,
157 getBearerToken: () => safeGetBearerToken(initMessenger),
158 }) as unknown as AssetsControllerOptions['queryApiClient'];
159 }
160 return apiClient;
161 }
== every messenger read in this file, by line ==
8c4904f63a66685b490f0fb015e2dc45e0497be6:app/scripts/messenger-client-init/assets/assets-controller-init.ts:40: return await initMessenger.call('AuthenticationController:getBearerToken');
8c4904f63a66685b490f0fb015e2dc45e0497be6:app/scripts/messenger-client-init/assets/assets-controller-init.ts:56: const preferencesState = initMessenger.call(
8c4904f63a66685b490f0fb015e2dc45e0497be6:app/scripts/messenger-client-init/assets/assets-controller-init.ts:79: const { completedOnboarding } = initMessenger.call(
8c4904f63a66685b490f0fb015e2dc45e0497be6:app/scripts/messenger-client-init/assets/assets-controller-init.ts:85: const preferencesState = initMessenger.call(
8c4904f63a66685b490f0fb015e2dc45e0497be6:app/scripts/messenger-client-init/assets/assets-controller-init.ts:109: const { remoteFeatureFlags } = initMessenger.call(
8c4904f63a66685b490f0fb015e2dc45e0497be6:app/scripts/messenger-client-init/assets/assets-controller-init.ts:243: const { completedOnboarding } = initMessenger.call(Produced by Follows from the above
Open for review: how often does |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
we now add traces under a FF for the new assets controller , this will help to make some future improvements
Changelog
CHANGELOG entry: assets unify balance and traces
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Changes how displayed wallet and group fiat totals are computed when unify is enabled, including a temporary aggregation workaround; tracing is off by default but touches observability when enabled.
Overview
Adds remote-gated Sentry tracing for
AssetsControllervia a newassetsUnifyState.tracesEnabledflag. Init wires atracecallback that only callstraceAsControllerCallbackwhen unify is enabled for the build and the flag is on; otherwise work runs with no span. Init messenger access toRemoteFeatureFlagController:getStatesupports reading that flag.Unified fiat balances are refactored: local
assets.balance-utilsis removed in favor of@metamask/assets-controllercalculateBalanceForAllWallets/calculateBalanceChangeForAccountGroupwhen assets-unify-state is on. Aggregation still stripsassetsInfobefore totals (#44786). NewselectUnifiedBalanceBySelectedAccountGroupandgetUnifiedBalanceForAccountGroupfeed the selected-group path;AccountGroupBalancepicks unified vs legacy selectors fromgetIsAssetsUnifyStateEnabled.E2E registry and UI flag schema include
tracesEnabled(default off in prod defaults).Reviewed by Cursor Bugbot for commit 8c4904f. Bugbot is set up for automated code reviews on this repo. Configure here.