feat: make inflation params a collection and add commands to update them #1776
Conversation
|
Warning Rate Limit Exceeded@matthiasmatt has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 26 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe inflation module of a blockchain application has undergone a comprehensive update. The changes focus on enhancing the parameter management system, introducing new gRPC services for inflation control, and refining the keeper's functionality. The module now supports dynamic inflation toggling and parameter editing, with improved permission checks and test coverage to ensure robust operation. Changes
Poem
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
x/inflation/types/tx.pb.gois excluded by:!**/*.pb.gox/inflation/types/tx.pb.gw.gois excluded by:!**/*.pb.gw.go
Files selected for processing (17)
- proto/nibiru/inflation/v1/tx.proto (1 hunks)
- x/inflation/genesis.go (1 hunks)
- x/inflation/keeper/grpc_query.go (2 hunks)
- x/inflation/keeper/grpc_query_test.go (1 hunks)
- x/inflation/keeper/hooks_test.go (2 hunks)
- x/inflation/keeper/inflation_test.go (3 hunks)
- x/inflation/keeper/keeper.go (3 hunks)
- x/inflation/keeper/msg_server.go (1 hunks)
- x/inflation/keeper/params.go (1 hunks)
- x/inflation/keeper/sudo.go (1 hunks)
- x/inflation/keeper/sudo_test.go (1 hunks)
- x/inflation/module.go (2 hunks)
- x/inflation/types/codec.go (1 hunks)
- x/inflation/types/interfaces.go (1 hunks)
- x/inflation/types/msgs.go (1 hunks)
- x/inflation/types/params.go (2 hunks)
- x/inflation/types/params_test.go (2 hunks)
Files skipped from review due to trivial changes (1)
- x/inflation/types/params_test.go
Additional comments: 26
x/inflation/genesis.go (1)
- 24-24: The
InitGenesisfunction now usesk.UpdateParamsinstead ofk.SetParams. This change is consistent with the PR's objective to refactor parameter handling.x/inflation/keeper/params.go (2)
- 9-10: The
UpdateParamsmethod has been added to theKeeperstruct to handle parameter updates. Ensure that theParamsfield is properly initialized and that theUpdateParamsmethod is correctly implemented.- 13-15: The
GetParamsmethod has been modified to retrieve parameters using theParamsfield. Ensure that theParamsfield is correctly populated during initialization and that theGetParamsmethod is retrieving the correct data.x/inflation/keeper/msg_server.go (2)
- 21-34: The
EditInflationParamsmethod has been added to handle editing inflation parameters. Ensure that the method correctly updates the parameters and that appropriate permission checks are in place.- 36-46: The
ToggleInflationmethod has been added to enable or disable inflation. Ensure that the method correctly updates the inflation status and that appropriate permission checks are in place.x/inflation/types/codec.go (2)
- 11-16: The
RegisterLegacyAminoCodecfunction has been updated to register new message types. Ensure that the message types are correctly registered and that the codec is properly initialized.- 18-25: The
RegisterInterfacesfunction has been updated to register new interface implementations. Ensure that the interface types are correctly registered and that the interface registry is properly initialized.x/inflation/types/interfaces.go (1)
- 45-45: A new method
CheckPermissionshas been added to theSudoKeeperinterface. Ensure that this method is implemented correctly and that it is used appropriately to check permissions.x/inflation/types/msgs.go (2)
- 19-38: The
MsgEditInflationParamsmessage type has been added with methods to satisfy thesdk.Msginterface. Ensure that the methods are correctly implemented and that the message type is properly used.- 49-75: The
MsgToggleInflationmessage type has been added with methods to satisfy thesdk.Msginterface. Ensure that the methods are correctly implemented and that the message type is properly used.proto/nibiru/inflation/v1/tx.proto (1)
- 11-21: New RPC methods
ToggleInflationandEditInflationParamshave been defined in thetx.protofile. Ensure that these methods are correctly implemented in the corresponding server and that the RPC endpoints are properly exposed.x/inflation/keeper/keeper.go (1)
- 30-30: The
Keeperstruct now includes a new fieldParamsfor inflation parameters. Ensure that this field is properly initialized in theNewKeeperfunction and that it is used correctly throughout the module.x/inflation/keeper/grpc_query_test.go (1)
- 98-98: The test for the
Paramsmethod has been updated to use the newParamsfield. Ensure that the test correctly retrieves the parameters and that the expected behavior is validated.x/inflation/keeper/sudo.go (2)
- 27-46: The
EditInflationParamsmethod in thesudoExtensionstruct has been added to handle administrative updates to inflation parameters. Ensure that the method correctly merges and validates the new parameters.- 48-63: The
ToggleInflationmethod in thesudoExtensionstruct has been added to enable or disable inflation. Ensure that the method correctly updates the inflation status and that the change is persisted.x/inflation/keeper/grpc_query.go (1)
- 12-35: The
queriertype has been introduced, and theParamsfunction has been moved from theKeepertype to the newqueriertype. Ensure that thequeriertype is correctly implemented and that theParamsfunction is correctly moved.x/inflation/types/params.go (1)
- 5-10: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-90]
Functions related to parameter key tables and parameter set pairs have been removed from
params.go. Ensure that the removal of these functions does not affect the module's ability to manage parameters correctly.x/inflation/keeper/hooks_test.go (1)
- 18-24: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-157]
The
hooks_test.gofile has been updated to use theUpdateParamsmethod instead ofSetParams. Ensure that the tests are updated accordingly and that they correctly test the new behavior.x/inflation/keeper/sudo_test.go (3)
- 22-69: The
MergeInflationParamsfunction is tested to ensure it correctly merges new parameters into the existing ones. Verify that the test covers all scenarios and that the merged parameters are validated.- 71-116: The
EditInflationParamsmethod is tested to ensure it correctly updates the inflation parameters. Verify that the test checks for all parameter changes and that the updated parameters are validated.- 118-133: The
ToggleInflationmethod is tested to ensure it correctly enables and disables inflation. Verify that the test checks the updated inflation status and that the changes are persisted.x/inflation/module.go (2)
- 51-53: The
RegisterInterfacesmethod has been updated to register the module's interface types. Ensure that the interface types are correctly registered and that the interface registry is properly initialized.- 119-123: The
RegisterServicesmethod has been updated to register module services and utilize new server implementations. Ensure that the services are correctly registered and that the new server implementations are properly utilized.x/inflation/keeper/inflation_test.go (3)
- 140-140: The
UpdateParamsmethod is used correctly in place ofSetParams. Ensure that the new method's behavior aligns with the test expectations.- 155-155: The
UpdateParamsmethod is used correctly in place ofSetParams. Verify that the behavior ofUpdateParamsis consistent with the intended logic of the test case.- 165-165: The
UpdateParamsmethod is used correctly in place ofSetParams. Confirm that theUpdateParamsmethod performs as expected within the test's context.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1776 +/- ##
==========================================
- Coverage 73.13% 73.08% -0.05%
==========================================
Files 192 196 +4
Lines 15472 15566 +94
==========================================
+ Hits 11315 11376 +61
- Misses 3496 3526 +30
- Partials 661 664 +3
|
…nibiru into mat/change-inflation-params ng with '#' will be ignored, and an empty message aborts
Co-authored-by: Unique Divine <[email protected]>
Co-authored-by: Unique Divine <[email protected]>
Co-authored-by: Unique Divine <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- x/inflation/keeper/grpc_query.go (2 hunks)
Additional comments: 3
x/inflation/keeper/grpc_query.go (3)
- 12-15: The introduction of the
querierstruct, which embeds theKeeperstruct, is a good design choice for separating query handling logic from the main keeper logic. This change enhances modularity and clarity in the codebase.- 17-21: The
NewQuerierfunction correctly instantiates aquerierwith the providedKeeper. This approach ensures that the query handling logic has access to all necessary keeper functionalities.- 9-39: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [37-57]
The
Period,EpochMintProvision,SkippedEpochs,InflationRate, andCirculatingSupplyfunctions remain within theKeepertype, which is appropriate as they pertain to core business logic rather than query handling. Ensure that their implementations are consistent with the new design and that they interact correctly with the updatedqueriertype where necessary.
…nibiru into mat/change-inflation-params t push# the commit.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- CHANGELOG.md (1 hunks)
- x/inflation/keeper/grpc_query.go (2 hunks)
- x/inflation/keeper/grpc_query_test.go (2 hunks)
- x/inflation/keeper/msg_server_test.go (1 hunks)
- x/inflation/keeper/sudo.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- CHANGELOG.md
- x/inflation/keeper/grpc_query_test.go
- x/inflation/keeper/msg_server_test.go
- x/inflation/keeper/sudo.go
Additional comments: 4
x/inflation/keeper/grpc_query.go (4)
- 12-15: The introduction of the
querierstruct, which embeds theKeeperstruct, is a good design choice for separating query handling logic from the mainKeeperlogic. This change enhances modularity and clarity in the codebase.- 17-21: The
NewQuerierfunction correctly instantiates aquerierwith the providedKeeper. This approach ensures that thequerierhas access to all methods ofKeeper, facilitating a clean separation of concerns between querying and state management.- 26-31: Moving the
Paramsfunction to thequeriertype aligns with the objective of separating query handling from the mainKeeperlogic. This change is consistent with the overall design goal of enhancing modularity and clarity. Ensure that all references to theParamsfunction throughout the codebase are updated to use thequeriertype.- 9-34: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [33-65]
The retention of functions such as
Period,EpochMintProvision,SkippedEpochs,InflationRate, andCirculatingSupplywithin theKeepertype is appropriate, as these functions pertain to the core responsibilities of theKeeper. However, ensure that the logic within these functions does not inadvertently depend on the now-removedParamsfunction from theKeepertype, which could lead to issues.Verification successful
The executed script aimed to verify if the retained functions
Period,EpochMintProvision,SkippedEpochs, andInflationRatein theKeepertype depend on a potentially removedParamsfunction. The script did not produce any output, indicating that these functions do not call aParamsmethod within their bodies. This suggests that the concern raised in the review comment about these functions inadvertently depending on a removedParamsfunction is unfounded based on the script's findings.Given this information, it can be concluded that the functions mentioned in the review comment do not depend on the
Paramsfunction, aligning with the initial approval of retaining these functions within theKeepertype without issues related to the removal of theParamsfunction.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the retained functions do not depend on the removed Params function. ast-grep --lang go --pattern $'func (k Keeper) $_($_, $_) { $$$ k.Params($_) $$$ }'Length of output: 90
…hem (#1776) * feat: make inflation params a collection and add commands to update them * fix: more tests * chore: changelog * Update x/inflation/keeper/grpc_query.go Co-authored-by: Unique Divine <[email protected]> * Update x/inflation/keeper/grpc_query.go Co-authored-by: Unique Divine <[email protected]> * Update x/inflation/keeper/sudo.go Co-authored-by: Unique Divine <[email protected]> * fix: fix tests and improve function * fix: add test for queryserver --------- Co-authored-by: Unique Divine <[email protected]>
Purpose / Abstract
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Documentation
Tests