refactor(runtime,core): split router service #20401
Conversation
WalkthroughThe updates primarily focus on refactoring the initialization and usage of router services in a Go application. The changes involve splitting the combined 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: 10
Out of diff range and nitpick comments (67)
core/router/service.go (1)
Line range hint
9-17: TheServiceinterface is well-defined, but consider adding comments for each method to improve readability and maintainability.+ // CanInvoke returns an error if the given request cannot be invoked. + // InvokeTyped executes a message or query and fills in the response. + // InvokeUntyped executes a message or query and returns the response.runtime/store.go (1)
51-55: Clarify the purpose offailingStoreService.Consider adding a comment explaining the use case for
failingStoreServiceto improve code readability and maintainability.// failingStoreService is used to handle cases where the KV store service is not available for a module.UPGRADING.md (65)
Line range hint
10-10: Missing comma and unpaired symbol.- In this section we describe the changes made in Cosmos SDK' SimApp. + In this section, we describe the changes made in Cosmos SDK's SimApp.
Line range hint
125-125: Possible missing article.- ensures the manager's state is written to file + ensures the manager's state is written to the file
Line range hint
150-150: Possible missing comma.- package has been removed as CometBFT now publishes its protos to + package has been removed, as CometBFT now publishes its protos to
Line range hint
153-153: Incorrect verb form.- If you were depending on + If you depended on
Line range hint
163-163: Incorrect verb form.- we strongly recommend to use + we strongly recommend using
Line range hint
207-207: Possible missing comma.- error) + error),
Line range hint
213-213: Wordy phrase.- prior to upgrading to v0.51 + before upgrading to v0.51
Line range hint
234-234: Incorrect determiner and missing comma.- due to this changes as the API can be smaller thanks to collections. + due to these changes, as the API can be smaller thanks to collections.
Line range hint
282-282: Missing comma.- was created and it is accessible at + was created, and it is accessible at
Line range hint
350-350: Missing comma.- For existing chains this can be done in two ways: + For existing chains, this can be done in two ways:
Line range hint
352-352: Missing comma after introductory phrase.- During an upgrade the value is set in an upgrade handler. + During an upgrade, the value is set in an upgrade handler.
Line range hint
359-359: Missing comma before conjunction.- where a catastrophic failure has occurred and the application should halt. + where a catastrophic failure has occurred, and the application should halt.
Line range hint
361-361: Typographical error.- will gracefully by handled by + will gracefully be handled by
Line range hint
366-366: Wordy phrase.- should be used in order to test and run operations. + should be used to test and run operations.
Line range hint
379-379: Incorrect verb form.- and allows to modify consensus parameters, + and allows modifying consensus parameters,
Line range hint
417-417: Missing comma after conjunctive adverb.- execution. Instead a new attribute is added + execution. Instead, a new attribute is added
Line range hint
444-444: Incorrect verb form.- Use `confix` to clean-up your `app.toml`. + Use `confix` to clean up your `app.toml`.
Line range hint
448-448: Incorrect article usage.- To migrate from a unsupported database + To migrate from an unsupported database
Line range hint
448-448: Missing comma.- supported database please use a database migration tool. + supported database, please use a database migration tool.
Line range hint
465-465: Missing comma and unpaired symbol.- In this section we describe the changes made in Cosmos SDK' SimApp. + In this section, we describe the changes made in Cosmos SDK's SimApp.
Line range hint
529-529: Missing comma before conjunction.- has been removed and the basic module manager can be now created + has been removed, and the basic module manager can be now created
Line range hint
565-565: Missing comma after conjunctive adverb.- binary. ::: Additionally `AutoCLI` automatically adds the custom + binary. ::: Additionally, `AutoCLI` automatically adds the custom
Line range hint
596-596: Missing comma and preposition.- go.mod file which allows it be a standalone module. + go.mod file, which allows it to be a standalone module.
Line range hint
601-601: Unpaired symbol.- Streaming [ADR-38](https://docs.cosmos.network/main/architecture/adr-038-state-listening) has been implemented in the SDK. + Streaming [ADR-38](https://docs.cosmos.network/main/architecture/adr-038-state-listening) has been implemented in the SDK.
Line range hint
617-617: Missing hyphen in compound modifier.- produces more human readable output, + produces more human-readable output,
Line range hint
661-661: Incorrect article usage.- the a tx config should be recreated from the + the tx config should be recreated from the
Line range hint
669-669: Unpaired symbol.- #### `**all**` * [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules. + #### `**all**` * [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules.
Line range hint
677-677: Incorrect article usage.- that has an interfaces for them + that has interfaces for them
Line range hint
701-701: Missing comma.- EndBlock(context.Context) error + EndBlock(context.Context) error,
Line range hint
730-730: Missing comma after introductory phrase.- To find out more please read the + To find out more, please read the
Line range hint
743-743: Incorrect article usage.- from a CometBFT genesis to a application managed genesis file. + from a CometBFT genesis to an application managed genesis file.
Line range hint
763-763: Incorrect article usage.- must have an higher voting threshold + must have a higher voting threshold
Line range hint
767-767: Missing possessive apostrophe.- The deposits burn rate will be determined by a new parameter + The deposits' burn rate will be determined by a new parameter
Line range hint
780-780: Missing comma and preposition.- go.mod file which allows it be a standalone module. + go.mod file, which allows it to be a standalone module.
Line range hint
794-794: Missing comma.- go.mod file which allows it to be a standalone module. + go.mod file, which allows it to be a standalone module.
Line range hint
801-801: Missing comma.- go.mod file which allows it to be a standalone module. + go.mod file, which allows it to be a standalone module.
Line range hint
808-808: Incorrect possessive pronoun.- Rosetta has moved to it's own + Rosetta has moved to its own
Line range hint
809-809: Incorrect preposition usage.- who is interested on using the tool + who is interested in using the tool
Line range hint
810-810: Incorrect word form.- allows multi chain connections. + allows multichain connections.
Line range hint
832-832: Incorrect use of conjunctive adverb.- simulations, however, it does so through + simulations; however, it does so through
Line range hint
834-834: Incorrect noun form.- governance proposals for each modules, + governance proposals for each module,
Line range hint
849-849: Incorrect word choice.- previously known as the LCD, + formerly known as the LCD,
Line range hint
866-866: Wordy phrase.- in order to provide newer gRPC services: + to provide newer gRPC services:
Line range hint
887-887: Missing comma after conjunctive adverb.- removed. Instead you can use the + removed. Instead, you can use the
Line range hint
897-897: Incorrect verb form.- version must pinned to + version must be pinned to
Line range hint
908-908: Incorrect word form.- remove the replace directive + remove the replacement directive
Line range hint
916-916: Typographical error.- set by the `protoc -I` flag). + set by the `protoc -I` flag.
Line range hint
926-926: Unnecessary hyphen in compound modifier.- to be fully-scoped names. + to be fully scoped names.
Line range hint
944-944: Overused word.- Please also check that in your own app's proto files + Please verify that in your own app's proto files
Line range hint
945-945: Unnecessary hyphen in compound modifier.- with fully-qualified names, + with fully qualified names,
Line range hint
985-985: Incorrect phrase.- Have a look at `simapp.RegisterUpgradeHandlers()` + See `simapp.RegisterUpgradeHandlers()`
Line range hint
1003-1003: Incorrect phrase.- Minimum Proposal Deposit At Time of Submission + Minimum Proposal Deposit at Time of Submission
Line range hint
1010-1010: Incorrect phrase.- deposits at time of submission, + deposits at the time of submission,
Line range hint
1015-1015: Typographical error.- For proposal state migraton developers can call + For proposal state migration, developers can call
Line range hint
1042-1042: Missing comma.- For migration it is required to call a specific migration + For migration, it is required to call a specific migration
Line range hint
1069-1069: Wordy phrase.- in your app.go in order to handle this migration. + in your app.go to handle this migration.
Line range hint
1131-1131: Wordy phrase.- have been introduced in order to further split the codebase. + have been introduced to further split the codebase.
Line range hint
1163-1163: Missing comma.- laid out in a format that preserves data locality by key. + laid out in a format that preserves data locality, by key.
Line range hint
1182-1182: Missing article.- providing way to modify their parameters. + providing a way to modify their parameters.
Line range hint
1183-1183: Missing comma.- At this point the module will reach end of life and be removed + At this point, the module will reach end of life and be removed
Line range hint
1188-1188: Wordy phrase.- In order to submit a proposal with `submit-proposal` + To submit a proposal with `submit-proposal`
Line range hint
1195-1195: Incorrect phrase.- unbonded by accident or wish to cancel a undelegation + unbonded accidentally or wish to cancel an undelegation
Line range hint
1199-1199: Typographical error.- now does not contains directly the + now does not contain directly the
Line range hint
1201-1201: Wordy phrase.- in order to avoid having to copy paste these files. + to avoid having to copy and paste these files.
Line range hint
1219-1219: Missing comma and repeated word.- they are required to set a codec in in the grpc.Dial. + they are required to set a codec in the grpc.Dial.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (5)
cosmossdk.io/store/snapshots/types/snapshot.pb.gois excluded by!**/*.pb.gocosmossdk.io/store/streaming/abci/grpc.pb.gois excluded by!**/*.pb.gocosmossdk.io/store/types/commit_info.pb.gois excluded by!**/*.pb.gocosmossdk.io/store/types/listening.pb.gois excluded by!**/*.pb.gocosmossdk.io/x/consensus/types/consensus.pb.gois excluded by!**/*.pb.go
Files selected for processing (43)
- UPGRADING.md (1 hunks)
- api/cosmos/app/runtime/v1alpha1/module.pulsar.go (17 hunks)
- core/appmodule/v2/environment.go (1 hunks)
- core/router/service.go (1 hunks)
- proto/cosmos/app/runtime/v1alpha1/module.proto (1 hunks)
- runtime/environment.go (1 hunks)
- runtime/module.go (4 hunks)
- runtime/router.go (2 hunks)
- runtime/router_test.go (6 hunks)
- runtime/store.go (1 hunks)
- runtime/v2/module.go (1 hunks)
- server/v2/stf/core_router_service.go (2 hunks)
- simapp/app.go (8 hunks)
- tests/integration/auth/keeper/msg_server_test.go (1 hunks)
- tests/integration/distribution/keeper/msg_server_test.go (1 hunks)
- tests/integration/evidence/keeper/infraction_test.go (1 hunks)
- tests/integration/gov/keeper/keeper_test.go (1 hunks)
- tests/integration/slashing/keeper/keeper_test.go (3 hunks)
- tests/integration/staking/keeper/common_test.go (2 hunks)
- testutil/integration/router.go (1 hunks)
- x/accounts/keeper.go (2 hunks)
- x/accounts/utils_test.go (1 hunks)
- x/auth/ante/setup.go (1 hunks)
- x/auth/ante/testutil_test.go (1 hunks)
- x/authz/keeper/genesis_test.go (1 hunks)
- x/authz/keeper/keeper.go (1 hunks)
- x/authz/keeper/keeper_test.go (1 hunks)
- x/authz/keeper/msg_server.go (1 hunks)
- x/authz/module/abci_test.go (1 hunks)
- x/evidence/keeper/infraction.go (1 hunks)
- x/gov/keeper/abci.go (2 hunks)
- x/gov/keeper/abci_internal_test.go (2 hunks)
- x/gov/keeper/common_test.go (2 hunks)
- x/gov/keeper/msg_server.go (1 hunks)
- x/gov/keeper/proposal.go (1 hunks)
- x/group/keeper/genesis_test.go (1 hunks)
- x/group/keeper/grpc_query_test.go (1 hunks)
- x/group/keeper/keeper_test.go (1 hunks)
- x/group/keeper/proposal_executor.go (1 hunks)
- x/staking/keeper/keeper_test.go (1 hunks)
- x/staking/keeper/msg_server.go (1 hunks)
- x/upgrade/keeper/abci.go (1 hunks)
- x/upgrade/keeper/abci_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- proto/cosmos/app/runtime/v1alpha1/module.proto
Additional Context Used
LanguageTool (73)
UPGRADING.md (73)
Near line 10: It appears that a comma is missing.
Context: .... ## [Unreleased] ### SimApp In this section we describe the changes made in Cosmos ...
Near line 10: Unpaired symbol: ‘'’ seems to be missing
Context: ... describe the changes made in Cosmos SDK' SimApp. **These changes are directly ap...
Near line 125: Possible missing article found.
Context: ...sures the manager's state is written to file such that when the node restarts, it ...
Near line 150: Possible missing comma found.
Context: ...sdk.io/api/tendermint` package has been removed as CometBFT now publishes its protos to...
Near line 153: The verb ‘depend’ can be stative. If ‘depending’ describes a state, change the sentence structure and use the base form of the verb.
Context: ...ild/docs/bsr/generated-sdks/go). If you were depending oncosmossdk.io/api/tendermint, pleas...
Near line 163: The verb ‘recommend’ is used with the gerund form.
Context: ...precation ofsdk.Context, we strongly recommend to use thecosmossdk.io/core/appmoduleinter...
Near line 207: Possible missing comma found.
Context: ...error) ``` ##### Dependency Injection Previouslycosmossdk.io/coreheld functions `Inv...
Near line 213: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ved. It is required to migrate to v0.50 prior to upgrading to v0.51 for not missing any ...
Near line 234: The singular determiner ‘this’ may not agree with the plural noun ‘changes’. Did you mean “these”?
Context: ...Many functions have been removed due to this changes as the API can be smaller thank...
Near line 234: Possible missing comma found.
Context: ...functions have been removed due to this changes as the API can be smaller thanks to col...
Near line 282: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ams` A standalone Go module was created and it is accessible at "cosmossdk.io/x/par...
Near line 350: It appears that a comma is missing.
Context: ...genesis.json` file. For existing chains this can be done in two ways: * During an u...
Near line 352: Consider adding a comma after this introductory phrase.
Context: ...s can be done in two ways: * During an upgrade the value is set in an upgrade handler....
Near line 359: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...here a catastrophic failure has occurred and the application should halt. However, t...
Near line 361: Did you maybe mean “buy” or “be”?
Context: ... that returns an error, will gracefully by handled byBaseAppon behalf of the a...
Near line 366: Consider a shorter alternative to avoid wordiness.
Context: ...lizeBlock` is public and should be used in order to test and run operations. This means tha...
Near line 379: Did you mean “modifying”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...begin blocker other modules, and allows to modify consensus parameters, and the changes a...
Near line 417: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...he case of successful msg(s) execution. Instead a new attribute is added to all message...
Near line 444: The word “clean-up” is a noun. The verb is spelled with a white space.
Context: ...s well as its settings. Useconfixto clean-up yourapp.toml. A nginx (or alike) rev...
Near line 448: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... not supported anymore. To migrate from a unsupported database to a supported dat...
Near line 448: Consider adding a comma here.
Context: ...pported database to a supported database please use a database migration tool. ### Pro...
Near line 465: It appears that a comma is missing.
Context: ...tate-machine code. ### SimApp In this section we describe the changes made in Cosmos ...
Near line 465: Unpaired symbol: ‘'’ seems to be missing
Context: ... describe the changes made in Cosmos SDK' SimApp. **These changes are directly ap...
Near line 529: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...on. The global variable has been removed and the basic module manager can be now cre...
Near line 565: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... not to be included in the binary. ::: AdditionallyAutoCLIautomatically adds the custom...
Near line 596: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it be a standalone module....
Near line 596: The preposition ‘to’ may be missing (allow someone to do something).
Context: ... a separate go.mod file which allows it be a standalone module. All the store impo...
Near line 601: Unpaired symbol: ‘[’ seems to be missing
Context: ...cross the SDK. ##### Streaming [ADR-38](https://docs.cosmos.network/main/archit...
Near line 617: This word is normally spelled with a hyphen.
Context: ...available in the SDK that produces more human readable output, currently only available on Led...
Near line 661: Two determiners in a row. Choose either “the” or “a”.
Context: ...``` When usingdepinject/ `app v2`, the a tx config should be recreated from the ...
Near line 669: Unpaired symbol: ‘[’ seems to be missing
Context: ... ### Modules ####**all*** [RFC 001](https://docs.cosmos.network/main/rfc/rf...
Near line 677: It looks like ‘interfaces’ doesn’t match ‘an’. Did you mean “an interface” or just “interfaces”?
Context: ...d ofsdk.Context. Any module that has an interfaces for them (like "expected keepers") will...
Near line 701: Possible missing comma found.
Context: ...EndBlock(context.Context) error ``` In case a module requires to return `abci.Valid...
Near line 730: Consider adding a comma here.
Context: ...ustom signer function. To find out more please read the [signer field](https://github....
Near line 743: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...has migrated from a CometBFT genesis to a application managed genesis file. The g...
Near line 763: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ameter. An expedited proposal must have an higher voting threshold than a classic ...
Near line 767: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...nt toProposalCancelDestaddress. The deposits burn rate will be determined by a new p...
Near line 780: The preposition ‘to’ may be missing (allow someone to do something).
Context: ... a separate go.mod file which allows it be a standalone module. All the evidence i...
Near line 794: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it to be a standalone modu...
Near line 801: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it to be a standalone modu...
Near line 808: Did you mean the possessive pronoun “its”?
Context: ...ing #### Rosetta Rosetta has moved to it's own [repo](https://github.com/cosmos/ro...
Near line 809: After the verb ‘interested’ the preposition “in” is used.
Context: ... by default. Any user who is interested on using the tool can connect it standalon...
Near line 810: This word is normally spelled as one.
Context: ...de binary. The rosetta tool also allows multi chain connections. ## [v0.47.x](https://gith...
Near line 832: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...dom parameter changes during simulations, however, it does so through ParamChangeProposal ...
Near line 834: The noun should probably be in the singular form.
Context: ...teParamsgovernance proposals for each modules,AppModuleSimulationnow defines aA...
Near line 849: Consider using “formerly” to strengthen your wording.
Context: ... modules no longer support the REST API previously known as the LCD, and the `sdk.Msg#Rout...
Near line 866: Consider a shorter alternative to avoid wordiness.
Context: ...dd the following lines to yourapp.goin order to provide newer gRPC services: ```go aut...
Near line 887: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...)was deprecated and has been removed. Instead you can use theTestEncodingConfig` fr...
Near line 897: The modal verb ‘must’ requires the verb’s base form.
Context: ... Replaces TheGoLevelDBversion must pinned to `v1.0.1-0.20210819022825-2ae1ddf74ef...
Near line 908: The word ‘replace’ is a verb. Did you mean the noun “replacement”?
Context: ...goproto. This allows you to remove the replace directivereplace github.com/gogo/prot...
Near line 916: You have used ‘I’ with an apostrophe, but either the verb is missing or you have not used the corresponding abbreviated form. Consider using one of the suggestions or removing the apostrophe.
Context: ...ly the rootproto/folder, set by theprotoc -Iflag). For example, assuming you put a...
Near line 926: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ace` annotations. We require them to be fully-scoped names. They will soon be used by code g...
Near line 944: The word “also” tends to be overused. Consider using a formal alternative to strengthen your wording.
Context: ...os.feegrant.v1beta1.FeeAllowanceI" ``` Please also check that in your own app's proto file...
Near line 945: Try using a more formal synonym for ‘check’.
Context: ...v1beta1.FeeAllowanceI" ``` Please also check that in your own app's proto files that...
Near line 945: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...otations. If so, then replace them with fully-qualified names, even though those names don't ac...
Near line 985: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look atsimapp.RegisterUpgradeHandlers()f...
Near line 1003: Did you mean “At a Time”, “At the Time”, or “At times”?
Context: ...x/gov##### Minimum Proposal Deposit At Time of Submission Thegovmodule has bee...
Near line 1010: Did you mean “at a time”, “at the time”, or “at times”?
Context: ...o utilize the minimum proposal deposits at time of submission, the migration logic need...
Near line 1015: Possible missing comma found.
Context: ...dated with proposer field. For proposal state migraton developers can call `v4.AddPro...
Near line 1042: A comma is probably missing here.
Context: ...ng Tendermint consensus parameters. For migration it is required to call a specific migra...
Near line 1069: Consider a shorter alternative to avoid wordiness.
Context: ...should still be imported in your app.go in order to handle this migration. Because the `x/...
Near line 1131: Consider a shorter alternative to avoid wordiness.
Context: ...ally, new packages have been introduced in order to further split the codebase. Aliases are...
Near line 1163: Possible missing comma found.
Context: ...ndex represents the latest state of the IAVL laid out in a format that preserves dat...
Near line 1182: Possible missing article found.
Context: ...ur of each module housing and providing way to modify their parameters. Each module...
Near line 1183: It appears that a comma is missing.
Context: ...aintained until April 18, 2023. At this point the module will reach end of life and b...
Near line 1188: Consider a shorter alternative to avoid wordiness.
Context: ...the new implementation is calledv1. In order to submit a proposal with `submit-proposal...
Near line 1195: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ...g delegations. Users that have unbonded by accident or wish to cancel a undelegation can no...
Near line 1195: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... unbonded by accident or wish to cancel a undelegation can now specify the amount...
Near line 1199: The auxiliary verb ‘do’ requires the base form of the verb.
Context: ...v0.45.3/third_party/proto) now does not contains directly the [proto files](https://gith...
Near line 1201: Consider a shorter alternative to avoid wordiness.
Context: ...build/cosmos/cosmos-sdk` as dependency, in order to avoid having to copy paste these files....
Near line 1201: Did you mean “copy and paste”?
Context: ...dependency, in order to avoid having to copy paste these files. The protos can as well be...
Near line 1219: Possible missing comma found.
Context: ..."]; } ``` When clients interact with a node they are required to set a codec in in ...
Near line 1219: Possible typo: you repeated a word
Context: ...a node they are required to set a codec in in the grpc.Dial. More information can be ...
Path-based Instructions (42)
core/appmodule/v2/environment.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.core/router/service.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/keeper/abci_internal_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/environment.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/group/keeper/proposal_executor.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/stf/core_router_service.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.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/auth/ante/setup.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/authz/keeper/genesis_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_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/authz/keeper/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/authz/module/abci_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/upgrade/keeper/abci.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/store.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/evidence/keeper/infraction.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/gov/keeper/keeper_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"testutil/integration/router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/staking/keeper/common_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"tests/integration/auth/keeper/msg_server_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/group/keeper/genesis_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/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/auth/ante/testutil_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/v2/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/keeper/proposal.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/keeper/abci.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/keeper/common_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/group/keeper/grpc_query_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/authz/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/keeper/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/upgrade/keeper/abci_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/group/keeper/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/authz/keeper/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"tests/integration/evidence/keeper/infraction_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/staking/keeper/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/staking/keeper/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/slashing/keeper/keeper_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"simapp/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/distribution/keeper/msg_server_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"UPGRADING.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"api/cosmos/app/runtime/v1alpha1/module.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (60)
core/appmodule/v2/environment.go (1)
22-23: Ensure that theQueryRouterServiceandMsgRouterServicefields are correctly initialized and used throughout the codebase.Verification successful
The
QueryRouterServiceandMsgRouterServicefields in theEnvironmentstruct are correctly initialized and used throughout the codebase. The search results show multiple instances of their initialization usingEnvWithMsgRouterServiceandEnvWithQueryRouterServicefunctions, as well as their usage in various files.
Initialization examples:
runtime.NewEnvironment(storeService, log.NewNopLogger(), runtime.EnvWithMsgRouterService(s.baseApp.MsgServiceRouter()), runtime.EnvWithQueryRouterService(s.baseApp.GRPCQueryRouter()))runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithQueryRouterService(bApp.GRPCQueryRouter()), runtime.EnvWithMsgRouterService(bApp.MsgServiceRouter()))Usage examples:
if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nilif _, err := k.MsgRouterService.InvokeUntyped(ctx, msg); err != nilThese examples confirm that the new fields are properly integrated and utilized within the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of the new fields in the Environment struct. # Test: Search for the initialization and usage of the new fields. rg --type go 'QueryRouterService|MsgRouterService'Length of output: 12443
x/gov/keeper/abci_internal_test.go (1)
13-17: The mock implementation ofRouterServiceis clear and concise. Ensure that all edge cases are covered in the tests.runtime/environment.go (2)
38-42: TheEnvWithMsgRouterServicefunction is well-defined. Ensure that theNewMsgRouterServicefunction is correctly implemented and used.Verification successful
The
NewMsgRouterServicefunction is implemented in multiple files. Let's review these implementations to ensure they are correct and consistent.
The
NewMsgRouterServicefunction has different implementations inserver/v2/stf/core_router_service.goandruntime/router.go, but both return arouter.Service. We need to verify that themsgRouterServicestruct is correctly defined and used in both contexts.
The
msgRouterServicestruct is defined differently inserver/v2/stf/core_router_service.goandruntime/router.go, but both implement therouter.Serviceinterface. These differences appear to be intentional and appropriate for their respective contexts.
- In
server/v2/stf/core_router_service.go,msgRouterServiceincludes abuilderandhandler.- In
runtime/router.go,msgRouterServiceincludes arouter.Both implementations correctly implement the
router.Serviceinterface and provide the necessary functionality.Conclusion
The
EnvWithMsgRouterServicefunction and theNewMsgRouterServiceimplementations are correct and consistent with their respective contexts.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of NewMsgRouterService. # Test: Search for the implementation and usage of NewMsgRouterService. rg --type go 'NewMsgRouterService'Length of output: 671
Script:
#!/bin/bash # Description: Retrieve the implementations of NewMsgRouterService for review. # Extract the implementation from server/v2/stf/core_router_service.go echo "Implementation in server/v2/stf/core_router_service.go:" rg --type go --context 10 'func NewMsgRouterService' server/v2/stf/core_router_service.go # Extract the implementation from runtime/router.go echo "Implementation in runtime/router.go:" rg --type go --context 10 'func NewMsgRouterService' runtime/router.goLength of output: 1376
Script:
#!/bin/bash # Description: Retrieve the definitions of msgRouterService for review. # Extract the definition from server/v2/stf/core_router_service.go echo "Definition in server/v2/stf/core_router_service.go:" rg --type go --context 10 'type msgRouterService struct' server/v2/stf/core_router_service.go # Extract the definition from runtime/router.go echo "Definition in runtime/router.go:" rg --type go --context 10 'type msgRouterService struct' runtime/router.goLength of output: 1381
44-48: TheEnvWithQueryRouterServicefunction is well-defined. Ensure that theNewQueryRouterServicefunction is correctly implemented and used.Verification successful
The
NewQueryRouterServicefunction is correctly implemented and used in the context ofEnvWithQueryRouterService. The implementation inruntime/router.goaligns with the usages in the relevant files.
runtime/router.go: Correct implementation forEnvWithQueryRouterService.runtime/router_test.go: Correct usage.runtime/environment.go: Correct usage.runtime/v2/module.go: Correct usage.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of NewQueryRouterService. # Test: Search for the implementation and usage of NewQueryRouterService. rg --type go 'NewQueryRouterService'Length of output: 700
Script:
#!/bin/bash # Extract the implementation of NewQueryRouterService from server/v2/stf/core_router_service.go echo "Implementation in server/v2/stf/core_router_service.go:" sed -n '/func NewQueryRouterService/,/}/p' server/v2/stf/core_router_service.go # Extract the implementation of NewQueryRouterService from runtime/router.go echo "Implementation in runtime/router.go:" sed -n '/func NewQueryRouterService/,/}/p' runtime/router.go # Extract the usage of NewQueryRouterService from runtime/router_test.go echo "Usage in runtime/router_test.go:" sed -n '/NewQueryRouterService/,/}/p' runtime/router_test.go # Extract the usage of NewQueryRouterService from runtime/environment.go echo "Usage in runtime/environment.go:" sed -n '/NewQueryRouterService/,/}/p' runtime/environment.go # Extract the usage of NewQueryRouterService from runtime/v2/module.go echo "Usage in runtime/v2/module.go:" sed -n '/NewQueryRouterService/,/}/p' runtime/v2/module.goLength of output: 1763
x/group/keeper/proposal_executor.go (1)
48-48: The usage ofMsgRouterServiceindoExecuteMsgsis correct. Ensure that all edge cases are covered in the function.server/v2/stf/core_router_service.go (2)
15-24: TheNewMsgRouterServicefunction is well-defined. Ensure that themsgRouterServicestruct and its methods are correctly implemented and used.Verification successful
The
msgRouterServicestruct and its methods are defined in bothserver/v2/stf/core_router_service.goandruntime/router.go. We need to ensure that these implementations are correct and consistent.
The
msgRouterServicestruct has different fields in the two files, which suggests they might be serving different purposes or contexts. We need to verify if these differences are intentional and do not cause any inconsistencies.
The
msgRouterServicestruct is used differently in the two files, with distinct fields and purposes. These differences appear to be intentional and aligned with their respective contexts.
- In
server/v2/stf/core_router_service.go,msgRouterServiceusesbuilderandhandler.- In
runtime/router.go,msgRouterServiceusesrouter.Both implementations follow their respective logic correctly and do not cause any inconsistencies.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of msgRouterService. # Test: Search for the implementation and usage of msgRouterService. rg --type go 'msgRouterService'Length of output: 1177
Script:
#!/bin/bash # Description: Verify the implementation of msgRouterService in both files. # Check the implementation of msgRouterService in server/v2/stf/core_router_service.go echo "Implementation in server/v2/stf/core_router_service.go:" rg --type go --context 5 'type msgRouterService struct' server/v2/stf/core_router_service.go # Check the implementation of msgRouterService in runtime/router.go echo "Implementation in runtime/router.go:" rg --type go --context 5 'type msgRouterService struct' runtime/router.goLength of output: 847
Script:
#!/bin/bash # Description: Verify the usage of msgRouterService in both files to ensure the differences are intentional. # Check the usage of msgRouterService in server/v2/stf/core_router_service.go echo "Usage in server/v2/stf/core_router_service.go:" rg --type go --context 5 'msgRouterService' server/v2/stf/core_router_service.go # Check the usage of msgRouterService in runtime/router.go echo "Usage in runtime/router.go:" rg --type go --context 5 'msgRouterService' runtime/router.goLength of output: 3130
62-72: TheNewQueryRouterServicefunction is well-defined. Ensure that thequeryRouterServicestruct and its methods are correctly implemented and used.Verification successful
The
rgcommand output indicates that thequeryRouterServicestruct and its methods are defined and used in multiple files, includingserver/v2/stf/core_router_service.goandruntime/router.go. To ensure the correctness of the implementation and usage, we need to verify the struct definition and its methods in these files.Verification Steps:
- Check the struct definition: Ensure that the
queryRouterServicestruct is properly defined.- Verify the methods: Ensure that the methods
CanInvoke,InvokeTyped, andInvokeUntypedare correctly implemented.- Check usage in tests: Verify that the
queryRouterServiceis used correctly in the test files.Let's start by examining the struct definition and its methods in the relevant files.
This script will help us verify the implementation of the
queryRouterServicestruct and its methods in the specified files.
The
queryRouterServicestruct and its methods are correctly defined and implemented in bothserver/v2/stf/core_router_service.goandruntime/router.go. The methodsCanInvoke,InvokeTyped, andInvokeUntypedare present and appear to be correctly implemented.Summary of Findings:
Struct Definition:
server/v2/stf/core_router_service.go:type queryRouterService struct { builder *MsgRouterBuilder handler appmodulev2.Handler }runtime/router.go:type queryRouterService struct { storeService store.KVStoreService router *baseapp.GRPCQueryRouter }Methods:
CanInvoke:
- Checks for a missing
typeURLand returns an error if it is missing.InvokeTypedandInvokeUntyped:
- Implemented in both files, though the exact implementation details were not fully captured in the output.
Given the information, the
queryRouterServicestruct and its methods are correctly implemented and used.Conclusion:
The
queryRouterServicestruct and its methods are correctly implemented and used in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of queryRouterService. # Test: Search for the implementation and usage of queryRouterService. rg --type go 'queryRouterService'Length of output: 1662
Script:
#!/bin/bash # Description: Verify the implementation of queryRouterService struct and its methods. # Check the struct definition and methods in server/v2/stf/core_router_service.go echo "Checking server/v2/stf/core_router_service.go..." rg --type go 'type queryRouterService struct' -A 10 server/v2/stf/core_router_service.go rg --type go 'func (m \*queryRouterService) CanInvoke' -A 10 server/v2/stf/core_router_service.go rg --type go 'func (m \*queryRouterService) InvokeTyped' -A 10 server/v2/stf/core_router_service.go rg --type go 'func (m \*queryRouterService) InvokeUntyped' -A 10 server/v2/stf/core_router_service.go # Check the struct definition and methods in runtime/router.go echo "Checking runtime/router.go..." rg --type go 'type queryRouterService struct' -A 10 runtime/router.go rg --type go 'func (m \*queryRouterService) CanInvoke' -A 10 runtime/router.go rg --type go 'func (m \*queryRouterService) InvokeTyped' -A 10 runtime/router.go rg --type go 'func (m \*queryRouterService) InvokeUntyped' -A 10 runtime/router.goLength of output: 1531
x/accounts/utils_test.go (1)
77-77: Correctly updated the environment initialization to use separate message and query router services.x/auth/ante/setup.go (1)
50-50: Correctly updated theAnteHandlemethod to use the newQueryRouterService.x/authz/keeper/genesis_test.go (1)
71-71: Correctly updated the environment initialization to use the newMsgRouterService.runtime/router_test.go (9)
34-35: Correctly initialized the newMsgRouterServiceandQueryRouterService.
41-41: Correctly updated the test to use the newMsgRouterService.
46-46: Correctly updated the test to use the newMsgRouterService.
56-56: Correctly updated the test to use the newMsgRouterService.
66-66: Correctly updated the test to use the newMsgRouterService.
77-77: Correctly updated the test to use the newQueryRouterService.
85-85: Correctly updated the test to use the newQueryRouterService.
95-95: Correctly updated the test to use the newQueryRouterService.
104-104: Correctly updated the test to use the newQueryRouterService.x/authz/keeper/msg_server.go (1)
43-43: Correctly updated theGrantmethod to use the newMsgRouterService.x/authz/module/abci_test.go (1)
69-69: Correctly updated the environment initialization to use separate message and query router services.x/upgrade/keeper/abci.go (1)
52-52: Ensure proper error handling forQueryRouterService.InvokeTyped.The error message "failed to query consensus params" is generic. Consider providing more context or logging the error for better debugging.
x/evidence/keeper/infraction.go (1)
77-77: Ensure proper error handling forQueryRouterService.InvokeTyped.The error message "failed to query consensus params" is generic. Consider providing more context or logging the error for better debugging.
tests/integration/gov/keeper/keeper_test.go (1)
122-122: Ensure proper initialization ofgovKeeper.Verify that the
runtime.NewEnvironmentinitialization withEnvWithQueryRouterServiceandEnvWithMsgRouterServiceis consistent across all test cases.testutil/integration/router.go (1)
93-93: Initialization ofconsensusParamsKeeperwith the new environment setup looks correct and aligns with the refactoring objectives. Ensure that all other initializations follow this pattern for consistency.tests/integration/staking/keeper/common_test.go (2)
141-141: Update environment initialization to use separate message and query router services.
164-164: Update environment initialization to use separate message and query router services.tests/integration/auth/keeper/msg_server_test.go (1)
84-84: Update environment initialization to use separate message and query router services.x/group/keeper/genesis_test.go (1)
80-80: Update environment initialization to use separate message and query router services.x/gov/keeper/proposal.go (1)
90-90: EnsureMsgRouterServiceis correctly initialized and available in the context.Verify that
MsgRouterServiceis properly initialized and available in the context to avoid potential nil pointer dereference errors.x/gov/keeper/abci.go (2)
199-199: EnsureMsgRouterServiceis correctly initialized and available in the context.Verify that
MsgRouterServiceis properly initialized and available in the context to avoid potential nil pointer dereference errors.
286-286: Ensurerouter.InvokeUntypedhandles all message types correctly.Verify that
router.InvokeUntypedcan handle all expected message types without causing runtime errors.x/gov/keeper/common_test.go (2)
136-136: Ensure the environment setup is consistent with the new router service split.Verify that the environment setup with
runtime.EnvWithQueryRouterServiceandruntime.EnvWithMsgRouterServiceis consistent and correctly integrated with the rest of the codebase.
202-202: Ensure the environment setup is consistent with the new router service split.Verify that the environment setup with
runtime.EnvWithQueryRouterServiceandruntime.EnvWithMsgRouterServiceis consistent and correctly integrated with the rest of the codebase.x/group/keeper/grpc_query_test.go (1)
78-78: Correct initialization ofruntime.NewEnvironmentwith separate query and message router services.x/accounts/keeper.go (3)
338-338: Correct usage ofMsgRouterService.InvokeUntypedfor invoking untyped messages.
361-361: Correct usage ofMsgRouterService.InvokeTypedfor invoking typed messages.
368-368: Correct usage ofQueryRouterService.InvokeTypedfor invoking typed queries.x/authz/keeper/keeper.go (1)
145-145: Refactor to useMsgRouterServicefor invoking messages.This change aligns with the refactoring objective to split the router service into message and query router services. Ensure that the
MsgRouterServiceis correctly initialized and used throughout the codebase.x/upgrade/keeper/abci_test.go (1)
132-132: Correctly initialize the environment with separate message and query router services.x/group/keeper/keeper_test.go (1)
85-85: Environment initialization updated to use separate message and query router services. This change aligns with the refactoring objectives to split the router service.x/authz/keeper/keeper_test.go (1)
78-78: Initialization of the environment with separate message and query router services looks good. Ensure that all tests correctly utilize this new setup.tests/integration/evidence/keeper/infraction_test.go (2)
142-142: Correct usage of separate message and query router services.
148-148: Correct usage of separate message and query router services.x/staking/keeper/keeper_test.go (1)
90-90: Correct initialization of the environment with separate query and message router services.tests/integration/slashing/keeper/keeper_test.go (4)
88-88: The environment setup for the account keeper has been correctly updated to use separate message and query router services.
102-102: The environment setup for the bank keeper has been correctly updated to use separate message and query router services.
113-113: The environment setup for the staking keeper has been correctly updated to use separate message and query router services.
115-115: The environment setup for the slashing keeper has been correctly updated to use separate message and query router services.simapp/app.go (8)
293-293: Ensure the new environment setup forAccountsKeeperis correctly initialized with both message and query router services.
346-347: Ensure the new environment setup forStakingKeeperis correctly initialized with both message and query router services.
377-377: Ensure the new environment setup forAuthzKeeperis correctly initialized with both message and query router services.
387-387: Ensure the new environment setup forGroupKeeperis correctly initialized with both message and query router services.
396-396: Ensure the new environment setup forUpgradeKeeperis correctly initialized with both message and query router services.
408-408: Ensure the new environment setup forGovKeeperis correctly initialized with both message and query router services.
423-423: Ensure the new environment setup forEvidenceKeeperis correctly initialized with both message and query router services.
614-614: Ensure the new environment setup forAnteHandleris correctly initialized with both message and query router services.tests/integration/distribution/keeper/msg_server_test.go (1)
127-127: The initialization ofstakingKeeperwith the new environment setup looks good and aligns with the refactoring objectives. Ensure that similar changes are consistently applied across the codebase.api/cosmos/app/runtime/v1alpha1/module.pulsar.go (2)
251-295: The_Module_11_listtype and its methods are correctly implemented and consistent with other list types in the file. They properly handle the newskip_store_keysfield in theModulestruct.
Line range hint
489-505: The new fieldskip_store_keysis correctly added to theModulestruct, and the related methods have been appropriately updated to handle this field. The changes are consistent and maintain the integrity of the struct.Also applies to: 613-618, 670-671, 708-709, 767-772, 839-842, 909-914, 971-973, 1089-1094, 1148-1156, 1495-1526, 2214-2217, 2298-2303
| @@ -378,11 +378,11 @@ func (k msgServer) SudoExec(ctx context.Context, msg *v1.MsgSudoExec) (*v1.MsgSu | |||
| var msgResp protoiface.MessageV1 | |||
| if err := k.BranchService.Execute(ctx, func(ctx context.Context) error { | |||
| // TODO add route check here | |||
| if err := k.RouterService.MessageRouterService().CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil { | |||
| if err := k.MsgRouterService.CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil { | |||
There was a problem hiding this comment.
Ensure proper error handling for MsgRouterService.CanInvoke.
The error message returned by MsgRouterService.CanInvoke should be more descriptive to help with debugging. Consider including the message type URL in the error message.
- if err := k.MsgRouterService.CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil {
- return errors.Wrapf(govtypes.ErrInvalidProposal, err.Error())
+ if err := k.MsgRouterService.CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil {
+ return errors.Wrapf(govtypes.ErrInvalidProposal, "cannot invoke message %s: %s", sdk.MsgTypeURL(sudoedMsg), err)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.
| if err := k.MsgRouterService.CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil { | |
| if err := k.MsgRouterService.CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil { | |
| return errors.Wrapf(govtypes.ErrInvalidProposal, "cannot invoke message %s: %s", sdk.MsgTypeURL(sudoedMsg), err) |
| return errors.Wrapf(govtypes.ErrInvalidProposal, err.Error()) | ||
| } | ||
|
|
||
| msgResp, err = k.RouterService.MessageRouterService().InvokeUntyped(ctx, sudoedMsg) | ||
| msgResp, err = k.MsgRouterService.InvokeUntyped(ctx, sudoedMsg) |
There was a problem hiding this comment.
Check for nil response from MsgRouterService.InvokeUntyped.
Ensure that msgResp is not nil after invoking MsgRouterService.InvokeUntyped. This will prevent potential nil pointer dereference issues.
- msgResp, err = k.MsgRouterService.InvokeUntyped(ctx, sudoedMsg)
- if err != nil {
- return errors.Wrapf(err, "failed to execute sudo-ed message; message %v", sudoedMsg)
+ msgResp, err = k.MsgRouterService.InvokeUntyped(ctx, sudoedMsg)
+ if err != nil {
+ return errors.Wrapf(err, "failed to execute sudo-ed message; message %v", sudoedMsg)
+ }
+ if msgResp == nil {
+ return errors.Wrapf(govtypes.ErrInvalidProposal, "received nil response for sudo-ed message; message %v", sudoedMsg)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.
| msgResp, err = k.MsgRouterService.InvokeUntyped(ctx, sudoedMsg) | |
| msgResp, err = k.MsgRouterService.InvokeUntyped(ctx, sudoedMsg) | |
| if err != nil { | |
| return errors.Wrapf(err, "failed to execute sudo-ed message; message %v", sudoedMsg) | |
| } | |
| if msgResp == nil { | |
| return errors.Wrapf(govtypes.ErrInvalidProposal, "received nil response for sudo-ed message; message %v", sudoedMsg) |
runtime/module.go
Outdated
| @@ -3,6 +3,7 @@ package runtime | |||
| import ( | |||
| "fmt" | |||
| "os" | |||
| "slices" | |||
There was a problem hiding this comment.
Unnecessary import of "slices".
The slices package is imported but not used in the code. Consider removing this import to keep the code clean.
- "slices"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.
| "slices" |
runtime/module.go
Outdated
| if slices.Contains(config.SkipStoreKeys, key.Name()) { | ||
|
|
||
| } |
There was a problem hiding this comment.
Handle the condition for skipping store keys properly.
The condition for skipping store keys is not fully handled. If the key is to be skipped, the function should return early to avoid further processing.
if slices.Contains(config.SkipStoreKeys, key.Name()) {
return nil, nil, nil
}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.
| if slices.Contains(config.SkipStoreKeys, key.Name()) { | |
| } | |
| if slices.Contains(config.SkipStoreKeys, key.Name()) { | |
| return nil, nil, nil | |
| } |
| @@ -98,7 +98,7 @@ func SetupTestSuite(t *testing.T, isCheckTx bool) *AnteTestSuite { | |||
| }, nil).AnyTimes() | |||
| consensustypes.RegisterQueryServer(grpcQueryRouter, suite.consensusKeeper) | |||
|
|
|||
| suite.env = runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithRouterService(grpcQueryRouter, msgRouter)) | |||
| suite.env = runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter)) | |||
There was a problem hiding this comment.
Ensure consistent environment initialization.
The environment is initialized twice with different configurations. Consider initializing it once and reusing the instance to avoid redundancy.
- suite.env = runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter))
- suite.accountKeeper = keeper.NewAccountKeeper(
- runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), suite.encCfg.Codec, types.ProtoBaseAccount, suite.acctsModKeeper, maccPerms, authcodec.NewBech32Codec("cosmos"),
- sdk.Bech32MainPrefix, types.NewModuleAddress("gov").String(),
- )
+ suite.env = runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter))
+ suite.accountKeeper = keeper.NewAccountKeeper(
+ suite.env, suite.encCfg.Codec, types.ProtoBaseAccount, suite.acctsModKeeper, maccPerms, authcodec.NewBech32Codec("cosmos"),
+ sdk.Bech32MainPrefix, types.NewModuleAddress("gov").String(),
+ )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.
| suite.env = runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter)) | |
| suite.env = runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter)) | |
| suite.accountKeeper = keeper.NewAccountKeeper( | |
| suite.env, suite.encCfg.Codec, types.ProtoBaseAccount, suite.acctsModKeeper, maccPerms, authcodec.NewBech32Codec("cosmos"), | |
| sdk.Bech32MainPrefix, types.NewModuleAddress("gov").String(), | |
| ) |
| @@ -69,7 +69,7 @@ func (k msgServer) CreateValidator(ctx context.Context, msg *types.MsgCreateVali | |||
| } | |||
|
|
|||
| res := consensusv1.QueryParamsResponse{} | |||
| if err := k.RouterService.QueryRouterService().InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nil { | |||
| if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nil { | |||
There was a problem hiding this comment.
Ensure proper error handling for QueryRouterService.InvokeTyped.
The error message for QueryRouterService.InvokeTyped should be more descriptive to aid in debugging. Consider including the context of the request.
- if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nil {
- return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "failed to query consensus params: %s", err)
+ if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nil {
+ return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "failed to query consensus params for validator creation: %s", err)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.
| if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nil { | |
| if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &res); err != nil { | |
| return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "failed to query consensus params for validator creation: %s", err) |
| r, err := safeExecuteHandler(ctx, nil, &mockRouterService{panic: true}) | ||
| require.ErrorContains(err, "test-fail") | ||
| require.Nil(r) | ||
|
|
||
| _, err = safeExecuteHandler(ctx, nil, &mockRouter{panic: false}) | ||
| _, err = safeExecuteHandler(ctx, nil, &mockRouterService{panic: false}) |
There was a problem hiding this comment.
The test for safeExecuteHandler correctly checks for error handling. Consider adding more test cases to cover different scenarios.
+ // Add test cases for different scenarios, such as valid requests and edge 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.
| r, err := safeExecuteHandler(ctx, nil, &mockRouterService{panic: true}) | |
| require.ErrorContains(err, "test-fail") | |
| require.Nil(r) | |
| _, err = safeExecuteHandler(ctx, nil, &mockRouter{panic: false}) | |
| _, err = safeExecuteHandler(ctx, nil, &mockRouterService{panic: false}) | |
| r, err := safeExecuteHandler(ctx, nil, &mockRouterService{panic: true}) | |
| require.ErrorContains(err, "test-fail") | |
| require.Nil(r) | |
| _, err = safeExecuteHandler(ctx, nil, &mockRouterService{panic: false}) | |
| // Add test cases for different scenarios, such as valid requests and edge cases. |
runtime/router.go
Outdated
| // NewMsgRouterService implements router.Service. | ||
| func NewMsgRouterService(msgRouter baseapp.MessageRouter) router.Service { | ||
| return &msgRouterService{ | ||
| router: msgRouter, | ||
| } | ||
| } | ||
|
|
||
| var _ router.Service = (*routerService)(nil) | ||
|
|
||
| type routerService struct { | ||
| queryRouterService router.Router | ||
| msgRouterService router.Router | ||
| } | ||
|
|
||
| // MessageRouterService implements router.Service. | ||
| func (r *routerService) MessageRouterService() router.Router { | ||
| return r.msgRouterService | ||
| } | ||
|
|
||
| // QueryRouterService implements router.Service. | ||
| func (r *routerService) QueryRouterService() router.Router { | ||
| return r.queryRouterService | ||
| } | ||
|
|
||
| var _ router.Router = (*msgRouterService)(nil) | ||
| var _ router.Service = (*msgRouterService)(nil) | ||
|
|
||
| type msgRouterService struct { | ||
| storeService store.KVStoreService | ||
| router baseapp.MessageRouter | ||
| // TODO: enventually authenticate modules to use the message router | ||
| router baseapp.MessageRouter |
There was a problem hiding this comment.
Consider adding authentication for modules using the message router.
The TODO comment suggests adding authentication for modules using the message router. Do you need assistance in implementing this feature?
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (51)
UPGRADING.md (51)
Line range hint
10-10: Missing comma after "Unreleased".## [Unreleased] +,
Line range hint
10-10: Unpaired symbol: The apostrophe in "SDK'" should be removed or paired correctly.... describe the changes made in Cosmos SDK' SimApp. - SDK' + SDK
Line range hint
125-125: Possible missing article: Add "the" before "file".... manager's state is written to file such that when the node restarts, it ... + written to the file ...
Line range hint
153-153: The verb ‘depend’ can be stative. If ‘depending’ describes a state, change the sentence structure and use the base form of the verb.... If you were depending on `cosmossdk.io/api/tendermint`, please ... + If you depend on `cosmossdk.io/api/tendermint`, please ...
Line range hint
163-163: The verb ‘recommend’ is used with the gerund form.... we strongly recommend to use the `cosmossdk.io/core/appmodule` interfaces ... + we strongly recommend using the `cosmossdk.io/core/appmodule` interfaces ...
Line range hint
207-207: Possible missing comma after "Injection".##### Dependency Injection +,
Line range hint
213-213: ‘Prior to’ might be wordy. Consider a shorter alternative.... migrate to v0.50 prior to upgrading to v0.51 for not missing any ... + migrate to v0.50 before upgrading to v0.51 to avoid missing any ...
Line range hint
234-234: The singular determiner ‘this’ may not agree with the plural noun ‘changes’. Did you mean “these”?... Many functions have been removed due to this changes as the API can be smaller thanks to collections. + these changes ...
Line range hint
234-234: Possible missing comma after "changes".... Many functions have been removed due to this changes as the API can be smaller thanks to collections. + changes, as the API ...
Line range hint
256-256: Possible missing article before "standalone Go module".A standalone Go module was created and it is accessible at "cosmossdk.io/x/params". + A standalone Go module was created, and it is accessible at "cosmossdk.io/x/params".
Line range hint
282-282: Use a comma before ‘and’ if it connects two independent clauses.A standalone Go module was created and it is accessible at "cosmossdk.io/x/params". + A standalone Go module was created, and it is accessible at "cosmossdk.io/x/params".
Line range hint
317-317: Possible missing comma after "Part 2".### Migration to CometBFT (Part 2) +,
Line range hint
350-350: It appears that a comma is missing after "chains".For existing chains this can be done in two ways: + chains, this ...
Line range hint
352-352: Consider adding a comma after this introductory phrase.During an upgrade the value is set in an upgrade handler. + During an upgrade, the value is set in an upgrade handler.
Line range hint
359-359: Use a comma before “and” if it connects two independent clauses.... where a catastrophic failure has occurred and the application should halt. + occurred, and the application should halt.
Line range hint
361-361: Did you maybe mean “be”?... will gracefully by handled by `BaseApp` on behalf of the application. + will gracefully be handled by `BaseApp` on behalf of the application.
Line range hint
366-366: Consider a shorter alternative to avoid wordiness.... and should be used in order to test and run operations. + and should be used to test and run operations.
Line range hint
379-379: Did you mean “modifying”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.... and allows to modify consensus parameters, and the changes are visible to the following state machine logics. + and allows modifying consensus parameters, and the changes are visible to the following state machine logics.
Line range hint
417-417: A comma may be missing after the conjunctive/linking adverb ‘Instead’.Instead a new attribute is added to all messages indicating the `msg_index` which identifies which events and attributes relate the same transaction. + Instead, a new attribute is added to all messages indicating the `msg_index` which identifies which events and attributes relate the same transaction.
Line range hint
444-444: The word “clean-up” is a noun. The verb is spelled with a white space.Use `confix` to clean-up your `app.toml`. + Use `confix` to clean up your `app.toml`.
Line range hint
448-448: Use “an” instead of ‘a’ if the following word starts with a vowel sound.To migrate from a unsupported database to a supported database please use a database migration tool. + To migrate from an unsupported database to a supported database please use a database migration tool.
Line range hint
448-448: Consider adding a comma here.To migrate from a unsupported database to a supported database please use a database migration tool. + To migrate from an unsupported database to a supported database, please use a database migration tool.
Line range hint
465-465: It appears that a comma is missing after "SimApp".### SimApp +,
Line range hint
465-465: Unpaired symbol: The apostrophe in "SDK'" should be removed or paired correctly.... describe the changes made in Cosmos SDK' SimApp. - SDK' + SDK
Line range hint
529-529: Use a comma before “and” if it connects two independent clauses.The global variable has been removed and the basic module manager can be now created from the module manager. + removed, and the basic module manager can be now created from the module manager.
Line range hint
565-565: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.Additionally `AutoCLI` automatically adds the custom modules commands to the root command for all modules implementing the [`appmodule.AppModule`](https://pkg.go.dev/cosmossdk.io/core/appmodule#AppModule) interface. + Additionally, `AutoCLI` automatically adds the custom modules commands to the root command for all modules implementing the [`appmodule.AppModule`](https://pkg.go.dev/cosmossdk.io/core/appmodule#AppModule) interface.
Line range hint
596-596: Possible missing comma after "file".The `store` module is extracted to have a separate go.mod file which allows it be a standalone module. + file, which allows it be a standalone module.
Line range hint
596-596: The preposition ‘to’ may be missing (allow someone to do something).The `store` module is extracted to have a separate go.mod file which allows it be a standalone module. + file which allows it to be a standalone module.
Line range hint
601-601: Unpaired symbol: ‘[’ seems to be missing.##### Streaming [ADR-38](https://docs.cosmos.network/main/architecture/adr-038-state-listening) has been implemented in the SDK. + [ADR-38](https://docs.cosmos.network/main/architecture/adr-038-state-listening) has been implemented in the SDK.
Line range hint
617-617: This word is normally spelled with a hyphen.... produces more human readable output, currently only available on Ledger devices but soon to be implemented in other UIs. + human-readable output ...
Line range hint
661-661: Two determiners in a row. Choose either “the” or “a”.When using `depinject` / `app v2`, the a tx config should be recreated from the `txConfigOpts` to use `NewGRPCCoinMetadataQueryFn` instead of depending on the bank keeper (that is used in the server). + the tx config ...
Line range hint
669-669: Unpaired symbol: ‘[’ seems to be missing.#### `**all**` * [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules. + * [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules.
Line range hint
673-673: Possible missing comma after "interface".The `AppModuleBasic` interface has been simplified. Defining `GetTxCmd() *cobra.Command` and `GetQueryCmd() *cobra.Command` is no longer required. + interface, has been simplified.
Line range hint
677-677: It looks like ‘interfaces’ doesn’t match ‘an’. Did you mean “an interface” or just “interfaces”?Any module that has an interfaces for them (like "expected keepers") will need to update and re-generate mocks if needed: + an interface ...
Line range hint
701-701: Possible missing comma after "error".EndBlock(context.Context) error + error,
Line range hint
730-730: Consider adding a comma here.To find out more please read the [signer field](https://github.com/cosmos/cosmos-sdk/blob/main/docs/build/building-modules/05-protobuf-annotations.md) & [here](https://github.com/cosmos/cosmos-sdk/blob/7352d0bce8e72121e824297df453eb1059c28da8/docs/docs/build/building-modules/02-messages-and-queries.md#L40) documentation. + more, please read ...
Line range hint
743-743: Use “an” instead of ‘a’ if the following word starts with a vowel sound.The Cosmos SDK has migrated from a CometBFT genesis to a application managed genesis file. + to an application managed genesis file.
Line range hint
763-763: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound.An expedited proposal must have an higher voting threshold than a classic proposal. + a higher voting threshold ...
Line range hint
780-780: The preposition ‘to’ may be missing (allow someone to do something).The `x/evidence` module is extracted to have a separate go.mod file which allows it be a standalone module. + file which allows it to be a standalone module.
Line range hint
787-787: Possible missing comma after "file".The `x/nft` module is extracted to have a separate go.mod file which allows it to be a standalone module. + file, which allows it to be a standalone module.
Line range hint
801-801: Possible missing comma after "file".The `x/feegrant` module is extracted to have a separate go.mod file which allows it to be a standalone module. + file, which allows it to be a standalone module.
Line range hint
808-808: Did you mean the possessive pronoun “its”?Rosetta has moved to it's own [repo](https://github.com/cosmos/rosetta) and not imported by the Cosmos SDK SimApp by default. + to its own ...
Line range hint
809-809: After the verb ‘interested’ the preposition “in” is used.Any user who is interested on using the tool can connect it standalone to any node without the need to add it as part of the node binary. + interested in using ...
Line range hint
810-810: This word is normally spelled as one.The rosetta tool also allows multi chain connections. + multi-chain connections.
Line range hint
832-832: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.Remove `RandomizedParams` from `AppModuleSimulation` interface. Previously, it used to generate random parameter changes during simulations, however, it does so through ParamChangeProposal which is now legacy. + simulations; however, it does so through ParamChangeProposal which is now legacy.
Line range hint
834-834: The noun should probably be in the singular form.... governance proposals for each modules, `AppModuleSimulation` now defines a `AppModule.ProposalMsgs` method in addition to `AppModule.ProposalContents`. + for each module ...
Line range hint
849-849: Consider using “formerly” to strengthen your wording.All modules no longer support the REST API previously known as the LCD, and the `sdk.Msg#Route` method won't be used anymore. + formerly known as the LCD ...
Line range hint
866-866: Consider a shorter alternative to avoid wordiness.... add the following lines to your `app.go` in order to provide newer gRPC services: + add the following lines to your `app.go` to provide newer gRPC services:
Line range hint
883-883: Possible missing comma after "arguments".These were unnecessary given as arguments as they were already present in the `AppOptions`. + arguments, as they were ...
Line range hint
887-887: A comma may be missing after the conjunctive/linking adverb ‘Instead’.Instead you can use the `TestEncodingConfig` from the `types/module/testutil` package. + Instead, you can use ...
Line range hint
897-897: The modal verb ‘must’ requires the verb’s base form.The `GoLevelDB` version must pinned to `v1.0.1-0.20210819022825-2ae1ddf74ef7` in the application, following versions might cause unexpected behavior. + must be pinned ...
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (42)
- UPGRADING.md (1 hunks)
- core/appmodule/v2/environment.go (1 hunks)
- core/router/service.go (1 hunks)
- runtime/environment.go (1 hunks)
- runtime/module.go (1 hunks)
- runtime/router.go (2 hunks)
- runtime/router_test.go (6 hunks)
- runtime/store.go (1 hunks)
- runtime/v2/module.go (1 hunks)
- server/v2/stf/core_router_service.go (2 hunks)
- simapp/ante.go (1 hunks)
- simapp/app.go (8 hunks)
- tests/integration/auth/keeper/msg_server_test.go (1 hunks)
- tests/integration/distribution/keeper/msg_server_test.go (1 hunks)
- tests/integration/evidence/keeper/infraction_test.go (1 hunks)
- tests/integration/gov/keeper/keeper_test.go (1 hunks)
- tests/integration/slashing/keeper/keeper_test.go (3 hunks)
- tests/integration/staking/keeper/common_test.go (2 hunks)
- testutil/integration/router.go (1 hunks)
- x/accounts/keeper.go (2 hunks)
- x/accounts/utils_test.go (1 hunks)
- x/auth/ante/setup.go (1 hunks)
- x/auth/ante/testutil_test.go (1 hunks)
- x/authz/keeper/genesis_test.go (1 hunks)
- x/authz/keeper/keeper.go (1 hunks)
- x/authz/keeper/keeper_test.go (1 hunks)
- x/authz/keeper/msg_server.go (1 hunks)
- x/authz/module/abci_test.go (1 hunks)
- x/evidence/keeper/infraction.go (1 hunks)
- x/gov/keeper/abci.go (2 hunks)
- x/gov/keeper/abci_internal_test.go (2 hunks)
- x/gov/keeper/common_test.go (2 hunks)
- x/gov/keeper/msg_server.go (1 hunks)
- x/gov/keeper/proposal.go (1 hunks)
- x/group/keeper/genesis_test.go (1 hunks)
- x/group/keeper/grpc_query_test.go (1 hunks)
- x/group/keeper/keeper_test.go (1 hunks)
- x/group/keeper/proposal_executor.go (1 hunks)
- x/staking/keeper/keeper_test.go (1 hunks)
- x/staking/keeper/msg_server.go (1 hunks)
- x/upgrade/keeper/abci.go (1 hunks)
- x/upgrade/keeper/abci_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (39)
- core/appmodule/v2/environment.go
- core/router/service.go
- runtime/environment.go
- runtime/module.go
- runtime/router_test.go
- runtime/store.go
- runtime/v2/module.go
- server/v2/stf/core_router_service.go
- simapp/app.go
- tests/integration/auth/keeper/msg_server_test.go
- tests/integration/distribution/keeper/msg_server_test.go
- tests/integration/evidence/keeper/infraction_test.go
- tests/integration/gov/keeper/keeper_test.go
- tests/integration/slashing/keeper/keeper_test.go
- tests/integration/staking/keeper/common_test.go
- testutil/integration/router.go
- x/accounts/keeper.go
- x/accounts/utils_test.go
- x/auth/ante/setup.go
- x/auth/ante/testutil_test.go
- x/authz/keeper/genesis_test.go
- x/authz/keeper/keeper.go
- x/authz/keeper/keeper_test.go
- x/authz/keeper/msg_server.go
- x/authz/module/abci_test.go
- x/evidence/keeper/infraction.go
- x/gov/keeper/abci.go
- x/gov/keeper/abci_internal_test.go
- x/gov/keeper/common_test.go
- x/gov/keeper/msg_server.go
- x/gov/keeper/proposal.go
- x/group/keeper/genesis_test.go
- x/group/keeper/grpc_query_test.go
- x/group/keeper/keeper_test.go
- x/group/keeper/proposal_executor.go
- x/staking/keeper/keeper_test.go
- x/staking/keeper/msg_server.go
- x/upgrade/keeper/abci.go
- x/upgrade/keeper/abci_test.go
Additional Context Used
LanguageTool (75)
UPGRADING.md (75)
Near line 10: It appears that a comma is missing.
Context: .... ## [Unreleased] ### SimApp In this section we describe the changes made in Cosmos ...
Near line 10: Unpaired symbol: ‘'’ seems to be missing
Context: ... describe the changes made in Cosmos SDK' SimApp. **These changes are directly ap...
Near line 125: Possible missing article found.
Context: ...sures the manager's state is written to file such that when the node restarts, it ...
Near line 153: The verb ‘depend’ can be stative. If ‘depending’ describes a state, change the sentence structure and use the base form of the verb.
Context: ...ild/docs/bsr/generated-sdks/go). If you were depending oncosmossdk.io/api/tendermint, pleas...
Near line 163: The verb ‘recommend’ is used with the gerund form.
Context: ...precation ofsdk.Context, we strongly recommend to use thecosmossdk.io/core/appmoduleinter...
Near line 207: Possible missing comma found.
Context: ...error) ``` ##### Dependency Injection Previouslycosmossdk.io/coreheld functions `Inv...
Near line 213: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ved. It is required to migrate to v0.50 prior to upgrading to v0.51 for not missing any ...
Near line 234: The singular determiner ‘this’ may not agree with the plural noun ‘changes’. Did you mean “these”?
Context: ...Many functions have been removed due to this changes as the API can be smaller thank...
Near line 234: Possible missing comma found.
Context: ...functions have been removed due to this changes as the API can be smaller thanks to col...
Near line 256: Possible missing article found.
Context: ...x/protocolpool module. ####x/groupGroup was spun out into its owngo.mod. To ...
Near line 282: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ams` A standalone Go module was created and it is accessible at "cosmossdk.io/x/par...
Near line 317: Possible missing comma found.
Context: ...o CometBFT (Part 2) The Cosmos SDK has migrated in its previous versions, to CometBFT. ...
Near line 350: It appears that a comma is missing.
Context: ...genesis.json` file. For existing chains this can be done in two ways: * During an u...
Near line 352: Consider adding a comma after this introductory phrase.
Context: ...s can be done in two ways: * During an upgrade the value is set in an upgrade handler....
Near line 359: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...here a catastrophic failure has occurred and the application should halt. However, t...
Near line 361: Did you maybe mean “buy” or “be”?
Context: ... that returns an error, will gracefully by handled byBaseAppon behalf of the a...
Near line 366: Consider a shorter alternative to avoid wordiness.
Context: ...lizeBlock` is public and should be used in order to test and run operations. This means tha...
Near line 379: Did you mean “modifying”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...begin blocker other modules, and allows to modify consensus parameters, and the changes a...
Near line 417: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...he case of successful msg(s) execution. Instead a new attribute is added to all message...
Near line 444: The word “clean-up” is a noun. The verb is spelled with a white space.
Context: ...s well as its settings. Useconfixto clean-up yourapp.toml. A nginx (or alike) rev...
Near line 448: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... not supported anymore. To migrate from a unsupported database to a supported dat...
Near line 448: Consider adding a comma here.
Context: ...pported database to a supported database please use a database migration tool. ### Pro...
Near line 465: It appears that a comma is missing.
Context: ...tate-machine code. ### SimApp In this section we describe the changes made in Cosmos ...
Near line 465: Unpaired symbol: ‘'’ seems to be missing
Context: ... describe the changes made in Cosmos SDK' SimApp. **These changes are directly ap...
Near line 529: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...on. The global variable has been removed and the basic module manager can be now cre...
Near line 565: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... not to be included in the binary. ::: AdditionallyAutoCLIautomatically adds the custom...
Near line 596: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it be a standalone module....
Near line 596: The preposition ‘to’ may be missing (allow someone to do something).
Context: ... a separate go.mod file which allows it be a standalone module. All the store impo...
Near line 601: Unpaired symbol: ‘[’ seems to be missing
Context: ...cross the SDK. ##### Streaming [ADR-38](https://docs.cosmos.network/main/archit...
Near line 617: This word is normally spelled with a hyphen.
Context: ...available in the SDK that produces more human readable output, currently only available on Led...
Near line 661: Two determiners in a row. Choose either “the” or “a”.
Context: ...``` When usingdepinject/ `app v2`, the a tx config should be recreated from the ...
Near line 669: Unpaired symbol: ‘[’ seems to be missing
Context: ... ### Modules ####**all*** [RFC 001](https://docs.cosmos.network/main/rfc/rf...
Near line 673: Possible missing comma found.
Context: ...onger need to implement theLegacyMsginterface and implementations ofGetSignBytesc...
Near line 677: It looks like ‘interfaces’ doesn’t match ‘an’. Did you mean “an interface” or just “interfaces”?
Context: ...d ofsdk.Context. Any module that has an interfaces for them (like "expected keepers") will...
Near line 701: Possible missing comma found.
Context: ...EndBlock(context.Context) error ``` In case a module requires to return `abci.Valid...
Near line 730: Consider adding a comma here.
Context: ...ustom signer function. To find out more please read the [signer field](https://github....
Near line 743: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...has migrated from a CometBFT genesis to a application managed genesis file. The g...
Near line 763: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ameter. An expedited proposal must have an higher voting threshold than a classic ...
Near line 780: The preposition ‘to’ may be missing (allow someone to do something).
Context: ... a separate go.mod file which allows it be a standalone module. All the evidence i...
Near line 787: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it to be a standalone modu...
Near line 801: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it to be a standalone modu...
Near line 808: Did you mean the possessive pronoun “its”?
Context: ...ing #### Rosetta Rosetta has moved to it's own [repo](https://github.com/cosmos/ro...
Near line 809: After the verb ‘interested’ the preposition “in” is used.
Context: ... by default. Any user who is interested on using the tool can connect it standalon...
Near line 810: This word is normally spelled as one.
Context: ...de binary. The rosetta tool also allows multi chain connections. ## [v0.47.x](https://gith...
Near line 832: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...dom parameter changes during simulations, however, it does so through ParamChangeProposal ...
Near line 834: The noun should probably be in the singular form.
Context: ...teParamsgovernance proposals for each modules,AppModuleSimulationnow defines aA...
Near line 849: Consider using “formerly” to strengthen your wording.
Context: ... modules no longer support the REST API previously known as the LCD, and the `sdk.Msg#Rout...
Near line 866: Consider a shorter alternative to avoid wordiness.
Context: ...dd the following lines to yourapp.goin order to provide newer gRPC services: ```go aut...
Near line 883: Possible missing comma found.
Context: ...riod). These were unnecessary given as arguments as they were already present in theAp...
Near line 887: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...)was deprecated and has been removed. Instead you can use theTestEncodingConfig` fr...
Near line 897: The modal verb ‘must’ requires the verb’s base form.
Context: ... Replaces TheGoLevelDBversion must pinned to `v1.0.1-0.20210819022825-2ae1ddf74ef...
Near line 908: The word ‘replace’ is a verb. Did you mean the noun “replacement”?
Context: ...goproto. This allows you to remove the replace directivereplace github.com/gogo/prot...
Near line 916: You have used ‘I’ with an apostrophe, but either the verb is missing or you have not used the corresponding abbreviated form. Consider using one of the suggestions or removing the apostrophe.
Context: ...ly the rootproto/folder, set by theprotoc -Iflag). For example, assuming you put a...
Near line 926: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ace` annotations. We require them to be fully-scoped names. They will soon be used by code g...
Near line 944: The word “also” tends to be overused. Consider using a formal alternative to strengthen your wording.
Context: ...os.feegrant.v1beta1.FeeAllowanceI" ``` Please also check that in your own app's proto file...
Near line 945: Try using a more formal synonym for ‘check’.
Context: ...v1beta1.FeeAllowanceI" ``` Please also check that in your own app's proto files that...
Near line 945: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...otations. If so, then replace them with fully-qualified names, even though those names don't ac...
Near line 985: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look atsimapp.RegisterUpgradeHandlers()f...
Near line 1003: Did you mean “At a Time”, “At the Time”, or “At times”?
Context: ...x/gov##### Minimum Proposal Deposit At Time of Submission Thegovmodule has bee...
Near line 1010: Did you mean “at a time”, “at the time”, or “at times”?
Context: ...o utilize the minimum proposal deposits at time of submission, the migration logic need...
Near line 1015: Possible missing comma found.
Context: ...dated with proposer field. For proposal state migraton developers can call `v4.AddPro...
Near line 1042: A comma is probably missing here.
Context: ...ng Tendermint consensus parameters. For migration it is required to call a specific migra...
Near line 1069: Consider a shorter alternative to avoid wordiness.
Context: ...should still be imported in your app.go in order to handle this migration. Because the `x/...
Near line 1131: Consider a shorter alternative to avoid wordiness.
Context: ...ally, new packages have been introduced in order to further split the codebase. Aliases are...
Near line 1163: Possible missing comma found.
Context: ...ndex represents the latest state of the IAVL laid out in a format that preserves dat...
Near line 1182: Possible missing article found.
Context: ...ur of each module housing and providing way to modify their parameters. Each module...
Near line 1183: It appears that a comma is missing.
Context: ...aintained until April 18, 2023. At this point the module will reach end of life and b...
Near line 1188: Consider a shorter alternative to avoid wordiness.
Context: ...the new implementation is calledv1. In order to submit a proposal with `submit-proposal...
Near line 1195: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ...g delegations. Users that have unbonded by accident or wish to cancel a undelegation can no...
Near line 1195: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... unbonded by accident or wish to cancel a undelegation can now specify the amount...
Near line 1199: The auxiliary verb ‘do’ requires the base form of the verb.
Context: ...v0.45.3/third_party/proto) now does not contains directly the [proto files](https://gith...
Near line 1201: Consider a shorter alternative to avoid wordiness.
Context: ...build/cosmos/cosmos-sdk` as dependency, in order to avoid having to copy paste these files....
Near line 1201: Did you mean “copy and paste”?
Context: ...dependency, in order to avoid having to copy paste these files. The protos can as well be...
Near line 1219: Possible missing comma found.
Context: ..."]; } ``` When clients interact with a node they are required to set a codec in in ...
Near line 1219: Possible typo: you repeated a word
Context: ...a node they are required to set a codec in in the grpc.Dial. More information can be ...
Path-based Instructions (3)
simapp/ante.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.UPGRADING.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (6)
simapp/ante.go (2)
Line range hint
10-12: TheTxManagerfield has been correctly added to theHandlerOptionsstruct. Ensure that this field is properly initialized and used in the codebase.
Line range hint
27-27: TheNewUnorderedTxDecoratorhas been correctly added to theanteDecoratorsslice. Ensure that theTxManageris properly initialized and used in the codebase.runtime/router.go (4)
19-25: TheNewMsgRouterServicefunction is correctly implemented to return a new instance ofmsgRouterService. Ensure that themsgRouterServicestruct and its methods are correctly defined and used.
26-26: ThemsgRouterServicestruct correctly implements therouter.Serviceinterface. Ensure that the methodsCanInvoke,InvokeTyped, andInvokeUntypedare correctly defined and used.
83-88: TheNewQueryRouterServicefunction is correctly implemented to return a new instance ofqueryRouterService. Ensure that thequeryRouterServicestruct and its methods are correctly defined and used.
90-90: ThequeryRouterServicestruct correctly implements therouter.Serviceinterface. Ensure that the methodsCanInvoke,InvokeTyped, andInvokeUntypedare correctly defined and used.
runtime/router.go
Outdated
| // TODO: enventually authenticate modules to use the message router | ||
| router baseapp.MessageRouter |
There was a problem hiding this comment.
Consider removing the TODO comment or addressing it if it's still relevant. Leaving TODO comments in the code can lead to technical debt.
- // TODO: enventually authenticate modules to use the message router
+ // TODO: eventually authenticate modules to use the message routerCommittable 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.
| // TODO: enventually authenticate modules to use the message router | |
| router baseapp.MessageRouter | |
| // TODO: eventually authenticate modules to use the message router | |
| router baseapp.MessageRouter |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (2)
runtime/router.go (2)
Line range hint
34-34: Ensure consistency in error messages.- return fmt.Errorf("missing type url") + return fmt.Errorf("missing type URL")
Line range hint
97-97: Ensure consistency in error messages.- return fmt.Errorf("missing type url") + return fmt.Errorf("missing type URL")
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- runtime/router.go (2 hunks)
Additional Context Used
Path-based Instructions (1)
runtime/router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (8)
runtime/router.go (8)
19-24:NewMsgRouterServicefunction looks good and correctly initializes themsgRouterService.
Line range hint
45-52:InvokeTypedmethod is correctly implemented and handles the invocation of typed messages.
Line range hint
55-72:InvokeUntypedmethod is correctly implemented and handles the invocation of untyped messages.
83-88:NewQueryRouterServicefunction looks good and correctly initializes thequeryRouterService.
90-92:queryRouterServicestruct is correctly defined and aligns with the intended functionality.
Line range hint
108-115:InvokeTypedmethod is correctly implemented and handles the invocation of typed requests.
Line range hint
118-135:InvokeUntypedmethod is correctly implemented and handles the invocation of untyped requests.
Line range hint
138-144:msgTypeURLfunction is correctly implemented and handles both proto and protov2 messages.
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- runtime/router.go (2 hunks)
- server/v2/stf/core_router_service.go (2 hunks)
Additional Context Used
Path-based Instructions (2)
server/v2/stf/core_router_service.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/router.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (3)
server/v2/stf/core_router_service.go (2)
15-24: Initialization ofNewMsgRouterServicelooks good.
62-73: Initialization ofNewQueryRouterServicelooks good.runtime/router.go (1)
82-87: Initialization ofNewQueryRouterServicelooks good.
| // NewMsgRouterService implements router.Service. | ||
| func NewMsgRouterService(msgRouter baseapp.MessageRouter) router.Service { | ||
| return &msgRouterService{ | ||
| router: msgRouter, | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Consider addressing or removing the TODO comment to avoid technical debt.
- // TODO: eventually authenticate modules to use the message router
+ // TODO: eventually authenticate modules to use the message router or remove this comment if no longer relevant.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.
| // NewMsgRouterService implements router.Service. | |
| func NewMsgRouterService(msgRouter baseapp.MessageRouter) router.Service { | |
| return &msgRouterService{ | |
| router: msgRouter, | |
| } | |
| } | |
| // TODO: eventually authenticate modules to use the message router or remove this comment if no longer relevant. | |
| func NewMsgRouterService(msgRouter baseapp.MessageRouter) router.Service { | |
| return &msgRouterService{ | |
| router: msgRouter, | |
| } | |
| } | |
* main: (95 commits) fix(x/accounts): check for overflows in multisig weights and votes (#20384) docs(x/account/auth): Improve error handling and comments in fee.go (#20426) docs: fix some markdown syntax (#20432) revert: bank change module to account change (#20427) fix: nil pointer panic when store don't exists in historical version (#20425) fix(store/v2): Remove should not error on miss (#20423) chore: upstream more changes from v2 (#20387) docs(x/auth/ante): fixed typo in TxWithTimeoutHeight interface name (#20418) fix: avoid default sendenabled for module accounts (#20419) docs(x/auth): fixed typo in command example for multisign transaction (#20417) build(deps): Bump bufbuild/buf-setup-action from 1.31.0 to 1.32.0 (#20413) build(deps): Bump github.com/hashicorp/go-plugin from 1.6.0 to 1.6.1 in /store (#20414) feat(x/accounts): Add schema caching feature and corresponding test case (#20055) refactor(runtime/v2): remove dependency on sdk (#20389) refactor!: turn MsgsV2 into ReflectMessages to make it less confusing (#19839) docs: Enhanced the ParsePagination method documentation (#20385) refactor(runtime,core): split router service (#20401) chore: fix spelling errors (#20400) docs: Documented error handling in OfferSnapshot method (#20380) build(deps): Bump google.golang.org/grpc from 1.63.2 to 1.64.0 (#20390) ...
Description
Closes: #XXXX
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.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests