Skip to content

feat(perps-controller): add isolated position modify preview - #9968

Open
michalconsensys wants to merge 10 commits into
mainfrom
feat/perps-position-modify-preview
Open

feat(perps-controller): add isolated position modify preview#9968
michalconsensys wants to merge 10 commits into
mainfrom
feat/perps-position-modify-preview

Conversation

@michalconsensys

@michalconsensys michalconsensys commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Explanation

Pro mode's increase/decrease UX (TAT-3705) needs a current → projected margin and liquidation. That projection is protocol math, not UI formatting: HyperLiquid applies selected leverage to the whole isolated position before the fill, and maintenance margin depends on the tier at the liquidation notional. Clients that recompute this locally get both cases wrong, and they can also show a false projection for cross-margin positions or drop a valid margin preview when liquidationPrice is null.

This adds a read-only previewPositionModify API on PerpsController / PerpsProvider. The client supplies the live position and proposed order; the HyperLiquid provider loads the asset's margin table from cached meta and returns a discriminated result:

  • open — increase, decrease, or flip with remaining size > 0
  • full_close — no remaining size (invalid leftover states are unrepresentable)
  • unsupportedcross_margin or a provider that cannot project (MYX)
  • none — no meaningful modify

Margin and liquidation availability are independent, so a missing live liquidation or missing multi-tier table withholds only liquidation. Isolated liquidation uses the maintenance tier at size * liqPrice, including that tier's maintenance deduction.

Clients should use resulting.direction (not the order direction) when validating TP/SL against the projected liquidation.

This is a breaking PerpsProvider interface addition. Mobile/Extension should consume it after the next @metamask/perps-controller release; they do not implement PerpsProvider themselves. Client wiring is intentionally not in this PR.

References

  • TAT-3705 — UX for increasing / decreasing position in Pro mode (before → after). This PR supplies the calculation source of truth the ticket does not specify.
  • Related Mobile work: keep the before → after presentation, wrapping, and localization; replace the local positionModifyPreview arithmetic with this API after the controller is released.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR. Client consumption follows the @metamask/perps-controller release; no client source changes are included here.

Note

Medium Risk
New trading projection logic affects margin/liquidation UX and TP/SL validation inputs; incorrect formulas could mislead users, though the API is read-only and does not submit orders.

Overview
Adds a breaking read-only previewPositionModify API on PerpsController and PerpsProvider so clients can show post-trade isolated margin and liquidation before placing an order, without duplicating HyperLiquid math locally.

Callers pass the live position plus proposed order (size, direction, expected price, selected leverage, optional fees). Results are a discriminated union: open (increase / decrease / flip with projected size, entry, mark-based leverage, margin, and optional liquidation), full_close, unsupported (cross-margin or MYX), or none when the order would not change the position. Margin and liquidation are independently optional so a valid margin projection can still return when tier data or live liquidation is missing.

HyperLiquid loads margin tiers from cached meta and projects leverage reallocation, fees, and tier-aware isolated liquidation (mark-based, not entry). Aggregated routing uses providerId / position.providerId. Pure helpers in hyperLiquidPositionPreview are exported for tests and advanced consumers; messenger action types, mocks, and broad unit coverage were updated.

Reviewed by Cursor Bugbot for commit 8305e8e. Bugbot is set up for automated code reviews on this repo. Configure here.

Give clients a read-only HyperLiquid isolated projection so Pro order forms
can show before→after margin and liquidation from the same result used for
TP/SL, including full-position leverage changes and maintenance tiers.
@michalconsensys
michalconsensys requested review from a team as code owners August 26, 2026 08:10
@michalconsensys michalconsensys self-assigned this Aug 26, 2026
@michalconsensys
michalconsensys deployed to default-branch August 26, 2026 08:11 — with GitHub Actions Active
Comment thread packages/perps-controller/src/utils/hyperLiquidPositionPreview.ts
Comment thread packages/perps-controller/src/utils/hyperLiquidPositionPreview.ts
@cursor
cursor Bot requested review from abretonc7s, ccharly and zelkibuilds August 26, 2026 08:19
Reject same-direction reduce-only and missing fill prices instead of
projecting a false decrease or falling back to entry, and lock in short,
limit, and leverage-down geometry with tests.
Add JSDoc param fields, drop the untyped notional IIFE, and rewrite
preview assertions so CI lint:eslint passes.
HyperLiquid isolated leverage is mark notional / margin. Deriving it
from entry notional drifted after updateLeverage whenever the position
had unrealized PnL.
Comment thread packages/perps-controller/src/utils/hyperLiquidPositionPreview.ts Outdated
Increase leverage used live mark notional plus fill notional, which is
not HyperLiquid's post-fill mark when those prices differ. Use the
resulting size at the fill so leverage matches the displayed position.
Comment thread packages/perps-controller/src/utils/hyperLiquidPositionPreview.ts
Comment thread packages/perps-controller/src/providers/AggregatedPerpsProvider.ts
@abretonc7s

Copy link
Copy Markdown
Contributor

Automated Review — PR #9968

BETA — Automated review from the farmslot pipeline.

Recommendation APPROVE
Reviewed commit ee9a18b4587dba5047fbba3ec64f2e05d0ed8b46
Tier full
Recipe N/A

Summary

Adds a read-only previewPositionModify to PerpsController, MarketDataService, and the
PerpsProvider interface, plus a new pure-math module
src/utils/hyperLiquidPositionPreview.ts. The change is additive across 24 files
(+1753/-1); the single deleted line is an unused test import.

The scope is well chosen: protocol math lives in Core, client wiring is deliberately excluded,
and the result is a status discriminated union so invalid intermediate states
(full_close with a remaining size, none carrying a flip kind) are unrepresentable.

Full review details

Correctness of the protocol math — verified

The liquidation formula was re-derived from first principles and checked against
HyperLiquid's published docs rather than accepted from the code comments.

Liquidation condition margin + size·(liq − entry)·dirSign = size·liq·mmr − deduction solves to:

  • long: liq = (entry − margin/size − ded/size) / (1 − mmr)
  • short: liq = (entry + margin/size + ded/size) / (1 + mmr)

This is exactly estimateIsolatedLiquidationPrice
(src/utils/hyperLiquidPositionPreview.ts:214). It is also algebraically identical to the
form used elsewhere in this provider (liq = price − side·margin_available/size/(1 − mmr·side),
HyperLiquidProvider.ts:12998) — the price term outside the denominator is exact, not an
approximation, because the price inside maintenance_margin_required cancels back in.

Tier handling matches the docs verbatim:

  • mmr = 1/(2 · tier_max_leverage)hyperLiquidPositionPreview.ts:146
  • deduction(n) = deduction(n−1) + lowerBound(n) · (mmr(n) − mmr(n−1)) — line 148

Notably, estimateIsolatedLiquidationPriceAtTier (line 239) selects the tier at the
liquidation notional (size · liqPrice) rather than the entry/mark notional. HyperLiquid's
docs specify exactly this, and it is the harder, strictly-correct choice — the entry-notional
shortcut most clients take is wrong near a tier boundary. The search-and-verify loop was swept
over 580 long/short × size × leverage combinations against the testnet ETH tier table with
zero unresolved cases, so the loop does not silently fall through to null in practice.

The three modify branches were each checked against what the placement path actually does:

  • increase (line 408) — size-weighted entry average, order margin at selected leverage.
  • decrease (line 433) — margin pro-rated by remaining ratio; mark-based leverage is preserved
    across the reduce (verified: 7.5x before and after when mark ≠ entry), which is correct.
  • flip (line 448) — leftover margin reduces to leftover · price / lev, matching a close-then-open.

Leverage reallocation on the whole position is also right: #prepareAssetForTrading
(HyperLiquidProvider.ts:4756) calls updateLeverage whenever params.leverage is supplied,
including for reduce-only orders, so the preview's reallocation-before-fill model matches
placement rather than diverging from it.

Downstream compatibility — Mobile / Extension

The PR is labelled BREAKING for adding a required method to PerpsProvider. Checked against
local clones (metamask-mobile-1, metamask-extension-1, both on @metamask/perps-controller ^12.0.0;
package is at 12.2.0):

  • No downstream implementor of Core's PerpsProvider exists. grep for
    implements PerpsProvider, satisfies PerpsProvider, as PerpsProvider across both clients
    returns nothing structural. The one hit —
    metamask-extension-1/ui/__mocks__/perps/perps-controller/index.ts:1828 — resolves to a
    locally declared PerpsProvider type at line 1571 in that same mock file; it does not import
    Core's interface, so it cannot break.
  • All three in-repo implementors are updated: HyperLiquidProvider, AggregatedPerpsProvider
    (forwards to the default provider), MYXProvider (returns unsupported/provider).
  • No exports were removed or renamed. git diff over src/index.ts and src/types/index.ts
    shows zero - lines. No action/state/constructor/enum/error-code shape changed.
  • No client currently calls previewPositionModify or a local positionModifyPreview, so there is
    no behavioural drift to reconcile on release.

Practical impact: breaking in name, non-breaking in effect for Mobile and Extension. The
**BREAKING:** changelog prefix is still the correct call — an external PerpsProvider
implementor would break — and the entry already tells consumers exactly what to add.

Non-blocking observations

  1. HyperLiquidProvider.ts:12971 — when the meta.universe entry is not found, marginTableId
    is undefined, so resolveHyperLiquidMarginTiers takes the single-tier branch using the
    client-supplied position.maxLeverage. For a genuinely multi-tier asset this returns a
    liquidation ~1.4% further away than reality (worked example: 20 ETH @ 2500, 5000 margin →
    2295.92 single-tier vs 2327.78 true-tier) — optimistic in the risk-increasing direction, and
    indistinguishable from a good result because available: true is still set. A missing
    universe entry is uncommon, and the thrown-meta path correctly withholds liquidation instead;
    consider treating "asset found but tier table unresolved" the same way for symmetry.

  2. hyperLiquidPositionPreview.ts:416 / :455Math.max(0, …) can produce
    margin: { available: true, value: 0 } on an open result when fees exceed margin, paired
    with liquidationPrice: { available: false } and a leverage that silently falls back to the
    selected leverage (resultingLeverage, line 280). Reproduced for both increase and flip. It
    needs an implausible fee to trigger, but a zero-margin open position is arguably none.

  3. hyperLiquidPositionPreview.ts:9SIZE_EPSILON = 1e-10 is documented as covering
    "szDecimals noise", but real szDecimals granularity is ~1e-4 (ETH) to 1e-5 (BTC), six orders
    of magnitude larger. An order 1e-9 below position size yields a dust open result
    (size 1e-9, margin $4e-7) where the venue would full-close. Callers pass szDecimals-rounded
    sizes, so this is latent; the comment is the misleading part.

  4. HyperLiquidProvider.previewPositionModify's catch fallback (line 12980) has no
    provider-level test; the withheld-liquidation contract is covered only at the util layer.

None of these block merge — each requires an already-degraded input, and all fail toward
withholding data rather than placing an order.

Tests

815 new lines of unit tests plus provider/service/aggregator coverage. Long and short are
covered symmetrically across increase, decrease, flip, and full close, including the cases that
usually get missed: leverage up and down, limit price away from entry, reduce-only overshoot
treated as full close, same-direction reduce-only → none, missing fill price, missing live
liquidation, and missing tier table. Assertions are on computed values, not snapshots.

Coverage of the new module: 93.54% stmts / 91.42% branch / 100% funcs / 93.33% lines
comfortably above the package thresholds (branches 69, functions 78, lines 80).

Validation commands and results

yarn workspace @metamask/perps-controller run jest --no-coverage \
  tests/src/utils/hyperLiquidPositionPreview.test.ts \
  tests/src/services/MarketDataService.test.ts \
  tests/src/providers/MYXProvider.test.ts \
  tests/src/providers/AggregatedPerpsProvider.test.ts \
  tests/src/providers/HyperLiquidProvider.validation.test.ts
→ Test Suites: 5 passed, 5 total · Tests: 351 passed, 351 total

yarn workspace @metamask/perps-controller run jest --no-coverage tests/src/PerpsController
→ Test Suites: 8 passed, 8 total · Tests: 440 passed, 440 total

yarn build            # root — public exports changed, so a build check is warranted
→ ✔ Project built successfully.

Emitted-artifact check (public export drift):

Symbol dist/index.d.cts dist/index.cjs
previewHyperLiquidIsolatedPositionModify present present
resolveHyperLiquidMarginTiers present present
estimateIsolatedLiquidationPriceAtTier present present
HyperLiquidMarginTier present present
PositionModifyPreviewResult present type-only (expected)

Working tree left clean; a temporary probe spec used for edge-case exploration was removed.

Per the static-review contract, no runtime QA, CDP, recipes, or screenshots were run, and no
Farmslot evidence was linked to audit.

Verdict

APPROVE. The math is correct against a first-principles derivation and the HyperLiquid docs,
including the tier-at-liquidation-notional detail that is easy to get wrong. The API shape makes
invalid states unrepresentable, the breaking-change declaration is honest while having no actual
downstream implementor, and test coverage is thorough on both sides of the book. The four
observations above are worth a follow-up but none justify holding the PR.

abretonc7s
abretonc7s previously approved these changes Aug 26, 2026

@abretonc7s abretonc7s left a comment

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.

Automated review — see comment above for full details.

@geositta geositta left a comment

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.

Requesting changes because the preview can still return incorrect pretrade risk values. Hyperliquid states that liquidations use mark price, but this implementation calculates from average entry while marginUsed is mark-based isolated equity. Aggregated mode can also apply Hyperliquid calculations to a MYX position, as described in Bugbot’s open provider-routing thread, and missing margin-table identity currently falls back to an assumed single tier instead of returning liquidation as unavailable. Bugbot’s open decrease-fill finding is also valid under this API’s expected-fill contract and should be addressed with the mark-price correction.

The overall direction is good: protocol math belongs in the controller, the discriminated result types are clear, and independent margin/liquidation availability is modeled well. Please address these cases and add independent numeric fixtures covering unrealized PnL, explicit provider routing, and missing table identity.

}): PositionModifyPreviewResult => {
const liquidationPrice = estimateIsolatedLiquidationPriceAtTier({
isLong: preview.resultingDirection === 'long',
entryPrice: preview.resultingEntryPrice,

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.

Use projected mark price for liquidation, not average entry. Hyperliquid documents that liquidations use mark price. Here newMargin is derived from marginUsed, which is mark-based isolated equity and already includes unrealized PnL, so pairing it with resultingEntryPrice mixes two different reference prices. Using this PR’s existing unrealized-PnL fixture (mark=2500, resulting entry≈2166.67, margin=375, size=1.5, mmr=1/50) produces liquidation of approximately 1955.78; the mark-based result is approximately 2295.92. Please carry projected mark separately from average entry and use it for liquidation; this is material because clients will use the result for TP/SL validation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Isolated marginUsed is mark-based equity, so liquidation now uses the projected mark (expected fill, or live mark when a reduce has no price) instead of average entry. The unrealized-PnL fixture now asserts liquidation 2295.918… rather than the entry-based ~1955.78.

return tiers && tiers.length > 0 ? tiers : null;
}

if (isPositiveFinite(maxLeverage)) {

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.

Fail closed when the margin-table ID is unknown. Hyperliquid specifies that only table IDs below 50 are single-tier and that tier definitions come from meta. If meta.universe does not contain this asset, the provider passes marginTableId: undefined; this branch then fabricates a single tier from maxLeverage. A missing multi-tier asset can therefore return an available but incorrect liquidation instead of unavailable. Please return null unless a known table ID establishes that the asset is single-tier, and add a missing-asset/table-ID test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. resolveHyperLiquidMarginTiers now returns null unless a finite table id is present. IDs below 50 stay single-tier; unknown identity (asset missing from meta.universe) withholds liquidation instead of inventing a tier from max leverage. Added a missing-asset provider test.

…nd routing

Use projected mark for isolated liquidation, withhold liquidation when
the margin-table id is unknown, mark decreases at the expected fill,
and route aggregated previews by providerId.
Comment thread packages/perps-controller/src/utils/hyperLiquidPositionPreview.ts
Release 13.0.0 landed on main while this PR still listed the preview under Unreleased together with those released notes. Merging parked #9968 under 13.0.0, which fails the merge-queue changelog check.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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 8305e8e. Configure here.

Math.abs(selectedLeverage - currentLeverage) > SIZE_EPSILON;
const existingMarginAfterLeverage = leverageChanged
? currentNotional / selectedLeverage
: currentMargin;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Leverage drop strips extra margin

Medium Severity

When selected leverage falls below the position's current setting, existingMarginAfterLeverage is always set to currentNotional / selectedLeverage. That discards any isolated cushion already above the new initial-margin target. HyperLiquid only tops up when collateral is short of the new IM; extras from updateIsolatedMargin or mark-based equity stay with the position. Previews then understate remaining margin and shift projected leverage and liquidation for any modify that also lowers leverage.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8305e8e. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants