-
Notifications
You must be signed in to change notification settings - Fork 7
fix zero division error in PriceDeviation when aggregate price is 0 #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cctdaniel
commented
Feb 3, 2022
- program crashes when aggregate price is 0 as it tries to divide delta (publisher aggregate price - aggregate price) by aggregate price
- not sure what's the best way to handle this case but what I did here was to return False if aggregate price is 0 (happy to get suggestions if there are better alternatives)
…check if deviation exists in event details
to reproduce error, run with devnet as network arg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm approving this, but please delete the dead code before merging.
pyth_observer/events.py
Outdated
details = [ | ||
f"Aggregate: {agg.price:.2f} ± {agg.confidence_interval:.2f} (slot {agg.slot})", | ||
f"Published: {published.price:.2f} ± {published.confidence_interval:.2f} (slot {published.slot})", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case won't ever trigger. The event details are only printed if is_valid == True. I think it makes sense to separate out the validation of the aggregate price into a separate PriceValidationEvent though, so no need for this case here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it triggers when I run the program locally -- I don't see the is_valid == True check in the code, are you able to point me to the right direction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't normally run this against devnet since that's just a throwaway playground for publishers to test things out. We use testnet as a UAT environment and then obviously mainnet is production.
However, this is a good addition. Nice job. I originally didn't want the events to change the title based on the data, but for PriceDeviation it makes sense to do things the way you did I think.
@@ -232,6 +242,8 @@ def get_event_details(self) -> Tuple[str, List[str]]: | |||
f"Published last slot: {self.publisher_latest.slot}" | |||
) | |||
|
|||
return title, details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eye!
Co-authored-by: Jeff Schroeder <jschroeder@jumptrading.com>