Skip to content

Commit

Permalink
fix(client/v2): fix duplicate modules addition and add logger (backport
Browse files Browse the repository at this point in the history
#17390) (#17391)

Co-authored-by: Julien Robert <[email protected]>
  • Loading branch information
mergify[bot] and julienrbrt authored Aug 15, 2023
1 parent e76102f commit ae48128
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 31 deletions.
5 changes: 5 additions & 0 deletions client/v2/autocli/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down Expand Up @@ -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,
Expand Down
12 changes: 10 additions & 2 deletions client/v2/autocli/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
20 changes: 11 additions & 9 deletions client/v2/autocli/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"

"github.com/spf13/cobra"
"golang.org/x/exp/maps"
"google.golang.org/protobuf/reflect/protoreflect"
"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -42,7 +41,7 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
}

cmd := &cobra.Command{
SilenceUsage: true,
SilenceUsage: false,
Use: use,
Long: long,
Short: options.Short,
Expand Down Expand Up @@ -85,21 +84,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
}
Expand All @@ -110,7 +112,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
}
Expand Down
12 changes: 8 additions & 4 deletions client/v2/autocli/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down
14 changes: 8 additions & 6 deletions client/v2/autocli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions client/v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.50.0-beta.0
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.56.2
google.golang.org/protobuf v1.31.0
gotest.tools/v3 v3.5.0
Expand All @@ -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
Expand Down Expand Up @@ -137,6 +136,7 @@ require (
github.com/zondax/ledger-go v0.14.1 // indirect
go.etcd.io/bbolt v1.3.7 // indirect
golang.org/x/crypto v0.11.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
Expand Down
2 changes: 1 addition & 1 deletion simapp/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.20

require (
cosmossdk.io/api v0.7.0
cosmossdk.io/client/v2 v2.0.0-20230814141159-7b791eefdcdc
cosmossdk.io/client/v2 v2.0.0-20230815132412-bc2d6742a8d9
cosmossdk.io/core v0.9.0
cosmossdk.io/depinject v1.0.0-alpha.4
cosmossdk.io/log v1.2.0
Expand Down
4 changes: 2 additions & 2 deletions simapp/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1V
cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M=
cosmossdk.io/api v0.7.0 h1:QsEMIWuv9xWDbF2HZnW4Lpu1/SejCztPu0LQx7t6MN4=
cosmossdk.io/api v0.7.0/go.mod h1:kJFAEMLN57y0viszHDPLMmieF0471o5QAwwApa+270M=
cosmossdk.io/client/v2 v2.0.0-20230814141159-7b791eefdcdc h1:Uvy/KLgvEnLaFtI21c9q6ojXnq4tSBq2scO8h0Pw0GU=
cosmossdk.io/client/v2 v2.0.0-20230814141159-7b791eefdcdc/go.mod h1:E+EMapyhYZdkr00uStWnN39RWsw7d/F8QY3C/eBQrZk=
cosmossdk.io/client/v2 v2.0.0-20230815132412-bc2d6742a8d9 h1:JcvL6Km0atzOBmBJ1GH3MXPT8dfXawGShcMjafbxAc4=
cosmossdk.io/client/v2 v2.0.0-20230815132412-bc2d6742a8d9/go.mod h1:yEMp1yf2FQYPpNH/QTjLkCB1O9x3jXshV5UQU9bLOKA=
cosmossdk.io/collections v0.3.0 h1:v0eEqLBxebAV+t+Ahwf9tSJOu95HVLINwROXx2TTZ08=
cosmossdk.io/collections v0.3.0/go.mod h1:CHE1+niUElL9ikCpevRZcp0yqQ4TU0TrEEGirN0mvIg=
cosmossdk.io/core v0.9.0 h1:30ScAOHDIUOCg1DKAwqkho9wuQJnu7GUrMcg0XLioic=
Expand Down
2 changes: 1 addition & 1 deletion tests/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
cloud.google.com/go/compute/metadata v0.2.3 // indirect
cloud.google.com/go/iam v1.1.0 // indirect
cloud.google.com/go/storage v1.30.1 // indirect
cosmossdk.io/client/v2 v2.0.0-20230814141159-7b791eefdcdc // indirect
cosmossdk.io/client/v2 v2.0.0-20230815132412-bc2d6742a8d9 // indirect
cosmossdk.io/x/circuit v0.0.0-20230719143845-dff6b0e26aa4 // indirect
filippo.io/edwards25519 v1.0.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
Expand Down
4 changes: 2 additions & 2 deletions tests/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1V
cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M=
cosmossdk.io/api v0.7.0 h1:QsEMIWuv9xWDbF2HZnW4Lpu1/SejCztPu0LQx7t6MN4=
cosmossdk.io/api v0.7.0/go.mod h1:kJFAEMLN57y0viszHDPLMmieF0471o5QAwwApa+270M=
cosmossdk.io/client/v2 v2.0.0-20230814141159-7b791eefdcdc h1:Uvy/KLgvEnLaFtI21c9q6ojXnq4tSBq2scO8h0Pw0GU=
cosmossdk.io/client/v2 v2.0.0-20230814141159-7b791eefdcdc/go.mod h1:E+EMapyhYZdkr00uStWnN39RWsw7d/F8QY3C/eBQrZk=
cosmossdk.io/client/v2 v2.0.0-20230815132412-bc2d6742a8d9 h1:JcvL6Km0atzOBmBJ1GH3MXPT8dfXawGShcMjafbxAc4=
cosmossdk.io/client/v2 v2.0.0-20230815132412-bc2d6742a8d9/go.mod h1:yEMp1yf2FQYPpNH/QTjLkCB1O9x3jXshV5UQU9bLOKA=
cosmossdk.io/collections v0.3.0 h1:v0eEqLBxebAV+t+Ahwf9tSJOu95HVLINwROXx2TTZ08=
cosmossdk.io/collections v0.3.0/go.mod h1:CHE1+niUElL9ikCpevRZcp0yqQ4TU0TrEEGirN0mvIg=
cosmossdk.io/core v0.9.0 h1:30ScAOHDIUOCg1DKAwqkho9wuQJnu7GUrMcg0XLioic=
Expand Down
23 changes: 23 additions & 0 deletions x/crisis/autocli.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package crisis

import (
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
crisisv1beta1 "cosmossdk.io/api/cosmos/crisis/v1beta1"
)

// AutoCLIOptions implements the autocli.HasAutoCLIConfig interface.
func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
return &autocliv1.ModuleOptions{
Tx: &autocliv1.ServiceCommandDescriptor{
Service: crisisv1beta1.Msg_ServiceDesc.ServiceName,
RpcCommandOptions: []*autocliv1.RpcCommandOptions{
{
RpcMethod: "VerifyInvariant",
Use: "invariant-broken [module-name] [invariant-route]",
Short: "Submit proof that an invariant broken to halt the chain",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "invariant_module_name"}, {ProtoField: "invariant_route"}},
},
},
},
}
}
8 changes: 6 additions & 2 deletions x/gov/autocli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
}
Expand Down

0 comments on commit ae48128

Please sign in to comment.