fix(perps): prevent reduce-only close orders from increasing positions - #9719
fix(perps): prevent reduce-only close orders from increasing positions#9719abretonc7s wants to merge 15 commits into
Conversation
HyperLiquid rejected roughly half of failed position closes with "Order 0: Reduce only order would increase position". Three paths let a reduce-only close carry a size or side the live position could not absorb: - The position snapshot callers pass (to avoid a getPositions() REST call and its 429s) was never re-validated, so a concurrent TP/SL fill, liquidation, or repeated close left the order side/size stale. It is now checked against the WebSocket position cache, which costs no extra request; when the cache holds no position for the symbol the close fails fast instead of submitting a doomed order. - A caller-supplied close size is clamped to the live position size. - A 100% close now submits exactly the live position size. It previously forwarded usdAmount, which made placeOrder recompute the size as usdAmount / currentPrice and add a whole size increment whenever the rounded notional fell below the requested USD. calculateFinalPositionSize also takes a reduceOnly flag that rounds the size down onto the asset's size grid and skips that increment. Also stop retrying a reduce-only order that the exchange rejected for the $10 minimum with a 1.5% larger size: the retry can only fail again as a reduce-only rejection and hides the real cause. TAT-3252
- Derive isFullClose from the clamped close size, so usdAmount is withheld for any close that covers the whole position. Previously a partial market close forwarded usdAmount, which placeOrder treats as the source of truth, discarding the size clamp and resubmitting the pre-clamp size. - Throw "No position found" directly when the initialized WebSocket cache holds no position for the symbol, instead of falling back to REST. The fallback spent a request on an already-closed position and could hand back a position the cache had already dropped. - Scope the $10-minimum retry exclusion to full closes. Partial closes have room inside the position for the 1.5% bump and retry as before. - Throw ORDER_SIZE_POSITIVE when flooring a reduce-only size takes a positive size to zero, rather than submitting a size of "0". - Ignore a non-positive or non-numeric close size, closing in full instead. - Scale the size-grid snap tolerance with magnitude, so a grid-aligned size large enough for double-precision error to exceed a fixed epsilon no longer loses an increment. - Document on ClosePositionParams.position that the snapshot must be WebSocket-sourced, since the provider overrides it with its own cache. Adds tests/src/utils/orderCalculations.test.ts and regression tests for each of the above. TAT-3252
- Stop a supplied close size that is not a positive number from becoming a 100% close. Only an omitted or empty size means "close everything"; '0' or a non-numeric size now fails with ORDER_SIZE_POSITIVE. - Make the size clamp binding for partial closes. calculateFinalPositionSize ignores size in its usdAmount branch, so an adverse price move could still submit more than the clamped size; for reduce-only orders the USD-derived size is now capped at the supplied size. - Round each reduce-only size down onto the size grid in the batch closePositions path too, so both close paths carry the same guarantee. - Limit the "no live position" fail-fast to main-DEX symbols. HIP-3 positions are only cached while their subscriptions are active, so a missing HIP-3 entry keeps the caller's snapshot instead of blocking the close. - Add a test for the cache-not-initialized path, where the caller snapshot is used verbatim and no REST lookup happens. - Correct a stale comment in an orderCalculations test. TAT-3252
- Stop retrying any reduce-only order rejected for the $10 minimum. Since the close size is capped at what the caller asked to close, the retry's 1.5% bump was capped straight back and resubmitted an identical order; the minimum-value error now surfaces instead. The test that claimed the retry still worked only counted calls against a mock that resolved regardless of payload, so it is replaced by one asserting the failure. - Key the "no live position" fail-fast on actual cache coverage rather than symbol shape. A DEX whose subscription throws is dropped from the expected set while the aggregate is still published, so even a main-DEX position could be missing from a cache that looks complete. - Fetch live positions once when the cache does not cover the symbol's DEX, instead of falling back to the caller's snapshot. This extends the stale-snapshot protection to HIP-3 markets, at the cost of one REST request only on a coverage miss. - Add HyperLiquidSubscriptionService.isPositionsCacheCoveringDex(), backed by the per-DEX positions cache. Not exported from the package index. TAT-3252
- Keep the caller's position snapshot when the uncovered-DEX lookup returns nothing. getPositions() swallows request failures and returns [], so a rate-limited lookup blocked a position that was open and closable. - Reject a non-positive or non-numeric size in the reduce-only cap instead of capping to it. placeOrder with usdAmount and size '0' or '-1' previously submitted a zero or negative order; closePosition already rejected both. - Skip a batch close position smaller than one size increment rather than submitting a zero-size order for it. The skip happens before the HIP-3 transfer is recorded, and the positions that produced orders are tracked separately so response statuses stay mapped to the right symbols. - Document floorToSizeDecimals under Added: src/utils/index.ts re-exports orderCalculations wholesale and package.json exports ./utils/*, so it is public API, contrary to what the report claimed. - Use closeSizeNumber directly instead of round-tripping it through a string. - Add a service test for the positions cache reporting DEX coverage once a DEX has published, and provider/unit tests for each fix above. TAT-3252
- Report a batch-close position skipped for being smaller than one size increment as a failure instead of dropping it. It appeared nowhere in the result, so closeAll over [dust, ETH] returned successCount 1 / failureCount 0 — indistinguishable from "closed everything" with a position left open. The all-dust case now returns those failures rather than an empty result that matched "no positions matched" byte for byte. - Distinguish a failed uncovered-DEX lookup from one that proves the position is gone. getPositions() returns [] for both, so keeping the snapshot on any miss resubmitted the doomed order this ticket exists to prevent; a non-empty response now fails the close, an empty one keeps the snapshot. - Throw ORDER_SIZE_POSITIVE for a non-positive reduce-only size in the legacy-size branch too, so both branches of calculateFinalPositionSize agree about the same input. TAT-3252
- Prove the uncovered-DEX lookup against the target DEX rather than the response being non-empty. getPositions() fans out across DEXs and returns only the subset that answered, so a rate-limited HIP-3 request alongside a successful main-DEX one looked like proof the symbol was gone and blocked a closable position. A position on the same DEX is now what proves its query ran; anything else keeps the caller's snapshot. - Note in the changelog that a full close no longer performs the priceAtCalculation staleness check, and that closePositions appends skipped positions to results rather than preserving the requested order. TAT-3252
- Stop floorToSizeDecimals from ever returning more than its input. The magnitude-scaled tolerance became wider than half a grid increment at large scaled values, turning the helper into round-to-nearest: it returned 0.123 for 0.1229999995 and 3000000000.00001 for 3000000000.000006, contradicting the invariant it exists to enforce. The snap is now accepted only when it did not increase the value, so a genuinely below-grid size is truncated. - Preserve closePositions result ordering. Skipped dust positions were appended after the submitted ones, reordering a public result array that consumers may correlate to positions by index; results now follow the requested order. - Correct the ClosePositionParams.position contract, which promised that supplying a snapshot skips the REST fetch. It does not when the WebSocket cache does not cover the symbol's DEX. Adds boundary and property coverage asserting result <= size across grid points, dust, and magnitudes where the old tolerance misbehaved. TAT-3252
- floorToSizeDecimals could still return more than its input. For a size less than half an ulp below a grid point, size * multiplier evaluates to exactly that grid integer, so the guard's Math.floor(scaled) fallback returned the same too-large value it had rejected: 0.8999999999999999 at szDecimals 1 gave 0.9. The fallback now steps down in grid units until the result no longer exceeds the input; a sweep of grid points 1..3000 across szDecimals 0-6 at 1-3 ulp below each point goes from 285 violations to zero. - Document the legacy-branch behaviour change the changelog understated: a reduce-only close between half an increment and one full increment previously succeeded by closing one whole increment, because formatHyperLiquidSize rounds half-up, and now fails with ORDER_SIZE_POSITIVE. Reachable from any partial close that omits usdAmount. Adds the one-ulp-below-grid coverage the earlier boundary and property tests never sampled. TAT-3252
…AT-3252) - floorToSizeDecimals could never return. Past 2^53 `units -= 1` is a no-op, so the round-9 step-down loop could not converge: (998999999.9999998, 8), (902999999999.9995, 4), (8390000000000000, 6) and (1e18, 7) all hung. A double cannot represent consecutive integers there, so the grid is finer than the spacing between representable values and there is nothing to shave — those sizes are now returned unchanged, before the loop. - The documented invariant was false for negative sizes. The loop guard was `units > 0`, so it never ran for them, while the tolerance snap rounds a negative value towards zero, i.e. upward: (-1.0000000001, 0) returned -1 and (-0.1220000001, 3) returned -0.122, both greater than their input. Stepping on `units !== 0` fixes it; a 36,000-case negative sweep goes to zero violations with positive results unchanged. - Scope the changelog's reachability note to the Mobile limit path, which is verifiable in this checkout; the Extension half was not. Adds termination and invariant coverage at both domain edges the earlier sweeps stopped short of: scaled sizes past 2^53, and negatives. TAT-3252
…T-3252) floorToSizeDecimals still returned more than its input for a negative size below the tolerance. Such a size snaps to -0, and the round-10 guard `units !== 0` skips it because -0 === 0, so the step-down never ran: (-1e-9, 0), (-5e-7, 0) and (-1e-12, 5) all returned -0, which is greater than the input. Only the public helper is affected — every reduce-only entry point rejects a non-positive size before flooring — but the exported contract and the changelog both state the invariant unconditionally. Dropped the loop's `units` guard instead of widening it: the value comparison alone terminates in both directions (for a non-negative size it stops at or before zero, for a negative one once the value is no longer above the input), and the 2^53 bail-out keeps it bounded. Verified equivalent to the wider condition across 7462 mixed-sign cases with zero violations and no non-termination, and it avoids an unmodified-loop-condition operand. Adds sub-increment negative coverage: fixed cases including -0 and a positive sub-tolerance size, plus a 1920-case sweep from 1e-14 to 1e-3 spanning the sub-tolerance, sub-increment and above-increment regions. Both new tests fail against the previous condition. TAT-3252
- Restore price-staleness validation for full closes. Withholding usdAmount so a full close submits the exact live size also silenced the priceAtCalculation check, which lived inside calculateFinalPositionSize's usdAmount branch, leaving priceAtCalculation and maxSlippageBps inert: a consumer could execute a full market close after the price moved past its tolerance. The check now runs whenever priceAtCalculation is supplied, before the sizing branches, so exact-size closes keep the guard. - Distinguish a successful empty target-DEX response from a failed one. getPositions() fans out across DEXs, flattens the subset that answered and turns any error into [], so "this DEX answered and holds nothing" was indistinguishable from "this DEX failed" — and the close reused the stale snapshot in the first case, submitting exactly the order this ticket prevents. closePosition now queries the symbol's own DEX through a new private helper that reports whether it answered: answered without the symbol fails the close, a failed query keeps the snapshot. Adds four provider regression tests: a full close rejected beyond maxSlippageBps without submitting, an in-tolerance full close submitting the exact live size, a target DEX answering with zero positions, and a failing target-DEX query retaining the snapshot. Each fails against the previous implementation. Corrects the two changelog bullets those changes invalidate. TAT-3252
…T-3252)
- closePositions credited a HIP-3 freed-margin transfer to the wrong order.
hip3Transfers was compacted (pushed only for HIP-3 positions) but read with
the response-status index, so for [BTC main, xyz:STOCK HIP-3] a successful
BTC status at index 0 read the xyz transfer and moved that margin even when
the HIP-3 close at index 1 failed — a balance-moving side effect. One
optional transfer slot is now pushed per submitted order, keeping the two
index-aligned. Predates the branch but sits in the block this diff modified.
- Correct the exported ClosePositionParams.position contract and the matching
changelog bullet: the uncovered-DEX path issues a targeted clearinghouseState
request for that DEX, not getPositions({ skipCache: true }), and a successful
response is authoritative — the snapshot is retained only when the request
fails, not whenever the lookup "returns nothing".
- Update the close-price comment, which still claimed full closes need no
price validation after the staleness guard was restored for them.
- Mark both new changelog entries "PR link pending" rather than inventing a
pull-request number; the link must be appended when the PR is opened.
Adds a mixed main-DEX/HIP-3 batch regression test asserting no transfer is
initiated when the HIP-3 close fails and the main-DEX close succeeds. It fails
against the compacted-index read.
TAT-3252
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e1cdcd. Configure here.
Automated fix-bug run — TAT-3252
Worker reportExplanationHyperLiquid rejects a reduce-only order with
Changes
References
Checklist
Behaviour changes for consumersNo breaking API surface:
Consumption is a Testing
|
closePosition read the position from the cross-DEX aggregate (getCachedPositions) while deciding coverage from the per-DEX map (isPositionsCacheCoveringDex). Those are different stores and they diverge after a WebSocket reconnect: restoreSubscriptions clears the subscription maps and re-running setup resets #initializedDexs without clearing either position cache, and #aggregateAndNotifySubscribers only rebuilds the aggregate once every expected DEX has published again. So the aggregate can sit frozen at pre-reconnect contents — indefinitely if one expected DEX never republishes — while the per-DEX slices keep updating. Revalidation could then reuse a stale size, or throw "No position found" for a position that is open, recreating the reduce-only failures this branch exists to prevent. Both reads now come from one store: isPositionsCacheCoveringDex is replaced by getCachedPositionsForDex(dexName), which returns that DEX's cached positions or null, and closePosition uses that single result for the lookup and for the "covered" decision, so the two can no longer disagree. Unchanged: an entirely uninitialized cache still uses the caller's snapshot with no network call, and an uncovered DEX still gets the targeted clearinghouseState query. Adds two regression tests pinning the reconnect window — a frozen aggregate must not supply a stale size, and a position present only in its DEX's fresh slice must not fail the close. Both fail against the previous cross-store read. TAT-3252

Explanation
HyperLiquid rejects a reduce-only order with
Order 0: Reduce only order would increase positionwhenever the submitted size or side cannot be absorbed by the live position. Per Mixpanel this
accounted for 50-60% of failed position closes (TAT-3252).
closePositionhad several ways to submit such an order:params.positionto avoid aREST
getPositions()call and its 429s, and both the order side and size were derived entirely fromthat throttled (~1s) snapshot. A concurrent TP/SL fill, a liquidation, or a double-tapped close
leaves it larger than — or opposite to — the real position.
usdAmount, whichcalculateFinalPositionSizetreats as the source of truth: it discards the requested size,recomputes
usdAmount / currentPrice, rounds toszDecimals, and then adds a whole sizeincrement whenever the rounded notional lands below the requested USD. That rule exists for opens
(clearing the $10 minimum); on a 100% close it submits
positionSize + 1 increment, which happensroughly half the time.
formatHyperLiquidSizerounds half-up on top of that.as a reduce-only rejection instead of naming the real problem.
Changes
closePositionre-validates the caller's snapshot against the WebSocket position cache and uses thelive side and size. This costs no extra request when the cache covers the symbol's DEX; a missing
entry there means the position is already closed, so the close fails fast with
No position found for <symbol>. When the cache does not cover that DEX — a HIP-3 DEX whosesubscription has not published this session — a single
clearinghouseStaterequest is issued for that DEXalone, so the outcome is attributable to it: a DEX that answers without the symbol (including one
reporting no positions at all) fails the close, while a failed request keeps the caller's snapshot
rather than blocking a position that may still be open.
whole position no longer forwards
usdAmount, andcalculateFinalPositionSizecaps a reduce-onlysize at the requested size so
usdAmountcannot recompute above it.sizemust be a positive number;'0'or a non-numeric value fails withORDER_SIZE_POSITIVEinstead of silently closing everything. Only an omitted or emptysizemeans"close 100%".
calculateFinalPositionSizetakes an optionalreduceOnlyflag: the size is rounded down onto theasset's size grid and the "+1 increment" top-up is skipped, and a size that floors to zero throws
ORDER_SIZE_POSITIVErather than submitting"0". AreduceOnlycall that supplies ausdAmountalongside a non-positive
sizethrows the same error instead of capping to it. The batchclosePositionspath floors the same way before formatting and skips a position smaller than one sizeincrement rather than submitting a zero-size order. It also credits each HIP-3 freed-margin transfer
only to the order that earned it: the transfer list was compacted to HIP-3 positions but read with the
response-status index, so in a mixed main-DEX/HIP-3 batch a successful main-DEX close moved HIP-3
margin even when the HIP-3 close failed.
the position or past what the caller asked to close. This applies to every reduce-only order through
placeOrder, including limit and TP/SL orders, not only closes.floorToSizeDecimals(size, szDecimals)from@metamask/perps-controller/utils/*, andHyperLiquidSubscriptionService.isPositionsCacheCoveringDex(dexName)(internal — that class is notreachable through the export map).
References
Checklist
Behaviour changes for consumers
No breaking API surface:
CalculateFinalPositionSizeParamsgains an optionalreduceOnly,floorToSizeDecimalsis an additive export under the existing./utils/*subpath, theClosePositionParams.positionchange is a doc comment, and no state, action, event, constant, errorcode, or subpath is added or removed. Behaviour clients will observe:
No position found for <symbol>instead of theHyperLiquid reduce-only rejection. A double-tapped close is the common trigger — worth checking any
UX that string-matches the old message.
size: '0'or a non-numeric size now returnsORDER_SIZE_POSITIVE. Mobile sends''for a full close, which is unaffected.
ORDER_SIZE_POSITIVE.including reduce-only limit and TP/SL orders submitted directly through
placeOrder, not justcloses. Partial closes previously recovered by closing ~1.5% more than requested; they no longer do.
reduceOnlyplaceOrderthat supplies bothusdAmountand a non-positivesizenow throwsORDER_SIZE_POSITIVEinstead of submitting a zero or negative size.closePositionsskips a position smaller than one size increment instead of submitting a zero-sizeorder for it, and returns an empty result if every position is that small.
sizecovers the position is treated as a full close for validation, so itskips the USD/$10-minimum check as an omitted
sizealready did. Where the live position is largerthan the snapshot the client rendered, such a close now closes the larger, live amount.
usdAmount, but keeps itspriceAtCalculationstaleness abort:calculateFinalPositionSizenow runs that check whenever the field is supplied rather than onlyinside its
usdAmountbranch, so an exact-size full close pastmaxSlippageBpsis still rejectedwith "Price moved too much". No guard is removed.
clearinghouseStaterequest for that DEX; the caller's snapshot is used only if that request fails.closePositionsbatch, a HIP-3 freed-margin transfer is now initiatedonly when that HIP-3 close itself succeeded. Previously a successful main-DEX close could trigger
it — a balance-moving defect that predates this branch.
Consumption is a
@metamask/perps-controllerversion bump; no client-side change is required.Testing
closePosition reduce-only safetyblock plus
tests/src/utils/orderCalculations.test.ts, which covers the size-calculation branches andthe flooring invariant directly.
yarn workspace @metamask/perps-controller run jest tests/src/providers/HyperLiquidProvider.trading.test.ts tests/src/utils/orderCalculations.test.ts— 103 passed (targeted; the full four-suite set runs inside the recipe proof below).packages/perps-controller/srcreverted toorigin/mainon a clean throwaway commit (
5a9c9d736), the same recipe fails — 51 of 219 tests fail, exit 4 — while branchsource
575dd8c03passes with 215 passed, 0 failed (14/14 nodes, 48/48 required identities).yarn eslintandyarn prettier --checkon the changed files, andchangelog:validate, all clean.packages/perps-controller/e2e/is absent).575dd8c03, includingBuild (24.x),lint:tsc, and package tests on Node 18, 20, and 22.Screenshots/Recordings
State-only evidence for TAT-3252: a 14-node Recipe v1 proof records the executed Core checkout provenance, runs the four regression suites this diff changes through Jest, and asserts zero failures, the intended 4-suite inventory, and all 48 required regression tests by full identity (suite file + ancestor titles + title, so same-named tests cannot substitute for one another). The required-test helper ships at recipe-support/assert-required-tests.cjs and is invoked by artifact path with explicit --report/--output arguments, so the run reproduces from this package alone. Every figure cited below is read from the bundled files: the authoritative run is 575dd8c and the reverted-source run is 5a9c9d736, both on clean trees. No visual evidence exists or applies — @metamask/perps-controller is a headless library with no UI, and the behaviour under test is the SDK order payload sent to HyperLiquid.
Note
High Risk
Changes live trading close/submit paths (size, side, retries, and batch margin transfers); incorrect logic could block valid closes or submit wrong reduce-only sizes, with direct user funds impact.
Overview
Fixes HyperLiquid "Reduce only order would increase position" failures (TAT-3252) by aligning close flows with live position size/side and never rounding reduce-only sizes up.
closePositionre-checks caller snapshots against per-DEX WebSocket position cache (getCachedPositionsForDex), fails fast when the DEX cache shows the symbol gone, and falls back to a single-DEXclearinghouseStatewhen that DEX has not published. Close size is clamped to the live position; invalidsizevalues error withORDER_SIZE_POSITIVE. Full closes submit the exact live size and omitusdAmountsoplaceOrdercannot recompute a larger size.calculateFinalPositionSizegainsreduceOnly: floor onto the size grid via newfloorToSizeDecimals, skip USD top-up, cap USD-derived size to requested size, and run price staleness wheneverpriceAtCalculationis set.placeOrderno longer 1.5% retries on the $10 minimum for reduce-only orders.closePositionsfloors each close size, skips sub-increment dust (reported as failures), preserves result order, and aligns HIP-3 margin transfers with the matching order index.Reviewed by Cursor Bugbot for commit 575dd8c. Bugbot is set up for automated code reviews on this repo. Configure here.