fix(client/v2): improve resolver and tests (backport #16842) (#16861)

Co-authored-by: Julien Robert <[email protected]>

client/v2/autocli/app.go

@@ -1,10 +1,10 @@
 package autocli
 
 import (
-	"errors"
-
+	"github.com/cosmos/gogoproto/proto"
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
+	"google.golang.org/protobuf/reflect/protoregistry"
 
 	autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
 	"cosmossdk.io/client/v2/autocli/flag"
@@ -60,6 +60,8 @@ type AppOptions struct {
 func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
 	builder := &Builder{
 		Builder: flag.Builder{
+			TypeResolver:          protoregistry.GlobalTypes,
+			FileResolver:          proto.HybridResolver,
 			AddressCodec:          appOptions.AddressCodec,
 			ValidatorAddressCodec: appOptions.ValidatorAddressCodec,
 		},
@@ -74,8 +76,8 @@ func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
 }
 
 func (appOptions AppOptions) EnhanceRootCommandWithBuilder(rootCmd *cobra.Command, builder *Builder) error {
-	if builder.AddressCodec == nil || builder.ValidatorAddressCodec == nil {
-		return errors.New("address codec is required in builder")
+	if err := builder.Validate(); err != nil {
+		return err
 	}
 
 	// extract any custom commands from modules

client/v2/autocli/builder.go

@@ -1,6 +1,8 @@
 package autocli
 
 import (
+	"errors"
+
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
 
@@ -20,3 +22,23 @@ type Builder struct {
 
 	AddTxConnFlags func(*cobra.Command)
 }
+
+func (b *Builder) Validate() error {
+	if b.AddressCodec == nil {
+		return errors.New("address codec is required in builder")
+	}
+
+	if b.ValidatorAddressCodec == nil {
+		return errors.New("validator address codec is required in builder")
+	}
+
+	if b.TypeResolver == nil {
+		return errors.New("type resolver is required in builder")
+	}
+
+	if b.FileResolver == nil {
+		return errors.New("file resolver is required in builder")
+	}
+
+	return nil
+}

client/v2/autocli/common.go

@@ -35,14 +35,15 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
 	}
 
 	cmd := &cobra.Command{
-		Use:        use,
-		Long:       long,
-		Short:      options.Short,
-		Example:    options.Example,
-		Aliases:    options.Alias,
-		SuggestFor: options.SuggestFor,
-		Deprecated: options.Deprecated,
-		Version:    options.Version,
+		SilenceUsage: true,
+		Use:          use,
+		Long:         long,
+		Short:        options.Short,
+		Example:      options.Example,
+		Aliases:      options.Alias,
+		SuggestFor:   options.SuggestFor,
+		Deprecated:   options.Deprecated,
+		Version:      options.Version,
 	}
 
 	cmd.SetContext(context.Background())

client/v2/autocli/common_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/protobuf/reflect/protoregistry"
 	"gotest.tools/v3/assert"
 
 	reflectionv2alpha1 "cosmossdk.io/api/cosmos/base/reflection/v2alpha1"
@@ -19,7 +20,13 @@ import (
 	addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
 )
 
-func testExecCommon(t *testing.T, buildModuleCommand func(string, *Builder) (*cobra.Command, error), args ...string) *testClientConn {
+type fixture struct {
+	conn *testClientConn
+	b    *Builder
+}
+
+func initFixture(t *testing.T) *fixture {
+	t.Helper()
 	server := grpc.NewServer()
 	testpb.RegisterQueryServer(server, &testEchoServer{})
 	reflectionv2alpha1.RegisterReflectionServiceServer(server, &testReflectionServer{})
@@ -34,21 +41,12 @@ func testExecCommon(t *testing.T, buildModuleCommand func(string, *Builder) (*co
 
 	clientConn, err := grpc.Dial(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	assert.NilError(t, err)
-	defer func() {
-		err := clientConn.Close()
-		if err != nil {
-			panic(err)
-		}
-	}()
 
-	conn := &testClientConn{
-		ClientConn: clientConn,
-		t:          t,
-		out:        &bytes.Buffer{},
-		errorOut:   &bytes.Buffer{},
-	}
+	conn := &testClientConn{ClientConn: clientConn}
 	b := &Builder{
 		Builder: flag.Builder{
+			TypeResolver:          protoregistry.GlobalTypes,
+			FileResolver:          protoregistry.GlobalFiles,
 			AddressCodec:          addresscodec.NewBech32Codec("cosmos"),
 			ValidatorAddressCodec: addresscodec.NewBech32Codec("cosmosvaloper"),
 		},
@@ -58,59 +56,24 @@ func testExecCommon(t *testing.T, buildModuleCommand func(string, *Builder) (*co
 		AddQueryConnFlags: flags.AddQueryFlagsToCmd,
 		AddTxConnFlags:    flags.AddTxFlagsToCmd,
 	}
+	assert.NilError(t, b.Validate())
 
-	cmd, err := buildModuleCommand("test", b)
-	assert.NilError(t, err)
-	assert.NilError(t, err)
-	cmd.SetArgs(args)
-	cmd.SetOut(conn.out)
-	cmd.SetErr(conn.errorOut)
-	cmd.Execute()
-	return conn
+	return &fixture{
+		conn: conn,
+		b:    b,
+	}
 }
 
-func testExecCommonWithErr(t *testing.T, expectedErr string, buildModuleCommand func(string, *Builder) (*cobra.Command, error), args ...string) {
-	server := grpc.NewServer()
-	testpb.RegisterQueryServer(server, &testEchoServer{})
-	reflectionv2alpha1.RegisterReflectionServiceServer(server, &testReflectionServer{})
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	assert.NilError(t, err)
-	go func() {
-		err := server.Serve(listener)
-		if err != nil {
-			panic(err)
-		}
-	}()
-
-	clientConn, err := grpc.Dial(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
-	assert.NilError(t, err)
-	defer func() {
-		err := clientConn.Close()
-		if err != nil {
-			panic(err)
-		}
-	}()
-
-	conn := &testClientConn{
-		ClientConn: clientConn,
-		t:          t,
-		out:        &bytes.Buffer{},
-		errorOut:   &bytes.Buffer{},
-	}
-	b := &Builder{
-		Builder: flag.Builder{
-			AddressCodec:          addresscodec.NewBech32Codec("cosmos"),
-			ValidatorAddressCodec: addresscodec.NewBech32Codec("cosmosvaloper"),
-		},
-		GetClientConn: func(*cobra.Command) (grpc.ClientConnInterface, error) {
-			return conn, nil
-		},
-		AddQueryConnFlags: flags.AddQueryFlagsToCmd,
-		AddTxConnFlags:    flags.AddTxFlagsToCmd,
+func runCmd(conn *testClientConn, b *Builder, command func(moduleName string, b *Builder) (*cobra.Command, error), args ...string) (*bytes.Buffer, error) {
+	out := &bytes.Buffer{}
+	cmd, err := command("test", b)
+	if err != nil {
+		return out, err
 	}
 
-	_, err = buildModuleCommand("test", b)
-	assert.Equal(t, expectedErr, err.Error())
+	cmd.SetArgs(args)
+	cmd.SetOut(out)
+	return out, cmd.Execute()
 }
 
 type testReflectionServer struct {
@@ -126,3 +89,26 @@ func (t testReflectionServer) GetConfigurationDescriptor(_ context.Context, clie
 }
 
 var _ reflectionv2alpha1.ReflectionServiceServer = testReflectionServer{}
+
+type testClientConn struct {
+	*grpc.ClientConn
+	lastRequest  interface{}
+	lastResponse interface{}
+}
+
+func (t *testClientConn) Invoke(ctx context.Context, method string, args, reply interface{}, opts ...grpc.CallOption) error {
+	err := t.ClientConn.Invoke(ctx, method, args, reply, opts...)
+	t.lastRequest = args
+	t.lastResponse = reply
+	return err
+}
+
+type testEchoServer struct {
+	testpb.UnimplementedQueryServer
+}
+
+func (t testEchoServer) Echo(_ context.Context, request *testpb.EchoRequest) (*testpb.EchoResponse, error) {
+	return &testpb.EchoResponse{Request: request}, nil
+}
+
+var _ testpb.QueryServer = testEchoServer{}

client/v2/autocli/flag/register.go

@@ -75,7 +75,7 @@ func (b *Builder) addMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, m
 	}
 
 	if hasVarargs {
-		handler.CobraArgs = cobra.MinimumNArgs(n)
+		handler.CobraArgs = cobra.MinimumNArgs(n - 1)
 		handler.hasVarargs = true
 	} else if hasOptional {
 		handler.CobraArgs = cobra.RangeArgs(n-1, n)

client/v2/autocli/msg.go

@@ -8,7 +8,6 @@ import (
 	"github.com/spf13/cobra"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/reflect/protoreflect"
-	"google.golang.org/protobuf/reflect/protoregistry"
 )
 
 // BuildMsgCommand builds the msg commands for all the provided modules. If a custom command is provided for a
@@ -42,12 +41,7 @@ func (b *Builder) AddMsgServiceCommands(cmd *cobra.Command, cmdDescriptor *autoc
 		return nil
 	}
 
-	resolver := b.FileResolver
-	if b.FileResolver == nil {
-		resolver = protoregistry.GlobalFiles
-	}
-
-	descriptor, err := resolver.FindDescriptorByName(protoreflect.FullName(cmdDescriptor.Service))
+	descriptor, err := b.FileResolver.FindDescriptorByName(protoreflect.FullName(cmdDescriptor.Service))
 	if err != nil {
 		return errors.Errorf("can't find service %s: %v", cmdDescriptor.Service, err)
 	}