-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add update params endpoint for ibcratelimit #1984
Add update params endpoint for ibcratelimit #1984
Conversation
WalkthroughThe recent updates focus on deprecating 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? 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: 3
Outside diff range and nitpick comments (2)
CHANGELOG.md (2)
Line range hint
108-354
: Please ensure consistent use of list markers in markdown. Replace dashes with asterisks for unordered lists to maintain consistency throughout the document.- - Bump cosmos-SDK to `v0.50.2` (from `v0.46.13-pio-2`) [#1772](https://github.com/provenance-io/provenance/issues/1772). + * Bump cosmos-SDK to `v0.50.2` (from `v0.46.13-pio-2`) [#1772](https://github.com/provenance-io/provenance/issues/1772).
Line range hint
228-228
: Consider using markdown links instead of bare URLs to improve the readability and appearance of the document.- [#1772](https://github.com/provenance-io/provenance/issues/1772) + [#1772](https://github.com/provenance-io/provenance/issues/1772)Also applies to: 259-259, 358-358, 408-408, 420-420, 436-436, 507-507, 518-518, 526-526, 562-562, 587-587, 599-599, 645-649, 696-696, 737-737, 790-790, 903-903
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/ibcratelimit/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (12)
- CHANGELOG.md (1 hunks)
- app/params/weights.go (1 hunks)
- client/docs/swagger-ui/swagger.yaml (1 hunks)
- proto/provenance/attribute/v1/tx.proto (1 hunks)
- proto/provenance/ibcratelimit/v1/tx.proto (1 hunks)
- proto/provenance/marker/v1/tx.proto (1 hunks)
- x/ibcratelimit/client/cli/tx.go (1 hunks)
- x/ibcratelimit/keeper/msg_server.go (3 hunks)
- x/ibcratelimit/msgs.go (2 hunks)
- x/ibcratelimit/msgs_test.go (2 hunks)
- x/ibcratelimit/simulation/operations.go (1 hunks)
- x/ibcratelimit/simulation/operations_test.go (2 hunks)
Files skipped from review due to trivial changes (2)
- proto/provenance/attribute/v1/tx.proto
- proto/provenance/marker/v1/tx.proto
Additional comments not posted (18)
x/ibcratelimit/msgs.go (2)
13-13
: AddedMsgUpdateParamsRequest
to the list of request messages. Ensure this addition is reflected wherever request messages are handled or dispatched.
21-23
: Correct implementation for creating a newMsgUpdateParamsRequest
. Ensure theNewParams
function correctly initializes parameters based on theratelimiter
string.Verification successful
The
NewParams
function inx/ibcratelimit/params.go
correctly initializes theParams
struct with thecontractAddress
parameter. This confirms that the initialization logic is correctly implemented.
x/ibcratelimit/params.go
:func NewParams(contractAddress string) Params { return Params{ ContractAddress: contractAddress, }Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct initialization of parameters in `NewParams` function. # Test: Search for the implementation of `NewParams`. Expect: Correct initialization logic. ast-grep --lang go --pattern $'func NewParams($_) { $$$ }'Length of output: 61
Script:
#!/bin/bash # Description: Verify the correct initialization of parameters in `NewParams` function. # Step 1: Search for the `NewParams` function in the codebase. rg 'func NewParams' --glob '*.go' # Step 2: If found, display the implementation of `NewParams` function. # Note: Adjust the file path and line numbers based on the search result. # Example: If found in file `params.go` at line 10, use `sed -n '10,20p' params.go` # This is a placeholder command and will be adjusted based on the actual search result. sed -n '10,30p' path/to/params.goLength of output: 508
Script:
#!/bin/bash # Description: Display the implementation of `NewParams` function in `x/ibcratelimit/params.go`. # Extract and display the implementation of `NewParams` function from `x/ibcratelimit/params.go`. sed -n '/func NewParams/,/}/p' x/ibcratelimit/params.goLength of output: 160
x/ibcratelimit/keeper/msg_server.go (1)
Line range hint
30-40
: Implementation ofUpdateParams
appears correct. Validate thatValidateAuthority
,SetParams
, andemitEvent
functions are implemented correctly and handle all edge cases.app/params/weights.go (1)
44-44
: Renaming ofDefaultWeightGovUpdateParams
toDefaultWeightUpdateParams
aligns with the new functionality. Ensure all references to this constant are updated accordingly.Verification successful
All references to the constant
DefaultWeightGovUpdateParams
have been successfully updated toDefaultWeightUpdateParams
.
- Verified occurrences in the following files:
x/sanction/simulation/operations.go
x/sanction/simulation/operations_test.go
x/ibcratelimit/simulation/operations_test.go
x/ibcratelimit/simulation/operations.go
app/params/weights.go
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all references to the old constant `DefaultWeightGovUpdateParams` are updated. # Test: Search for the usage of `DefaultWeightGovUpdateParams`. Expect: No occurrences. rg --type go $'DefaultWeightGovUpdateParams'Length of output: 43
Script:
#!/bin/bash # Description: Verify that the new constant `DefaultWeightUpdateParams` is used correctly throughout the codebase. # Test: Search for the usage of `DefaultWeightUpdateParams`. Expect: Multiple occurrences. rg --type go $'DefaultWeightUpdateParams'Length of output: 689
x/ibcratelimit/client/cli/tx.go (1)
52-52
: The CLI command for updating parameters is correctly implemented using the new message type. Confirm that theGenerateOrBroadcastTxCLIAsGovProp
function handles the message correctly.x/ibcratelimit/msgs_test.go (3)
18-18
: AddedMsgUpdateParamsRequest
to the list of message makers for signers. Ensure this addition is reflected in all relevant tests.
25-29
: The test for creating a newMsgUpdateParamsRequest
is implemented correctly. Ensure that theNewParams
function is tested thoroughly to handle differentratelimiter
configurations.Verification successful
The
NewParams
function is thoroughly tested across various test files, covering different configurations and scenarios.
x/ibcratelimit/params_test.go
x/ibcratelimit/simulation/genesis_test.go
x/ibcratelimit/keeper/msg_server_test.go
x/ibcratelimit/keeper/keeper_test.go
x/ibcratelimit/keeper/grpc_query_test.go
x/ibcratelimit/genesis_test.go
x/ibcratelimit/client/cli/cli_test.go
x/ibcratelimit/keeper/rate_limit_test.go
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the thorough testing of `NewParams` function. # Test: Search for the tests of `NewParams`. Expect: Comprehensive tests covering various configurations. rg --type go $'NewParams'Length of output: 5672
Line range hint
33-66
: The validation tests forMsgUpdateParamsRequest
are comprehensive. Ensure that all edge cases are covered, especially for invalid authority and contract values.Verification successful
The validation tests for
MsgUpdateParamsRequest
inmsgs_test.go
include the following functions:
TestAllMsgsGetSigners
TestNewMsgGovUpdateParamsRequest
TestNewMsgUpdateParamsValidateBasic
The
TestNewMsgUpdateParamsValidateBasic
function already covers various edge cases for valid and invalid authority and contract values. Based on the available information, it appears that the existing tests are comprehensive.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all edge cases are covered in the validation tests for `MsgUpdateParamsRequest`. # Test: Search for additional tests covering edge cases. Expect: Comprehensive coverage. rg --type go $'MsgUpdateParamsRequest' -A 10 -B 10Length of output: 31089
Script:
#!/bin/bash # Description: Verify that all edge cases are covered in the validation tests for `MsgUpdateParamsRequest`. # Test: Search for additional tests covering edge cases. Expect: Comprehensive coverage. rg --type go 'Test.*MsgUpdateParamsRequest' -A 10 -B 10Length of output: 57
Script:
#!/bin/bash # Description: Verify that all edge cases are covered in the validation tests for `MsgUpdateParamsRequest`. # Test: List all test functions in msgs_test.go to ensure comprehensive coverage. rg --type go '^func Test' x/ibcratelimit/msgs_test.goLength of output: 209
proto/provenance/ibcratelimit/v1/tx.proto (5)
17-20
: MarkedGovUpdateParams
as deprecated and introducedUpdateParams
as its replacement.
22-23
: Introduced new RPC endpointUpdateParams
for updating parameters.
27-35
: DeprecatedMsgGovUpdateParamsRequest
and added appropriate deprecation annotations.
40-43
: DeprecatedMsgGovUpdateParamsResponse
with appropriate annotations.
45-54
: Introduced new message typesMsgUpdateParamsRequest
andMsgUpdateParamsResponse
for the newUpdateParams
endpoint.x/ibcratelimit/simulation/operations.go (3)
40-40
: Updated simulation weight to use the newDefaultWeightUpdateParams
.
43-43
: Added a new weighted operation for theUpdateParams
simulation.
47-61
: Implemented theSimulateMsgUpdateParams
function to simulate sending aMsgUpdateParamsRequest
.x/ibcratelimit/simulation/operations_test.go (2)
82-82
: Updated test to include the newMsgUpdateParamsRequest
operation with the correct weight.
110-122
: Implemented a test for theSimulateMsgUpdateParams
operation, ensuring it handles the operation correctly and logs appropriate messages.
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/ibcratelimit/keeper/msg_server.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/ibcratelimit/keeper/msg_server.go
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/ibcratelimit/keeper/msg_server_test.go (2 hunks)
Additional comments not posted (5)
x/ibcratelimit/keeper/msg_server_test.go (5)
Line range hint
11-43
: Ensure comprehensive test coverage for the newUpdateParams
endpoint.The test cases provided seem to cover both success and failure scenarios effectively. It's good to see that the tests check for the correct number of events and the correct error messages. However, consider adding more edge cases if applicable, such as invalid parameters or unauthorized access scenarios.
23-23
: Test case for authority mismatch is well-implemented.The dynamic construction of the error message using
fmt.Sprintf
is a good practice as it adapts to changes in the expected authority value.
32-36
: Test case for successful update is correctly implemented.The initialization of the response and event objects is done correctly, and the assertions ensure that the expected outcomes are verified.
43-43
: Proper use of test case encapsulation and execution.Using
s.Run
for running each test case provides isolation and clarity in test results, which is beneficial for debugging and understanding test outcomes.
Line range hint
58-62
: Helper functiontypedEventToEvent
is implemented correctly.The straightforward use of
TypedEventToEvent
and the handling of the conversion process are correctly implemented. This utility function aids in simplifying event handling in tests.
Description
Adds
UpdateParams
rpc endpoint and deprecates/disablesGovUpdateParams
endpoint.Params
query endpoint was already present.closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passesSummary by CodeRabbit
Summary by CodeRabbit
New Features
UpdateParams
andParams
query endpoints in theibcratelimit
module.Deprecations
GovUpdateParams
in favor ofUpdateParams
across the platform.Documentation
UpdateParams
endpoint.Bug Fixes
Refactor
GovUpdateParams
toUpdateParams
for clarity and consistency.