chore: init activeAssetCtxPrice price resolution#9160
Conversation
|
@metamaskbot publish-previews |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
geositta
left a comment
geositta
left a comment
There was a problem hiding this comment.
This is a good direction. The main thing I think we should tighten is the subscriber contract. The PR currently says includeMarketData: false subscribers remain allMids only, but the implementation stores the fast selected price in shared cached price data and fans it out to every subscriber for the symbol. If the intended behavior is per-subscriber price source selection, the cache/notify path needs to project PriceUpdate per subscriber. If the intended behavior is shared fast price per symbol, the changelog/comments should say that clearly and include mixed subscriber coverage.
There is also one startup edge case where activeAssetCtx can publish '0' before either activeAssetCtx or allMids has a real price. That should be skipped until a usable price exists, since focused screens may feed this value into display and calculations.
| // fallback string if ctxPrice is absent so #createPriceUpdate still | ||
| // has something to work with. The preference logic inside | ||
| // #createPriceUpdate will pick up the cached activeAssetCtxPrice. | ||
| const fallbackPrice = |
There was a problem hiding this comment.
- activeAssetCtx can send a 0 price before a real price exists.
When activeAssetCtx has neither midPx nor markPx, and no allMids price is cached yet, the new path falls back to '0', writes that as the cached price, and notifies subscribers immediately.
Changed hunk: packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts:3099
Proof this PR introduces/regresses it: before this PR, activeAssetCtx only refreshed an existing cached price; this PR makes activeAssetCtx drive a first price update and uses '0' when neither source has a real price. That can briefly surface 0 on focused screens and feed 0 into calculations.
Suggested fix: skip the price update until either activeAssetCtx has midPx ?? markPx or a real cached allMids fallback exists. Please add coverage for activeAssetCtx first without price.
| now - marketData.priceLastUpdated <= | ||
| HyperLiquidSubscriptionService.#activeAssetCtxPriceTtlMs; | ||
|
|
||
| const effectivePrice = hasFreshActiveAssetCtxPrice |
There was a problem hiding this comment.
The PR description/changelog says list and overview screens with includeMarketData: false are unaffected and continue to use allMids exclusively, but the changed implementation stores the activeAssetCtx-selected price in shared #cachedPriceData and then fans that same PriceUpdate out to every subscriber for the symbol.
Fanout path: packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts:3850
Contract text: packages/perps-controller/CHANGELOG.md:13
Proof this PR introduces/regresses it: this PR adds activeAssetCtxPrice preference inside #createPriceUpdate, writes the selected fast price into shared #cachedPriceData, and leaves #notifyAllPriceSubscribers subscriber agnostic. If a focused subscriber and a list row subscribe to the same symbol, the list subscriber can receive the fast activeAssetCtx price without opting in.
Suggested fix: keep raw price sources separate and project PriceUpdate per subscriber at notify time. A pragmatic version would be a price source aware #createPriceUpdate(symbol, { priceSource }) path or separate cached allMids and activeAssetCtx prices.
Please add a mixed subscriber test where allMids emits 50000, activeAssetCtx emits 50500, the focused subscriber receives 50500, and the list
subscriber remains at 50000.
…grade Resolves conflict in HyperLiquidSubscriptionService.market-data.test.ts by keeping both the activeAssetCtx price preference tests (HEAD) and the new Market tradability (isTradable) tests (main). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
2 participants
Explanation
Two related improvements to
HyperLiquidSubscriptionService:TAT-3334 — Per-subscriber
activeAssetCtxprice projectionHyperliquid throttles the main-DEX
allMidsstream to ~2 s per push. For list/overview screens that's fine, but a focused detail/ticket screen would show a noticeably stale price at that interval.Previous approach (PR 9160 initial draft):
#createPriceUpdateunconditionally baked theactiveAssetCtxfast-stream price into#cachedPriceData, meaning all subscribers — including list/overview — received the fast price, contradicting the per-subscriber contract documented in the code.New approach — per-subscriber projection:
#createPriceUpdateis reverted to a pure allMids-baseline builder;#cachedPriceDataalways holds the rawallMidsprice.#projectPriceUpdate(symbol, base)helper shallow-clones the base withprice/timestampoverridden by the freshactiveAssetCtxvalue (within a 10 s staleness window).#notifyAllPriceSubscribersprojects per callback: focused (includeMarketData: true) callbacks receive the fast-stream price; list/overview (includeMarketData: false) callbacks always receive the rawallMidsbaseline, guaranteed, even when both subscriber types share the same symbol.subscribeToPricesimmediate emit projects for focused new subscribers, and can send a fast-stream-only update even before the firstallMidstick so detail screens aren't blank on first render.activeAssetCtxfires beforeallMidswith nomidPx/markPx, no notification is sent — the old'0'fallback is removed entirely.activeAssetCtxwas already reference-counted underincludeMarketData: true.TAT-3333 — Order book
fastflag + SDK bumpHyperliquid is changing the default
l2Bookcadence (20 levels @ ~2 s). A newfastsubscription mode is available (5 levels @ ~0.5 s).@nktkas/hyperliquidfrom^0.32.2to^0.33.1(addsfastto thel2Bookrequest schema).fast?: booleantoSubscribeOrderBookParamswith JSDoc explaining the 5-level / ~0.5 s tradeoff.fastthroughsubscribeToOrderBookinto the SDKl2Bookcall.#processOrderBookDataor cumulative-total math.References
Checklist