-
Notifications
You must be signed in to change notification settings - Fork 39
feat: Update full test to use tycho-test and test on stream of updates #307
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
Conversation
61e277e to
2aba30b
Compare
tamaralipows
left a comment
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.
Thank you! Damn this was massive. I have a few small questions for my understanding - the most important is the one about the block delay imo. Overall the refactor itself looks good though!
| "--retention-horizon", | ||
| "2024-01-01T00:00:00", |
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.
Does this mean we throw away data older than this? Don't we have block from before this date possibly?
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.
I'm actually not sure what this retention-horizon means.. I will ask cairoline 👀
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.
it just means that whatever comes before this date won't be stored in the DB! Not that it won't be indexed, it just wont' be stored!
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.
Do we want this in the case where we are not clearing the DB? For example for the constantly running test? Will that still work as expected?
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.
huuumm no idea tbh 😕 but if we are not clearing the DB it shouldn't sync past blocks.. but yeah if it's a really old protocol we might have issues here
4409d58 to
fec7ee0
Compare
- It uses the index command of tycho indexer to sync the protocol until now and then stream the live updates - Connects to the stream and processes updates - Calculates spot prices, limits and get amount out using Tycho Simulation - It simulates execution using RPC batched RPC requests - Uses router and executor overwrites (so that the executor doesn't need to be deployed) - It batches the simulations in smaller chunks. The requests are really big because of the overwrites Other changes: - Using ProtocolStreamBuilder in the state registry instead of TychoStreamDecoder. Then in the ranged test we are just extracting the decoder from the ProtocolStreamBuilder (hacky but it works) - In the full test we need to keep track of the ProtocolComponents to simulate in multiple blocks - Changed get_current_block to get_block(block_number) #time 7h 5m #time 8m #time 1m #time 3m
If true (default), clears the DB and re syncs from start. If not: - for the ranged test, it skips indexing and uses the current state - for the full test, it starts syncing from the last block in the db #time 18m
- Imports at the top - Remove unnecessary chain_block_time logic - Fix uniswap v3 integration test by increasing the stop block a bit #time 2h 37m
fec7ee0 to
4b26d1b
Compare
tamaralipows
left a comment
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.
I'm a bit sus about the retention horizon but I guess we will realize pretty easily if it is broken!
Other changes: