feat(client/v2): get keyring from context #19646
Conversation
julienrbrt
added
the
backport/v0.50.x
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe updates focus on refining the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
julienrbrt
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: .coderabbit.yml
Files selected for processing (11)
- client/v2/CHANGELOG.md (2 hunks)
- client/v2/README.md (5 hunks)
- client/v2/autocli/app.go (3 hunks)
- client/v2/autocli/common.go (2 hunks)
- client/v2/autocli/common_test.go (2 hunks)
- client/v2/autocli/flag/address.go (6 hunks)
- client/v2/autocli/flag/builder.go (3 hunks)
- client/v2/autocli/msg.go (2 hunks)
- client/v2/autocli/query_test.go (1 hunks)
- simapp/simd/cmd/root.go (2 hunks)
- simapp/simd/cmd/root_v2.go (4 hunks)
Additional comments: 15
client/v2/README.md (10)
- 75-84: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [5-5]
The front matter syntax seems correct, but the static analysis hinted at a possible spelling mistake. This is a false positive as it's interpreting YAML front matter as regular text.
- 75-84: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]
The term "Autocli" is correctly used in the context of the Cosmos SDK's
autoclipackage. The suggestion to replace it with "AutoCAD" is incorrect.
- 78-78: The description of
autocliusing a keyring for key name resolving and signing transactions is clear and relevant to the PR objectives. However, ensure that the implementation details align with the Cosmos SDK's security practices.- 81-81: The example commands provided are helpful for understanding how
autocliimproves user experience by resolving key names directly from the keyring. This aligns with the PR's goal of enhancing keyring utilization.- 90-92: The explanation of how the keyring is sourced from the
client.Contextand its necessity for signing transactions is crucial. It's important to ensure that this change does not introduce any security vulnerabilities or regressions in functionality.- 95-95: The mention of both the Cosmos SDK keyring and Hubl keyring implementing the
client/v2/autocli/keyringinterface demonstrates the flexibility of theautoclipackage. This is a positive change, enhancing modularity and compatibility.- 103-103: The support for signing transactions with the keyring is a key feature of
autocli. It's important to verify that this functionality works as expected and that the documentation accurately reflects the implementation.- 212-216: The off-chain functionalities for signing and verifying files are well-documented. These features are important for users who need to work with transactions outside of the blockchain. Ensure that the implementation is secure and user-friendly.
- 220-220: The
sign-filecommand's flags are well-documented, providing users with options for encoding and output formatting. This flexibility is beneficial for various use cases. Ensure that the documentation matches the actual command behavior.- 75-84: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [267-267]
The documentation for verifying a signed file is straightforward and provides a clear example. This functionality is essential for ensuring the integrity of off-chain data. Verify that the implementation aligns with the documentation.
client/v2/autocli/flag/builder.go (4)
- 16-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The package declaration and import statements are correctly structured, adhering to Go conventions.
- 16-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [76-76]
The
Builderstruct is well-defined, with clear documentation for each field. The removal of theKeyringfield, as mentioned in the summary, is not directly visible here, but the struct appears to be correctly adjusted for its current responsibilities.
- 91-91: The
initmethod for theBuilderstruct is correctly initializing maps for message and scalar flag types. This method ensures that the builder is ready for use without manual initialization of these maps by the caller.- 16-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [129-129]
The
ValidateAndCompletemethod correctly checks for the presence of necessary codecs and resolvers. Given the context of the changes, it's important to ensure that all required fields are validated here. The removal of theKeyringfield seems to have no direct impact on this method, which is expected based on the summary.client/v2/autocli/query_test.go (1)
- 687-687: The modification of
buildModuleQueryCommandfunction signature to remove themoduleNameparameter and replace it with an underscore as the first parameter indicates a shift towards not using the module name directly within this function. This change aligns with the PR's objective to refactor keyring handling by possibly reducing the reliance on specific module names for command generation. However, it's crucial to ensure that this change does not impact the functionality where the module name is essential for command context or logging purposes.
- Ensure that all usages of
buildModuleQueryCommandthroughout the test suite and potentially in the actual application logic have been updated to reflect this change.- Verify that removing the
moduleNameparameter does not affect the clarity of error messages or logging, where the module name might have been used to provide context.
client/v2/README.md
Outdated
|
|
||
| ### Keyring | ||
|
|
||
| `autocli` uses a keyring for key name resolving and signing transactions. Providing a keyring is optional, but if you want to use the `autocli` generated commands to sign transactions, you must provide a keyring. | ||
| `autocli` uses a keyring for key name resolving names and signing transactions. | ||
|
|
||
| :::tip | ||
| This provides a better UX as it allows to resolve key names directly from the keyring in all transactions and commands. | ||
| AutoCLI provides a better UX than normal cli as it allows to resolve key names directly from the keyring in all transactions and commands. | ||
|
|
||
| ```sh | ||
| <appd> q bank balances alice |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]
The phrase "CLI interface" is redundant since "CLI" stands for "Command Line Interface". Consider simplifying this to just "CLI".
- CLI interface
+ CLI
client/v2/CHANGELOG.md
Outdated
| @@ -46,8 +46,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
|
|
|||
| ### Improvements | |||
|
|
|||
| * []() Use keyring from command context. | |||
There was a problem hiding this comment.
The entry lacks a GitHub issue reference, which is essential for tracking and context.
Please include the GitHub issue reference for the change "Use keyring from command context."
client/v2/CHANGELOG.md
Outdated
| @@ -56,6 +57,7 @@ | |||
|
|
|||
| ### API Breaking Changes | |||
|
|
|||
| * []() Remove keyring from autocli options and flag builder options. | |||
There was a problem hiding this comment.
Similar to the previous comment, this entry is missing a GitHub issue reference.
Please include the GitHub issue reference for the change "Remove keyring from autocli options and flag builder options."
client/v2/autocli/flag/address.go
Outdated
| func getKeyringFromCtx(ctx context.Context) keyring.Keyring { | ||
| if ctx != nil { | ||
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | ||
| keyring, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to create keyring: %w", err)) | ||
| } | ||
|
|
||
| return keyring | ||
| } | ||
| } | ||
|
|
||
| return keyring.NoKeyring{} | ||
| } |
There was a problem hiding this comment.
The function getKeyringFromCtx uses a panic for error handling, which is not ideal for library code.
Consider returning an error instead of using panic, to allow calling code to handle the error gracefully.
- func getKeyringFromCtx(ctx context.Context) keyring.Keyring {
+ func getKeyringFromCtx(ctx context.Context) (keyring.Keyring, error) {
...
- panic(fmt.Errorf("failed to create keyring: %w", err))
+ return nil, fmt.Errorf("failed to create keyring: %w", err)
...
- return keyring.NoKeyring{}
+ return keyring.NoKeyring{}, nil
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| func getKeyringFromCtx(ctx context.Context) keyring.Keyring { | |
| if ctx != nil { | |
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | |
| keyring, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | |
| if err != nil { | |
| panic(fmt.Errorf("failed to create keyring: %w", err)) | |
| } | |
| return keyring | |
| } | |
| } | |
| return keyring.NoKeyring{} | |
| } | |
| func getKeyringFromCtx(ctx context.Context) (keyring.Keyring, error) { | |
| if ctx != nil { | |
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | |
| keyring, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to create keyring: %w", err) | |
| } | |
| return keyring, nil | |
| } | |
| } | |
| return keyring.NoKeyring{}, nil | |
| } |
|
|
||
| autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" | ||
| msgv1 "cosmossdk.io/api/cosmos/msg/v1" | ||
| "cosmossdk.io/client/v2/autocli/keyring" | ||
| "cosmossdk.io/client/v2/internal/flags" | ||
| "cosmossdk.io/client/v2/internal/util" | ||
| "cosmossdk.io/core/address" |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
Given the removal of the Keyring field and adjustments in the ValidateAndComplete method, it's crucial to ensure that these changes are thoroughly tested, especially in scenarios where the Builder is used extensively across the client. Consider adding or updating unit tests to cover these changes and any new logic introduced in this refactor.
Would you like assistance in creating or updating tests to cover these changes?
client/v2/CHANGELOG.md
Outdated
| * [#19646](https://github.com/cosmos/cosmos-sdk/pull/19646) Use keyring from command context. | ||
| * [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands. | ||
| * Note, the given command must have a `client.Context` in its context. | ||
| * Note, the given command must have a `client.Context` in its context. |
There was a problem hiding this comment.
The entry under "Improvements" for PR #19646 and #19060 is clear and concise. However, there's an extra whitespace before the asterisk in the note for PR #19060. Additionally, consider adding a space after the period in the note for better readability.
- commands. * Note, the given command must have a `client.Context` in its context.
+ commands. * Note, the given command must have a `client.Context` in its context.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.
| * [#19646](https://github.com/cosmos/cosmos-sdk/pull/19646) Use keyring from command context. | |
| * [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands. | |
| * Note, the given command must have a `client.Context` in its context. | |
| * Note, the given command must have a `client.Context` in its context. | |
| * [#19646](https://github.com/cosmos/cosmos-sdk/pull/19646) Use keyring from command context. | |
| * [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands. | |
| * Note, the given command must have a `client.Context` in its context. |
client/v2/CHANGELOG.md
Outdated
| * []() Remove keyring from autocli options and flag builder options. | ||
| * [#17709](https://github.com/cosmos/cosmos-sdk/pull/17709) Address codecs have been removed from `autocli.AppOptions` and `flag.Builder`. Instead client/v2 uses the address codecs present in the context (introduced in [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503)). |
There was a problem hiding this comment.
In the "API Breaking Changes" section, the description for the removal of keyring from autocli options and the sourcing of address codecs from the context is clear. However, there's a missing comma after "Instead" which would improve the readability of the sentence.
- Instead client/v2 uses the address codecs present in the context
+ Instead, client/v2 uses the address codecs present in the contextCommittable 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.
| * []() Remove keyring from autocli options and flag builder options. | |
| * [#17709](https://github.com/cosmos/cosmos-sdk/pull/17709) Address codecs have been removed from `autocli.AppOptions` and `flag.Builder`. Instead client/v2 uses the address codecs present in the context (introduced in [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503)). | |
| * []() Remove keyring from autocli options and flag builder options. | |
| * [#17709](https://github.com/cosmos/cosmos-sdk/pull/17709) Address codecs have been removed from `autocli.AppOptions` and `flag.Builder`. Instead, client/v2 uses the address codecs present in the context (introduced in [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503)). |
client/v2/autocli/flag/address.go
Outdated
| func getKeyringFromCtx(ctx context.Context) keyring.Keyring { | ||
| if ctx != nil { | ||
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | ||
| k, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to create keyring: %w", err)) | ||
| } | ||
|
|
||
| return k | ||
| } else if k := ctx.Value(keyring.KeyringContextKey); k != nil { | ||
| return k.(*keyring.KeyringImpl) | ||
| } | ||
| } | ||
|
|
||
| return keyring.NoKeyring{} | ||
| } |
There was a problem hiding this comment.
Replace panic with error handling in getKeyringFromCtx to improve robustness and error recovery.
- panic(fmt.Errorf("failed to create keyring: %w", err))
+ return nil, fmt.Errorf("failed to create keyring: %w", err)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| func getKeyringFromCtx(ctx context.Context) keyring.Keyring { | |
| if ctx != nil { | |
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | |
| k, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | |
| if err != nil { | |
| panic(fmt.Errorf("failed to create keyring: %w", err)) | |
| } | |
| return k | |
| } else if k := ctx.Value(keyring.KeyringContextKey); k != nil { | |
| return k.(*keyring.KeyringImpl) | |
| } | |
| } | |
| return keyring.NoKeyring{} | |
| } | |
| func getKeyringFromCtx(ctx context.Context) keyring.Keyring { | |
| if ctx != nil { | |
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | |
| k, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to create keyring: %w", err) | |
| } | |
| return k | |
| } else if k := ctx.Value(keyring.KeyringContextKey); k != nil { | |
| return k.(*keyring.KeyringImpl) | |
| } | |
| } | |
| return keyring.NoKeyring{} | |
| } |
client/v2/README.md
Outdated
| * `sign-file` for signing a file. | ||
| * `verify-file` for verifying a previously signed file. |
There was a problem hiding this comment.
Correct minor grammatical issues in the off-chain functionalities section for improved readability.
- * `sign-file` for signing a file.
- * `verify-file` for verifying a previously signed file.
+ * `sign-file`: For signing a file.
+ * `verify-file`: For verifying a previously signed file.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.
| * `sign-file` for signing a file. | |
| * `verify-file` for verifying a previously signed file. | |
| * `sign-file`: For signing a file. | |
| * `verify-file`: For verifying a previously signed file. |
There was a problem hiding this comment.
Actionable comments posted: 0
Actionable comments outside the diff hunks (1)
client/v2/autocli/common_test.go (1)
Line range hint
40-40: Ensure the gRPC server in tests uses secure credentials to prevent potential security vulnerabilities.- server := grpc.NewServer() + server := grpc.NewServer(grpc.Creds(credentials.NewServerTLSFromFile("cert.pem", "cert.key")))
julienrbrt
force-pushed
the
julien/autocli
branch
from
f40b6c5
to
953fcc2
Compare
|
reverted the commits you made @sontrinh16 |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
closing this for now until there is time to come back to it |
julienrbrt
force-pushed
the
julien/autocli
branch
from
9afc58e
to
0069483
Compare
julienrbrt
force-pushed
the
julien/autocli
branch
from
2d1bd2f
to
ce4ba2d
Compare
|
@coderabbitai pause |
There was a problem hiding this comment.
Actionable comments posted: 11
Outside diff range and nitpick comments (11)
client/v2/autocli/flag/address.go (3)
41-43: Ensure the context is properly documented to explain why a pointer is necessary here, aiding future maintainability.
Line range hint
57-74: This error handling strategy is improved by checking the key name lookup first. However, consider consolidating the error messages for better clarity and maintainability.- return fmt.Errorf("invalid account address or key name: %w", err) + return fmt.Errorf("Failed key name lookup or invalid account address: %w", err)
Line range hint
112-139: The error handling in this block is robust, but the error message could be more descriptive to aid debugging.- return fmt.Errorf("input isn't a pubkey (%w) or is an invalid account address (%w)", err, err2) + return fmt.Errorf("Input validation failed: not a pubkey (%w) or invalid account address (%w)", err, err2)client/v2/autocli/flag/map.go (1)
183-191: The introduction of context handling in the map value setting is a good improvement. However, ensure that the context and its changes are well-documented to avoid confusion.client/v2/README.md (6)
Line range hint
17-17: Consider simplifying "CLI interface" to "CLI" to avoid redundancy.- CLI interface + CLITools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
Line range hint
115-115: Clarify the language to enhance readability.- The `AutoCLIOptions()` method on your module allows to specify custom commands, sub-commands or flags for each service. + The `AutoCLIOptions()` method on your module allows you to specify custom commands, sub-commands, or flags for each service.Tools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
Line range hint
131-131: Consider adding commas for better readability.- Users can however use the `--no-proposal` flag to disable the proposal creation. + Users, however, can use the `--no-proposal` flag to disable the proposal creation.Tools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
Line range hint
145-145: Add a comma for clarity.- By default `autocli` generates a flag for each field in your protobuf message. + By default, `autocli` generates a flag for each field in your protobuf message.Tools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
Line range hint
164-166: Ensure consistency in the use of "customize" and "customise".- However, you can customise the flag names by providing a `FlagOptions`. + However, you can customize the flag names by providing a `FlagOptions`. - ...can use the following naming options to customise the flags: + ...can use the following naming options to customize the flags:Tools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
Line range hint
182-182: Simplify the language for clarity.- In order to enable this behavior, set in `AutoCLIOptions()` the `EnhanceCustomCommand` field to `true`, for the command type (queries and/or transactions) you want to enhance. + To enable this behavior, set the `EnhanceCustomCommand` field to `true` in `AutoCLIOptions()` for the command type (queries and/or transactions) you want to enhance.Tools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...client/v2/autocli/flag/builder.go (1)
Line range hint
113-193: Ensure proper handling ofcontext.Contextinstead of*context.Contextto align with Go best practices.- func (b *Builder) AddMessageFlags(ctx *context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions) (*MessageBinder, error) { + func (b *Builder) AddMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions) (*MessageBinder, error) { - func (b *Builder) addMessageFlags(ctx *context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions, options namingOptions) (*MessageBinder, error) { + func (b *Builder) addMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions, options namingOptions) (*MessageBinder, error) {
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (21)
- client/v2/CHANGELOG.md (3 hunks)
- client/v2/README.md (6 hunks)
- client/v2/autocli/app.go (3 hunks)
- client/v2/autocli/common.go (1 hunks)
- client/v2/autocli/common_test.go (2 hunks)
- client/v2/autocli/flag/address.go (8 hunks)
- client/v2/autocli/flag/binary.go (1 hunks)
- client/v2/autocli/flag/builder.go (11 hunks)
- client/v2/autocli/flag/coin.go (1 hunks)
- client/v2/autocli/flag/duration.go (1 hunks)
- client/v2/autocli/flag/enum.go (1 hunks)
- client/v2/autocli/flag/interface.go (1 hunks)
- client/v2/autocli/flag/json_message.go (1 hunks)
- client/v2/autocli/flag/list.go (2 hunks)
- client/v2/autocli/flag/map.go (1 hunks)
- client/v2/autocli/flag/messager_binder.go (1 hunks)
- client/v2/autocli/flag/pubkey.go (1 hunks)
- client/v2/autocli/flag/timestamp.go (1 hunks)
- client/v2/autocli/keyring/keyring.go (1 hunks)
- client/v2/autocli/msg.go (4 hunks)
- client/v2/autocli/query_test.go (1 hunks)
Files skipped from review due to trivial changes (2)
- client/v2/autocli/app.go
- client/v2/autocli/common_test.go
Additional context used
Path-based instructions (19)
client/v2/autocli/flag/duration.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/interface.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/timestamp.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/coin.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/pubkey.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/keyring/keyring.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/binary.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/enum.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/json_message.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/messager_binder.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/list.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"client/v2/autocli/flag/address.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/common.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/msg.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/flag/map.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"client/v2/autocli/flag/builder.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/v2/autocli/query_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
LanguageTool
client/v2/CHANGELOG.md
[uncategorized] ~67-~67: A comma may be missing after the conjunctive/linking adverb ‘Instead’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...autocli.AppOptionsandflag.Builder`. Instead client/v2 uses the address codecs prese...client/v2/README.md
[style] ~17-~17: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”. (ACRONYM_TAUTOLOGY)
Context: ...s. This means that you can easily add a CLI interface to your application without having to m...
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
[grammar] ~115-~115: Did you mean “specifying”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ...Options()` method on your module allows to specify custom commands, sub-commands or flags ...
[typographical] ~131-~131: Consider adding two commas here. (HOWEVER_COMMA)
Context: ...cli.RpcCommandOptionsstruct. Users can however use the--no-proposal` flag to disable...
[uncategorized] ~145-~145: Did you mean: “By default,”? (BY_DEFAULT_COMMA)
Context: ...#L94-L97 ``` ### Positional Arguments By defaultautocligenerates a flag for each fie...
[uncategorized] ~164-~164: Do not mix variants of the same word (‘customise’ and ‘customize’) within a single text. (EN_WORD_COHERENCY)
Context: ...your protobuf message. However, you can customise the flag names by providing a `FlagOpti...
[uncategorized] ~166-~166: Do not mix variants of the same word (‘customise’ and ‘customize’) within a single text. (EN_WORD_COHERENCY)
Context: ...can use the following naming options to customise the flags: ``` go autocliv1.RpcCommand...
[style] ~182-~182: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...mmands for theproposersubcommands. In order to enable this behavior, set in `AutoCLIOp...
[uncategorized] ~233-~233: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...s/cip-X.md). ## Sign a file To sign a filesign-filecommand offers some helpful...
[uncategorized] ~280-~280: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... ``` ## Verify a file To verify a file only the key name used and the previous...
Markdownlint
client/v2/README.md
247-247: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
258-258: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
269-269: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
222-222: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
253-253: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
264-264: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
254-254: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
265-265: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (13)
client/v2/autocli/flag/duration.go (1)
13-13: Verify the necessity and safety of passingcontext.Contextby pointer.Verification successful
The search results show that the
NewValuefunctions in theclient/v2/autocli/flag/directory generally do not modify thecontext.Contextpassed to them. They typically use the context to create new value types, and the context is stored within these types but not modified directly. This suggests that the change to use a pointer tocontext.Contextis likely a design choice to maintain consistency across the codebase rather than for modifying the context.
client/v2/autocli/flag/timestamp.goclient/v2/autocli/flag/pubkey.goclient/v2/autocli/flag/map.goclient/v2/autocli/flag/list.goclient/v2/autocli/flag/json_message.goclient/v2/autocli/flag/enum.goclient/v2/autocli/flag/duration.goclient/v2/autocli/flag/coin.goclient/v2/autocli/flag/binary.goclient/v2/autocli/flag/address.goThese files contain the
NewValuefunction implementations, and none of them appear to modify thecontext.Contextdirectly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of NewValue in the codebase to verify context manipulation. rg --type go 'NewValue\('Length of output: 2908
Script:
#!/bin/bash # Inspect the implementation details of NewValue in the client/v2/autocli/flag/ directory to verify context usage. rg --type go 'func (.*) NewValue\(' -A 10 client/v2/autocli/flag/Length of output: 7825
client/v2/autocli/flag/interface.go (1)
13-13: Ensure all implementations ofNewValuecorrectly handle the context pointer without side effects.client/v2/autocli/flag/timestamp.go (1)
13-13: Confirm the intended use ofcontext.Contextas a pointer inNewValueand validate its implementation across the system.Verification successful
The results show that the
NewValuefunction implementations intimestamp.goand other related files correctly handle thecontext.Contextpointer, aligning with the overall pattern in the codebase. The use of context pointers is consistent and appears to be implemented correctly.
timestamp.go: TheNewValuefunction returns atimestampValuewithout directly utilizing the context, which is consistent with its intended use.- Other related files show similar patterns of correct context handling and struct initialization.
No issues have been found with the usage of
context.Contextpointers in theNewValuefunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of NewValue in timestamp related code to ensure correct context handling. rg --type go 'NewValue\('Length of output: 2908
Script:
#!/bin/bash # Description: Extract the implementation of NewValue in timestamp.go and related files to verify correct context handling. ast-grep --lang go --pattern 'func ($_ $_) NewValue($_, $_) $_' --jsonLength of output: 8222
client/v2/autocli/keyring/keyring.go (1)
1-47: The implementation ofKeyringImpland its methods are well-designed, following Go's best practices for context usage and interface implementation. Good job on encapsulating keyring functionality in a clean and extendable way.client/v2/autocli/flag/json_message.go (1)
23-23: The change to use a pointer for the context inNewValueis well-implemented and aligns with Go best practices for performance optimization.client/v2/autocli/flag/messager_binder.go (1)
28-30: The addition ofmessageTypetoMessageBinderenhances the flexibility and dynamic handling of protobuf messages.client/v2/autocli/flag/list.go (1)
55-55: The update to use a context pointer incompositeListTypeis consistent with other parts of the module and enhances performance.Also applies to: 71-71
client/v2/autocli/flag/address.go (3)
22-23: LGTM! The update to pass a pointer to the context is consistent with the PR's objective to refine context handling.
32-33: LGTM! Consistent changes with the context pointer, ensuring uniformity across the module.
86-90: Good application of consistent context handling across different types.client/v2/autocli/msg.go (2)
136-136: Handling government proposals and settingSilenceUsageto true are significant changes. Ensure these are well-documented and tested, as they could affect user interactions and error visibility.Also applies to: 180-180
113-113: Ensure that the addition of the command to the parent command is error-checked to prevent runtime panics if the command cannot be added.client/v2/autocli/query_test.go (1)
687-687: The error propagation inbuildModuleQueryCommandfollows Go's idiomatic approach by returning errors to the caller, which is a good practice for error management in Go applications.
| @@ -684,7 +684,7 @@ func TestNotFoundErrorsQuery(t *testing.T) { | |||
| b.AddQueryConnFlags = nil | |||
| b.AddTxConnFlags = nil | |||
|
|
|||
| buildModuleQueryCommand := func(moduleName string, cmdDescriptor *autocliv1.ServiceCommandDescriptor) (*cobra.Command, error) { | |||
| buildModuleQueryCommand := func(_ string, cmdDescriptor *autocliv1.ServiceCommandDescriptor) (*cobra.Command, error) { | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The function buildModuleQueryCommand is still being called with the old signature, including the moduleName parameter. These instances need to be updated to match the new function signature.
client/v2/autocli/query_test.go:- Line 687:
_, err := buildModuleQueryCommand("test", &autocliv1.ServiceCommandDescriptor{Service: "foo"}) - Lines following 687: Multiple instances of
buildModuleQueryCommandcalled withmoduleName.
- Line 687:
Please update all instances of buildModuleQueryCommand to match the new signature that does not include moduleName.
Analysis chain
The removal of the moduleName parameter in buildModuleQueryCommand simplifies the function signature. Ensure that all references to this function have been updated accordingly to pass the correct cmdDescriptor without expecting a module name.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all function calls to `buildModuleQueryCommand` match the new signature.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type go $'buildModuleQueryCommand'
Length of output: 3331
Script:
#!/bin/bash
# Description: Verify function calls to `buildModuleQueryCommand` and `buildModuleQueryCommandOptional` match the new signature.
# Test: Search for function calls and check for the correct number of parameters.
rg --type go 'buildModuleQueryCommand\('
rg --type go 'buildModuleQueryCommandOptional\('
Length of output: 554
| @@ -14,7 +14,7 @@ import ( | |||
|
|
|||
| type pubkeyType struct{} | |||
|
|
|||
| func (a pubkeyType) NewValue(_ context.Context, _ *Builder) Value { | |||
| func (a pubkeyType) NewValue(_ *context.Context, _ *Builder) Value { | |||
There was a problem hiding this comment.
Passing context by pointer is unconventional and could lead to unintended side effects. Recommend reverting to passing context.Context by value unless there's a specific reason for this change.
| @@ -14,7 +14,7 @@ type binaryType struct{} | |||
|
|
|||
| var _ Value = (*fileBinaryValue)(nil) | |||
|
|
|||
| func (f binaryType) NewValue(context.Context, *Builder) Value { | |||
| func (f binaryType) NewValue(*context.Context, *Builder) Value { | |||
There was a problem hiding this comment.
As with other files, passing context.Context by pointer is not recommended. Consider reverting to passing by value to maintain the immutability of the context.
| @@ -14,7 +14,7 @@ type enumType struct { | |||
| enum protoreflect.EnumDescriptor | |||
| } | |||
|
|
|||
| func (b enumType) NewValue(context.Context, *Builder) Value { | |||
| func (b enumType) NewValue(*context.Context, *Builder) Value { | |||
There was a problem hiding this comment.
The change to pass context.Context by pointer in enumType.NewValue should be reconsidered. Contexts are designed to be immutable and passed by value in Go.
client/v2/autocli/flag/address.go
Outdated
| func getKeyringFromCtx(ctx *context.Context) keyring.Keyring { | ||
| if ctx != nil { | ||
| ctx := *ctx | ||
|
|
||
| if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { | ||
| k, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to create keyring: %w", err)) | ||
| } | ||
|
|
||
| return k | ||
| } else if k := ctx.Value(keyring.KeyringContextKey); k != nil { | ||
| return k.(*keyring.KeyringImpl) | ||
| } | ||
| } | ||
|
|
||
| return keyring.NoKeyring{} | ||
| } |
There was a problem hiding this comment.
Using panics for error handling in library code is not ideal. It's better to return errors to allow calling code to handle them gracefully.
- panic(fmt.Errorf("failed to create keyring: %w", err))
+ return nil, fmt.Errorf("failed to create keyring: %w", err)Committable suggestion was skipped due to low confidence.
| // we need to use a pointer to the context as the correct context is set in the RunE function | ||
| // however we need to set the flags before the RunE function is called | ||
| ctx := cmd.Context() | ||
| binder, err := b.AddMessageFlags(&ctx, cmd.Flags(), inputType, options) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| cmd.Args = binder.CobraArgs | ||
|
|
||
| cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
| ctx = cmd.Context() | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
The context management in client/v2/autocli/common.go is inconsistent. The code snippet provided shows the use of cmd.Context() directly, which contradicts the comment about needing to use a pointer to the context.
client/v2/autocli/common.go: Lines 56-67, usage ofcmd.Context()directly.
Analysis chain
The update to use a pointer for the context in the command setup is in line with the PR's objective of consistent context usage. Ensure that the context is correctly managed across command executions.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify proper context management in command executions.
# Test: Search for context usage in command setups. Expect: Only usage of pointer context.
rg --type go $'cmd.Context()'
Length of output: 3486
|
|
||
| :::tip | ||
| This provides a better UX as it allows to resolve key names directly from the keyring in all transactions and commands. | ||
| AutoCLI provides a better UX than normal CLI as it allows to resolve key names directly from the keyring in all transactions and commands. |
There was a problem hiding this comment.
Clarify the language in the tip box for better readability.
- AutoCLI provides a better UX than normal CLI as it allows to resolve key names directly from the keyring in all transactions and commands.
+ AutoCLI provides a better UX than a normal CLI by allowing direct resolution of key names from the keyring in all transactions and commands.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.
| AutoCLI provides a better UX than normal CLI as it allows to resolve key names directly from the keyring in all transactions and commands. | |
| AutoCLI provides a better UX than a normal CLI by allowing direct resolution of key names from the keyring in all transactions and commands. |
Tools
LanguageTool
[grammar] ~81-~81: Did you mean “resolving”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ... better UX than normal CLI as it allows to resolve key names directly from the keyring in ...
| @@ -274,7 +269,7 @@ func (b *Builder) addMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, m | |||
| } | |||
|
|
|||
| // bindPageRequest create a flag for pagination | |||
| func (b *Builder) bindPageRequest(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { | |||
| func (b *Builder) bindPageRequest(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { | |||
There was a problem hiding this comment.
Ensure correct usage of context.Context.
- func (b *Builder) bindPageRequest(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) {
+ func (b *Builder) bindPageRequest(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) {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.
| func (b *Builder) bindPageRequest(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { | |
| func (b *Builder) bindPageRequest(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { |
| @@ -291,7 +286,7 @@ | |||
| } | |||
|
|
|||
| // addFieldFlag adds a flag for the provided field to the flag set. | |||
| func (b *Builder) addFieldFlag(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) { | |||
| func (b *Builder) addFieldFlag(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) { | |||
There was a problem hiding this comment.
Correct the parameter type for better alignment with Go conventions.
- func (b *Builder) addFieldFlag(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) {
+ func (b *Builder) addFieldFlag(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) {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.
| func (b *Builder) addFieldFlag(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) { | |
| func (b *Builder) addFieldFlag(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) { |
| @@ -17,7 +17,7 @@ type coinValue struct { | |||
| value *basev1beta1.Coin | |||
| } | |||
|
|
|||
| func (c coinType) NewValue(context.Context, *Builder) Value { | |||
| func (c coinType) NewValue(*context.Context, *Builder) Value { | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process reveals that the NewValue function in coin.go and several other files uses the context.Context pointer without actually utilizing it within the function body. This indicates an inconsistency in the usage of the context pointer across the codebase.
client/v2/autocli/flag/coin.go: func (c coinType) NewValue(*context.Context, *Builder) Value { return &coinValue{} }- Similar patterns are observed in other files like
timestamp.go,binary.go, etc.
Given this, it appears that the context pointer is not necessary in its current form and could be removed to maintain code consistency and clarity.
client/v2/autocli/flag/coin.goclient/v2/autocli/flag/timestamp.goclient/v2/autocli/flag/binary.go
Analysis chain
Check the necessity and correct implementation of using context.Context as a pointer in NewValue. Ensure it aligns with best practices.
The output from the shell script shows multiple instances of the NewValue function across different files, with some using the context pointer and others not. This suggests inconsistent usage of the context pointer in the NewValue function across the codebase.
To verify the necessity and correct implementation of the context pointer in the NewValue function, we should:
- Check if the context pointer is actually used within the
NewValuefunction incoin.go. - Verify if there are any specific reasons or patterns for using the context pointer in some instances and not in others.
Let's proceed with these steps.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the correct and necessary use of context pointer in NewValue for coin related operations.
rg --type go 'NewValue\('
Length of output: 2908
Script:
#!/bin/bash
# Check if the context pointer is used within the NewValue function in coin.go
ast-grep --lang go --pattern 'func (c coinType) NewValue(ctx *context.Context, b *Builder) Value {
$$$
}' client/v2/autocli/flag/coin.go
# Check if there are any comments or documentation explaining the use of context pointer in NewValue
rg --type go 'NewValue\(' --context 5
Length of output: 19107
Actions performedReviews paused. |
(cherry picked from commit ca195c1) # Conflicts: # client/v2/CHANGELOG.md # client/v2/README.md # client/v2/autocli/app.go # client/v2/autocli/common.go # client/v2/autocli/common_test.go # client/v2/autocli/flag/pubkey.go # client/v2/autocli/msg.go # simapp/simd/cmd/root.go
* build(deps): Bump github.com/cosmos/gogoproto from 1.4.12 to 1.5.0 (cosmos#20567) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * refactor(x/authz,x/feegrant): provide updated keeper in depinject (cosmos#20590) * docs: Update high level overview and introduction (backport cosmos#20535) (cosmos#20627) Co-authored-by: samricotta <37125168+samricotta@users.noreply.github.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> * fix: Properly parse json in the wait-tx command. (backport cosmos#20631) (cosmos#20660) Co-authored-by: Daniel Wedul <github@wedul.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> * docs: remove Ineffective code block (backport cosmos#20703) (cosmos#20711) * feat(client): Add flag & reading mnemonic from file (backport cosmos#20690) (cosmos#20712) Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> * fix: nested multisig signatures using CLI (backport cosmos#20438) (cosmos#20692) Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> Co-authored-by: Facundo <facundomedica@gmail.com> * feat(client/v2): get keyring from context (backport cosmos#19646) (cosmos#20727) Co-authored-by: Julien Robert <julien@rbrt.fr> * docs(x/group): orm codespace comment (backport cosmos#20749) (cosmos#20751) * feat: parse home flag earlier (backport cosmos#20771) (cosmos#20777) Co-authored-by: Julien Robert <julien@rbrt.fr> * build(deps): Bump github.com/cometbft/cometbft from 0.38.7 to 0.38.8 (cosmos#20805) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * build(deps): Bump github.com/cometbft/cometbft from 0.38.8 to 0.38.9 (cosmos#20836) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * fix(simulation): fix the problem of `validator set is empty after InitGenesis` in simulation test (backport cosmos#18196) (cosmos#20897) Co-authored-by: Chenqun Lu <luchenqun@qq.com> Co-authored-by: Julien Robert <julien@rbrt.fr> * fix(simulation): Fix all problems `make test-sim-custom-genesis-fast` for simulation test. (backport cosmos#17911) (cosmos#20909) Co-authored-by: Chenqun Lu <luchenqun@qq.com> Co-authored-by: Julien Robert <julien@rbrt.fr> * chore: prepare v0.50.8 (cosmos#20910) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: samricotta <37125168+samricotta@users.noreply.github.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> Co-authored-by: Daniel Wedul <github@wedul.com> Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Co-authored-by: Facundo <facundomedica@gmail.com> Co-authored-by: Chenqun Lu <luchenqun@qq.com>
Description
Closes: #19645
ref: #19530 (comment) and 1. of #18310
I'll update Hubl after the client/v2 tag after this and #19618
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Improvements
Documentation
Refactor