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.
2 participants
Explanation
Prefer
activeAssetCtxprice on detail/ticket screens to absorb 5sallMidscadenceHyperliquid is throttling the
allMidsWebSocket stream to push every ~5 seconds. For list/overview screens this is fine, but on a focused single-symbol view (trade detail, order ticket) the displayed price would noticeably lag at that interval.This PR fixes that by making
subscribeToPricescalls withincludeMarketData: trueuse the per-symbolactiveAssetCtxstream as the source of truth for the displayed price. That stream already ticks at a faster cadence and was already established for market-data-enabled subscriptions — this change just wires itsmidPx/markPxthrough toPriceUpdate.priceinstead of waiting for the nextallMidssnapshot.A 10-second staleness gate ensures that if the fast stream goes quiet,
allMidstakes back over automatically.What changed:
#marketDataCachenow stores the lastactiveAssetCtxprice and its timestamp per symbol#createPriceUpdateprefers the fresh fast-stream price over the passed-inallMidsprice (with TTL fallback)The
activeAssetCtxhandler drives price updates directly from the incomingmidPx/markPxrather than replaying the last cachedallMidsvalueA comment on
#ensureGlobalAllMidsSubscriptiondocuments the 5s cadence and the mitigation strategyWhat didn't change:
List/overview screens (
includeMarketData: false) are completely unaffected — they still run onallMidsonlyNo new WebSocket subscriptions are created;
activeAssetCtxwas already reference-counted underincludeMarketDataReferences
Checklist