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

feat: introduce x/validate and split x/auth/tx/config #21822

Merged
merged 9 commits into from
Sep 20, 2024
Merged

Conversation

julienrbrt
Copy link
Member

@julienrbrt julienrbrt commented Sep 19, 2024

Description

  • Split x/auth/tx/config into two
    • x/auth/tx/config provides txconfig and txoptions (stays only a depinject module like in 0.50)
    • x/validate appmodule provides ante/post handlers and tx validators
  • Move back auth proto to main buf module (as still included in sdk)
  • Remove options from x/auth/tx/config (proto breaking, but not in state machine, so should be fine)

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, you can find examples of the prefixes below:
  • 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.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

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

  • New Features

    • Introduced the x/validate module for transaction validation and handler setup.
    • Enhanced the application configuration with a new validation module across various components.
  • Bug Fixes

    • Updated account information representation in responses for clarity.
  • Documentation

    • Added details about the x/validate module and its functionalities in the README.
    • Improved the structure and content of the x/auth/tx documentation for better navigation.
  • Refactor

    • Simplified the transaction configuration and validation logic, removing unnecessary dependencies and fields.
    • Renamed package from tx to validate to better reflect its purpose.
  • Chores

    • Updated configuration files and removed obsolete ones related to protobuf generation.

Copy link
Contributor

coderabbitai bot commented Sep 19, 2024

Walkthrough

Walkthrough

The changes in this pull request introduce a new x/validate module to the Cosmos SDK, which facilitates the management of ante and post handlers, as well as transaction validators. The modifications also include updates to existing modules, such as x/auth/tx/config, which has been refactored to streamline transaction handling. Additionally, various documentation files have been updated to reflect these changes, enhancing the overall structure and clarity of the codebase.

Changes

File Path Change Summary
.github/CODEOWNERS Added new code owners for /x/auth/tx/config and /x/validate paths.
.github/pr_labeler.yml Added labeling for x/validate/**/* path.
CHANGELOG.md Introduced x/validate module for transaction validators and updated x/auth/tx/config module to remove automatic sign mode inclusion.
UPGRADING.md Updated gRPC Web section, added "Sign Mode Textual" configuration, removed x/crisis module, and introduced x/validate module.
api/cosmos/auth/v1beta1/accounts.pulsar.go Renamed info field to base in QueryLegacyAccountResponse.
api/cosmos/tx/config/v1/config.pulsar.go Removed SkipAnteHandler and SkipPostHandler fields from Config struct.
api/cosmos/validate/module/v1/module.pulsar.go Added Module message type for the validation module.
proto/cosmos/tx/config/v1/config.proto Removed skip_ante_handler and skip_post_handler fields from Config message.
proto/cosmos/validate/module/v1/module.proto Added Module message for the validation module.
simapp/app_config.go Integrated new validation module and refactored transaction configuration.
simapp/simd/cmd/testnet_test.go Added configurator.ValidateModule() in the testnet command function.
tests/e2e/... Added configurator.ValidateModule() in various test configurations to enhance validation during tests.
testutil/configurator/configurator.go Added ValidateModule() function for the validation module configuration.
x/README.md Added reference to the Validate module in documentation.
x/accounts/defaults/base/account.go Renamed Info field to Base in QueryLegacyAccountResponse.
x/auth/keeper/grpc_query.go Updated account existence check from Info to Base.
x/auth/proto/buf.gen.gogo.yaml Removed configuration for gocosmos and grpc-gateway plugins.
x/auth/proto/buf.gen.pulsar.yaml Removed configuration for protobuf generation.
x/auth/proto/buf.yaml Removed versioning and dependency declarations for the auth module.
x/auth/tx/README.md Reorganized documentation, added sections for x/auth/tx/config.
x/auth/tx/config/depinject.go Refactored module structure, removed several fields from ModuleInputs, and simplified transaction configuration handling.
x/tx/README.md Adjusted formatting for improved readability.
x/validate/README.md Introduced documentation for the new x/validate module, detailing its purpose and functionality.
x/validate/depinject.go Added module for transaction validation and configuration, including ante and post handlers.
x/validate/keys.go Defined constant ModuleName for the validation module.
x/validate/module.go Renamed package from tx to validate.

Possibly related PRs

  • feat(auth): support accounts from auth #21688: This PR introduces support for querying accounts from the authentication module's gRPC query service, which is directly related to the changes in the main PR that enhance the ownership structure and management of transaction validators and handlers, including the x/validate module.
  • docs(x/auth): vesting #21715: This PR focuses on documentation changes related to vesting functionality within the x/auth module, which is relevant as it reflects the ongoing adjustments in account management and the transition towards using the x/accounts module, aligning with the ownership and validation changes in the main PR.

Suggested labels

C:x/upgrade


Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 87751ac and e54aea3.

Files selected for processing (5)
  • CHANGELOG.md (2 hunks)
  • tests/e2e/accounts/base_account_test.go (1 hunks)
  • tests/integration/auth/keeper/accounts_retro_compatibility_test.go (2 hunks)
  • x/auth/tx/config/depinject.go (3 hunks)
  • x/validate/depinject.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • CHANGELOG.md
  • x/validate/depinject.go
Additional context used
Path-based instructions (3)
tests/e2e/accounts/base_account_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/accounts_retro_compatibility_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/auth/tx/config/depinject.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (8)
tests/e2e/accounts/base_account_test.go (1)

9-9: Verify the usage of the imported package.

The gogoproto package is imported but not used in the provided code snippet. Please ensure that the package is being used somewhere in the file. If not, consider removing the import to keep the codebase clean and avoid unnecessary dependencies.

Run the following script to verify the package usage:

Verification successful

The gogoproto package import is correctly used and necessary.

The verification process confirms that the gogoproto package is actively used in the file. It's utilized for type definitions (gogoproto.Message) and function calls (gogoproto.MessageName). The import is justified and contributes to the functionality of the code.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `gogoproto` package in the file.

# Test: Search for the package usage. Expect: At least one occurrence.
rg --type go -w gogoproto tests/e2e/accounts/base_account_test.go

Length of output: 303

x/auth/tx/config/depinject.go (5)

27-28: LGTM!

The constant DepinjectModuleName is declared correctly and its value "tx" accurately represents the module's purpose.


40-46: Looks good!

The ModuleInputs struct is defined correctly with appropriate fields for the module's dependency injection. The optional fields provide flexibility for customization.


52-52: LGTM!

The BaseAppOption field in the ModuleOutputs struct is defined correctly with an appropriate type and a clear comment explaining its purpose.


88-91: Looks good!

The anonymous function assigned to the BaseAppOption field is implemented correctly. It sets the transaction decoder and encoder for the base app using the txConfig, ensuring proper handling of transactions.


97-97: LGTM!

The NewBankKeeperCoinMetadataQueryFn function is implemented correctly. It creates a coin metadata query function using the provided bank keeper, allowing retrieval of metadata for a specified denom. The function handles errors appropriately and has a clear comment explaining its purpose.

tests/integration/auth/keeper/accounts_retro_compatibility_test.go (2)

Line range hint 49-55: LGTM!

The change in the field name from Info to Base aligns with the alteration mentioned in the AI-generated summary. The initialization of the Base field with an authtypes.BaseAccount object is appropriate.


89-89: LGTM!

The assertion has been updated to compare infoResp.Info to valid.retroCompat.Base, reflecting the structural change in authtypes.QueryLegacyAccountResponse. This change aligns with the alteration mentioned in the AI-generated summary.


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>, please review it.
    -- Generate unit testing code 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 testing code 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@julienrbrt julienrbrt changed the title Julien/validate feat: introduce x/validate and split x/auth/tx/config Sep 19, 2024
protoiface "google.golang.org/protobuf/runtime/protoiface"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
io "io"
reflect "reflect"

Check notice

Code scanning / CodeQL

Sensitive package import Note

Certain system packages contain functions which may be a possible source of non-determinism
switch fd.FullName() {
default:
if fd.IsExtension() {
panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.validate.module.v1.Module"))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
if fd.IsExtension() {
panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.validate.module.v1.Module"))
}
panic(fmt.Errorf("message cosmos.validate.module.v1.Module does not contain field %s", fd.FullName()))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
@julienrbrt julienrbrt added the backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release label Sep 19, 2024
@julienrbrt
Copy link
Member Author

We can maybe move x/auth/ante to x/validate eventually, but it isn't really necessary right now and for 0.52 (this limits the amount of breakage anyway)

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.

Actionable comments posted: 3

Outside diff range and nitpick comments (14)
x/validate/README.md (1)

4-4: Consider adding a comma before "and" (optional).

According to the grammar rule, a comma should be used before "and" when it connects two independent clauses, unless they are closely connected and short. In this case, the clauses are closely connected and relatively short, so the comma is optional. However, adding the comma may improve readability:

-This module is only required when using runtime and runtime v2 and you want to make use of the pre-defined ante/poste handlers or tx validators.
+This module is only required when using runtime and runtime v2, and you want to make use of the pre-defined ante/poste handlers or tx validators.
Tools
LanguageTool

[uncategorized] ~4-~4: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...quired when using runtime and runtime v2 and you want to make use of the pre-defined...

(COMMA_COMPOUND_SENTENCE)

x/auth/tx/config/depinject.go (3)

54-54: Clarify the Comment for BaseAppOption

The comment for BaseAppOption can be made more concise and aligned with GoDoc standards for better readability.

Consider rephrasing the comment:

- BaseAppOption   runtime.BaseAppOption // This is only useful for chains using baseapp.
+ BaseAppOption   runtime.BaseAppOption // Useful only for chains using baseapp.

99-99: Align Function Comment with GoDoc Conventions

The comment for NewBankKeeperCoinMetadataQueryFn should start with the function name and clearly describe its purpose, following GoDoc guidelines.

Update the comment to:

- // NewBankKeeperCoinMetadataQueryFn creates a new Textual struct using the given BankKeeper to retrieve coin metadata.
+ // NewBankKeeperCoinMetadataQueryFn creates a new `CoinMetadataQueryFn` using the provided `BankKeeper` to retrieve coin metadata.

41-48: Consider Grouping Related Dependencies

For improved readability and consistency with the Uber Golang style guide, consider grouping related codec dependencies together in the ModuleInputs struct.

Arrange the fields as follows:

 Config                 *txconfigv1.Config
-AddressCodec           address.Codec
-ValidatorAddressCodec  address.ValidatorAddressCodec
 Codec                  codec.Codec
 ProtoFileResolver      txsigning.ProtoFileResolver
+AddressCodec           address.Codec
+ValidatorAddressCodec  address.ValidatorAddressCodec
 Environment            appmodulev2.Environment
 CustomSignModeHandlers func() []txsigning.SignModeHandler `optional:"true"`
 CustomGetSigners       []txsigning.CustomGetSigner        `optional:"true"`
x/validate/depinject.go (2)

3-22: Organize import statements according to Go conventions

The import statements should be organized into three groups separated by blank lines:

  1. Standard library packages
  2. Third-party packages
  3. Local packages

This improves readability and maintains conformity with the Uber Go Style Guide.

Example:

import (
	"errors"
	"fmt"

	modulev1 "cosmossdk.io/api/cosmos/validate/module/v1"
	appmodulev2 "cosmossdk.io/core/appmodule/v2"
	"cosmossdk.io/core/server"
	"cosmossdk.io/core/transaction"
	"cosmossdk.io/depinject"
	"cosmossdk.io/depinject/appconfig"

	"github.com/cosmos/cosmos-sdk/baseapp"
	"github.com/cosmos/cosmos-sdk/client"
	"github.com/cosmos/cosmos-sdk/runtime"
	sdk "github.com/cosmos/cosmos-sdk/types"
	"github.com/cosmos/cosmos-sdk/x/auth/ante"
	"github.com/cosmos/cosmos-sdk/x/auth/ante/unorderedtx"
	"github.com/cosmos/cosmos-sdk/x/auth/posthandler"
	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

59-59: Use descriptive variable names

The variable svd is not immediately descriptive. Consider renaming it to sigVerificationDecorator to enhance code readability.

Apply this diff:

-svd := ante.NewSigVerificationDecorator(
+sigVerificationDecorator := ante.NewSigVerificationDecorator(
    in.AccountKeeper,
    in.TxConfig.SignModeHandler(),
    ante.DefaultSigVerificationGasConsumer,
    in.AccountAbstractionKeeper,
)

And update its usage:

-return ModuleOutputs{
-    Module:        NewAppModule(svd, feeTxValidator, unorderedTxValidator, in.ExtraTxValidators...),
+return ModuleOutputs{
+    Module:        NewAppModule(sigVerificationDecorator, feeTxValidator, unorderedTxValidator, in.ExtraTxValidators...),
UPGRADING.md (6)

109-109: Correct capitalization of "gRPC-Web"

In the sentence, "Grpc-web embedded client has been removed from the server.", "Grpc-web" should be capitalized as "gRPC-Web" to match the standard naming convention.

Apply this diff to fix the capitalization:

-Grpc-web embedded client has been removed from the server. If you would like to use grpc-web, you can use the [envoy proxy](https://www.envoyproxy.io/docs/envoy/latest/start/start). Here's how to set it up:
+gRPC-Web embedded client has been removed from the server. If you would like to use gRPC-Web, you can use the [Envoy proxy](https://www.envoyproxy.io/docs/envoy/latest/start/start). Here's how to set it up:

324-324: Use "no longer" instead of "no more"

In the sentence, "sign mode textual is no more automatically configured...", "no more" should be replaced with "no longer" for correct usage.

Apply this diff to correct the phrasing:

-With the split of `x/auth/tx/config` in two (x/auth/tx/config as depinject module for txconfig and tx options) and `x/validate`, sign mode textual is no more automatically configured when using runtime (it was previously the case).
+With the split of `x/auth/tx/config` into two (x/auth/tx/config as a depinject module for txconfig and tx options) and `x/validate`, sign mode textual is no longer automatically configured when using runtime (it was previously the case).

325-325: Correct comparative conjunction in the sentence

The phrase "For the same instructions than for legacy app wiring..." should be corrected to "For the same instructions as for legacy app wiring..." to use the correct comparative conjunction.

Apply this diff to fix the grammatical error:

-For the same instructions than for legacy app wiring to enable sign mode textual (see in v0.50 UPGRADING documentation).
+Refer to the same instructions as for legacy app wiring to enable sign mode textual (see in v0.50 UPGRADING documentation).
Tools
LanguageTool

[grammar] ~325-~325: Comparison is written ‘the same … as’.
Context: ...ly the case). For the same instructions than for legacy app wiring to enable sign mo...

(THE_SAME_AS)


467-467: Improve sentence clarity

The sentence can be rephrased for better readability: "The x/crisis module has been removed because it is no longer supported or functional."

Apply this diff to improve readability:

-The `x/crisis` module was removed due to it not being supported or functional any longer. 
+The `x/crisis` module has been removed because it is no longer supported or functional.

513-513: Add comma after "x/validate" for clarity

In the sentence "Introducing x/validate a module that is solely used...", a comma should be added after "x/validate" to separate the introductory phrase.

Apply this diff to improve clarity:

-Introducing `x/validate` a module that is solely used for registering default ante/post handlers and global transaction validators when using runtime and runtime/v2.
+Introducing `x/validate`, a module that is solely used for registering default ante/post handlers and global transaction validators when using runtime and runtime/v2.

514-514: Add missing comma after "however"

In the sentence "You can however always extend them by adding extra tx validators...", a comma should be added after "however" to improve readability.

Apply this diff to fix the missing comma:

-You can however always extend them by adding extra tx validators (see `x/validate` documentation).
+You can, however, always extend them by adding extra tx validators (see `x/validate` documentation).
Tools
LanguageTool

[uncategorized] ~514-~514: Possible missing comma found.
Context: ...rs, no need to use this module. You can however always extend them by adding extra tx v...

(AI_HYDRA_LEO_MISSING_COMMA)

CHANGELOG.md (2)

50-50: Improve clarity and grammar in the explanatory note

The sentence on line 50 is grammatically incorrect and could be rephrased for better readability.

Apply this diff to improve clarity:

-    * In comparison to x/auth/tx/config, there is no app config to skip ante/post handlers, as overwriting them is baseapp or not injecting the x/validate module has the same effect.
+    * In comparison to x/auth/tx/config, there is no app config to skip ante/post handlers, because overwriting them in baseapp or not injecting the x/validate module has the same effect.

66-67: Correct grammatical errors and improve clarity in documentation

There are grammatical issues on lines 66-67 that can be corrected for better readability.

Apply the following changes to improve clarity:

-* (x/auth/tx/config)  [#21822](https://github.com/cosmos/cosmos-sdk/pull/21822) Sign mode textual is no more automatically added to tx config when using runtime. Should be added manually on the server side.
+* (x/auth/tx/config)  [#21822](https://github.com/cosmos/cosmos-sdk/pull/21822) Sign mode textual is no longer automatically added to tx config when using runtime. It should be added manually on the server side.
-* (x/auth/tx/config)  [#21822](https://github.com/cosmos/cosmos-sdk/pull/21822) This depinject module now only provide txconfig and tx config options. `x/validate` now handles the providing of ante/post handlers, alongside tx validators for v2. The corresponding app config options have been removed from the depinject module config.
+* (x/auth/tx/config)  [#21822](https://github.com/cosmos/cosmos-sdk/pull/21822) This depinject module now only provides txconfig and tx config options. `x/validate` now handles providing ante/post handlers, alongside tx validators for v2. The corresponding app config options have been removed from the depinject module config.
Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bdda21f and 87751ac.

Files ignored due to path filters (2)
  • x/auth/proto/buf.lock is excluded by !**/*.lock
  • x/auth/types/accounts.pb.go is excluded by !**/*.pb.go
Files selected for processing (40)
  • .github/CODEOWNERS (2 hunks)
  • .github/pr_labeler.yml (1 hunks)
  • CHANGELOG.md (2 hunks)
  • UPGRADING.md (4 hunks)
  • api/cosmos/auth/v1beta1/accounts.pulsar.go (16 hunks)
  • api/cosmos/tx/config/v1/config.pulsar.go (2 hunks)
  • api/cosmos/validate/module/v1/module.pulsar.go (1 hunks)
  • proto/cosmos/tx/config/v1/config.proto (0 hunks)
  • proto/cosmos/validate/module/v1/module.proto (1 hunks)
  • simapp/app_config.go (3 hunks)
  • simapp/simd/cmd/testnet_test.go (1 hunks)
  • simapp/v2/app_config.go (3 hunks)
  • tests/e2e/auth/keeper/app_config.go (1 hunks)
  • tests/e2e/baseapp/block_gas_test.go (1 hunks)
  • tests/integration/bank/app_test.go (1 hunks)
  • tests/integration/distribution/appconfig.go (1 hunks)
  • tests/integration/evidence/app_config.go (1 hunks)
  • tests/integration/mint/app_config.go (1 hunks)
  • tests/integration/protocolpool/app_config.go (1 hunks)
  • tests/integration/runtime/query_test.go (1 hunks)
  • tests/integration/server/grpc/out_of_gas_test.go (1 hunks)
  • tests/integration/server/grpc/server_test.go (1 hunks)
  • tests/integration/slashing/app_config.go (1 hunks)
  • tests/integration/slashing/slashing_test.go (1 hunks)
  • tests/integration/staking/app_config.go (1 hunks)
  • testutil/configurator/configurator.go (2 hunks)
  • testutil/types.go (1 hunks)
  • x/README.md (1 hunks)
  • x/accounts/defaults/base/account.go (1 hunks)
  • x/auth/keeper/grpc_query.go (1 hunks)
  • x/auth/proto/buf.gen.gogo.yaml (0 hunks)
  • x/auth/proto/buf.gen.pulsar.yaml (0 hunks)
  • x/auth/proto/buf.yaml (0 hunks)
  • x/auth/tx/README.md (2 hunks)
  • x/auth/tx/config/depinject.go (3 hunks)
  • x/tx/README.md (5 hunks)
  • x/validate/README.md (1 hunks)
  • x/validate/depinject.go (1 hunks)
  • x/validate/keys.go (1 hunks)
  • x/validate/module.go (1 hunks)
Files not reviewed due to no reviewable changes (4)
  • proto/cosmos/tx/config/v1/config.proto
  • x/auth/proto/buf.gen.gogo.yaml
  • x/auth/proto/buf.gen.pulsar.yaml
  • x/auth/proto/buf.yaml
Files skipped from review due to trivial changes (7)
  • .github/CODEOWNERS
  • api/cosmos/tx/config/v1/config.pulsar.go
  • api/cosmos/validate/module/v1/module.pulsar.go
  • x/README.md
  • x/accounts/defaults/base/account.go
  • x/validate/keys.go
  • x/validate/module.go
Additional context used
Path-based instructions (27)
CHANGELOG.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

UPGRADING.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

api/cosmos/auth/v1beta1/accounts.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

simapp/app_config.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

simapp/simd/cmd/testnet_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"

simapp/v2/app_config.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

tests/e2e/auth/keeper/app_config.go (2)

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"

tests/e2e/baseapp/block_gas_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/bank/app_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/distribution/appconfig.go (2)

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"

tests/integration/evidence/app_config.go (2)

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"

tests/integration/mint/app_config.go (2)

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"

tests/integration/protocolpool/app_config.go (2)

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"

tests/integration/runtime/query_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/server/grpc/out_of_gas_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/server/grpc/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"

tests/integration/slashing/app_config.go (2)

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"

tests/integration/slashing/slashing_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/staking/app_config.go (2)

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"

testutil/configurator/configurator.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

testutil/types.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/auth/keeper/grpc_query.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/auth/tx/README.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

x/auth/tx/config/depinject.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/tx/README.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

x/validate/README.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

x/validate/depinject.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

LanguageTool
UPGRADING.md

[grammar] ~325-~325: Comparison is written ‘the same … as’.
Context: ...ly the case). For the same instructions than for legacy app wiring to enable sign mo...

(THE_SAME_AS)


[uncategorized] ~514-~514: Possible missing comma found.
Context: ...rs, no need to use this module. You can however always extend them by adding extra tx v...

(AI_HYDRA_LEO_MISSING_COMMA)

x/auth/tx/README.md

[grammar] ~76-~76: The verb after “to” should be in the base form as part of the to-infinitive. A verb can take many forms, but the base form is always used in the to-infinitive.
Context: ...ject module. The depinject module is to outputs the TxConfig and TxConfigOptions fo...

(TO_NON_BASE)

x/validate/README.md

[uncategorized] ~4-~4: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...quired when using runtime and runtime v2 and you want to make use of the pre-defined...

(COMMA_COMPOUND_SENTENCE)


[uncategorized] ~7-~7: Possible missing comma found.
Context: ...dators. ::: The x/validate is an app module solely there to setup ante/post handler...

(AI_HYDRA_LEO_MISSING_COMMA)

buf
proto/cosmos/validate/module/v1/module.proto

5-5: import "cosmos/app/v1alpha1/module.proto": file does not exist

(COMPILE)

Markdownlint
x/auth/tx/README.md

23-23: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


24-24: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


25-25: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


26-26: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


27-27: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


28-28: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


29-29: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


30-30: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


31-31: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


32-32: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


33-33: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


34-34: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


35-35: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


36-36: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


37-37: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


38-38: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


39-39: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


40-40: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


41-41: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)

x/tx/README.md

23-23: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


24-24: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


25-25: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


26-26: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


27-27: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


28-28: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


29-29: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


30-30: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


31-31: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


32-32: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)

Additional comments not posted (42)
proto/cosmos/validate/module/v1/module.proto (2)

8-12: LGTM!

The introduction of the Module message as a configuration object for the x/validate module is a great addition. It enhances the modularity and reusability of validation logic within the Cosmos SDK.


5-5: Verify the import statement.

The static analysis tool has flagged the import statement import "cosmos/app/v1alpha1/module.proto"; indicating that the file does not exist.

Please verify if the file exists at the specified path and make necessary corrections if required.

Verification successful

Import statement verified: File exists with a slight path difference

The imported file cosmos/app/v1alpha1/module.proto exists in the repository, but it's located at proto/cosmos/app/v1alpha1/module.proto. The current import statement might work as-is, depending on how import paths are resolved in your project. However, you may want to consider updating the import statement to include the proto/ prefix for consistency.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the imported file exists.

# Test: Check if the file exists. Expect: File found.
fd --no-ignore --type file --full-path "cosmos/app/v1alpha1/module.proto"

Length of output: 112

Tools
buf

5-5: import "cosmos/app/v1alpha1/module.proto": file does not exist

(COMPILE)

tests/integration/mint/app_config.go (1)

22-22: Excellent addition of the validation module to the application configuration!

The inclusion of configurator.ValidateModule() in the AppConfig initialization is a great step towards enhancing the robustness and integrity of the application. This module will likely be responsible for validating transactions and other operations, rejecting any invalid ones and maintaining data correctness.

This change aligns with the best practice of validating inputs and ensuring data integrity in the application. Well done!

tests/e2e/auth/keeper/app_config.go (1)

23-23: LGTM! The addition of the ValidateModule() enhances the application's configuration management.

The inclusion of the configurator.ValidateModule() in the AppConfig variable initialization is a positive change that aligns with the PR objectives of introducing the new x/validate module. This addition incorporates validation capabilities into the application's configuration, ensuring that the settings adhere to expected formats and constraints. By validating the configuration, the application can potentially prevent runtime errors related to misconfigured settings, thereby improving its overall robustness and reliability.

tests/integration/evidence/app_config.go (1)

24-24: LGTM!

The addition of configurator.ValidateModule() to the AppConfig variable initialization is a positive change that enhances the application's configuration management process by integrating validation capabilities. This change is likely to improve the application's robustness by ensuring that the configuration adheres to certain rules or constraints, thereby maintaining data integrity and correctness.

The change is well-integrated into the existing code and is unlikely to have any negative impact on the application's functionality.

testutil/types.go (1)

7-26: LGTM! The addition of the new module name constant and the formatting enhancements improve the codebase.

The introduction of the ValidateModuleName constant suggests an expansion of the SDK's capabilities, potentially adding new validation functionalities. This change aligns with the PR objectives mentioned in the summary.

The reformatting of the module name constants enhances the readability and maintainability of the code, making it easier for developers to reference and utilize these constants across the SDK.

Great work on improving both the functionality and the code quality!

tests/integration/protocolpool/app_config.go (1)

24-24: LGTM!

The addition of configurator.ValidateModule() to the AppConfig variable initialization is a positive change that aligns with the PR objective of introducing the x/validate module to improve the validation process within the SDK. This change enhances the application's robustness by incorporating validation logic into its configuration.

tests/integration/distribution/appconfig.go (1)

24-24: Approve the addition of the validation module.

The introduction of the ValidateModule() in the AppConfig initialization is a good practice, as it can help ensure that configurations adhere to specified rules or constraints, potentially improving robustness and error handling.

Please verify if there are any dependencies or downstream impacts of adding this module. Consider running the following script to search for potential usage or references to the ValidateModule() across the codebase:

Verification successful

Confirm the addition of ValidateModule() as consistent and appropriate

The addition of configurator.ValidateModule() in the AppConfig initialization is consistent with the usage pattern observed across multiple test files and app configurations in the codebase. This module appears to be a standard component in the setup process for tests and possibly for the actual application.

Given its widespread use and consistent integration alongside other modules, it's unlikely to introduce any unexpected behavior or have negative downstream impacts. The change aligns with the existing practices in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage or references to the `ValidateModule()` 
rg --type go -i -C 5 'ValidateModule\(\)'

Length of output: 11315

tests/integration/staking/app_config.go (1)

25-25: LGTM! The addition of the validation module is a positive change.

The introduction of the configurator.ValidateModule() in the AppConfig is a straightforward and beneficial modification. This module is likely to improve the application's error handling and ensure data integrity by validating certain aspects of its operation.

The change enhances the overall robustness of the application without introducing any apparent issues or complexities.

tests/integration/slashing/app_config.go (1)

26-26: LGTM! The addition of the ValidateModule enhances the application's robustness.

The inclusion of the configurator.ValidateModule() in the AppConfig is a positive change that strengthens the application's validation capabilities. This module will likely ensure that transactions and states within the application adhere to specified rules and constraints, thereby maintaining data integrity and correctness.

By integrating this validation module into the application's configuration, error handling and validation processes during runtime should be improved. This proactive approach enhances the overall robustness of the application.

x/validate/README.md (4)

1-8: The module description is clear and informative.

The overview of the x/validate module effectively communicates its purpose and key features. It clearly explains how the module facilitates the setup of ante/post handlers and global transaction validators in runtime applications, and how it leverages dependency injection to automatically integrate these components.

Tools
LanguageTool

[uncategorized] ~4-~4: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...quired when using runtime and runtime v2 and you want to make use of the pre-defined...

(COMMA_COMPOUND_SENTENCE)


[uncategorized] ~7-~7: Possible missing comma found.
Context: ...dators. ::: The x/validate is an app module solely there to setup ante/post handler...

(AI_HYDRA_LEO_MISSING_COMMA)


9-24: The "Extra TxValidators" section provides valuable guidance.

The section offers a clear explanation of how developers can add extra transaction validators to their application, along with a relevant example use case. The included code snippet is a helpful reference for configuring custom validators using depinject in the app.go file.


26-35: The "Storage" section provides essential information.

The section clearly communicates that the x/validate module does not have a store key and reminds developers to include the module name in the SkipStoreKeys runtime config. The provided code snippet is a helpful reference for correctly adding the module name to the SkipStoreKeys array.


7-7: Ignore the static analysis hint about a missing comma.

The hint suggesting a possible missing comma in the sentence "The x/validate is an app module solely there to setup ante/post handlers on a runtime app (via baseapp options) and the global tx validators on a runtime/v2 app (via app module)." is a false positive. The sentence is a single independent clause with multiple prepositional phrases, and there is no grammatical requirement for a comma in this structure. Adding a comma may actually disrupt the flow and readability of the sentence.

Tools
LanguageTool

[uncategorized] ~7-~7: Possible missing comma found.
Context: ...dators. ::: The x/validate is an app module solely there to setup ante/post handler...

(AI_HYDRA_LEO_MISSING_COMMA)

.github/pr_labeler.yml (1)

83-84: LGTM!

The addition of the C:x/validate label for the x/validate/**/* path aligns with the PR objective of introducing the new x/validate module. This change enhances the existing labeling structure by ensuring that contributions to the validation aspect of the project can be easily identified and managed.

simapp/simd/cmd/testnet_test.go (1)

37-37: LGTM!

The addition of configurator.ValidateModule() to the list of modules being configured for the test is a positive change. It ensures that the new x/validate module is included in the test setup, which aligns with the PR objective and enhances the robustness of the tests.

tests/integration/server/grpc/out_of_gas_test.go (1)

48-48: Enhancing test suite with validation capabilities

The addition of configurator.ValidateModule() to the list of modules being initialized in the SetupSuite function is a positive change that improves the robustness of the integration tests. By incorporating validation capabilities, the test suite can now cover scenarios that involve validation checks on transactions or operations. This change aligns with best practices of comprehensive testing and enhances the overall quality of the test suite.

tests/integration/runtime/query_test.go (1)

49-49: LGTM! The addition of the validation module enhances the integration tests.

The inclusion of configurator.ValidateModule() in the test fixture setup aligns with the PR objective of introducing the new x/validate module to improve the validation process within the SDK.

This change could enable more comprehensive testing scenarios that verify certain conditions or constraints are met during test execution, thereby improving the robustness and reliability of the integration tests.

tests/integration/slashing/slashing_test.go (1)

70-70: LGTM! Adding validation to the test setup is a good enhancement.

Including the ValidateModule in the app configuration for the integration test is a positive change. It ensures that the slashing messages being tested adhere to the expected validation rules, thereby improving the robustness and reliability of the tests. Catching any validation issues early in the testing phase helps maintain a high quality codebase.

The TestSlashingMsgs function provides sufficient coverage for the core slashing messages (MsgCreateValidator and MsgUnjail). It tests the success scenario of creating a validator and the failure scenario of unjailing an unknown validator, covering the key functionalities.

tests/e2e/baseapp/block_gas_test.go (1)

91-91: LGTM!

The addition of the ValidateModule to the app configuration is a good change. It ensures that the validation functionality is available for use in the tests, thereby improving the robustness and reliability of the test suite.

x/auth/keeper/grpc_query.go (2)

234-234: Verify that the Base field provides the necessary account information.

The condition for checking account existence has been updated to use xAccount.Base instead of xAccount.Info. Ensure that the Base field contains all the required account information for the desired functionality.


237-237: Ensure that the Base field contains the expected account details.

The response now returns xAccount.Base instead of xAccount.Info. Verify that the Base field provides all the necessary account information expected by the calling code.

x/auth/tx/README.md (3)

80-87: LGTM!

The "Storage" section provides clear information about the module's lack of a store key and includes a helpful code snippet to illustrate the configuration of SkipStoreKeys. The changes in this section are accurate and informative.


Line range hint 88-137: LGTM!

The "CLI" section is well-documented and provides detailed information about the CLI commands for querying transactions and blocks. The examples are clear and cover various scenarios, making it easy for users to understand and use the CLI commands effectively. The changes in this section are comprehensive and informative.

Tools
LanguageTool

[grammar] ~76-~76: The verb after “to” should be in the base form as part of the to-infinitive. A verb can take many forms, but the base form is always used in the to-infinitive.
Context: ...ject module. The depinject module is to outputs the TxConfig and TxConfigOptions fo...

(TO_NON_BASE)


Line range hint 138-235: LGTM!

The "gRPC" section provides detailed information about the gRPC endpoints for encoding and decoding transactions. The examples are well-documented and include sample requests and responses, making it easy for users to understand and interact with the gRPC endpoints. The changes in this section are comprehensive and informative.

Tools
Markdownlint

23-23: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


24-24: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


25-25: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


26-26: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


27-27: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


28-28: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


29-29: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


30-30: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


31-31: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


32-32: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


33-33: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


34-34: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


35-35: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


36-36: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


37-37: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


38-38: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


39-39: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


40-40: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


41-41: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)

tests/integration/server/grpc/server_test.go (1)

59-59: LGTM! The addition of the ValidateModule aligns with the PR objectives.

The inclusion of the configurator.ValidateModule() in the test suite setup is a positive change that incorporates validation functionality into the testing framework. This addition has the potential to enhance the robustness of the tests by ensuring data integrity and correctness.

simapp/app_config.go (4)

31-31: LGTM!

The import statement for the validatemodulev1 module is correctly added.


73-78: LGTM!

The import statements for the authtxconfig and validate modules are correctly added.


186-187: LGTM!

Adding the authtxconfig.DepinjectModuleName and validate.ModuleName to the SkipStoreKeys list is a valid configuration for modules that do not require a store key.


191-198: LGTM!

The module configurations for the authtxconfig.DepinjectModuleName and validate.ModuleName modules are correctly added.

  • The authtxconfig.DepinjectModuleName module is configured as a depinject module to provide transaction configuration.
  • The validate.ModuleName module is configured as an application module for validation functionality.

These configurations align with the overall changes to integrate the x/validate module and refactor the transaction configuration.

simapp/v2/app_config.go (4)

31-31: LGTM!

The import statement for the validation module is correctly added.


72-73: LGTM!

The import statement for the authtxconfig module is correctly added.


188-193: LGTM!

The SkipStoreKeys field is correctly updated to include the authtxconfig.DepinjectModuleName and validate.ModuleName. This change aligns with the design decision to streamline the module's interaction with the state store.


196-203: LGTM!

The new entries in the configuration list correctly integrate the authtxconfig.DepinjectModuleName and validate.ModuleName modules with their respective configurations. These changes enhance the application's modularity by incorporating validation capabilities and refining transaction handling.

testutil/configurator/configurator.go (2)

198-205: LGTM!

The ValidateModule function correctly registers the validation module and wraps its configuration using the appconfig.WrapAny function. The module name is defined in the testutil package, which is a good practice for consistency. The function follows the same pattern as other module configuration functions in the file.


191-195: Verify the module name change in the codebase.

The TxModule function correctly registers the transaction module and wraps its configuration using the appconfig.WrapAny function. The module name has been changed to indicate a shift in focus towards the authentication transaction configuration. The function follows the same pattern as other module configuration functions in the file.

Run the following script to verify the module name usage:

Verification successful

Module name change verified and properly implemented.

The change from testutil.TxModuleName to testutil.AuthTxConfigDepinjectModuleName has been successfully implemented:

  1. The old module name has been completely removed from the codebase.
  2. The new module name is used consistently in the testutil/configurator/configurator.go file.
  3. The constant AuthTxConfigDepinjectModuleName is properly defined in testutil/types.go with the value "tx".

The change appears to be isolated to the specific file under review, and there are no unexpected usages of the new module name throughout the codebase. The TxModule() function, which is used in various test configurations, is not directly affected by this change.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the transaction module match the new name.

# Test: Search for the old module name. Expect: No occurrences.
rg --type go $'testutil\.TxModuleName'

# Test: Search for the new module name. Expect: Only occurrences in the intended locations.
rg --type go $'testutil\.AuthTxConfigDepinjectModuleName'

Length of output: 321


Script:

#!/bin/bash
# Description: Further verify the transaction module references and constant definition

# Search for any references to 'tx' or 'transaction' module
rg --type go -i '(tx|transaction).*module'

# Look for the definition of the new module name constant
rg --type go 'AuthTxConfigDepinjectModuleName\s*='

# Check for any remaining references to 'TxModule' function or variable
rg --type go 'TxModule'

Length of output: 85159

tests/integration/bank/app_test.go (1)

100-100: LGTM! The addition of ValidateModule enhances the test suite.

The inclusion of configurator.ValidateModule() in the test suite configuration is a positive change that is likely to improve the robustness of the testing process by incorporating validation functionalities. This addition follows the existing pattern of module configuration and is placed appropriately in the list of configurations passed to simtestutil.SetupWithConfiguration.

api/cosmos/auth/v1beta1/accounts.pulsar.go (2)

375-375: LGTM!

The renaming of the Account field to Base enhances clarity and aligns with the BaseAccount type. The updated comments also provide better context for the field's purpose. The changes are consistent across the struct and its methods.

Also applies to: 943-944, 996-999


973-977: LGTM!

The GetBase method has been appropriately updated to return the Base field, aligning with the struct field renaming. The logic remains unchanged.

x/auth/tx/config/depinject.go (3)

28-29: Module Name Definition is Clear and Correct

The DepinjectModuleName constant is appropriately defined and clearly indicates the module name used for dependency injection.


41-48: Struct Fields in ModuleInputs Are Well-Defined

The ModuleInputs struct fields are properly declared with clear types, and the use of optional:"true" tags for CustomSignModeHandlers and CustomGetSigners is correctly applied for optional dependencies.


90-93: Correctly Setting Transaction Encoder and Decoder

The BaseAppOption function correctly sets the TxDecoder and TxEncoder, ensuring that transactions are properly encoded and decoded within the application.

Comment on lines +22 to +41
* [`x/auth/tx`](#xauthtx)
* [Abstract](#abstract)
* [Contents](#contents)
* [Transactions](#transactions)
* [`TxConfig`](#txconfig)
* [`TxBuilder`](#txbuilder)
* [`TxEncoder`/ `TxDecoder`](#txencoder-txdecoder)
* [Depinject \& App Module](#depinject--app-module)
* [Client](#client)
* [`x/auth/tx/config`](#xauthtxconfig)
* [Storage](#storage)
* [Client](#client)
* [CLI](#cli)
* [Query](#query)
* [Transactions](#transactions-1)
* [`encode`](#encode)
* [`decode`](#decode)
* [gRPC](#grpc)
* [`TxDecode`](#txdecode)
* [`TxEncode`](#txencode)
* [`TxDecodeAmino`](#txdecodeamino)
* [`TxEncodeAmino`](#txencodeamino)
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix the indentation in the "Contents" section.

The list items in the "Contents" section have inconsistent indentation. Please ensure that the indentation follows a consistent pattern to improve readability and maintain a clear visual hierarchy.

Tools
Markdownlint

23-23: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


24-24: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


25-25: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


26-26: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


27-27: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


28-28: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


29-29: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


30-30: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


31-31: Expected: 4; Actual: 2
Unordered list indentation

(MD007, ul-indent)


32-32: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


33-33: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


34-34: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


35-35: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


36-36: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


37-37: Expected: 8; Actual: 4
Unordered list indentation

(MD007, ul-indent)


38-38: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


39-39: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


40-40: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)


41-41: Expected: 12; Actual: 6
Unordered list indentation

(MD007, ul-indent)

x/auth/tx/README.md Show resolved Hide resolved
x/validate/depinject.go Outdated Show resolved Hide resolved
Copy link
Contributor

@testinginprod testinginprod left a comment

Choose a reason for hiding this comment

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

LGTM

@testinginprod testinginprod added this pull request to the merge queue Sep 20, 2024
@testinginprod testinginprod removed this pull request from the merge queue due to a manual request Sep 20, 2024
@julienrbrt julienrbrt added this pull request to the merge queue Sep 20, 2024
Merged via the queue into main with commit 763e4d6 Sep 20, 2024
75 of 77 checks passed
@julienrbrt julienrbrt deleted the julien/validate branch September 20, 2024 15:11
mergify bot pushed a commit that referenced this pull request Sep 20, 2024
(cherry picked from commit 763e4d6)

# Conflicts:
#	.github/CODEOWNERS
#	CHANGELOG.md
#	UPGRADING.md
#	api/cosmos/auth/v1beta1/accounts.pulsar.go
#	api/cosmos/tx/config/v1/config.pulsar.go
#	simapp/app_config.go
#	x/tx/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release C:x/accounts/base C:x/accounts C:x/auth C:x/tx Type: CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants