-
Notifications
You must be signed in to change notification settings - Fork 41
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 UpdateParams
rpc and Params
query to ibchooks module
#2006
Add UpdateParams
rpc and Params
query to ibchooks module
#2006
Conversation
WalkthroughThe recent updates introduce new query RPC endpoints ( Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant CLI
participant IBC-Hooks Module
participant Keeper
participant MsgServer
User->>CLI: Execute UpdateParams Command
CLI->>IBC-Hooks Module: Send UpdateParams Request
IBC-Hooks Module->>Keeper: Validate Authority
Keeper-->>IBC-Hooks Module: Authority Validated
IBC-Hooks Module->>MsgServer: Forward UpdateParams Request
MsgServer->>Keeper: Update Params
Keeper-->>MsgServer: Params Updated
MsgServer-->>IBC-Hooks Module: UpdateParams Response
IBC-Hooks Module-->>CLI: Response to User
CLI-->>User: Display UpdateParams Result
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 (
|
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: 1
Outside diff range and nitpick comments (4)
CHANGELOG.md (4)
Line range hint
831-831
: Multiple consecutive blank lines were found. It's a good practice to limit to one blank line to separate sections for better readability and to maintain a clean codebase.Also applies to: 1232-1232, 1249-1249, 1301-1301, 1405-1405
Line range hint
242-242
: URLs are used directly in the text. Consider using Markdown link syntax to make the document cleaner and links clickable.Also applies to: 273-273, 372-372, 422-422, 434-434, 450-450, 521-521, 532-532, 540-540, 576-576, 601-601, 613-613, 659-663, 710-710, 751-751, 804-804, 917-917
Line range hint
959-959
: Spaces found inside emphasis markers. This can lead to rendering issues in some Markdown parsers. It's best to remove any leading or trailing spaces inside emphasis markers.
Line range hint
355-355
: Spaces found inside code span elements. This can lead to rendering issues in some Markdown parsers. It's best to remove any leading or trailing spaces inside code span elements.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
x/ibchooks/types/event.pb.go
is excluded by!**/*.pb.go
x/ibchooks/types/query.pb.go
is excluded by!**/*.pb.go
x/ibchooks/types/query.pb.gw.go
is excluded by!**/*.pb.gw.go
x/ibchooks/types/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (19)
- CHANGELOG.md (1 hunks)
- client/docs/swagger-ui/swagger.yaml (1 hunks)
- proto/provenance/ibchooks/v1/event.proto (1 hunks)
- proto/provenance/ibchooks/v1/query.proto (1 hunks)
- proto/provenance/ibchooks/v1/tx.proto (2 hunks)
- x/ibchooks/client/cli/cli_test.go (1 hunks)
- x/ibchooks/client/cli/query.go (3 hunks)
- x/ibchooks/client/cli/tx.go (1 hunks)
- x/ibchooks/keeper/keeper.go (4 hunks)
- x/ibchooks/keeper/msg_server.go (1 hunks)
- x/ibchooks/keeper/msg_server_test.go (1 hunks)
- x/ibchooks/keeper/query_server.go (1 hunks)
- x/ibchooks/keeper/query_server_test.go (1 hunks)
- x/ibchooks/module.go (2 hunks)
- x/ibchooks/types/msgs.go (1 hunks)
- x/ibchooks/types/msgs_test.go (1 hunks)
- x/ibcratelimit/module/module.go (1 hunks)
- x/ibcratelimit/simulation/operations.go (2 hunks)
- x/sanction/simulation/operations.go (1 hunks)
Files skipped from review due to trivial changes (1)
- x/ibcratelimit/simulation/operations.go
Additional Context Used
Markdownlint (109)
CHANGELOG.md (109)
120: Expected: asterisk; Actual: dash
Unordered list style
121: Expected: asterisk; Actual: dash
Unordered list style
122: Expected: asterisk; Actual: dash
Unordered list style
123: Expected: asterisk; Actual: dash
Unordered list style
124: Expected: asterisk; Actual: dash
Unordered list style
125: Expected: asterisk; Actual: dash
Unordered list style
126: Expected: asterisk; Actual: dash
Unordered list style
127: Expected: asterisk; Actual: dash
Unordered list style
128: Expected: asterisk; Actual: dash
Unordered list style
129: Expected: asterisk; Actual: dash
Unordered list style
130: Expected: asterisk; Actual: dash
Unordered list style
131: Expected: asterisk; Actual: dash
Unordered list style
132: Expected: asterisk; Actual: dash
Unordered list style
133: Expected: asterisk; Actual: dash
Unordered list style
134: Expected: asterisk; Actual: dash
Unordered list style
135: Expected: asterisk; Actual: dash
Unordered list style
136: Expected: asterisk; Actual: dash
Unordered list style
137: Expected: asterisk; Actual: dash
Unordered list style
207: Expected: asterisk; Actual: dash
Unordered list style
208: Expected: asterisk; Actual: dash
Unordered list style
209: Expected: asterisk; Actual: dash
Unordered list style
210: Expected: asterisk; Actual: dash
Unordered list style
211: Expected: asterisk; Actual: dash
Unordered list style
212: Expected: asterisk; Actual: dash
Unordered list style
213: Expected: asterisk; Actual: dash
Unordered list style
214: Expected: asterisk; Actual: dash
Unordered list style
215: Expected: asterisk; Actual: dash
Unordered list style
216: Expected: asterisk; Actual: dash
Unordered list style
217: Expected: asterisk; Actual: dash
Unordered list style
218: Expected: asterisk; Actual: dash
Unordered list style
219: Expected: asterisk; Actual: dash
Unordered list style
220: Expected: asterisk; Actual: dash
Unordered list style
221: Expected: asterisk; Actual: dash
Unordered list style
222: Expected: asterisk; Actual: dash
Unordered list style
223: Expected: asterisk; Actual: dash
Unordered list style
224: Expected: asterisk; Actual: dash
Unordered list style
225: Expected: asterisk; Actual: dash
Unordered list style
226: Expected: asterisk; Actual: dash
Unordered list style
227: Expected: asterisk; Actual: dash
Unordered list style
228: Expected: asterisk; Actual: dash
Unordered list style
229: Expected: asterisk; Actual: dash
Unordered list style
230: Expected: asterisk; Actual: dash
Unordered list style
231: Expected: asterisk; Actual: dash
Unordered list style
232: Expected: asterisk; Actual: dash
Unordered list style
233: Expected: asterisk; Actual: dash
Unordered list style
234: Expected: asterisk; Actual: dash
Unordered list style
235: Expected: asterisk; Actual: dash
Unordered list style
236: Expected: asterisk; Actual: dash
Unordered list style
237: Expected: asterisk; Actual: dash
Unordered list style
238: Expected: asterisk; Actual: dash
Unordered list style
267: Expected: asterisk; Actual: dash
Unordered list style
268: Expected: asterisk; Actual: dash
Unordered list style
269: Expected: asterisk; Actual: dash
Unordered list style
346: Expected: asterisk; Actual: dash
Unordered list style
347: Expected: asterisk; Actual: dash
Unordered list style
348: Expected: asterisk; Actual: dash
Unordered list style
349: Expected: asterisk; Actual: dash
Unordered list style
350: Expected: asterisk; Actual: dash
Unordered list style
351: Expected: asterisk; Actual: dash
Unordered list style
352: Expected: asterisk; Actual: dash
Unordered list style
353: Expected: asterisk; Actual: dash
Unordered list style
354: Expected: asterisk; Actual: dash
Unordered list style
355: Expected: asterisk; Actual: dash
Unordered list style
356: Expected: asterisk; Actual: dash
Unordered list style
357: Expected: asterisk; Actual: dash
Unordered list style
358: Expected: asterisk; Actual: dash
Unordered list style
359: Expected: asterisk; Actual: dash
Unordered list style
360: Expected: asterisk; Actual: dash
Unordered list style
361: Expected: asterisk; Actual: dash
Unordered list style
362: Expected: asterisk; Actual: dash
Unordered list style
363: Expected: asterisk; Actual: dash
Unordered list style
364: Expected: asterisk; Actual: dash
Unordered list style
365: Expected: asterisk; Actual: dash
Unordered list style
366: Expected: asterisk; Actual: dash
Unordered list style
367: Expected: asterisk; Actual: dash
Unordered list style
368: Expected: asterisk; Actual: dash
Unordered list style
724: Expected: asterisk; Actual: dash
Unordered list style
732: Expected: asterisk; Actual: dash
Unordered list style
777: Expected: asterisk; Actual: dash
Unordered list style
785: Expected: asterisk; Actual: dash
Unordered list style
831: Expected: 1; Actual: 2
Multiple consecutive blank lines
1232: Expected: 1; Actual: 2
Multiple consecutive blank lines
1249: Expected: 1; Actual: 2
Multiple consecutive blank lines
1301: Expected: 1; Actual: 2
Multiple consecutive blank lines
1405: Expected: 1; Actual: 2
Multiple consecutive blank lines
242: null
Bare URL used
273: null
Bare URL used
372: null
Bare URL used
422: null
Bare URL used
434: null
Bare URL used
450: null
Bare URL used
521: null
Bare URL used
532: null
Bare URL used
540: null
Bare URL used
576: null
Bare URL used
601: null
Bare URL used
613: null
Bare URL used
659: null
Bare URL used
660: null
Bare URL used
661: null
Bare URL used
662: null
Bare URL used
663: null
Bare URL used
710: null
Bare URL used
751: null
Bare URL used
804: null
Bare URL used
917: null
Bare URL used
959: null
Spaces inside emphasis markers
959: null
Spaces inside emphasis markers
355: null
Spaces inside code span elements
Additional comments not posted (16)
x/ibchooks/keeper/query_server.go (1)
13-15
: The implementation of theParams
function in theKeeper
struct is correct and aligns with the PR objectives.proto/provenance/ibchooks/v1/event.proto (1)
9-12
: The definition ofEventIBCHooksParamsUpdated
is clear and well-documented. It correctly uses therepeated
keyword for handling multiple contracts.x/ibchooks/types/msgs.go (1)
20-43
: TheMsgUpdateParamsRequest
is well-defined with robust validation logic for the contract addresses and authority, ensuring they are valid Cosmos addresses.proto/provenance/ibchooks/v1/tx.proto (1)
Line range hint
20-51
: TheMsg
service is correctly defined with appropriate methods for IBC acknowledgement and updating parameters. The request and response structures are well-defined.x/ibchooks/keeper/msg_server.go (1)
44-60
: Well-implementedUpdateParams
method.Consider adding a detailed comment explaining the purpose and functionality of this method for better maintainability.
x/ibchooks/types/msgs_test.go (1)
23-64
: Comprehensive test coverage forMsgUpdateParamsRequest
.Consider adding more edge cases if applicable to further ensure robustness.
x/ibchooks/client/cli/tx.go (1)
34-65
: Well-structured command for updating module parameters.Consider adding more examples in the command's help text to aid users in understanding its usage.
x/ibchooks/keeper/query_server_test.go (1)
39-58
: Thorough testing of query parameters functionality.Consider adding tests for potential error scenarios to enhance coverage.
x/ibchooks/client/cli/query.go (1)
82-103
: Well-implemented command for querying module parameters.Consider enhancing the help text to provide more detailed information about what parameters can be queried.
x/ibchooks/keeper/msg_server_test.go (1)
65-110
: Comprehensive testing of theUpdateParams
method.Consider adding more detailed assertions to verify the contents of the events emitted.
x/ibchooks/module.go (2)
121-121
: The addition oftypes.RegisterQueryServer(cfg.QueryServer(), am.keeper)
allows the module to handle gRPC queries. Verify that the gRPC server is correctly registered and functional.
77-77
: The change to returncli.NewTxCmd()
instead ofnil
enables CLI transaction commands for the ibchooks module. Ensure that the new command functions as expected.x/ibcratelimit/module/module.go (1)
23-23
: The addition of the import statement foribcratelimit
is straightforward and necessary for the module's functionality.x/ibchooks/client/cli/cli_test.go (1)
1-181
: The integration tests are well-structured and cover the new functionalities added to the ibchooks module. Consider adding more edge cases to ensure robustness.x/ibchooks/keeper/keeper.go (1)
Line range hint
34-76
: The addition of theauthority
field and related methods enhances the security and governance capabilities of the keeper. Verify the authority checks in a live environment to ensure they function as expected.Verification successful
The search results indicate that there are multiple test cases related to the
GetAuthority
,IsAuthority
, andValidateAuthority
methods across various modules. This suggests that the authority checks are indeed being tested.
- Test files found:
x/ibchooks/keeper/msg_server_test.go
x/exchange/keeper/keeper_test.go
x/exchange/keeper/msg_server_test.go
x/exchange/keeper/orders_test.go
x/exchange/keeper/market_test.go
x/exchange/keeper/grpc_query_test.go
x/sanction/keeper/keeper_test.go
x/sanction/keeper/msg_server_test.go
x/marker/keeper/keeper_test.go
x/marker/keeper/msg_server_test.go
Given the extensive presence of tests, it is reasonable to conclude that the authority checks are well-covered.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the authority checks in a live environment. # Test: Perform authority checks using different addresses. echo "Testing authority checks..." ibchooks test-authority-checksLength of output: 173
Script:
#!/bin/bash # Search for test cases related to the authority methods in the keeper. rg 'GetAuthority|IsAuthority|ValidateAuthority' --glob '*_test.go'Length of output: 15136
x/sanction/simulation/operations.go (1)
546-546
: The implementation ofSimulatePropMsgUpdateParams
aligns with the PR's objectives to enable parameter updates in thesanction
module.
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 ignored due to path filters (2)
x/ibchooks/types/query.pb.go
is excluded by!**/*.pb.go
x/ibchooks/types/query.pb.gw.go
is excluded by!**/*.pb.gw.go
Files selected for processing (2)
- proto/provenance/ibchooks/v1/query.proto (1 hunks)
- x/ibchooks/client/cli/cli_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- proto/provenance/ibchooks/v1/query.proto
- x/ibchooks/client/cli/cli_test.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 (3)
- x/ibchooks/client/cli/cli_test.go (1 hunks)
- x/ibchooks/types/msgs.go (1 hunks)
- x/ibchooks/types/msgs_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- x/ibchooks/client/cli/cli_test.go
- x/ibchooks/types/msgs.go
- x/ibchooks/types/msgs_test.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/ibchooks/client/cli/cli_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/ibchooks/client/cli/cli_test.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
Outside diff range and nitpick comments (5)
CHANGELOG.md (5)
Line range hint
120-367
: Consider using a consistent list marker style in Markdown.- * message #<issue-number> + - message #<issue-number>Also applies to: 724-724, 732-732, 777-777, 785-785
Line range hint
831-831
: Remove extra blank lines to maintain a clean and consistent format in the document.-
Also applies to: 1232-1232, 1249-1249, 1301-1301, 1405-1405
Line range hint
242-242
: Consider using Markdown links instead of bare URLs for better readability and navigation.- https://github.com/provenance-io/provenance/issues/123 + [Issue 123](https://github.com/provenance-io/provenance/issues/123)Also applies to: 273-273, 372-372, 422-422, 434-434, 450-450, 521-521, 532-532, 540-540, 576-576, 601-601, 613-613, 659-659, 660-660, 661-661, 662-662, 663-663, 710-710, 751-751, 804-804, 917-917
Line range hint
959-959
: Ensure there are no spaces inside emphasis markers to maintain proper Markdown formatting.- * this is a test * + *this is a test*
Line range hint
355-355
: Ensure there are no spaces inside code span elements to maintain proper Markdown formatting.- ` code ` + `code`
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional Context Used
Markdownlint (109)
CHANGELOG.md (109)
120: Expected: asterisk; Actual: dash
Unordered list style
121: Expected: asterisk; Actual: dash
Unordered list style
122: Expected: asterisk; Actual: dash
Unordered list style
123: Expected: asterisk; Actual: dash
Unordered list style
124: Expected: asterisk; Actual: dash
Unordered list style
125: Expected: asterisk; Actual: dash
Unordered list style
126: Expected: asterisk; Actual: dash
Unordered list style
127: Expected: asterisk; Actual: dash
Unordered list style
128: Expected: asterisk; Actual: dash
Unordered list style
129: Expected: asterisk; Actual: dash
Unordered list style
130: Expected: asterisk; Actual: dash
Unordered list style
131: Expected: asterisk; Actual: dash
Unordered list style
132: Expected: asterisk; Actual: dash
Unordered list style
133: Expected: asterisk; Actual: dash
Unordered list style
134: Expected: asterisk; Actual: dash
Unordered list style
135: Expected: asterisk; Actual: dash
Unordered list style
136: Expected: asterisk; Actual: dash
Unordered list style
137: Expected: asterisk; Actual: dash
Unordered list style
207: Expected: asterisk; Actual: dash
Unordered list style
208: Expected: asterisk; Actual: dash
Unordered list style
209: Expected: asterisk; Actual: dash
Unordered list style
210: Expected: asterisk; Actual: dash
Unordered list style
211: Expected: asterisk; Actual: dash
Unordered list style
212: Expected: asterisk; Actual: dash
Unordered list style
213: Expected: asterisk; Actual: dash
Unordered list style
214: Expected: asterisk; Actual: dash
Unordered list style
215: Expected: asterisk; Actual: dash
Unordered list style
216: Expected: asterisk; Actual: dash
Unordered list style
217: Expected: asterisk; Actual: dash
Unordered list style
218: Expected: asterisk; Actual: dash
Unordered list style
219: Expected: asterisk; Actual: dash
Unordered list style
220: Expected: asterisk; Actual: dash
Unordered list style
221: Expected: asterisk; Actual: dash
Unordered list style
222: Expected: asterisk; Actual: dash
Unordered list style
223: Expected: asterisk; Actual: dash
Unordered list style
224: Expected: asterisk; Actual: dash
Unordered list style
225: Expected: asterisk; Actual: dash
Unordered list style
226: Expected: asterisk; Actual: dash
Unordered list style
227: Expected: asterisk; Actual: dash
Unordered list style
228: Expected: asterisk; Actual: dash
Unordered list style
229: Expected: asterisk; Actual: dash
Unordered list style
230: Expected: asterisk; Actual: dash
Unordered list style
231: Expected: asterisk; Actual: dash
Unordered list style
232: Expected: asterisk; Actual: dash
Unordered list style
233: Expected: asterisk; Actual: dash
Unordered list style
234: Expected: asterisk; Actual: dash
Unordered list style
235: Expected: asterisk; Actual: dash
Unordered list style
236: Expected: asterisk; Actual: dash
Unordered list style
237: Expected: asterisk; Actual: dash
Unordered list style
238: Expected: asterisk; Actual: dash
Unordered list style
267: Expected: asterisk; Actual: dash
Unordered list style
268: Expected: asterisk; Actual: dash
Unordered list style
269: Expected: asterisk; Actual: dash
Unordered list style
346: Expected: asterisk; Actual: dash
Unordered list style
347: Expected: asterisk; Actual: dash
Unordered list style
348: Expected: asterisk; Actual: dash
Unordered list style
349: Expected: asterisk; Actual: dash
Unordered list style
350: Expected: asterisk; Actual: dash
Unordered list style
351: Expected: asterisk; Actual: dash
Unordered list style
352: Expected: asterisk; Actual: dash
Unordered list style
353: Expected: asterisk; Actual: dash
Unordered list style
354: Expected: asterisk; Actual: dash
Unordered list style
355: Expected: asterisk; Actual: dash
Unordered list style
356: Expected: asterisk; Actual: dash
Unordered list style
357: Expected: asterisk; Actual: dash
Unordered list style
358: Expected: asterisk; Actual: dash
Unordered list style
359: Expected: asterisk; Actual: dash
Unordered list style
360: Expected: asterisk; Actual: dash
Unordered list style
361: Expected: asterisk; Actual: dash
Unordered list style
362: Expected: asterisk; Actual: dash
Unordered list style
363: Expected: asterisk; Actual: dash
Unordered list style
364: Expected: asterisk; Actual: dash
Unordered list style
365: Expected: asterisk; Actual: dash
Unordered list style
366: Expected: asterisk; Actual: dash
Unordered list style
367: Expected: asterisk; Actual: dash
Unordered list style
368: Expected: asterisk; Actual: dash
Unordered list style
724: Expected: asterisk; Actual: dash
Unordered list style
732: Expected: asterisk; Actual: dash
Unordered list style
777: Expected: asterisk; Actual: dash
Unordered list style
785: Expected: asterisk; Actual: dash
Unordered list style
831: Expected: 1; Actual: 2
Multiple consecutive blank lines
1232: Expected: 1; Actual: 2
Multiple consecutive blank lines
1249: Expected: 1; Actual: 2
Multiple consecutive blank lines
1301: Expected: 1; Actual: 2
Multiple consecutive blank lines
1405: Expected: 1; Actual: 2
Multiple consecutive blank lines
242: null
Bare URL used
273: null
Bare URL used
372: null
Bare URL used
422: null
Bare URL used
434: null
Bare URL used
450: null
Bare URL used
521: null
Bare URL used
532: null
Bare URL used
540: null
Bare URL used
576: null
Bare URL used
601: null
Bare URL used
613: null
Bare URL used
659: null
Bare URL used
660: null
Bare URL used
661: null
Bare URL used
662: null
Bare URL used
663: null
Bare URL used
710: null
Bare URL used
751: null
Bare URL used
804: null
Bare URL used
917: null
Bare URL used
959: null
Spaces inside emphasis markers
959: null
Spaces inside emphasis markers
355: null
Spaces inside code span elements
Description
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
New Features
UpdateParams
andParams
) foribchooks
,ibcratelimit
,attribute
, andmarker
modules.ibchooks
module.Bug Fixes
ibchooks
to ensure accurate network parameter configurations and command testing.Tests
ibchooks
module, including tests for query parameters and update commands.Documentation
CHANGELOG.md
to reflect new RPC endpoints and CLI command enhancements.