-
Notifications
You must be signed in to change notification settings - Fork 602
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
(CL): Improve GetTickLiquidityForRange
and GetTickLiquidityNetInDirection
query performance by using iterators
#4805
Conversation
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.
Nice work.
Have a couple of questions/comments:
- Can We rename the function to the
GetTickLiquidityForFullRange
because we do not allow specifying lower and upper bounds? - The godoc needs to be updated
- Can this iterator improvement algorithm be also applied to the query introduce in: (CL): Add Liquidity Net In Direction Query #4714
x/concentrated-liquidity/tick.go
Outdated
tick, err := k.getTickByTickIndex(ctx, poolId, nextTick) | ||
keyTick := types.KeyTick(poolId, tickIndex) | ||
tickStruct := model.TickInfo{} | ||
found, err := osmoutils.Get(store, keyTick, &tickStruct) |
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.
Why do we have to re-get the value? Can't we just get it from iterator.Value()
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.
Fixed it to use directly using iterator.Value()
Co-authored-by: Roman <[email protected]>
Hey @p0mvn thanks for the review, while I was resolving your suggestions, I actually came across the fundamental question of needing to improve query performance via this PR. Right now, the code operates based on NextInitialized tick. If we take a look at the implementation, |
GetTickLiquidityForRange
query performance by using iteratorsGetTickLiquidityForRange
and GetTickLiquidityNetInDirection
query performance by using iterators
Hey @p0mvn thanks for the review, in the most recent commit I have made the following changes,
|
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.
Awesome, LGTM!
Closes: #4704
What is the purpose of the change
Improves performance of
GetTickLiquidityForRange
query by using prefix iterators directly.Brief Changelog
Improve
GetTickLiquidityForRange
query performance by using iteratorsTesting and Verifying
Existing tests passes
Documentation and Release Note
Unreleased
section inCHANGELOG.md
? (yes / no)x/<module>/spec/
) / Osmosis docs repo / not documented)