fix(client/v2): fix duplicate modules addition and add logger (#17390)

client/v2/autocli/app.go

@@ -12,6 +12,7 @@ import (
 	"cosmossdk.io/core/address"
 	"cosmossdk.io/core/appmodule"
 	"cosmossdk.io/depinject"
+	"cosmossdk.io/log"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
@@ -29,6 +30,9 @@ import (
 type AppOptions struct {
 	depinject.In
 
+	// Logger is the logger to use for client/v2.
+	Logger log.Logger
+
 	// Modules are the AppModule implementations for the modules in the app.
 	Modules map[string]appmodule.AppModule
 
@@ -64,6 +68,7 @@ type AppOptions struct {
 //	err = autoCliOpts.EnhanceRootCommand(rootCmd)
 func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
 	builder := &Builder{
+		Logger: appOptions.Logger,
 		Builder: flag.Builder{
 			TypeResolver:          protoregistry.GlobalTypes,
 			FileResolver:          proto.HybridResolver,

client/v2/autocli/builder.go

@@ -8,23 +8,31 @@ import (
 
 	"cosmossdk.io/client/v2/autocli/flag"
 	"cosmossdk.io/client/v2/autocli/keyring"
+	"cosmossdk.io/log"
 )
 
 // Builder manages options for building CLI commands.
 type Builder struct {
 	// flag.Builder embeds the flag builder and its options.
 	flag.Builder
 
+	// Logger is the logger used by the builder.
+	Logger log.Logger
+
 	// GetClientConn specifies how CLI commands will resolve a grpc.ClientConnInterface
 	// from a given context.
 	GetClientConn func(*cobra.Command) (grpc.ClientConnInterface, error)
 
+	// AddQueryConnFlags and AddTxConnFlags are functions that add flags to query and transaction commands
 	AddQueryConnFlags func(*cobra.Command)
-
-	AddTxConnFlags func(*cobra.Command)
+	AddTxConnFlags    func(*cobra.Command)
 }
 
 func (b *Builder) Validate() error {
+	if b.Logger == nil {
+		b.Logger = log.NewNopLogger()
+	}
+
 	if b.AddressCodec == nil {
 		return errors.New("address codec is required in builder")
 	}

client/v2/autocli/common.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
-	"golang.org/x/exp/maps"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"sigs.k8s.io/yaml"
 
@@ -43,7 +42,7 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
 	}
 
 	cmd := &cobra.Command{
-		SilenceUsage: true,
+		SilenceUsage: false,
 		Use:          use,
 		Long:         long,
 		Short:        options.Short,
@@ -86,21 +85,24 @@ func (b *Builder) enhanceCommandCommon(
 ) error {
 	moduleOptions := appOptions.ModuleOptions
 	if len(moduleOptions) == 0 {
-		moduleOptions = map[string]*autocliv1.ModuleOptions{}
-		for name, module := range appOptions.Modules {
+		moduleOptions = make(map[string]*autocliv1.ModuleOptions)
+	}
+	for name, module := range appOptions.Modules {
+		if _, ok := moduleOptions[name]; !ok {
 			if module, ok := module.(HasAutoCLIConfig); ok {
 				moduleOptions[name] = module.AutoCLIOptions()
+			} else {
+				moduleOptions[name] = nil
 			}
 		}
 	}
 
-	modules := append(maps.Keys(appOptions.Modules), maps.Keys(moduleOptions)...)
-	for _, moduleName := range modules {
-		modOpts, hasModuleOptions := moduleOptions[moduleName]
+	for moduleName, modOpts := range moduleOptions {
+		hasModuleOptions := modOpts != nil
 
 		// if we have an existing command skip adding one here
 		if subCmd := findSubCommand(cmd, moduleName); subCmd != nil {
-			if hasModuleOptions {
+			if hasModuleOptions { // check if we need to enhance the existing command
 				if err := enhanceCustomCmd(b, subCmd, cmdType, modOpts); err != nil {
 					return err
 				}
@@ -111,7 +113,7 @@ func (b *Builder) enhanceCommandCommon(
 
 		// if we have a custom command use that instead of generating one
 		if custom, ok := customCmds[moduleName]; ok {
-			if hasModuleOptions {
+			if hasModuleOptions { // check if we need to enhance the existing command
 				if err := enhanceCustomCmd(b, custom, cmdType, modOpts); err != nil {
 					return err
 				}

client/v2/autocli/msg.go

@@ -27,12 +27,16 @@ func (b *Builder) BuildMsgCommand(appOptions AppOptions, customCmds map[string]*
 // order to add auto-generated commands to an existing command.
 func (b *Builder) AddMsgServiceCommands(cmd *cobra.Command, cmdDescriptor *autocliv1.ServiceCommandDescriptor) error {
 	for cmdName, subCmdDescriptor := range cmdDescriptor.SubCommands {
-		subCmd := topLevelCmd(cmdName, fmt.Sprintf("Tx commands for the %s service", subCmdDescriptor.Service))
+		subCmd := findSubCommand(cmd, cmdName)
+		if subCmd == nil {
+			subCmd = topLevelCmd(cmdName, fmt.Sprintf("Tx commands for the %s service", subCmdDescriptor.Service))
+		}
+
 		// Add recursive sub-commands if there are any. This is used for nested services.
-		err := b.AddMsgServiceCommands(subCmd, subCmdDescriptor)
-		if err != nil {
+		if err := b.AddMsgServiceCommands(subCmd, subCmdDescriptor); err != nil {
 			return err
 		}
+
 		cmd.AddCommand(subCmd)
 	}
 
@@ -77,7 +81,7 @@ func (b *Builder) AddMsgServiceCommands(cmd *cobra.Command, cmdDescriptor *autoc
 
 		if findSubCommand(cmd, methodCmd.Name()) != nil {
 			// do not overwrite existing commands
-			// @julienrbrt: should we display a warning?
+			// we do not display a warning because you may want to overwrite an autocli command
 			continue
 		}
 

client/v2/autocli/query.go

@@ -31,9 +31,12 @@ func (b *Builder) BuildQueryCommand(appOptions AppOptions, customCmds map[string
 // order to add auto-generated commands to an existing command.
 func (b *Builder) AddQueryServiceCommands(cmd *cobra.Command, cmdDescriptor *autocliv1.ServiceCommandDescriptor) error {
 	for cmdName, subCmdDesc := range cmdDescriptor.SubCommands {
-		subCmd := topLevelCmd(cmdName, fmt.Sprintf("Querying commands for the %s service", subCmdDesc.Service))
-		err := b.AddQueryServiceCommands(subCmd, subCmdDesc)
-		if err != nil {
+		subCmd := findSubCommand(cmd, cmdName)
+		if subCmd == nil {
+			subCmd = topLevelCmd(cmdName, fmt.Sprintf("Querying commands for the %s service", subCmdDesc.Service))
+		}
+
+		if err := b.AddQueryServiceCommands(subCmd, subCmdDesc); err != nil {
 			return err
 		}
 
@@ -63,8 +66,7 @@ func (b *Builder) AddQueryServiceCommands(cmd *cobra.Command, cmdDescriptor *aut
 		}
 	}
 
-	n := methods.Len()
-	for i := 0; i < n; i++ {
+	for i := 0; i < methods.Len(); i++ {
 		methodDescriptor := methods.Get(i)
 		methodOpts, ok := rpcOptMap[methodDescriptor.Name()]
 		if !ok {
@@ -82,7 +84,7 @@ func (b *Builder) AddQueryServiceCommands(cmd *cobra.Command, cmdDescriptor *aut
 
 		if findSubCommand(cmd, methodCmd.Name()) != nil {
 			// do not overwrite existing commands
-			// @julienrbrt: should we display a warning?
+			// we do not display a warning because you may want to overwrite an autocli command
 			continue
 		}
 

client/v2/go.mod

@@ -6,13 +6,13 @@ require (
 	cosmossdk.io/api v0.7.0
 	cosmossdk.io/core v0.9.0
 	cosmossdk.io/depinject v1.0.0-alpha.4
+	cosmossdk.io/log v1.2.0
 	github.com/cockroachdb/errors v1.10.0
 	github.com/cosmos/cosmos-proto v1.0.0-beta.3
 	github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230718211500-1d74652f6021
 	github.com/cosmos/gogoproto v1.4.10
 	github.com/spf13/cobra v1.7.0
 	github.com/spf13/pflag v1.0.5
-	golang.org/x/exp v0.0.0-20230711153332-06a737ee72cb
 	google.golang.org/grpc v1.57.0
 	google.golang.org/protobuf v1.31.0
 	gotest.tools/v3 v3.5.0
@@ -22,7 +22,6 @@ require (
 require (
 	cosmossdk.io/collections v0.3.0 // indirect
 	cosmossdk.io/errors v1.0.0 // indirect
-	cosmossdk.io/log v1.2.0 // indirect
 	cosmossdk.io/math v1.0.1 // indirect
 	cosmossdk.io/store v1.0.0-alpha.1 // indirect
 	cosmossdk.io/x/tx v0.9.1 // indirect
@@ -136,6 +135,7 @@ require (
 	github.com/zondax/ledger-go v0.14.1 // indirect
 	go.etcd.io/bbolt v1.3.7 // indirect
 	golang.org/x/crypto v0.12.0 // indirect
+	golang.org/x/exp v0.0.0-20230711153332-06a737ee72cb // indirect
 	golang.org/x/net v0.12.0 // indirect
 	golang.org/x/sync v0.3.0 // indirect
 	golang.org/x/sys v0.11.0 // indirect

x/crisis/autocli.go

@@ -18,9 +18,6 @@ func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
 					PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "invariant_module_name"}, {ProtoField: "invariant_route"}},
 				},
 			},
-			SubCommands: map[string]*autocliv1.ServiceCommandDescriptor{
-				"v1beta1": {Service: crisisv1beta1.Msg_ServiceDesc.ServiceName},
-			},
 		},
 	}
 }

x/gov/autocli.go

@@ -93,15 +93,19 @@ func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
 			},
 			// map v1beta1 as a sub-command
 			SubCommands: map[string]*autocliv1.ServiceCommandDescriptor{
-				"v1beta1": {Service: govv1beta1.Query_ServiceDesc.ServiceName},
+				"v1beta1": {
+					Service: govv1beta1.Query_ServiceDesc.ServiceName,
+				},
 			},
 			EnhanceCustomCommand: true, // We still have manual commands in gov that we want to keep
 		},
 		Tx: &autocliv1.ServiceCommandDescriptor{
 			Service: govv1.Msg_ServiceDesc.ServiceName,
 			// map v1beta1 as a sub-command
 			SubCommands: map[string]*autocliv1.ServiceCommandDescriptor{
-				"v1beta1": {Service: govv1beta1.Msg_ServiceDesc.ServiceName},
+				"v1beta1": {
+					Service: govv1beta1.Msg_ServiceDesc.ServiceName,
+				},
 			},
 		},
 	}