-
Notifications
You must be signed in to change notification settings - Fork 97
Conversation
How and when do we update |
We haven't finished the code for #8 but that could be where we change the bid/ask based on volume changes. OR We could have something that scans our own database on a regular frequency, compares it to ticker data, then changes ticker data where it sees fit. We can invalidate orders separately and the change would be reflected there. |
5b46334
to
e427fe9
Compare
app/services/websocket_server.py
Outdated
@@ -231,6 +231,36 @@ def format_order(record): | |||
|
|||
await sio.emit('market', response, room=sid) | |||
|
|||
#return ticker information, if no token specified show all | |||
@sio.on('returnTicker') |
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.
There isn't a separate returnTicker
event: instead, returnTicker
is a key on a market
event, which is emitted to a specific user in response to a getMarket
event by that user.
…in the database now.
app/services/websocket_server.py
Outdated
if token: | ||
|
||
#get ticker for passed in token | ||
tickers = await get_tickers(token) |
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 should return all tickers regardless.
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.
If you pass in a specific token, it shouldn't transmit unnecessary data?
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 data fills out the values in the bottom-left ticker though, so it needs to be for all tokens.
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.
Referenced here: #21
Fixed now.
app/services/websocket_server.py
Outdated
#get returnTicker, grabs data from tickers table | ||
async def get_tickers(token_hexstr): | ||
|
||
#init |
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.
Nitpick: according to PEP8, the standard Python code style guide, "They should start with a # and a single space."
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.
Fixed.
app/services/websocket_server.py
Outdated
async def get_tickers(token_hexstr): | ||
|
||
#init | ||
where = '' |
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.
placeholder_args
should also be defined 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.
fixed.
"quoteVolume": str(contract.denormalize_value(ticker["quote_volume"])), | ||
"baseVolume": str(contract.denormalize_value(ticker["base_volume"])), | ||
"last": str(contract.denormalize_value(ticker["last"])), | ||
"percentChange": str(contract.denormalize_value(ticker["percent_change"])), |
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.
Not a 100% sure what the values are going to look like in raw format, we should double-check these conversions once we have them.
app/services/websocket_server.py
Outdated
"percentChange": str(contract.denormalize_value(ticker["percent_change"])), | ||
"bid": str(contract.denormalize_value(ticker["bid"])), | ||
"ask": str(contract.denormalize_value(ticker["ask"])), | ||
"modified": datetime.now().isoformat() |
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.
What is the modified value supposed to reflect?
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.
When the last time that ticker was updated
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.
Sounds like it should come from the DB then. Is this a placeholder?
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.
You're right. Whoops.
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.
Fixed.
app/services/websocket_server.py
Outdated
"bid": str(contract.denormalize_value(ticker["bid"])), | ||
"ask": str(contract.denormalize_value(ticker["ask"])), | ||
"modified": ticker["modified"] | ||
# "modified": datetime.now().isoformat() |
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.
Dead lines should be removed.
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.
Placeholder for when I write the code that updates the data. Will remove at that time.
app/services/websocket_server.py
Outdated
|
||
# get ticker for passed in token | ||
#tickers = await get_tickers(token) | ||
tickers = await get_tickers() |
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 could probably extract that to above if we no longer differentiate by token.
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.
Fixed.
""".format(where), | ||
*placeholder_args) | ||
|
||
# format ticker information to camelcase from underscore, sanitize, add modified date |
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.
Oh, also, suggestion! Python docstrings might be a better way to do this
app/services/websocket_server.py
Outdated
if token: | ||
|
||
# get ticker for passed in token | ||
#tickers = await get_tickers(token) |
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.
Also dead code
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.
Fixed.
app/services/websocket_server.py
Outdated
if token: | ||
|
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.
Unnecessary new lines
Now, we need something that actually populates this data. I think the best method may be using an observer to scan our Maybe there is a better way to only update it on certain events, but this would be a catch all and we could cancel/update orders separately without fear of breaking this system. |
Convo copied here for posterity.
|
Clean websocket_server a little.
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.
Some minor things pointed out.
I'm also not sure about iterating over all orders to build a ticker. Perhaps, the ticker updater should know a list of token addresses which it should update; then, for each token, fetch fields with specific SQL queries.
For example, a ticker bid value can be obtained by running something like
SELECT MAX(amount_give / amount_get)
FROM orders
WHERE token_get = :zero_addr
AND token_give = :token_addr
AND (state = 'OPEN'::orderstate AND expires > :current_block AND available_volume > 0)
This approach is a little harder on the DB, but it is faster than iterating over all orders and is somewhat simpler to implement than partial updates.
return | ||
|
||
# Main function. Grabs orders and trades, then scans through orders and trades to update `tickers` | ||
async def update_tickers(): |
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.
Main function should be called main()
and be called if the module is the main module of the python invocation, like so.
# Main function. Grabs orders and trades, then scans through orders and trades to update `tickers` | ||
async def update_tickers(): | ||
|
||
orders = await get_orders(); |
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.
No semicolons needed if you have just one expression / statement on a line.
return await conn.fetch( | ||
""" | ||
SELECT token_give, amount_give, token_get, amount_get, expires | ||
FROM orders |
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.
Pretty good SELECT statement for pulling all orders!
app = web.Application() | ||
sio.attach(app) | ||
|
||
ZERO_ADDR = "0x0000000000000000000000000000000000000000" |
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 should extract these into reusable constants. app/constants.py
, perhaps?
|
||
else: | ||
# If there isn't a current recorded bid OR current recorded bid is lower than the bid in this order. | ||
if not has_attr(ticker_updates[order["token_get"]], 'bid') || ticker_updates[order["token_get"]]["bid"] < this_orders_bid: |
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 logical or operator is or
.
Moved to #40 |
We will need another database table for ticker information.
Proposed information to store:
bid and ask can come from
orders
last can come from
trades
Would volume have to be determined by adding all of the orders in our orderbook?