feat(hip-3-pusher): Support SEDA last price field #3358
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tejasbadadare
left a comment
tejasbadadare
left a comment
There was a problem hiding this comment.
Overall needs some better documentation since this is all very specific business logic motivated by a hack we're putting in place. I feel like future readers (or even us in a month) won't be able to make sense of it unless it's well documented. Docstrings on the appropriate methods explaining their motivation should suffice for this.
Also, can we add unit tests for the added price waterfall logic in get_price_from_session_ema_source ? The logic there looks non-trivial
| px = self.get_price(source_config, oracle_update) | ||
| if px is not None: | ||
| pxs[f"{self.market_name}:{symbol}"] = str(px) | ||
| if not isinstance(px, str) and not isinstance(px, list): |
There was a problem hiding this comment.
could you leave a comment as to why this is necessary? its a bit mysterious to future readers
| def get_price_from_session_ema_source(self, oracle_source: PriceSource, ema_source: PriceSource): | ||
| now = time.time() | ||
| oracle_update: PriceUpdate | None = self.all_states.get(oracle_source.source_name, {}).get(oracle_source.source_id) | ||
|
|
||
| if oracle_update is None: | ||
| logger.warning("source {} id {} is missing", oracle_source.source_name, oracle_source.source_id) | ||
| return None | ||
| # check staleness | ||
| time_diff = oracle_update.time_diff(now) | ||
| if time_diff >= self.stale_price_threshold_seconds: | ||
| logger.warning("source {} id {} is stale by {} seconds", oracle_source.source_name, oracle_source.source_id, time_diff) | ||
| return None | ||
|
|
||
| if oracle_update.session_flag: | ||
| return [oracle_update.price, oracle_update.price] | ||
|
|
||
| ema_price = self.get_price_from_single_source(ema_source) | ||
| if ema_price is None: | ||
| logger.warning("source {} id {} ema price is missing, reverting to just oracle", oracle_source.source_name, oracle_source.source_id) | ||
| return oracle_update.price | ||
|
|
||
| return [oracle_update.price, ema_price] |
There was a problem hiding this comment.
please leave a docstring explaining the logic/waterfall that's happening here, its hard to follow just reading the code
There was a problem hiding this comment.
also, pls also add return value typehints to function sigs to make it easier to mentally track the types.
for example, this func can return any one of None | int | [int, int]... hard to reason about the correctness of how the caller is handling all of those possibilities without typehints
Summary
Rationale
How has this been tested?