[pull] main from MetaMask:main#344
Merged
Merged
Conversation
…22231) ## **Description** This PR implements a simplified A/B testing framework for Perps to support TAT-1937 (Long/Short button color test). The framework leverages LaunchDarkly for user identification, variant assignment, and persistence, keeping our implementation minimal and focused on reading variants and applying them. **What is the reason for the change?** - We need to test which button colors (green/red vs white/white) drive better trading behavior - This is the first A/B test for Perps and establishes the framework for future tests - The test will help us understand if traditional colors or neutral colors lead to better metrics (trade initiation rate, execution rate, misclick rate) **What is the improvement/solution?** - Created lightweight A/B testing utilities in `app/components/UI/Perps/utils/abTesting/` - LaunchDarkly handles all complex logic (assignment, persistence, tracking) - Simple hook (`usePerpsABTest`) reads variant and applies button colors - Flat property pattern supports multiple concurrent AB tests (TAT-1937, TAT-1940, TAT-1827) - Dual tracking approach: screen views (baseline exposure) + button presses (engagement) to calculate engagement rate ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: TAT-1937 ## **Manual testing steps** ### Testing Control Variant (Production - LaunchDarkly) ```gherkin Feature: Button Color A/B Test - Control Variant Scenario: user sees traditional green/red buttons Given LaunchDarkly assigns user to "control" variant When user navigates to Perps home And user selects any asset (e.g., BTC) Then user sees asset details screen with green Long / red Short buttons And dev banner shows "control (LaunchDarkly)" ``` ### Testing Monochrome Variant (Production - LaunchDarkly) ```gherkin Feature: Button Color A/B Test - Monochrome Variant Scenario: user sees neutral white buttons Given LaunchDarkly assigns user to "monochrome" variant When user navigates to asset details screen Then user sees white Long / white Short buttons And dev banner shows "monochrome (LaunchDarkly)" ``` ### Testing Fallback Behavior ```gherkin Feature: Button Color A/B Test - Fallback Scenario: LaunchDarkly unavailable Given LaunchDarkly flag is disabled or unavailable When user navigates to asset details screen Then user sees control variant (green/red) as fallback And dev banner shows "control (Fallback)" ``` ### Quick Local Testing Instructions **For LaunchDarkly Testing (Default):** 1. Configure LaunchDarkly flag `perps-abtest-button-color` to return `"control"` or `"monochrome"` 2. App automatically uses the feature flag value 3. **Navigate to**: Perps � Select any asset (view asset details screen) 4. **Check dev banner**: Shows current variant and source (LaunchDarkly/Fallback) **For Local Dev Testing (Temporary Hardcode):** If you need to test a specific variant locally without LaunchDarkly: 1. Open `PerpsMarketDetailsView.tsx` 2. Temporarily hardcode the variant after the `usePerpsABTest` call: ```typescript const buttonColorVariant = 'monochrome'; // Force specific variant ``` 3. **Remember to remove before committing!** **Dev Info Banner:** - Blue banner at top of asset details screen - Shows: variant name, source (LaunchDarkly/Fallback), and raw flag value - Read-only informational display - Helps debug which variant is active ### Verifying AB Test Tracking **Dual Tracking Approach:** 1. **Screen View (Baseline Exposure)** - **Event:** `PERPS_SCREEN_VIEWED` with `screen_type: 'asset_details'` - **Properties:** `ab_test_button_color` (only when test enabled), `asset`, `source` - **Purpose:** Establishes how many users were exposed to each variant 2. **Button Press (Engagement)** - **Event:** `PERPS_UI_INTERACTION` with `interaction_type: 'tap'` - **Properties:** `ab_test_button_color` (only when test enabled), `asset`, `direction` - **Purpose:** Measures which variant drives more button presses **Metric Calculation:** - **Engagement Rate** = Button presses / Screen views per variant - Answers: "Which button color makes users more likely to press the button?" ## **Screenshots/Recordings** ### **Before** N/A - New feature ### **After** <!-- TODO: Add screenshots showing: 1. Control variant (green Long button, red Short button) 2. Monochrome variant (white Long button, white Short button) 3. Event tracking in analytics showing AB test properties --> ## **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 (manual validation first, unit tests will be added later) - [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. --- ## **Additional Context** ### Architecture ``` LaunchDarkly (Remote) � Returns variant name ('control' | 'monochrome') Feature Flag Selector (selectPerpsButtonColorTestVariant) � usePerpsABTest Hook � Returns variant data (e.g., button colors) Component (PerpsMarketDetailsView) ``` ### Files Changed **New Files:** - `app/components/UI/Perps/utils/abTesting/types.ts` - Type definitions - `app/components/UI/Perps/utils/abTesting/usePerpsABTest.ts` - Main hook - `app/components/UI/Perps/utils/abTesting/tests.ts` - Test configurations - `app/components/UI/Perps/constants/buttonColors.ts` - Color mappings - `docs/perps/perps-ab-testing.md` - Centralized AB testing documentation **Modified Files:** - `app/components/UI/Perps/selectors/featureFlags/index.ts` - Added variant selector - `app/components/UI/Perps/constants/eventNames.ts` - Added flat AB test properties (per test) - `app/components/UI/Perps/Views/PerpsMarketDetailsView/PerpsMarketDetailsView.tsx` - Dual tracking: screen view (baseline) + button press (engagement) - `docs/perps/perps-metametrics-reference.md` - Documented dual tracking approach in both events ### Backend Requirements **LaunchDarkly Flag Setup:** **IMPORTANT: Use String flag type, not Boolean or JSON** **Naming Convention:** `perps-abtest-{test-name}` (no `-enabled` suffix needed) **MetaMetrics Requirement:** Users must have MetaMetrics enabled (Settings → Security & Privacy → MetaMetrics) for A/B testing (for privacy). 1. **Flag key**: `perps-abtest-button-color` (kebab-case) 2. **Flag type**: **String** (select from dropdown) 3. **String Variations**: - Variation 0: Name=`Control`, Value=`control` - Variation 1: Name=`Monochrome`, Value=`monochrome` 4. **Targeting Rules**: - IF user `anonymous` is one of `false` (filters MetaMetrics opted-in users) - THEN serve percentage rollout: - 50% → `control` - 50% → `monochrome` - Optional: Add custom rule for `appVersion >= 7.60.0` 5. **Default Variations**: - When ON: `control` (for opted-in users) - When OFF: flag disabled (returns null/undefined) **What the app receives:** ```typescript // Redux state (kebab-case converted to camelCase) { remoteFeatureFlags: { perpsAbtestButtonColor: "control" // Simple string value } } ``` ### Metrics to Monitor **Analytics Approach:** Dual tracking (screen views + button presses) enables engagement rate calculation. Once deployed with LaunchDarkly enabled: - **Engagement rate**: (Button presses / Screen views) per variant - PRIMARY METRIC - **Button presses**: Total Long/Short button presses per variant - **Trade execution rate**: % users pressing buttons → completing trades (by variant) - **Misclick rate**: % trades cancelled/reversed within 10s (by variant) - **Transaction speed**: Time from button press to trade execution (by variant) **Key Question:** Which button color makes users more likely to press the button? ### Documentation Full framework documentation available in: - `docs/perps/perps-ab-testing.md` - AB testing architecture and patterns - `docs/perps/perps-metametrics-reference.md` - Event properties and tracking <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds LaunchDarkly-driven A/B testing with a button color experiment, integrates variant-driven UI and analytics on Perps market details, and updates selectors, utilities, tests, and docs. > > - **Perps UI (Market Details)**: > - Integrates A/B test for Long/Short buttons; renders monochrome (Secondary) or semantic green/red based on `perpsAbtestButtonColor`. > - Tracks variant on screen view and button press for analytics (baseline + engagement). > - **AB Testing Framework**: > - New hook `usePerpsABTest`, types, and test config `BUTTON_COLOR_TEST`. > - Button color utilities (`constants/buttonColors.ts`). > - **Feature Flags & Selectors**: > - New selector `selectPerpsButtonColorTestVariant` with version-gated handling. > - Adds type guard `isVersionGatedFeatureFlag` and enhances remote flag validation. > - **Analytics**: > - Extends `eventNames` with AB test properties/values (e.g., `ab_test_button_color`). > - **Tests**: > - Unit tests for selector variants, hook behavior, and remote feature flag type guard/validation. > - **Docs**: > - Adds `perps-ab-testing.md`; updates MetaMetrics reference for dual-tracking and flat AB test properties. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 61fbc1b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Bump slippage for Sell orders to 3% (from 1.5%). This will help reducing the failure rate. ## **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: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/PRED-327 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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] > Increase sell-order slippage to 3% and introduce separate buy/sell slippage constants, applying them in order previews. > > - **Polymarket provider**: > - **Slippage constants**: Replace `SLIPPAGE` with `SLIPPAGE_BUY` (1.5%) and `SLIPPAGE_SELL` (3%) in `app/components/UI/Predict/providers/polymarket/constants.ts`. > - **Order preview**: In `app/components/UI/Predict/providers/polymarket/utils.ts`, use `SLIPPAGE_BUY` for buy previews and `SLIPPAGE_SELL` for sell previews; update imports accordingly. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 582821f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!--
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**
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
Currently the GTM modal for Predict looks funky as it seems to render
different pieces at different times.
This PR attempts to fix this by introducing two main changes:
- Animate opacity for the whole modals 0 -> 1
- Only start animation when the background image finishes loading
## **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: null
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: my feature name
Scenario: user [verb for user action]
Given [describe expected initial app state]
When user [verb for user action]
Then [describe expected outcome]
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
https://github.com/user-attachments/assets/a8b61805-9f8a-4518-b37c-7cedf56d806b
### **After**
<!-- [screenshots/recordings] -->
https://github.com/user-attachments/assets/41d44fed-56ff-4ec6-b800-867d5530e15f
## **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]
> Adds a reanimated opacity fade-in for the Predict GTM modal that
starts once the background image finishes loading.
>
> - **UI/Animation**:
> - Wraps modal root in `Animated.View` and animates `opacity` from 0→1
with `withTiming`.
> - Starts animation on background image `onLoad` via `useState` +
`useEffect` and `useSharedValue`/`useAnimatedStyle`.
> - Ensures content appears only after image loads for smoother reveal.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
ae4bba0. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
…2232) ## **Description** ### Problem The `PerpsMarketDetailsView` was creating duplicate Sentry traces for "Perps Fetch Historical Candles" due to two concurrent REST API calls for the same coin: 1. **Chart Data**: User-selected interval (e.g., 3m, 5m, 15m) via `usePerpsPositionData` 2. **24h Stats**: Fixed 1h interval for high/low calculation via `usePerpsMarketStats` Both used `setInterval` polling, resulting in: - Unnecessary network traffic from repeated REST calls - No cache sharing between different intervals - Mixed architecture (prices use WebSocket, candles use REST) - Identical Sentry trace names making debugging difficult ### Solution Migrated candle data from REST polling to WebSocket streaming pattern, following the established architecture for prices, orders, and positions: 1. **Created `CandleStreamChannel`** - Lazy-loaded streaming infrastructure (no prewarm) 2. **Added `subscribeToCandles()`** to HyperLiquidClientService and PerpsController 3. **Created `usePerpsLiveCandles`** hook - Drop-in replacement for `usePerpsPositionData` 4. **Migrated consumers** - Updated `usePerpsMarketStats` and `PerpsMarketDetailsView` 5. **Removed old implementation** - Deleted `usePerpsPositionData` and tests ### Benefits � **Eliminates duplicate Sentry traces** - Each (coin, interval) combination gets distinct subscription � **Real-time updates** - WebSocket push instead of polling (no `setInterval`) � **Cache sharing** - Multiple components using same (coin, interval) = 1 WebSocket connection � **Auto-cleanup** - Unsubscribe when components unmount � **Protocol-agnostic** - Architecture supports future providers (Binance, dYdX, etc.) � **Consistent pattern** - Matches existing streaming channels (prices, orders, positions) ### Trade-off The system maintains **2 WebSocket subscriptions per coin** as required by product: - **Chart**: User-selected interval (3m/5m/15m/etc) for visual detail - **Stats**: Fixed 1h interval for 24h high/low calculation This is **not a bug** - different granularities are needed for different use cases. If product unifies to a single interval in the future, the architecture will automatically reduce to 1 subscription (no code changes needed). ### Android Rendering Fix During testing, we discovered that the WebSocket migration exposed a race condition on Android devices. The chart data often arrived before the WebView finished initializing (Android WebView is 2-3x slower than iOS), causing: - Black screens when switching markets or intervals - Skeleton loader staying visible indefinitely - Poor user experience on mid-range Android devices **Fix:** Added data buffering to hold early-arriving data until the chart is ready, and kept the chart component mounted during interval switches to avoid expensive WebView reinitialization. **Impact:** Chart now loads smoothly on Android with no black screens, even on slower devices. This was caught during testing of the WebSocket streaming feature. ## **Changelog** CHANGELOG entry: Improved candle data loading with real-time WebSocket updates ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-1867 ## **Manual testing steps** **⚠️ Important:** Test on Android device (especially mid-range) - chart should display smoothly when switching markets/intervals with no black screens. ```gherkin Feature: Real-time Candle Data via WebSocket Scenario: User views market details with live chart updates Given user is on the Perps Market Details screen for BTC And the chart displays candle data with user-selected interval (e.g., 3m) When market price changes Then the chart updates in real-time without manual refresh And the 24h high/low stats update automatically And no duplicate Sentry traces appear in monitoring Scenario: User switches between different chart intervals Given user is viewing BTC market with 1h candles When user switches to 3m interval Then the chart loads 3m candles via WebSocket And the old 1h subscription is cleaned up And the new 3m subscription is established Scenario: Multiple components share candle cache Given two components both need BTC 1h candles When both components mount Then only 1 WebSocket subscription is created And both components receive the same cached data Scenario: User navigates away from market details Given user is viewing BTC market with active candle subscriptions When user navigates away Then WebSocket subscriptions are automatically cleaned up And no orphaned connections remain Scenario: User switches between markets Given user is viewing BTC market with candles loaded When user switches to ETH market Then BTC candle subscription is cleaned up And ETH candle subscription is established And each market has its own cached data ``` ## **Screenshots/Recordings** ### **Before** - REST polling via `setInterval` every 1-4 hours depending on interval - Duplicate Sentry traces with identical names - No cache sharing between intervals - Manual refresh required for updates ### **After** - Real-time WebSocket streaming - Distinct subscriptions per (coin, interval) - Automatic cache sharing across components - Live updates without manual refresh https://github.com/user-attachments/assets/0eda7d8a-6469-478e-b13f-dbe2a6239ee9 ## **Technical Details** ### Architecture ``` Component (usePerpsLiveCandles) � (React integration, throttling) CandleStreamChannel � (cache sharing, lazy load/cleanup) PerpsController.subscribeToCandles() � (protocol-agnostic abstraction) HyperLiquidClientService.subscribeToCandles() � (REST initial + WebSocket append) SDK: subscriptionClient.candle() � (individual CandleEvent objects) ``` ### Files Changed **Created (2)**: - `providers/channels/CandleStreamChannel.ts` - Core streaming infrastructure - `hooks/stream/usePerpsLiveCandles.ts` - React hook API **Modified (8)**: - `services/HyperLiquidClientService.ts` - Added `subscribeToCandles()` method - `controllers/PerpsController.ts` - Added protocol-agnostic delegation - `providers/PerpsStreamManager.tsx` - Integrated CandleStreamChannel - `hooks/usePerpsMarketStats.ts` - Migrated to new hook - `Views/PerpsMarketDetailsView/PerpsMarketDetailsView.tsx` - Migrated to new hook, removed conditional chart rendering - `components/TradingViewChart/TradingViewChart.tsx` - Added data buffering and fixed skeleton logic for Android - `app/util/trace.ts` - Removed unused trace name **Deleted (2)**: - `hooks/usePerpsPositionData.ts` - `hooks/usePerpsPositionData.test.ts` ### Performance Impact **Before (REST Polling)**: - 2 REST calls per market (different intervals) - Polling every 60s - 4h depending on interval - No cache sharing **After (WebSocket Streaming)**: - 2 WebSocket subscriptions per market - Real-time push updates (no polling) - Cache shared across components using same (coin, interval) - Auto-cleanup when unmounted � 0 connections ## **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 REST/polling candles with WebSocket streaming across stack, updates UI chart to consume live data and fetch more history, and adds provider/controller support with tests. > > - **Streaming architecture**: > - Add `subscribeToCandles` in provider (`HyperLiquidClientService`), controller (`PerpsController`), and types (`SubscribeCandlesParams`). > - Introduce `CandleStreamChannel` in `providers/channels` and wire into `PerpsStreamManager` as `candles`. > - Expose `usePerpsLiveCandles` hook (`hooks/stream`) for real-time candles (with `fetchMoreHistory`). > - **UI integration**: > - Migrate `PerpsMarketDetailsView` to `usePerpsLiveCandles`; remove manual refresh of candles. > - Enhance `TradingViewChart`/template: data buffering, symbol validation, `CLEAR_DATA`, `NEED_MORE_HISTORY` edge-detection callback, skeleton logic, `onNeedMoreHistory` prop. > - Update `usePerpsMarketStats` to derive 24h high/low from live 1h candles. > - **Historical candles API**: > - Support `endTime` in `MarketDataService.fetchHistoricalCandles` and provider client; use dynamic limits and merge older candles on demand. > - **Removals/cleanup**: > - Remove `usePerpsPositionData` and related tests; adjust existing tests to WebSocket behavior (no candle refresh expectations). > - **Tests**: > - Add comprehensive tests for `usePerpsLiveCandles`, `CandleStreamChannel`, `HyperLiquidClientService.subscribeToCandles`, and updated services; update `MarketDataService` tests for `endTime`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 59514c6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> * fix: race condition in rive animations when animation fail to start. * Jira: https://consensyssoftware.atlassian.net/browse/SL-332 * Issue: #23048 In some device on edge cases, sometime the MetaMask riv logo does not appear during the onboarding screen animation. This is due to race conditions between state triggers and the rive onPlay state. This PR wait for onPlay state before fire the inputState ## **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: null ## **Related issues** Fixes: #23048 * race condition in rive animations when animation fail to start. ## **Manual testing steps** ```gherkin Feature: onboarding Scenario: user open the app after install Given user open the app after install Then user should see the Metamask riv logo ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/834c82d8-9fbf-4c96-943d-1ea067115401 ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/c4bca6d3-3bec-4be8-a68a-882770676ac4 ## **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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > Delay animation triggers until Rive reports onPlay, fixing race conditions that caused onboarding/fox animations to not start. > > - **UI** > - `FoxAnimation`: add `isPlaying` state and `onPlay` handler; trigger `fireState('FoxRaiseUp', ...)` only after play starts; remove `autoplay` usage. > - `OnboardingAnimation`: gate `startRiveAnimation` on `isPlaying`; add `onPlay` to set ready state; remove `autoplay` usage. > - **Testing/Mocks** > - Update `app/__mocks__/rive-react-native.tsx` to accept `onPlay` and invoke it on mount for tests. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b36851c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Gaurav Goel <grvgoel19@gmail.com>
## **Description**
Improves bottom sheet responsiveness by reducing animation duration from
`300ms` to `150ms`. This makes interactions feel 2x faster while
remaining smooth and within industry standards (150-300ms range).
**Why this change:**
- Current 300ms feels slightly sluggish for frequent interactions
- 150ms strikes the optimal balance between speed and smoothness
- Aligns with modern UI expectations for responsive interactions
**Technical change:**
- Updated `DEFAULT_BOTTOMSHEETDIALOG_DISPLAY_DURATION` constant
- Changed from `AnimationDuration.Regularly` (300ms) to
`AnimationDuration.Fast` (150ms)
- Single line change in
`/app/.../foundation/BottomSheetDialog/BottomSheetDialog.constants.ts`
## **Impacted Bottom Sheets**
This affects **all** bottom sheet components across the app:
#### Account Management
- `AccountSelector` - Account picker/switcher
- `AddAccountActions` - Add account options menu
- `AddNewAccountBottomSheet` - New account creation flow
- `DeleteAccount` - Account deletion confirmation
- `EditAccountName` - Rename account sheet
- `EditMultichainAccountName` - Rename multichain account
- `MultichainAccountActions` - Multichain account options
- `ShareAddress` / `ShareAddressQR` - Address sharing sheets
- `WalletAddAccountActions` - Wallet details add account menu
#### Network & Connections
- `NetworkSelector` - Network picker/switcher
- `NetworkListBottomSheet` - Network selection list
- `NetworkFilterBottomSheet` - Network filter options
- `RpcSelectionModal` - RPC endpoint selector
- `AccountConnect` - dApp connection flow
- `AccountPermissions` - Connection permissions manager
- `MultichainAccountConnect` - Multichain connection flow
- `MultichainPermissionsSummary` - Permissions overview
- `ConnectedAccountsModal` - Connected accounts list
#### Wallet Actions & Trading
- `WalletActions` - Buy, Sell, Send, Receive, Bridge actions
- `TradeWalletActions` - Trading-specific actions
- `SendActionBottomSheet` - Send flow actions
#### Transactions & Confirmations
- `ConfirmComponent` - Transaction confirmation sheet
- `PersonalSign` - Personal signature requests
- `TypedSign` - Typed signature requests
- `SignatureRequest` - Generic signature requests
- `ContractApprovalBottomSheet` - Contract approval confirmations
- `SmartAccountUpdateModal` - Smart account updates
- `SwitchAccountTypeModal` - Account type switching
#### Settings & Configuration
- `ContactForm` - Add/edit contact
- `DeleteContactBottomSheet` - Contact deletion confirmation
- `NetworkSettings` - Network configuration
- `FiatOnTestnetsFriction` - Testnet fiat warning
- `DataCollectionModal` - Analytics preferences
- `ExperienceEnhancerModal` - Onboarding enhancement
#### Assets & NFTs
- `AddAsset` - Import token/NFT flow
- `AssetOptions` - Asset action menu
- `NftOptions` - NFT action menu
- `ShowTokenIdSheet` - Token ID display
- `ShowIpfsGatewaySheet` - IPFS gateway display
- `ShowDisplayNFTMediaSheet` - NFT media display
#### SDK & Browser
- `SDKSessionModal` - SDK session management
- `SDKLoadingModal` - SDK loading state
- `SDKFeedbackModal` - SDK feedback collection
- `SDKDisconnectModal` - SDK disconnect confirmation
- `MaxBrowserTabsModal` - Browser tab limit warning
#### Other UI Components
- `TooltipModal` - Contextual tooltips
- `SuccessErrorSheet` - Success/error messages
- `OriginSpamModal` - Spam origin warnings
- `ChangeInSimulationModal` - Simulation change alerts
- `SelectSRPBottomSheet` - SRP selection
- `OnboardingSheet` - Onboarding flows
- `AmbiguousAddressSheet` - Address disambiguation
---
## **Changelog**
CHANGELOG entry: Improved bottom sheet animation speed for more
responsive interactions
## **Related issues**
N/A
## **Manual testing steps**
```gherkin
Feature: Faster bottom sheet animations
Scenario: User opens and closes bottom sheets
Given the user is on the wallet home screen
When the user opens account selector or network selector
Then the bottom sheet should animate up in 150ms (noticeably faster than 300ms)
When the user dismisses the sheet by swiping down or tapping outside
Then it should animate down in 150ms
Scenario: User interacts with wallet actions
Given the user is on the wallet home screen
When the user taps "Buy & Sell" or any wallet action button
Then the wallet actions bottom sheet should appear quickly with 150ms animation
Scenario: User connects to a dApp
Given the user is browsing a dApp
When the dApp requests connection
Then the account connect bottom sheet should animate up smoothly in 150ms
```
## **Screenshots/Recordings**
### **Before**
300ms animation (`AnimationDuration.Regularly`)
### **After**
150ms animation (`AnimationDuration.Fast`) - 2x faster, more responsive
feel
## **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]
> Speeds up BottomSheetDialog initial display by changing duration from
`AnimationDuration.Regularly` to `AnimationDuration.Fast`.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
c432773. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Co-authored-by: Brian August Nguyen <brianacnguyen@gmail.com>
<!-- 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** Fixes bug where bridge transactions didn't have network badges in the activity list <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **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 bug where bridge transactions didn't have network badges in the activity list ## **Related issues** Fixes: #22868 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] 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] > Adds a network badge overlay to bridge transaction icons in the activity list using the source asset’s chain ID. > > - **UI**: > - **Bridge Activity List Item (`MultichainBridgeTransactionListItem.tsx`)**: > - Wraps transaction icon with `BadgeWrapper` and `Badge` (Network variant) showing the network image. > - Derives `chainId` from `quote.srcAsset.assetId` via `parseCaipAssetType` and fetches image with `getNetworkImageSource`. > - Falls back to the original icon when `chainId` is unavailable. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c538bac. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…tInput` (#23037) <!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** This PR adds zero balance check before `onAmountInput` RPC call to nonEVM send flow amount validations. <!-- 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: null ## **Related issues** Fixes: #22812 ## **Manual testing steps** 1. Go to send flow 2. Don't have any Tron / SOL / BTC 3. Try sending it but see "insufficient balance" error ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/ab16a8fe-7da7-4da5-9ee5-684bb0a62e50 ## **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] > Adds a zero-balance pre-check for non-EVM assets in amount validation and updates tests to cover it. > > - **Validation (useAmountValidation)**: > - Add non-EVM pre-check `rawBalanceBN.isZero()` to return `"send.insufficient_funds"` before `onAmountInput` snap validation in `useAmountValidation.ts`. > - Update hook dependencies to include `rawBalanceBN`. > - **Tests**: > - Extend `useAmountValidation.test.ts` with SOLANA mocks and a test asserting error when non-EVM asset balance is zero. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bf695af. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…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] -->  ### **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 -->
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.
7 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )