Skip to content

Commit 407d4c2

Browse files
chore(runway): cherry-pick fix(perps): calculate weighted ROE percentage for aggregated account states cp-7.60.0 (#23090)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR fixes the calculation of return on equity (ROE) percentage for aggregated account states across multiple DEX accounts in the Perps feature. Previously, the ROE was calculated using a simple formula that didn't account for the different margin amounts across accounts, which could lead to inaccurate percentage values. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Fixed return on equity percentage calculation for aggregated Perps positions across multiple DEX accounts ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2086 ## **Manual testing steps** ```gherkin Feature: Perps aggregated account ROE calculation Scenario: user views aggregated account state with multiple DEX positions Given user has multiple Perps positions across different DEX accounts with varying margin amounts And each position has different unrealized PnL and ROE values When user views the aggregated account state Then the displayed ROE percentage should be a weighted average based on margin used And the calculation should correctly handle accounts with zero or negative values And invalid or NaN values should be skipped in the calculation ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ![Screenshot_20251121_104805_MetaMask (1)](https://github.com/user-attachments/assets/17e6528f-981d-4b36-a83b-4fa43762645d) ### **After** <!-- [screenshots/recordings] --> <img width="1170" height="2532" alt="Simulator Screenshot - iPhone 16e - 2025-11-21 at 11 23 26" src="https://github.com/user-attachments/assets/23a33e94-e8b2-4b63-b57a-031fc5276653" /> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Replaces aggregate ROE with a margin-weighted calculation and adds comprehensive unit tests. > > - **Perps utils**: > - **Weighted ROE**: Add `calculateWeightedReturnOnEquity` (and `ReturnOnEquityInput`) in `app/components/UI/Perps/utils/accountUtils.ts` to compute margin-weighted ROE across accounts. > - Integrate weighted ROE in `HyperLiquidSubscriptionService.aggregateAccountStates()` by using `calculateWeightedReturnOnEquity` instead of the previous simple ratio. > - **Tests**: > - Add extensive unit tests in `app/components/UI/Perps/utils/accountUtils.test.ts` covering mixed types, edge cases (zero/negative/NaN), precision, and multi-account scenarios. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 334fcd6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 394dcb5 commit 407d4c2

File tree

3 files changed

+443
-8
lines changed

3 files changed

+443
-8
lines changed

app/components/UI/Perps/services/HyperLiquidSubscriptionService.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
adaptAccountStateFromSDK,
3232
parseAssetName,
3333
} from '../utils/hyperLiquidAdapter';
34+
import { calculateWeightedReturnOnEquity } from '../utils/accountUtils';
3435
import type { HyperLiquidClientService } from './HyperLiquidClientService';
3536
import type { HyperLiquidWalletService } from './HyperLiquidWalletService';
3637
import type { CaipAccountId } from '@metamask/utils';
@@ -476,6 +477,12 @@ export class HyperLiquidSubscriptionService {
476477
let totalMarginUsed = 0;
477478
let totalUnrealizedPnl = 0;
478479

480+
// Collect account states for weighted ROE calculation
481+
const accountStatesForROE: {
482+
unrealizedPnl: string;
483+
returnOnEquity: string;
484+
}[] = [];
485+
479486
// Aggregate all cached account states
480487
Array.from(this.dexAccountCache.entries()).forEach(
481488
([currentDex, state]) => {
@@ -488,20 +495,21 @@ export class HyperLiquidSubscriptionService {
488495
totalBalance += parseFloat(state.totalBalance);
489496
totalMarginUsed += parseFloat(state.marginUsed);
490497
totalUnrealizedPnl += parseFloat(state.unrealizedPnl);
498+
499+
// Collect data for weighted ROE calculation
500+
accountStatesForROE.push({
501+
unrealizedPnl: state.unrealizedPnl,
502+
returnOnEquity: state.returnOnEquity,
503+
});
491504
},
492505
);
493506

494507
// Use first DEX's account state as base and override aggregated values
495508
const firstDexAccount =
496509
this.dexAccountCache.values().next().value || ({} as AccountState);
497510

498-
// Calculate returnOnEquity across all DEXs (same formula as HyperLiquidProvider.getAccountState)
499-
let returnOnEquity = '0';
500-
if (totalMarginUsed > 0) {
501-
returnOnEquity = ((totalUnrealizedPnl / totalMarginUsed) * 100).toFixed(
502-
1,
503-
);
504-
}
511+
// Calculate weighted returnOnEquity across all DEXs
512+
const returnOnEquity = calculateWeightedReturnOnEquity(accountStatesForROE);
505513

506514
return {
507515
...firstDexAccount,

0 commit comments

Comments
 (0)