feat(baseapp): Remove hybrid handlers #20782
Conversation
WalkthroughWalkthroughThe recent update to 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Outside diff range and nitpick comments (2)
runtime/v2/stub.go (1)
9-9: Ensure alignment with error handling conventions.Using
panicfor control flow is generally discouraged unless it's a critical error that cannot be recovered. If these methods are expected to fail under normal circumstances, consider returning an error instead.runtime/router_test.go (1)
Line range hint
1-1: Validate new routing logic comprehensively.Given the significant changes to routing logic in the
runtimepackage, ensure that there are comprehensive tests covering all new paths and error conditions introduced in this PR.Would you like assistance in writing additional test cases to cover these new scenarios?
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
runtime/v2/go.sumis excluded by!**/*.sumserver/v2/go.sumis excluded by!**/*.sum
Files selected for processing (11)
- CHANGELOG.md (1 hunks)
- baseapp/grpcrouter.go (5 hunks)
- baseapp/grpcrouter_test.go (1 hunks)
- baseapp/internal/protoutils/protoutils.go (1 hunks)
- baseapp/msg_service_router.go (5 hunks)
- baseapp/msg_service_router_test.go (1 hunks)
- runtime/router.go (5 hunks)
- runtime/router_test.go (3 hunks)
- runtime/v2/go.mod (1 hunks)
- runtime/v2/stub.go (1 hunks)
- server/v2/go.mod (1 hunks)
Files skipped from review due to trivial changes (4)
- baseapp/grpcrouter_test.go
- baseapp/msg_service_router_test.go
- runtime/v2/go.mod
- server/v2/go.mod
Additional context used
Path-based instructions (7)
runtime/v2/stub.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.baseapp/internal/protoutils/protoutils.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/router_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"runtime/router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.baseapp/grpcrouter.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.baseapp/msg_service_router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Learnings (1)
baseapp/msg_service_router.go (1)
User: testinginprod PR: cosmos/cosmos-sdk#18499 File: baseapp/msg_service_router.go:31-36 Timestamp: 2023-11-21T11:30:34.255Z Learning: The `responseByRequest` map in the `MsgServiceRouter` struct of the `baseapp/msg_service_router.go` file is populated only once during the construction of the `MsgServiceRouter` and is not intended to be modified afterwards, which addresses potential concurrency issues.
golangci-lint
runtime/router.go
63-63: ineffectual assignment to resp (ineffassign)
53-53: SA4009: argument resp is overwritten before first use (staticcheck)
Markdownlint
CHANGELOG.md
72-72: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
73-73: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
77-77: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
78-78: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
79-79: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
80-80: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
85-85: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
130-130: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
131-131: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
132-132: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
136-136: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
139-139: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
140-140: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
141-141: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
148-148: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
158-158: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
160-160: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
163-163: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
182-182: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
183-183: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
185-185: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
186-186: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
239-239: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
240-240: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
241-241: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
405-405: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
408-408: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
430-430: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
431-431: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
444-444: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
476-476: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
477-477: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
478-478: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
479-479: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
481-481: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
482-482: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
483-483: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
484-484: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
498-498: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
500-500: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
502-502: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
504-504: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
507-507: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
508-508: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
509-509: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
517-517: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
518-518: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
520-520: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
521-521: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
523-523: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
524-524: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
525-525: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
527-527: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
528-528: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
536-536: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
547-547: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
548-548: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
549-549: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
555-555: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
556-556: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
557-557: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
563-563: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
579-579: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
580-580: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
581-581: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
582-582: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
583-583: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
584-584: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
589-589: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
590-590: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
591-591: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
592-592: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
599-599: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
600-600: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
601-601: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
635-635: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
636-636: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
637-637: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
638-638: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
643-643: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
644-644: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
792-792: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
935-935: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
956-956: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
959-959: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1041-1041: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1042-1042: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1043-1043: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1044-1044: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1045-1045: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1046-1046: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1143-1143: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1229-1229: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1275-1275: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1281-1281: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1282-1282: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1283-1283: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1284-1284: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1285-1285: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1286-1286: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1386-1386: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1511-1511: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1512-1512: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1513-1513: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1514-1514: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1515-1515: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1516-1516: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1517-1517: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1518-1518: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1521-1521: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1522-1522: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1523-1523: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1524-1524: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1525-1525: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1526-1526: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
1775-1775: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1776-1776: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1777-1777: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1778-1778: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1779-1779: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1780-1780: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
1890-1890: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2227-2227: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2228-2228: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2229-2229: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2232-2232: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2233-2233: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2234-2234: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2256-2256: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2257-2257: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2258-2258: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2259-2259: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2260-2260: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2268-2268: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2269-2269: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2270-2270: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2271-2271: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2272-2272: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2274-2274: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2275-2275: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2276-2276: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2603-2603: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2604-2604: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2605-2605: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2606-2606: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2607-2607: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2609-2609: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2611-2611: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2612-2612: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2613-2613: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2614-2614: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2615-2615: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2616-2616: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2618-2618: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2619-2619: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2620-2620: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2623-2623: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2624-2624: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2625-2625: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2626-2626: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2627-2627: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2630-2630: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2633-2633: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2636-2636: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2637-2637: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2640-2640: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2647-2647: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2648-2648: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2649-2649: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2650-2650: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2651-2651: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2653-2653: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2654-2654: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2655-2655: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2656-2656: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2657-2657: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2658-2658: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2659-2659: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2660-2660: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2661-2661: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2664-2664: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2665-2665: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2666-2666: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2667-2667: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2668-2668: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2669-2669: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2676-2676: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2677-2677: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2678-2678: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2679-2679: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2686-2686: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2688-2688: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2690-2690: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2691-2691: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2692-2692: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2693-2693: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2694-2694: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2695-2695: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2696-2696: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2697-2697: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2698-2698: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2699-2699: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2700-2700: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2701-2701: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2702-2702: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2703-2703: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2704-2704: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2705-2705: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2706-2706: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2707-2707: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2708-2708: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2709-2709: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2710-2710: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2711-2711: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2712-2712: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2713-2713: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2714-2714: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2715-2715: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2716-2716: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2718-2718: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2719-2719: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2721-2721: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2722-2722: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2723-2723: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2724-2724: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2725-2725: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2726-2726: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2727-2727: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2730-2730: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2731-2731: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2733-2733: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2734-2734: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2737-2737: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2738-2738: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2739-2739: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2740-2740: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2741-2741: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2742-2742: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2743-2743: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2744-2744: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2746-2746: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2747-2747: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2748-2748: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2754-2754: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2757-2757: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2763-2763: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2771-2771: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2772-2772: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2773-2773: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2774-2774: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2782-2782: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2789-2789: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2790-2790: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2797-2797: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2799-2799: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2803-2803: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2804-2804: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2806-2806: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2814-2814: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2816-2816: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2817-2817: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2823-2823: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2831-2831: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2832-2832: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2833-2833: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2834-2834: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2835-2835: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2836-2836: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2837-2837: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2838-2838: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2839-2839: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2840-2840: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2841-2841: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2842-2842: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2843-2843: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2844-2844: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2846-2846: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2847-2847: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2849-2849: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2850-2850: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2851-2851: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2852-2852: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2853-2853: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2854-2854: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2855-2855: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2856-2856: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2857-2857: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2858-2858: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2860-2860: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2861-2861: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2864-2864: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2865-2865: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2866-2866: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2867-2867: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2868-2868: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2869-2869: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2870-2870: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2871-2871: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2872-2872: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2873-2873: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2874-2874: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2875-2875: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2876-2876: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2877-2877: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2878-2878: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2879-2879: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2884-2884: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2885-2885: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2886-2886: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2887-2887: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2888-2888: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2889-2889: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2891-2891: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2893-2893: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2907-2907: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2908-2908: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2909-2909: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2914-2914: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2915-2915: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2916-2916: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2920-2920: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2921-2921: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2922-2922: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2923-2923: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2924-2924: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2925-2925: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2928-2928: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2929-2929: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2930-2930: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2931-2931: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2932-2932: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2933-2933: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2934-2934: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2935-2935: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2936-2936: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2938-2938: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2940-2940: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2942-2942: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2947-2947: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2948-2948: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2949-2949: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2950-2950: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2951-2951: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2952-2952: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2953-2953: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2954-2954: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2955-2955: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2956-2956: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2957-2957: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2958-2958: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2959-2959: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2960-2960: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2961-2961: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2962-2962: Expected: 4; Actual: 8 (MD007, ul-indent)
Unordered list indentation
2963-2963: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2964-2964: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2965-2965: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2966-2966: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
2967-2967: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
192-192: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
236-236: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
197-197: Expected: 1; Actual: 0; Above (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
1145-1145: null (MD024, no-duplicate-heading)
Multiple headings with the same content
1935-1935: null (MD024, no-duplicate-heading)
Multiple headings with the same content
196-196: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
1693-1693: null (MD034, no-bare-urls)
Bare URL used
1723-1723: null (MD034, no-bare-urls)
Bare URL used
2670-2670: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2758-2758: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2760-2760: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2765-2765: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2767-2767: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2777-2777: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2779-2779: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2784-2784: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2792-2792: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2809-2809: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2811-2811: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2819-2819: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2825-2825: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2895-2895: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2898-2898: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2901-2901: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2903-2903: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
2943-2943: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
1064-1064: null (MD038, no-space-in-code)
Spaces inside code span elements
2711-2711: null (MD038, no-space-in-code)
Spaces inside code span elements
2711-2711: null (MD038, no-space-in-code)
Spaces inside code span elements
Additional comments not posted (3)
runtime/v2/stub.go (1)
9-9: Document the purpose offailingStoreServicemore clearly.While the comment explains that this is a stub implementation, it would be beneficial to elaborate on the scenarios in which this service might be used, especially for new developers or in documentation.
baseapp/grpcrouter.go (1)
11-11: Ensure correct integration ofprotoutils.The import of
protoutilsis crucial for the new functionality. Verify that it is integrated correctly throughout the file and used where necessary to support the new routing logic.baseapp/msg_service_router.go (1)
15-15: Updated Import Path Approved.The replacement of
protocompatwithprotoutilsaligns with the PR's refactoring objectives.
| @@ -6,6 +6,7 @@ import ( | |||
| "cosmossdk.io/core/store" | |||
| ) | |||
|
|
|||
| // failingStoreService is a stub implementation of a store service that always fails. | |||
There was a problem hiding this comment.
Consider custom error messages for clearer debugging.
The current implementation uses generic panic messages which may not be helpful during debugging. Consider using more descriptive messages that include the method being called.
- panic("kv store service not available for this module: verify runtime `skip_store_keys` app config if not expected")
+ panic("OpenKVStore failed: kv store service not available for this module. Verify 'skip_store_keys' in the app config.")Committable suggestion was skipped due to low confidence.
| // RequestAndResponseFullNameFromMethodDesc returns the fully-qualified name of the request message of the provided service's method. | ||
| func RequestAndResponseFullNameFromMethodDesc(sd *grpc.ServiceDesc, method grpc.MethodDesc) (protoreflect.FullName, protoreflect.FullName, error) { | ||
| methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) | ||
| desc, err := gogoproto.HybridResolver.FindDescriptorByName(methodFullName) | ||
| if err != nil { | ||
| return "", "", fmt.Errorf("cannot find method descriptor %s", methodFullName) | ||
| } | ||
| methodDesc, ok := desc.(protoreflect.MethodDescriptor) | ||
| if !ok { | ||
| return "", "", fmt.Errorf("invalid method descriptor %s", methodFullName) | ||
| } | ||
| return methodDesc.Input().FullName(), methodDesc.Output().FullName(), nil | ||
| } |
There was a problem hiding this comment.
Ensure robust error handling in RequestAndResponseFullNameFromMethodDesc.
This function performs critical operations for gRPC message handling. Ensure that all possible error paths are handled gracefully, and consider adding more specific error messages for better troubleshooting.
- return "", "", fmt.Errorf("cannot find method descriptor %s", methodFullName)
+ return "", "", fmt.Errorf("cannot find method descriptor for '%s': ensure the descriptor is correctly defined and accessible", methodFullName)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // RequestAndResponseFullNameFromMethodDesc returns the fully-qualified name of the request message of the provided service's method. | |
| func RequestAndResponseFullNameFromMethodDesc(sd *grpc.ServiceDesc, method grpc.MethodDesc) (protoreflect.FullName, protoreflect.FullName, error) { | |
| methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) | |
| desc, err := gogoproto.HybridResolver.FindDescriptorByName(methodFullName) | |
| if err != nil { | |
| return "", "", fmt.Errorf("cannot find method descriptor %s", methodFullName) | |
| } | |
| methodDesc, ok := desc.(protoreflect.MethodDescriptor) | |
| if !ok { | |
| return "", "", fmt.Errorf("invalid method descriptor %s", methodFullName) | |
| } | |
| return methodDesc.Input().FullName(), methodDesc.Output().FullName(), nil | |
| } | |
| // RequestAndResponseFullNameFromMethodDesc returns the fully-qualified name of the request message of the provided service's method. | |
| func RequestAndResponseFullNameFromMethodDesc(sd *grpc.ServiceDesc, method grpc.MethodDesc) (protoreflect.FullName, protoreflect.FullName, error) { | |
| methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) | |
| desc, err := gogoproto.HybridResolver.FindDescriptorByName(methodFullName) | |
| if err != nil { | |
| return "", "", fmt.Errorf("cannot find method descriptor for '%s': ensure the descriptor is correctly defined and accessible", methodFullName) | |
| } | |
| methodDesc, ok := desc.(protoreflect.MethodDescriptor) | |
| if !ok { | |
| return "", "", fmt.Errorf("invalid method descriptor %s", methodFullName) | |
| } | |
| return methodDesc.Input().FullName(), methodDesc.Output().FullName(), nil | |
| } |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| sdkCtx := sdk.UnwrapSDKContext(ctx) | ||
|
|
||
| var err error | ||
| resp, err = handler(sdkCtx, msg) // Assign value to resp |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
x/accounts/testing/mockmodule/mockmodule.pb.gois excluded by!**/*.pb.gox/staking/types/query.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- api/cosmos/staking/v1beta1/query.pulsar.go (1 hunks)
- x/accounts/account_test.go (4 hunks)
- x/accounts/defaults/multisig/utils_test.go (2 hunks)
- x/accounts/depinject.go (2 hunks)
- x/accounts/internal/implementation/encoding.go (1 hunks)
- x/accounts/keeper.go (7 hunks)
- x/accounts/keeper_test.go (1 hunks)
- x/accounts/proto/cosmos/accounts/testing/mockmodule/mockmodule.proto (1 hunks)
- x/accounts/testing/mockmodule/mock.go (1 hunks)
- x/accounts/utils_test.go (3 hunks)
- x/bank/keeper/keeper.go (2 hunks)
Files skipped from review due to trivial changes (1)
- api/cosmos/staking/v1beta1/query.pulsar.go
Additional context used
Path-based instructions (9)
x/accounts/internal/implementation/encoding.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/testing/mockmodule/mock.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/utils_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/account_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/defaults/multisig/utils_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (10)
x/accounts/proto/cosmos/accounts/testing/mockmodule/mockmodule.proto (1)
1-25: Protobuf definitions look correct and well-formed.The protobuf file correctly defines syntax, package, go_package option, and several messages with appropriate fields and options. This aligns with standard practices for defining protobuf messages in Go projects.
x/accounts/internal/implementation/encoding.go (1)
13-18: Type alias and interface for protobuf messages are correctly defined.The type alias
ProtoMsgand the generic interfaceProtoMsgGare well-defined and adhere to Go's best practices for type safety and clarity in handling protobuf messages.x/accounts/depinject.go (1)
Line range hint
37-70: Dependency injection setup is comprehensive and well-structured.The
ModuleInputsstruct is correctly populated with necessary dependencies. TheProvideModulefunction effectively uses these dependencies to configure the module. The addition ofCoinTransfereris a significant enhancement.x/accounts/keeper_test.go (1)
88-88: Test case for querying withtypes.StringValueis correctly implemented.The test case ensures that the
Querymethod can handletypes.StringValuecorrectly, which is crucial for validating the new query functionalities introduced.x/accounts/utils_test.go (3)
5-5: Approved import addition.The import of the
errorspackage is appropriate for error handling in the test functions.
44-50: Review ofcoinTransfererimplementation.The
coinTransfererstruct and its methodMakeTransferCoinsMessageare correctly implemented for testing purposes. The method returns an error, simulating a failure scenario, which is a common pattern in unit tests to ensure error handling paths are tested.
71-76: Enhancements in test environment setup innewKeeper.The modifications to use mock routing services (
MockQueryRouterandMockMsgRouter) and an event service are appropriate for isolating the unit tests from external dependencies. This ensures that tests are predictable and focused on the functionality being tested.x/accounts/account_test.go (1)
31-31: Enhancements to intermodule communication testing inTestAccount.The use of mock modules (
mockmodule.QueryEchoResponseandmockmodule.MsgEchoResponse) in the test methods enhances the isolation and focus of the tests. This approach is beneficial for accurately testing the intermodule communication aspects without external dependencies.Also applies to: 52-63, 86-96
x/accounts/defaults/multisig/utils_test.go (1)
28-28: Update toProtoMsgtype alias.Changing
ProtoMsgto aliasgogoproto.Messageis a sensible update, likely for better compatibility with the updated gogoproto versions or internal API changes. Ensure that this new alias is consistently used in related test utilities.x/accounts/keeper.go (1)
41-51: Comprehensive review ofCoinTransfererintegration inKeeper.The integration of the
CoinTransfererinterface in theKeeperstruct is well-executed. The methodsMakeTransferCoinsMessage,sendModuleMessage, andmaybeSendFundsare correctly implemented to utilize this interface, enhancing the module's capabilities for handling coin transfers. The error handling and modular approach in these methods are commendable.Also applies to: 63-63, 69-69, 71-71, 98-98, 386-386, 409-409, 436-436
| func (k BaseKeeper) MakeTransferCoinsMessage( | ||
| ctx context.Context, | ||
| from, | ||
| to []byte, | ||
| amount sdk.Coins, | ||
| ) (gogoproto.Message, gogoproto.Message, error) { | ||
|
|
||
| } |
There was a problem hiding this comment.
Incomplete implementation of MakeTransferCoinsMessage.
The function MakeTransferCoinsMessage is defined but contains no implementation. This could be an oversight or placeholder. Ensure that the function is fully implemented or removed if not needed.
| type msgRouter struct{} | ||
|
|
||
| func (m msgRouter) CanInvoke(ctx context.Context, typeURL string) error { | ||
| return errors.New("not implemented") | ||
| } | ||
|
|
||
| func (m msgRouter) InvokeTyped(ctx context.Context, req, res protoiface.MessageV1) error { | ||
| typedReq, ok := req.(*MsgEcho) | ||
| if !ok { | ||
| return fmt.Errorf("invalid msg request got %T", req) | ||
| } | ||
| typedRes, ok := res.(*MsgEchoResponse) | ||
| if !ok { | ||
| return fmt.Errorf("invalid msg response got %T", res) | ||
| } | ||
| typedRes.MsgEcho = typedReq.Msg | ||
| return nil | ||
| } | ||
|
|
||
| func (m msgRouter) InvokeUntyped(ctx context.Context, req protoiface.MessageV1) (res protoiface.MessageV1, err error) { | ||
| // TODO implement me | ||
| panic("implement me") | ||
| } | ||
|
|
||
| func MockMsgRouter() router.Service { | ||
| return msgRouter{} | ||
| } | ||
|
|
||
| type queryRouter struct{} | ||
|
|
||
| func (q queryRouter) CanInvoke(ctx context.Context, typeURL string) error { | ||
| return errors.New("do not call") | ||
| } | ||
|
|
||
| func (q queryRouter) InvokeTyped(ctx context.Context, req, res protoiface.MessageV1) error { | ||
| typedReq, ok := req.(*QueryEchoRequest) | ||
| if !ok { | ||
| return fmt.Errorf("invalid query request got %T", req) | ||
| } | ||
| typedRes, ok := res.(*QueryEchoResponse) | ||
| if !ok { | ||
| return fmt.Errorf("invalid query response got %T", res) | ||
| } | ||
| typedRes.MsgEcho = typedReq.Msg | ||
| return nil | ||
| } | ||
|
|
||
| func (q queryRouter) InvokeUntyped(ctx context.Context, req protoiface.MessageV1) (res protoiface.MessageV1, err error) { | ||
| typedReq, ok := req.(*QueryEchoRequest) | ||
| if !ok { | ||
| return nil, fmt.Errorf("invalid request") | ||
| } | ||
| return &QueryEchoResponse{MsgEcho: typedReq.Msg}, nil | ||
| } | ||
|
|
||
| func MockQueryRouter() router.Service { | ||
| return queryRouter{} | ||
| } |
There was a problem hiding this comment.
Mock implementations for routers are properly structured but need error handling improvements.
The mock routers msgRouter and queryRouter correctly simulate the behavior for testing. However, the use of panic in InvokeUntyped (line 34) should be replaced with proper error handling to avoid abrupt terminations in tests.
- panic("implement me")
+ return nil, errors.New("functionality not implemented")Consider using more descriptive error messages and handling them gracefully in your test cases.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type msgRouter struct{} | |
| func (m msgRouter) CanInvoke(ctx context.Context, typeURL string) error { | |
| return errors.New("not implemented") | |
| } | |
| func (m msgRouter) InvokeTyped(ctx context.Context, req, res protoiface.MessageV1) error { | |
| typedReq, ok := req.(*MsgEcho) | |
| if !ok { | |
| return fmt.Errorf("invalid msg request got %T", req) | |
| } | |
| typedRes, ok := res.(*MsgEchoResponse) | |
| if !ok { | |
| return fmt.Errorf("invalid msg response got %T", res) | |
| } | |
| typedRes.MsgEcho = typedReq.Msg | |
| return nil | |
| } | |
| func (m msgRouter) InvokeUntyped(ctx context.Context, req protoiface.MessageV1) (res protoiface.MessageV1, err error) { | |
| // TODO implement me | |
| panic("implement me") | |
| } | |
| func MockMsgRouter() router.Service { | |
| return msgRouter{} | |
| } | |
| type queryRouter struct{} | |
| func (q queryRouter) CanInvoke(ctx context.Context, typeURL string) error { | |
| return errors.New("do not call") | |
| } | |
| func (q queryRouter) InvokeTyped(ctx context.Context, req, res protoiface.MessageV1) error { | |
| typedReq, ok := req.(*QueryEchoRequest) | |
| if !ok { | |
| return fmt.Errorf("invalid query request got %T", req) | |
| } | |
| typedRes, ok := res.(*QueryEchoResponse) | |
| if !ok { | |
| return fmt.Errorf("invalid query response got %T", res) | |
| } | |
| typedRes.MsgEcho = typedReq.Msg | |
| return nil | |
| } | |
| func (q queryRouter) InvokeUntyped(ctx context.Context, req protoiface.MessageV1) (res protoiface.MessageV1, err error) { | |
| typedReq, ok := req.(*QueryEchoRequest) | |
| if !ok { | |
| return nil, fmt.Errorf("invalid request") | |
| } | |
| return &QueryEchoResponse{MsgEcho: typedReq.Msg}, nil | |
| } | |
| func MockQueryRouter() router.Service { | |
| return queryRouter{} | |
| } | |
| func (m msgRouter) InvokeUntyped(ctx context.Context, req protoiface.MessageV1) (res protoiface.MessageV1, err error) { | |
| // TODO implement me | |
| return nil, errors.New("functionality not implemented") | |
| } |
Description
This PR removes hybrid handlers from baseapp.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
failingStoreServiceas a stub implementation in runtime/v2.Bug Fixes
Refactor
Documentation
QueryValidatorsResponseto clarify field relationships.Tests