-
Notifications
You must be signed in to change notification settings - Fork 302
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
dex: Extend arbitrage routing #4292
Conversation
587bc40
to
b949ec0
Compare
.cloned() | ||
// Limit the inclusion of recently accessed assets to 10 to avoid | ||
// potentially blowing up routing time. | ||
.chain(state.recently_accessed_assets().iter().take(10).cloned()) |
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 could have duplicates right? should we accumulate into a BTreeSet instead until its size increases by the max limit?
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 recently_accessed_assets
set is also incredibly likely to feature assets that are already in the fixed list, perhaps they should be filtered out on insertion as well.
…nd recently swapped assets
… size enforcement to the setter
78b7e3c
to
c6449fa
Compare
c6449fa
to
1425f61
Compare
1425f61
to
fcdb1e5
Compare
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.
LGTM, tested manually on a devnet
); | ||
self.add_recently_accessed_asset( | ||
position.phi.pair.asset_2(), | ||
routing_params.fixed_candidates, |
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 was a minor surprise that this wasn't internalized by add_recently_accessed_asset
but I guess we technically save a lookup.
Describe your changes
This PR extends arbitrage routing to route against assets involved in swaps and positions opened during the current block.
Issue ticket number and link
#4177
Checklist before requesting a review