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

fix(autocli): fix simapp enhancing #15906

Merged
merged 6 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/cosmos/autocli/v1/options.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion client/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,11 @@ func GetClientContextFromCmd(cmd *cobra.Command) Context {
}

// SetCmdClientContext sets a command's Context value to the provided argument.
// If the context has not been set, set the given context as the default.
func SetCmdClientContext(cmd *cobra.Command, clientCtx Context) error {
v := cmd.Context().Value(ClientContextKey)
if v == nil {
return errors.New("client context not set")
v = &clientCtx
}

clientCtxPtr := v.(*Context)
Expand Down
37 changes: 37 additions & 0 deletions client/v2/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!--
Guiding Principles:

Changelogs are for humans, not machines.
There should be an entry for every single version.
The same types of changes should be grouped.
Versions and sections should be linkable.
The latest version comes first.
The release date of each version is displayed.
Mention whether you follow Semantic Versioning.

Usage:

Change log entries are to be added to the Unreleased section under the
appropriate stanza (see below). Each entry should ideally include a tag and
the Github issue reference in the following format:

* (<tag>) \#<issue-number> message

The issue numbers will later be link-ified during the release process so you do
not have to worry about including a link manually, but you can if you wish.

Types of changes (Stanzas):

"Features" for new features.
"Improvements" for changes in existing functionality.
"Deprecated" for soon-to-be removed features.
"Bug Fixes" for any bug fixes.
"Client Breaking" for breaking Protobuf, gRPC and REST routes used by end-users.
"CLI Breaking" for breaking CLI commands.
"API Breaking" for breaking exported APIs used by developers building on SDK.
Ref: https://keepachangelog.com/en/1.0.0/
-->

# Changelog

## [Unreleased]
7 changes: 7 additions & 0 deletions client/v2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# AutoCLI

The `autocli` package is a Go library for generating CLIs (command line interfaces) for Cosmos SDK-based applications.

Read more about in it the Cosmos SDK documentation:

* https://docs.cosmos.network/main/building-modules/autocli
79 changes: 20 additions & 59 deletions client/v2/autocli/app.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package autocli

import (
"fmt"

autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
"cosmossdk.io/client/v2/autocli/flag"
"cosmossdk.io/core/address"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"

Expand Down Expand Up @@ -32,23 +32,18 @@ type AppOptions struct {
// app to override module options if they are either not provided by a
// module or need to be improved.
ModuleOptions map[string]*autocliv1.ModuleOptions `optional:"true"`
}

// RootCmd generates a root command for an app based on the AppOptions. This
// command currently only includes query commands but will be enhanced over
// time to cover the full scope of an app CLI.
func (appOptions AppOptions) RootCmd() (*cobra.Command, error) {
rootCmd := &cobra.Command{}
err := appOptions.EnhanceRootCommand(rootCmd)
return rootCmd, err
// AddressCodec is the address codec to use for the app.
// If not provided the default address prefix will be fetched from the reflection client.
AddressCodec address.Codec `optional:"true"`
}

// EnhanceRootCommand enhances the provided root command with autocli AppOptions,
// only adding missing query commands and doesn't override commands already
// only adding missing commands and doesn't override commands already
// in the root command. This allows for the graceful integration of autocli with
// existing app CLI commands where autocli simply automatically adds things that
// weren't manually provided. It does take into account custom query commands
// provided by modules with the HasCustomQueryCommand extension interface.
// weren't manually provided. It does take into account custom commands
// provided by modules with the HasCustomQueryCommand or HasCustomTxCommand extension interface.
// Example Usage:
//
// var autoCliOpts autocli.AppOptions
Expand All @@ -60,6 +55,12 @@ func (appOptions AppOptions) RootCmd() (*cobra.Command, error) {
// err = autoCliOpts.EnhanceRootCommand(rootCmd)
func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
builder := &Builder{
Builder: flag.Builder{
AddressCodec: appOptions.AddressCodec,
GetClientConn: func() (grpc.ClientConnInterface, error) {
return client.GetClientQueryContext(rootCmd)
},
},
GetClientConn: func(cmd *cobra.Command) (grpc.ClientConnInterface, error) {
return client.GetClientQueryContext(cmd)
},
Expand All @@ -71,19 +72,8 @@ func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
}

func (appOptions AppOptions) EnhanceRootCommandWithBuilder(rootCmd *cobra.Command, builder *Builder) error {
moduleOptions := appOptions.ModuleOptions
if moduleOptions == nil {
moduleOptions = map[string]*autocliv1.ModuleOptions{}

for name, module := range appOptions.Modules {
if module, ok := module.(HasAutoCLIConfig); ok {
moduleOptions[name] = module.AutoCLIOptions()
}
}
}

customQueryCmds := map[string]*cobra.Command{}
customMsgCmds := map[string]*cobra.Command{}
// extract any custom commands from modules
customQueryCmds, customMsgCmds := map[string]*cobra.Command{}, map[string]*cobra.Command{}
for name, module := range appOptions.Modules {
if queryModule, ok := module.(HasCustomQueryCommand); ok {
queryCmd := queryModule.GetQueryCmd()
Expand All @@ -101,54 +91,25 @@ func (appOptions AppOptions) EnhanceRootCommandWithBuilder(rootCmd *cobra.Comman
}
}

// if we have an existing query command, enhance it or build a custom one
enhanceQuery := func(cmd *cobra.Command, modOpts *autocliv1.ModuleOptions, moduleName string) error {
queryCmdDesc := modOpts.Query
if queryCmdDesc != nil {
subCmd := topLevelCmd(moduleName, fmt.Sprintf("Querying commands for the %s module", moduleName))
err := builder.AddQueryServiceCommands(cmd, queryCmdDesc)
if err != nil {
return err
}

cmd.AddCommand(subCmd)
}
return nil
}

if queryCmd := findSubCommand(rootCmd, "query"); queryCmd != nil {
if err := builder.enhanceCommandCommon(queryCmd, moduleOptions, customQueryCmds, enhanceQuery); err != nil {
if err := builder.enhanceCommandCommon(queryCmd, appOptions, customQueryCmds, enhanceQuery); err != nil {
return err
}
} else {
queryCmd, err := builder.BuildQueryCommand(moduleOptions, customQueryCmds)
queryCmd, err := builder.BuildQueryCommand(appOptions, customQueryCmds, enhanceQuery)
if err != nil {
return err
}

rootCmd.AddCommand(queryCmd)
}

enhanceMsg := func(cmd *cobra.Command, modOpts *autocliv1.ModuleOptions, moduleName string) error {
txCmdDesc := modOpts.Tx
if txCmdDesc != nil {
subCmd := topLevelCmd(moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
err := builder.AddQueryServiceCommands(cmd, txCmdDesc)
if err != nil {
return err
}

cmd.AddCommand(subCmd)
}
return nil
}

if msgCmd := findSubCommand(rootCmd, "tx"); msgCmd != nil {
if err := builder.enhanceCommandCommon(msgCmd, moduleOptions, customQueryCmds, enhanceMsg); err != nil {
Copy link
Member Author

@julienrbrt julienrbrt Apr 21, 2023

Choose a reason for hiding this comment

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

So here it was passing the custom query commands, which is incorrect.

if err := builder.enhanceCommandCommon(msgCmd, appOptions, customMsgCmds, enhanceMsg); err != nil {
return err
}
} else {
subCmd, err := builder.BuildMsgCommand(moduleOptions, customQueryCmds)
subCmd, err := builder.BuildMsgCommand(appOptions, customMsgCmds, enhanceMsg)
if err != nil {
return err
}
Expand Down
71 changes: 54 additions & 17 deletions client/v2/autocli/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/spf13/cobra"
"golang.org/x/exp/maps"
"google.golang.org/protobuf/reflect/protoreflect"
"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -67,22 +68,27 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
// options or the provided custom commands for each module. If the provided query command already contains a command
// for a module, that command is not over-written by this method. This allows a graceful addition of autocli to
// automatically fill in missing commands.
func (b *Builder) enhanceCommandCommon(cmd *cobra.Command, moduleOptions map[string]*autocliv1.ModuleOptions, customCmds map[string]*cobra.Command, buildModuleCommand func(*cobra.Command, *autocliv1.ModuleOptions, string) error) error {
allModuleNames := map[string]bool{}
for moduleName := range moduleOptions {
allModuleNames[moduleName] = true
}
for moduleName := range customCmds {
allModuleNames[moduleName] = true
func (b *Builder) enhanceCommandCommon(
cmd *cobra.Command,
appOptions AppOptions,
customCmds map[string]*cobra.Command,
buildModuleCommand enhanceCommandFunc,
) error {
moduleOptions := appOptions.ModuleOptions
if len(moduleOptions) == 0 {
moduleOptions = map[string]*autocliv1.ModuleOptions{}
for name, module := range appOptions.Modules {
if module, ok := module.(HasAutoCLIConfig); ok {
moduleOptions[name] = module.AutoCLIOptions()
}
}
}

for moduleName := range allModuleNames {
modules := append(maps.Keys(appOptions.Modules), maps.Keys(moduleOptions)...)
for _, moduleName := range modules {
// if we have an existing command skip adding one here
if cmd.HasSubCommands() {
if _, _, err := cmd.Find([]string{moduleName}); err == nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

This was always returning true for simapp. We already had a helper that was working.

// command already exists, skip
continue
}
if findSubCommand(cmd, moduleName) != nil {
continue
}

// if we have a custom command use that instead of generating one
Expand All @@ -98,28 +104,59 @@ func (b *Builder) enhanceCommandCommon(cmd *cobra.Command, moduleOptions map[str
continue
}

err := buildModuleCommand(cmd, modOpts, moduleName)
if err != nil {
if err := buildModuleCommand(b, moduleName, cmd, modOpts); err != nil {
return err
}
}

return nil
}

type enhanceCommandFunc func(builder *Builder, moduleName string, cmd *cobra.Command, modOpts *autocliv1.ModuleOptions) error

// enhanceQuery enhances the provided query command with the autocli commands for a module.
func enhanceQuery(builder *Builder, moduleName string, cmd *cobra.Command, modOpts *autocliv1.ModuleOptions) error {
queryCmdDesc := modOpts.Query
if queryCmdDesc != nil {
subCmd := topLevelCmd(moduleName, fmt.Sprintf("Querying commands for the %s module", moduleName))
if err := builder.AddQueryServiceCommands(subCmd, queryCmdDesc); err != nil {
return err
}

cmd.AddCommand(subCmd)
}

return nil
}

// enhanceMsg enhances the provided msg command with the autocli commands for a module.
func enhanceMsg(builder *Builder, moduleName string, cmd *cobra.Command, modOpts *autocliv1.ModuleOptions) error {
txCmdDesc := modOpts.Tx
if txCmdDesc != nil {
subCmd := topLevelCmd(moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
if err := builder.AddMsgServiceCommands(subCmd, txCmdDesc); err != nil {
return err
}

cmd.AddCommand(subCmd)
}

return nil
}

// outOrStdoutFormat formats the output based on the output flag and writes it to the command's output stream.
func (b *Builder) outOrStdoutFormat(cmd *cobra.Command, out []byte) error {
var err error
outputType := cmd.Flag(flags.FlagOutput)
// if the output type is text, convert the json to yaml
// if output type is json or nil, default to json
if outputType != nil && outputType.Value.String() == "text" {
if outputType != nil && outputType.Value.String() == flags.OutputFormatText {
out, err = yaml.JSONToYAML(out)
if err != nil {
return err
}
}
_, err = fmt.Fprintln(cmd.OutOrStdout(), string(out))

_, err = fmt.Fprintln(cmd.OutOrStdout(), string(out))
return err
}
22 changes: 14 additions & 8 deletions client/v2/autocli/flag/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ package flag

import (
"context"
"fmt"

reflectionv2alpha1 "cosmossdk.io/api/cosmos/base/reflection/v2alpha1"
"github.com/cosmos/cosmos-sdk/types"
"cosmossdk.io/core/address"
"google.golang.org/protobuf/reflect/protoreflect"

addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
)

type addressStringType struct{}

func (a addressStringType) NewValue(ctx context.Context, b *Builder) Value {
if b.AddressPrefix == "" {
if b.AddressCodec == nil {
conn, err := b.GetClientConn()
if err != nil {
panic(err)
Expand All @@ -24,18 +27,20 @@ func (a addressStringType) NewValue(ctx context.Context, b *Builder) Value {
if resp == nil || resp.Config == nil {
panic("bech32 account address prefix is not set")
}
b.AddressPrefix = resp.Config.Bech32AccountAddressPrefix

b.AddressCodec = addresscodec.NewBech32Codec(resp.Config.Bech32AccountAddressPrefix)
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't call grpc every time to get the bech32 prefix. This should be done by hubl during unit and update and then cached

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, do you want me to do that here or in a follow-up? This PR does not touch hubl's code.

Copy link
Member

Choose a reason for hiding this comment

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

In a follow up

}
return &addressValue{addressPrefix: b.AddressPrefix}

return &addressValue{addressCodec: b.AddressCodec}
}

func (a addressStringType) DefaultValue() string {
return ""
}

type addressValue struct {
value string
addressPrefix string
value string
addressCodec address.Codec
}

func (a addressValue) Get(protoreflect.Value) (protoreflect.Value, error) {
Expand All @@ -48,10 +53,11 @@ func (a addressValue) String() string {

// Set implements the flag.Value interface for addressValue it only supports bech32 addresses.
func (a *addressValue) Set(s string) error {
_, err := types.GetFromBech32(s, a.addressPrefix)
_, err := a.addressCodec.StringToBytes(s)
if err != nil {
return err
return fmt.Errorf("invalid bech32 account address: %w", err)
}

a.value = s

return nil
Expand Down
Loading