From e626973b334e70783b7509775efbfa137e7c5baa Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Thu, 1 Oct 2026 13:35:05 +0800 Subject: [PATCH 1/6] fix(perps-controller): detect agents rejected with "Must deposit", sign user actions for the signer's chain, stop re-prompting a refused builder fee - An agent-signed write that the venue answers with "Must deposit before performing actions" is a rejection when extraAgents no longer lists the agent (or lists it as expired): the agent is dropped, onAgentRejected is called and the write fails with KEYRING_LOCKED. Otherwise the venue error is returned as before. - Add optional PerpsAccountSigner.getChainId, passed to the SDK as the signatureChainId of user-signed actions. Without it nothing changes. - A builder fee approval the venue refuses for a reason signing again cannot fix is not requested again until disconnect. --- packages/perps-controller/CHANGELOG.md | 9 + packages/perps-controller/README.md | 8 +- .../src/providers/HyperLiquidProvider.ts | 332 +++++++++++---- .../src/services/HyperLiquidClientService.ts | 5 + .../src/services/HyperLiquidWalletService.ts | 61 ++- packages/perps-controller/src/types/index.ts | 18 +- .../tests/helpers/agentFixtures.ts | 14 + ...rLiquidProvider.agent-must-deposit.test.ts | 392 ++++++++++++++++++ ...LiquidProvider.builder-fee-refusal.test.ts | 151 +++++++ .../services/HyperLiquidClientService.test.ts | 12 + ...LiquidWalletService.account-signer.test.ts | 94 +++++ 11 files changed, 988 insertions(+), 108 deletions(-) create mode 100644 packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts create mode 100644 packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 81b930dd8c9..ddc0cf00e88 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `PerpsAccountSigner.getChainId`. When set, HyperLiquid user-signed actions (builder fee approval, withdrawals, transfers) are signed for the chain it returns instead of chain 1, so a wallet that only signs for its connected chain does not switch chains ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) + ### Changed - **BREAKING:** `OrderFill.pnl` is optional when the venue omits realized PnL. Consumers must preserve missing amounts as unknown when aggregating or displaying fills; only a reported `'0'` is zero ([#10605](https://github.com/MetaMask/core/pull/10605)) @@ -14,6 +18,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Accept Lighter trades that omit position-sign flags, preserve omitted account PnL as unknown, and reject known reductions without realized PnL. Retain side-only fill directions when lifecycle context is ambiguous ([#10605](https://github.com/MetaMask/core/pull/10605)) +- Detect a revoked or expired HyperLiquid agent that the venue answers with "Must deposit before performing actions" ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) + - The provider then checks `extraAgents` for the main account. When the agent is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called + - When the agent is still listed (an account with no funds), or the list cannot be read, the venue error is returned as before +- Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) + - Until the provider disconnects, the approval is not requested again: orders are sent as after any failed approval, TP/SL updates fail with `TPSL_UPDATE_FAILED`, and `prepareTradingWallet` returns the venue error ## [19.0.0] diff --git a/packages/perps-controller/README.md b/packages/perps-controller/README.md index 1bf4e25ee94..b07938e876a 100644 --- a/packages/perps-controller/README.md +++ b/packages/perps-controller/README.md @@ -70,9 +70,11 @@ increase or reduce the existing position. By default the controller signs through the `KeyringController:*` messenger actions. A client without a keyring passes `accountSigner` in its platform -dependencies (`signTypedData`, `signPersonalMessage`, optional `isReady` and -`requiresSignatureConfirmation`); the signing address still comes from the -selected account, and a signer that is not ready fails with `KEYRING_LOCKED`. +dependencies (`signTypedData`, `signPersonalMessage`, optional `isReady`, +`requiresSignatureConfirmation` and `getChainId`); the signing address still +comes from the selected account, and a signer that is not ready fails with +`KEYRING_LOCKED`. HyperLiquid user-signed actions are signed for the chain +`getChainId` returns, or for chain 1 without it. HyperLiquid L1 actions (orders, cancels, leverage, ...) can be signed by a client-owned agent key: return it from diff --git a/packages/perps-controller/src/providers/HyperLiquidProvider.ts b/packages/perps-controller/src/providers/HyperLiquidProvider.ts index b03319169af..198fe3d4b77 100644 --- a/packages/perps-controller/src/providers/HyperLiquidProvider.ts +++ b/packages/perps-controller/src/providers/HyperLiquidProvider.ts @@ -488,6 +488,40 @@ const getCancelStatusesFromError = ( // The address in HyperLiquid's "User or API Wallet 0x... does not exist." const UNKNOWN_WALLET_ADDRESS_PATTERN = /user or api wallet (0x[0-9a-f]{40})/iu; +// The account in HyperLiquid's "Must deposit before performing actions. +// User: 0x...", which names the main account, not the signer. +const MUST_DEPOSIT_USER_PATTERN = + /must deposit before performing actions\. user: (0x[0-9a-f]{40})/iu; + +// Builder fee approval refusals the venue repeats whoever signs, such as a +// builder without funds: signing the approval again cannot help. +const BUILDER_FEE_REFUSALS = ['builder has insufficient balance']; + +// An agent's L1 signature: the account and network it signed for. +type AgentSignature = { + key: string; + account: PerpsAgentAccount; + agentAddress: Hex; + // Whether the venue still lists the agent, asked at most once per + // signature (see #findRevokedAgent). + approvalCheck?: Promise; +}; + +/** + * The venue's refusal of a builder fee approval that signing again cannot + * fix. + * + * @param error - The approval error. + * @returns The venue's message, or undefined for another error. + */ +function getBuilderFeeRefusal(error: unknown): string | undefined { + const { message } = ensureError(error, 'HyperLiquidProvider.builderFee'); + const lower = message.toLowerCase(); + return BUILDER_FEE_REFUSALS.some((refusal) => lower.includes(refusal)) + ? message + : undefined; +} + /** * Exchange messages that mean a cancel was refused because the order is not on * the book any more. @@ -1617,10 +1651,11 @@ export class HyperLiquidProvider implements PerpsProvider { // the write was in flight; an L1 signature names no user, so one agent acts // for one account. Kept across clearAgentSigners, so an agent replaced while // its action was in flight is still recognized. - readonly #agentSignedFor = new Map< - string, - { key: string; account: PerpsAgentAccount; agentAddress: Hex } - >(); + readonly #agentSignedFor = new Map(); + + // The agent that signed the latest L1 action per network and main account + // (see getAgentAccountKey); none when the main account signed it. + readonly #latestAgentSignatures = new Map(); readonly #onAgentRejected: HyperLiquidProviderOptions['onAgentRejected']; @@ -2249,13 +2284,12 @@ export class HyperLiquidProvider implements PerpsProvider { pendingEntry.agent = agentSigner; // The wallet adapter resolves at every L1 signature, so this records // the account the agent is about to sign for. - this.#agentSignedFor.set(agentSigner.address.toLowerCase(), { - key, - account, - agentAddress: agentSigner.address, - }); + const signature = { key, account, agentAddress: agentSigner.address }; + this.#agentSignedFor.set(agentSigner.address.toLowerCase(), signature); + this.#latestAgentSignatures.set(key, signature); } else { this.#agentSigners.delete(key); + this.#latestAgentSignatures.delete(key); } return agentSigner; } @@ -2275,7 +2309,7 @@ export class HyperLiquidProvider implements PerpsProvider { method: string, extra: Record, ): Promise { - const signerFailure = this.#handleSignerFailure(error, method, extra); + const signerFailure = await this.#handleSignerFailure(error, method, extra); if (signerFailure) { return signerFailure; } @@ -2291,29 +2325,95 @@ export class HyperLiquidProvider implements PerpsProvider { * The agent a venue rejection names, with the account and network it last * signed for. HyperLiquid answers "User or API Wallet 0x... does not * exist." with the signer's address, so for a revoked or expired agent it - * reads like a wallet with no account. + * reads like a wallet with no account. It sometimes answers "Must deposit + * before performing actions. User: 0x..." with the main account instead + * (see #findRevokedAgent). * * @param error - The caught error. * @returns The rejected agent, with its address as the host supplied it * and the account it signed for, or undefined. */ - #findRejectedAgent( + async #findRejectedAgent( error: unknown, - ): - | { account: PerpsAgentAccount; key: string; agentAddress: Hex } - | undefined { - if ( - this.#agentSignedFor.size === 0 || - !isHyperLiquidUserNotFoundError(error) - ) { + ): Promise { + if (this.#agentSignedFor.size === 0) { return undefined; } - const reportedAddress = UNKNOWN_WALLET_ADDRESS_PATTERN.exec( - ensureError(error, 'HyperLiquidProvider.findRejectedAgent').message, - )?.[1]; - return reportedAddress === undefined + const { message } = ensureError( + error, + 'HyperLiquidProvider.findRejectedAgent', + ); + if (isHyperLiquidUserNotFoundError(error)) { + const reportedAddress = UNKNOWN_WALLET_ADDRESS_PATTERN.exec(message)?.[1]; + return reportedAddress === undefined + ? undefined + : this.#agentSignedFor.get(reportedAddress.toLowerCase()); + } + const mainAddress = MUST_DEPOSIT_USER_PATTERN.exec(message)?.[1]; + return mainAddress === undefined ? undefined - : this.#agentSignedFor.get(reportedAddress.toLowerCase()); + : await this.#findRevokedAgent(mainAddress as Hex); + } + + /** + * The agent that signed the latest L1 action for a main account the venue + * answered with "Must deposit before performing actions", if the venue no + * longer lists it (revoked, or past its `validUntil`). The venue answers an + * account with no funds the same way, so the agent list decides; it is + * asked once per signature. + * + * @param mainAddress - The main account the venue names. + * @returns The revoked agent, or undefined. + */ + async #findRevokedAgent( + mainAddress: Hex, + ): Promise { + const latest = this.#latestAgentSignatures.get( + getAgentAccountKey({ + mainAddress, + isTestnet: this.#clientService.isTestnetMode(), + }), + ); + if (!latest) { + return undefined; + } + latest.approvalCheck ??= this.#isAgentListed(latest); + return (await latest.approvalCheck) ? undefined : latest; + } + + /** + * Whether the venue lists an agent as approved for its main account. + * + * @param agent - The agent and the account it signed for. + * @param agent.account - The account the agent signed for. + * @param agent.agentAddress - The agent. + * @returns False when the agent is missing or expired. True when it is + * listed, or when the list cannot be read, so the venue's error stands. + */ + async #isAgentListed({ + account, + agentAddress, + }: { + account: PerpsAgentAccount; + agentAddress: Hex; + }): Promise { + try { + const agents = await this.#clientService + .getInfoClient({ useHttp: true }) + .extraAgents({ user: account.mainAddress }); + const listed = agents.find( + ({ address }) => address.toLowerCase() === agentAddress.toLowerCase(), + ); + return ( + listed !== undefined && + (listed.validUntil === null || listed.validUntil > Date.now()) + ); + } catch (error) { + this.#deps.debugLogger.log('HyperLiquidProvider: extraAgents failed', { + error: ensureError(error, 'HyperLiquidProvider.isAgentListed').message, + }); + return true; + } } /** @@ -2324,8 +2424,8 @@ export class HyperLiquidProvider implements PerpsProvider { * @param error - The caught error. * @returns True when the error was the rejection of an agent. */ - #evictRejectedAgent(error: unknown): boolean { - const rejected = this.#findRejectedAgent(error); + async #evictRejectedAgent(error: unknown): Promise { + const rejected = await this.#findRejectedAgent(error); if (!rejected) { return false; } @@ -2358,11 +2458,11 @@ export class HyperLiquidProvider implements PerpsProvider { * @param error - The caught error. * @returns True for a signer failure. */ - #isSignerFailure(error: unknown): boolean { + async #isSignerFailure(error: unknown): Promise { return ( isKeyringLockedError(error) || isAgentSignerUnavailableError(error) || - this.#findRejectedAgent(error) !== undefined + (await this.#findRejectedAgent(error)) !== undefined ); } @@ -2373,8 +2473,8 @@ export class HyperLiquidProvider implements PerpsProvider { * @param error - The caught error. * @returns `KEYRING_LOCKED` for a signer failure, else undefined. */ - #classifySignerFailure(error: unknown): Error | undefined { - const rejectedAgent = this.#evictRejectedAgent(error); + async #classifySignerFailure(error: unknown): Promise { + const rejectedAgent = await this.#evictRejectedAgent(error); return rejectedAgent || isKeyringLockedError(error) || isAgentSignerUnavailableError(error) @@ -2391,12 +2491,12 @@ export class HyperLiquidProvider implements PerpsProvider { * @param extra - Context for the debug log. * @returns `KEYRING_LOCKED` for a signer failure, else undefined. */ - #handleSignerFailure( + async #handleSignerFailure( error: unknown, method: string, extra: Record, - ): Error | undefined { - const signerFailure = this.#classifySignerFailure(error); + ): Promise { + const signerFailure = await this.#classifySignerFailure(error); if (signerFailure) { this.#logRetryableSignerFailure(method, extra); } @@ -2795,7 +2895,7 @@ export class HyperLiquidProvider implements PerpsProvider { // The signer could not sign (a locked keyring, or an unavailable or // rejected agent; HyperLiquid keeps the cause in `cause`): leave the // cache empty and retry later. - if (this.#classifySignerFailure(error)) { + if (await this.#classifySignerFailure(error)) { this.#deps.debugLogger.log( '[ensureUnifiedAccountEnabled] Signer unavailable, will retry later', ); @@ -2997,11 +3097,19 @@ export class HyperLiquidProvider implements PerpsProvider { readonly #builderFeeSetupPromises = new Map>(); + // The venue's refusal of an approval per network, account and builder, + // when signing again cannot fix it. Kept until disconnect, so the approval + // is not asked for again. + readonly #builderFeeRefusals = new Map(); + /** * Approve the standard builder fee once for the current account and network. * TWAP never calls this because its native action has no builder field. * Without an approval failure code, approval remains non-blocking and the * returned context describes attribution only; it does not prove approval. + * An approval the venue refused for a reason signing again cannot fix is + * not asked for again until disconnect; it then fails as a failed approval + * does. * * @param approvalFailureCode - Operation-specific error to throw when * approval is unavailable or fails. @@ -3009,12 +3117,15 @@ export class HyperLiquidProvider implements PerpsProvider { * @param options.reportSignerFailure - Throw `KEYRING_LOCKED` when the * signer could not sign the approval, even without an approval failure * code (the approval is otherwise non-blocking). + * @param options.reportRefusal - Throw the venue's refusal, even without an + * approval failure code. * @returns The account, network, and configured builder for the action. */ async #ensureBuilderFeeSetup( approvalFailureCode?: PerpsErrorCode, - options: { reportSignerFailure?: boolean } = {}, + options: { reportSignerFailure?: boolean; reportRefusal?: boolean } = {}, ): Promise { + const lifecycleGeneration = this.#lifecycleGeneration; const isTestnet = this.#clientService.isTestnetMode(); const network = isTestnet ? 'testnet' : 'mainnet'; const userAddress = await this.#walletService.getUserAddressWithDefault(); @@ -3034,12 +3145,6 @@ export class HyperLiquidProvider implements PerpsProvider { return context; } - let pendingApproval = this.#builderFeeSetupPromises.get(setupKey); - if (!pendingApproval) { - pendingApproval = this.#ensureBuilderFeeApproval(context); - this.#builderFeeSetupPromises.set(setupKey, pendingApproval); - } - // An approval the signer could not sign is retryable, not a failure. const approvalFailure = (code: PerpsErrorCode): Error => new Error( @@ -3048,6 +3153,23 @@ export class HyperLiquidProvider implements PerpsProvider { : PERPS_ERROR_CODES.KEYRING_LOCKED, ); + const knownRefusal = this.#builderFeeRefusals.get(setupKey); + if (knownRefusal) { + if (approvalFailureCode) { + throw approvalFailure(approvalFailureCode); + } + if (options.reportRefusal) { + throw new Error(knownRefusal); + } + return context; + } + + let pendingApproval = this.#builderFeeSetupPromises.get(setupKey); + if (!pendingApproval) { + pendingApproval = this.#ensureBuilderFeeApproval(context); + this.#builderFeeSetupPromises.set(setupKey, pendingApproval); + } + try { await pendingApproval; } catch (error) { @@ -3057,14 +3179,22 @@ export class HyperLiquidProvider implements PerpsProvider { ); // A signer that could not sign is retryable, not an approval failure. if (approvalFailureCode || options.reportSignerFailure) { - const signerFailure = this.#classifySignerFailure(error); + const signerFailure = await this.#classifySignerFailure(error); if (signerFailure) { throw signerFailure; } } + const refusal = getBuilderFeeRefusal(error); + // A refusal that settles after a disconnect belongs to the old session. + if (refusal && lifecycleGeneration === this.#lifecycleGeneration) { + this.#builderFeeRefusals.set(setupKey, refusal); + } if (approvalFailureCode) { throw approvalFailure(approvalFailureCode); } + if (refusal && options.reportRefusal) { + throw new Error(refusal); + } } finally { if (this.#builderFeeSetupPromises.get(setupKey) === pendingApproval) { this.#builderFeeSetupPromises.delete(setupKey); @@ -4442,9 +4572,9 @@ export class HyperLiquidProvider implements PerpsProvider { * @param message - The status entry's error. * @returns The mapped error. */ - #mapStatusError(message: string): Error { + async #mapStatusError(message: string): Promise { const error = new Error(message); - return this.#classifySignerFailure(error) ?? this.#mapError(error); + return (await this.#classifySignerFailure(error)) ?? this.#mapError(error); } /** @@ -4455,10 +4585,12 @@ export class HyperLiquidProvider implements PerpsProvider { * @param statuses - The status entries. * @returns `KEYRING_LOCKED` for a signer failure, else undefined. */ - #classifyStatusSignerFailure(statuses: unknown[]): Error | undefined { + async #classifyStatusSignerFailure( + statuses: unknown[], + ): Promise { for (const status of statuses) { if (isStatusObject(status) && typeof status.error === 'string') { - const signerFailure = this.#classifySignerFailure( + const signerFailure = await this.#classifySignerFailure( new Error(status.error), ); if (signerFailure) { @@ -5931,7 +6063,7 @@ export class HyperLiquidProvider implements PerpsProvider { params: HandleOrderErrorParams, ): Promise { const { error, symbol, orderType, isBuy } = params; - const signerFailure = this.#handleSignerFailure(error, 'placeOrder', { + const signerFailure = await this.#handleSignerFailure(error, 'placeOrder', { symbol, orderType, isBuy, @@ -6738,13 +6870,13 @@ export class HyperLiquidProvider implements PerpsProvider { isStatusObject(cancelStatus) && typeof cancelStatus.error === 'string' ) { - this.#evictRejectedAgent(new Error(cancelStatus.error)); + await this.#evictRejectedAgent(new Error(cancelStatus.error)); } remainsLive = classifyCancelStatus(cancelStatus) === CancelChildOutcome.Refused; } catch (error) { // A rejected agent is dropped and reported; the TWAP stays live. - this.#evictRejectedAgent(error); + await this.#evictRejectedAgent(error); this.#deps.debugLogger.log( 'Stale TWAP placement could not be retracted', { @@ -7415,9 +7547,9 @@ export class HyperLiquidProvider implements PerpsProvider { session.timer = null; this.#chaseTickQueue = this.#chaseTickQueue .then(() => this.#runChaseTick(sessionId)) - .catch((error: unknown) => { + .catch(async (error: unknown) => { // A rejected agent is dropped so the next tick asks for another. - this.#evictRejectedAgent(error); + await this.#evictRejectedAgent(error); // Resolve the shared queue after every failure. Otherwise one // rejected tick prevents all later ticks and teardown from running. this.#deps.debugLogger.log('Chase tick failed', { @@ -8068,7 +8200,7 @@ export class HyperLiquidProvider implements PerpsProvider { } const status: unknown = result.response?.data?.statuses?.[0]; - const signerFailure = this.#classifyStatusSignerFailure([status]); + const signerFailure = await this.#classifyStatusSignerFailure([status]); if (signerFailure) { throw signerFailure; } @@ -8102,7 +8234,7 @@ export class HyperLiquidProvider implements PerpsProvider { return outcome; } catch (error) { // A rejected agent is dropped and reported; the order stays resting. - this.#evictRejectedAgent(error); + await this.#evictRejectedAgent(error); this.#deps.debugLogger.log('Could not retract abandoned chase order', { orderId: session.orderId, error: ensureError(error, 'HyperLiquidProvider.startChaseSession') @@ -8664,7 +8796,7 @@ export class HyperLiquidProvider implements PerpsProvider { isStatusObject(status) && typeof status.error === 'string' ? status.error : 'TWAP cancellation failed'; - return createErrorResult(this.#mapStatusError(rawError), { + return createErrorResult(await this.#mapStatusError(rawError), { success: false, orderId: params.orderId, }); @@ -8899,12 +9031,12 @@ export class HyperLiquidProvider implements PerpsProvider { return { remainingClientOrderIds: [] }; } - const classifyStatuses = ( + const classifyStatuses = async ( statuses: unknown[], - ): { remainingClientOrderIds: Hex[]; signerFailure?: Error } => { + ): Promise<{ remainingClientOrderIds: Hex[]; signerFailure?: Error }> => { // A signer failure is classified (and reported) once for the request; // entries the venue cancelled are not resting either way. - const signerFailure = this.#classifyStatusSignerFailure(statuses); + const signerFailure = await this.#classifyStatusSignerFailure(statuses); return { remainingClientOrderIds: requests.flatMap((request, index) => classifyCancelStatus(statuses[index]) === CancelChildOutcome.Refused @@ -8926,15 +9058,15 @@ export class HyperLiquidProvider implements PerpsProvider { }; } - return classifyStatuses(statuses); + return await classifyStatuses(statuses); } catch (error) { // The SDK throws when any entry failed, with every entry's status. const statuses = getCancelStatusesFromError(error, requests.length); if (statuses) { - return classifyStatuses(statuses); + return await classifyStatuses(statuses); } // The signer could not sign, so nothing was cancelled. - const signerFailure = this.#classifySignerFailure(error); + const signerFailure = await this.#classifySignerFailure(error); if (signerFailure) { return { remainingClientOrderIds: requests.map((request) => request.cloid), @@ -8974,10 +9106,12 @@ export class HyperLiquidProvider implements PerpsProvider { }; } - const classifyStatuses = (statuses: unknown[]): CancelOrderBatchOutcome => { + const classifyStatuses = async ( + statuses: unknown[], + ): Promise => { // A signer failure is classified (and reported) once for the request; // entries the venue cancelled still count as cancelled. - const signerFailure = this.#classifyStatusSignerFailure(statuses); + const signerFailure = await this.#classifyStatusSignerFailure(statuses); const remainingOrderIds: number[] = []; const cancelledOrderIds: number[] = []; requests.forEach((request, index) => { @@ -9007,15 +9141,15 @@ export class HyperLiquidProvider implements PerpsProvider { }; } - return classifyStatuses(statuses); + return await classifyStatuses(statuses); } catch (error) { // The SDK throws when any entry failed, with every entry's status. const statuses = getCancelStatusesFromError(error, requests.length); if (statuses) { - return classifyStatuses(statuses); + return await classifyStatuses(statuses); } // The signer could not sign, so nothing was cancelled. - const signerFailure = this.#classifySignerFailure(error); + const signerFailure = await this.#classifySignerFailure(error); if (signerFailure) { return { remainingOrderIds: requests.map((request) => request.o), @@ -9631,9 +9765,13 @@ export class HyperLiquidProvider implements PerpsProvider { : { orderId: replacementOrderId }), }; } catch (error) { - const signerFailure = this.#handleSignerFailure(error, 'editOrder', { - orderId: params.orderId, - }); + const signerFailure = await this.#handleSignerFailure( + error, + 'editOrder', + { + orderId: params.orderId, + }, + ); if (signerFailure) { return createErrorResult(signerFailure, { success: false }); } @@ -9735,7 +9873,7 @@ export class HyperLiquidProvider implements PerpsProvider { ? status.error : 'Order cancellation failed'; - return createErrorResult(this.#mapStatusError(rawError), { + return createErrorResult(await this.#mapStatusError(rawError), { success: false, orderId: params.orderId, }); @@ -9857,8 +9995,12 @@ export class HyperLiquidProvider implements PerpsProvider { if (statuses) { // One signature covers the batch, so a signer failure is classified // (and reported to the host) once. Each entry keeps its own result. - const signerFailure = this.#classifyStatusSignerFailure(statuses); - ordinaryOrders.forEach(({ index, order }, statusIndex) => { + const signerFailure = + await this.#classifyStatusSignerFailure(statuses); + for (const [ + statusIndex, + { index, order }, + ] of ordinaryOrders.entries()) { const status: unknown = statuses[statusIndex]; const success = status === 'success'; const statusError = @@ -9868,7 +10010,7 @@ export class HyperLiquidProvider implements PerpsProvider { let error: string = PERPS_ERROR_CODES.BATCH_CANCEL_FAILED; if (statusError) { error = - signerFailure && this.#isSignerFailure(statusError) + signerFailure && (await this.#isSignerFailure(statusError)) ? signerFailure.message : this.#mapError(statusError).message; } @@ -9878,7 +10020,7 @@ export class HyperLiquidProvider implements PerpsProvider { success, ...(success ? {} : { error }), }; - }); + } } } } catch (error) { @@ -10247,9 +10389,13 @@ export class HyperLiquidProvider implements PerpsProvider { ], }; } catch (error) { - const signerFailure = this.#handleSignerFailure(error, 'closePositions', { - positionCount: positionsToClose.length, - }); + const signerFailure = await this.#handleSignerFailure( + error, + 'closePositions', + { + positionCount: positionsToClose.length, + }, + ); const safeError = signerFailure ?? ensureError(error, 'HyperLiquidProvider.closePositions'); @@ -10837,10 +10983,10 @@ export class HyperLiquidProvider implements PerpsProvider { // A signer that could not sign is retryable, not a defect; the // caller still learns the protection was lost. if ( - !this.#handleSignerFailure(error, 'updatePositionTPSL', { + !(await this.#handleSignerFailure(error, 'updatePositionTPSL', { symbol, grouping: protection.grouping, - }) + })) ) { this.#deps.logger.error( ensureError( @@ -10979,7 +11125,7 @@ export class HyperLiquidProvider implements PerpsProvider { } catch (error) { // Classify first, so a rejected agent is dropped (and reported) before // the restoration signs. - const signerFailure = this.#classifySignerFailure(error); + const signerFailure = await this.#classifySignerFailure(error); const restoration = await restoreCancelledProtection( confirmedCancelledOldOrderIds, ); @@ -11086,7 +11232,7 @@ export class HyperLiquidProvider implements PerpsProvider { } throw new Error(PERPS_ERROR_CODES.TPSL_UPDATE_FAILED); } catch (error) { - const signerFailure = this.#handleSignerFailure( + const signerFailure = await this.#handleSignerFailure( error, 'updatePositionTPSL', { symbol: params.symbol }, @@ -11381,9 +11527,13 @@ export class HyperLiquidProvider implements PerpsProvider { success: true, }; } catch (error) { - const signerFailure = this.#handleSignerFailure(error, 'updateMargin', { - symbol: params.symbol, - }); + const signerFailure = await this.#handleSignerFailure( + error, + 'updateMargin', + { + symbol: params.symbol, + }, + ); if (signerFailure) { return { success: false, error: signerFailure.message }; } @@ -14315,7 +14465,7 @@ export class HyperLiquidProvider implements PerpsProvider { error: errorMessage, }; } catch (error) { - const signerFailure = this.#handleSignerFailure(error, 'withdraw', { + const signerFailure = await this.#handleSignerFailure(error, 'withdraw', { assetId: params.assetId, }); if (signerFailure) { @@ -14428,7 +14578,7 @@ export class HyperLiquidProvider implements PerpsProvider { throw new Error(PERPS_ERROR_CODES.TRANSFER_FAILED); } catch (error) { - const signerFailure = this.#handleSignerFailure( + const signerFailure = await this.#handleSignerFailure( error, 'transferBetweenDexs', { sourceDex: params.sourceDex, destinationDex: params.destinationDex }, @@ -14810,6 +14960,7 @@ export class HyperLiquidProvider implements PerpsProvider { } await this.#ensureBuilderFeeSetup(undefined, { reportSignerFailure: true, + reportRefusal: true, }); // The builder fee setup ends quietly when the provider disconnects. await assertPreparationCurrent(); @@ -14844,6 +14995,12 @@ export class HyperLiquidProvider implements PerpsProvider { this.#deps.debugLogger.log( '[prepareTradingWallet] No account selected', ); + } else if (getBuilderFeeRefusal(caughtError)) { + // Signing again cannot fix it, and orders are not blocked by it. + this.#deps.debugLogger.log( + '[prepareTradingWallet] Builder fee approval refused', + { error: caughtError.message }, + ); } else { this.#deps.logger.error( caughtError, @@ -15636,6 +15793,7 @@ export class HyperLiquidProvider implements PerpsProvider { this.clearFeeCache(); this.#referralCheckCache.clear(); this.#builderFeeCheckCache.clear(); + this.#builderFeeRefusals.clear(); this.#subscriptionBuilderApprovalEpoch += 1; this.#approvedBuilderAddresses.clear(); this.#userFeeResolution = undefined; @@ -16068,7 +16226,7 @@ export class HyperLiquidProvider implements PerpsProvider { // The signer could not sign (a locked keyring, or an unavailable or // rejected agent; HyperLiquid keeps the cause in `cause`): leave the // cache empty and attempt the referral again at the next setup. - if (this.#classifySignerFailure(error)) { + if (await this.#classifySignerFailure(error)) { this.#deps.debugLogger.log( '[ensureReferralSet] Signer unavailable, will retry later', ); @@ -16242,7 +16400,7 @@ export class HyperLiquidProvider implements PerpsProvider { // Retryable (a locked keyring, an unavailable agent, or one the venue // rejected): `#ensureReferralSet` retries at the next entry, so it is // not an error to report. - if (this.#isSignerFailure(error)) { + if (await this.#isSignerFailure(error)) { throw error; } // Benign for unfunded wallets — downgrade and rethrow so the outer diff --git a/packages/perps-controller/src/services/HyperLiquidClientService.ts b/packages/perps-controller/src/services/HyperLiquidClientService.ts index 7549943d0fb..0953daeed5f 100644 --- a/packages/perps-controller/src/services/HyperLiquidClientService.ts +++ b/packages/perps-controller/src/services/HyperLiquidClientService.ts @@ -50,6 +50,8 @@ export type HyperLiquidWalletParams = { address: Hex; signTypedData: (params: PerpsTypedDataPayload) => Promise; getChainId?: () => Promise; + /** The chain user-signed actions are signed for. When omitted, the SDK uses 1. */ + signatureChainId?: () => Promise; }; // WebSocketConnectionState is now imported from controllers/types @@ -355,6 +357,9 @@ export class HyperLiquidClientService { this.#exchangeClient = new ExchangeClient({ wallet: effectiveWallet as any, // eslint-disable-line @typescript-eslint/no-explicit-any -- Type widening for SDK compatibility transport: this.#httpTransport, + ...(effectiveWallet.signatureChainId && { + signatureChainId: effectiveWallet.signatureChainId, + }), }); } else { this.#exchangeClient = undefined; diff --git a/packages/perps-controller/src/services/HyperLiquidWalletService.ts b/packages/perps-controller/src/services/HyperLiquidWalletService.ts index a58ad40bd63..a18ca0e503e 100644 --- a/packages/perps-controller/src/services/HyperLiquidWalletService.ts +++ b/packages/perps-controller/src/services/HyperLiquidWalletService.ts @@ -1,6 +1,7 @@ import { hasProperty, isValidHexAddress, + numberToHex, parseCaipAccountId, } from '@metamask/utils'; import type { CaipAccountId, Hex } from '@metamask/utils'; @@ -12,6 +13,7 @@ import { } from '../constants/hyperLiquidConfig.js'; import { PERPS_ERROR_CODES } from '../perpsErrorCodes.js'; import type { + PerpsAccountSigner, PerpsAgentSigner, PerpsPlatformDependencies, PerpsTypedDataPayload, @@ -183,18 +185,10 @@ export class HyperLiquidWalletService { const { accountSigner } = this.#deps; if (accountSigner) { - if (!isAccountSignerReady(accountSigner)) { - throw new Error(PERPS_ERROR_CODES.KEYRING_LOCKED); - } - try { - return await accountSigner.signTypedData(mainAddress, params); - } catch (error) { - // A signer that locked while signing throws its own error. - if (!isAccountSignerReady(accountSigner)) { - throw new Error(PERPS_ERROR_CODES.KEYRING_LOCKED, { cause: error }); - } - throw error; - } + return await this.#callAccountSigner( + accountSigner, + async () => await accountSigner.signTypedData(mainAddress, params), + ); } const signature = await this.#signTypedMessage({ @@ -205,6 +199,32 @@ export class HyperLiquidWalletService { return signature as Hex; } + /** + * Call the account signer, failing with `KEYRING_LOCKED` when it cannot + * sign, before the call or while it runs. + * + * @param accountSigner - The injected account signer. + * @param call - The call to make. + * @returns The call's result. + */ + async #callAccountSigner( + accountSigner: PerpsAccountSigner, + call: () => Promise, + ): Promise { + if (!isAccountSignerReady(accountSigner)) { + throw new Error(PERPS_ERROR_CODES.KEYRING_LOCKED); + } + try { + return await call(); + } catch (error) { + // A signer that locked during the call throws its own error. + if (!isAccountSignerReady(accountSigner)) { + throw new Error(PERPS_ERROR_CODES.KEYRING_LOCKED, { cause: error }); + } + throw error; + } + } + /** * Create the wallet adapter the HyperLiquid SDK signs with. * @@ -212,11 +232,15 @@ export class HyperLiquidWalletService { * agent resolver returns an agent for that account, L1 actions (orders, * cancels, leverage, ...) are signed by the agent. User-signed actions * (builder fee, withdraw, ...) authorize the main account itself, so the - * main account always signs them. + * main account always signs them, for the chain the account signer reports + * when it reports one. * - * @returns The wallet adapter with address, signTypedData, and getChainId methods. + * @returns The wallet adapter with address, signTypedData, and getChainId + * methods, and signatureChainId when the account signer has getChainId. */ public createWalletAdapter(): HyperLiquidWalletParams { + const { accountSigner } = this.#deps; + const getSignerChainId = accountSigner?.getChainId?.bind(accountSigner); return { address: this.#getSelectedMainAddress(), signTypedData: async (params: PerpsTypedDataPayload): Promise => { @@ -242,6 +266,15 @@ export class HyperLiquidWalletService { }, getChainId: async (): Promise => parseInt(getChainId(this.#isTestnet), 10), + ...(accountSigner && + getSignerChainId && { + // Asked just before the main account signs, so it fails the same + // way when the signer cannot sign. + signatureChainId: async (): Promise => + numberToHex( + await this.#callAccountSigner(accountSigner, getSignerChainId), + ), + }), }; } diff --git a/packages/perps-controller/src/types/index.ts b/packages/perps-controller/src/types/index.ts index 38d1bed141c..f6fb48d796d 100644 --- a/packages/perps-controller/src/types/index.ts +++ b/packages/perps-controller/src/types/index.ts @@ -2697,10 +2697,10 @@ export type PerpsAccountSigner = { * Sign EIP-712 typed data as `address`, exactly as given. HyperLiquid's * `domain.chainId` is not the wallet's connected chain: L1 actions signed * without an agent use 1337, and user-signed actions (builder fee, - * withdraw, ...) use 1. A wallet that only signs for its connected chain - * (many EIP-1193 wallets) must route L1 actions through an agent (see - * `providerCredentials.hyperliquid.getAgentSigner`) and still has to sign - * user-signed actions with chain ID 1. + * withdraw, ...) use 1 unless `getChainId` is set. A wallet that only signs + * for its connected chain (many EIP-1193 wallets) must route L1 actions + * through an agent (see `providerCredentials.hyperliquid.getAgentSigner`) + * and set `getChainId`. * * @param address - The account that signs. * @param payload - The typed data to sign. @@ -2732,6 +2732,16 @@ export type PerpsAccountSigner = { * omitted, the selected account's keyring type decides. */ requiresSignatureConfirmation?(): boolean; + + /** + * The chain the signer signs for, such as a browser wallet's connected + * chain. HyperLiquid user-signed actions are then signed for it instead of + * chain 1, so the wallet does not switch chains to sign them. Read at + * every user-signed action. + * + * @returns The chain ID. + */ + getChainId?(): Promise; }; /** diff --git a/packages/perps-controller/tests/helpers/agentFixtures.ts b/packages/perps-controller/tests/helpers/agentFixtures.ts index d02eb375654..49a0eb0de58 100644 --- a/packages/perps-controller/tests/helpers/agentFixtures.ts +++ b/packages/perps-controller/tests/helpers/agentFixtures.ts @@ -156,6 +156,20 @@ export function unknownWalletError(address: string): Error { return new Error(`User or API Wallet ${address} does not exist.`); } +/** + * HyperLiquid's rejection of a write for an account with no funds. The venue + * also answers an L1 action signed by a revoked or expired agent this way, + * naming the main account the agent was approved for. + * + * @param address - The main account the venue names. + * @returns The venue error. + */ +export function mustDepositError(address: string): Error { + return new Error( + `Must deposit before performing actions. User: ${address.toLowerCase()}`, + ); +} + /** * Sign through a wallet the way the HyperLiquid SDK does: a failure is * wrapped with the wallet's error as its cause. diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts new file mode 100644 index 00000000000..bb6690def3d --- /dev/null +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts @@ -0,0 +1,392 @@ +import { getChecksumAddress } from '@metamask/utils'; + +import { PERPS_ERROR_CODES } from '../../../src/perpsErrorCodes.js'; +import { + AGENT_ADDRESS, + L1_PAYLOAD, + MAINNET_ACCOUNT, + MAIN_ADDRESS, + OTHER_MAIN_ADDRESS, + mustDepositError, +} from '../../helpers/agentFixtures.js'; +import { + BTC_MARKET_ORDER, + CANCEL_DELIVERIES, + REFERRAL_WRITE, + cancelStatusesResponse, + createAccountSignerProvider, + referralAttempted, + setUpAccountSignerSuite, +} from '../../helpers/hyperLiquidAccountSignerFixture.js'; +import type { AccountSignerFixture } from '../../helpers/hyperLiquidAccountSignerFixture.js'; + +// The SDK ships ES modules only; the provider reaches it through the mocked +// client service, so the module itself is never loaded. The provider checks +// cancel errors against its error class. +jest.mock('@nktkas/hyperliquid', () => ({ + HyperliquidError: class MockHyperliquidError extends Error {}, +})); + +// The client and subscription services are mocked: they own the SDK's +// REST/exchange/info clients and the WebSocket subscriptions. The wallet +// service, the signing caches and the validation run for real. +jest.mock('../../../src/services/HyperLiquidClientService'); +jest.mock('../../../src/services/HyperLiquidSubscriptionService'); + +const HOUR_MS = 60 * 60 * 1000; + +type ListedAgent = { + address: string; + name: string; + validUntil: number | null; +}; + +/** + * The agent as `extraAgents` lists it. + * + * @param validUntil - When the approval ends, in milliseconds, or null for + * no expiry. + * @param address - The agent address as the venue reports it. + * @returns The listing entry. + */ +function listedAgent( + validUntil: number | null, + address: string = AGENT_ADDRESS, +): ListedAgent { + return { address, name: 'agent', validUntil }; +} + +describe('HyperLiquidProvider with accountSigner: an agent the venue answers with "Must deposit"', () => { + let loggerError: jest.SpyInstance; + + beforeEach(() => { + ({ loggerError } = setUpAccountSignerSuite()); + }); + + /** + * A provider whose L1 writes are signed by the agent, then refused by the + * venue as an account that must deposit. + * + * @param write - The exchange write that fails. + * @param agents - Answers `extraAgents` for the main account. + * @param user - The main account the venue names. + * @returns The provider, its mocks and the rejected agent. + */ + function createMustDepositProvider( + write: 'order' | 'cancel' | 'setReferrer', + agents: () => Promise, + user: string = MAIN_ADDRESS, + ): AccountSignerFixture & { + getAgentSigner: jest.Mock; + onAgentRejected: jest.Mock; + extraAgents: jest.Mock; + } { + const getAgentSigner = jest.fn(); + const onAgentRejected = jest.fn(); + const extraAgents = jest.fn(agents); + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + onAgentRejected, + info: { extraAgents }, + }); + getAgentSigner.mockResolvedValue(built.agentSigner); + built.exchangeClient[write].mockImplementation(async () => { + await built.sdkWallet().signTypedData(L1_PAYLOAD); + throw mustDepositError(user); + }); + return { ...built, getAgentSigner, onAgentRejected, extraAgents }; + } + + describe.each([ + { + venue: 'no longer lists the agent', + agents: async (): Promise => [], + }, + { + venue: 'lists the agent as expired', + agents: async (): Promise => [listedAgent(Date.now() - 1)], + }, + ])('when the venue $venue', ({ agents }) => { + it('fails the order with KEYRING_LOCKED, drops the agent and asks again', async () => { + const { + accountSignerProvider, + extraAgents, + getAgentSigner, + onAgentRejected, + sdkWallet, + } = createMustDepositProvider('order', agents); + await accountSignerProvider.getMarketDataWithPrices(); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(order).toStrictEqual({ + success: false, + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }); + expect(extraAgents.mock.calls).toStrictEqual([[{ user: MAIN_ADDRESS }]]); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT, AGENT_ADDRESS], + ]); + expect(getAgentSigner.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT], + [MAINNET_ACCOUNT], + ]); + expect(loggerError).not.toHaveBeenCalled(); + }); + + it.each(CANCEL_DELIVERIES)( + 'drops the agent named in batch cancel status entries, checking the venue once ($label)', + async ({ delivery }) => { + const { + accountSignerProvider, + exchangeClient, + extraAgents, + getAgentSigner, + onAgentRejected, + sdkWallet, + } = createMustDepositProvider('cancel', agents); + await accountSignerProvider.getMarketDataWithPrices(); + const wallet = sdkWallet(); + exchangeClient.cancel.mockImplementation(async () => { + await wallet.signTypedData(L1_PAYLOAD); + return cancelStatusesResponse( + [ + { error: mustDepositError(MAIN_ADDRESS).message }, + { error: mustDepositError(MAIN_ADDRESS).message }, + ], + delivery, + ); + }); + + const result = await accountSignerProvider.cancelOrders([ + { orderId: '123', symbol: 'BTC' }, + { orderId: '124', symbol: 'BTC' }, + ]); + await wallet.signTypedData(L1_PAYLOAD); + + expect(result).toStrictEqual({ + success: false, + successCount: 0, + failureCount: 2, + results: [ + { + orderId: '123', + symbol: 'BTC', + success: false, + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }, + { + orderId: '124', + symbol: 'BTC', + success: false, + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }, + ], + }); + // One signed write: one check, and the host is told once. + expect(extraAgents.mock.calls).toStrictEqual([ + [{ user: MAIN_ADDRESS }], + ]); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT, AGENT_ADDRESS], + ]); + expect(getAgentSigner.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT], + [MAINNET_ACCOUNT], + ]); + expect(loggerError).not.toHaveBeenCalled(); + }, + ); + + it('fails a cancel with KEYRING_LOCKED when the venue reports it in a status entry', async () => { + const { + accountSignerProvider, + exchangeClient, + onAgentRejected, + sdkWallet, + } = createMustDepositProvider('cancel', agents); + await accountSignerProvider.getMarketDataWithPrices(); + const wallet = sdkWallet(); + exchangeClient.cancel.mockImplementation(async () => { + await wallet.signTypedData(L1_PAYLOAD); + return { + status: 'ok', + response: { + data: { + statuses: [{ error: mustDepositError(MAIN_ADDRESS).message }], + }, + }, + }; + }); + + const result = await accountSignerProvider.cancelOrder({ + orderId: '123', + symbol: 'BTC', + }); + + expect(result).toStrictEqual({ + success: false, + orderId: '123', + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT, AGENT_ADDRESS], + ]); + expect(loggerError).not.toHaveBeenCalled(); + }); + + it('leaves the referral to retry, unrecorded, without logging', async () => { + const { accountSignerProvider, exchangeClient, onAgentRejected } = + createMustDepositProvider('setReferrer', agents); + + const result = await accountSignerProvider.prepareTradingWallet(); + + expect(result).toStrictEqual({ ready: false }); + expect(exchangeClient.setReferrer.mock.calls).toStrictEqual([ + REFERRAL_WRITE, + ]); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT, AGENT_ADDRESS], + ]); + expect(referralAttempted()).toBe(false); + expect(loggerError).not.toHaveBeenCalled(); + }); + }); + + describe.each([ + { + venue: 'still lists the agent', + agents: async (): Promise => [ + listedAgent(Date.now() + HOUR_MS), + ], + }, + { + venue: 'lists the agent without an expiry', + agents: async (): Promise => [listedAgent(null)], + }, + { + venue: 'lists the agent in another case', + agents: async (): Promise => [ + listedAgent(Date.now() + HOUR_MS, getChecksumAddress(AGENT_ADDRESS)), + ], + }, + { + venue: 'cannot list the agents', + agents: async (): Promise => { + throw new Error('network down'); + }, + }, + ])('when the venue $venue', ({ agents }) => { + it('fails the order with the venue error and keeps the agent', async () => { + const { + accountSignerProvider, + extraAgents, + getAgentSigner, + onAgentRejected, + sdkWallet, + } = createMustDepositProvider('order', agents); + await accountSignerProvider.getMarketDataWithPrices(); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(order).toStrictEqual({ + success: false, + error: mustDepositError(MAIN_ADDRESS).message, + }); + expect(extraAgents.mock.calls).toStrictEqual([[{ user: MAIN_ADDRESS }]]); + expect(onAgentRejected).not.toHaveBeenCalled(); + expect(getAgentSigner.mock.calls).toStrictEqual([[MAINNET_ACCOUNT]]); + }); + }); + + it('does not check the agents of a main account the agent did not sign for', async () => { + const { accountSignerProvider, extraAgents, onAgentRejected } = + createMustDepositProvider('order', async () => [], OTHER_MAIN_ADDRESS); + await accountSignerProvider.getMarketDataWithPrices(); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(order).toStrictEqual({ + success: false, + error: mustDepositError(OTHER_MAIN_ADDRESS).message, + }); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + }); + + it('checks the agent list again after the agent signs again', async () => { + const { accountSignerProvider, extraAgents } = createMustDepositProvider( + 'order', + async () => [listedAgent(Date.now() + HOUR_MS)], + ); + await accountSignerProvider.getMarketDataWithPrices(); + + await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(extraAgents.mock.calls).toStrictEqual([ + [{ user: MAIN_ADDRESS }], + [{ user: MAIN_ADDRESS }], + ]); + }); + + it('fails with the venue error once the main account signs after the agent was dropped', async () => { + const { + accountSignerProvider, + extraAgents, + getAgentSigner, + onAgentRejected, + } = createMustDepositProvider('order', async () => []); + await accountSignerProvider.getMarketDataWithPrices(); + + const rejected = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + // The host now signs with the main account, whose account has no funds. + getAgentSigner.mockResolvedValue(null); + const unfunded = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(rejected).toStrictEqual({ + success: false, + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }); + expect(unfunded).toStrictEqual({ + success: false, + error: mustDepositError(MAIN_ADDRESS).message, + }); + expect(extraAgents).toHaveBeenCalledTimes(1); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT, AGENT_ADDRESS], + ]); + }); + + describe('without an agent', () => { + it('fails the order with the venue error without checking the agents', async () => { + const getAgentSigner = jest.fn().mockResolvedValue(null); + const onAgentRejected = jest.fn(); + const extraAgents = jest.fn().mockResolvedValue([]); + const { accountSignerProvider, exchangeClient, sdkWallet } = + createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + onAgentRejected, + info: { extraAgents }, + }); + exchangeClient.order.mockImplementation(async () => { + await sdkWallet().signTypedData(L1_PAYLOAD); + throw mustDepositError(MAIN_ADDRESS); + }); + await accountSignerProvider.getMarketDataWithPrices(); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(order).toStrictEqual({ + success: false, + error: mustDepositError(MAIN_ADDRESS).message, + }); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts new file mode 100644 index 00000000000..ce50a46f828 --- /dev/null +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts @@ -0,0 +1,151 @@ +import { PERPS_ERROR_CODES } from '../../../src/perpsErrorCodes.js'; +import { + APPROVE_BUILDER_FEE_PAYLOAD, + signThroughWallet, +} from '../../helpers/agentFixtures.js'; +import { + BTC_MARKET_ORDER, + BUILDER_FEE_WRITE, + createAccountSignerProvider, + setUpAccountSignerSuite, +} from '../../helpers/hyperLiquidAccountSignerFixture.js'; +import type { AccountSignerFixture } from '../../helpers/hyperLiquidAccountSignerFixture.js'; + +// The SDK ships ES modules only; the provider reaches it through the mocked +// client service, so the module itself is never loaded. The provider checks +// cancel errors against its error class. +jest.mock('@nktkas/hyperliquid', () => ({ + HyperliquidError: class MockHyperliquidError extends Error {}, +})); + +// The client and subscription services are mocked: they own the SDK's +// REST/exchange/info clients and the WebSocket subscriptions. The wallet +// service, the signing caches and the validation run for real. +jest.mock('../../../src/services/HyperLiquidClientService'); +jest.mock('../../../src/services/HyperLiquidSubscriptionService'); + +// The venue refuses every approval of a builder without funds the same way, +// whoever signs it. +const BUILDER_REFUSAL = 'Builder has insufficient balance to be approved.'; + +describe('HyperLiquidProvider with accountSigner: a builder fee approval the venue refuses', () => { + let loggerError: jest.SpyInstance; + + beforeEach(() => { + ({ loggerError } = setUpAccountSignerSuite()); + }); + + /** + * A provider whose builder fee is not approved yet, and whose approval the + * venue refuses once signed. + * + * @param refusal - The venue's answer to the signed approval. + * @returns The provider and its mocks. + */ + function createRefusingProvider( + refusal: string = BUILDER_REFUSAL, + ): AccountSignerFixture { + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + }); + built.infoClient.maxBuilderFee.mockResolvedValue(0); + built.exchangeClient.approveBuilderFee.mockImplementation(async () => { + await signThroughWallet(built.sdkWallet(), APPROVE_BUILDER_FEE_PAYLOAD); + throw new Error(refusal); + }); + return built; + } + + it('reports the refusal from preparation without logging it, asking for the approval once', async () => { + const { accountSignerProvider, exchangeClient } = createRefusingProvider(); + await accountSignerProvider.getMarketDataWithPrices(); + + const prepared = [ + await accountSignerProvider.prepareTradingWallet(), + await accountSignerProvider.prepareTradingWallet(), + ]; + + expect(prepared).toStrictEqual([ + { ready: false, error: BUILDER_REFUSAL }, + { ready: false, error: BUILDER_REFUSAL }, + ]); + expect(exchangeClient.approveBuilderFee.mock.calls).toStrictEqual([ + BUILDER_FEE_WRITE, + ]); + expect(loggerError).not.toHaveBeenCalled(); + }); + + it('sends later orders without asking for the approval again', async () => { + const { accountSignerProvider, accountSigner, exchangeClient } = + createRefusingProvider(); + await accountSignerProvider.getMarketDataWithPrices(); + + await accountSignerProvider.prepareTradingWallet(); + const orders = [ + await accountSignerProvider.placeOrder(BTC_MARKET_ORDER), + await accountSignerProvider.placeOrder(BTC_MARKET_ORDER), + ]; + + // The venue decides on each order, as after any failed approval. + expect(orders.map(({ success }) => success)).toStrictEqual([true, true]); + expect(exchangeClient.order).toHaveBeenCalledTimes(2); + expect(exchangeClient.approveBuilderFee.mock.calls).toStrictEqual([ + BUILDER_FEE_WRITE, + ]); + expect( + accountSigner.signTypedData.mock.calls.filter( + ([, payload]) => payload === APPROVE_BUILDER_FEE_PAYLOAD, + ), + ).toHaveLength(1); + }); + + it('fails a TP/SL update with its own error code without asking for the approval again', async () => { + const { accountSignerProvider, exchangeClient } = createRefusingProvider(); + await accountSignerProvider.getMarketDataWithPrices(); + + await accountSignerProvider.prepareTradingWallet(); + const update = await accountSignerProvider.updatePositionTPSL({ + symbol: 'BTC', + takeProfitPrice: '60000', + stopLossPrice: '40000', + }); + + expect(update).toStrictEqual({ + success: false, + error: PERPS_ERROR_CODES.TPSL_UPDATE_FAILED, + }); + expect(exchangeClient.approveBuilderFee.mock.calls).toStrictEqual([ + BUILDER_FEE_WRITE, + ]); + }); + + it('asks for the approval again after the provider reconnects', async () => { + const { accountSignerProvider, exchangeClient } = createRefusingProvider(); + await accountSignerProvider.getMarketDataWithPrices(); + await accountSignerProvider.prepareTradingWallet(); + + await accountSignerProvider.toggleTestnet(); + await accountSignerProvider.toggleTestnet(); + const prepared = await accountSignerProvider.prepareTradingWallet(); + + expect(prepared).toStrictEqual({ ready: false, error: BUILDER_REFUSAL }); + expect(exchangeClient.approveBuilderFee.mock.calls).toStrictEqual([ + BUILDER_FEE_WRITE, + BUILDER_FEE_WRITE, + ]); + }); + + it('asks for the approval again at the next preparation after a failure the venue may not repeat', async () => { + const { accountSignerProvider, exchangeClient } = + createRefusingProvider('fetch failed'); + await accountSignerProvider.getMarketDataWithPrices(); + + await accountSignerProvider.prepareTradingWallet(); + await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(exchangeClient.approveBuilderFee.mock.calls).toStrictEqual([ + BUILDER_FEE_WRITE, + BUILDER_FEE_WRITE, + ]); + }); +}); diff --git a/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts index c1f0176a1b3..ddf31567e81 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts @@ -228,6 +228,18 @@ describe('HyperLiquidClientService', () => { }); }); + it('signs user-signed actions for the chain the wallet names', async () => { + const signatureChainId = jest.fn().mockResolvedValue('0xaa36a7'); + const wallet = { ...mockWallet, signatureChainId }; + + await service.initialize(wallet); + + const { ExchangeClient } = require('@nktkas/hyperliquid'); + expect(ExchangeClient.mock.calls).toStrictEqual([ + [{ wallet, transport: mockHttpTransport, signatureChainId }], + ]); + }); + it('handles initialization errors', async () => { const { ExchangeClient } = require('@nktkas/hyperliquid'); ExchangeClient.mockImplementationOnce(() => { diff --git a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts index 5f2aa7642ac..16656cf3ea4 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts @@ -39,6 +39,7 @@ type SignerOverrides = { signTypedData?: jest.Mock; isReady?: () => boolean; requiresSignatureConfirmation?: () => boolean; + getChainId?: () => Promise; }; type Built = { @@ -57,6 +58,7 @@ function buildService( signPersonalMessage: jest.fn().mockResolvedValue(MAIN_SIGNATURE), isReady: overrides.isReady, requiresSignatureConfirmation: overrides.requiresSignatureConfirmation, + getChainId: overrides.getChainId, }; const { messenger, call } = createKeyringlessMessenger(keyringType); const service = new HyperLiquidWalletService( @@ -163,6 +165,56 @@ describe('HyperLiquidWalletService with accountSigner', () => { expect(service.requiresSignatureConfirmation()).toBe(expected); }, ); + + it('signs user-signed actions for the chain the signer reports at signing time', async () => { + const getChainId = jest + .fn() + .mockResolvedValueOnce(11155111) + .mockResolvedValueOnce(42161); + const { service } = buildService({ getChainId }); + const adapter = service.createWalletAdapter(); + + expect(await adapter.signatureChainId?.()).toBe('0xaa36a7'); + expect(await adapter.signatureChainId?.()).toBe('0xa4b1'); + }); + + it('fails with KEYRING_LOCKED without asking for the chain when isReady returns false', async () => { + const getChainId = jest.fn().mockResolvedValue(11155111); + const { service } = buildService({ getChainId, isReady: () => false }); + + await expect( + service.createWalletAdapter().signatureChainId?.(), + ).rejects.toThrow(PERPS_ERROR_CODES.KEYRING_LOCKED); + expect(getChainId).not.toHaveBeenCalled(); + }); + + it('fails with KEYRING_LOCKED, keeping the host error as its cause, when the signer locks while reporting its chain', async () => { + let ready = true; + const hostError = new Error('Wallet disconnected'); + const { service } = buildService({ + isReady: () => ready, + getChainId: jest.fn(async () => { + ready = false; + throw hostError; + }), + }); + + const error: unknown = await service + .createWalletAdapter() + .signatureChainId?.() + .catch((caught: unknown) => caught); + + expect(error).toStrictEqual(new Error(PERPS_ERROR_CODES.KEYRING_LOCKED)); + expect((error as Error).cause).toBe(hostError); + }); + + it('leaves the chain of user-signed actions to the SDK when the signer reports none', () => { + const { service } = buildService(); + + expect(service.createWalletAdapter()).not.toHaveProperty( + 'signatureChainId', + ); + }); }); describe('HyperLiquidWalletService wallet adapter with an agent', () => { @@ -360,6 +412,12 @@ describe('HyperLiquidWalletService wallet adapter with an agent and a keyring', return { adapter: service.createWalletAdapter(), agentSign, call }; } + it('leaves the chain of user-signed actions to the SDK', () => { + const { adapter } = buildKeyringAdapter(); + + expect(adapter).not.toHaveProperty('signatureChainId'); + }); + it('signs L1 actions with the agent without calling KeyringController', async () => { const { adapter, agentSign, call } = buildKeyringAdapter(); @@ -426,6 +484,7 @@ describeWithSdk( resolveAgent: () => Promise = async () => // A viem local account is a PerpsAgentSigner as it is. agentAccount, + getChainId?: () => Promise, ): { adapter: ReturnType; signatures: RecordedSignature[]; @@ -450,6 +509,7 @@ describeWithSdk( await recordSignature(mainAccount)(payload), signPersonalMessage: async (_address, message): Promise => await mainAccount.signMessage({ message }), + ...(getChainId && { getChainId }), }, }, messenger, @@ -523,6 +583,40 @@ describeWithSdk( expect(await recoverSigner(signatures[0])).toBe(mainAccount.address); }); + it('signs an SDK builder fee approval for the chain the signer reports', async () => { + const { adapter, signatures } = buildSdkAdapter( + undefined, + async () => 11155111, + ); + // The exchange endpoint, answering the way the venue does. + const request = jest + .fn() + .mockResolvedValue({ status: 'ok', response: { type: 'default' } }); + + // The SDK's exchange client is configured this way by the client + // service; the bare function takes the same config. + await exchange.approveBuilderFee( + { + transport: { isTestnet: true, request }, + wallet: adapter, + signatureChainId: adapter.signatureChainId, + }, + { + builder: agentAccount.address, + maxFeeRate: BUILDER_FEE_CONFIG.MaxFeeRate, + }, + ); + + expect(signatures).toHaveLength(1); + expect(signatures[0].payload.domain.chainId).toBe(11155111); + const [, sent] = request.mock.calls[0] as [ + string, + { action: { signatureChainId: string } }, + ]; + expect(sent.action.signatureChainId).toBe('0xaa36a7'); + expect(await recoverSigner(signatures[0])).toBe(mainAccount.address); + }); + it('keeps an unavailable agent recognizable through the SDK error', async () => { const { adapter } = buildSdkAdapter(async () => { throw new AgentSignerUnavailableError(new Error('agent store down')); From c3f79b266c357ecc8c2f89e2936627ddca7c4300 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Thu, 1 Oct 2026 13:35:58 +0800 Subject: [PATCH 2/6] docs(perps-controller): link the changelog entries to #10643 --- packages/perps-controller/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index ddc0cf00e88..41cde8c4b58 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add optional `PerpsAccountSigner.getChainId`. When set, HyperLiquid user-signed actions (builder fee approval, withdrawals, transfers) are signed for the chain it returns instead of chain 1, so a wallet that only signs for its connected chain does not switch chains ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) +- Add optional `PerpsAccountSigner.getChainId`. When set, HyperLiquid user-signed actions (builder fee approval, withdrawals, transfers) are signed for the chain it returns instead of chain 1, so a wallet that only signs for its connected chain does not switch chains ([#10643](https://github.com/MetaMask/core/pull/10643)) ### Changed @@ -18,10 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Accept Lighter trades that omit position-sign flags, preserve omitted account PnL as unknown, and reject known reductions without realized PnL. Retain side-only fill directions when lifecycle context is ambiguous ([#10605](https://github.com/MetaMask/core/pull/10605)) -- Detect a revoked or expired HyperLiquid agent that the venue answers with "Must deposit before performing actions" ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) +- Detect a revoked or expired HyperLiquid agent that the venue answers with "Must deposit before performing actions" ([#10643](https://github.com/MetaMask/core/pull/10643)) - The provider then checks `extraAgents` for the main account. When the agent is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called - When the agent is still listed (an account with no funds), or the list cannot be read, the venue error is returned as before -- Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) +- Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#10643](https://github.com/MetaMask/core/pull/10643)) - Until the provider disconnects, the approval is not requested again: orders are sent as after any failed approval, TP/SL updates fail with `TPSL_UPDATE_FAILED`, and `prepareTradingWallet` returns the venue error ## [19.0.0] From ffcf092ea0ccca87a098efc44d33805cc61f9811 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Thu, 1 Oct 2026 14:41:01 +0800 Subject: [PATCH 3/6] fix(perps-controller): attribute a "Must deposit" answer to the agent that signed the request The rejection check used the latest agent signature for the main account on the current network, so a main-signed builder fee approval, an answer that arrived after a network switch, or a request signed by an agent the host had just replaced could evict the wrong agent. - The client service reports each exchange request with the venue's answer, from inside the SDK request; the wallet adapter reports each agent signature; the provider matches them by the signature's r value. - Main-signed requests are never attributed to an agent; requests signed on another network or before a disconnect are not checked. - A builder fee refusal settling after a disconnect is not recorded. - Document that the "Must deposit" check relies on named agents and the device clock, and the prepareTradingWallet result for a refused builder fee. --- packages/perps-controller/CHANGELOG.md | 2 +- packages/perps-controller/README.md | 5 +- .../PerpsController-method-action-types.ts | 4 + .../perps-controller/src/PerpsController.ts | 4 + .../src/providers/HyperLiquidProvider.ts | 215 ++++++++++++---- .../src/services/HyperLiquidClientService.ts | 59 ++++- .../src/services/HyperLiquidWalletService.ts | 23 +- packages/perps-controller/src/types/index.ts | 9 +- .../hyperLiquidAccountSignerFixture.ts | 87 ++++++- ...rLiquidProvider.agent-must-deposit.test.ts | 240 +++++++++++++++--- ...LiquidProvider.builder-fee-refusal.test.ts | 32 ++- .../services/HyperLiquidClientService.test.ts | 65 +++++ ...LiquidWalletService.account-signer.test.ts | 74 +++++- 13 files changed, 723 insertions(+), 96 deletions(-) diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 41cde8c4b58..969fdecf698 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Accept Lighter trades that omit position-sign flags, preserve omitted account PnL as unknown, and reject known reductions without realized PnL. Retain side-only fill directions when lifecycle context is ambiguous ([#10605](https://github.com/MetaMask/core/pull/10605)) - Detect a revoked or expired HyperLiquid agent that the venue answers with "Must deposit before performing actions" ([#10643](https://github.com/MetaMask/core/pull/10643)) - - The provider then checks `extraAgents` for the main account. When the agent is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called + - The provider then checks `extraAgents` for the agent that signed the answered request. When it is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called. Requests signed by the main account, on another network or before a disconnect are not checked - When the agent is still listed (an account with no funds), or the list cannot be read, the venue error is returned as before - Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#10643](https://github.com/MetaMask/core/pull/10643)) - Until the provider disconnects, the approval is not requested again: orders are sent as after any failed approval, TP/SL updates fail with `TPSL_UPDATE_FAILED`, and `prepareTradingWallet` returns the venue error diff --git a/packages/perps-controller/README.md b/packages/perps-controller/README.md index b07938e876a..d3ad0b4092c 100644 --- a/packages/perps-controller/README.md +++ b/packages/perps-controller/README.md @@ -83,7 +83,10 @@ account and network with `PerpsController:setAgentSigner`. User-signed actions (builder fee, withdrawals) stay on the main account, and approving the agent is the client's job. When the venue rejects an agent (revoked or expired), the write fails with `KEYRING_LOCKED`, the agent is dropped and -`providerCredentials.hyperliquid.onAgentRejected` is called. Call +`providerCredentials.hyperliquid.onAgentRejected` is called. A "Must deposit +before performing actions" answer to a request the agent signed counts as a +rejection only when `extraAgents` no longer lists the agent, which needs a +named agent. Call `PerpsController:clearAgentSigners` when the agent key locks. `PerpsController:prepareTradingWallet` runs the setup that needs signatures diff --git a/packages/perps-controller/src/PerpsController-method-action-types.ts b/packages/perps-controller/src/PerpsController-method-action-types.ts index c5fe72ee0e2..0205b106afb 100644 --- a/packages/perps-controller/src/PerpsController-method-action-types.ts +++ b/packages/perps-controller/src/PerpsController-method-action-types.ts @@ -975,6 +975,10 @@ export type PerpsControllerClearAgentSignersAction = { * - `NO_ACCOUNT_SELECTED` when no account is selected; * - `PROVIDER_LIFECYCLE_STALE` when the provider disconnected or the account * changed during setup; + * - the venue's message when HyperLiquid refused the builder fee approval + * for a reason signing again cannot fix (for example "Builder has + * insufficient balance to be approved"); it is not logged, and the + * approval is not asked for again until the provider disconnects; * - otherwise the message of the error that stopped setup, which is logged. * @throws Like the other provider-backed actions, `CLIENT_NOT_INITIALIZED` * before `init`, and `CLIENT_REINITIALIZING` or `PROVIDER_NOT_AVAILABLE` diff --git a/packages/perps-controller/src/PerpsController.ts b/packages/perps-controller/src/PerpsController.ts index a54363329fb..6a019fdda16 100644 --- a/packages/perps-controller/src/PerpsController.ts +++ b/packages/perps-controller/src/PerpsController.ts @@ -6091,6 +6091,10 @@ export class PerpsController extends BaseController< * - `NO_ACCOUNT_SELECTED` when no account is selected; * - `PROVIDER_LIFECYCLE_STALE` when the provider disconnected or the account * changed during setup; + * - the venue's message when HyperLiquid refused the builder fee approval + * for a reason signing again cannot fix (for example "Builder has + * insufficient balance to be approved"); it is not logged, and the + * approval is not asked for again until the provider disconnects; * - otherwise the message of the error that stopped setup, which is logged. * @throws Like the other provider-backed actions, `CLIENT_NOT_INITIALIZED` * before `init`, and `CLIENT_REINITIALIZING` or `PROVIDER_NOT_AVAILABLE` diff --git a/packages/perps-controller/src/providers/HyperLiquidProvider.ts b/packages/perps-controller/src/providers/HyperLiquidProvider.ts index 198fe3d4b77..38aa3f7bab4 100644 --- a/packages/perps-controller/src/providers/HyperLiquidProvider.ts +++ b/packages/perps-controller/src/providers/HyperLiquidProvider.ts @@ -502,11 +502,48 @@ type AgentSignature = { key: string; account: PerpsAgentAccount; agentAddress: Hex; + // The provider lifecycle generation it was signed in. + generation?: number; // Whether the venue still lists the agent, asked at most once per // signature (see #findRevokedAgent). approvalCheck?: Promise; }; +/** + * The venue answer an SDK error carries. + * + * @param error - The caught error. + * @returns The answer, or undefined. + */ +function getVenueAnswer(error: unknown): unknown { + return isStatusObject(error) && hasProperty(error, 'response') + ? error.response + : undefined; +} + +/** + * A rejection the venue reported in a status entry, carrying the answer it + * came in, like the error the SDK throws. + * + * @param message - The status entry's error. + * @param answer - The venue answer with the entry. + * @returns The error. + */ +function createStatusError(message: string, answer: unknown): Error { + return Object.assign(new Error(message), { response: answer }); +} + +/** + * The key a signature is recognized by in an exchange request: its r value, + * which the SDK sends as is. + * + * @param signature - The signature, or its r value. + * @returns The key. + */ +function getSignatureKey(signature: string): string { + return signature.slice(0, 66).toLowerCase(); +} + /** * The venue's refusal of a builder fee approval that signing again cannot * fix. @@ -1653,9 +1690,14 @@ export class HyperLiquidProvider implements PerpsProvider { // its action was in flight is still recognized. readonly #agentSignedFor = new Map(); - // The agent that signed the latest L1 action per network and main account - // (see getAgentAccountKey); none when the main account signed it. - readonly #latestAgentSignatures = new Map(); + // Agent signatures whose exchange request has not been answered yet, keyed + // by getSignatureKey. + readonly #unansweredAgentSignatures = new Map(); + + // The agent signature of each answered exchange request, keyed by the + // venue answer. A "Must deposit" answer is attributed from it, so only to + // the agent that signed that request. + readonly #agentSignatureByAnswer = new WeakMap(); readonly #onAgentRejected: HyperLiquidProviderOptions['onAgentRejected']; @@ -1713,6 +1755,8 @@ export class HyperLiquidProvider implements PerpsProvider { // Initialize services with injected platform dependencies this.#clientService = new HyperLiquidClientService(this.#deps, { isTestnet, + onExchangeRequest: (payload: unknown, answer: unknown): void => + this.#matchAgentSignature(payload, answer), }); this.#dexDiscoveryCache = new DexDiscoveryCacheManager({ isTestnetMode: (): boolean => this.#clientService.isTestnetMode(), @@ -1728,6 +1772,11 @@ export class HyperLiquidProvider implements PerpsProvider { mainAddress: Hex, ): Promise => await this.#resolveAgentSigner(mainAddress), + onAgentSignature: ( + signature: Hex, + mainAddress: Hex, + agent: PerpsAgentSigner, + ): void => this.#recordAgentSignature(signature, mainAddress, agent), }, ); this.#subscriptionService = new HyperLiquidSubscriptionService( @@ -2284,12 +2333,13 @@ export class HyperLiquidProvider implements PerpsProvider { pendingEntry.agent = agentSigner; // The wallet adapter resolves at every L1 signature, so this records // the account the agent is about to sign for. - const signature = { key, account, agentAddress: agentSigner.address }; - this.#agentSignedFor.set(agentSigner.address.toLowerCase(), signature); - this.#latestAgentSignatures.set(key, signature); + this.#agentSignedFor.set(agentSigner.address.toLowerCase(), { + key, + account, + agentAddress: agentSigner.address, + }); } else { this.#agentSigners.delete(key); - this.#latestAgentSignatures.delete(key); } return agentSigner; } @@ -2326,8 +2376,9 @@ export class HyperLiquidProvider implements PerpsProvider { * signed for. HyperLiquid answers "User or API Wallet 0x... does not * exist." with the signer's address, so for a revoked or expired agent it * reads like a wallet with no account. It sometimes answers "Must deposit - * before performing actions. User: 0x..." with the main account instead - * (see #findRevokedAgent). + * before performing actions. User: 0x..." with the main account instead; + * that is attributed to the agent that signed the answered request (see + * #findRevokedAgent). * * @param error - The caught error. * @returns The rejected agent, with its address as the host supplied it @@ -2350,39 +2401,94 @@ export class HyperLiquidProvider implements PerpsProvider { : this.#agentSignedFor.get(reportedAddress.toLowerCase()); } const mainAddress = MUST_DEPOSIT_USER_PATTERN.exec(message)?.[1]; - return mainAddress === undefined - ? undefined - : await this.#findRevokedAgent(mainAddress as Hex); + const answer = getVenueAnswer(error); + const signature = isStatusObject(answer) + ? this.#agentSignatureByAnswer.get(answer) + : undefined; + return mainAddress !== undefined && + signature?.account.mainAddress.toLowerCase() === mainAddress.toLowerCase() + ? await this.#findRevokedAgent(signature) + : undefined; } /** - * The agent that signed the latest L1 action for a main account the venue - * answered with "Must deposit before performing actions", if the venue no - * longer lists it (revoked, or past its `validUntil`). The venue answers an - * account with no funds the same way, so the agent list decides; it is - * asked once per signature. + * The agent that signed a request the venue answered with "Must deposit + * before performing actions", if the venue no longer lists it (revoked, or + * past its `validUntil`). The venue answers an account with no funds the + * same way, so the agent list decides; it is asked once per signature. A + * request signed on another network or before a disconnect is not checked. * - * @param mainAddress - The main account the venue names. + * @param signature - The agent signature of the request. * @returns The revoked agent, or undefined. */ async #findRevokedAgent( - mainAddress: Hex, + signature: AgentSignature, ): Promise { - const latest = this.#latestAgentSignatures.get( - getAgentAccountKey({ - mainAddress, - isTestnet: this.#clientService.isTestnetMode(), - }), - ); - if (!latest) { + const isCurrent = (): boolean => + signature.generation === this.#lifecycleGeneration && + signature.account.isTestnet === this.#clientService.isTestnetMode(); + if (!isCurrent()) { return undefined; } - latest.approvalCheck ??= this.#isAgentListed(latest); - return (await latest.approvalCheck) ? undefined : latest; + signature.approvalCheck ??= this.#isAgentListed(signature); + const listed = await signature.approvalCheck; + return listed || !isCurrent() ? undefined : signature; + } + + /** + * Note an agent's L1 signature, so the venue's answer to the request it + * signs can be attributed to it. + * + * @param signature - The signature. + * @param mainAddress - The main account the agent signed for. + * @param agent - The agent. + */ + #recordAgentSignature( + signature: Hex, + mainAddress: Hex, + agent: PerpsAgentSigner, + ): void { + const account: PerpsAgentAccount = { + mainAddress, + isTestnet: this.#clientService.isTestnetMode(), + }; + this.#unansweredAgentSignatures.set(getSignatureKey(signature), { + key: getAgentAccountKey(account), + account, + agentAddress: agent.address, + generation: this.#lifecycleGeneration, + }); + } + + /** + * Link the venue's answer to an exchange request with the agent signature + * the request carries, if an agent signed it. + * + * @param payload - The exchange request. + * @param answer - The venue's answer, or undefined when there is none. + */ + #matchAgentSignature(payload: unknown, answer: unknown): void { + const signature = + isStatusObject(payload) && isStatusObject(payload.signature) + ? payload.signature.r + : undefined; + if (typeof signature !== 'string') { + return; + } + const key = getSignatureKey(signature); + const agentSignature = this.#unansweredAgentSignatures.get(key); + if (!agentSignature) { + return; + } + this.#unansweredAgentSignatures.delete(key); + if (isStatusObject(answer)) { + this.#agentSignatureByAnswer.set(answer, agentSignature); + } } /** * Whether the venue lists an agent as approved for its main account. + * `validUntil` is compared with this device's clock. * * @param agent - The agent and the account it signed for. * @param agent.account - The account the agent signed for. @@ -4570,10 +4676,11 @@ export class HyperLiquidProvider implements PerpsProvider { * handling a rejected agent the way a thrown rejection is. * * @param message - The status entry's error. + * @param answer - The venue answer with the entry. * @returns The mapped error. */ - async #mapStatusError(message: string): Promise { - const error = new Error(message); + async #mapStatusError(message: string, answer: unknown): Promise { + const error = createStatusError(message, answer); return (await this.#classifySignerFailure(error)) ?? this.#mapError(error); } @@ -4583,15 +4690,17 @@ export class HyperLiquidProvider implements PerpsProvider { * reported to the host) once for all its entries. * * @param statuses - The status entries. + * @param answer - The venue answer with the entries. * @returns `KEYRING_LOCKED` for a signer failure, else undefined. */ async #classifyStatusSignerFailure( statuses: unknown[], + answer: unknown, ): Promise { for (const status of statuses) { if (isStatusObject(status) && typeof status.error === 'string') { const signerFailure = await this.#classifySignerFailure( - new Error(status.error), + createStatusError(status.error, answer), ); if (signerFailure) { return signerFailure; @@ -6870,7 +6979,9 @@ export class HyperLiquidProvider implements PerpsProvider { isStatusObject(cancelStatus) && typeof cancelStatus.error === 'string' ) { - await this.#evictRejectedAgent(new Error(cancelStatus.error)); + await this.#evictRejectedAgent( + createStatusError(cancelStatus.error, cancelResult), + ); } remainsLive = classifyCancelStatus(cancelStatus) === CancelChildOutcome.Refused; @@ -8200,7 +8311,10 @@ export class HyperLiquidProvider implements PerpsProvider { } const status: unknown = result.response?.data?.statuses?.[0]; - const signerFailure = await this.#classifyStatusSignerFailure([status]); + const signerFailure = await this.#classifyStatusSignerFailure( + [status], + result, + ); if (signerFailure) { throw signerFailure; } @@ -8796,7 +8910,7 @@ export class HyperLiquidProvider implements PerpsProvider { isStatusObject(status) && typeof status.error === 'string' ? status.error : 'TWAP cancellation failed'; - return createErrorResult(await this.#mapStatusError(rawError), { + return createErrorResult(await this.#mapStatusError(rawError, result), { success: false, orderId: params.orderId, }); @@ -9033,10 +9147,14 @@ export class HyperLiquidProvider implements PerpsProvider { const classifyStatuses = async ( statuses: unknown[], + answer: unknown, ): Promise<{ remainingClientOrderIds: Hex[]; signerFailure?: Error }> => { // A signer failure is classified (and reported) once for the request; // entries the venue cancelled are not resting either way. - const signerFailure = await this.#classifyStatusSignerFailure(statuses); + const signerFailure = await this.#classifyStatusSignerFailure( + statuses, + answer, + ); return { remainingClientOrderIds: requests.flatMap((request, index) => classifyCancelStatus(statuses[index]) === CancelChildOutcome.Refused @@ -9058,12 +9176,12 @@ export class HyperLiquidProvider implements PerpsProvider { }; } - return await classifyStatuses(statuses); + return await classifyStatuses(statuses, result); } catch (error) { // The SDK throws when any entry failed, with every entry's status. const statuses = getCancelStatusesFromError(error, requests.length); if (statuses) { - return await classifyStatuses(statuses); + return await classifyStatuses(statuses, getVenueAnswer(error)); } // The signer could not sign, so nothing was cancelled. const signerFailure = await this.#classifySignerFailure(error); @@ -9108,10 +9226,14 @@ export class HyperLiquidProvider implements PerpsProvider { const classifyStatuses = async ( statuses: unknown[], + answer: unknown, ): Promise => { // A signer failure is classified (and reported) once for the request; // entries the venue cancelled still count as cancelled. - const signerFailure = await this.#classifyStatusSignerFailure(statuses); + const signerFailure = await this.#classifyStatusSignerFailure( + statuses, + answer, + ); const remainingOrderIds: number[] = []; const cancelledOrderIds: number[] = []; requests.forEach((request, index) => { @@ -9141,12 +9263,12 @@ export class HyperLiquidProvider implements PerpsProvider { }; } - return await classifyStatuses(statuses); + return await classifyStatuses(statuses, result); } catch (error) { // The SDK throws when any entry failed, with every entry's status. const statuses = getCancelStatusesFromError(error, requests.length); if (statuses) { - return await classifyStatuses(statuses); + return await classifyStatuses(statuses, getVenueAnswer(error)); } // The signer could not sign, so nothing was cancelled. const signerFailure = await this.#classifySignerFailure(error); @@ -9873,7 +9995,7 @@ export class HyperLiquidProvider implements PerpsProvider { ? status.error : 'Order cancellation failed'; - return createErrorResult(await this.#mapStatusError(rawError), { + return createErrorResult(await this.#mapStatusError(rawError, result), { success: false, orderId: params.orderId, }); @@ -9974,10 +10096,12 @@ export class HyperLiquidProvider implements PerpsProvider { }), ); let statuses: unknown[] | undefined; + let answer: unknown; try { const result = await exchangeClient.cancel({ cancels: cancelRequests, }); + answer = result; const returnedStatuses = result.response?.data?.statuses ?? []; statuses = result.status === 'ok' && @@ -9986,6 +10110,7 @@ export class HyperLiquidProvider implements PerpsProvider { : undefined; } catch (error) { // The SDK throws when any entry failed, with every entry's status. + answer = getVenueAnswer(error); statuses = getCancelStatusesFromError(error, ordinaryOrders.length); if (!statuses) { throw error; @@ -9995,8 +10120,10 @@ export class HyperLiquidProvider implements PerpsProvider { if (statuses) { // One signature covers the batch, so a signer failure is classified // (and reported to the host) once. Each entry keeps its own result. - const signerFailure = - await this.#classifyStatusSignerFailure(statuses); + const signerFailure = await this.#classifyStatusSignerFailure( + statuses, + answer, + ); for (const [ statusIndex, { index, order }, @@ -10005,7 +10132,7 @@ export class HyperLiquidProvider implements PerpsProvider { const success = status === 'success'; const statusError = isStatusObject(status) && typeof status.error === 'string' - ? new Error(status.error) + ? createStatusError(status.error, answer) : undefined; let error: string = PERPS_ERROR_CODES.BATCH_CANCEL_FAILED; if (statusError) { diff --git a/packages/perps-controller/src/services/HyperLiquidClientService.ts b/packages/perps-controller/src/services/HyperLiquidClientService.ts index 0953daeed5f..1766e30c3a4 100644 --- a/packages/perps-controller/src/services/HyperLiquidClientService.ts +++ b/packages/perps-controller/src/services/HyperLiquidClientService.ts @@ -6,7 +6,10 @@ import { SubscriptionClient, WebSocketTransport, } from '@nktkas/hyperliquid'; -import type { HistoricalOrdersResponse } from '@nktkas/hyperliquid'; +import type { + HistoricalOrdersResponse, + IRequestTransport, +} from '@nktkas/hyperliquid'; import { CandlePeriod, @@ -77,6 +80,8 @@ export class HyperLiquidClientService { #httpTransport?: HttpTransport; + readonly #onExchangeRequest?: (payload: unknown, response: unknown) => void; + #walletParams?: HyperLiquidWalletParams; #isTestnet: boolean; @@ -117,12 +122,24 @@ export class HyperLiquidClientService { // Platform dependencies for logging readonly #deps: PerpsPlatformDependencies; + /** + * @param deps - Platform dependencies. + * @param options - Options. + * @param options.isTestnet - Start on testnet. + * @param options.onExchangeRequest - Called with every exchange request's + * payload and the venue's answer (undefined when the request failed before + * one), before the SDK checks the answer. + */ constructor( deps: PerpsPlatformDependencies, - options: { isTestnet?: boolean } = {}, + options: { + isTestnet?: boolean; + onExchangeRequest?: (payload: unknown, response: unknown) => void; + } = {}, ) { this.#deps = deps; this.#isTestnet = options.isTestnet ?? false; + this.#onExchangeRequest = options.onExchangeRequest; } /** @@ -356,7 +373,7 @@ export class HyperLiquidClientService { if (effectiveWallet) { this.#exchangeClient = new ExchangeClient({ wallet: effectiveWallet as any, // eslint-disable-line @typescript-eslint/no-explicit-any -- Type widening for SDK compatibility - transport: this.#httpTransport, + transport: this.#reportingExchangeRequests(this.#httpTransport), ...(effectiveWallet.signatureChainId && { signatureChainId: effectiveWallet.signatureChainId, }), @@ -366,6 +383,42 @@ export class HyperLiquidClientService { } } + /** + * The transport the exchange client sends through, reporting each exchange + * request and its answer to `onExchangeRequest` when it is set. + * + * @param transport - The HTTP transport. + * @returns The transport to send exchange requests through. + */ + #reportingExchangeRequests(transport: HttpTransport): IRequestTransport { + const onExchangeRequest = this.#onExchangeRequest; + if (!onExchangeRequest) { + return transport; + } + return { + isTestnet: transport.isTestnet, + request: async ( + endpoint: 'info' | 'exchange', + payload: unknown, + signal?: AbortSignal, + ): Promise => { + let response: Response | undefined; + try { + response = await transport.request( + endpoint, + payload, + signal, + ); + return response; + } finally { + if (endpoint === 'exchange') { + onExchangeRequest(payload, response); + } + } + }, + }; + } + /** * Toggle testnet mode and reinitialize clients * diff --git a/packages/perps-controller/src/services/HyperLiquidWalletService.ts b/packages/perps-controller/src/services/HyperLiquidWalletService.ts index a18ca0e503e..679ab1959c4 100644 --- a/packages/perps-controller/src/services/HyperLiquidWalletService.ts +++ b/packages/perps-controller/src/services/HyperLiquidWalletService.ts @@ -47,6 +47,15 @@ const HARDWARE_KEYRING_TYPES = new Set([ */ type AgentResolver = (mainAddress: Hex) => Promise; +/** + * Told of every L1 signature an agent made for a main account. + */ +type AgentSignatureListener = ( + signature: Hex, + mainAddress: Hex, + agent: PerpsAgentSigner, +) => void; + /** * Whether a signing request is an L1 action. * @@ -74,15 +83,22 @@ export class HyperLiquidWalletService { readonly #resolveAgent: AgentResolver | undefined; + readonly #onAgentSignature: AgentSignatureListener | undefined; + constructor( deps: PerpsPlatformDependencies, messenger: PerpsControllerMessengerBase, - options: { isTestnet?: boolean; resolveAgent?: AgentResolver } = {}, + options: { + isTestnet?: boolean; + resolveAgent?: AgentResolver; + onAgentSignature?: AgentSignatureListener; + } = {}, ) { this.#deps = deps; this.#messenger = messenger; this.#isTestnet = options.isTestnet ?? false; this.#resolveAgent = options.resolveAgent; + this.#onAgentSignature = options.onAgentSignature; } /** @@ -256,13 +272,16 @@ export class HyperLiquidWalletService { 'HyperLiquidWalletService: Signing L1 action with agent', { address: mainAddress, agent: agentSigner.address }, ); + let signature: Hex; try { - return await agentSigner.signTypedData(params); + signature = await agentSigner.signTypedData(params); } catch (error) { // The host could not sign with its agent key (for example it locked // after resolving it). Retryable, like a locked keyring. throw new AgentSignerUnavailableError(error); } + this.#onAgentSignature?.(signature, mainAddress, agentSigner); + return signature; }, getChainId: async (): Promise => parseInt(getChainId(this.#isTestnet), 10), diff --git a/packages/perps-controller/src/types/index.ts b/packages/perps-controller/src/types/index.ts index f6fb48d796d..1ea285dfc2a 100644 --- a/packages/perps-controller/src/types/index.ts +++ b/packages/perps-controller/src/types/index.ts @@ -1131,8 +1131,13 @@ export type HyperLiquidCredentials = { account: PerpsAgentAccount, ) => Promise; /** - * Called when the venue rejects an agent as unknown (revoked or expired, - * for example after the user approved another unnamed agent). The provider + * Called when the venue rejects an agent (revoked or expired, for example + * after the user approved another agent under its name). The venue either + * names the agent as an unknown wallet or answers the request it signed + * with "Must deposit before performing actions"; the latter counts only + * when `extraAgents` no longer lists the agent, or lists it past its + * `validUntil` by this device's clock. That list holds named agents, so + * approve the agent with a name. The provider * has dropped it, with a `setAgentSigner` binding to it, and the next L1 * action asks `getAgentSigner` again, so re-check the approval before * answering. The rejected action failed with `KEYRING_LOCKED`. It gets the diff --git a/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts b/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts index 401c2bb6902..aa81d8e42f2 100644 --- a/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts +++ b/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts @@ -133,6 +133,45 @@ export function cancelStatusesResponse( throw Object.assign(error, { response }); } +/** + * The message of the ApiRequestError the SDK (0.33.1) throws for a venue + * answer, or undefined when the answer is not an error. + * + * @param answer - The venue's answer. + * @returns The message, or undefined. + */ +function getVenueErrorMessage( + answer: Record, +): string | undefined { + if (answer.status === 'err') { + return String(answer.response); + } + const response = answer.response as + | { type?: string; data?: { statuses?: unknown[] } } + | undefined; + const failures = (response?.data?.statuses ?? []).flatMap((status, index) => + typeof status === 'object' && status !== null && 'error' in status + ? [`${response?.type} ${index}: ${String(status.error)}`] + : [], + ); + return failures.length > 0 ? failures.join(', ') : undefined; +} + +/** + * The ApiRequestError the SDK (0.33.1) throws for a venue answer that is an + * error, carrying the answer. + * + * @param answer - The venue's answer. + * @returns The SDK error. + */ +export function apiRequestError(answer: Record): Error { + const error = new HyperliquidError( + getVenueErrorMessage(answer) ?? 'Unknown error', + ); + error.name = 'ApiRequestError'; + return Object.assign(error, { response: answer }); +} + /** * The order ID a placement returned. * @@ -311,6 +350,18 @@ export type AccountSignerFixture = { initialize: jest.Mock, [HyperLiquidWalletParams]>; // The wallet the provider last initialized the SDK clients with. sdkWallet: () => HyperLiquidWalletParams; + // Report the venue's answer to a request carrying `signature` to the + // provider, as the client service does before the SDK reads the answer. + reportAnswer: (signature: Hex, answer: Record) => void; + // Sign an exchange request through the SDK wallet and send it the way the + // SDK does: the venue's answer is reported to the provider with the + // request's signature, then thrown as the SDK's ApiRequestError when it is + // an error, unless it is `returned`. + signAndSend: ( + payload: PerpsTypedDataPayload, + answer: Record, + delivery?: 'thrown' | 'returned', + ) => Promise>; selectAccount: (address: Hex) => void; deselectAccount: () => void; }; @@ -351,15 +402,34 @@ export function createAccountSignerProvider( } return wallet; }; + // The provider's listener for exchange requests, which the client service + // calls with each request and the venue's answer. + let onExchangeRequest: + | ((payload: unknown, answer: unknown) => void) + | undefined; + const reportAnswer = ( + signature: Hex, + answer: Record, + ): void => + onExchangeRequest?.({ signature: { r: signature.slice(0, 66) } }, answer); + const signAndSend = async ( + payload: PerpsTypedDataPayload, + answer: Record, + delivery: 'thrown' | 'returned' = 'thrown', + ): Promise> => { + reportAnswer(await signThroughWallet(sdkWallet(), payload), answer); + if (delivery === 'returned' || getVenueErrorMessage(answer) === undefined) { + return answer; + } + throw apiRequestError(answer); + }; const signThroughSdkWallet = ( payload: PerpsTypedDataPayload, response: Record = { status: 'ok' }, ): (() => Promise>) => - async () => { - await signThroughWallet(sdkWallet(), payload); - return response; - }; + async () => + await signAndSend(payload, response); const exchangeClient = createMockExchangeClient({ agentSetAbstraction: jest.fn(signThroughSdkWallet(L1_PAYLOAD)), setReferrer: jest.fn(signThroughSdkWallet(L1_PAYLOAD)), @@ -390,7 +460,12 @@ export function createAccountSignerProvider( getExchangeClient: jest.fn().mockReturnValue(exchangeClient), getInfoClient: jest.fn().mockReturnValue(infoClient), } as Partial as jest.Mocked; - MockedHyperLiquidClientService.mockImplementationOnce(() => clientService); + MockedHyperLiquidClientService.mockImplementationOnce( + (_deps, serviceOptions) => { + onExchangeRequest = serviceOptions?.onExchangeRequest; + return clientService; + }, + ); const accountSignerProvider = new HyperLiquidProvider({ platformDependencies: options.keyring ? mockPlatformDependencies @@ -413,6 +488,8 @@ export function createAccountSignerProvider( infoClient, initialize, sdkWallet, + reportAnswer, + signAndSend, selectAccount, deselectAccount, }; diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts index bb6690def3d..d39a768aead 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts @@ -1,24 +1,30 @@ import { getChecksumAddress } from '@metamask/utils'; import { PERPS_ERROR_CODES } from '../../../src/perpsErrorCodes.js'; +import type { PerpsAgentAccount } from '../../../src/types/index.js'; import { AGENT_ADDRESS, + APPROVE_BUILDER_FEE_PAYLOAD, L1_PAYLOAD, MAINNET_ACCOUNT, MAIN_ADDRESS, + OTHER_AGENT_ADDRESS, + OTHER_AGENT_SIGNATURE, OTHER_MAIN_ADDRESS, + TESTNET_ACCOUNT, mustDepositError, } from '../../helpers/agentFixtures.js'; import { BTC_MARKET_ORDER, CANCEL_DELIVERIES, REFERRAL_WRITE, - cancelStatusesResponse, + apiRequestError, createAccountSignerProvider, referralAttempted, setUpAccountSignerSuite, } from '../../helpers/hyperLiquidAccountSignerFixture.js'; import type { AccountSignerFixture } from '../../helpers/hyperLiquidAccountSignerFixture.js'; +import { createDeferred } from '../../helpers/serviceMocks.js'; // The SDK ships ES modules only; the provider reaches it through the mocked // client service, so the module itself is never loaded. The provider checks @@ -35,6 +41,26 @@ jest.mock('../../../src/services/HyperLiquidSubscriptionService'); const HOUR_MS = 60 * 60 * 1000; +/** + * The venue's answer to a write for an account that must deposit. + * + * @param user - The main account the venue names. + * @returns The answer. + */ +function mustDepositAnswer(user: string): Record { + return { status: 'err', response: mustDepositError(user).message }; +} + +/** + * The venue's answer to a cancel, one status per entry. + * + * @param statuses - The entries' statuses. + * @returns The answer. + */ +function cancelAnswer(statuses: unknown[]): Record { + return { status: 'ok', response: { type: 'cancel', data: { statuses } } }; +} + type ListedAgent = { address: string; name: string; @@ -58,9 +84,12 @@ function listedAgent( describe('HyperLiquidProvider with accountSigner: an agent the venue answers with "Must deposit"', () => { let loggerError: jest.SpyInstance; + let mockClientService: ReturnType< + typeof setUpAccountSignerSuite + >['mockClientService']; beforeEach(() => { - ({ loggerError } = setUpAccountSignerSuite()); + ({ loggerError, mockClientService } = setUpAccountSignerSuite()); }); /** @@ -91,10 +120,9 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit info: { extraAgents }, }); getAgentSigner.mockResolvedValue(built.agentSigner); - built.exchangeClient[write].mockImplementation(async () => { - await built.sdkWallet().signTypedData(L1_PAYLOAD); - throw mustDepositError(user); - }); + built.exchangeClient[write].mockImplementation( + async () => await built.signAndSend(L1_PAYLOAD, mustDepositAnswer(user)), + ); return { ...built, getAgentSigner, onAgentRejected, extraAgents }; } @@ -146,25 +174,26 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit getAgentSigner, onAgentRejected, sdkWallet, + signAndSend, } = createMustDepositProvider('cancel', agents); await accountSignerProvider.getMarketDataWithPrices(); - const wallet = sdkWallet(); - exchangeClient.cancel.mockImplementation(async () => { - await wallet.signTypedData(L1_PAYLOAD); - return cancelStatusesResponse( - [ - { error: mustDepositError(MAIN_ADDRESS).message }, - { error: mustDepositError(MAIN_ADDRESS).message }, - ], - delivery, - ); - }); + exchangeClient.cancel.mockImplementation( + async () => + await signAndSend( + L1_PAYLOAD, + cancelAnswer([ + { error: mustDepositError(MAIN_ADDRESS).message }, + { error: mustDepositError(MAIN_ADDRESS).message }, + ]), + delivery, + ), + ); const result = await accountSignerProvider.cancelOrders([ { orderId: '123', symbol: 'BTC' }, { orderId: '124', symbol: 'BTC' }, ]); - await wallet.signTypedData(L1_PAYLOAD); + await sdkWallet().signTypedData(L1_PAYLOAD); expect(result).toStrictEqual({ success: false, @@ -205,21 +234,17 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit accountSignerProvider, exchangeClient, onAgentRejected, - sdkWallet, + signAndSend, } = createMustDepositProvider('cancel', agents); await accountSignerProvider.getMarketDataWithPrices(); - const wallet = sdkWallet(); - exchangeClient.cancel.mockImplementation(async () => { - await wallet.signTypedData(L1_PAYLOAD); - return { - status: 'ok', - response: { - data: { - statuses: [{ error: mustDepositError(MAIN_ADDRESS).message }], - }, - }, - }; - }); + exchangeClient.cancel.mockImplementation( + async () => + await signAndSend( + L1_PAYLOAD, + cancelAnswer([{ error: mustDepositError(MAIN_ADDRESS).message }]), + 'returned', + ), + ); const result = await accountSignerProvider.cancelOrder({ orderId: '123', @@ -361,22 +386,165 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit ]); }); + it('does not check the agent when the main account signed the answered request', async () => { + const getAgentSigner = jest.fn(); + const onAgentRejected = jest.fn(); + const extraAgents = jest.fn().mockResolvedValue([]); + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + onAgentRejected, + info: { extraAgents }, + }); + getAgentSigner.mockResolvedValue(built.agentSigner); + built.infoClient.maxBuilderFee.mockResolvedValue(0); + // The builder fee approval is user-signed: the main account signs it. + built.exchangeClient.approveBuilderFee.mockImplementation( + async () => + await built.signAndSend( + APPROVE_BUILDER_FEE_PAYLOAD, + mustDepositAnswer(MAIN_ADDRESS), + ), + ); + + // The referral, an L1 action the agent signs, comes first. + const prepared = await built.accountSignerProvider.prepareTradingWallet(); + + expect(built.agentSigner.signTypedData).toHaveBeenCalledWith(L1_PAYLOAD); + expect(prepared).toStrictEqual({ ready: false }); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + expect(getAgentSigner.mock.calls).toStrictEqual([[MAINNET_ACCOUNT]]); + }); + + it('checks the agent that signed the answered request after the host switched agents', async () => { + const getAgentSigner = jest.fn(); + const onAgentRejected = jest.fn(); + const replacement = { + address: OTHER_AGENT_ADDRESS, + signTypedData: jest.fn().mockResolvedValue(OTHER_AGENT_SIGNATURE), + }; + // The venue lists the replacement only. + const extraAgents = jest + .fn() + .mockResolvedValue([listedAgent(null, OTHER_AGENT_ADDRESS)]); + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + onAgentRejected, + info: { extraAgents }, + }); + getAgentSigner.mockResolvedValue(built.agentSigner); + await built.accountSignerProvider.getMarketDataWithPrices(); + const signed = createDeferred(); + const venue = createDeferred(); + const answer = mustDepositAnswer(MAIN_ADDRESS); + built.exchangeClient.order.mockImplementationOnce(async () => { + const signature = await built.sdkWallet().signTypedData(L1_PAYLOAD); + signed.resolve(); + await venue.promise; + built.reportAnswer(signature, answer); + throw apiRequestError(answer); + }); + + const ordering = built.accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await signed.promise; + built.accountSignerProvider.clearAgentSigners(); + getAgentSigner.mockResolvedValue(replacement); + // The replacement signs a request the venue accepts, then the first + // request's answer arrives. + await built.signAndSend(L1_PAYLOAD, { status: 'ok' }); + venue.resolve(); + const order = await ordering; + await built.sdkWallet().signTypedData(L1_PAYLOAD); + + expect(order).toStrictEqual({ + success: false, + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }); + expect(extraAgents.mock.calls).toStrictEqual([[{ user: MAIN_ADDRESS }]]); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT, AGENT_ADDRESS], + ]); + // The replacement stays in use. + expect(replacement.signTypedData).toHaveBeenCalledTimes(2); + expect(getAgentSigner.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT], + [MAINNET_ACCOUNT], + ]); + }); + + it('does not check a request signed before the provider switched networks', async () => { + let isTestnet = false; + mockClientService.isTestnetMode.mockImplementation(() => isTestnet); + mockClientService.setTestnetMode.mockImplementation((value: boolean) => { + isTestnet = value; + }); + const getAgentSigner = jest.fn(); + const onAgentRejected = jest.fn(); + const extraAgents = jest.fn().mockResolvedValue([]); + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + onAgentRejected, + info: { extraAgents }, + }); + const testnetAgent = { + address: OTHER_AGENT_ADDRESS, + signTypedData: jest.fn().mockResolvedValue(OTHER_AGENT_SIGNATURE), + }; + getAgentSigner.mockImplementation(async (account: PerpsAgentAccount) => + account.isTestnet ? testnetAgent : built.agentSigner, + ); + await built.accountSignerProvider.getMarketDataWithPrices(); + const signed = createDeferred(); + const venue = createDeferred(); + const answer = mustDepositAnswer(MAIN_ADDRESS); + built.exchangeClient.order.mockImplementationOnce(async () => { + const signature = await built.sdkWallet().signTypedData(L1_PAYLOAD); + signed.resolve(); + await venue.promise; + built.reportAnswer(signature, answer); + throw apiRequestError(answer); + }); + + const ordering = built.accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await signed.promise; + const toggled = await built.accountSignerProvider.toggleTestnet(); + const testnetOrder = + await built.accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + venue.resolve(); + await ordering; + await built.sdkWallet().signTypedData(L1_PAYLOAD); + + expect(toggled).toStrictEqual({ success: true, isTestnet: true }); + expect(testnetOrder.success).toBe(true); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + // The testnet agent stays in use. + expect( + (getAgentSigner.mock.calls as [PerpsAgentAccount][]).filter( + ([account]) => account.isTestnet, + ), + ).toStrictEqual([[TESTNET_ACCOUNT]]); + }); + describe('without an agent', () => { it('fails the order with the venue error without checking the agents', async () => { const getAgentSigner = jest.fn().mockResolvedValue(null); const onAgentRejected = jest.fn(); const extraAgents = jest.fn().mockResolvedValue([]); - const { accountSignerProvider, exchangeClient, sdkWallet } = + const { accountSignerProvider, exchangeClient, signAndSend } = createAccountSignerProvider({ abstraction: 'unifiedAccount', getAgentSigner, onAgentRejected, info: { extraAgents }, }); - exchangeClient.order.mockImplementation(async () => { - await sdkWallet().signTypedData(L1_PAYLOAD); - throw mustDepositError(MAIN_ADDRESS); - }); + exchangeClient.order.mockImplementation( + async () => + await signAndSend(L1_PAYLOAD, mustDepositAnswer(MAIN_ADDRESS)), + ); await accountSignerProvider.getMarketDataWithPrices(); const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts index ce50a46f828..dde7d74e682 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.builder-fee-refusal.test.ts @@ -10,6 +10,7 @@ import { setUpAccountSignerSuite, } from '../../helpers/hyperLiquidAccountSignerFixture.js'; import type { AccountSignerFixture } from '../../helpers/hyperLiquidAccountSignerFixture.js'; +import { createDeferred } from '../../helpers/serviceMocks.js'; // The SDK ships ES modules only; the provider reaches it through the mocked // client service, so the module itself is never loaded. The provider checks @@ -135,7 +136,36 @@ describe('HyperLiquidProvider with accountSigner: a builder fee approval the ven ]); }); - it('asks for the approval again at the next preparation after a failure the venue may not repeat', async () => { + it('asks for the approval again after a reconnect during which the venue refused it', async () => { + const { accountSignerProvider, exchangeClient, sdkWallet } = + createRefusingProvider(); + const requested = createDeferred(); + const refused = createDeferred(); + exchangeClient.approveBuilderFee.mockImplementationOnce(async () => { + await signThroughWallet(sdkWallet(), APPROVE_BUILDER_FEE_PAYLOAD); + requested.resolve(); + await refused.promise; + throw new Error(BUILDER_REFUSAL); + }); + await accountSignerProvider.getMarketDataWithPrices(); + + const preparing = accountSignerProvider.prepareTradingWallet(); + await requested.promise; + // The refusal settles once the reconnect has started. + const reconnecting = accountSignerProvider.toggleTestnet(); + refused.resolve(); + await preparing; + await reconnecting; + const prepared = await accountSignerProvider.prepareTradingWallet(); + + expect(prepared).toStrictEqual({ ready: false, error: BUILDER_REFUSAL }); + expect(exchangeClient.approveBuilderFee.mock.calls).toStrictEqual([ + BUILDER_FEE_WRITE, + BUILDER_FEE_WRITE, + ]); + }); + + it('asks for the approval again at the next order after a failure the venue may not repeat', async () => { const { accountSignerProvider, exchangeClient } = createRefusingProvider('fetch failed'); await accountSignerProvider.getMarketDataWithPrices(); diff --git a/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts index ddf31567e81..9408717f789 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts @@ -228,6 +228,71 @@ describe('HyperLiquidClientService', () => { }); }); + describe('with onExchangeRequest', () => { + const answer = { status: 'ok', response: { type: 'default' } }; + let transport: { isTestnet: boolean; request: jest.Mock }; + let onExchangeRequest: jest.Mock; + + beforeEach(async () => { + const { HttpTransport } = require('@nktkas/hyperliquid'); + transport = { isTestnet: true, request: jest.fn() }; + HttpTransport.mockImplementation(() => transport); + onExchangeRequest = jest.fn(); + await new HyperLiquidClientService(mockDeps, { + isTestnet: true, + onExchangeRequest, + }).initialize(mockWallet); + }); + + /** + * The transport the exchange client was built with. + * + * @returns The transport. + */ + function exchangeTransport(): { + isTestnet: boolean; + request: (endpoint: string, payload: unknown) => Promise; + } { + const { ExchangeClient } = require('@nktkas/hyperliquid'); + return ExchangeClient.mock.calls.at(-1)[0].transport; + } + + it('reports each exchange request with the answer before returning it', async () => { + transport.request.mockResolvedValue(answer); + const payload = { action: { type: 'order' } }; + + const returned = await exchangeTransport().request('exchange', payload); + + expect(returned).toBe(answer); + expect(exchangeTransport().isTestnet).toBe(true); + expect(transport.request.mock.calls).toStrictEqual([ + ['exchange', payload, undefined], + ]); + expect(onExchangeRequest.mock.calls).toStrictEqual([[payload, answer]]); + }); + + it('reports an exchange request that failed without an answer', async () => { + const failure = new Error('fetch failed'); + transport.request.mockRejectedValue(failure); + const payload = { action: { type: 'order' } }; + + await expect( + exchangeTransport().request('exchange', payload), + ).rejects.toBe(failure); + expect(onExchangeRequest.mock.calls).toStrictEqual([ + [payload, undefined], + ]); + }); + + it('does not report info requests', async () => { + transport.request.mockResolvedValue([]); + + await exchangeTransport().request('info', { type: 'meta' }); + + expect(onExchangeRequest).not.toHaveBeenCalled(); + }); + }); + it('signs user-signed actions for the chain the wallet names', async () => { const signatureChainId = jest.fn().mockResolvedValue('0xaa36a7'); const wallet = { ...mockWallet, signatureChainId }; diff --git a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts index 16656cf3ea4..bd36c1c2074 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts @@ -224,9 +224,11 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { resolveAgent: jest.Mock; agentSign: jest.Mock; mainSign: jest.Mock; + onAgentSignature: jest.Mock; call: jest.SpyInstance; selectAccount: (address: `0x${string}`) => void; } { + const onAgentSignature = jest.fn(); const signer = { signTypedData: jest.fn().mockResolvedValue(MAIN_SIGNATURE), signPersonalMessage: jest.fn(), @@ -243,13 +245,14 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { const service = new HyperLiquidWalletService( { ...createMockInfrastructure(), accountSigner: signer }, messenger, - { isTestnet: true, resolveAgent }, + { isTestnet: true, resolveAgent, onAgentSignature }, ); return { adapter: service.createWalletAdapter(), resolveAgent, agentSign, mainSign: signer.signTypedData, + onAgentSignature, call, selectAccount, }; @@ -261,6 +264,33 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { expect(adapter.address).toBe(mainAddress); }); + it('reports each agent signature with the main account it signed for', async () => { + const { adapter, onAgentSignature } = buildAdapter(); + + await adapter.signTypedData(L1_PAYLOAD); + + expect(onAgentSignature).toHaveBeenCalledTimes(1); + const [signature, account, agent] = onAgentSignature.mock.calls[0] as [ + Hex, + Hex, + PerpsAgentSigner, + ]; + expect([signature, account, agent.address]).toStrictEqual([ + AGENT_SIGNATURE, + mainAddress, + AGENT_ADDRESS, + ]); + }); + + it('does not report signatures of the main account', async () => { + const { adapter, onAgentSignature } = buildAdapter(false); + + await adapter.signTypedData(L1_PAYLOAD); + await adapter.signTypedData(USER_SIGNED_PAYLOAD); + + expect(onAgentSignature).not.toHaveBeenCalled(); + }); + it('signs L1 actions with the agent resolved for the selected account', async () => { const { adapter, resolveAgent, agentSign, mainSign, call } = buildAdapter(); @@ -583,6 +613,48 @@ describeWithSdk( expect(await recoverSigner(signatures[0])).toBe(mainAccount.address); }); + it('reports the agent signature an SDK L1 request carries', async () => { + const { messenger, selectAccount } = createKeyringlessMessenger(); + selectAccount(mainAccount.address); + const onAgentSignature = jest.fn(); + const adapter = new HyperLiquidWalletService( + { + ...createMockInfrastructure(), + accountSigner: { + signTypedData: async (): Promise => MAIN_SIGNATURE, + signPersonalMessage: async (): Promise => MAIN_SIGNATURE, + }, + }, + messenger, + { + isTestnet: true, + resolveAgent: async (): Promise => agentAccount, + onAgentSignature, + }, + ).createWalletAdapter(); + // The exchange endpoint, answering the way the venue does. + const request = jest.fn().mockResolvedValue({ + status: 'ok', + response: { type: 'cancel', data: { statuses: ['success'] } }, + }); + + await exchange.cancel( + { wallet: adapter, transport: { isTestnet: true, request } }, + { cancels: [{ a: 0, o: 1 }] }, + ); + + expect(onAgentSignature).toHaveBeenCalledTimes(1); + const [[signature]] = onAgentSignature.mock.calls as [[Hex]]; + const [, sent] = request.mock.calls[0] as [ + string, + { signature: { r: string } }, + ]; + // The provider recognizes the request by this r value. + expect(sent.signature.r.toLowerCase()).toBe( + signature.slice(0, 66).toLowerCase(), + ); + }); + it('signs an SDK builder fee approval for the chain the signer reports', async () => { const { adapter, signatures } = buildSdkAdapter( undefined, From bc350bfb6dd92e8d16d6f5865508ad647011a1c2 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Thu, 1 Oct 2026 15:12:10 +0800 Subject: [PATCH 4/6] fix(perps-controller): read an agent signature's network and session before signing The network and lifecycle generation of an agent signature were read once the signature completed, so a mainnet signature that finished after a network switch or a reconnect was checked as a current request. - Read the network, lifecycle generation and main account when the agent is asked for, and record each signature the resolved agent makes with them; the wallet adapter no longer reports signatures. - Do not track a signature the SDK would refuse before sending it, and drop unanswered signatures on disconnect. --- packages/perps-controller/CHANGELOG.md | 2 +- .../src/providers/HyperLiquidProvider.ts | 61 +++-- .../src/services/HyperLiquidWalletService.ts | 23 +- .../hyperLiquidAccountSignerFixture.ts | 19 +- ...rLiquidProvider.agent-must-deposit.test.ts | 240 +++++++++++++++++- ...LiquidWalletService.account-signer.test.ts | 56 +--- 6 files changed, 293 insertions(+), 108 deletions(-) diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 969fdecf698..b2ee5d74790 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Accept Lighter trades that omit position-sign flags, preserve omitted account PnL as unknown, and reject known reductions without realized PnL. Retain side-only fill directions when lifecycle context is ambiguous ([#10605](https://github.com/MetaMask/core/pull/10605)) - Detect a revoked or expired HyperLiquid agent that the venue answers with "Must deposit before performing actions" ([#10643](https://github.com/MetaMask/core/pull/10643)) - - The provider then checks `extraAgents` for the agent that signed the answered request. When it is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called. Requests signed by the main account, on another network or before a disconnect are not checked + - The provider then checks `extraAgents` for the agent that signed the answered request. When it is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called. Requests signed by the main account, or whose signature was asked for on another network or before a disconnect, are not checked - When the agent is still listed (an account with no funds), or the list cannot be read, the venue error is returned as before - Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#10643](https://github.com/MetaMask/core/pull/10643)) - Until the provider disconnects, the approval is not requested again: orders are sent as after any failed approval, TP/SL updates fail with `TPSL_UPDATE_FAILED`, and `prepareTradingWallet` returns the venue error diff --git a/packages/perps-controller/src/providers/HyperLiquidProvider.ts b/packages/perps-controller/src/providers/HyperLiquidProvider.ts index 38aa3f7bab4..b9319ae69fe 100644 --- a/packages/perps-controller/src/providers/HyperLiquidProvider.ts +++ b/packages/perps-controller/src/providers/HyperLiquidProvider.ts @@ -502,7 +502,7 @@ type AgentSignature = { key: string; account: PerpsAgentAccount; agentAddress: Hex; - // The provider lifecycle generation it was signed in. + // The provider lifecycle generation the signature was asked for in. generation?: number; // Whether the venue still lists the agent, asked at most once per // signature (see #findRevokedAgent). @@ -1691,7 +1691,7 @@ export class HyperLiquidProvider implements PerpsProvider { readonly #agentSignedFor = new Map(); // Agent signatures whose exchange request has not been answered yet, keyed - // by getSignatureKey. + // by getSignatureKey. Cleared on disconnect. readonly #unansweredAgentSignatures = new Map(); // The agent signature of each answered exchange request, keyed by the @@ -1771,12 +1771,7 @@ export class HyperLiquidProvider implements PerpsProvider { resolveAgent: async ( mainAddress: Hex, ): Promise => - await this.#resolveAgentSigner(mainAddress), - onAgentSignature: ( - signature: Hex, - mainAddress: Hex, - agent: PerpsAgentSigner, - ): void => this.#recordAgentSignature(signature, mainAddress, agent), + await this.#resolveTrackedAgentSigner(mainAddress), }, ); this.#subscriptionService = new HyperLiquidSubscriptionService( @@ -2416,7 +2411,8 @@ export class HyperLiquidProvider implements PerpsProvider { * before performing actions", if the venue no longer lists it (revoked, or * past its `validUntil`). The venue answers an account with no funds the * same way, so the agent list decides; it is asked once per signature. A - * request signed on another network or before a disconnect is not checked. + * signature asked for on another network or before a disconnect is not + * checked. * * @param signature - The agent signature of the request. * @returns The revoked agent, or undefined. @@ -2436,28 +2432,44 @@ export class HyperLiquidProvider implements PerpsProvider { } /** - * Note an agent's L1 signature, so the venue's answer to the request it - * signs can be attributed to it. + * Resolve the agent for an L1 signature, as #resolveAgentSigner does, and + * note each signature it makes with the network and session it was asked + * in, so the venue's answer to that request is attributed to it. Both are + * read before resolving and signing, which can outlast a network switch or + * a disconnect. * - * @param signature - The signature. - * @param mainAddress - The main account the agent signed for. - * @param agent - The agent. + * @param mainAddress - The selected main account. + * @returns The agent, or null to sign with the main account. */ - #recordAgentSignature( - signature: Hex, + async #resolveTrackedAgentSigner( mainAddress: Hex, - agent: PerpsAgentSigner, - ): void { + ): Promise { const account: PerpsAgentAccount = { mainAddress, isTestnet: this.#clientService.isTestnetMode(), }; - this.#unansweredAgentSignatures.set(getSignatureKey(signature), { - key: getAgentAccountKey(account), - account, - agentAddress: agent.address, - generation: this.#lifecycleGeneration, - }); + const generation = this.#lifecycleGeneration; + const agent = await this.#resolveAgentSigner(mainAddress); + if (!agent) { + return null; + } + return { + address: agent.address, + signTypedData: async (payload): Promise => { + const signature = await agent.signTypedData(payload); + // The SDK refuses any other signature before sending the request, + // so no answer would come for it. + if (isHexString(signature) && signature.length === 132) { + this.#unansweredAgentSignatures.set(getSignatureKey(signature), { + key: getAgentAccountKey(account), + account, + agentAddress: agent.address, + generation, + }); + } + return signature; + }, + }; } /** @@ -15921,6 +15933,7 @@ export class HyperLiquidProvider implements PerpsProvider { this.#referralCheckCache.clear(); this.#builderFeeCheckCache.clear(); this.#builderFeeRefusals.clear(); + this.#unansweredAgentSignatures.clear(); this.#subscriptionBuilderApprovalEpoch += 1; this.#approvedBuilderAddresses.clear(); this.#userFeeResolution = undefined; diff --git a/packages/perps-controller/src/services/HyperLiquidWalletService.ts b/packages/perps-controller/src/services/HyperLiquidWalletService.ts index 679ab1959c4..a18ca0e503e 100644 --- a/packages/perps-controller/src/services/HyperLiquidWalletService.ts +++ b/packages/perps-controller/src/services/HyperLiquidWalletService.ts @@ -47,15 +47,6 @@ const HARDWARE_KEYRING_TYPES = new Set([ */ type AgentResolver = (mainAddress: Hex) => Promise; -/** - * Told of every L1 signature an agent made for a main account. - */ -type AgentSignatureListener = ( - signature: Hex, - mainAddress: Hex, - agent: PerpsAgentSigner, -) => void; - /** * Whether a signing request is an L1 action. * @@ -83,22 +74,15 @@ export class HyperLiquidWalletService { readonly #resolveAgent: AgentResolver | undefined; - readonly #onAgentSignature: AgentSignatureListener | undefined; - constructor( deps: PerpsPlatformDependencies, messenger: PerpsControllerMessengerBase, - options: { - isTestnet?: boolean; - resolveAgent?: AgentResolver; - onAgentSignature?: AgentSignatureListener; - } = {}, + options: { isTestnet?: boolean; resolveAgent?: AgentResolver } = {}, ) { this.#deps = deps; this.#messenger = messenger; this.#isTestnet = options.isTestnet ?? false; this.#resolveAgent = options.resolveAgent; - this.#onAgentSignature = options.onAgentSignature; } /** @@ -272,16 +256,13 @@ export class HyperLiquidWalletService { 'HyperLiquidWalletService: Signing L1 action with agent', { address: mainAddress, agent: agentSigner.address }, ); - let signature: Hex; try { - signature = await agentSigner.signTypedData(params); + return await agentSigner.signTypedData(params); } catch (error) { // The host could not sign with its agent key (for example it locked // after resolving it). Retryable, like a locked keyring. throw new AgentSignerUnavailableError(error); } - this.#onAgentSignature?.(signature, mainAddress, agentSigner); - return signature; }, getChainId: async (): Promise => parseInt(getChainId(this.#isTestnet), 10), diff --git a/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts b/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts index aa81d8e42f2..bd1de439f5e 100644 --- a/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts +++ b/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts @@ -350,9 +350,15 @@ export type AccountSignerFixture = { initialize: jest.Mock, [HyperLiquidWalletParams]>; // The wallet the provider last initialized the SDK clients with. sdkWallet: () => HyperLiquidWalletParams; - // Report the venue's answer to a request carrying `signature` to the - // provider, as the client service does before the SDK reads the answer. - reportAnswer: (signature: Hex, answer: Record) => void; + // Report an exchange request and the venue's answer (undefined when the + // request failed without one) to the provider, as the client service does + // before the SDK reads the answer. + reportExchangeRequest: (payload: unknown, answer: unknown) => void; + // reportExchangeRequest for a request carrying `signature`. + reportAnswer: ( + signature: Hex, + answer: Record | undefined, + ) => void; // Sign an exchange request through the SDK wallet and send it the way the // SDK does: the venue's answer is reported to the provider with the // request's signature, then thrown as the SDK's ApiRequestError when it is @@ -407,11 +413,13 @@ export function createAccountSignerProvider( let onExchangeRequest: | ((payload: unknown, answer: unknown) => void) | undefined; + const reportExchangeRequest = (payload: unknown, answer: unknown): void => + onExchangeRequest?.(payload, answer); const reportAnswer = ( signature: Hex, - answer: Record, + answer: Record | undefined, ): void => - onExchangeRequest?.({ signature: { r: signature.slice(0, 66) } }, answer); + reportExchangeRequest({ signature: { r: signature.slice(0, 66) } }, answer); const signAndSend = async ( payload: PerpsTypedDataPayload, answer: Record, @@ -488,6 +496,7 @@ export function createAccountSignerProvider( infoClient, initialize, sdkWallet, + reportExchangeRequest, reportAnswer, signAndSend, selectAccount, diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts index d39a768aead..248c20fc50d 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts @@ -1,4 +1,5 @@ import { getChecksumAddress } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; import { PERPS_ERROR_CODES } from '../../../src/perpsErrorCodes.js'; import type { PerpsAgentAccount } from '../../../src/types/index.js'; @@ -41,6 +42,9 @@ jest.mock('../../../src/services/HyperLiquidSubscriptionService'); const HOUR_MS = 60 * 60 * 1000; +// The agent's signature of a mainnet order, distinct from AGENT_SIGNATURE. +const MAINNET_ORDER_SIGNATURE = `0x${'1f'.repeat(65)}` as const; + /** * The venue's answer to a write for an account that must deposit. * @@ -92,6 +96,47 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit ({ loggerError, mockClientService } = setUpAccountSignerSuite()); }); + /** + * Let `toggleTestnet` switch the mocked client service's network. + */ + function useSwitchableNetwork(): void { + let isTestnet = false; + mockClientService.isTestnetMode.mockImplementation(() => isTestnet); + mockClientService.setTestnetMode.mockImplementation((value: boolean) => { + isTestnet = value; + }); + } + + /** + * A provider whose L1 actions an agent signs on either network, with its + * trading setup (and the referral the agent signs) done. + * + * @param agents - Answers `extraAgents` for the main account. + * @returns The provider and its mocks. + */ + async function createPreparedAgentProvider( + agents: () => Promise = async () => [], + ): Promise< + AccountSignerFixture & { + getAgentSigner: jest.Mock; + onAgentRejected: jest.Mock; + extraAgents: jest.Mock; + } + > { + const getAgentSigner = jest.fn(); + const onAgentRejected = jest.fn(); + const extraAgents = jest.fn(agents); + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + onAgentRejected, + info: { extraAgents }, + }); + getAgentSigner.mockResolvedValue(built.agentSigner); + await built.accountSignerProvider.prepareTradingWallet(); + return { ...built, getAgentSigner, onAgentRejected, extraAgents }; + } + /** * A provider whose L1 writes are signed by the agent, then refused by the * venue as an account that must deposit. @@ -475,11 +520,7 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit }); it('does not check a request signed before the provider switched networks', async () => { - let isTestnet = false; - mockClientService.isTestnetMode.mockImplementation(() => isTestnet); - mockClientService.setTestnetMode.mockImplementation((value: boolean) => { - isTestnet = value; - }); + useSwitchableNetwork(); const getAgentSigner = jest.fn(); const onAgentRejected = jest.fn(); const extraAgents = jest.fn().mockResolvedValue([]); @@ -529,6 +570,195 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit ).toStrictEqual([[TESTNET_ACCOUNT]]); }); + it('does not check a request whose agent signature finished after the provider switched networks', async () => { + useSwitchableNetwork(); + // The host reuses its agent on both networks. + const { + accountSignerProvider, + agentSigner, + exchangeClient, + extraAgents, + getAgentSigner, + onAgentRejected, + sdkWallet, + signAndSend, + } = await createPreparedAgentProvider(); + const signingStarted = createDeferred(); + const signing = createDeferred(); + agentSigner.signTypedData.mockImplementationOnce(async () => { + signingStarted.resolve(); + return await signing.promise; + }); + exchangeClient.order.mockImplementationOnce( + async () => + await signAndSend(L1_PAYLOAD, mustDepositAnswer(MAIN_ADDRESS)), + ); + + const ordering = accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await signingStarted.promise; + await accountSignerProvider.toggleTestnet(); + // The agent signs on testnet, then the mainnet signature finishes and + // the venue answers it. + await sdkWallet().signTypedData(L1_PAYLOAD); + signing.resolve(MAINNET_ORDER_SIGNATURE); + await ordering; + await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + // The agent stays bound on testnet. + expect( + (getAgentSigner.mock.calls as [PerpsAgentAccount][]).filter( + ([account]) => account.isTestnet, + ), + ).toStrictEqual([[TESTNET_ACCOUNT]]); + }); + + it('does not check a request whose agent signature finished after the provider reconnected on the same network', async () => { + useSwitchableNetwork(); + const { + accountSignerProvider, + agentSigner, + exchangeClient, + extraAgents, + onAgentRejected, + signAndSend, + } = await createPreparedAgentProvider(); + const signingStarted = createDeferred(); + const signing = createDeferred(); + agentSigner.signTypedData.mockImplementationOnce(async () => { + signingStarted.resolve(); + return await signing.promise; + }); + exchangeClient.order.mockImplementationOnce( + async () => + await signAndSend(L1_PAYLOAD, mustDepositAnswer(MAIN_ADDRESS)), + ); + + const ordering = accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await signingStarted.promise; + // Back on mainnet, in a new session. + await accountSignerProvider.toggleTestnet(); + await accountSignerProvider.toggleTestnet(); + signing.resolve(MAINNET_ORDER_SIGNATURE); + await ordering; + + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + }); + + it('keeps the agent when the provider disconnects while the agent list is read', async () => { + const asked = createDeferred(); + const listing = createDeferred(); + const { + accountSignerProvider, + exchangeClient, + extraAgents, + onAgentRejected, + signAndSend, + } = await createPreparedAgentProvider(async () => { + asked.resolve(); + return await listing.promise; + }); + exchangeClient.order.mockImplementationOnce( + async () => + await signAndSend(L1_PAYLOAD, mustDepositAnswer(MAIN_ADDRESS)), + ); + + const ordering = accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await asked.promise; + const disconnecting = accountSignerProvider.disconnect(); + listing.resolve([]); + await disconnecting; + const order = await ordering; + + expect(order).toStrictEqual({ + success: false, + error: mustDepositError(MAIN_ADDRESS).message, + }); + expect(extraAgents).toHaveBeenCalledTimes(1); + expect(onAgentRejected).not.toHaveBeenCalled(); + }); + + it('does not attribute an answer to a request that carries no signature', async () => { + const { + accountSignerProvider, + exchangeClient, + extraAgents, + onAgentRejected, + reportExchangeRequest, + sdkWallet, + } = await createPreparedAgentProvider(); + const answer = mustDepositAnswer(MAIN_ADDRESS); + exchangeClient.order.mockImplementationOnce(async () => { + await sdkWallet().signTypedData(L1_PAYLOAD); + reportExchangeRequest({ action: { type: 'order' } }, answer); + throw apiRequestError(answer); + }); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(order).toStrictEqual({ + success: false, + error: mustDepositError(MAIN_ADDRESS).message, + }); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + }); + + it('keeps the agent when its request fails without an answer', async () => { + const { + accountSignerProvider, + exchangeClient, + extraAgents, + getAgentSigner, + onAgentRejected, + reportAnswer, + sdkWallet, + } = await createPreparedAgentProvider(); + exchangeClient.order.mockImplementationOnce(async () => { + const signature = await sdkWallet().signTypedData(L1_PAYLOAD); + reportAnswer(signature, undefined); + throw new Error('fetch failed'); + }); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(order).toStrictEqual({ success: false, error: 'fetch failed' }); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + expect(getAgentSigner.mock.calls).toStrictEqual([[MAINNET_ACCOUNT]]); + }); + + it('does not track an agent signature the SDK would refuse', async () => { + const { + accountSignerProvider, + agentSigner, + exchangeClient, + extraAgents, + onAgentRejected, + reportAnswer, + sdkWallet, + } = await createPreparedAgentProvider(); + // Not a 65-byte signature: the SDK refuses it before sending. + agentSigner.signTypedData.mockResolvedValueOnce('0x1234'); + const answer = mustDepositAnswer(MAIN_ADDRESS); + exchangeClient.order.mockImplementationOnce(async () => { + reportAnswer(await sdkWallet().signTypedData(L1_PAYLOAD), answer); + throw apiRequestError(answer); + }); + + const order = await accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + + expect(order).toStrictEqual({ + success: false, + error: mustDepositError(MAIN_ADDRESS).message, + }); + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + }); + describe('without an agent', () => { it('fails the order with the venue error without checking the agents', async () => { const getAgentSigner = jest.fn().mockResolvedValue(null); diff --git a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts index bd36c1c2074..d1bb8494671 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts @@ -224,11 +224,9 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { resolveAgent: jest.Mock; agentSign: jest.Mock; mainSign: jest.Mock; - onAgentSignature: jest.Mock; call: jest.SpyInstance; selectAccount: (address: `0x${string}`) => void; } { - const onAgentSignature = jest.fn(); const signer = { signTypedData: jest.fn().mockResolvedValue(MAIN_SIGNATURE), signPersonalMessage: jest.fn(), @@ -245,14 +243,13 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { const service = new HyperLiquidWalletService( { ...createMockInfrastructure(), accountSigner: signer }, messenger, - { isTestnet: true, resolveAgent, onAgentSignature }, + { isTestnet: true, resolveAgent }, ); return { adapter: service.createWalletAdapter(), resolveAgent, agentSign, mainSign: signer.signTypedData, - onAgentSignature, call, selectAccount, }; @@ -264,33 +261,6 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { expect(adapter.address).toBe(mainAddress); }); - it('reports each agent signature with the main account it signed for', async () => { - const { adapter, onAgentSignature } = buildAdapter(); - - await adapter.signTypedData(L1_PAYLOAD); - - expect(onAgentSignature).toHaveBeenCalledTimes(1); - const [signature, account, agent] = onAgentSignature.mock.calls[0] as [ - Hex, - Hex, - PerpsAgentSigner, - ]; - expect([signature, account, agent.address]).toStrictEqual([ - AGENT_SIGNATURE, - mainAddress, - AGENT_ADDRESS, - ]); - }); - - it('does not report signatures of the main account', async () => { - const { adapter, onAgentSignature } = buildAdapter(false); - - await adapter.signTypedData(L1_PAYLOAD); - await adapter.signTypedData(USER_SIGNED_PAYLOAD); - - expect(onAgentSignature).not.toHaveBeenCalled(); - }); - it('signs L1 actions with the agent resolved for the selected account', async () => { const { adapter, resolveAgent, agentSign, mainSign, call } = buildAdapter(); @@ -613,25 +583,8 @@ describeWithSdk( expect(await recoverSigner(signatures[0])).toBe(mainAccount.address); }); - it('reports the agent signature an SDK L1 request carries', async () => { - const { messenger, selectAccount } = createKeyringlessMessenger(); - selectAccount(mainAccount.address); - const onAgentSignature = jest.fn(); - const adapter = new HyperLiquidWalletService( - { - ...createMockInfrastructure(), - accountSigner: { - signTypedData: async (): Promise => MAIN_SIGNATURE, - signPersonalMessage: async (): Promise => MAIN_SIGNATURE, - }, - }, - messenger, - { - isTestnet: true, - resolveAgent: async (): Promise => agentAccount, - onAgentSignature, - }, - ).createWalletAdapter(); + it('sends the r value of the agent signature as the agent signed it', async () => { + const { adapter, agentSignatures } = buildSdkAdapter(); // The exchange endpoint, answering the way the venue does. const request = jest.fn().mockResolvedValue({ status: 'ok', @@ -643,8 +596,7 @@ describeWithSdk( { cancels: [{ a: 0, o: 1 }] }, ); - expect(onAgentSignature).toHaveBeenCalledTimes(1); - const [[signature]] = onAgentSignature.mock.calls as [[Hex]]; + const [{ signature }] = await agentSignatures(); const [, sent] = request.mock.calls[0] as [ string, { signature: { r: string } }, From 0c6379375382ed2eac765ba5cfc8b7e02d4b8330 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Thu, 1 Oct 2026 16:00:07 +0800 Subject: [PATCH 5/6] fix(perps-controller): bind agent signatures to the wallet that made them The SDK queues a write before it asks the wallet to sign, so a mainnet write queued behind another one could sign after toggleTestnet(): it then resolved the testnet agent, recorded the new network and session, and its mainnet rejection was checked against testnet agents. - Each wallet adapter the provider creates is bound to the network and lifecycle generation of its exchange client; it resolves agents on that network and records its agent signatures in the binding. - The client service reports the wallet with each exchange request, so the venue's answer is matched in that wallet's binding. - createWalletAdapter takes the resolver for that adapter. --- packages/perps-controller/CHANGELOG.md | 3 +- .../src/providers/HyperLiquidProvider.ts | 105 +++++++++++------- .../src/services/HyperLiquidClientService.ts | 32 ++++-- .../src/services/HyperLiquidWalletService.ts | 10 +- .../hyperLiquidAccountSignerFixture.ts | 40 +++++-- ...rLiquidProvider.agent-must-deposit.test.ts | 100 ++++++++++++++++- .../HyperLiquidProvider.agent-signer.test.ts | 13 ++- .../services/HyperLiquidClientService.test.ts | 8 +- ...LiquidWalletService.account-signer.test.ts | 28 +++++ 9 files changed, 273 insertions(+), 66 deletions(-) diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index b2ee5d74790..87a1f483787 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -19,8 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Accept Lighter trades that omit position-sign flags, preserve omitted account PnL as unknown, and reject known reductions without realized PnL. Retain side-only fill directions when lifecycle context is ambiguous ([#10605](https://github.com/MetaMask/core/pull/10605)) - Detect a revoked or expired HyperLiquid agent that the venue answers with "Must deposit before performing actions" ([#10643](https://github.com/MetaMask/core/pull/10643)) - - The provider then checks `extraAgents` for the agent that signed the answered request. When it is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called. Requests signed by the main account, or whose signature was asked for on another network or before a disconnect, are not checked + - The provider then checks `extraAgents` for the agent that signed the answered request. When it is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called. Requests signed by the main account, or by a wallet of an earlier network or session, are not checked - When the agent is still listed (an account with no funds), or the list cannot be read, the venue error is returned as before +- Sign a HyperLiquid L1 action the SDK queued before a network switch with the agent of the network it was queued on, not the new network's ([#10643](https://github.com/MetaMask/core/pull/10643)) - Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#10643](https://github.com/MetaMask/core/pull/10643)) - Until the provider disconnects, the approval is not requested again: orders are sent as after any failed approval, TP/SL updates fail with `TPSL_UPDATE_FAILED`, and `prepareTradingWallet` returns the venue error diff --git a/packages/perps-controller/src/providers/HyperLiquidProvider.ts b/packages/perps-controller/src/providers/HyperLiquidProvider.ts index b9319ae69fe..f7f92adb570 100644 --- a/packages/perps-controller/src/providers/HyperLiquidProvider.ts +++ b/packages/perps-controller/src/providers/HyperLiquidProvider.ts @@ -58,6 +58,7 @@ import { HyperLiquidClientService, WebSocketConnectionState, } from '../services/HyperLiquidClientService.js'; +import type { HyperLiquidWalletParams } from '../services/HyperLiquidClientService.js'; import { HyperLiquidSubscriptionService } from '../services/HyperLiquidSubscriptionService.js'; import { HyperLiquidWalletService } from '../services/HyperLiquidWalletService.js'; import { @@ -502,13 +503,23 @@ type AgentSignature = { key: string; account: PerpsAgentAccount; agentAddress: Hex; - // The provider lifecycle generation the signature was asked for in. + // The provider lifecycle generation of the wallet that made it. generation?: number; // Whether the venue still lists the agent, asked at most once per // signature (see #findRevokedAgent). approvalCheck?: Promise; }; +// What a wallet adapter, and so the exchange client signing through it, was +// created for: its network and provider lifecycle generation. It holds the +// agent signatures it made that the venue has not answered yet, keyed by +// getSignatureKey. +type WalletBinding = { + isTestnet: boolean; + generation: number; + unansweredSignatures: Map; +}; + /** * The venue answer an SDK error carries. * @@ -1690,9 +1701,11 @@ export class HyperLiquidProvider implements PerpsProvider { // its action was in flight is still recognized. readonly #agentSignedFor = new Map(); - // Agent signatures whose exchange request has not been answered yet, keyed - // by getSignatureKey. Cleared on disconnect. - readonly #unansweredAgentSignatures = new Map(); + // The binding of each wallet adapter this provider created. + readonly #walletBindings = new WeakMap< + HyperLiquidWalletParams, + WalletBinding + >(); // The agent signature of each answered exchange request, keyed by the // venue answer. A "Must deposit" answer is attributed from it, so only to @@ -1755,8 +1768,11 @@ export class HyperLiquidProvider implements PerpsProvider { // Initialize services with injected platform dependencies this.#clientService = new HyperLiquidClientService(this.#deps, { isTestnet, - onExchangeRequest: (payload: unknown, answer: unknown): void => - this.#matchAgentSignature(payload, answer), + onExchangeRequest: ( + payload: unknown, + answer: unknown, + wallet: HyperLiquidWalletParams, + ): void => this.#matchAgentSignature(payload, answer, wallet), }); this.#dexDiscoveryCache = new DexDiscoveryCacheManager({ isTestnetMode: (): boolean => this.#clientService.isTestnetMode(), @@ -1766,13 +1782,7 @@ export class HyperLiquidProvider implements PerpsProvider { this.#walletService = new HyperLiquidWalletService( this.#deps, this.#messenger, - { - isTestnet, - resolveAgent: async ( - mainAddress: Hex, - ): Promise => - await this.#resolveTrackedAgentSigner(mainAddress), - }, + { isTestnet }, ); this.#subscriptionService = new HyperLiquidSubscriptionService( this.#clientService, @@ -2215,7 +2225,19 @@ export class HyperLiquidProvider implements PerpsProvider { throw new Error(PERPS_ERROR_CODES.PROVIDER_LIFECYCLE_STALE); } - const wallet = this.#walletService.createWalletAdapter(); + // The adapter keeps the network and session it is created for: its + // agents are resolved, and its signatures attributed, for them, even + // when a queued write signs after a network switch or a disconnect. + const binding: WalletBinding = { + isTestnet: this.#clientService.isTestnetMode(), + generation: this.#lifecycleGeneration, + unansweredSignatures: new Map(), + }; + const wallet = this.#walletService.createWalletAdapter( + async (mainAddress: Hex): Promise => + await this.#resolveTrackedAgentSigner(mainAddress, binding), + ); + this.#walletBindings.set(wallet, binding); await this.#clientService.initialize(wallet); if (this.#disconnectOperationsInFlight > 0) { throw new Error(PERPS_ERROR_CODES.PROVIDER_LIFECYCLE_STALE); @@ -2267,21 +2289,20 @@ export class HyperLiquidProvider implements PerpsProvider { } /** - * Resolve the agent that signs L1 actions for a main account on the - * current network through `getAgentSigner`. A non-null answer is kept; + * Resolve the agent that signs L1 actions for a main account on a network + * through `getAgentSigner`. A non-null answer is kept; * null and failures are not, so the next L1 action asks again. An answer * pending across clearAgentSigners is discarded and asked again. * * @param mainAddress - The selected main account. + * @param isTestnet - The network of the wallet adapter that signs. * @returns The agent, or null to sign with the main account. */ async #resolveAgentSigner( mainAddress: Hex, + isTestnet: boolean, ): Promise { - const account: PerpsAgentAccount = { - mainAddress, - isTestnet: this.#clientService.isTestnetMode(), - }; + const account: PerpsAgentAccount = { mainAddress, isTestnet }; const key = getAgentAccountKey(account); const generation = this.#agentSignersGeneration; let entry = this.#agentSigners.get(key); @@ -2311,7 +2332,7 @@ export class HyperLiquidProvider implements PerpsProvider { agentSigner = await pendingEntry.answer; } catch (error) { if (isSuperseded()) { - return await this.#resolveAgentSigner(mainAddress); + return await this.#resolveAgentSigner(mainAddress, isTestnet); } this.#agentSigners.delete(key); this.#deps.debugLogger.log('HyperLiquidProvider: getAgentSigner failed', { @@ -2322,7 +2343,7 @@ export class HyperLiquidProvider implements PerpsProvider { } if (isSuperseded()) { - return await this.#resolveAgentSigner(mainAddress); + return await this.#resolveAgentSigner(mainAddress, isTestnet); } if (agentSigner) { pendingEntry.agent = agentSigner; @@ -2411,8 +2432,8 @@ export class HyperLiquidProvider implements PerpsProvider { * before performing actions", if the venue no longer lists it (revoked, or * past its `validUntil`). The venue answers an account with no funds the * same way, so the agent list decides; it is asked once per signature. A - * signature asked for on another network or before a disconnect is not - * checked. + * signature made through the wallet of an earlier network or session is + * not checked. * * @param signature - The agent signature of the request. * @returns The revoked agent, or undefined. @@ -2432,24 +2453,26 @@ export class HyperLiquidProvider implements PerpsProvider { } /** - * Resolve the agent for an L1 signature, as #resolveAgentSigner does, and - * note each signature it makes with the network and session it was asked - * in, so the venue's answer to that request is attributed to it. Both are - * read before resolving and signing, which can outlast a network switch or - * a disconnect. + * Resolve the agent for an L1 signature of a wallet adapter, on the + * adapter's network, and note each signature it makes in the adapter's + * binding, so the venue's answer to that request is attributed to it. * * @param mainAddress - The selected main account. + * @param binding - The binding of the wallet adapter that signs. * @returns The agent, or null to sign with the main account. */ async #resolveTrackedAgentSigner( mainAddress: Hex, + binding: WalletBinding, ): Promise { const account: PerpsAgentAccount = { mainAddress, - isTestnet: this.#clientService.isTestnetMode(), + isTestnet: binding.isTestnet, }; - const generation = this.#lifecycleGeneration; - const agent = await this.#resolveAgentSigner(mainAddress); + const agent = await this.#resolveAgentSigner( + mainAddress, + binding.isTestnet, + ); if (!agent) { return null; } @@ -2460,11 +2483,11 @@ export class HyperLiquidProvider implements PerpsProvider { // The SDK refuses any other signature before sending the request, // so no answer would come for it. if (isHexString(signature) && signature.length === 132) { - this.#unansweredAgentSignatures.set(getSignatureKey(signature), { + binding.unansweredSignatures.set(getSignatureKey(signature), { key: getAgentAccountKey(account), account, agentAddress: agent.address, - generation, + generation: binding.generation, }); } return signature; @@ -2478,8 +2501,13 @@ export class HyperLiquidProvider implements PerpsProvider { * * @param payload - The exchange request. * @param answer - The venue's answer, or undefined when there is none. + * @param wallet - The wallet adapter the request was signed through. */ - #matchAgentSignature(payload: unknown, answer: unknown): void { + #matchAgentSignature( + payload: unknown, + answer: unknown, + wallet: HyperLiquidWalletParams, + ): void { const signature = isStatusObject(payload) && isStatusObject(payload.signature) ? payload.signature.r @@ -2487,12 +2515,14 @@ export class HyperLiquidProvider implements PerpsProvider { if (typeof signature !== 'string') { return; } + const unansweredSignatures = + this.#walletBindings.get(wallet)?.unansweredSignatures; const key = getSignatureKey(signature); - const agentSignature = this.#unansweredAgentSignatures.get(key); + const agentSignature = unansweredSignatures?.get(key); if (!agentSignature) { return; } - this.#unansweredAgentSignatures.delete(key); + unansweredSignatures?.delete(key); if (isStatusObject(answer)) { this.#agentSignatureByAnswer.set(answer, agentSignature); } @@ -15933,7 +15963,6 @@ export class HyperLiquidProvider implements PerpsProvider { this.#referralCheckCache.clear(); this.#builderFeeCheckCache.clear(); this.#builderFeeRefusals.clear(); - this.#unansweredAgentSignatures.clear(); this.#subscriptionBuilderApprovalEpoch += 1; this.#approvedBuilderAddresses.clear(); this.#userFeeResolution = undefined; diff --git a/packages/perps-controller/src/services/HyperLiquidClientService.ts b/packages/perps-controller/src/services/HyperLiquidClientService.ts index 1766e30c3a4..b1cb486b6d8 100644 --- a/packages/perps-controller/src/services/HyperLiquidClientService.ts +++ b/packages/perps-controller/src/services/HyperLiquidClientService.ts @@ -57,6 +57,16 @@ export type HyperLiquidWalletParams = { signatureChainId?: () => Promise; }; +/** + * Told of an exchange request, the venue's answer (undefined when the request + * failed before one) and the wallet the request was signed with. + */ +type ExchangeRequestListener = ( + payload: unknown, + answer: unknown, + wallet: HyperLiquidWalletParams, +) => void; + // WebSocketConnectionState is now imported from controllers/types // Re-export for backward compatibility with existing consumers export { WebSocketConnectionState } from '../types/index.js'; @@ -80,7 +90,7 @@ export class HyperLiquidClientService { #httpTransport?: HttpTransport; - readonly #onExchangeRequest?: (payload: unknown, response: unknown) => void; + readonly #onExchangeRequest?: ExchangeRequestListener; #walletParams?: HyperLiquidWalletParams; @@ -127,14 +137,15 @@ export class HyperLiquidClientService { * @param options - Options. * @param options.isTestnet - Start on testnet. * @param options.onExchangeRequest - Called with every exchange request's - * payload and the venue's answer (undefined when the request failed before - * one), before the SDK checks the answer. + * payload, the venue's answer (undefined when the request failed before + * one) and the wallet the exchange client signs with, before the SDK + * checks the answer. */ constructor( deps: PerpsPlatformDependencies, options: { isTestnet?: boolean; - onExchangeRequest?: (payload: unknown, response: unknown) => void; + onExchangeRequest?: ExchangeRequestListener; } = {}, ) { this.#deps = deps; @@ -373,7 +384,10 @@ export class HyperLiquidClientService { if (effectiveWallet) { this.#exchangeClient = new ExchangeClient({ wallet: effectiveWallet as any, // eslint-disable-line @typescript-eslint/no-explicit-any -- Type widening for SDK compatibility - transport: this.#reportingExchangeRequests(this.#httpTransport), + transport: this.#reportingExchangeRequests( + this.#httpTransport, + effectiveWallet, + ), ...(effectiveWallet.signatureChainId && { signatureChainId: effectiveWallet.signatureChainId, }), @@ -388,9 +402,13 @@ export class HyperLiquidClientService { * request and its answer to `onExchangeRequest` when it is set. * * @param transport - The HTTP transport. + * @param wallet - The wallet the exchange client signs with. * @returns The transport to send exchange requests through. */ - #reportingExchangeRequests(transport: HttpTransport): IRequestTransport { + #reportingExchangeRequests( + transport: HttpTransport, + wallet: HyperLiquidWalletParams, + ): IRequestTransport { const onExchangeRequest = this.#onExchangeRequest; if (!onExchangeRequest) { return transport; @@ -412,7 +430,7 @@ export class HyperLiquidClientService { return response; } finally { if (endpoint === 'exchange') { - onExchangeRequest(payload, response); + onExchangeRequest(payload, response, wallet); } } }, diff --git a/packages/perps-controller/src/services/HyperLiquidWalletService.ts b/packages/perps-controller/src/services/HyperLiquidWalletService.ts index a18ca0e503e..4a4da4d3037 100644 --- a/packages/perps-controller/src/services/HyperLiquidWalletService.ts +++ b/packages/perps-controller/src/services/HyperLiquidWalletService.ts @@ -235,10 +235,14 @@ export class HyperLiquidWalletService { * main account always signs them, for the chain the account signer reports * when it reports one. * + * @param resolveAgent - Resolves the agent for this adapter's L1 + * signatures, in place of the service's `resolveAgent`. * @returns The wallet adapter with address, signTypedData, and getChainId * methods, and signatureChainId when the account signer has getChainId. */ - public createWalletAdapter(): HyperLiquidWalletParams { + public createWalletAdapter( + resolveAgent: AgentResolver | undefined = this.#resolveAgent, + ): HyperLiquidWalletParams { const { accountSigner } = this.#deps; const getSignerChainId = accountSigner?.getChainId?.bind(accountSigner); return { @@ -246,8 +250,8 @@ export class HyperLiquidWalletService { signTypedData: async (params: PerpsTypedDataPayload): Promise => { const mainAddress = this.#getSelectedMainAddress(); const agentSigner = - this.#resolveAgent && isL1Action(params) - ? await this.#resolveAgent(mainAddress) + resolveAgent && isL1Action(params) + ? await resolveAgent(mainAddress) : null; if (!agentSigner) { return await this.#signWithMainAccount(mainAddress, params); diff --git a/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts b/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts index bd1de439f5e..57f3ed5aa9a 100644 --- a/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts +++ b/packages/perps-controller/tests/helpers/hyperLiquidAccountSignerFixture.ts @@ -352,12 +352,18 @@ export type AccountSignerFixture = { sdkWallet: () => HyperLiquidWalletParams; // Report an exchange request and the venue's answer (undefined when the // request failed without one) to the provider, as the client service does - // before the SDK reads the answer. - reportExchangeRequest: (payload: unknown, answer: unknown) => void; + // before the SDK reads the answer. `wallet` is the wallet the request was + // signed with, by default the current one. + reportExchangeRequest: ( + payload: unknown, + answer: unknown, + wallet?: HyperLiquidWalletParams, + ) => void; // reportExchangeRequest for a request carrying `signature`. reportAnswer: ( signature: Hex, answer: Record | undefined, + wallet?: HyperLiquidWalletParams, ) => void; // Sign an exchange request through the SDK wallet and send it the way the // SDK does: the venue's answer is reported to the provider with the @@ -367,6 +373,7 @@ export type AccountSignerFixture = { payload: PerpsTypedDataPayload, answer: Record, delivery?: 'thrown' | 'returned', + wallet?: HyperLiquidWalletParams, ) => Promise>; selectAccount: (address: Hex) => void; deselectAccount: () => void; @@ -409,23 +416,40 @@ export function createAccountSignerProvider( return wallet; }; // The provider's listener for exchange requests, which the client service - // calls with each request and the venue's answer. + // calls with each request, the venue's answer and the wallet it signed with. let onExchangeRequest: - | ((payload: unknown, answer: unknown) => void) + | (( + payload: unknown, + answer: unknown, + wallet: HyperLiquidWalletParams, + ) => void) | undefined; - const reportExchangeRequest = (payload: unknown, answer: unknown): void => - onExchangeRequest?.(payload, answer); + const reportExchangeRequest = ( + payload: unknown, + answer: unknown, + signingWallet: HyperLiquidWalletParams = sdkWallet(), + ): void => onExchangeRequest?.(payload, answer, signingWallet); const reportAnswer = ( signature: Hex, answer: Record | undefined, + signingWallet: HyperLiquidWalletParams = sdkWallet(), ): void => - reportExchangeRequest({ signature: { r: signature.slice(0, 66) } }, answer); + reportExchangeRequest( + { signature: { r: signature.slice(0, 66) } }, + answer, + signingWallet, + ); const signAndSend = async ( payload: PerpsTypedDataPayload, answer: Record, delivery: 'thrown' | 'returned' = 'thrown', + signingWallet: HyperLiquidWalletParams = sdkWallet(), ): Promise> => { - reportAnswer(await signThroughWallet(sdkWallet(), payload), answer); + reportAnswer( + await signThroughWallet(signingWallet, payload), + answer, + signingWallet, + ); if (delivery === 'returned' || getVenueErrorMessage(answer) === undefined) { return answer; } diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts index 248c20fc50d..f5187918377 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts @@ -542,10 +542,12 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit const venue = createDeferred(); const answer = mustDepositAnswer(MAIN_ADDRESS); built.exchangeClient.order.mockImplementationOnce(async () => { - const signature = await built.sdkWallet().signTypedData(L1_PAYLOAD); + // The mainnet client's wallet, which the request is signed with. + const wallet = built.sdkWallet(); + const signature = await wallet.signTypedData(L1_PAYLOAD); signed.resolve(); await venue.promise; - built.reportAnswer(signature, answer); + built.reportAnswer(signature, answer, wallet); throw apiRequestError(answer); }); @@ -614,6 +616,100 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit ).toStrictEqual([[TESTNET_ACCOUNT]]); }); + it('does not check a write queued on mainnet that signs after the provider switched to testnet', async () => { + useSwitchableNetwork(); + // The host reuses its agent on both networks. + const { + accountSignerProvider, + exchangeClient, + extraAgents, + getAgentSigner, + onAgentRejected, + sdkWallet, + signAndSend, + } = await createPreparedAgentProvider(); + const queued = createDeferred(); + const ahead = createDeferred(); + exchangeClient.order.mockImplementationOnce(async () => { + // The mainnet client's wallet. The SDK queues the write behind another + // one, so it signs later. + const wallet = sdkWallet(); + queued.resolve(); + await ahead.promise; + return await signAndSend( + L1_PAYLOAD, + mustDepositAnswer(MAIN_ADDRESS), + 'thrown', + wallet, + ); + }); + + const ordering = accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await queued.promise; + await accountSignerProvider.toggleTestnet(); + // The agent signs on testnet, then the queued mainnet write signs and + // the venue answers it. + await sdkWallet().signTypedData(L1_PAYLOAD); + ahead.resolve(); + await ordering; + await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(extraAgents).not.toHaveBeenCalled(); + expect(onAgentRejected).not.toHaveBeenCalled(); + // The queued write resolved the mainnet agent, and the agent stays bound + // on testnet. + expect(getAgentSigner.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT], + [TESTNET_ACCOUNT], + ]); + }); + + it('signs a write queued on mainnet with the mainnet agent after the provider switched to testnet', async () => { + useSwitchableNetwork(); + const getAgentSigner = jest.fn(); + const built = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + }); + const testnetAgent = { + address: OTHER_AGENT_ADDRESS, + signTypedData: jest.fn().mockResolvedValue(OTHER_AGENT_SIGNATURE), + }; + getAgentSigner.mockImplementation(async (account: PerpsAgentAccount) => + account.isTestnet ? testnetAgent : built.agentSigner, + ); + await built.accountSignerProvider.prepareTradingWallet(); + const queued = createDeferred(); + const ahead = createDeferred(); + built.exchangeClient.order.mockImplementationOnce(async () => { + // The mainnet client's wallet. The SDK queues the write behind another + // one, so it signs later. + const wallet = built.sdkWallet(); + queued.resolve(); + await ahead.promise; + return await built.signAndSend( + L1_PAYLOAD, + { status: 'ok', response: { data: { statuses: ['success'] } } }, + 'thrown', + wallet, + ); + }); + + const ordering = built.accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await queued.promise; + await built.accountSignerProvider.toggleTestnet(); + await built.sdkWallet().signTypedData(L1_PAYLOAD); + built.agentSigner.signTypedData.mockClear(); + testnetAgent.signTypedData.mockClear(); + ahead.resolve(); + await ordering; + + expect(built.agentSigner.signTypedData.mock.calls).toStrictEqual([ + [L1_PAYLOAD], + ]); + expect(testnetAgent.signTypedData).not.toHaveBeenCalled(); + }); + it('does not check a request whose agent signature finished after the provider reconnected on the same network', async () => { useSwitchableNetwork(); const { diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts index 10fa7f5497a..12019daaf81 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts @@ -407,12 +407,17 @@ describe('HyperLiquidProvider with accountSigner: agents', () => { getAgentSigner.mockImplementation(async (account: PerpsAgentAccount) => account.isTestnet ? null : agentSigner, ); + let isTestnet = false; + mockClientService.isTestnetMode.mockImplementation(() => isTestnet); + mockClientService.setTestnetMode.mockImplementation((value: boolean) => { + isTestnet = value; + }); await accountSignerProvider.getMarketDataWithPrices(); - const wallet = sdkWallet(); - await wallet.signTypedData(L1_PAYLOAD); + await sdkWallet().signTypedData(L1_PAYLOAD); - mockClientService.isTestnetMode.mockReturnValue(true); - await wallet.signTypedData(L1_PAYLOAD); + // Switching networks replaces the wallet the SDK signs with. + await accountSignerProvider.toggleTestnet(); + await sdkWallet().signTypedData(L1_PAYLOAD); expect(getAgentSigner.mock.calls).toStrictEqual([ [MAINNET_ACCOUNT], diff --git a/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts index 9408717f789..d659bb8447a 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidClientService.test.ts @@ -257,7 +257,7 @@ describe('HyperLiquidClientService', () => { return ExchangeClient.mock.calls.at(-1)[0].transport; } - it('reports each exchange request with the answer before returning it', async () => { + it('reports each exchange request with the answer and the wallet before returning it', async () => { transport.request.mockResolvedValue(answer); const payload = { action: { type: 'order' } }; @@ -268,7 +268,9 @@ describe('HyperLiquidClientService', () => { expect(transport.request.mock.calls).toStrictEqual([ ['exchange', payload, undefined], ]); - expect(onExchangeRequest.mock.calls).toStrictEqual([[payload, answer]]); + expect(onExchangeRequest.mock.calls).toStrictEqual([ + [payload, answer, mockWallet], + ]); }); it('reports an exchange request that failed without an answer', async () => { @@ -280,7 +282,7 @@ describe('HyperLiquidClientService', () => { exchangeTransport().request('exchange', payload), ).rejects.toBe(failure); expect(onExchangeRequest.mock.calls).toStrictEqual([ - [payload, undefined], + [payload, undefined, mockWallet], ]); }); diff --git a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts index d1bb8494671..fd2f61e7fca 100644 --- a/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts +++ b/packages/perps-controller/tests/src/services/HyperLiquidWalletService.account-signer.test.ts @@ -261,6 +261,34 @@ describe('HyperLiquidWalletService wallet adapter with an agent', () => { expect(adapter.address).toBe(mainAddress); }); + it('resolves agents through the resolver given to the adapter instead of the service one', async () => { + const { resolveAgent } = buildAdapter(); + const { messenger } = createKeyringlessMessenger(); + const adapterAgentSign = jest.fn().mockResolvedValue(AGENT_SIGNATURE); + const adapterResolveAgent = jest.fn().mockResolvedValue({ + address: AGENT_ADDRESS, + signTypedData: adapterAgentSign, + }); + const adapter = new HyperLiquidWalletService( + { + ...createMockInfrastructure(), + accountSigner: { + signTypedData: jest.fn().mockResolvedValue(MAIN_SIGNATURE), + signPersonalMessage: jest.fn(), + }, + }, + messenger, + { isTestnet: true, resolveAgent }, + ).createWalletAdapter(adapterResolveAgent); + + const signature = await adapter.signTypedData(L1_PAYLOAD); + + expect(signature).toBe(AGENT_SIGNATURE); + expect(adapterResolveAgent.mock.calls).toStrictEqual([[mainAddress]]); + expect(adapterAgentSign.mock.calls).toStrictEqual([[L1_PAYLOAD]]); + expect(resolveAgent).not.toHaveBeenCalled(); + }); + it('signs L1 actions with the agent resolved for the selected account', async () => { const { adapter, resolveAgent, agentSign, mainSign, call } = buildAdapter(); From 0d76c4c0d10083b07f19fbaf5efae70999d97fe1 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Thu, 1 Oct 2026 16:34:21 +0800 Subject: [PATCH 6/6] fix(perps-controller): attribute unknown-wallet rejections per request and bind every wallet - A "User or API Wallet ... does not exist" answer is attributed to the agent signature linked to the request it answers, on that wallet's network, so an agent used on both networks is dropped only where the venue rejected it. The agent's last account is used only when no signature is linked. - The subscription service creates its wallets through the provider, so they are bound and resolve agents like the provider's. - HyperLiquidProvider.getExchangeClient initializes the provider's clients before handing out the exchange client. --- packages/perps-controller/CHANGELOG.md | 2 + .../src/providers/HyperLiquidProvider.ts | 74 +++++++++++++------ .../HyperLiquidSubscriptionService.ts | 13 +++- ...rLiquidProvider.agent-must-deposit.test.ts | 72 ++++++++++++++++++ .../HyperLiquidProvider.agent-signer.test.ts | 47 ++++++++++++ 5 files changed, 185 insertions(+), 23 deletions(-) diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 87a1f483787..b347e7fb96d 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** `OrderFill.pnl` is optional when the venue omits realized PnL. Consumers must preserve missing amounts as unknown when aggregating or displaying fills; only a reported `'0'` is zero ([#10605](https://github.com/MetaMask/core/pull/10605)) +- `HyperLiquidProvider.getExchangeClient` initializes the provider's SDK clients before returning the exchange client, so it signs L1 actions with the provider's agent ([#10643](https://github.com/MetaMask/core/pull/10643)) ### Fixed @@ -22,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The provider then checks `extraAgents` for the agent that signed the answered request. When it is missing or past its `validUntil`, the write fails with `KEYRING_LOCKED`, the agent is dropped and `onAgentRejected` is called. Requests signed by the main account, or by a wallet of an earlier network or session, are not checked - When the agent is still listed (an account with no funds), or the list cannot be read, the venue error is returned as before - Sign a HyperLiquid L1 action the SDK queued before a network switch with the agent of the network it was queued on, not the new network's ([#10643](https://github.com/MetaMask/core/pull/10643)) +- Attribute a HyperLiquid "User or API Wallet ... does not exist" rejection to the network of the request it answers, so an agent used on both networks is dropped, and `onAgentRejected` called, only for that network ([#10643](https://github.com/MetaMask/core/pull/10643)) - Stop asking again for a HyperLiquid builder fee approval the venue refused for a reason signing again cannot fix, such as "Builder has insufficient balance to be approved" ([#10643](https://github.com/MetaMask/core/pull/10643)) - Until the provider disconnects, the approval is not requested again: orders are sent as after any failed approval, TP/SL updates fail with `TPSL_UPDATE_FAILED`, and `prepareTradingWallet` returns the venue error diff --git a/packages/perps-controller/src/providers/HyperLiquidProvider.ts b/packages/perps-controller/src/providers/HyperLiquidProvider.ts index f7f92adb570..39b3fc63f86 100644 --- a/packages/perps-controller/src/providers/HyperLiquidProvider.ts +++ b/packages/perps-controller/src/providers/HyperLiquidProvider.ts @@ -1786,7 +1786,16 @@ export class HyperLiquidProvider implements PerpsProvider { ); this.#subscriptionService = new HyperLiquidSubscriptionService( this.#clientService, - this.#walletService, + { + // The subscription service may initialize the SDK clients too, so + // its wallets are bound like the provider's. + createWalletAdapter: (): HyperLiquidWalletParams => + this.#createWalletAdapter(), + getUserAddressWithDefault: async ( + accountId?: CaipAccountId, + ): Promise => + await this.#walletService.getUserAddressWithDefault(accountId), + }, this.#deps, this.#hip3Enabled, [], // enabledDexs - will be populated after DEX discovery in buildAssetMapping @@ -2225,19 +2234,7 @@ export class HyperLiquidProvider implements PerpsProvider { throw new Error(PERPS_ERROR_CODES.PROVIDER_LIFECYCLE_STALE); } - // The adapter keeps the network and session it is created for: its - // agents are resolved, and its signatures attributed, for them, even - // when a queued write signs after a network switch or a disconnect. - const binding: WalletBinding = { - isTestnet: this.#clientService.isTestnetMode(), - generation: this.#lifecycleGeneration, - unansweredSignatures: new Map(), - }; - const wallet = this.#walletService.createWalletAdapter( - async (mainAddress: Hex): Promise => - await this.#resolveTrackedAgentSigner(mainAddress, binding), - ); - this.#walletBindings.set(wallet, binding); + const wallet = this.#createWalletAdapter(); await this.#clientService.initialize(wallet); if (this.#disconnectOperationsInFlight > 0) { throw new Error(PERPS_ERROR_CODES.PROVIDER_LIFECYCLE_STALE); @@ -2288,6 +2285,28 @@ export class HyperLiquidProvider implements PerpsProvider { } } + /** + * Create a wallet adapter for the SDK clients, bound to the network and + * session it is created in: its agents are resolved, and its signatures + * attributed, for them, even when a queued write signs after a network + * switch or a disconnect. + * + * @returns The wallet adapter. + */ + #createWalletAdapter(): HyperLiquidWalletParams { + const binding: WalletBinding = { + isTestnet: this.#clientService.isTestnetMode(), + generation: this.#lifecycleGeneration, + unansweredSignatures: new Map(), + }; + const wallet = this.#walletService.createWalletAdapter( + async (mainAddress: Hex): Promise => + await this.#resolveTrackedAgentSigner(mainAddress, binding), + ); + this.#walletBindings.set(wallet, binding); + return wallet; + } + /** * Resolve the agent that signs L1 actions for a main account on a network * through `getAgentSigner`. A non-null answer is kept; @@ -2410,17 +2429,28 @@ export class HyperLiquidProvider implements PerpsProvider { error, 'HyperLiquidProvider.findRejectedAgent', ); - if (isHyperLiquidUserNotFoundError(error)) { - const reportedAddress = UNKNOWN_WALLET_ADDRESS_PATTERN.exec(message)?.[1]; - return reportedAddress === undefined - ? undefined - : this.#agentSignedFor.get(reportedAddress.toLowerCase()); - } - const mainAddress = MUST_DEPOSIT_USER_PATTERN.exec(message)?.[1]; const answer = getVenueAnswer(error); const signature = isStatusObject(answer) ? this.#agentSignatureByAnswer.get(answer) : undefined; + if (isHyperLiquidUserNotFoundError(error)) { + const reportedAddress = + UNKNOWN_WALLET_ADDRESS_PATTERN.exec(message)?.[1]?.toLowerCase(); + if (reportedAddress === undefined) { + return undefined; + } + // The agent that signed the answered request, on its wallet's network. + // When no signature is linked to the answer (it did not come through + // an exchange client of this provider), the account the agent last + // signed an L1 action for. + if (signature) { + return signature.agentAddress.toLowerCase() === reportedAddress + ? signature + : undefined; + } + return this.#agentSignedFor.get(reportedAddress); + } + const mainAddress = MUST_DEPOSIT_USER_PATTERN.exec(message)?.[1]; return mainAddress !== undefined && signature?.account.mainAddress.toLowerCase() === mainAddress.toLowerCase() ? await this.#findRevokedAgent(signature) @@ -15888,6 +15918,8 @@ export class HyperLiquidProvider implements PerpsProvider { * (hl-provision-fixture) that chain `.then` on the result. */ public async getExchangeClient(): Promise { + // Initialized here so the client signs through this provider's wallet. + await this.#ensureClientsInitialized(); return this.#clientService.getExchangeClient(); } diff --git a/packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts b/packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts index 6d7103f5735..65f1a0957c2 100644 --- a/packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts +++ b/packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts @@ -78,6 +78,15 @@ import { import type { HyperLiquidClientService } from './HyperLiquidClientService.js'; import type { HyperLiquidWalletService } from './HyperLiquidWalletService.js'; +/** + * What the service needs from the wallet: the account, and the wallet + * adapters it (re)initializes the SDK clients with. + */ +type SubscriptionWallet = Pick< + HyperLiquidWalletService, + 'createWalletAdapter' | 'getUserAddressWithDefault' +>; + /** * Cap on *terminal* TWAP schedules retained per account from the venue's * history stream. Without it a long-lived session accumulates every schedule @@ -110,7 +119,7 @@ export class HyperLiquidSubscriptionService { // Service dependencies readonly #clientService: HyperLiquidClientService; - readonly #walletService: HyperLiquidWalletService; + readonly #walletService: SubscriptionWallet; // HIP-3 feature flag support #hip3Enabled: boolean; @@ -458,7 +467,7 @@ export class HyperLiquidSubscriptionService { constructor( clientService: HyperLiquidClientService, - walletService: HyperLiquidWalletService, + walletService: SubscriptionWallet, platformDependencies: PerpsPlatformDependencies, hip3Enabled?: boolean, enabledDexs?: string[], diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts index f5187918377..f1e2f417a5e 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-must-deposit.test.ts @@ -14,6 +14,7 @@ import { OTHER_MAIN_ADDRESS, TESTNET_ACCOUNT, mustDepositError, + unknownWalletError, } from '../../helpers/agentFixtures.js'; import { BTC_MARKET_ORDER, @@ -855,6 +856,77 @@ describe('HyperLiquidProvider with accountSigner: an agent the venue answers wit expect(onAgentRejected).not.toHaveBeenCalled(); }); + it('drops the agent of the network whose request the venue rejected as an unknown wallet, when a queued write of the other network signed after it', async () => { + useSwitchableNetwork(); + // The host reuses its agent on both networks. + const { + accountSignerProvider, + exchangeClient, + getAgentSigner, + onAgentRejected, + reportAnswer, + sdkWallet, + signAndSend, + } = await createPreparedAgentProvider(); + const mainnetQueued = createDeferred(); + const mainnetAhead = createDeferred(); + exchangeClient.order.mockImplementationOnce(async () => { + // The mainnet client's wallet; the SDK queues the write. + const wallet = sdkWallet(); + mainnetQueued.resolve(); + await mainnetAhead.promise; + return await signAndSend( + L1_PAYLOAD, + { status: 'ok', response: { data: { statuses: ['success'] } } }, + 'thrown', + wallet, + ); + }); + const mainnetOrdering = accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await mainnetQueued.promise; + await accountSignerProvider.toggleTestnet(); + const testnetSigned = createDeferred(); + const testnetVenue = createDeferred(); + const answer = { + status: 'err', + response: unknownWalletError(AGENT_ADDRESS).message, + }; + exchangeClient.order.mockImplementationOnce(async () => { + const wallet = sdkWallet(); + const signature = await wallet.signTypedData(L1_PAYLOAD); + testnetSigned.resolve(); + await testnetVenue.promise; + reportAnswer(signature, answer, wallet); + throw apiRequestError(answer); + }); + + // The testnet request signs, the queued mainnet write signs, then the + // venue rejects the testnet request. + const testnetOrdering = accountSignerProvider.placeOrder(BTC_MARKET_ORDER); + await testnetSigned.promise; + mainnetAhead.resolve(); + await mainnetOrdering; + testnetVenue.resolve(); + const testnetOrder = await testnetOrdering; + // Testnet asks for an agent again; mainnet keeps its agent. + await sdkWallet().signTypedData(L1_PAYLOAD); + await accountSignerProvider.toggleTestnet(); + await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(testnetOrder).toStrictEqual({ + success: false, + error: PERPS_ERROR_CODES.KEYRING_LOCKED, + }); + expect(onAgentRejected.mock.calls).toStrictEqual([ + [TESTNET_ACCOUNT, AGENT_ADDRESS], + ]); + expect(getAgentSigner.mock.calls).toStrictEqual([ + [MAINNET_ACCOUNT], + [TESTNET_ACCOUNT], + [TESTNET_ACCOUNT], + ]); + }); + describe('without an agent', () => { it('fails the order with the venue error without checking the agents', async () => { const getAgentSigner = jest.fn().mockResolvedValue(null); diff --git a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts index 12019daaf81..5bc2821bf83 100644 --- a/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts +++ b/packages/perps-controller/tests/src/providers/HyperLiquidProvider.agent-signer.test.ts @@ -8,9 +8,11 @@ import { AgentSignerUnavailableError, } from '../../../src/services/agentSigner.js'; import type { HyperLiquidClientService } from '../../../src/services/HyperLiquidClientService.js'; +import { HyperLiquidSubscriptionService } from '../../../src/services/HyperLiquidSubscriptionService.js'; import { PerpsAnalyticsEvent } from '../../../src/types/index.js'; import type { PerpsAgentAccount } from '../../../src/types/index.js'; import { + AGENT_SIGNATURE, APPROVE_BUILDER_FEE_PAYLOAD, L1_PAYLOAD, MAIN_ADDRESS, @@ -54,6 +56,51 @@ describe('HyperLiquidProvider with accountSigner: agents', () => { }); describe('with an agent', () => { + it('signs the L1 actions of a wallet the subscription service creates with the agent', async () => { + const getAgentSigner = jest.fn(); + const { accountSigner, agentSigner } = createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + }); + getAgentSigner.mockResolvedValue(agentSigner); + // The wallet source the provider gave the subscription service, which + // can initialize the SDK clients before the provider does. + const [, subscriptionWallet] = jest + .mocked(HyperLiquidSubscriptionService) + .mock.calls.at(-1) as ConstructorParameters< + typeof HyperLiquidSubscriptionService + >; + + const signature = await subscriptionWallet + .createWalletAdapter() + .signTypedData(L1_PAYLOAD); + + expect(signature).toBe(AGENT_SIGNATURE); + expect(agentSigner.signTypedData.mock.calls).toStrictEqual([ + [L1_PAYLOAD], + ]); + expect(accountSigner.signTypedData).not.toHaveBeenCalled(); + expect(getAgentSigner.mock.calls).toStrictEqual([[MAINNET_ACCOUNT]]); + }); + + it('initializes the clients with the provider wallet before handing out the exchange client', async () => { + const getAgentSigner = jest.fn(); + const { accountSignerProvider, agentSigner, sdkWallet } = + createAccountSignerProvider({ + abstraction: 'unifiedAccount', + getAgentSigner, + }); + getAgentSigner.mockResolvedValue(agentSigner); + + await accountSignerProvider.getExchangeClient(); + const signature = await sdkWallet().signTypedData(L1_PAYLOAD); + + expect(signature).toBe(AGENT_SIGNATURE); + expect(agentSigner.signTypedData.mock.calls).toStrictEqual([ + [L1_PAYLOAD], + ]); + }); + it('resolves the agent at the first L1 signature and signs with it', async () => { const getAgentSigner = jest.fn(); const { accountSignerProvider, accountSigner, agentSigner, sdkWallet } =