(CL): Add query for Liquidity in Ranges #4691

mattverse · 2023-03-21T13:17:55Z

Closes: #XXX

What is the purpose of the change

Adds Query for Liquidity in Ranges.
Iterates over all ticks that are initialized starting from min tick, until we hit the upper tick given. We cumulate the liquidity net in the process of iterating it, using the final result in the last iteration as the value for the liquidity.

Brief Changelog

Add query for Liquidity in Ranges

Testing and Verifying

Added new tests.

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

p0mvn · 2023-03-21T18:38:46Z

x/concentrated-liquidity/tick.go

@@ -183,6 +184,125 @@ func GetMinAndMaxTicksFromExponentAtPriceOne(exponentAtPriceOne sdk.Int) (minTic
 	return math.GetMinAndMaxTicksFromExponentAtPriceOneInternal(exponentAtPriceOne)
 }

+// GetTickLiquidityForRangeInBatches returns an array of liquidity depth within the given range of lower tick and upper tick.
+func (k Keeper) GetTickLiquidityForRange(ctx sdk.Context, poolId uint64, lowerTick, upperTick int64) (sdk.Dec, error) {


We had more discussions with @jonator around this query today and based on the conclusions, I think we can separate 2 queries

Query for the liquidity depths for the liquidity graph across the entire tick range

I think we do not need lowerTick and upperTick for that since we need the entire price range for the graph

We return "liquidity depths", that is "total overlapped liquidity for positions within that range" so that frontend already has data in the final format for plotting the graph

Query for estimating swaps. The chain returns tick liquidity net amounts in the direction of the swap starting from the current current active tick

The reason we start from the current active tick is that for the context of the swap, we do not need to know the full tick range, only starting from the current one and until the liquidity runs out

We can specify an upper bound or lower bound here signifying the price impact protection

Instead of returning liquidity depths here, we simply return liquidity_net amounts exactly as they are stored instate.

We can use a very similar algorithm for both queries with minor modifications. I propose to make this PR focus on 1), and I can start integrating #2 by branching off of this PR tomorrow. Let me know what you think.

Sweet, sounds great, revised this PR so that it suits the first query.

p0mvn · 2023-03-21T18:44:43Z

x/concentrated-liquidity/tick.go

+		if nextTick.GT(sdk.NewInt(upperTick)) {
+			break
+		}


Related to my other comment - I don't think we need to use the upperTick assuming that this query is meant for liquidity graph.

Fixed and removed upper tock and lower tick

p0mvn · 2023-03-22T03:51:09Z

x/concentrated-liquidity/tick.go

+	currentTick = nextTick.Int64()
+
+	totalLiquidityWithinRange := currentLiquidity
+	for currentTick <= upperTick {


I think that instead of looping here and calling swapStrategy.NextInitializedTick, we can iterate the tick prefix for that pool using this:

osmosis/osmoutils/store_helper.go

Lines 101 to 108 in f447c87

func gatherValuesFromIterator[T any](iterator db.Iterator, parseValue func([]byte) (T, error), stopFn func([]byte) bool) ([]T, error) {

// Replace a callback with the one that takes both key and value

// but ignores the key.

parseKeyValue := func(_ []byte, value []byte) (T, error) {

return parseValue(value)

}

return gatherValuesFromIteratorWithKeyParser(iterator, parseKeyValue, stopFn)

}

I haven't looked deep enough into it so please proceed with caution on this suggestion. However, if we get the iterator to work, it should be considerably more performant than looping

Would you mind if we create an issue this for further improvements? I have took a look at it, seems like this implementation would take more time than the current implmenetation, while I do not want to block front end for this query time wise.

Some of the implementation I was working on: https://github.com/osmosis-labs/osmosis/compare/mattverse/use-iterator?expand=1
Mainly more time consuming because we need to utilize both key + value used, and we need to parse the key as well to receive the tick index for the key, while we do not have enough helper methods to do this right away

Agreed and approved, I trust you'll either make an issue or PR a follow up change 👍

p0mvn

LGTM once new comments are addressed. Nice work!

Nothing blocking and agreed to get v1 in for frontend and work on v2 in the next PR

p0mvn · 2023-03-22T22:58:52Z

proto/osmosis/concentrated-liquidity/pool-model/query.proto

+  string lower_tick = 2 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.moretags) = "yaml:\"tick\"",
+    (gogoproto.nullable) = false
+  ];
+  string upper_tick = 3 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.moretags) = "yaml:\"tick\"",
+    (gogoproto.nullable) = false
+  ];


Should we remove these?

good catch, addressed

Actually, this should not be removed. This is a proto struct being used to return values for the response

p0mvn · 2023-03-22T22:59:46Z

x/concentrated-liquidity/grpc_query.go

+// TickLiquidityInBatches returns array of liquidity depths in the given range of lower tick and upper tick.
+// Note that the space between the ticks in the returned array would always be guaranteed spacing greater than given batch unit.
+func (q Querier) TotalLiquidityForRange(goCtx context.Context, req *clquery.QueryTotalLiquidityForRangeRequest) (*clquery.QueryTotalLiquidityForRangeResponse, error) {


godoc needs to be revised

Godoc revised

p0mvn · 2023-03-22T23:03:24Z

x/concentrated-liquidity/tick.go

+		keyTick := types.KeyTick(poolId, nextTick.Int64())
+		tickStruct := model.TickInfo{}
+		found, err := osmoutils.Get(store, keyTick, &tickStruct)


Can we use the GetTickByTickIndex here?

p0mvn · 2023-03-22T23:04:47Z

x/concentrated-liquidity/tick.go

@@ -226,3 +306,17 @@ func (k Keeper) GetPerTickLiquidityDepthFromRange(ctx sdk.Context, poolId uint64

 	return liquidityDepths, nil
 }
+
+func (k Keeper) GetTickByTickIndex(ctx sdk.Context, poolId uint64, tickIndex sdk.Int) (model.TickInfo, error) {


nit: consider unexporting

p0mvn · 2023-03-22T23:08:00Z

x/concentrated-liquidity/tick_test.go

+
+		expectedLiquidityDepthForRange []query.LiquidityDepthWithRange
+	}{
+		{


Can we add a test case with 2 full range and 2 narrow range positions?

I don't think there is one where we have both full range and narrow range

WIP: initial liquidity for range

8d5fa1d

mattverse marked this pull request as draft March 21, 2023 13:18

github-actions bot added the C:x/concentrated-liquidity label Mar 21, 2023

Add test cases

2135e20

mattverse marked this pull request as ready for review March 22, 2023 01:05

mattverse added the V:state/breaking State machine breaking PR label Mar 22, 2023

Merge branch 'main' into mattverse/cl-liquidity-query

02b682f

mattverse requested a review from p0mvn March 22, 2023 01:17

p0mvn reviewed Mar 22, 2023

View reviewed changes

mattverse added 2 commits March 22, 2023 19:20

WIP

881581e

Change to revised logic

95cb59c

mattverse requested a review from p0mvn March 22, 2023 13:55

p0mvn approved these changes Mar 22, 2023

View reviewed changes

mattverse mentioned this pull request Mar 23, 2023

[CL] Improve Performance of Liquidity Query via using iterators #4704

Closed

mattverse added 4 commits March 23, 2023 15:05

Add test cases

10e4e11

Add proto fixes

54d088a

Revert back proto changes

13aa5dd

Revert unncessary proto changes

e296c54

mattverse merged commit fbf577e into main Mar 23, 2023

mattverse deleted the mattverse/cl-liquidity-query branch March 23, 2023 08:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(CL): Add query for Liquidity in Ranges #4691

(CL): Add query for Liquidity in Ranges #4691

mattverse commented Mar 21, 2023 •

edited

Loading

p0mvn Mar 21, 2023

mattverse Mar 22, 2023

p0mvn Mar 21, 2023

mattverse Mar 22, 2023

p0mvn Mar 22, 2023

mattverse Mar 22, 2023

p0mvn Mar 22, 2023

mattverse Mar 23, 2023

p0mvn left a comment •

edited

Loading

p0mvn Mar 22, 2023

mattverse Mar 23, 2023

mattverse Mar 23, 2023

p0mvn Mar 22, 2023

mattverse Mar 23, 2023

p0mvn Mar 22, 2023

mattverse Mar 23, 2023

p0mvn Mar 22, 2023

mattverse Mar 23, 2023

p0mvn Mar 22, 2023

mattverse Mar 23, 2023

	func gatherValuesFromIterator[T any](iterator db.Iterator, parseValue func([]byte) (T, error), stopFn func([]byte) bool) ([]T, error) {
	// Replace a callback with the one that takes both key and value
	// but ignores the key.
	parseKeyValue := func(_ []byte, value []byte) (T, error) {
	return parseValue(value)
	}
	return gatherValuesFromIteratorWithKeyParser(iterator, parseKeyValue, stopFn)
	}

(CL): Add query for Liquidity in Ranges #4691

(CL): Add query for Liquidity in Ranges #4691

Conversation

mattverse commented Mar 21, 2023 • edited Loading

What is the purpose of the change

Brief Changelog

Testing and Verifying

Documentation and Release Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p0mvn left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattverse commented Mar 21, 2023 •

edited

Loading

p0mvn left a comment •

edited

Loading