Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: rename core.registry.LegacyRegistry -> core.registry.InterfaceRegistrar #19758

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

kocubinski
Copy link
Member

@kocubinski kocubinski commented Mar 14, 2024

Description

Following core API review in calls, this name is more appropriate given there is not a clear deprecation path for usages of RegisterInterfaces.


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

    • Updated the registration functions across various modules to use registry.InterfaceRegistrar, enhancing the interface registration process.
    • Modified the HasRegisterInterfaces extension interface and various module implementations to align with the new registration method.
  • Chores

    • Updated documentation in CHANGELOG.md and UPGRADING.md to reflect the changes in interface registration and module updates.

Copy link
Contributor

coderabbitai bot commented Mar 14, 2024

Walkthrough

Walkthrough

The core update revolves around enhancing the interface registration system within a software framework. The changes primarily involve moving RegisterInterfaces and RegisterImplementations functions from InterfaceRegistry to a new entity, cosmossdk.io/core/registry.InterfaceRegistrar. Additionally, there's a shift in how genesis interfaces are handled, eliminating the dependency on codec.JsonCodec. Modules across the board are updated to align with these changes, requiring adjustments in their implementations to accommodate the new method signatures and interface registration mechanisms.

Changes

Files Change Summary
CHANGELOG.md, .../v2/module.go, .../legacy.go Moved RegisterInterfaces and RegisterImplementations from InterfaceRegistry to InterfaceRegistrar. Removed codec.JsonCodec from genesis interfaces.
UPGRADING.md, .../v2/module.go Updated HasRegisterInterfaces to use InterfaceRegistrar instead of codec.InterfaceRegistry.
codec/types/interface_registry.go, .../legacy.go Replaced registry.LegacyRegistry with registry.InterfaceRegistrar in InterfaceRegistry.
testutil/mock/..., types/module/..., x/.../module.go, x/.../types/codec.go, etc. Updated modules and types to use registry.InterfaceRegistrar for interface and implementation registration.

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

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.
    • Open a follow-up GitHub issue for this discussion.
  • 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.

@kocubinski kocubinski changed the title rename core.registry.LegacyRegistry -> core.registry.InterfaceRegistrar refactor: rename core.registry.LegacyRegistry -> core.registry.InterfaceRegistrar Mar 14, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 1b545a5 and fb0b3f4.
Files selected for processing (53)
  • CHANGELOG.md (1 hunks)
  • UPGRADING.md (1 hunks)
  • codec/types/interface_registry.go (1 hunks)
  • core/appmodule/v2/module.go (1 hunks)
  • core/registry/legacy.go (1 hunks)
  • testutil/mock/types_mock_appmodule.go (2 hunks)
  • testutil/mock/types_module_module.go (3 hunks)
  • types/module/core_module.go (1 hunks)
  • types/module/module.go (1 hunks)
  • types/msgservice/msg_service.go (2 hunks)
  • x/accounts/module.go (1 hunks)
  • x/auth/module.go (1 hunks)
  • x/auth/types/codec.go (1 hunks)
  • x/auth/vesting/module.go (1 hunks)
  • x/auth/vesting/types/codec.go (4 hunks)
  • x/authz/codec.go (2 hunks)
  • x/authz/module/module.go (1 hunks)
  • x/bank/module.go (1 hunks)
  • x/bank/types/codec.go (1 hunks)
  • x/circuit/module.go (1 hunks)
  • x/circuit/types/codec.go (1 hunks)
  • x/consensus/module.go (1 hunks)
  • x/consensus/types/codec.go (1 hunks)
  • x/counter/module.go (1 hunks)
  • x/counter/types/codec.go (1 hunks)
  • x/crisis/module.go (1 hunks)
  • x/crisis/types/codec.go (1 hunks)
  • x/distribution/module.go (1 hunks)
  • x/distribution/types/codec.go (2 hunks)
  • x/evidence/module.go (1 hunks)
  • x/evidence/types/codec.go (1 hunks)
  • x/feegrant/codec.go (1 hunks)
  • x/feegrant/module/module.go (1 hunks)
  • x/genutil/module.go (1 hunks)
  • x/gov/module.go (1 hunks)
  • x/gov/types/v1/codec.go (2 hunks)
  • x/gov/types/v1beta1/codec.go (1 hunks)
  • x/group/codec.go (2 hunks)
  • x/group/module/module.go (1 hunks)
  • x/mint/module.go (1 hunks)
  • x/mint/types/codec.go (1 hunks)
  • x/nft/codec.go (1 hunks)
  • x/nft/module/module.go (1 hunks)
  • x/params/module.go (1 hunks)
  • x/params/types/proposal/codec.go (1 hunks)
  • x/protocolpool/module.go (1 hunks)
  • x/protocolpool/types/codec.go (2 hunks)
  • x/slashing/module.go (1 hunks)
  • x/slashing/types/codec.go (1 hunks)
  • x/staking/module.go (1 hunks)
  • x/staking/types/codec.go (2 hunks)
  • x/upgrade/module.go (1 hunks)
  • x/upgrade/types/codec.go (1 hunks)
Files not reviewed due to errors (1)
  • x/group/module.go (no review received)
Additional comments: 69
x/counter/types/codec.go (1)
  • 10-16: The changes in x/counter/types/codec.go correctly update the RegisterInterfaces function to use the new registry.InterfaceRegistrar type. This aligns with the PR's objective to refactor the naming for clarity and accuracy. The usage of registrar.RegisterImplementations and msgservice.RegisterMsgServiceDesc is consistent with the expected patterns for interface registration and message service description registration in the Cosmos SDK.
x/nft/codec.go (1)
  • 11-15: The modifications in x/nft/codec.go are consistent with the PR's goal, successfully updating the RegisterInterfaces function to utilize registry.InterfaceRegistrar. The implementation of registrar.RegisterImplementations and msgservice.RegisterMsgServiceDesc follows the expected patterns for registering message types and service descriptions, ensuring compatibility with the Cosmos SDK's interface registration mechanisms.
x/circuit/types/codec.go (1)
  • 11-17: The updates in x/circuit/types/codec.go correctly reflect the PR's intention by transitioning to registry.InterfaceRegistrar in the RegisterInterfaces function. The use of registrar.RegisterImplementations for registering message types and msgservice.RegisterMsgServiceDesc for service descriptions is properly implemented, ensuring the code adheres to the Cosmos SDK's standards for interface and service registration.
x/protocolpool/types/codec.go (1)
  • 7-14: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [10-22]

In x/protocolpool/types/codec.go, the refactor to use registry.InterfaceRegistrar in the RegisterInterfaces function is correctly implemented. The registration of message types and service descriptions through registrar.RegisterImplementations and msgservice.RegisterMsgServiceDesc is consistent with the Cosmos SDK's interface registration practices, ensuring the code's compatibility and adherence to the project's standards.

x/params/types/proposal/codec.go (1)
  • 15-16: The changes in x/params/types/proposal/codec.go are in line with the PR's objectives, successfully updating the RegisterInterfaces function to use registry.InterfaceRegistrar. The implementation for registering the ParameterChangeProposal type through registrar.RegisterImplementations is correctly done, ensuring that the code adheres to the Cosmos SDK's interface registration standards.
x/consensus/types/codec.go (1)
  • 12-18: The refactor in x/consensus/types/codec.go to use registry.InterfaceRegistrar in the RegisterInterfaces function is correctly implemented. The registration of the MsgUpdateParams message type and the corresponding service description through registrar.RegisterImplementations and msgservice.RegisterMsgServiceDesc is properly done, aligning with the Cosmos SDK's interface registration conventions.
x/mint/types/codec.go (1)
  • 19-25: The updates in x/mint/types/codec.go correctly transition to using registry.InterfaceRegistrar in the RegisterInterfaces function. The registration of the MsgUpdateParams message type and the corresponding service description through registrar.RegisterImplementations and msgservice.RegisterMsgServiceDesc is properly executed, ensuring adherence to the Cosmos SDK's interface registration guidelines.
x/slashing/types/codec.go (1)
  • 20-26: The modifications in x/slashing/types/codec.go align with the PR's goals, updating the RegisterInterfaces function to utilize registry.InterfaceRegistrar. The registration of message types (MsgUnjail, MsgUpdateParams) and the service description through registrar.RegisterImplementations and msgservice.RegisterMsgServiceDesc is correctly implemented, adhering to the Cosmos SDK's standards for interface and service registration.
core/registry/legacy.go (1)
  • 7-7: The renaming of LegacyRegistry to InterfaceRegistrar is clear and well-documented. The comments provide a good understanding of the interface's purpose and usage.
x/crisis/types/codec.go (1)
  • 20-26: The update to use InterfaceRegistrar in the RegisterInterfaces function is correctly implemented. The renaming of the parameter to registrar is consistent and improves readability.
x/evidence/types/codec.go (1)
  • 22-30: The changes in RegisterInterfaces to use InterfaceRegistrar are correctly implemented. The method calls and parameter naming are consistent with the new interface.
x/upgrade/types/codec.go (1)
  • 22-28: The modifications to use InterfaceRegistrar in the RegisterInterfaces function are correctly implemented. The changes are consistent and improve the clarity of the interface's purpose.
x/bank/types/codec.go (1)
  • 24-31: The update to use InterfaceRegistrar in the RegisterInterfaces function is correctly implemented. The renaming of the parameter to registrar is consistent and improves readability.
x/gov/types/v1beta1/codec.go (1)
  • 24-37: The changes in RegisterInterfaces to use InterfaceRegistrar are correctly implemented. The method calls and parameter naming are consistent with the new interface.
x/auth/vesting/module.go (1)
  • 48-49: The update to use InterfaceRegistrar in the RegisterInterfaces method of AppModule is correctly implemented. This change aligns with the refactor's objectives.
x/counter/module.go (1)
  • 50-51: The update to use InterfaceRegistrar in the RegisterInterfaces method of AppModule is correctly implemented. This change is consistent with the refactor's objectives and improves the clarity of the interface's purpose.
x/distribution/types/codec.go (1)
  • 22-29: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-35]

The changes in the RegisterInterfaces function, including the parameter renaming and usage of registry.InterfaceRegistrar, align well with the PR's objectives and improve clarity. Good job on ensuring consistency and correctness in the implementation.

x/feegrant/codec.go (1)
  • 25-39: The updates in the RegisterInterfaces function, including the parameter renaming to registrar and the use of registry.InterfaceRegistrar, are consistent with the PR's goals and enhance the code's clarity and maintainability.
x/gov/types/v1/codec.go (1)
  • 24-31: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [27-40]

The modifications in the RegisterInterfaces function, including the parameter renaming to registrar and the adoption of registry.InterfaceRegistrar, are in line with the PR's objectives, enhancing clarity and consistency across the project.

x/authz/codec.go (1)
  • 34-45: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [26-44]

The changes in the RegisterInterfaces function, including the parameter renaming to registrar and the use of registry.InterfaceRegistrar, are consistent with the PR's goals, enhancing the code's clarity and maintainability across different modules.

x/auth/types/codec.go (1)
  • 31-59: The updates in the RegisterInterfaces function, including the parameter renaming to registrar and the use of registry.InterfaceRegistrar, are consistent with the PR's goals and enhance the code's clarity and maintainability across different modules.
types/msgservice/msg_service.go (1)
  • 51-58: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-55]

The modifications in the RegisterMsgServiceDesc function, including the parameter renaming to registrar and the adoption of registry.InterfaceRegistrar, are in line with the PR's objectives, enhancing clarity and consistency across the project.

x/staking/types/codec.go (1)
  • 29-36: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [32-43]

The changes in the RegisterInterfaces function, including the parameter renaming to registrar and the use of registry.InterfaceRegistrar, are consistent with the PR's goals, enhancing the code's clarity and maintainability across different modules.

x/auth/vesting/types/codec.go (1)
  • 55-65: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [30-64]

The updates in the RegisterInterfaces function, including the parameter renaming to registrar and the use of registry.InterfaceRegistrar, are consistent with the PR's goals and enhance the code's clarity and maintainability across different modules. This systematic approach to refactoring is commendable.

x/consensus/module.go (1)
  • 64-65: The change from registry.LegacyRegistry to registry.InterfaceRegistrar in the RegisterInterfaces method aligns with the PR's objectives and correctly updates the parameter type. Good job on maintaining consistency with the new naming convention.
x/accounts/module.go (1)
  • 53-54: The update to use registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented and aligns with the refactor's goals. Well done.
x/group/codec.go (1)
  • 52-60: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [37-57]

The update to registry.InterfaceRegistrar in the RegisterInterfaces method and the adjustments within the function are correctly implemented. This change aligns with the refactor's objectives and is well executed.

x/params/module.go (1)
  • 67-68: The change to registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented, aligning with the refactor's goals. Nicely done.
x/circuit/module.go (1)
  • 56-57: The update to registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented, aligning with the refactor's objectives. Good work.
core/appmodule/v2/module.go (1)
  • 105-105: Updating the HasRegisterInterfaces interface to require a registry.InterfaceRegistrar parameter is a crucial part of the refactor. This change is correctly implemented and aligns with the PR's objectives.
x/genutil/module.go (1)
  • 122-122: The change to registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented, aligning with the refactor's goals. Well executed.
x/protocolpool/module.go (1)
  • 73-74: The update to registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented, aligning with the refactor's objectives. Excellent work.
x/nft/module/module.go (1)
  • 74-75: The update of the parameter type in the RegisterInterfaces method from registry.LegacyRegistry to registry.InterfaceRegistrar aligns with the PR's objectives and improves clarity and consistency across the project.
x/upgrade/module.go (1)
  • 81-82: The change in the RegisterInterfaces method to use registry.InterfaceRegistrar is consistent with the PR's objectives, enhancing clarity and consistency in the codebase.
x/feegrant/module/module.go (1)
  • 74-75: The update to use registry.InterfaceRegistrar in the RegisterInterfaces method is consistent with the PR's objectives, enhancing the codebase's clarity and consistency.
x/crisis/module.go (1)
  • 77-78: The change in the RegisterInterfaces method to use registry.InterfaceRegistrar is consistent with the PR's objectives, enhancing clarity and consistency in the codebase.
x/evidence/module.go (1)
  • 87-88: The update to use registry.InterfaceRegistrar in the RegisterInterfaces method is consistent with the PR's objectives, enhancing the codebase's clarity and consistency.
types/module/core_module.go (1)
  • 192-194: The change in the RegisterInterfaces method to use registry.InterfaceRegistrar in the coreAppModuleAdaptor struct is consistent with the PR's objectives, enhancing clarity and consistency in the codebase.
x/authz/module/module.go (1)
  • 95-96: The update to use registry.InterfaceRegistrar in the RegisterInterfaces method is consistent with the PR's objectives, enhancing the codebase's clarity and consistency.
x/auth/module.go (1)
  • 76-77: The change in the RegisterInterfaces method to use registry.InterfaceRegistrar is consistent with the PR's objectives, enhancing clarity and consistency in the codebase.
x/mint/module.go (1)
  • 85-86: The change to use registry.InterfaceRegistrar in the RegisterInterfaces method is consistent with the overall refactor goal and correctly implemented. This enhances the semantic clarity of the interface registration process.
x/bank/module.go (1)
  • 83-84: The update to use registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented and aligns with the refactor's objectives. This change ensures consistency across modules in how interfaces are registered.
x/slashing/module.go (1)
  • 85-86: The modification to utilize registry.InterfaceRegistrar in the RegisterInterfaces method is properly executed, enhancing the module's interface registration clarity and accuracy.
x/distribution/module.go (1)
  • 95-96: The change to use registry.InterfaceRegistrar in the RegisterInterfaces method is correctly implemented, aligning with the refactor's goal to improve semantic clarity in interface registration across the project.
x/staking/module.go (1)
  • 84-85: The update to utilize registry.InterfaceRegistrar in the RegisterInterfaces method is properly executed. This change is in line with the refactor's objectives, ensuring consistency and clarity in interface registration across modules.
x/gov/module.go (1)
  • 113-115: The change from registry.LegacyRegistry to registry.InterfaceRegistrar in the RegisterInterfaces method aligns with the PR's objectives and improves the clarity of the interface's purpose. Ensure that all references to the old LegacyRegistry type throughout the codebase have been updated to InterfaceRegistrar.
Verification successful

The execution of the shell script confirmed that there are no remaining references to LegacyRegistry in the Go files of the codebase, aligning with the PR's objectives to update to InterfaceRegistrar. This supports the conclusion that the change has been successfully applied across the codebase.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for any remaining references to LegacyRegistry in the codebase. Expecting no results.
rg --type go 'LegacyRegistry'

Length of output: 29

codec/types/interface_registry.go (1)
  • 38-38: The replacement of registry.LegacyRegistry with registry.InterfaceRegistrar in the InterfaceRegistry interface is consistent with the PR's objectives and enhances the clarity of the interface's role. Ensure that all implementations and usages of the InterfaceRegistry interface throughout the codebase have been updated to reflect this change.
Verification successful

The search for any remaining references to LegacyRegistry in the codebase did not produce any results, indicating that all references have been successfully updated to InterfaceRegistrar. This confirms the correctness of the change in the InterfaceRegistry interface.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for any remaining references to LegacyRegistry in the codebase. Expecting no results.
rg --type go 'LegacyRegistry'

Length of output: 29

testutil/mock/types_module_module.go (3)
  • 71-71: The parameter type in the RegisterInterfaces method of MockAppModuleBasic has been correctly updated from registry.LegacyRegistry to registry.InterfaceRegistrar. This change aligns with the PR's objective to rename LegacyRegistry to InterfaceRegistrar, ensuring consistency across the project.
  • 156-156: The parameter type in the RegisterInterfaces method of MockAppModule has been successfully updated from registry.LegacyRegistry to registry.InterfaceRegistrar. This modification is consistent with the overarching goal of the PR to rename LegacyRegistry to InterfaceRegistrar, enhancing clarity and accuracy in naming conventions.
  • 581-581: The parameter type in the RegisterInterfaces method of MockHasABCIEndBlock has been updated from registry.LegacyRegistry to registry.InterfaceRegistrar. This change is in line with the PR's objective to rename LegacyRegistry to InterfaceRegistrar, contributing to a more accurate representation of the component's functionality.
testutil/mock/types_mock_appmodule.go (2)
  • 153-153: The update to the RegisterInterfaces method in MockAppModuleWithAllExtensions to accept an InterfaceRegistrar parameter aligns with the PR's objective of renaming LegacyRegistry to InterfaceRegistrar. This change is consistent and correctly reflects the new naming convention.
  • 337-337: Similarly, the update to the RegisterInterfaces method in MockAppModuleWithAllExtensionsABCI to accept an InterfaceRegistrar parameter is consistent with the overarching refactor goals. It correctly implements the new interface, ensuring that the mock aligns with the updated API.
types/module/module.go (1)
  • 315-318: The change from registry.LegacyRegistry to registry.InterfaceRegistrar in the RegisterInterfaces method aligns with the PR's objective to rename and refactor the component for clarity and better representation of its functionality. This update is consistent and correctly implemented according to the PR description. However, ensure that all modules implementing appmodule.HasRegisterInterfaces have been updated accordingly to accept the new InterfaceRegistrar interface without issues.
UPGRADING.md (16)
  • 166-176: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [4-4]

The term "SimApp" might be incorrectly flagged as a spelling mistake. It's a specific term used within the Cosmos SDK context referring to the example application.

  • 169-169: The change in the signature of the HasRegisterInterfaces method to accept cosmossdk.io/core/registry.InterfaceRegistrar instead of codec.InterfaceRegistry is correctly documented. However, ensure that all module implementations across the project are updated to reflect this change.
Verification successful

To accurately verify if all module implementations have been updated to reflect the change in the signature of the HasRegisterInterfaces method, we need to inspect the context of the "RegisterInterfaces" calls found in the grep output. Specifically, we need to check if these instances are part of module implementations and if they use the new signature accepting registry.InterfaceRegistrar instead of codec.InterfaceRegistry. The current information is insufficient to make a definitive conclusion.


The provided output from the script execution shows several instances of RegisterInterfaces method implementations across different modules. These snippets confirm that the RegisterInterfaces method is indeed being implemented with the registry.InterfaceRegistrar parameter, aligning with the updated signature mentioned in the review comment. Each module's AppModule struct implements the RegisterInterfaces method, accepting a registry.InterfaceRegistrar as its parameter, which is consistent with the documentation change described in the UPGRADING.md file.

Based on the evidence from the script output, it can be concluded that the module implementations across the project have been updated to reflect the change in the signature of the HasRegisterInterfaces method to accept cosmossdk.io/core/registry.InterfaceRegistrar instead of codec.InterfaceRegistry.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify if all modules have updated their HasRegisterInterfaces implementation.
grep -R "RegisterInterfaces" --include=*.go

Length of output: 21510



Script:

#!/bin/bash
# Inspect a few instances to determine the signature used in module implementations.
fd "module.go" --exec grep -C 5 "RegisterInterfaces" {}

Length of output: 26313

* 169-169: The diff block correctly shows the change in the method signature for `RegisterInterfaces`. This is a crucial update for module developers to ensure compatibility with the new interface. * 169-169: The documentation correctly highlights the migration of dependency injection functions from `cosmossdk.io/core` to `cosmossdk.io/depinject/appconfig`. This is an important update for developers using dependency injection in their modules. * 169-169: The section on Genesis Interface changes correctly outlines the migration to `context.Context` from `sdk.Context` and the removal of the codec from the framework's responsibility. This aligns with the broader goal of simplifying and streamlining module interfaces. * 169-169: The documentation mentions the migration of most Cosmos SDK modules to [collections](https://docs.cosmos.network/main/build/packages/collections). It's important for developers to note the removal of certain functions and the need to check against `collections.ErrNotFound` where applicable. * 169-169: The documentation correctly advises on the extraction of various modules (`x/auth`, `x/authz`, `x/bank`, etc.) into their own `go.mod` files. This modularization is crucial for better dependency management and should be noted by developers. * 169-169: The introduction of the `x/protocolpool` module to handle community pool funds is an important update. The documentation provides clear instructions for adding the `x/protocolpool` store during the upgrade process, which is essential for a smooth transition. * 169-169: The documentation on the migration to CometBFT and the renaming of various functions and packages to reflect this change is well-documented. Developers should pay close attention to these updates to ensure compatibility. * 169-169: The section on enabling Vote Extensions provides valuable information on an optional feature that can enhance the governance process. It's important for developers to understand how to enable this feature and its implications. * 169-169: The documentation on the changes to BaseApp, including the introduction of `SetPreBlocker` and the migration of ABCI methods to use pointers, is crucial for understanding the internal workings of the Cosmos SDK. Developers should review these changes carefully. * 169-169: The updates to the Protobuf section, including the migration from `gogo/protobuf` to `cosmos/gogoproto` and the new requirements for `{accepts,implements}_interface` annotations, are significant. Developers need to ensure their proto files are updated accordingly. * 169-169: The documentation on the changes to the Keyring, including the refactoring and the new requirements for using `codec.Codec`, is important for developers working with key management. Ensure that all relevant code is updated to comply with these changes. * 169-169: The introduction of the `postHandler` as a mechanism to execute custom logic after message execution is a significant addition. Developers should consider how this feature can be utilized in their applications for enhanced flexibility. * 169-169: The documentation on the IAVL "fast" index and its implications for node operators during the upgrade process is crucial. It's important for operators to be aware of the potential time required for the index creation. * 169-169: The comprehensive updates to modules, including the deprecation of `x/params` and the introduction of new modules like `x/consensus`, are well-documented. Developers should review these changes closely to ensure their modules are compatible with the latest SDK versions.

Comment on lines +147 to +149
* (types) [#19652](https://github.com/cosmos/cosmos-sdk/pull/19652) and [#19758](https://github.com/cosmos/cosmos-sdk/pull/19758)
* Moved`types/module.HasRegisterInterfaces` to `cosmossdk.io/core`.
* Moved `RegisterInterfaces` and `RegisterImplementations` from `InterfaceRegistry` to `cosmossdk.io/core/registry.LegacyRegistry` interface.
* Moved `RegisterInterfaces` and `RegisterImplementations` from `InterfaceRegistry` to `cosmossdk.io/core/registry.InterfaceRegistrar` interface.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a minor typographical error in the changelog entry. A space is missing after "Moved". Correcting this will improve the readability of the document.

- Moved`types/module.HasRegisterInterfaces` to `cosmossdk.io/core`.
+ Moved `types/module.HasRegisterInterfaces` to `cosmossdk.io/core`.

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
* (types) [#19652](https://github.com/cosmos/cosmos-sdk/pull/19652) and [#19758](https://github.com/cosmos/cosmos-sdk/pull/19758)
* Moved`types/module.HasRegisterInterfaces` to `cosmossdk.io/core`.
* Moved `RegisterInterfaces` and `RegisterImplementations` from `InterfaceRegistry` to `cosmossdk.io/core/registry.LegacyRegistry` interface.
* Moved `RegisterInterfaces` and `RegisterImplementations` from `InterfaceRegistry` to `cosmossdk.io/core/registry.InterfaceRegistrar` interface.
* (types) [#19652](https://github.com/cosmos/cosmos-sdk/pull/19652) and [#19758](https://github.com/cosmos/cosmos-sdk/pull/19758)
* Moved `types/module.HasRegisterInterfaces` to `cosmossdk.io/core`.
* Moved `RegisterInterfaces` and `RegisterImplementations` from `InterfaceRegistry` to `cosmossdk.io/core/registry.InterfaceRegistrar` interface.

@tac0turtle tac0turtle added this pull request to the merge queue Mar 14, 2024
Merged via the queue into main with commit d54335e Mar 14, 2024
60 of 61 checks passed
@tac0turtle tac0turtle deleted the kocubinski/iface-registrar branch March 14, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants