Skip to content

Commit 3fa1e22

Browse files
authored
fix(perps): TP/SL orders disappear after creating a market order (#28073)
## **Description** **Bug 1 (TAT-2636):** The WS-cache path in `updatePositionTPSL` was missing an `isPositionTpsl` filter, causing it to cancel all trigger reduce-only orders (including `normalTpsl` children of pending limit orders) instead of only position-bound TP/SL orders. The REST fallback already had the correct filter. This fix adds the matching `isPositionTpsl` check to the cache path. **Bug 2 (TAT-2835):** Fixing bug 1 exposed a display-side issue: the position TP/SL display logic (both REST `getPositions` and WS cached orders path in `extractTPSLFromOrders`) wasn't filtering by `isPositionTpsl`, so when normalTpsl and positionTpsl orders coexist, the position would show the limit order's TP/SL values instead of the position-bound ones. Added `isPositionTpsl` filter to both display paths. ## **Changelog** CHANGELOG entry: Fixed a bug where placing a market order with TP/SL would incorrectly cancel TP/SL orders belonging to existing pending limit orders, and fixed position TP/SL display showing limit order values instead of position-bound values. ## **Related issues** Fixes: [TAT-2636](https://consensyssoftware.atlassian.net/browse/TAT-2636) Fixes: [TAT-2835](https://consensyssoftware.atlassian.net/browse/TAT-2835) ## **Manual testing steps** ```gherkin Feature: TP/SL orders from limit orders preserved after market order with TP/SL Scenario: user places a market order with TP/SL when a limit order with TP/SL already exists Given testnet mode is active And user has placed a BTC limit order at $50,000 with TP/SL (normalTpsl grouping) And the normalTpsl child orders are visible in open orders When user places a BTC market order with TP/SL (new position, triggers updatePositionTPSL) Then positionTpsl orders are created for the new position And the normalTpsl TP/SL children of the limit order are NOT cancelled And the position displays the market order's TP/SL values, not the limit order's ``` ## **Screenshots/Recordings** _Evidence available in task artifacts — will be added by reviewer if needed._ ### **Before** <!-- Recipe fails at `verify-display-matches-position-tpsl`: displayedTp=62500 (limit order value), expectedTp=74285 --> ### **After** <!-- Recipe passes 12/12: displayedTp=74285 matches expectedTp=74285 (position-bound value) --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. ## **Recipe proof (before/after)** | | Key assertion | displayedTp | expectedTp | Result | |---|---|---|---|---| | **Without fix** | `verify-display-matches-position-tpsl` | **62,500** (limit order's TP) | 74,285 | **FAIL** | | **With fix** | `verify-display-matches-position-tpsl` | **74,285** (position's TP) | 74,285 | **PASS** | <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches perps TP/SL cancellation and position display logic; incorrect filtering could cancel the wrong orders or show wrong TP/SL values in trading UI. > > **Overview** > Fixes TP/SL handling to distinguish *position-bound* TP/SL orders from `normalTpsl` child orders. > > `updatePositionTPSL` now applies the same `isPositionTpsl` filter when using the WebSocket orders cache as it already did in the REST fallback, preventing cancellation of limit-order TP/SL children. Position TP/SL extraction/display is also updated (both `getPositions` and `HyperLiquidSubscriptionService.#extractTPSLFromOrders`) to ignore non-position-bound TP/SL when `UsePositionBoundTpsl` is enabled. > > Adds/updates tests to cover mixed cached-order scenarios and ensures `isPositionTpsl`/`triggerPrice` are propagated in subscription order adaptation. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 371108e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [TAT-2636]: https://consensyssoftware.atlassian.net/browse/TAT-2636?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent 7362ef0 commit 3fa1e22

4 files changed

Lines changed: 157 additions & 4 deletions

File tree

app/controllers/perps/providers/HyperLiquidProvider.test.ts

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { TradingReadinessCache } from '../services/TradingReadinessCache';
1717
import type {
1818
ClosePositionParams,
1919
DepositParams,
20+
Order,
2021
PerpsPlatformDependencies,
2122
LiveDataConfig,
2223
OrderParams,
@@ -3570,6 +3571,138 @@ describe('HyperLiquidProvider', () => {
35703571
],
35713572
});
35723573
});
3574+
3575+
it('cache path: only cancels positionTpsl orders, not normalTpsl children of limit orders', async () => {
3576+
provider = createTestProvider({
3577+
initialAssetMapping: [['BTC', 0]],
3578+
});
3579+
3580+
// Simulate WS cache with mixed orders: normalTpsl children (isPositionTpsl: false)
3581+
// from a pending limit order AND positionTpsl orders (isPositionTpsl: true)
3582+
const cachedOrders: Order[] = [
3583+
{
3584+
orderId: '500',
3585+
symbol: 'BTC',
3586+
side: 'buy',
3587+
orderType: 'limit',
3588+
size: '0.01',
3589+
originalSize: '0.01',
3590+
price: '50000',
3591+
filledSize: '0',
3592+
remainingSize: '0.01',
3593+
status: 'open',
3594+
timestamp: 1000,
3595+
isTrigger: false,
3596+
reduceOnly: false,
3597+
isPositionTpsl: false,
3598+
},
3599+
{
3600+
orderId: '501',
3601+
symbol: 'BTC',
3602+
side: 'sell',
3603+
orderType: 'limit',
3604+
size: '0.01',
3605+
originalSize: '0.01',
3606+
price: '60000',
3607+
filledSize: '0',
3608+
remainingSize: '0.01',
3609+
status: 'open',
3610+
timestamp: 1001,
3611+
isTrigger: true,
3612+
reduceOnly: true,
3613+
isPositionTpsl: false,
3614+
detailedOrderType: 'Take Profit Limit',
3615+
},
3616+
{
3617+
orderId: '502',
3618+
symbol: 'BTC',
3619+
side: 'sell',
3620+
orderType: 'market',
3621+
size: '0.01',
3622+
originalSize: '0.01',
3623+
price: '40000',
3624+
filledSize: '0',
3625+
remainingSize: '0.01',
3626+
status: 'open',
3627+
timestamp: 1002,
3628+
isTrigger: true,
3629+
reduceOnly: true,
3630+
isPositionTpsl: false,
3631+
detailedOrderType: 'Stop Market',
3632+
},
3633+
{
3634+
orderId: '503',
3635+
symbol: 'BTC',
3636+
side: 'sell',
3637+
orderType: 'limit',
3638+
size: '0',
3639+
originalSize: '0',
3640+
price: '58000',
3641+
filledSize: '0',
3642+
remainingSize: '0',
3643+
status: 'open',
3644+
timestamp: 1003,
3645+
isTrigger: true,
3646+
reduceOnly: true,
3647+
isPositionTpsl: true,
3648+
detailedOrderType: 'Take Profit Limit',
3649+
},
3650+
{
3651+
orderId: '504',
3652+
symbol: 'BTC',
3653+
side: 'sell',
3654+
orderType: 'market',
3655+
size: '0',
3656+
originalSize: '0',
3657+
price: '42000',
3658+
filledSize: '0',
3659+
remainingSize: '0',
3660+
status: 'open',
3661+
timestamp: 1004,
3662+
isTrigger: true,
3663+
reduceOnly: true,
3664+
isPositionTpsl: true,
3665+
detailedOrderType: 'Stop Market',
3666+
},
3667+
];
3668+
3669+
mockSubscriptionService.getOrdersCacheIfInitialized = jest
3670+
.fn()
3671+
.mockReturnValue(cachedOrders);
3672+
3673+
const mockCancel = jest.fn().mockResolvedValue({
3674+
status: 'ok',
3675+
response: { data: { statuses: ['success', 'success'] } },
3676+
});
3677+
mockClientService.getExchangeClient = jest.fn().mockReturnValue(
3678+
createMockExchangeClient({
3679+
cancel: mockCancel,
3680+
order: jest.fn().mockResolvedValue({
3681+
status: 'ok',
3682+
response: {
3683+
data: { statuses: [{ resting: { oid: '999' } }] },
3684+
},
3685+
}),
3686+
}),
3687+
);
3688+
3689+
mockWalletService.getUserAddressWithDefault.mockResolvedValue('0x123');
3690+
3691+
const result = await provider.updatePositionTPSL({
3692+
symbol: 'BTC',
3693+
takeProfitPrice: '60000',
3694+
stopLossPrice: '40000',
3695+
});
3696+
3697+
expect(result.success).toBe(true);
3698+
// Must cancel positionTpsl orders (503, 504) only — not normalTpsl children (501, 502)
3699+
expect(mockCancel).toHaveBeenCalledWith({
3700+
cancels: [
3701+
{ a: 0, o: 503 },
3702+
{ a: 0, o: 504 },
3703+
],
3704+
});
3705+
});
35733706
});
35743707

35753708
describe('getAccountState error handling', () => {

app/controllers/perps/providers/HyperLiquidProvider.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4350,21 +4350,24 @@ export class HyperLiquidProvider implements PerpsProvider {
43504350
{ cachedOrdersCount: cachedOrders.length },
43514351
);
43524352

4353-
// Filter using normalized Order type properties
4354-
// Note: Cached orders don't have isPositionTpsl, but we identify TP/SL orders by:
4353+
// Filter using normalized Order type properties, matching the REST fallback criteria:
4354+
// - symbol matches
43554355
// - isTrigger === true
43564356
// - reduceOnly === true
4357+
// - isPositionTpsl matches the configured mode (only cancel position-bound TP/SL,
4358+
// not normalTpsl children that belong to pending limit orders)
43574359
// - detailedOrderType contains 'Take Profit' or 'Stop'
43584360
const tpslOrders = cachedOrders.filter(
43594361
(order) =>
43604362
order.symbol === symbol &&
43614363
order.reduceOnly === true &&
43624364
order.isTrigger === true &&
4365+
order.isPositionTpsl ===
4366+
Boolean(TP_SL_CONFIG.UsePositionBoundTpsl) &&
43634367
order.detailedOrderType &&
43644368
(order.detailedOrderType.includes('Take Profit') ||
43654369
order.detailedOrderType.includes('Stop')),
43664370
);
4367-
43684371
cancelRequests = tpslOrders.map((order) => ({
43694372
a: assetId,
43704373
o: parseInt(order.orderId, 10),
@@ -4953,11 +4956,15 @@ export class HyperLiquidProvider implements PerpsProvider {
49534956

49544957
// Find TP/SL orders for this position
49554958
// First check direct trigger orders (raw SDK uses 'coin', adapted position uses 'symbol')
4959+
// Only match position-bound TP/SL orders when UsePositionBoundTpsl is enabled,
4960+
// to avoid picking up normalTpsl children from pending limit orders
49564961
const positionOrders = allOrders.filter(
49574962
(order) =>
49584963
order.coin === position.symbol &&
49594964
order.isTrigger &&
4960-
order.reduceOnly,
4965+
order.reduceOnly &&
4966+
order.isPositionTpsl ===
4967+
Boolean(TP_SL_CONFIG.UsePositionBoundTpsl),
49614968
);
49624969

49634970
// Also check for parent orders that might have TP/SL children

app/controllers/perps/services/HyperLiquidSubscriptionService.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ jest.mock('../utils/hyperLiquidAdapter', () => ({
5959
detailedOrderType: order.orderType || 'Limit',
6060
isTrigger: order.isTrigger ?? false,
6161
reduceOnly: order.reduceOnly ?? false,
62+
triggerPrice: order.triggerPx,
63+
...(typeof order.isPositionTpsl === 'boolean'
64+
? { isPositionTpsl: order.isPositionTpsl }
65+
: {}),
6266
})),
6367
adaptAccountStateFromSDK: jest.fn(() => ({
6468
availableBalance: '1000.00',

app/controllers/perps/services/HyperLiquidSubscriptionService.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,15 @@ export class HyperLiquidSubscriptionService {
732732
// This ensures consistency with raw SDK order processing which uses triggerPx
733733
const tpslPrice = order.triggerPrice ?? order.price;
734734
if (order.isTrigger && tpslPrice) {
735+
// When UsePositionBoundTpsl is enabled, only position-bound TP/SL orders
736+
// should be shown on positions — skip normalTpsl children of limit orders
737+
if (
738+
TP_SL_CONFIG.UsePositionBoundTpsl &&
739+
order.isPositionTpsl !== true
740+
) {
741+
return;
742+
}
743+
735744
const isTakeProfit = order.detailedOrderType?.includes('Take Profit');
736745
const isStop = order.detailedOrderType?.includes('Stop');
737746

0 commit comments

Comments
 (0)