(CL): Add Liquidity Net In Direction Query #4714
Conversation
| return nil, err | ||
| } | ||
|
|
||
| return &clquery.QueryLiquidityNetInDirectionResponse{LiquidityDepths: liquidityDepths, CurrentLiquidity: pool.GetLiquidity()}, nil |
There was a problem hiding this comment.
It seems to me that we include "current_tick" and it's liquidity net in the liquidityDepths. However, current tick's liquidity_net is irrelevant for FE swap estimate.
In terms of the current tick, we only need to know a) the current tick index b) current tick liquidity (which we already return)
So I suggest only including liquidity net amounts starting from the "next tick" from current in the swap direction
Instead, we can have a separate field called CurrentTick similar to CurrentLiquidity
Let me know what you think
There was a problem hiding this comment.
CC: @jonator for awareness on this suggestion
| return nil, err | ||
| } | ||
|
|
||
| return &clquery.QueryLiquidityNetInDirectionResponse{LiquidityDepths: liquidityDepths, CurrentLiquidity: pool.GetLiquidity()}, nil |
There was a problem hiding this comment.
Consider renaming liquidityDepths -> liquidityNetAmounts or something like that because net amount is different from depths IMO
There was a problem hiding this comment.
Overall LGTM
Some minor comments about handling the current tick separately from the rest. I resolved all comments that I think were addressed, leaving only the comments that still require replies/changes.
I'm ready to merge once the current tick request is addressed and the overall design is accepted by Jon.
Will wait till Jon's acceptance before approving
| // LiquidityNetInDirection returns liquidity net in the direction given. | ||
| // Uses the bound if specified, if not uses either min tick / max tick | ||
| // depending on the direction. |
There was a problem hiding this comment.
nit: if we go with the separate "current tick" and "current tick liquidity", it might be worth updating the comment to reflect these returns as well
| @@ -211,6 +211,39 @@ func (q Querier) TotalLiquidityForRange(goCtx context.Context, req *clquery.Quer | |||
| return &clquery.QueryTotalLiquidityForRangeResponse{Liquidity: liquidity}, nil | |||
| } | |||
|
|
|||
| // LiquidityNetInDirection returns an array of LiquidityDepthWithRange, which contains the range(lower tick and upper tick) and the liquidity amount in the range. | |||
There was a problem hiding this comment.
nit: ditto for godoc updates to reflect additional returns
| message QueryLiquidityNetInDirectionRequest { | ||
| uint64 pool_id = 1 [ (gogoproto.moretags) = "yaml:\"pool_id\"" ]; | ||
| string token_in = 2 [ (gogoproto.moretags) = "yaml:\"token_in\"" ]; | ||
| string bound_tick = 3 [ |
There was a problem hiding this comment.
How can we know we're requesting all ticks (in a direction)? Should we just pass in the hardcoded max_tick value?
I'm thinking in the frontend, the estimates will go like:
- Try to reduce query size and roughly estimate how many ticks we'll need based on size of trade and liquidity in pool.
Above is an edge case, as I want the above to work in most common cases (trading a moderate amount against a higher liquidity pool), this is an edge case scenario:
- If we run out of ticks in simulation iteration, we'll assume it's because we didn't request enough ticks. Then we'll try again with all ticks in a given direction, by passing in either
maxTickintobound_tickor using some special token/flag?? @mattverse @p0mvn - Only after we've tried getting the remaining tick net values, can we conclude the pool doesn't have enough liquidity to handle the user's current trade, where we will show an error.
There was a problem hiding this comment.
To estimate the expected final tick very roughly, we could take the current tick liquidity, take some proportion of that (e.g. 95%) because we assume that as we move away from the current tick, the liquidity decreases. Note, that we can come up with an algorithm to estimate the proportion too but can be hard coded to start. Finally, compute sqrt price delta in the following way:
- swapping y for x:
- swapping x for y:
Once we have that, we compute the "next expected sqrt price". From sqrt price, we can compute the expected final tick.
If that fails, we could then do some form of binary search where we take the estimated tick delta
$$2 \Delta T$$ $$4 \Delta T$$
exclusively from the ticks we already received during prior querying to avoid redundant processing.
Similar to how Go slices reallocate buffer as we append to them.
Do you think we can keep the current design of querying until max bound for v1 and then work out a design for bounds estimates separately? @jonator @mattverse
I think merging this would allow @mattverse and I to parallelize and further iterate on improvements
Let me know what you all think
There was a problem hiding this comment.
So we can run that calculation to get the next tick index then perhaps add a few extra ticks to avoid any rounding error?
|
Todo after this PR @p0mvn @mattverse Inspired by this thread
|
Closes: #XXX
What is the purpose of the change
A query to better support front-end. Given ZeroForOne and the tick designated to iterate until(optional), iterates over ticks that are initialized until we meet the designated tick, and returns the tick indexes along with the liquidity net in each tick.
Brief Changelog
Testing and Verifying
Documentation and Release Note
Unreleasedsection inCHANGELOG.md? (yes / no)x/<module>/spec/) / Osmosis docs repo / not documented)