-
Notifications
You must be signed in to change notification settings - Fork 54
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
[ECO-484] Add order book state #518
Conversation
- add a prev column to limit orders - updated views (when updating the underlying tables, views need to be updated) - fix bigdecimal dependency - fix sdk (broke by aptos update) - fix fills (orders would not go into 'closed' mode)
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Additional TODOs that I could not attach directly to comments
- Take out the
aptos-indexer-processors
submodule update - When ready to merge and all comments addressed, add in the description a printout like the one in [ECO-486][ECO-538] Add balance update support #511
a. Show the config used for global testing and some of the printouts (e.g. screenshots like in [ECO-486][ECO-538] Add balance update support aptos-indexer-processors#9 (comment))
b. Potentially use a more recent version number to hit relevant functionality
c. Also run the local DSS per the docker readme to verify it works, since that docker compose has changed
src/rust/dbv2/migrations/2023-10-03-070930_add_order_order/down.sql
Outdated
Show resolved
Hide resolved
src/rust/dbv2/migrations/2023-10-03-070930_add_order_order/down.sql
Outdated
Show resolved
Hide resolved
src/rust/dbv2/migrations/2023-10-03-071834_fix_price_field/up.sql
Outdated
Show resolved
Hide resolved
src/rust/dbv2/migrations/2023-10-03-071834_fix_price_field/down.sql
Outdated
Show resolved
Hide resolved
src/rust/dbv2/migrations/2023-10-03-070930_add_order_order/up.sql
Outdated
Show resolved
Hide resolved
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.
Need to account for change size (increase) event, reconsider use of persistent data structure.
How does this handle bids/asks having separate books? I think the data structure assumes all markets have just one book which isn't true. |
You should be able to provide two views: |
- [uncomment accidentally commented line](#518 (comment)) - [use `last_increase_time` instead of `prev`](#518 (comment)) - [update JWT secret](#518 (comment)) - [add documentation on how to generate `.sqlx/*` files](#518 (comment)) - [no, it's not](#518 (comment)) - [compile aggregator in build step](#518 (comment)) - [renamed migration to reduce confusion](#518 (comment)) - [consolidate migrations](#518 (comment))
src/rust/dbv2/migrations/2023-10-03-070930_add_order_queue/up.sql
Outdated
Show resolved
Hide resolved
@@ -288,6 +293,8 @@ impl Data for UserHistory { | |||
&change.order_id, | |||
&change.market_id, | |||
&change.time, | |||
&fill.txn_version, | |||
&fill.event_idx, |
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.
@CRBl69 It doesn't look like you de-duplicate fill events in this code here. There two fill events emitted for every fill, one for the maker and one for the taker. You can de-duplicated them using the order id field (I believe).
@@ -137,10 +137,13 @@ impl Data for UserHistory { | |||
.await | |||
.map_err(|e| DataAggregationError::ProcessingError(anyhow!(e)))?; | |||
for x in &limit_events { | |||
let txn = x.txn_version.to_bigint().ok_or(DataAggregationError::ProcessingError(anyhow!("txn_version not integer")))? << 64; | |||
let event = x.event_idx.to_bigint().ok_or(DataAggregationError::ProcessingError(anyhow!("event_idx not integer")))?; | |||
let txn_event: BigDecimal = BigDecimal::from(txn & event); |
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.
@CRBl69 You mean (txn << 64) | (event)
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.
The << 64
is done at line 140.
- fix user history not registering anymore because use of fetch_one instead of execute - add a better error handling system in the aggregator - fix limit_orders view
[ECO-616] Add assorted order book PR tweaks
[ECO-624] Fix async dedupe aggregator logic
to be updated)
Order is implemented with a field named
last_increase_stamp
. This is a number where the order with the lowestlast_increase_stamp
is the highest priority one. It is computed as(txn_version << 64) | event_idx
.The
limit_orders
endpoint is basically the order book state endpoint.Example query: `http://0.0.0.0:3001/limit_orders?order=price.desc,last_increase_stamp.asc&market_id=eq.3&side=eq.ask&order_status=eq.open&limit=3
Example result:
Config used: