fix CL pool valid key length check #1634
Conversation
joe-bowman
requested review from
faddat,
minhngoc274 and
tuantran1702
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent update to the Changes
Poem
Tip Early Access Features
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/participationrewards/keeper/callbacks.go (1 hunks)
Additional comments not posted (1)
x/participationrewards/keeper/callbacks.go (1)
174-175: The updated key length and prefix checks align with the PR's objectives to handle variable CL pool key lengths and ensure correct prefixes.Verification successful
The function
OsmosisClPoolUpdateCallbackis used in several places, including the main implementation file and test files. To ensure the changes are correct, we need to review the function's implementation and the test cases that cover it.
The implementation of the
OsmosisClPoolUpdateCallbackfunction and its corresponding test cases have been retrieved. The function includes checks for the length ofquery.Requestand the prefix, ensuring they meet the expected criteria. The test cases incallbacks_test.gocover various scenarios, including the unmarshalling of responses and validation of protocol data.The changes made to the function appear to be correctly implemented and are adequately tested.
- The function checks if
query.Requestis at least 2 bytes long.- It verifies that the first byte of
query.Requestis0x03.- The test cases validate these checks and ensure the function behaves as expected.
Based on the retrieved information, the changes are correctly implemented and tested.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the behavior of the updated key length and prefix checks. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type go $'OsmosisClPoolUpdateCallback'Length of output: 1085
Script:
#!/bin/bash # Description: Retrieve the implementation of the OsmosisClPoolUpdateCallback function and relevant test cases. # Retrieve the implementation of the OsmosisClPoolUpdateCallback function ast-grep --lang go --pattern $'func OsmosisClPoolUpdateCallback($_, $_, $_, $_) $_' # Retrieve the test cases for OsmosisClPoolUpdateCallback ast-grep --lang go --pattern $'func (suite *KeeperTestSuite) TestOsmosisClPoolUpdateCallback() $_'Length of output: 9639
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1634 +/- ##
=======================================
Coverage 63.72% 63.72%
=======================================
Files 194 194
Lines 13740 13740
=======================================
Hits 8756 8756
Misses 4130 4130
Partials 854 854
Flags with carried forward coverage won't be shown. Click here to find out more.
|
1. Summary
Fixes issue in checking CL pool key validity in updateosmosisclpool callback.
CL pool keys are formed by prefix + []byte(pool_id.(string))
vs.
Gamm pool keys which are prefix + encode(uint64)
Gamm pool keys are always 9 bytes in length
CL pool keys are 2 bytes for pools 1-9, 3 bytes for pools 10-99, etc.
Key length check should reflect this.