refactor(x/authz)!: use router service

[julienrbrt]

Description

ref: #19542

---

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...

• included the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• reviewed "Files changed" and left comments if necessary
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• updated the relevant documentation or specification, including comments for documenting Go code
• confirmed all CI checks have passed

Reviewers 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...

• confirmed the correct type prefix in the PR title
• confirmed all author checklist items have been addressed
• reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

• Refactor

  • Enhanced request invocation checks in the Router service by optimizing parameter types for improved efficiency and error handling.
  • Streamlined internal logic and initialization in the authz module for better performance.

• Bug Fixes

  • Refined message routing and grant checks to address issues and improve overall functionality.

• Documentation

  • Updated CHANGELOG.md in the authz module to document changes including account creation adjustments and grants pruning enhancements.

• Tests

  • Aligned test setups in the authz module with recent codebase modifications to maintain reliability.

[coderabbitai]

Walkthrough

Walkthrough

The recent modifications streamline the invocation and routing logic across the system, particularly focusing on enhancing the authz module and router services. By shifting from protoiface.MessageV1 to a simpler typeURL string for invocation checks, the system simplifies type handling and error management. Additionally, the authz module sees significant restructuring, including the removal of certain dependencies and parameters, aimed at improving performance and maintainability. These changes collectively refine the system's efficiency and error handling capabilities.

Changes

Files	Change Summary
core/router/service.go runtime/router.go	Changed CanInvoke method parameter from protoiface.MessageV1 to typeURL string.
x/authz/CHANGELOG.md	Updated with several changes including removal of BaseAccount creation, addition of grants pruning limits, and a consensus-breaking change.
x/authz/keeper/genesis_test.go x/authz/keeper/msg_server.go x/authz/module/abci_test.go	Adjusted initialization order and method parameters, and updated logic for handling type URLs.
x/authz/keeper/keeper.go x/authz/keeper/keeper_test.go	Removed parameters and dependencies, altering initialization and message routing logic.
x/authz/module/depinject.go	Removed MsgServiceRouter dependency, updated NewKeeper initialization reflecting changes in module inputs.

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between 7f84815 and 45e9c10.

Files selected for processing (9)

• core/router/service.go (1 hunks)
• runtime/router.go (2 hunks)
• x/authz/CHANGELOG.md (1 hunks)
• x/authz/keeper/genesis_test.go (2 hunks)
• x/authz/keeper/keeper.go (3 hunks)
• x/authz/keeper/keeper_test.go (2 hunks)
• x/authz/keeper/msg_server.go (1 hunks)
• x/authz/module/abci_test.go (2 hunks)
• x/authz/module/depinject.go (3 hunks)

Additional comments: 11

core/router/service.go (1)

• 19-19: The change in the CanInvoke method signature from accepting a protoiface.MessageV1 to a typeURL string simplifies the process of checking request invocation validity. This approach is more straightforward and efficient, aligning well with the PR's goal of streamlining method signatures and functionality.

x/authz/module/depinject.go (2)

• 30-34: The removal of the MsgServiceRouter dependency and the direct inclusion of Environment in the NewKeeper call simplifies the dependency injection process and reduces coupling. This change enhances maintainability and aligns with best practices in dependency injection by directly providing what is needed.
• 45-45: Including Environment directly in the ModuleInputs struct and adjusting the ProvideModule function accordingly simplifies the module initialization process and makes it more explicit. This change enhances clarity and maintainability.

x/authz/CHANGELOG.md (1)

• 34-41: The changelog entries accurately reflect the changes made in the PR, providing clear information on modifications such as the removal of BaseAccount creation, addition of limits for grants pruning, and updates to the NewKeeper and DequeueAndDeleteExpiredGrants methods. The entries are well-organized and follow the guiding principles for changelogs, enhancing clarity and providing sufficient detail on the changes.

x/authz/keeper/genesis_test.go (1)

• 70-72: Moving the env initialization and modifying the keeper.NewKeeper call to exclude the msr parameter in the SetupTest function aligns with the changes in dependency injection and simplification of module initialization. These changes enhance maintainability and simplify dependencies, consistent with the PR's objectives.

x/authz/keeper/msg_server.go (1)

• 43-44: Using a method from RouterService to check the existence of a type URL in the Grant function is a more modular and maintainable approach. It abstracts the logic of checking the type URL's existence and allows for easier updates and modifications in the future, enhancing modularity and maintainability.

x/authz/module/abci_test.go (1)

• 68-69: Moving the initialization of env and including runtime.EnvWithRouterService in the TestExpiredGrantsQueue function aligns with the changes in dependency injection and the use of router services. This change simplifies the setup process and enhances the test's clarity by directly providing the necessary services, improving maintainability and testability.

runtime/router.go (1)

• 57-64: Changing the CanInvoke methods in both msgRouterService and queryRouterService to accept a typeURL string parameter simplifies the process of checking request invocation validity. This approach is more efficient and enhances error handling by providing more specific error messages for unknown messages or requests, aligning with the PR's objectives of streamlining method signatures and functionality.

x/authz/keeper/keeper.go (2)

• 36-36: The NewKeeper function's signature has been modified to remove the router parameter, aligning with the PR's objective to refactor the x/authz module for better integration with a router service. This change simplifies the keeper's initialization by removing an unnecessary dependency.
• 149-152: Direct invocation of the MessageRouterService within the DispatchActions method represents a significant change in how messages are routed. This approach simplifies the message dispatching process by eliminating intermediary routing layers. However, it's crucial to ensure that this change does not introduce any unintended side effects, such as bypassing important middleware or hooks that were previously executed as part of the routing process.

x/authz/keeper/keeper_test.go (1)

• 77-78: The modification in the test setup to include the router service for GRPC queries is a direct consequence of the changes made in the keeper.go file. This adjustment ensures that the tests accurately reflect the new architecture and are capable of testing the functionality with the integrated router service. It's important to verify that all tests still pass and that the new setup does not omit any critical aspects of the previous environment setup.

[julienrbrt]

If we merge this first I'll go update the api usage in the gov pr.

[likhita-809]

lgtm
small spell correction

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between 45e9c10 and b43128d.

Files selected for processing (2)

• simapp/app.go (1 hunks)
• x/authz/CHANGELOG.md (1 hunks)

Additional comments: 3

x/authz/CHANGELOG.md (2)

• 30-30: The "Features" section correctly documents the addition of a limit for grants pruning. The use of specific terms like BeginBlock and PruneExpiredGrants is appropriate for the context of the Cosmos SDK.
• 41-41: The "Consensus Breaking Changes" section accurately documents the removal of BaseAccount creation in a specific scenario. This change is significant and correctly highlighted in the changelog.

simapp/app.go (1)

• 352-352: The initialization of app.AuthzKeeper has been updated to include additional parameters for the router service. This change aligns with the PR's objective to utilize a router service within the x/authz module. It's crucial to ensure that the added parameters (runtime.EnvWithRouterService(app.GRPCQueryRouter(), app.MsgServiceRouter())) are correctly implemented and that they integrate seamlessly with the existing architecture. Additionally, verify that all necessary tests and documentation have been updated to reflect this change.

• The change correctly implements the router service integration as intended.
• Ensure that the integration of the router service with app.AuthzKeeper does not introduce any unintended side effects or performance issues. It would be beneficial to conduct thorough testing, including unit and integration tests, to validate the functionality and performance of the x/authz module post-refactor.

[coderabbitai]

Consider adding more descriptive text to clarify the impact of the API breaking changes for developers. While the technical terms and references are correctly used, a brief explanation of why these changes were made and how developers can adapt would enhance the documentation. For instance:

+ * [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) The `NewKeeper` function signature has been updated: it no longer requires a message router parameter. Developers should now set the message router directly in the `appmodule.Environment`.
+ * [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) The `appmodule.Environment` is now explicitly passed to the Keeper, allowing access to various application services more directly.

This approach provides clearer guidance to developers on adapting to the new changes.

---

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.

Suggested change

	* [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) `NewKeeper` doesn't take a message router anymore. Set the message router in the `appmodule.Environment` instead.
	* [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) `appmodule.Environment` is received on the Keeper to get access to different application services.
	* [#18737](https://github.com/cosmos/cosmos-sdk/pull/18737) Update the keeper method `DequeueAndDeleteExpiredGrants` to take a limit argument for the number of grants to prune.
	* [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`.
	* [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) The `NewKeeper` function signature has been updated: it no longer requires a message router parameter. Developers should now set the message router directly in the `appmodule.Environment`.
	* [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) The `appmodule.Environment` is now explicitly passed to the Keeper, allowing access to various application services more directly.
	* [#18737](https://github.com/cosmos/cosmos-sdk/pull/18737) Update the keeper method `DequeueAndDeleteExpiredGrants` to take a limit argument for the number of grants to prune.
	* [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`.