Skip to content

Conversation

@ali-behjati
Copy link
Contributor

@ali-behjati ali-behjati commented Jan 6, 2023

This PR mostly fixes some logic bugs or improve them. Also updates the calendar for the new year.

Please see my comments for further explanation.


# When min_publishers is high it means that the price is not production-ready
# yet and it is still being tested. We need no alerting for these prices.
if price_account.min_publishers >= 10:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is equivalent to "beta" on the website and filters not ready prices.



class PriceFeedAggregateCheck(PriceFeedCheck):
class PriceFeedOfflineCheck(PriceFeedCheck):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name was ambiguous. Offline is a better name.


distance = abs(
self.__state.slot_aggregate_attempted - self.__state.slot_aggregate
self.__state.latest_block_slot - self.__state.latest_trading_slot
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A price feed can go stale without its status becoming non-trading. This check relies on checking the latest slot with the latest trading slot (which includes non-trading checks before)

return True

# Pass if price has been stale for a long time
if distance > self.__abandoned_slot_distance:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some feeds are abandoned but still in our list.



class PublisherAggregateCheck(PublisherCheck):
class PublisherWithinAggregateConfidenceCheck(PublisherCheck):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better name again.


# Pass if publisher slot is far from aggregate slot
distance = abs(self.__state.slot - self.__state.aggregate_slot)
if distance > 25:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This value is hard-coded but it is not very important. It is used to filter out the non-trading or behind publishers. We can change it later in the future.

return True

# Skip if no aggregate
if self.__state.price_aggregate == 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When aggregate status is trading this is impossible.

else:
raise RuntimeError("Invalid check")

text = self.check.error_message()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added publisher_name to publisher state and the code here is therefore simpler.

Also with the previous logic if a new publisher was added this code would crash as it couldn't look up the name.

@ali-behjati ali-behjati requested review from cctdaniel, jayantk and thmzlt and removed request for thmzlt January 6, 2023 15:11
Copy link
Contributor

@jayantk jayantk left a comment

Choose a reason for hiding this comment

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

nice!

@ali-behjati ali-behjati merged commit 4aa010c into main Jan 6, 2023
@ali-behjati ali-behjati deleted the bug-fix-improvement-2 branch January 6, 2023 17:26
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