fix: [Mobile] Perps KEYRING_LOCKED error in HyperLiquidWalletService (#30077)

abretonc7s · web-flow · commit 8cfa9412501f · 2026-05-13T10:23:47.000Z
## **Description** Fixes Perps `KEYRING_LOCKED` handling when HyperLiquid wraps wallet signing failures. The provider now walks the `Error.cause` chain so wrapped keyring-lock errors use the retry-later path instead of being reported as generic setup failures. Follow-up considered: preflight-gating Perps signing setup until the keyring is unlocked/available. That may avoid entering SDK signing paths that cannot succeed, but it changes initialization timing and retry behavior. This PR keeps the existing retry-later flow and fixes the concrete error-boundary bug: HyperLiquid SDK wraps `KEYRING_LOCKED` as the error `cause`, so provider catches must inspect the cause chain. We will monitor Sentry after release; if locked signing events remain meaningful, the next step is a keyring-availability gate before signing-dependent setup. ## **Changelog** CHANGELOG entry: Fixed a Perps setup error that could report locked-keyring retries as failures ## **Related issues** Fixes: [TAT-3176](https://consensyssoftware.atlassian.net/browse/TAT-3176) ## **Manual testing steps** ```gherkin Feature: Perps keyring lock retry Scenario: wrapped keyring lock is handled as retry-later Given HyperLiquid setup receives a wrapped signing error with cause.message KEYRING_LOCKED When the provider classifies the setup error Then the provider uses the keyring-locked retry-later branch And the generic Sentry failure branch is not used ``` ## **Screenshots/Recordings** No visual evidence selected for publication. ## **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) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] 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 - [x] 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. ## **Validation Recipe** <details> <summary>recipe.json</summary> ```json { "pr": "", "title": "TAT-3176 wrapped KEYRING_LOCKED classifier", "jira": "TAT-3176", "acceptance_criteria": [ "Wrapped KEYRING_LOCKED errors whose cause.message is KEYRING_LOCKED are classified as keyring-locked retry-later errors instead of falling through to generic logger.error/Sentry, failed AccountSetup analytics, or failure caches." ], "validate": { "workflow": { "entry": "ac1-assert-wrapped-keyring-locked", "nodes": { "ac1-assert-wrapped-keyring-locked": { "action": "eval_sync", "expression": "(function(){var mods=globalThis.__r.getModules();var moduleId=null;mods.forEach(function(v,k){if(v&&v.verboseName==='app/controllers/perps/utils/errorUtils.ts'){moduleId=k;}});var errorUtils=globalThis.__r(moduleId);var cause=new Error('KEYRING_LOCKED');var wrapped=new Error('Failed to sign typed data with viem wallet');wrapped.cause=cause;return JSON.stringify({moduleFound:moduleId!==null,classified:errorUtils.isKeyringLockedError(wrapped)});})()", "assert": { "operator": "eq", "field": "classified", "value": true }, "next": "done" }, "done": { "action": "end", "status": "pass" } } } } } ``` </details> ## **Recipe Workflow** <details> <summary>workflow.mmd</summary> ```mermaid flowchart TD ac1["ac1-assert-wrapped-keyring-locked"] done["done"] ac1 --> done ``` </details> [TAT-3176]: https://consensyssoftware.atlassian.net/browse/TAT-3176?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ  --- > [!NOTE] > **Medium Risk** > Changes error classification for keyring-locked signing failures, affecting retry/caching and whether failures get logged/reported; mistakes here could cause repeated prompts or suppressed retries in trading setup. > > **Overview** > Prevents HyperLiquid SDK-wrapped signing failures from being treated as generic setup errors by introducing `isKeyringLockedError()` (walks the `Error.cause` chain) and using it in HyperLiquid unified account enablement, builder-fee approval, and referral setup. > > Adds test coverage ensuring wrapped `KEYRING_LOCKED` errors do **not** populate failure caches (`TradingReadinessCache`/`PerpsSigningCache`) or trigger error logging, while still releasing in-flight locks and allowing the order/setup flow to proceed. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4fdd74f. 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
@@ -5969,6 +5969,52 @@ describe('HyperLiquidProvider', () => {
       expect(result.orderId).toBeDefined();
     });
 
+    it('leaves builder fee cache empty when wrapped KEYRING_LOCKED is thrown', async () => {
+      const wrappedKeyringLockedError = Object.assign(
+        new Error('Failed to sign typed data with viem wallet'),
+        { cause: new Error('KEYRING_LOCKED') },
+      );
+      const mockCompleteInFlight = jest.fn();
+      (
+        TradingReadinessCache as jest.Mocked<typeof TradingReadinessCache>
+      ).setInFlight.mockReturnValue(mockCompleteInFlight);
+      mockClientService.getInfoClient = jest.fn().mockReturnValue(
+        createMockInfoClient({
+          maxBuilderFee: jest.fn().mockResolvedValue(0),
+          referral: jest.fn().mockResolvedValue({
+            referrerState: {
+              stage: 'not_ready',
+              data: null,
+            },
+          }),
+        }),
+      );
+      mockClientService.getExchangeClient = jest.fn().mockReturnValue(
+        createMockExchangeClient({
+          approveBuilderFee: jest
+            .fn()
+            .mockRejectedValue(wrappedKeyringLockedError),
+        }),
+      );
+
+      const orderParams: OrderParams = {
+        symbol: 'BTC',
+        isBuy: true,
+        size: '0.1',
+        orderType: 'market',
+        currentPrice: 50000,
+      };
+
+      const result = await provider.placeOrder(orderParams);
+
+      expect(result.success).toBe(true);
+      expect(
+        (TradingReadinessCache as jest.Mocked<typeof TradingReadinessCache>)
+          .setBuilderFee,
+      ).not.toHaveBeenCalled();
+      expect(mockCompleteInFlight).toHaveBeenCalled();
+    });
+
     it('handles referral code setup failure (non-blocking)', async () => {
       // Mock builder fee already approved
       mockClientService.getInfoClient = jest
@@ -6007,6 +6053,53 @@ describe('HyperLiquidProvider', () => {
       expect(result.orderId).toBeDefined();
     });
 
+    it('leaves referral cache empty when wrapped KEYRING_LOCKED is thrown', async () => {
+      const wrappedKeyringLockedError = Object.assign(
+        new Error('Failed to sign typed data with viem wallet'),
+        { cause: new Error('KEYRING_LOCKED') },
+      );
+      const mockCompleteInFlight = jest.fn();
+      (
+        TradingReadinessCache as jest.Mocked<typeof TradingReadinessCache>
+      ).setInFlight.mockReturnValue(mockCompleteInFlight);
+      mockClientService.getInfoClient = jest.fn().mockReturnValue(
+        createMockInfoClient({
+          maxBuilderFee: jest.fn().mockResolvedValue(1),
+          referral: jest.fn().mockResolvedValue({
+            referrerState: {
+              stage: 'ready',
+              data: { code: REFERRAL_CONFIG.MainnetCode },
+            },
+            referredBy: null,
+          }),
+        }),
+      );
+      mockClientService.getExchangeClient = jest.fn().mockReturnValue(
+        createMockExchangeClient({
+          setReferrer: jest.fn().mockRejectedValue(wrappedKeyringLockedError),
+        }),
+      );
+
+      const orderParams: OrderParams = {
+        symbol: 'BTC',
+        isBuy: true,
+        size: '0.1',
+        orderType: 'market',
+        currentPrice: 50000,
+      };
+
+      const result = await provider.placeOrder(orderParams);
+      await Promise.resolve();
+      await Promise.resolve();
+
+      expect(result.success).toBe(true);
+      expect(
+        (TradingReadinessCache as jest.Mocked<typeof TradingReadinessCache>)
+          .setReferral,
+      ).not.toHaveBeenCalled();
+      expect(mockCompleteInFlight).toHaveBeenCalled();
+    });
+
     it('skips referral setup when referral code is not ready', async () => {
       // Mock referral code not ready
       mockClientService.getInfoClient = jest.fn().mockReturnValue(
@@ -9325,6 +9418,41 @@ describe('HyperLiquidProvider', () => {
       expect(mockCompleteInFlight).toHaveBeenCalled();
     });
 
+    it('does NOT cache or log to Sentry when a wrapped KEYRING_LOCKED error is thrown', async () => {
+      // Arrange
+      const wrappedKeyringLockedError = Object.assign(
+        new Error('Failed to sign typed data with viem wallet'),
+        { cause: new Error('KEYRING_LOCKED') },
+      );
+      const mockExchangeClient = createMockExchangeClient();
+      mockExchangeClient.agentSetAbstraction = jest
+        .fn()
+        .mockRejectedValue(wrappedKeyringLockedError);
+      const mockCompleteInFlight = jest.fn();
+      (
+        TradingReadinessCache as jest.Mocked<typeof TradingReadinessCache>
+      ).setInFlight.mockReturnValue(mockCompleteInFlight);
+      mockClientService.getInfoClient = jest.fn().mockReturnValue(
+        createMockInfoClient({
+          userAbstraction: jest.fn().mockResolvedValue('default'),
+        }),
+      );
+      mockClientService.getExchangeClient = jest
+        .fn()
+        .mockReturnValue(mockExchangeClient);
+
+      // Act
+      await provider.getMarketDataWithPrices();
+
+      // Assert
+      expect(
+        (TradingReadinessCache as jest.Mocked<typeof TradingReadinessCache>)
+          .set,
+      ).not.toHaveBeenCalled();
+      expect(mockPlatformDependencies.logger.error).not.toHaveBeenCalled();
+      expect(mockCompleteInFlight).toHaveBeenCalled();
+    });
+
     it('does NOT cache failure when userAbstraction read itself rejects', async () => {
       // Read-only userAbstraction lookup failures (transient HL outage /
       // network) must not block all future migration attempts for the rest
diff --git a/app/controllers/perps/providers/HyperLiquidProvider.ts b/app/controllers/perps/providers/HyperLiquidProvider.ts
@@ -128,7 +128,7 @@ import {
   addSpotBalanceToAccountState,
   aggregateAccountStates,
 } from '../utils/accountUtils';
-import { ensureError } from '../utils/errorUtils';
+import { ensureError, isKeyringLockedError } from '../utils/errorUtils';
 import {
   adaptAccountStateFromSDK,
   adaptHyperLiquidLedgerUpdateToUserHistoryItem,
@@ -814,8 +814,9 @@ export class HyperLiquidProvider implements PerpsProvider {
       );
       completeInFlight();
     } catch (error) {
-      // If keyring is locked, don't cache so it retries when unlocked
-      if (ensureError(error).message === PERPS_ERROR_CODES.KEYRING_LOCKED) {
+      // HyperLiquid wraps wallet signing failures and preserves KEYRING_LOCKED
+      // in `cause`, so classify the full chain and leave retry caches empty.
+      if (isKeyringLockedError(error)) {
         this.#deps.debugLogger.log(
           '[ensureUnifiedAccountEnabled] Keyring locked, will retry later',
         );
@@ -2650,8 +2651,9 @@ export class HyperLiquidProvider implements PerpsProvider {
       }
       completeInFlight();
     } catch (error) {
-      // If keyring is locked, don't cache so it retries when unlocked
-      if (ensureError(error).message === PERPS_ERROR_CODES.KEYRING_LOCKED) {
+      // HyperLiquid wraps wallet signing failures and preserves KEYRING_LOCKED
+      // in `cause`, so classify the full chain and leave retry caches empty.
+      if (isKeyringLockedError(error)) {
         this.#deps.debugLogger.log(
           '[ensureBuilderFeeApproval] Keyring locked, will retry later',
         );
@@ -8468,8 +8470,9 @@ export class HyperLiquidProvider implements PerpsProvider {
       }
       completeInFlight();
     } catch (error) {
-      // If keyring is locked, don't cache so it retries when unlocked
-      if (ensureError(error).message === PERPS_ERROR_CODES.KEYRING_LOCKED) {
+      // HyperLiquid wraps wallet signing failures and preserves KEYRING_LOCKED
+      // in `cause`, so classify the full chain and leave retry caches empty.
+      if (isKeyringLockedError(error)) {
         this.#deps.debugLogger.log(
           '[ensureReferralSet] Keyring locked, will retry later',
         );
diff --git a/app/controllers/perps/utils/errorUtils.test.ts b/app/controllers/perps/utils/errorUtils.test.ts
@@ -1,4 +1,4 @@
-import { isAbortError, ensureError } from './errorUtils';
+import { isAbortError, ensureError, isKeyringLockedError } from './errorUtils';
 
 describe('errorUtils', () => {
   describe('isAbortError', () => {
@@ -71,4 +71,28 @@ describe('errorUtils', () => {
       expect(result.message).toContain('Unknown error');
     });
   });
+
+  describe('isKeyringLockedError', () => {
+    it('returns true for direct KEYRING_LOCKED errors', () => {
+      const error = new Error('KEYRING_LOCKED');
+
+      expect(isKeyringLockedError(error)).toBe(true);
+    });
+
+    it('returns true for wrapped KEYRING_LOCKED causes', () => {
+      const error = Object.assign(
+        new Error('Failed to sign typed data with viem wallet'),
+        { cause: new Error('KEYRING_LOCKED') },
+      );
+
+      expect(isKeyringLockedError(error)).toBe(true);
+    });
+
+    it('returns false for non-keyring errors with cyclic causes', () => {
+      const error = new Error('Network error') as Error & { cause?: unknown };
+      error.cause = error;
+
+      expect(isKeyringLockedError(error)).toBe(false);
+    });
+  });
 });
diff --git a/app/controllers/perps/utils/errorUtils.ts b/app/controllers/perps/utils/errorUtils.ts
@@ -1,9 +1,11 @@
 /**
- * Utility functions for error handling across the application.
- * These are general-purpose utilities, not domain-specific.
+ * Utility functions for error handling across Perps controller code.
+ * Includes generic error helpers and Perps error classification helpers.
  */
 import { hasProperty } from '@metamask/utils';
 
+import { PERPS_ERROR_CODES } from '../perpsErrorCodes';
+
 /**
  * Detects expected cancellation/abort errors that should not be reported to Sentry.
  * These occur during normal navigation or view teardown when in-flight fetch requests
@@ -23,6 +25,30 @@ export function isAbortError(error: unknown): boolean {
   return false;
 }
 
+/**
+ * Detects keyring-locked errors, including SDK-wrapped errors that preserve the
+ * original error in `cause`.
+ *
+ * @param error - The error to check.
+ * @returns True if any error in the cause chain is KEYRING_LOCKED.
+ */
+export function isKeyringLockedError(error: unknown): boolean {
+  let current: unknown = error;
+  const seen = new Set<unknown>();
+
+  while (current instanceof Error && !seen.has(current)) {
+    seen.add(current);
+
+    if (current.message === PERPS_ERROR_CODES.KEYRING_LOCKED) {
+      return true;
+    }
+
+    current = (current as { cause?: unknown }).cause;
+  }
+
+  return false;
+}
+
 /**
  * Ensures we have a proper Error object for logging.
  * Converts unknown/string errors to proper Error instances.
