-
Notifications
You must be signed in to change notification settings - Fork 609
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
Change LiquidityNetInDirection
return type to sdk math
#8509
Conversation
WalkthroughThe recent modifications focus on addressing type inconsistencies and potential panics in the Changes
Sequence Diagram(s)The conceptual change involves the modification of return types, which doesn't significantly alter the control flow or introduce new features. Therefore, no sequence diagrams are provided. Assessment against linked issues
These changes address the objectives, ensuring type consistency and preventing runtime panics due to type mismatches. Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
x/concentrated-liquidity/types/cl_pool_extensionI.go (1)
25-25
: Update allGetLiquidity
method implementations to returnsdkmath.LegacyDec
.The following instances of the
GetLiquidity
method still returnosmomath.Dec
and need to be updated tosdkmath.LegacyDec
:
x/concentrated-liquidity/model/pool.go
tests/mocks/cl_pool.go
Please update these implementations to ensure consistency across the codebase.
Analysis chain
Update to
GetLiquidity
method return type is correct.The return type of the
GetLiquidity
method has been updated tosdkmath.LegacyDec
to prevent panics and ensure proper unmarshalling of the response.Ensure that all implementations of
GetLiquidity
are updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all implementations of the `GetLiquidity` method are updated to return `sdkmath.LegacyDec`. # Test: Search for the `GetLiquidity` method implementation. Expect: Only occurances of the new return type. rg --type go -A 5 $'GetLiquidity() sdkmath.LegacyDec'Length of output: 54
Script:
#!/bin/bash # Description: Verify all implementations of the `GetLiquidity` method are updated to return `sdkmath.LegacyDec`. # Test: Search for the `GetLiquidity` method implementation. Expect: Only occurrences of the new return type. rg --type go 'GetLiquidity()' -A 5Length of output: 33231
Closes: #8469
What is the purpose of the change
Changes return value for pool.GetLiquidity, as it was returning osmomath.LegacyDec thus was not able to be unmarshalled when being returned as result for the query.
Testing and Verifying
This change is a trivial rework / code cleanup without any test coverage.
Documentation and Release Note
Unreleased
section ofCHANGELOG.md
?Where is the change documented?
x/{module}/README.md
)