feat(auth): support accounts from auth #21688
Conversation
WalkthroughWalkthroughThe changes introduce new message types for querying account information in the Cosmos SDK, specifically aimed at maintaining backward compatibility with legacy systems. The modifications include updates to various files, implementing new methods, and enhancing the functionality of existing modules. Additionally, several dependencies have been updated, and new tests have been added to ensure the integrity and compatibility of the account management functionalities. Changes
Possibly related PRs
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
C:x/auth
C:x/accounts
C:x/accounts/lockup
C:x/accounts/multisig
labels
testinginprod
added
backport/v0.52.x
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
x/accounts/README.md (1)
5-61: Great work on the new section!The content is well-structured, and the explanations are clear and concise. The example implementation is particularly helpful for understanding the concept.
Please add a comma after "x/accounts" in this sentence:
-The x/accounts module provides a mechanism for custom account types to be exposed via its `Account` and `AccountInfo` gRPC -queries. This feature is particularly useful for ensuring compatibility with existing wallets that have not yet integrated -with x/accounts but still need to parse account information post-migration. +The x/accounts module provides a mechanism for custom account types to be exposed via its `Account` and `AccountInfo` gRPC +queries. This feature is particularly useful for ensuring compatibility with existing wallets that have not yet integrated +with x/accounts, but still need to parse account information post-migration.Tools
LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...have not yet integrated with x/accounts but still need to parse account information...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
59-59: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
60-60: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (8)
- .github/dependabot.yml (1 hunks)
- .github/pr_labeler.yml (1 hunks)
- .github/workflows/test.yml (1 hunks)
- go.work.example (1 hunks)
- x/accounts/README.md (1 hunks)
- x/accounts/defaults/base/CHANGELOG.md (1 hunks)
- x/accounts/defaults/base/sonar-project.properties (1 hunks)
- x/auth/keeper/grpc_query.go (3 hunks)
Additional context used
Path-based instructions (3)
x/accounts/defaults/base/CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"x/accounts/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"x/auth/keeper/grpc_query.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Markdownlint
x/accounts/defaults/base/CHANGELOG.md
26-26: null
Files should end with a single newline character(MD047, single-trailing-newline)
x/accounts/README.md
59-59: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
60-60: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
LanguageTool
x/accounts/README.md
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...have not yet integrated with x/accounts but still need to parse account information...(AI_HYDRA_LEO_MISSING_COMMA)
Additional comments not posted (27)
go.work.example (1)
29-29: LGTM!The addition of the
./x/accounts/defaults/basemodule path is a valid change that expands the project's functionality to include base defaults from the accounts module. This change is unlikely to introduce any issues.x/accounts/defaults/base/sonar-project.properties (14)
1-1: LGTM!The
sonar.projectKeyproperty is set correctly, following the naming convention of the project.
2-2: LGTM!The
sonar.organizationproperty is set correctly, matching the organization name of the project.
4-4: LGTM!The
sonar.projectNameproperty is set correctly, accurately describing the project and its location within the Cosmos SDK.
5-5: LGTM!Enabling
sonar.project.monorepo.enabledis appropriate for the Cosmos SDK project structure and allows for better management of multiple projects within a single repository.
7-7: LGTM!Setting
sonar.sourcesto.ensures that all relevant source files in the directory are included in the SonarQube analysis, which is appropriate for the project structure.
8-8: LGTM!The
sonar.exclusionsproperty is set correctly, excluding test files, generated protocol buffer files, and generated Pulsar files from the main analysis. This helps focus on the quality of the core source code.
9-9: LGTM!The
sonar.coverage.exclusionsproperty is set correctly, excluding test files, test utilities, generated code, and documentation from coverage analysis. This helps provide a more accurate representation of the project's test coverage.
10-10: LGTM!Setting
sonar.teststo.ensures that all relevant test files in the directory are included in the SonarQube analysis, which is appropriate for the project structure.
11-11: LGTM!The
sonar.test.inclusionsproperty is set correctly, including files ending with_test.go, which ensures that all relevant Go test files are included in the SonarQube analysis.
12-12: LGTM!The
sonar.go.coverage.reportPathsproperty is set correctly, specifying the path to the coverage report file, which allows SonarQube to import and analyze the project's test coverage data.
14-14: LGTM!Setting
sonar.sourceEncodingtoUTF-8ensures that SonarQube correctly interprets the source files, and UTF-8 is a widely used and appropriate encoding for the project.
15-15: LGTM!Setting
sonar.scm.providertogitallows SonarQube to integrate with the project's Git repository, which is the appropriate SCM provider for the project.
16-16: LGTM!Enabling
sonar.scm.forceReloadAllensures that SonarQube reloads all SCM data and bases the analysis on the latest repository state, which is appropriate for the project to ensure accurate analysis results.
17-17: LGTM!Enabling
sonar.pullrequest.github.summary_commentallows SonarQube to post a summary comment on GitHub pull requests, enhancing integration with GitHub workflows and providing quick feedback on code quality and potential issues directly within the pull request.x/accounts/defaults/base/CHANGELOG.md (1)
1-26: Excellent addition of a structured changelog file!The introduction of the
CHANGELOG.mdfile is a great step towards improving the documentation and communication of changes within the project. The outlined guiding principles and format align with industry best practices, promoting clarity, traceability, and effective change management.Keep up the good work in maintaining a comprehensive and well-structured changelog!
Tools
Markdownlint
26-26: null
Files should end with a single newline character(MD047, single-trailing-newline)
.github/pr_labeler.yml (1)
31-32: LGTM!The addition of the new path mapping for
"C:x/accounts/base"is consistent with the existing structure and naming conventions. It will help improve the organization and automation of the review process for contributions related to base accounts without introducing any breaking changes or compatibility issues.x/accounts/README.md (3)
64-64: Past review comment addressed.The missing
# Genesisheader has been added. Good job on addressing the feedback from the previous review.
Line range hint
66-72: LGTM!The explanation of creating accounts at genesis is clear and concise. No issues found.
Tools
LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...have not yet integrated with x/accounts but still need to parse account information...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
59-59: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
60-60: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
Line range hint
74-103: Looks good!The additional details on generating init messages offline and the example
genesis.jsonfile are valuable for developers. No issues found.Tools
LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...have not yet integrated with x/accounts but still need to parse account information...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
59-59: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
60-60: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
x/auth/keeper/grpc_query.go (5)
83-87: Improved account retrieval with legacy account fallback.The added code block introduces a fallback mechanism to handle cases where an account is not found in the primary storage. By attempting to retrieve a legacy account representation using the new
getFromXAccountsmethod, the system can now provide account information even for legacy accounts. This enhances compatibility and robustness of the account retrieval process.
231-237: Enhanced account info retrieval with legacy account fallback.The added code block introduces a fallback mechanism to handle cases where an account is not found in the primary storage. By attempting to retrieve a legacy account representation using the new
getFromXAccountsmethod, the system can now provide account info even for legacy accounts. This enhances compatibility and robustness of the account info retrieval process.The check for
xAccount.Info == nilensures that the legacy account can be properly encapsulated into a base account representation before returning the info.
260-263: Improved error handling with specific error variables.The introduction of the
errNotXAccountanderrInvalidLegacyAccountImplerror variables enhances error handling in the code. These variables provide specific error messages for different failure scenarios related to legacy accounts, making the code more readable and maintainable. The distinct error messages clearly convey the reason for the failure, aiding in debugging and error tracing.
265-281: Well-structured function for retrieving legacy accounts.The
getFromXAccountsfunction is a well-structured and modular implementation for retrieving legacy account representations. It encapsulates the necessary logic and performs appropriate checks to ensure the account is an x/account and the response is valid.The use of the
AccountsModKeeperand theQuerymethod allows for a flexible and extensible approach to querying legacy accounts. The function returns specific errors (errNotXAccountanderrInvalidLegacyAccountImpl) for different failure scenarios, enhancing error handling and making the code more maintainable.The function is a valuable addition to the codebase, improving the system's ability to handle legacy accounts seamlessly.
Line range hint
1-281: Adherence to Uber Golang style guide.The code in this file adheres to the Uber Golang style guide. The naming conventions, formatting, and code structure are consistent with the guidelines. The functions are well-documented with clear and concise comments, making the code readable and maintainable.
The error handling is consistent and follows best practices, using specific error variables and returning appropriate errors for different failure scenarios.
Overall, the code is well-written, idiomatic, and follows the recommended style and practices outlined in the Uber Golang style guide.
.github/dependabot.yml (1)
190-198: LGTM!The new Dependabot configuration entry for the
x/accounts/defaults/basedirectory follows the established conventions and looks good..github/workflows/test.yml (1)
940-961: LGTM!The
test-x-accounts-basejob is well-structured and follows best practices:
- It is triggered only when there are changes in the
x/accounts/defaults/basemodule, which is an efficient way to run tests only when necessary.- It uses the latest versions of the actions to stay up-to-date with the latest features and bug fixes.
- The
go testcommand is run with the-mod=readonlyflag to ensure reproducibility of the tests.- The
go testcommand generates a coverage profile to measure the test coverage of the module.
|
|
||
| # Changelog | ||
|
|
||
| ## [Unreleased] |
There was a problem hiding this comment.
Add a newline character at the end of the file.
The static analysis tool has flagged a missing newline character at the end of the file. To address this and ensure compliance with common coding conventions, please add a newline character after the last line of the file.
## [Unreleased]
+Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## [Unreleased] | |
| ## [Unreleased] | |
Tools
Markdownlint
26-26: null
Files should end with a single newline character(MD047, single-trailing-newline)
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
(cherry picked from commit 81ec7ea) # Conflicts: # simapp/go.mod # simapp/v2/go.mod # tests/go.mod # x/accounts/go.mod # x/group/go.mod
Description
This PR allows x/accounts to be queriable from the auth's gRPC query service, specifically
AccountInfoandAccountmethods.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
QueryLegacyAccountandQueryLegacyAccountResponsein the Cosmos SDK.Bug Fixes
Documentation
Tests