-
Notifications
You must be signed in to change notification settings - Fork 0
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
Price Feeds + Restructuring #28
Conversation
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.
Have not checked the code in detail yet, but the overall code structure looks good to me.
I would probably move all price providers into a prices
folde for not clutter the main folder too much.
One additional price provider might be native prices from the auction. That would require the backend database for now.
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.
Looks great to me. I found some small formatting things but overall looks good.
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
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.
Edit: I moved all my comments to the other PR
I added a few comment. Some of the things are easy to address, some can just be turned into issues.
Did you plan on restructuring more, e.g. splitting the daemon file? That could be done in some other PR as well.
I do not see any tests for the added code. I assume some parts of the code did change behavior. How did you check correctness of the code locally? Maybe that could be turned into a test.
Actually, ignore the comments for now. Will look at the actual restructuring PR first. |
Is this PR still relevant? It seems all the changes are contained in #29 as well. |
This PR includes the following changes:
fetch from coingecko.
write_coingecko_prices()
has been changed towrite_prices()
. There is a new table called "slippage_prices"in the solver_slippage DB with an additional field for the price source.
Restructures:
ids list in a fixed interval (currently, every 24 hours) to account for any updated lists.