Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/perps-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Covers orders, edits, single and batch cancels (TWAP, scale and chase cancels included), position closes, TP/SL updates and clears, margin updates, withdrawals and transfers between DEXs, including the HIP-3 transfers around an order
- HyperLiquid `cancelOrders` reports each order of a batch with its own result when an entry fails: orders the venue cancelled are no longer reported as failed with the batch's error ([#10559](https://github.com/MetaMask/core/pull/10559))
- HyperLiquid orders that set leverage without `marginMode` now keep the open position's margin mode instead of switching to isolated, so flipping a Cross position no longer fails with "Cannot switch leverage type with open position" ([#10588](https://github.com/MetaMask/core/pull/10588))
- Wait up to `PERPS_CONSTANTS.ConnectionTimeoutMs` for the client's follow-up `init()` when a controller action (such as `placeOrder`) was waiting on a `disconnect()`, so an order submitted during a disconnect-then-init reconnect is placed instead of failing with `CLIENT_NOT_INITIALIZED` ([#10589](https://github.com/MetaMask/core/pull/10589))
- If that reconnect switched the selected account, the network or the active provider, the action fails with `PROVIDER_LIFECYCLE_STALE` instead of running under the new context.

## [18.0.1]

Expand Down
69 changes: 69 additions & 0 deletions packages/perps-controller/src/PerpsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,9 @@ export class PerpsController extends BaseController<

#initializationPromise: Promise<void> | null = null;

// Actions that saw a disconnect wait here for the client's follow-up init().
readonly #initializationStartWaiters = new Set<() => void>();

#isReinitializing = false;

#reinitializationOperationPromise: Promise<void> | null = null;
Expand Down Expand Up @@ -2192,9 +2195,33 @@ export class PerpsController extends BaseController<
}

this.#initializationPromise = this.#performInitialization();
this.#initializationStartWaiters.forEach((notifyStarted) =>
notifyStarted(),
);
return this.#initializationPromise;
}

/**
* Resolve once a new initialization starts, or after the timeout.
*
* @param timeoutMs - Longest time to wait for init() to be called.
* @returns A promise that resolves when init starts or the timeout elapses.
*/
async #waitForInitializationStart(timeoutMs: number): Promise<void> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: wdyt about a default for timeoutMs here to inform callers in the future?

let notifyStarted = (): void => undefined;
const started = new Promise<void>((resolve) => {
notifyStarted = resolve;
});
this.#initializationStartWaiters.add(notifyStarted);
const timeout = setTimeout(notifyStarted, timeoutMs);
try {
await started;
} finally {
clearTimeout(timeout);
this.#initializationStartWaiters.delete(notifyStarted);
}
}

/**
* Track a network or provider reinitialization so disconnect can serialize
* behind the whole operation, including work before and after init().
Expand Down Expand Up @@ -2717,9 +2744,16 @@ export class PerpsController extends BaseController<
* @returns The active provider once initialization completes.
*/
async #getActiveProviderWhenReady(): Promise<PerpsProvider> {
// The context the action was issued under. A client reconnect
// (disconnect then init) that switches account, network or provider must
// not carry the action into the new context.
const issuedContext = this.#getActionContext();
let awaitedDisconnect = false;
let awaitedInitializationStart = false;
while (true) {
const pendingDisconnect = this.#disconnectOperationPromise;
if (pendingDisconnect) {
awaitedDisconnect = true;
await pendingDisconnect;
continue;
}
Expand All @@ -2739,10 +2773,45 @@ export class PerpsController extends BaseController<
continue;
}

// Clients reconnect with disconnect() followed by init(), and the
// disconnect settles before init() is called. Give that init a bounded
// window to start rather than failing an action the reconnect will
// serve. Nothing here starts a connection the client did not ask for.
if (
awaitedDisconnect &&
!awaitedInitializationStart &&
!this.isInitialized &&
!pendingInitialization
) {
awaitedInitializationStart = true;
await this.#waitForInitializationStart(
PERPS_CONSTANTS.ConnectionTimeoutMs,
);
continue;
}

if (awaitedDisconnect && this.#getActionContext() !== issuedContext) {
throw new Error(PERPS_ERROR_CODES.PROVIDER_LIFECYCLE_STALE);
}

return this.getActiveProvider();
}
}

/**
* Identify the account, network and provider an action runs under.
*
* @returns A key that changes when any of them changes.
*/
#getActionContext(): string {
const address = getSelectedEvmAccountFromMessenger(this.messenger)?.address;
return [
address?.toLowerCase() ?? '',
this.state.isTestnet ? 'testnet' : 'mainnet',
this.state.activeProvider,
].join('|');
}

/**
* Get the currently active provider, returning null if not available
* Use this method when the caller can gracefully handle a missing provider
Expand Down
189 changes: 174 additions & 15 deletions packages/perps-controller/tests/src/PerpsController.lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import {

jest.mock('@nktkas/hyperliquid', () => ({}));

import { PERPS_DISK_CACHE_MARKETS } from '../../src/constants/perpsConfig.js';
import {
PERPS_CONSTANTS,
PERPS_DISK_CACHE_MARKETS,
} from '../../src/constants/perpsConfig.js';
import {
PerpsController,
getDefaultPerpsControllerState,
Expand Down Expand Up @@ -1282,23 +1285,179 @@ describe('PerpsController', () => {
return { success: true };
});

const disconnectPromise = controller.disconnect();
await disconnectStarted.promise;
const orderPromise = controller.placeOrder({
symbol: 'BTC',
isBuy: true,
size: '0.1',
orderType: 'market',
jest.useFakeTimers();
try {
const disconnectPromise = controller.disconnect();
await disconnectStarted.promise;
const orderPromise = controller.placeOrder({
symbol: 'BTC',
isBuy: true,
size: '0.1',
orderType: 'market',
});
const orderRejection = expect(orderPromise).rejects.toThrow(
PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED,
);
await Promise.resolve();

expect(mockTradingServiceInstance.placeOrder).not.toHaveBeenCalled();
pendingDisconnect.resolve();
await disconnectPromise;
// No init follows this disconnect: the order fails once the bounded
// wait for a reconnect runs out.
await jest.advanceTimersByTimeAsync(
PERPS_CONSTANTS.ConnectionTimeoutMs,
);
await orderRejection;
expect(mockTradingServiceInstance.placeOrder).not.toHaveBeenCalled();
} finally {
jest.useRealTimers();
}
});

describe('TAT-4041: order submitted during a client reconnect', () => {
it('places an order submitted while a disconnect is followed by init', async () => {
await controller.init();
const disconnectStarted = createDeferred<void>();
const pendingDisconnect = createDeferred<void>();
mockProvider.disconnect.mockImplementationOnce(async () => {
disconnectStarted.resolve();
await pendingDisconnect.promise;
return { success: true };
});
jest
.spyOn(mockTradingServiceInstance, 'placeOrder')
.mockResolvedValue({ success: true, orderId: '123' });

// Clients reconnect with disconnect() then init(), with async work
// (for example a cleanup delay) between the two calls.
const reconnect = (async (): Promise<void> => {
await controller.disconnect();
await new Promise((resolve) => setTimeout(resolve, 50));
await controller.init();
})();
await disconnectStarted.promise;
const orderPromise = controller.placeOrder({
symbol: 'BTC',
isBuy: true,
size: '0.1',
orderType: 'market',
});
pendingDisconnect.resolve();
await reconnect;

await expect(orderPromise).resolves.toStrictEqual(
expect.objectContaining({ success: true, orderId: '123' }),
);
expect(mockTradingServiceInstance.placeOrder).toHaveBeenCalledTimes(1);
});
await Promise.resolve();

expect(mockTradingServiceInstance.placeOrder).not.toHaveBeenCalled();
pendingDisconnect.resolve();
await disconnectPromise;
await expect(orderPromise).rejects.toThrow(
PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED,
it('bounds the wait for init after a disconnect', async () => {
await controller.init();
jest.useFakeTimers();
try {
const disconnectPromise = controller.disconnect();
const orderPromise = controller.placeOrder({
symbol: 'BTC',
isBuy: true,
size: '0.1',
orderType: 'market',
});
let settled = false;
orderPromise
.catch(() => undefined)
.finally(() => {
settled = true;
});
await disconnectPromise;

await jest.advanceTimersByTimeAsync(
PERPS_CONSTANTS.ConnectionTimeoutMs - 1,
);
expect(settled).toBe(false);

await jest.advanceTimersByTimeAsync(1);
await expect(orderPromise).rejects.toThrow(
PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED,
);
expect(mockTradingServiceInstance.placeOrder).not.toHaveBeenCalled();
} finally {
jest.useRealTimers();
}
});

it.each([
[
'account',
(): void => {
const call = mockMessenger.call as unknown as jest.Mock;
const baseCall = call.getMockImplementation();
call.mockImplementation((action: string, ...args: unknown[]) =>
action ===
'AccountTreeController:getAccountsFromSelectedAccountGroup'
? [
{
address: '0x9999999999999999999999999999999999999999',
type: 'eip155:eoa',
id: 'account-2',
options: {},
scopes: ['eip155:1'],
methods: [],
metadata: {
name: 'Other',
importTime: 0,
keyring: { type: 'HD Key Tree' },
},
},
]
: baseCall?.(action, ...args),
);
},
],
[
'network',
(): void => {
controller.testUpdate((state) => {
state.isTestnet = !state.isTestnet;
});
},
],
])(
'refuses an order parked across a reconnect that switched the %s',
async (_context, switchContext) => {
await controller.init();
const disconnectStarted = createDeferred<void>();
const pendingDisconnect = createDeferred<void>();
mockProvider.disconnect.mockImplementationOnce(async () => {
disconnectStarted.resolve();
await pendingDisconnect.promise;
return { success: true };
});
jest
.spyOn(mockTradingServiceInstance, 'placeOrder')
.mockResolvedValue({ success: true, orderId: '123' });

const reconnect = (async (): Promise<void> => {
await controller.disconnect();
switchContext();
await controller.init();
})();
await disconnectStarted.promise;
const orderPromise = controller.placeOrder({
symbol: 'BTC',
isBuy: true,
size: '0.1',
orderType: 'market',
});
pendingDisconnect.resolve();
await reconnect;

await expect(orderPromise).rejects.toThrow(
PERPS_ERROR_CODES.PROVIDER_LIFECYCLE_STALE,
);
expect(mockTradingServiceInstance.placeOrder).not.toHaveBeenCalled();
},
);
expect(mockTradingServiceInstance.placeOrder).not.toHaveBeenCalled();
});

it('keeps init queued when disconnect starts during reinitialization', async () => {
Expand Down
Loading