feat(uibc): add missing params to MsgGovUpdateQuota #2370
Conversation
WalkthroughThe updates across the Umee protocol files reflect a significant enhancement of the 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? TipsChat with CodeRabbit Bot (
|
![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 filter (3)
- swagger/swagger.yaml
- x/uibc/tx.pb.go
- x/uibc/uibc.pb.go
Files selected for processing (10)
- proto/umee/uibc/v1/tx.proto (1 hunks)
- proto/umee/uibc/v1/uibc.proto (2 hunks)
- util/checkers/number.go (1 hunks)
- util/checkers/number_test.go (2 hunks)
- x/uibc/msg.go (2 hunks)
- x/uibc/msg_test.go (3 hunks)
- x/uibc/quota/intest/msg_server_test.go (1 hunks)
- x/uibc/quota/msg_server.go (1 hunks)
- x/uibc/quota/params.go (3 hunks)
- x/uibc/quota/params_test.go (1 hunks)
Additional comments: 16
proto/umee/uibc/v1/uibc.proto (1)
- 12-15: The addition of the
ICS20Memomessage with a repeated field of typegoogle.protobuf.Anyis correct and follows protobuf best practices for defining messages that can contain a variety of types. Ensure that the consuming code properly handles the dynamic nature ofAnytypes.x/uibc/quota/msg_server.go (1)
- 37-37: The refactoring to pass the entire
msgobject toUpdateQuotaParamsis a good practice as it simplifies the function signature and allows for easier extension in the future. Ensure that theUpdateQuotaParamsfunction is updated accordingly to handle themsgobject.util/checkers/number.go (2)
56-61: The
DecNotNegativefunction correctly checks if asdk.Decvalue is not negative. This is a good utility function that enhances code reusability.64-69: The
DecPositivefunction correctly checks if asdk.Decvalue is positive. This function follows Go best practices and improves the maintainability of the code.x/uibc/quota/params.go (2)
32-41: The refactoring of
UpdateQuotaParamsto accept aMsgGovUpdateQuotaobject simplifies the function signature and allows for more straightforward updates to the quota parameters. Ensure that all fields of theMsgGovUpdateQuotaare being validated and used correctly within the function.53-57: The validation logic within
validateEmergencyQuotaParamsUpdatecorrectly checks if the new parameters are not increased by the emergency group. This follows the business logic as described and ensures that emergency updates do not inadvertently raise quotas.x/uibc/msg.go (1)
- 28-45: The use of a slice to accumulate validation errors in
MsgGovUpdateQuotais a good practice as it allows for the collection of all errors before returning. This provides a more informative response to the caller. Ensure that the error messages are clear and that the validation logic is comprehensive.x/uibc/quota/params_test.go (2)
37-44: The
mkParamsfunction has been updated to include new quota parameters, which is necessary for testing the updated quota functionality. Ensure that the test cases cover a range of valid and invalid scenarios for these new parameters.48-65: The test cases have been correctly updated to include checks for the new quota parameters. It's important to ensure that the test cases cover both increases and decreases in quota parameters, as well as invalid updates that should be rejected.
util/checkers/number_test.go (1)
- 86-92: The
TestDecNotNegativefunction correctly tests theDecNotNegativeutility function with both negative and non-negativesdk.Decvalues. This ensures that the function behaves as expected.x/uibc/msg_test.go (2)
15-22: The
TestMsgGovUpdateQuotafunction has been updated to include new fields in thevalidMsgstruct, which is necessary for testing the updatedMsgGovUpdateQuotamessage. Ensure that the test cases cover a range of valid and invalid scenarios for these new fields.64-73: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [39-70]
The new test case for invalid
InflowOutflowQuotaBaseis a good addition to ensure that the validation logic for the new quota fields is working as expected. It's important to have comprehensive test coverage for all new validation rules.proto/umee/uibc/v1/tx.proto (1)
- 60-77: The new fields
inflow_outflow_quota_base,inflow_outflow_quota_rate, andinflow_outflow_token_quota_baseare correctly defined with sequential protobuf field numbers starting from 7. Ensure that the custom typegh.neting.cc/cosmos/cosmos-sdk/types.Decis the appropriate choice for representing these quota values, as they are defined as strings but expected to hold decimal values.x/uibc/quota/intest/msg_server_test.go (3)
24-35: The test case "unauthorized to increase the quota" has been updated with the new quota fields and the error message has been changed to "unauthorized". Ensure that this error message accurately reflects the expected outcome of the test, especially in the context of the new quota parameters.
40-49: In the test case "invalid quota in msg", the
Totalfield has been changed fromsdk.NewDec(100)tosdk.NewDec(10000), and new quota fields have been added. Verify that the error message "total quota must be greater than or equal to per_denom quota" is still valid in the context of the updatedTotalfield and the new quota parameters.70-77: The last test case "valid update the new params again" has an updated
QuotaDurationfield with a significantly longer duration. Confirm that this new duration is intentional and correctly tests the functionality of the quota duration parameter.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2370 +/- ##
==========================================
- Coverage 75.38% 70.07% -5.32%
==========================================
Files 100 174 +74
Lines 8025 13009 +4984
==========================================
+ Hits 6050 9116 +3066
- Misses 1589 3288 +1699
- Partials 386 605 +219
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- x/uibc/msg.go (2 hunks)
- x/uibc/msg_test.go (3 hunks)
Additional comments: 5
x/uibc/msg.go (3)
5-5: The addition of the
errorspackage is appropriate for the updated error handling logic in theValidateBasicmethod.28-46: The refactoring of the
ValidateBasicmethod to accumulate errors is a good practice, allowing for more comprehensive error reporting. Ensure that all calls toDecPositiveandDecNotNegativecorrectly pass theerrsslice for error accumulation.46-46: Verify that the
errors.Joinfunction is correctly implemented and available within the project's dependencies, as it is not part of the standard Go library.x/uibc/msg_test.go (2)
15-22: The addition of new fields
InflowOutflowQuotaBase,InflowOutflowTokenQuotaBase, andInflowOutflowQuotaRateto thevalidMsgstruct in theTestMsgGovUpdateQuotafunction aligns with the updated message structure.64-73: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [39-70]
The new test case "invalid inflow outflow quota base with respect to per denom" correctly tests the validation logic for the
InflowOutflowTokenQuotaBasefield. Ensure that the error message is complete and matches the expected validation error.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- util/checkers/number.go (1 hunks)
- x/uibc/msg.go (2 hunks)
Additional comments: 3
util/checkers/number.go (2)
56-61: The function
DecNotNegativecorrectly checks if thesdk.Decvalue is not negative. However, the check forIsNilmay not be necessary ifsdk.Deccannot be nil. Confirm ifsdk.Dechas a nil state.64-69: The function
DecPositivecorrectly checks if thesdk.Decvalue is positive. Similar toDecNotNegative, ensure thatsdk.Deccan have a nil state before checking withIsNil.x/uibc/msg.go (1)
- 28-45: The refactoring of the
ValidateBasicmethod to accumulate errors usingerrors.Joinis a good practice as it allows for the collection of all validation errors. Ensure that theerrorspackage supports theJoinfunction as expected.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- swagger/swagger.yaml
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- util/checkers/number_test.go (2 hunks)
Additional comments: 6
util/checkers/number_test.go (2)
6-6: The addition of the Cosmos SDK import statement is necessary for the new test cases that involve SDK types.
86-92: The new test function
TestDecNotNegativecorrectly tests for non-negativity of decimal numbers using theDecNotNegativefunction.CHANGELOG.md (4)
51-53: New queries and handlers have been added as per the PR objectives and AI-generated summaries.
58-58: The missing parameters for
uic/MsgGovUpdateQuotahave been documented, aligning with the PR's objectives.62-62: The order of execution for metoken endblocker and oracle has been adjusted, which is a bug fix and should improve the system's reliability.
63-63: The inflow amount to registered tokens has been fixed to prevent the inflow amount from being overridden, as mentioned in the PR objectives.
Description
Summary by CodeRabbit
New Features
MsgGovUpdateQuotato include additional quota management fields.ICS20Memomessage with support for multiple message types.Improvements
MsgGovUpdateQuotafor better error handling.Bug Fixes
Documentation