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

Orders by Owner Pagination #30

Merged
merged 6 commits into from
Feb 17, 2024
Merged

Orders by Owner Pagination #30

merged 6 commits into from
Feb 17, 2024

Conversation

crnbarr93
Copy link
Collaborator

@crnbarr93 crnbarr93 commented Feb 17, 2024

Closes: #28

What is the purpose of the change

The previous implementation of get_orders_by_owner did not employ pagination, leading to the possibility of DoS if a user placed a significant amount of orders. These changes add pagination in a cursor-like format by providing:

  • Minimum limit order (including book and tick)
  • Maximum limit order (including book and tick)
  • Page size

The page size defaults to 50 and has a maximum of 100 (can be adjusted), and all three parameters are optional. A user could iterate through all orders by simply passing the (order.book_id, order.tick_id, order.order_id) of the last order in the previous page as the min parameter. Orders are ordered by the following priority:

  1. Book ID
  2. Tick ID
  3. Order ID

Testing and Verifying

This change added tests and can be verified as follows:

cargo unit-test test_state
  • Added a test for pagination in test_state

Future Work

The current implementation has a troubling ordering due to the tuple used to index orders. A potential approach is to use the multiindex for filtering orders to book/tick and have orders be a direct mapping between order id and the order itself, however this may incur execution overhead at the benefit of easier querying, cancellations and pagination.

@crnbarr93 crnbarr93 linked an issue Feb 17, 2024 that may be closed by this pull request
Copy link
Collaborator

@AlpinYukseloglu AlpinYukseloglu left a comment

Choose a reason for hiding this comment

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

Nice, this should improve integrator/FE UX quite a bit

Base automatically changed from connor/test-refactor to main February 17, 2024 22:02
@AlpinYukseloglu AlpinYukseloglu merged commit 3fc8189 into main Feb 17, 2024
2 checks passed
@AlpinYukseloglu AlpinYukseloglu deleted the connor/pagination branch February 17, 2024 22:06
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.

[Orderbook]: Add pagination to Orders Multi-index
2 participants