fix: resolve ORDER_PRICE_REQUIRED error on perps position flip (#29691)

gambinish · web-flow · commit fcd05a60c005 · 2026-05-05T19:37:03.000Z
## **Description** TradingService.flipPosition() deliberately omits currentPrice from the order params it passes to provider.placeOrder() — passing a stale entry price would corrupt IOC slippage calculation. However, placeOrder() was running #validateOrderBeforePlacement before fetching the live price, so the minimum-USD check had no price to work with and threw ORDER_PRICE_REQUIRED on every flip. Root cause: step ordering in HyperLiquidProvider.placeOrder(). Validation ran at step 1; the live price fetch (#getAssetInfo) ran at step 3. Fix: reorder placeOrder() so #getAssetInfo runs first, compute effectivePrice (live price, or caller-supplied price if present), then pass it into #validateOrderBeforePlacement. #ensureReadyForTrading stays after validation, so invalid orders never trigger builder-fee / DEX-abstraction signature prompts unnecessarily. As a follow-on, effectivePrice is hoisted above the try block so the $10-minimum retry path can also use the fetched price when the caller omits currentPrice — without this, a flip order that hit the $10 edge case would fail silently instead of retrying. No changes to TradingService.flipPosition — the intentional omission of entryPrice is correct and preserved. Affected tests: Three HyperLiquidProvider tests updated to reflect the new ordering (the "missing price data" error now surfaces from #getAssetInfo rather than validation; the price-less PUMP retry test now succeeds with two exchange calls) Two new tests added for the explicit flip-order path ## **Changelog**  CHANGELOG entry: fix regression to flip position perps ## **Related issues** Fixes: MetaMask/metamask-extension#42375 ## **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** https://github.com/user-attachments/assets/1a789aff-37e1-4e7f-825b-a4af00f14211 ## **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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **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.  --- > [!NOTE] > **Medium Risk** > Changes the execution order in `HyperLiquidProvider.placeOrder`, affecting validation/signing flow and retry behavior for minimum-order errors. Risk is moderate because it touches core order placement logic, but the change is localized and covered by updated/new tests. > > **Overview** > Fixes a regression where price-less market orders (notably `flipPosition`) failed with `ORDER_PRICE_REQUIRED` by fetching the live price via `#getAssetInfo` before provider-level validation and validating against the computed `effectivePrice`. > > Also hoists `effectivePrice` so the "$10 minimum order value" retry path can derive an adjusted `usdAmount` from the fetched price when `currentPrice` is omitted. Updates existing tests and adds new coverage to assert success for flip-style params, the $10-minimum retry behavior, and the revised error surfaced when price data is missing. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 106369c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
diff --git a/app/controllers/perps/providers/HyperLiquidProvider.test.ts b/app/controllers/perps/providers/HyperLiquidProvider.test.ts
@@ -1226,7 +1226,7 @@ describe('HyperLiquidProvider', () => {
       );
     });
 
-    it('validates price requirement before attempting order placement', async () => {
+    it('retries with adjusted USD when price-less order hits $10 minimum (uses fetched price from allMids)', async () => {
       // Create provider with PUMP in the asset mapping
       provider = createTestProvider({
         initialAssetMapping: [
@@ -1256,24 +1256,38 @@ describe('HyperLiquidProvider', () => {
         isBuy: true,
         size: '2553',
         orderType: 'market',
+        // No currentPrice: provider fetches live price (0.003918) and uses it
+        // for both validation and the $10-minimum retry path.
       };
 
+      const mockOrder = jest
+        .fn()
+        .mockRejectedValueOnce(
+          new Error('Order must have minimum value of $10'),
+        )
+        .mockResolvedValueOnce({
+          status: 'ok',
+          response: {
+            data: {
+              statuses: [
+                { filled: { oid: 123, totalSz: '2553', avgPx: '0.004' } },
+              ],
+            },
+          },
+        });
+
       mockClientService.getExchangeClient = jest.fn().mockReturnValue({
         ...createMockExchangeClient(),
-        order: jest
-          .fn()
-          .mockRejectedValueOnce(
-            new Error('Order must have minimum value of $10'),
-          ),
+        order: mockOrder,
       });
 
       const result = await provider.placeOrder(orderParams);
 
-      expect(result.success).toBe(false);
-      expect(result.error).toBe(PERPS_ERROR_CODES.ORDER_PRICE_REQUIRED);
-      expect(
-        mockClientService.getExchangeClient().order,
-      ).not.toHaveBeenCalled();
+      // The live price is fetched → validation passes → order is submitted →
+      // first call hits $10 minimum → retry uses fetched price to compute
+      // adjusted usdAmount → second call succeeds.
+      expect(result.success).toBe(true);
+      expect(mockOrder).toHaveBeenCalledTimes(2);
     });
 
     it('closes a position successfully', async () => {
@@ -2812,6 +2826,7 @@ describe('HyperLiquidProvider', () => {
     });
 
     it('handles missing price data', async () => {
+      mockSubscriptionService.getCachedPrice.mockReturnValueOnce(undefined);
       (
         mockClientService.getInfoClient().allMids as jest.Mock
       ).mockResolvedValueOnce({});
@@ -2825,8 +2840,10 @@ describe('HyperLiquidProvider', () => {
 
       const result = await provider.placeOrder(orderParams);
 
+      // allMids returns {} so #getOrFetchPrice parses price as 0, which is
+      // invalid. The error surfaces from #getAssetInfo before validation runs.
       expect(result.success).toBe(false);
-      expect(result.error).toContain(PERPS_ERROR_CODES.ORDER_PRICE_REQUIRED);
+      expect(result.error).toContain('Invalid price for BTC: 0');
     });
 
     it('handles missing position in close operation', async () => {
@@ -3545,19 +3562,45 @@ describe('HyperLiquidProvider', () => {
         expect(result.error).toContain('Failed to update leverage');
       });
 
-      it('fails market order without current price or usdAmount', async () => {
+      it('succeeds with market order without current price or usdAmount (uses fetched price)', async () => {
+        // The provider now fetches the live price before validation so callers
+        // that intentionally omit currentPrice (e.g. flipPosition) work correctly.
         const orderParams: OrderParams = {
           symbol: 'BTC',
           isBuy: true,
           size: '0.1',
           orderType: 'market',
-          // No currentPrice or usdAmount provided - should fail validation
+          // No currentPrice or usdAmount: provider fetches live price (50000)
         };
 
         const result = await provider.placeOrder(orderParams);
 
-        expect(result.success).toBe(false);
-        expect(result.error).toContain(PERPS_ERROR_CODES.ORDER_PRICE_REQUIRED);
+        expect(result.success).toBe(true);
+        expect(mockClientService.getExchangeClient().order).toHaveBeenCalled();
+      });
+
+      it('placeOrder validates against fetched price when params omit currentPrice (flipPosition path)', async () => {
+        // Simulate the exact OrderParams shape that TradingService.flipPosition
+        // builds: symbol + isBuy + size + orderType + leverage, no price fields.
+        const flipOrderParams: OrderParams = {
+          symbol: 'BTC',
+          isBuy: false, // flipping long → short
+          size: '1', // 2× the 0.5 BTC position
+          orderType: 'market',
+          leverage: 10,
+          // currentPrice, usdAmount, price intentionally absent
+        };
+
+        const result = await provider.placeOrder(flipOrderParams);
+
+        // Live price (50000) is fetched from allMids → validation passes
+        // (0.1 BTC × $50 000 = $5 000 >> $10 minimum) → order executes.
+        expect(result.success).toBe(true);
+        expect(
+          mockClientService.getExchangeClient().order,
+        ).toHaveBeenCalledWith(
+          expect.objectContaining({ orders: expect.any(Array) }),
+        );
       });
 
       it('handles order with custom slippage', async () => {
diff --git a/app/controllers/perps/providers/HyperLiquidProvider.ts b/app/controllers/perps/providers/HyperLiquidProvider.ts
@@ -3543,6 +3543,9 @@ export class HyperLiquidProvider implements PerpsProvider {
    * @returns A promise that resolves to the result.
    */
   async placeOrder(params: OrderParams, retryCount = 0): Promise<OrderResult> {
+    // Hoisted so the retry path in the catch block can use the fetched price
+    // even when the caller (e.g. flipPosition) omits currentPrice from params.
+    let effectivePrice: number | undefined;
     try {
       this.#deps.debugLogger.log('Placing order via HyperLiquid SDK:', params);
 
@@ -3557,38 +3560,18 @@ export class HyperLiquidProvider implements PerpsProvider {
         throw new Error(validation.error);
       }
 
-      // Validate order at provider level (enforces USD validation rules)
-      await this.#validateOrderBeforePlacement(params);
-
-      // Ensure provider is ready for trading (includes signing operations)
-      await this.#ensureReadyForTrading();
-
-      // Debug: Log asset map state before order placement
-      const allMapKeys = Array.from(this.#symbolToAssetId.keys());
-      const hip3Keys = allMapKeys.filter((key) => key.includes(':'));
-      const assetExists = this.#symbolToAssetId.has(params.symbol);
-      this.#deps.debugLogger.log('Asset map state at order time', {
-        requestedCoin: params.symbol,
-        assetExistsInMap: assetExists,
-        totalAssetsInMap: this.#symbolToAssetId.size,
-        hip3AssetsCount: hip3Keys.length,
-        hip3AssetsSample: hip3Keys.slice(0, 10),
-        hip3Enabled: this.#hip3Enabled,
-        allowlistMarkets: this.#allowlistMarkets,
-        blocklistMarkets: this.#blocklistMarkets,
-      });
-
       // Extract DEX name for API calls (main DEX = null)
       const { dex: dexName } = parseAssetName(params.symbol);
 
-      // 1. Get asset info and current price
+      // 1. Get asset info and current price before validation so price-less
+      // callers (e.g. flipPosition) can validate against the live fetched price.
       const { assetInfo, currentPrice, meta } = await this.#getAssetInfo({
         symbol: params.symbol,
         dexName,
       });
 
-      // Allow override with UI-provided price (optimization to avoid API call)
-      const effectivePrice =
+      // Allow override with UI-provided price (optimization to avoid API call).
+      effectivePrice =
         params.currentPrice && params.currentPrice > 0
           ? params.currentPrice
           : currentPrice;
@@ -3601,6 +3584,35 @@ export class HyperLiquidProvider implements PerpsProvider {
         });
       }
 
+      // Validate order at provider level (enforces USD validation rules).
+      // Pass effectivePrice so price-less market orders (e.g. flipPosition)
+      // validate against the live fetched price instead of failing with
+      // ORDER_PRICE_REQUIRED.
+      await this.#validateOrderBeforePlacement({
+        ...params,
+        currentPrice: effectivePrice,
+      });
+
+      // Ensure provider is ready for trading (includes signing operations).
+      // Kept after validation so invalid orders never trigger signature prompts
+      // (builder-fee approval, DEX abstraction enablement, etc.).
+      await this.#ensureReadyForTrading();
+
+      // Debug: Log asset map state before order placement
+      const allMapKeys = Array.from(this.#symbolToAssetId.keys());
+      const hip3Keys = allMapKeys.filter((key) => key.includes(':'));
+      const assetExists = this.#symbolToAssetId.has(params.symbol);
+      this.#deps.debugLogger.log('Asset map state at order time', {
+        requestedCoin: params.symbol,
+        assetExistsInMap: assetExists,
+        totalAssetsInMap: this.#symbolToAssetId.size,
+        hip3AssetsCount: hip3Keys.length,
+        hip3AssetsSample: hip3Keys.slice(0, 10),
+        hip3Enabled: this.#hip3Enabled,
+        allowlistMarkets: this.#allowlistMarkets,
+        blocklistMarkets: this.#blocklistMarkets,
+      });
+
       // 2. Calculate final position size with USD reconciliation
       const { finalPositionSize } = calculateFinalPositionSize({
         usdAmount: params.usdAmount,
@@ -3706,10 +3718,13 @@ export class HyperLiquidProvider implements PerpsProvider {
           // USD-based order: adjust the USD amount directly
           originalValue = params.usdAmount;
           adjustedUsdAmount = (parseFloat(params.usdAmount) * 1.015).toFixed(2);
-        } else if (params.currentPrice) {
-          // Size-based order: calculate USD from size and adjust
+        } else if (effectivePrice) {
+          // Size-based order: calculate USD from size and adjust.
+          // Use the hoisted effectivePrice (fetched live price) so callers that
+          // omit currentPrice (e.g. flipPosition) can still recover from the
+          // $10-minimum edge case.
           const sizeValue = parseFloat(params.size);
-          const estimatedUsd = sizeValue * params.currentPrice;
+          const estimatedUsd = sizeValue * effectivePrice;
           originalValue = `${estimatedUsd.toFixed(2)} (calculated from size ${params.size})`;
           adjustedUsdAmount = (estimatedUsd * 1.015).toFixed(2);
         } else {
