fix: balance double counting pnl#41796
Merged
Merged
Conversation
Contributor
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/perps (3 files, +33 -10)
|
Contributor
Builds ready [aea5412]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
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 a0351ea. Configure here.
Contributor
Builds ready [bafc9b3]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
Contributor
Automated Review — PR #41796
SummaryThe PR correctly fixes a double-counting bug where Full review detailsRecipe Coverage
Overall recipe coverage: 1/3 ACs PROVEN
Prior Reviews
No CHANGES_REQUESTED reviews. Acceptance Criteria Validation
Code Quality
Fix Quality
Live Validation
Correctness
Static Analysis
Mobile Comparison
Architecture & Domain
Risk Assessment
Recommended ActionAPPROVE Line comments (1 comments: 1 suggestion)
Recipe (N/A){
"title": "Verify fix: balance not double-counting PnL",
"validate": {
"workflow": {
"pre_conditions": ["wallet.unlocked", "perps.feature_enabled"],
"entry": "setup-navigate-perps",
"nodes": {
"setup-navigate-perps": {
"action": "call",
"ref": "perps/navigate-perps-tab",
"next": "ac1-wait-balance-dropdown"
},
"ac1-wait-balance-dropdown": {
"action": "wait_for",
"test_id": "perps-balance-dropdown",
"timeout_ms": 10000,
"next": "ac1-eval-balance-state"
},
"ac1-eval-balance-state": {
"action": "eval_ref",
"ref": "perps-balance",
"save_as": "account_state",
"assert": {
"operator": "not_null",
"field": "totalBalance"
},
"next": "ac1-screenshot-balance-dropdown"
},
"ac1-screenshot-balance-dropdown": {
"action": "screenshot",
"filename": "evidence-ac1-perps-balance-dropdown.png",
"next": "done"
},
"done": {
"action": "end",
"status": "pass",
"message": "AC1: Balance dropdown displays totalBalance directly (no double-counting PnL). AC2: UNTESTABLE — PerpsMarketBalanceActions not rendered in any live route, code review only. AC3: UNTESTABLE — loading skeleton is transient hook state, covered by unit tests."
}
}
}
}
}No video evidence recorded. |
abretonc7s
approved these changes
Apr 16, 2026
abretonc7s
left a comment
Contributor
There was a problem hiding this comment.
Automated review — see comment above for full details.
abretonc7s
reviewed
Apr 16, 2026
| // Account value = totalBalance + unrealizedPnl (includes open position PnL) | ||
| const accountValue = parseFloat(totalBalance) + parseFloat(unrealizedPnl); | ||
| // totalBalance is HL accountValue (perps equity, already includes unrealizedPnl) + spot | ||
| const accountValue = parseFloat(totalBalance); |
Contributor
There was a problem hiding this comment.
suggestion — Suggestion: The existing test file for this component (perps-market-balance-actions.test.tsx) doesn't assert on the displayed balance amount. Consider adding a test similar to the dropdown's displays the formatted total balance from mock data test to guard against future regressions in this component's balance calculation.
Contributor
Visual EvidenceEvidence/baseline
Evidence/evidence Ac1 Perps Balance Dropdown
|
|
geositta
approved these changes
Apr 16, 2026
geositta
left a comment
Contributor
There was a problem hiding this comment.
The balance/PnL correction looks right and aligns with mobile/Core: totalBalance is already the account value, so removing the extra + unrealizedPnl fixes the inconsistency without changing the intended data model.
The reconnect and healthcheck additions look reasonable for the current architecture, and the focused tests plus circular dependency check are green.
Contributor
Builds ready [5ecb63c]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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
Fixes issue where PnL was being double counted in total balance
Changelog
CHANGELOG entry: fix perps total balance miscalculation
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Changes user-visible balance math in the perps UI and could misrepresent funds if the upstream
totalBalancesemantics differ; scope is limited to display logic with added test coverage.Overview
Fixes perps UI balance calculations by stopping the addition of
unrealizedPnlontototalBalance, aligning bothPerpsBalanceDropdownandPerpsMarketBalanceActionswith the updated meaning oftotalBalance.PerpsBalanceDropdownnow rendersPerpsControlBarSkeletonwhileusePerpsLiveAccountreportsisInitialLoading, and tests were updated to mock the hook, assert the corrected formatted total, and cover the loading state.Reviewed by Cursor Bugbot for commit 5ecb63c. Bugbot is set up for automated code reviews on this repo. Configure here.