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(client/v2): fix duplicate modules addition and add logger (backport #17390) #17391

Merged
merged 3 commits into from
Aug 15, 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
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