diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 900d8d11c0..402170dbfd 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Add `EXCHANGE_MULTI_SIG_REQUIRED` and `EXCHANGE_INVALID_NONCE` to `PERPS_ERROR_CODES` for HyperLiquid exchange rejections that previously surfaced as raw `"multi-sig required"` / `"invalid nonce"` strings (TAT-3633) ([#9750](https://github.com/MetaMask/core/pull/9750)) - Like `EXCHANGE_ACCOUNT_NOT_FOUND` above, this widens the exported `PerpsErrorCode` union, so consumers that key an exhaustive `Record` stop compiling until they add entries for both new codes — including Mobile's `app/components/UI/Perps/utils/translatePerpsError.ts` and Extension's `ui/components/app/perps/utils/translate-perps-error.ts`. - To migrate: add translation entries for both codes before bumping. `EXCHANGE_MULTI_SIG_REQUIRED` means the account requires a multi-sig wrapper for exchange writes; `EXCHANGE_INVALID_NONCE` means the action nonce was stale or reused and the request should be retried. +- Add `isHyperLiquidMultiSigRequiredError(error)` (exported from `@metamask/perps-controller/utils/*`), which classifies HyperLiquid's `Multi-sig required` rejection — matching both the hyphenated and unhyphenated spellings the venue returns (TAT-3214) ([#9764](https://github.com/MetaMask/core/pull/9764)) ### Changed @@ -91,6 +92,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Size the max order amount off the price a resting limit order is submitted at, fixing `order 0: insufficient margin to place order` rejections on max-size limit orders resting above the market price ([#9694](https://github.com/MetaMask/core/pull/9694)) - `getMaxAllowedAmount` derived the maximum from the market price, but HyperLiquid reserves initial margin for a resting order against the price that order is submitted at. A max-size limit order resting above the market price - typically a sell - therefore reserved more margin than the account had and the exchange rejected it. - `getMaxAllowedAmount` now accepts optional `orderType` and `limitPrice` params. When a limit order rests above the market price the maximum is scaled by `limitPrice / marketPrice`; orders at or below the market price, and market orders, are unchanged. Both params are optional, so existing callers keep the previous behavior. +- Skip the unified-account migration for HyperLiquid multi-sig accounts, fixing the `ApiRequestError: Multi-sig required` error raised on every Perps entry for such an account (TAT-3214) ([#9764](https://github.com/MetaMask/core/pull/9764)) + - HyperLiquid rejects every single-signer exchange write for an account converted to multi-sig, so the silent `agentSetAbstraction` (and user-signed `userSetAbstraction`) migration could never succeed. `HyperLiquidProvider` now reads `userToMultiSigSigners` immediately before the migration write and, for a multi-sig account, skips it, emits the `Perp Account Setup` event with `status: not_applicable` / `error_message: multi_sig_account`, and records `{ attempted: true, enabled: false }` in the trading-readiness cache so the attempt is not repeated. The signer lookup only runs when a migration write would otherwise be made, so accounts already on `unifiedAccount` / `portfolioMargin` and deferred `dexAbstraction` accounts are unaffected. + - The same rejection is now classified in the migration's error handler, covering the case where the account is converted between the lookup and the write, so it is no longer reported to the error logger as a failed setup. + - Unified account mode stays off for these accounts; HIP-3 collateral continues to be handled by the existing programmatic transfer fallback. ## [10.0.0] diff --git a/packages/perps-controller/src/providers/HyperLiquidProvider.ts b/packages/perps-controller/src/providers/HyperLiquidProvider.ts index 9b52dfdfd6..cc92ede9d1 100644 --- a/packages/perps-controller/src/providers/HyperLiquidProvider.ts +++ b/packages/perps-controller/src/providers/HyperLiquidProvider.ts @@ -136,6 +136,7 @@ import { } from '../utils/accountUtils.js'; import { ensureError, + isHyperLiquidMultiSigRequiredError, isHyperLiquidUserNotFoundError, isKeyringLockedError, } from '../utils/errorUtils.js'; @@ -787,6 +788,44 @@ export class HyperLiquidProvider implements PerpsProvider { } } + /** + * Decide whether the Hyperliquid account is a multi-sig account. + * + * Hyperliquid rejects every single-signer exchange write for a converted + * multi-sig account with `ApiRequestError: Multi-sig required`, so the + * unified-account migration must not be attempted for those accounts. + * + * If the probe throws (transient network), returns `false` — fail open so + * one bad probe never blocks migration for a normal single-signer account. + * The `isHyperLiquidMultiSigRequiredError` fallback in the write's catch + * block remains the safety net. + * + * @param userAddress - The wallet address to check. + * @returns True only when Hyperliquid reports a multi-sig signer set. + * @private + */ + async #isHyperliquidMultiSigAccount(userAddress: string): Promise { + try { + const infoClient = this.#clientService.getInfoClient(); + const signers = await infoClient.userToMultiSigSigners({ + user: userAddress, + }); + return signers !== null && signers !== undefined; + } catch (error) { + this.#deps.debugLogger.log( + '[isHyperliquidMultiSigAccount] Probe failed, assuming single-signer', + { + user: userAddress, + error: ensureError( + error, + 'HyperLiquidProvider.isHyperliquidMultiSigAccount', + ).message, + }, + ); + return false; + } + } + /** * Attempt to enable HyperLiquid Unified Account mode for HIP-3 orders * @@ -982,6 +1021,34 @@ export class HyperLiquidProvider implements PerpsProvider { return; } + // Hyperliquid rejects every single-signer exchange write for a converted + // multi-sig account with "ApiRequestError: Multi-sig required", which + // surfaced on the Perps tab on every entry (TAT-3214). Probe right + // before the write so accounts that never reach one (already compatible, + // deferred, unknown mode) do not pay the extra round trip. + const isMultiSig = await this.#isHyperliquidMultiSigAccount(userAddress); + if (isMultiSig) { + this.#deps.debugLogger.log( + '[ensureUnifiedAccountEnabled] Multi-sig account, skipping unified account migration', + { user: userAddress, network, mode: currentMode }, + ); + this.#deps.metrics.trackPerpsEvent(PerpsAnalyticsEvent.AccountSetup, { + [PERPS_EVENT_PROPERTY.PREVIOUS_ABSTRACTION_MODE]: currentMode, + [PERPS_EVENT_PROPERTY.STATUS]: + PERPS_EVENT_VALUE.STATUS.NOT_APPLICABLE, + [PERPS_EVENT_PROPERTY.ERROR_MESSAGE]: 'multi_sig_account', + }); + // Final state: the write can never succeed for this account, so cache + // it as attempted with unified mode off. Perps keeps working through + // the programmatic collateral-transfer fallback. + TradingReadinessCache.set(network, userAddress, { + attempted: true, + enabled: false, + }); + completeInFlight(); + return; + } + // Track which mode users are currently on before we attempt migration. // This tells us the distribution of legacy modes across our user base. this.#deps.metrics.trackPerpsEvent(PerpsAnalyticsEvent.AccountSetup, { @@ -1077,6 +1144,31 @@ export class HyperLiquidProvider implements PerpsProvider { return; } + // Safety net for the multi-sig probe: the account can be converted + // between the lookup and the write, and the probe fails open on + // transient info-API errors. Either way the rejection is a permanent + // account-shape condition, not a failure worth reporting or retrying. + if (isHyperLiquidMultiSigRequiredError(error)) { + this.#deps.debugLogger.log( + '[ensureUnifiedAccountEnabled] Multi-sig account (race/probe fallback), skipping unified account migration', + { user: userAddress, network, mode: currentMode }, + ); + this.#deps.metrics.trackPerpsEvent(PerpsAnalyticsEvent.AccountSetup, { + ...(currentMode && { + [PERPS_EVENT_PROPERTY.PREVIOUS_ABSTRACTION_MODE]: currentMode, + }), + [PERPS_EVENT_PROPERTY.STATUS]: + PERPS_EVENT_VALUE.STATUS.NOT_APPLICABLE, + [PERPS_EVENT_PROPERTY.ERROR_MESSAGE]: 'multi_sig_account', + }); + TradingReadinessCache.set(network, userAddress, { + attempted: true, + enabled: false, + }); + completeInFlight(); + return; + } + // Cache failure ONLY for the user-prompted path // (`dexAbstraction → unifiedAccount` via `userSetAbstraction`). The // rationale for caching is "don't re-prompt a user who already saw the diff --git a/packages/perps-controller/src/utils/errorUtils.ts b/packages/perps-controller/src/utils/errorUtils.ts index c3e7c581d9..e3c8f8d189 100644 --- a/packages/perps-controller/src/utils/errorUtils.ts +++ b/packages/perps-controller/src/utils/errorUtils.ts @@ -93,3 +93,21 @@ export function isHyperLiquidUserNotFoundError(error: unknown): boolean { lower.includes('user or api wallet') && lower.includes('does not exist') ); } + +/** + * Hyperliquid rejects every single-signer exchange write for an account that + * has been converted to multi-sig (`ApiRequestError: Multi-sig required`). + * MetaMask signs Perps actions with a single agent/user wallet, so this is a + * permanent account-shape condition rather than a failure we should retry or + * forward to Sentry. Hyperliquid is inconsistent about the hyphen across + * endpoints, so both spellings are matched. + * + * @param error - The caught error. + * @returns True if the error indicates multi-sig signing is required. + */ +export function isHyperLiquidMultiSigRequiredError(error: unknown): boolean { + const lower = ensureError(error).message.toLowerCase(); + return ( + lower.includes('multi-sig required') || lower.includes('multisig required') + ); +} diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.account-mode.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.account-mode.test.ts index 5278f07892..91efa48e2a 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.account-mode.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.account-mode.test.ts @@ -188,6 +188,9 @@ const createMockInfoClient = (overrides: Record = {}) => ({ // Mode-aware fold gate reads userAbstraction; default to unifiedAccount // so tests that predated the gate still see spot folded into spendable/withdrawable. userAbstraction: jest.fn().mockResolvedValue('unifiedAccount'), + // Single-signer account by default; Hyperliquid returns null when the user + // has no multi-sig signer set. + userToMultiSigSigners: jest.fn().mockResolvedValue(null), meta: jest.fn().mockResolvedValue({ universe: [ { name: 'BTC', szDecimals: 3, maxLeverage: 50 }, @@ -1556,6 +1559,197 @@ describe('HyperLiquidProvider', () => { }); }); + // ───────────────────────────────────────────────── + // Hyperliquid multi-sig accounts (TAT-3214) + // + // Hyperliquid rejects every single-signer exchange write for an account + // that was converted to multi-sig with "ApiRequestError: Multi-sig + // required". Attempting the migration surfaced that error on the Perps + // tab on every entry. + // ───────────────────────────────────────────────── + + it('skips unified account migration for Hyperliquid multi-sig accounts', async () => { + // Arrange - migratable mode, but the account has a multi-sig signer set + const mockExchangeClient = createMockExchangeClient(); + const userToMultiSigSigners = jest.fn().mockResolvedValue({ + authorizedUsers: ['0xabc0000000000000000000000000000000000001'], + threshold: 2, + }); + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + userAbstraction: jest.fn().mockResolvedValue('default'), + userToMultiSigSigners, + }), + ); + mockClientService.getExchangeClient = jest + .fn() + .mockReturnValue(mockExchangeClient); + + // Act + await provider.getMarketDataWithPrices(); + + // Assert - the signer set was queried and no write was attempted + expect(userToMultiSigSigners).toHaveBeenCalledWith({ + user: USER_ADDRESS, + }); + expect(mockExchangeClient.agentSetAbstraction).not.toHaveBeenCalled(); + expect(mockExchangeClient.userSetAbstraction).not.toHaveBeenCalled(); + // No migration_required event — the migration is not possible at all. + expect( + mockPlatformDependencies.metrics.trackPerpsEvent, + ).not.toHaveBeenCalledWith( + 'Perp Account Setup', + expect.objectContaining({ status: 'migration_required' }), + ); + expect( + mockPlatformDependencies.metrics.trackPerpsEvent, + ).toHaveBeenCalledWith( + 'Perp Account Setup', + expect.objectContaining({ + status: 'not_applicable', + error_message: 'multi_sig_account', + }), + ); + }); + + it('caches attempted-but-not-enabled readiness for Hyperliquid multi-sig accounts', async () => { + // Arrange + const mockCompleteInFlight = jest.fn(); + ( + TradingReadinessCache as jest.Mocked + ).setInFlight.mockReturnValue(mockCompleteInFlight); + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + userAbstraction: jest.fn().mockResolvedValue('default'), + userToMultiSigSigners: jest.fn().mockResolvedValue({ + authorizedUsers: ['0xabc0000000000000000000000000000000000001'], + threshold: 2, + }), + }), + ); + mockClientService.getExchangeClient = jest + .fn() + .mockReturnValue(createMockExchangeClient()); + + // Act + await provider.getMarketDataWithPrices(); + + // Assert - final state, so the next entry short-circuits instead of + // re-attempting a write that can never succeed. + expect( + (TradingReadinessCache as jest.Mocked) + .set, + ).toHaveBeenCalledWith('mainnet', USER_ADDRESS, { + attempted: true, + enabled: false, + }); + // Unified mode stays off, so spot must not be folded. + expect( + mockSubscriptionService.setUserAbstractionMode, + ).not.toHaveBeenCalled(); + expect(mockCompleteInFlight).toHaveBeenCalled(); + }); + + it('treats a Multi-sig required rejection as benign instead of reporting an error', async () => { + // The signer-set lookup and the write can race (the account is + // converted between the two calls), and other single-signer write paths + // can hit the same rejection. Classify it rather than surfacing it. + const mockExchangeClient = createMockExchangeClient(); + mockExchangeClient.agentSetAbstraction = jest + .fn() + .mockRejectedValue(new Error('ApiRequestError: Multi-sig required')); + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + userAbstraction: jest.fn().mockResolvedValue('default'), + // Stale null — the account became multi-sig after the probe. + userToMultiSigSigners: jest.fn().mockResolvedValue(null), + }), + ); + mockClientService.getExchangeClient = jest + .fn() + .mockReturnValue(mockExchangeClient); + + // Act + await provider.getMarketDataWithPrices(); + + // Assert - not forwarded to the client error surface / Sentry + expect(mockPlatformDependencies.logger.error).not.toHaveBeenCalled(); + expect( + mockPlatformDependencies.metrics.trackPerpsEvent, + ).not.toHaveBeenCalledWith( + 'Perp Account Setup', + expect.objectContaining({ status: 'failed' }), + ); + expect( + mockPlatformDependencies.metrics.trackPerpsEvent, + ).toHaveBeenCalledWith( + 'Perp Account Setup', + expect.objectContaining({ + previous_abstraction_mode: 'default', + status: 'not_applicable', + error_message: 'multi_sig_account', + }), + ); + expect( + (TradingReadinessCache as jest.Mocked) + .set, + ).toHaveBeenCalledWith('mainnet', USER_ADDRESS, { + attempted: true, + enabled: false, + }); + }); + + it('still migrates single-signer accounts when the multi-sig probe fails', async () => { + // Fail open: a transient info-API failure must never block migration + // for the overwhelming majority of accounts. The catch-path classifier + // remains the safety net. + const mockExchangeClient = createMockExchangeClient(); + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + userAbstraction: jest.fn().mockResolvedValue('default'), + userToMultiSigSigners: jest + .fn() + .mockRejectedValue(new Error('Transient HL network blip')), + }), + ); + mockClientService.getExchangeClient = jest + .fn() + .mockReturnValue(mockExchangeClient); + + // Act + await provider.getMarketDataWithPrices(); + + // Assert + expect(mockExchangeClient.agentSetAbstraction).toHaveBeenCalledWith({ + abstraction: 'u', + }); + expect( + (TradingReadinessCache as jest.Mocked) + .set, + ).toHaveBeenCalledWith('mainnet', USER_ADDRESS, { + attempted: true, + enabled: true, + }); + }); + + it('does not query the multi-sig signer set when no migration write is needed', async () => { + // Accounts already on a compatible mode never reach a write, so they + // must not pay an extra Hyperliquid round trip. + const userToMultiSigSigners = jest.fn().mockResolvedValue(null); + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + userAbstraction: jest.fn().mockResolvedValue('unifiedAccount'), + userToMultiSigSigners, + }), + ); + + // Act + await provider.getMarketDataWithPrices(); + + // Assert + expect(userToMultiSigSigners).not.toHaveBeenCalled(); + }); + // ───────────────────────────────────────────────── // Signing-backed unifiedAccount migration on init // diff --git a/packages/perps-controller/tests/src/utils/errorUtils.test.ts b/packages/perps-controller/tests/src/utils/errorUtils.test.ts index fa1e8a1268..b82b5052df 100644 --- a/packages/perps-controller/tests/src/utils/errorUtils.test.ts +++ b/packages/perps-controller/tests/src/utils/errorUtils.test.ts @@ -1,6 +1,7 @@ import { isAbortError, ensureError, + isHyperLiquidMultiSigRequiredError, isHyperLiquidUserNotFoundError, isKeyringLockedError, } from '../../../src/utils/errorUtils.js'; @@ -124,4 +125,41 @@ describe('errorUtils', () => { ).toBe(false); }); }); + + describe('isHyperLiquidMultiSigRequiredError', () => { + it('returns true for both Hyperliquid multi-sig required spellings', () => { + // Hyperliquid is not consistent about the hyphen across endpoints, so + // both spellings must classify as the same benign condition. + expect( + isHyperLiquidMultiSigRequiredError( + new Error('ApiRequestError: Multi-sig required'), + ), + ).toBe(true); + expect( + isHyperLiquidMultiSigRequiredError( + new Error('ApiRequestError: Multisig required'), + ), + ).toBe(true); + }); + + it('returns true for non-Error rejections carrying the same message', () => { + expect(isHyperLiquidMultiSigRequiredError('multi-sig required')).toBe( + true, + ); + }); + + it('returns false for unrelated Hyperliquid and network errors', () => { + expect( + isHyperLiquidMultiSigRequiredError(new Error('Network error')), + ).toBe(false); + expect( + isHyperLiquidMultiSigRequiredError( + new Error( + 'User or API Wallet 0x340ed4af8642491fe02fa28403cad1a53268e510 does not exist.', + ), + ), + ).toBe(false); + expect(isHyperLiquidMultiSigRequiredError(undefined)).toBe(false); + }); + }); });