dex: prelude to on-chain arb implementation #2538
Conversation
We don't want to actually close the position in the ActionHandler, because otherwise the economic effects could depend on intra-block ordering, and we'd lose the ability to do block-scoped JIT liquidity, where a single transaction opens and closes a position, keeping liquidity live only during that block's batch swap execution.
This gives a common set of parameters for routing.
I think there's a latent bug here, in that the SwapExecution should be indexed by a _directed_ trading pair...
hdevalence
changed the title
| /// Clamps the spill price to the price limit and returns whether or not it was clamped. | ||
| pub fn clamp_to_limit(&self, spill_price: Option<U128x128>) -> (Option<U128x128>, bool) { | ||
| match (spill_price, self.price_limit) { | ||
| (Some(spill_price), Some(price_limit)) => { | ||
| if spill_price > price_limit { | ||
| (Some(price_limit), true) | ||
| } else { | ||
| (Some(spill_price), false) | ||
| } | ||
| } | ||
| (Some(spill_price), None) => (Some(spill_price), false), | ||
| (None, Some(price_limit)) => (Some(price_limit), true), | ||
| (None, None) => (None, false), | ||
| } | ||
| } |
There was a problem hiding this comment.
I wrote this for use in distinguishing fill_route and fill_route_exact but didn't end up using it yet (and didn't end up doing that distinguishing, either), if it remains unused it could be deleted later maybe?
| // TODO: how does this work when there are trades in both directions? | ||
| // Won't that mix up the traces? Should the SwapExecution be indexed by | ||
| // the _DirectedTradingPair_? |
There was a problem hiding this comment.
@zbuc I noticed this and was a bit confused, don't we want to be recording two distinct executions for each pair per block? otherwise the traces would get mixed up between them, right? Or?
There was a problem hiding this comment.
I think you're right. That does seem wrong. It seems like we need to execute a batch swap per trading direction.
| @@ -164,6 +159,7 @@ trait RouteAndFillInner: StateWrite + Sized { | |||
|
|
|||
| tracing::debug!(?path, delta_1 = ?delta_1.amount, "found path, starting to fill up to spill price"); | |||
|
|
|||
| // TODO: in what circumstances should we use fill_route_exact? | |||
There was a problem hiding this comment.
Ideally we'd have one route_and_fill method that we could use for both swaps and arbitrage. I'm not sure, however, how we want to handle price limits in the two cases, relative to the changes in #2535
This PR has changes along the way to starting an arb implementation, split out since they're all mostly finished.