Skip to content
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

fix: make order tracking more robust. #96

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

adamamsmith
Copy link
Contributor

@adamamsmith adamamsmith commented Aug 29, 2023

Make order tracking more robust.

Description

This PR aims to make order tracking more robust by changing the following:

  1. Returning block_number for queries to get_offers from the Subgraph.
  2. Adding an event queue on_subgrounds_order_query to the gridbot to update inventory accordingly.
  3. Adding an optional from_block to the start_event_poller method so we can create filters from a specific block number.
  4. Adding a stop_event_poller method so we can also stop old event pollers.
  5. Additionally added the ability to update the grid fair price.

What was the issue?

The order tracking client would sometimes miss orders. This introduces a periodic poll to the subgraph to ensure all orders are correct.

What type of change was this

  • none - deployment should be skipped
  • fix - fixing bugs and adding small changes (X.X.X+1)
  • feat - introducing a new feature (X.X+1.X)
  • breaking - a breaking API change (X+1.X.X)

@adamamsmith adamamsmith added the enhancement New feature or request label Aug 29, 2023
@adamamsmith adamamsmith requested a review from bghughes August 29, 2023 10:52
@adamamsmith adamamsmith self-assigned this Aug 29, 2023
@@ -29,34 +45,35 @@ def __init__(
self.open = open

@staticmethod
def get_fields(offer_query: Any) -> List:
def get_fields(field_paths: Dict[str, FieldPath]) -> List:
Copy link
Contributor

Choose a reason for hiding this comment

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

with how we are initializing the Offer/Trade entities in _initialize_subgraph_trade for example, we will most likely want the fields that are returned to have the option to not include the added synthetic fields.

return offers_query
block_field_path = self.subgraph.Query._meta().block() # noqa

return {"offer": offers_field_path, "block": block_field_path}

def _build_trades_query(
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it makes sense to also update the way we retrieve trades to match what we are doing with offers in this PR, it seems like a better approach going forward

@@ -88,19 +88,36 @@ def __init__(
# inventory functions
######################################################################

def update_fair_price(self, fair_price: Decimal):
Copy link
Contributor

Choose a reason for hiding this comment

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

it may be helpful to have the ability to return back whatever the difference between the existing grid and the new grid is. however, without tracking what existing offers there are on the market this may not be all that helpful within the Grid class itself.


while polling:
while self.running_event_pollers[event_poller_name] == from_block:
try:
for event_data in event_filter.get_new_entries():
Copy link
Contributor

Choose a reason for hiding this comment

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

this is great

Copy link
Contributor

@denverbaumgartner denverbaumgartner left a comment

Choose a reason for hiding this comment

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

looks great, event poller handling should be a big quality of life improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants