Remove logic of FundingRateToIndex
#1528
Conversation
WalkthroughThe recent updates involve enhancements across various packages in the codebase. Changes include refining arithmetic functions with big integers for precision, introducing new functions for calculations, and improving testing coverage for mathematical operations. Changes
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 (3)
- protocol/Makefile (2 hunks)
- protocol/lib/quantums.go (4 hunks)
- protocol/lib/quantums_test.go (1 hunks)
Additional Context Used
Path-based Instructions (3)
protocol/lib/quantums.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/lib/quantums_test.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/Makefile (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (7)
protocol/lib/quantums.go (3)
32-32: No issues found inBaseToQuoteQuantums.
Line range hint
55-55: No issues found inQuoteToBaseQuantums.
Line range hint
87-113: Performance improvements inMultiplyByPricelook good. Ensure all usages are updated to the new public function.Verification successful
All usages of the
MultiplyByPricefunction have been updated to match the new public function signature.
protocol/lib/quantums_test.goprotocol/lib/quantums.goScripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `MultiplyByPrice` match the new public function. # Test: Search for the function usage. Expect: Only occurrences of the new public function. rg --type go $'MultiplyByPrice'Length of output: 409
Script:
#!/bin/bash # Extract lines around the function calls in the test file and the main file to verify the usage of MultiplyByPrice. # Extract lines around the function call in the test file rg --type go --context 5 'MultiplyByPrice' protocol/lib/quantums_test.go # Extract lines around the function call in the main file rg --type go --context 5 'MultiplyByPrice' protocol/lib/quantums.goLength of output: 1819
protocol/lib/quantums_test.go (3)
Line range hint
14-14: No issues found inTestBaseToQuoteQuantums.
Line range hint
123-123: No issues found inTestQuoteToBaseQuantums.
224-238: No issues found inBenchmarkMultiplyByPrice.protocol/Makefile (1)
234-234: No issues found in thebenchmarktarget.
BrendanChou
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- protocol/Makefile (2 hunks)
- protocol/lib/big_math.go (1 hunks)
- protocol/lib/big_math_test.go (1 hunks)
- protocol/lib/quantums.go (4 hunks)
- protocol/lib/quantums_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- protocol/Makefile
- protocol/lib/quantums.go
- protocol/lib/quantums_test.go
Additional Context Used
Path-based Instructions (2)
protocol/lib/big_math.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/lib/big_math_test.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- protocol/Makefile (2 hunks)
- protocol/lib/big_math.go (1 hunks)
- protocol/lib/big_math_test.go (1 hunks)
- protocol/lib/quantums.go (4 hunks)
- protocol/lib/quantums_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- protocol/Makefile
- protocol/lib/big_math.go
- protocol/lib/big_math_test.go
- protocol/lib/quantums.go
- protocol/lib/quantums_test.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- protocol/lib/big_math.go (1 hunks)
- protocol/lib/big_math_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/lib/big_math.go
- protocol/lib/big_math_test.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- protocol/lib/big_math.go (1 hunks)
- protocol/lib/quantums.go (4 hunks)
- protocol/lib/quantums_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- protocol/lib/big_math.go
- protocol/lib/quantums.go
- protocol/lib/quantums_test.go
BrendanChou
changed the title
MultiplyByPrice and BigRatMulPpm
BrendanChou
changed the title
MultiplyByPrice and BigRatMulPpmFundingRateToIndex
BrendanChou
force-pushed
the
bc/bench
branch
3 times, most recently
from
aa19716 to
a10ee8d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- protocol/lib/quantums.go (1 hunks)
- protocol/testutil/perpetuals/perpetuals.go (1 hunks)
- protocol/x/perpetuals/funding/funding.go (1 hunks)
- protocol/x/perpetuals/funding/funding_test.go (1 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/lib/quantums.go
Additional Context Used
Path-based Instructions (4)
protocol/x/perpetuals/funding/funding.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/perpetuals/funding/funding_test.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/testutil/perpetuals/perpetuals.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/perpetuals/keeper/perpetual.go (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (5)
protocol/x/perpetuals/funding/funding.go (1)
11-46: The implementation ofGetFundingIndexDeltaappears correct and efficient. Consider addressing the TODO regarding the 8-hour funding rate period configurability in future iterations.protocol/x/perpetuals/funding/funding_test.go (1)
15-88: The unit tests forGetFundingIndexDeltaare comprehensive and well-structured. Good use of table-driven tests to cover various scenarios.protocol/testutil/perpetuals/perpetuals.go (1)
47-50: The addition ofWithAtomicResolutionfollows the established pattern of perpetual modifier options and is implemented correctly.protocol/x/perpetuals/keeper/perpetual.go (2)
734-753: The changes from lines 734 to 753 introduce a new method for updating the funding index based on the new funding rate. This method retrieves the market price, calculates the funding index delta, and updates the funding index in the state. Ensure that the error handling and the logic for updating the funding index are thoroughly tested, especially since this involves financial calculations which are critical to the system's integrity.Verification successful
The integration of the new funding index update logic appears to be well-covered in both the main code and the test cases. The test cases include scenarios for success, non-existent perpetual, and integer overflow/underflow, indicating thorough testing.
perpetual.goreferencesGetFundingIndexDeltaandModifyFundingIndexwith proper error handling.perpetual_test.goincludes multiple test cases forModifyFundingIndex.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new funding index update logic with the rest of the system. # Test: Search for the usage of `ModifyFundingIndex` and `GetFundingIndexDelta`. Expect: Proper integration and error handling. rg --type go $'ModifyFundingIndex|GetFundingIndexDelta' ./protocol/x/perpetuals/keeper/Length of output: 1268
1065-1065: Ensure that the negation ofbigNetSettlementPpmis always correct, as this affects financial calculations directly. Consider adding a unit test specifically for this logic to verify its correctness under various scenarios.Verification successful
The negation logic for
bigNetSettlementPpmin theGetSettlementPpmWithPerpetualfunction is correctly implemented. The function ensures thatbigNetSettlementPpmis always negated, as required by the financial calculations.
- The function
GetSettlementPpmWithPerpetualis located inprotocol/x/perpetuals/keeper/perpetual.go.- The negation logic is explicitly handled with the line
bigNetSettlementPpm = bigNetSettlementPpm.Neg(bigNetSettlementPpm).Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the negation logic in `GetSettlementPpmWithPerpetual`. # Test: Search for the usage of `GetSettlementPpmWithPerpetual`. Expect: Correct implementation of negation logic. rg --type go $'GetSettlementPpmWithPerpetual' ./protocol/x/perpetuals/keeper/Length of output: 271
Script:
#!/bin/bash # Extract the implementation of the function `GetSettlementPpmWithPerpetual` for review. ast-grep --lang go --pattern $'func GetSettlementPpmWithPerpetual($_, $_, $_, $_) ($_, $_) { $$$ }' ./protocol/x/perpetuals/keeper/Length of output: 134
Script:
#!/bin/bash # Extract the implementation of the function `GetSettlementPpmWithPerpetual` for review with a more flexible pattern. ast-grep --lang go --pattern $'func GetSettlementPpmWithPerpetual($$$) { $$$ }' ./protocol/x/perpetuals/keeper/Length of output: 114
Script:
#!/bin/bash # Locate the function definition of `GetSettlementPpmWithPerpetual` and extract the surrounding lines for review. rg --type go --context 20 'func GetSettlementPpmWithPerpetual' ./protocol/x/perpetuals/keeper/Length of output: 3204
Changelist
keeper.getFundingIndexDelta()to a stateless helper function,GetFundingIndexDelta(), which replicates the old behavior of the function except for:MaybeProcessNewFundingTickEpoch()multiplyByPrice()andFundingRateToIndex()inlib/quantums.goand isolates the code to thex/perpetualsmoduleTest Plan
MaybeProcessNewFundingTickEpoch()Summary by CodeRabbit
New Features
AtomicResolutionparameter in thePerpetualstruct.Bug Fixes
MultiplyByPricefunction.Tests