fix(client/v2): resolve keyring flags properly (backport cosmos#19060) (

cosmos#19105)

Co-authored-by: Julien Robert <julien@rbrt.fr>

client/cmd.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"context"
 	"crypto/tls"
 	"fmt"
 	"strings"
@@ -278,7 +279,7 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err
 		from, _ := flagSet.GetString(flags.FlagFrom)
 		fromAddr, fromName, keyType, err := GetFromFields(clientCtx, clientCtx.Keyring, from)
 		if err != nil {
-			return clientCtx, err
+			return clientCtx, fmt.Errorf("failed to convert address field to address: %w", err)
 		}
 
 		clientCtx = clientCtx.WithFrom(from).WithFromAddress(fromAddr).WithFromName(fromName)
@@ -358,13 +359,6 @@ 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 {
-		v = &clientCtx
-	}
-
-	clientCtxPtr := v.(*Context)
-	*clientCtxPtr = clientCtx
-
+	cmd.SetContext(context.WithValue(cmd.Context(), ClientContextKey, &clientCtx))
 	return nil
 }

client/v2/CHANGELOG.md

@@ -36,6 +36,15 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### Improvements
+
+* [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands.
+  * Note, the given command must have a `client.Context` in its context.
+
+### Bug Fixes
+
+* [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Simplify key flag parsing logic in flag handler.
+
 ## [v2.0.0-beta.1] - 2023-11-07
 
 This is the first tagged version of client/v2.

client/v2/autocli/app.go

@@ -78,7 +78,6 @@ func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
 			ConsensusAddressCodec: appOptions.ConsensusAddressCodec,
 			Keyring:               appOptions.Keyring,
 		},
-		ClientCtx:    appOptions.ClientCtx,
 		TxConfigOpts: appOptions.TxConfigOpts,
 		GetClientConn: func(cmd *cobra.Command) (grpc.ClientConnInterface, error) {
 			return client.GetClientQueryContext(cmd)
@@ -119,7 +118,7 @@ func (appOptions AppOptions) EnhanceRootCommandWithBuilder(rootCmd *cobra.Comman
 			return err
 		}
 	} else {
-		queryCmd, err := builder.BuildQueryCommand(appOptions, customQueryCmds)
+		queryCmd, err := builder.BuildQueryCommand(rootCmd.Context(), appOptions, customQueryCmds)
 		if err != nil {
 			return err
 		}
@@ -132,7 +131,7 @@ func (appOptions AppOptions) EnhanceRootCommandWithBuilder(rootCmd *cobra.Comman
 			return err
 		}
 	} else {
-		subCmd, err := builder.BuildMsgCommand(appOptions, customMsgCmds)
+		subCmd, err := builder.BuildMsgCommand(rootCmd.Context(), appOptions, customMsgCmds)
 		if err != nil {
 			return err
 		}

client/v2/autocli/builder.go

@@ -6,7 +6,6 @@ import (
 
 	"cosmossdk.io/client/v2/autocli/flag"
 
-	"github.com/cosmos/cosmos-sdk/client"
 	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 )
 
@@ -19,9 +18,6 @@ type Builder struct {
 	// from a given context.
 	GetClientConn func(*cobra.Command) (grpc.ClientConnInterface, error)
 
-	// ClientCtx contains the necessary information needed to execute the commands.
-	ClientCtx client.Context
-
 	// TxConfigOptions is required to support sign mode textual
 	TxConfigOpts authtx.ConfigOptions
 

client/v2/autocli/common.go

@@ -1,7 +1,6 @@
 package autocli
 
 import (
-	"context"
 	"fmt"
 
 	"github.com/spf13/cobra"
@@ -56,7 +55,6 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
 		Version:      options.Version,
 	}
 
-	cmd.SetContext(context.Background())
 	binder, err := b.AddMessageFlags(cmd.Context(), cmd.Flags(), inputType, options)
 	if err != nil {
 		return nil, err
@@ -180,7 +178,7 @@ func (b *Builder) enhanceCommandCommon(
 // 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 {
 	if queryCmdDesc := modOpts.Query; queryCmdDesc != nil {
-		subCmd := topLevelCmd(moduleName, fmt.Sprintf("Querying commands for the %s module", moduleName))
+		subCmd := topLevelCmd(cmd.Context(), moduleName, fmt.Sprintf("Querying commands for the %s module", moduleName))
 		if err := builder.AddQueryServiceCommands(subCmd, queryCmdDesc); err != nil {
 			return err
 		}
@@ -194,7 +192,7 @@ func enhanceQuery(builder *Builder, moduleName string, cmd *cobra.Command, modOp
 // 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 {
 	if txCmdDesc := modOpts.Tx; txCmdDesc != nil {
-		subCmd := topLevelCmd(moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
+		subCmd := topLevelCmd(cmd.Context(), moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
 		if err := builder.AddMsgServiceCommands(subCmd, txCmdDesc); err != nil {
 			return err
 		}

client/v2/autocli/common_test.go

@@ -28,8 +28,9 @@ import (
 )
 
 type fixture struct {
-	conn *testClientConn
-	b    *Builder
+	conn      *testClientConn
+	b         *Builder
+	clientCtx client.Context
 }
 
 func initFixture(t *testing.T) *fixture {
@@ -60,8 +61,7 @@ func initFixture(t *testing.T) *fixture {
 	interfaceRegistry := encodingConfig.Codec.InterfaceRegistry()
 	banktypes.RegisterInterfaces(interfaceRegistry)
 
-	var initClientCtx client.Context
-	initClientCtx = initClientCtx.
+	clientCtx := client.Context{}.
 		WithKeyring(kr).
 		WithKeyringDir(home).
 		WithHomeDir(home).
@@ -86,19 +86,19 @@ func initFixture(t *testing.T) *fixture {
 		},
 		AddQueryConnFlags: flags.AddQueryFlagsToCmd,
 		AddTxConnFlags:    flags.AddTxFlagsToCmd,
-		ClientCtx:         initClientCtx,
 	}
 	assert.NilError(t, b.ValidateAndComplete())
 
 	return &fixture{
-		conn: conn,
-		b:    b,
+		conn:      conn,
+		b:         b,
+		clientCtx: clientCtx,
 	}
 }
 
-func runCmd(conn *testClientConn, b *Builder, command func(moduleName string, b *Builder) (*cobra.Command, error), args ...string) (*bytes.Buffer, error) {
+func runCmd(fixture *fixture, command func(moduleName string, f *fixture) (*cobra.Command, error), args ...string) (*bytes.Buffer, error) {
 	out := &bytes.Buffer{}
-	cmd, err := command("test", b)
+	cmd, err := command("test", fixture)
 	if err != nil {
 		return out, err
 	}
@@ -212,14 +212,13 @@ func TestErrorBuildCommand(t *testing.T) {
 				Tx:    commandDescriptor,
 			},
 		},
-		ClientCtx: b.ClientCtx,
 	}
 
-	_, err := b.BuildMsgCommand(appOptions, nil)
+	_, err := b.BuildMsgCommand(context.Background(), appOptions, nil)
 	assert.ErrorContains(t, err, "can't find field un-existent-proto-field")
 
 	appOptions.ModuleOptions["test"].Tx = &autocliv1.ServiceCommandDescriptor{Service: "un-existent-service"}
 	appOptions.ModuleOptions["test"].Query = &autocliv1.ServiceCommandDescriptor{Service: "un-existent-service"}
-	_, err = b.BuildMsgCommand(appOptions, nil)
+	_, err = b.BuildMsgCommand(context.Background(), appOptions, nil)
 	assert.ErrorContains(t, err, "can't find service un-existent-service")
 }

client/v2/autocli/flag/address.go

@@ -62,11 +62,9 @@ func (a *addressValue) Set(s string) error {
 		return nil
 	}
 
-	_, err = a.addressCodec.StringToBytes(s)
-	if err != nil {
-		return fmt.Errorf("invalid account address or key name: %w", err)
-	}
-
+	// failed all validation, just accept the input.
+	// TODO(@julienrbrt), for final client/v2 2.0.0 revert the logic and
+	// do a better keyring instantiation.
 	a.value = s
 
 	return nil
@@ -129,7 +127,11 @@ func (a *consensusAddressValue) Set(s string) error {
 	var pk cryptotypes.PubKey
 	err2 := cdc.UnmarshalInterfaceJSON([]byte(s), &pk)
 	if err2 != nil {
-		return fmt.Errorf("input isn't a pubkey %w or is an invalid account address: %w", err, err2)
+		// failed all validation, just accept the input.
+		// TODO(@julienrbrt), for final client/v2 2.0.0 revert the logic and
+		// do a better keyring instantiation.
+		a.value = s
+		return nil
 	}
 
 	a.value, err = a.addressCodec.BytesToString(pk.Address())

client/v2/autocli/msg.go

@@ -25,8 +25,9 @@ import (
 // BuildMsgCommand builds the msg commands for all the provided modules. If a custom command is provided for a
 // module, this is used instead of any automatically generated CLI commands. This allows apps to a fully dynamic client
 // with a more customized experience if a binary with custom commands is downloaded.
-func (b *Builder) BuildMsgCommand(appOptions AppOptions, customCmds map[string]*cobra.Command) (*cobra.Command, error) {
-	msgCmd := topLevelCmd("tx", "Transaction subcommands")
+func (b *Builder) BuildMsgCommand(ctx context.Context, appOptions AppOptions, customCmds map[string]*cobra.Command) (*cobra.Command, error) {
+	msgCmd := topLevelCmd(ctx, "tx", "Transaction subcommands")
+
 	if err := b.enhanceCommandCommon(msgCmd, msgCmdType, appOptions, customCmds); err != nil {
 		return nil, err
 	}
@@ -41,7 +42,7 @@ func (b *Builder) AddMsgServiceCommands(cmd *cobra.Command, cmdDescriptor *autoc
 	for cmdName, subCmdDescriptor := range cmdDescriptor.SubCommands {
 		subCmd := findSubCommand(cmd, cmdName)
 		if subCmd == nil {
-			subCmd = topLevelCmd(cmdName, fmt.Sprintf("Tx commands for the %s service", subCmdDescriptor.Service))
+			subCmd = topLevelCmd(cmd.Context(), 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.
@@ -112,8 +113,6 @@ func (b *Builder) AddMsgServiceCommands(cmd *cobra.Command, cmdDescriptor *autoc
 // BuildMsgMethodCommand returns a command that outputs the JSON representation of the message.
 func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor, options *autocliv1.RpcCommandOptions) (*cobra.Command, error) {
 	cmd, err := b.buildMethodCommandCommon(descriptor, options, func(cmd *cobra.Command, input protoreflect.Message) error {
-		cmd.SetContext(context.WithValue(context.Background(), client.ClientContextKey, &b.ClientCtx))
-
 		clientCtx, err := client.GetClientTxContext(cmd)
 		if err != nil {
 			return err

client/v2/autocli/msg_test.go

@@ -1,6 +1,7 @@
 package autocli
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
@@ -11,18 +12,22 @@ import (
 	autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
 	bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
 	"cosmossdk.io/client/v2/internal/testpb"
+
+	"github.com/cosmos/cosmos-sdk/client"
 )
 
-var buildModuleMsgCommand = func(moduleName string, b *Builder) (*cobra.Command, error) {
-	cmd := topLevelCmd(moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
-	err := b.AddMsgServiceCommands(cmd, bankAutoCLI)
+var buildModuleMsgCommand = func(moduleName string, f *fixture) (*cobra.Command, error) {
+	ctx := context.WithValue(context.Background(), client.ClientContextKey, &f.clientCtx)
+	cmd := topLevelCmd(ctx, moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
+	err := f.b.AddMsgServiceCommands(cmd, bankAutoCLI)
 	return cmd, err
 }
 
-func buildCustomModuleMsgCommand(cmdDescriptor *autocliv1.ServiceCommandDescriptor) func(moduleName string, b *Builder) (*cobra.Command, error) {
-	return func(moduleName string, b *Builder) (*cobra.Command, error) {
-		cmd := topLevelCmd(moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
-		err := b.AddMsgServiceCommands(cmd, cmdDescriptor)
+func buildCustomModuleMsgCommand(cmdDescriptor *autocliv1.ServiceCommandDescriptor) func(moduleName string, f *fixture) (*cobra.Command, error) {
+	return func(moduleName string, f *fixture) (*cobra.Command, error) {
+		ctx := context.WithValue(context.Background(), client.ClientContextKey, &f.clientCtx)
+		cmd := topLevelCmd(ctx, moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
+		err := f.b.AddMsgServiceCommands(cmd, cmdDescriptor)
 		return cmd, err
 	}
 }
@@ -42,15 +47,15 @@ var bankAutoCLI = &autocliv1.ServiceCommandDescriptor{
 
 func TestMsg(t *testing.T) {
 	fixture := initFixture(t)
-	out, err := runCmd(fixture.conn, fixture.b, buildModuleMsgCommand, "send",
+	out, err := runCmd(fixture, buildModuleMsgCommand, "send",
 		"cosmos1y74p8wyy4enfhfn342njve6cjmj5c8dtl6emdk", "cosmos1y74p8wyy4enfhfn342njve6cjmj5c8dtl6emdk", "1foo",
 		"--generate-only",
 		"--output", "json",
 	)
 	assert.NilError(t, err)
 	golden.Assert(t, out.String(), "msg-output.golden")
 
-	out, err = runCmd(fixture.conn, fixture.b, buildCustomModuleMsgCommand(&autocliv1.ServiceCommandDescriptor{
+	out, err = runCmd(fixture, buildCustomModuleMsgCommand(&autocliv1.ServiceCommandDescriptor{
 		Service: bankv1beta1.Msg_ServiceDesc.ServiceName,
 		RpcCommandOptions: []*autocliv1.RpcCommandOptions{
 			{
@@ -71,7 +76,7 @@ func TestMsg(t *testing.T) {
 	assert.NilError(t, err)
 	golden.Assert(t, out.String(), "msg-output.golden")
 
-	out, err = runCmd(fixture.conn, fixture.b, buildCustomModuleMsgCommand(&autocliv1.ServiceCommandDescriptor{
+	out, err = runCmd(fixture, buildCustomModuleMsgCommand(&autocliv1.ServiceCommandDescriptor{
 		Service: bankv1beta1.Msg_ServiceDesc.ServiceName,
 		RpcCommandOptions: []*autocliv1.RpcCommandOptions{
 			{
@@ -98,12 +103,12 @@ func TestMsg(t *testing.T) {
 func TestMsgOptionsError(t *testing.T) {
 	fixture := initFixture(t)
 
-	_, err := runCmd(fixture.conn, fixture.b, buildModuleMsgCommand,
+	_, err := runCmd(fixture, buildModuleMsgCommand,
 		"send", "5",
 	)
 	assert.ErrorContains(t, err, "accepts 3 arg(s)")
 
-	_, err = runCmd(fixture.conn, fixture.b, buildModuleMsgCommand,
+	_, err = runCmd(fixture, buildModuleMsgCommand,
 		"send", "foo", "bar", "invalid",
 	)
 	assert.ErrorContains(t, err, "invalid argument")
@@ -112,11 +117,11 @@ func TestMsgOptionsError(t *testing.T) {
 func TestHelpMsg(t *testing.T) {
 	fixture := initFixture(t)
 
-	out, err := runCmd(fixture.conn, fixture.b, buildModuleMsgCommand, "-h")
+	out, err := runCmd(fixture, buildModuleMsgCommand, "-h")
 	assert.NilError(t, err)
 	golden.Assert(t, out.String(), "help-toplevel-msg.golden")
 
-	out, err = runCmd(fixture.conn, fixture.b, buildModuleMsgCommand, "send", "-h")
+	out, err = runCmd(fixture, buildModuleMsgCommand, "send", "-h")
 	assert.NilError(t, err)
 	golden.Assert(t, out.String(), "help-echo-msg.golden")
 }
@@ -135,7 +140,7 @@ func TestBuildCustomMsgCommand(t *testing.T) {
 		},
 	}
 
-	cmd, err := b.BuildMsgCommand(appOptions, map[string]*cobra.Command{
+	cmd, err := b.BuildMsgCommand(context.Background(), appOptions, map[string]*cobra.Command{
 		"test": {Use: "test", Run: func(cmd *cobra.Command, args []string) {
 			customCommandCalled = true
 		}},
@@ -153,7 +158,7 @@ func TestNotFoundErrorsMsg(t *testing.T) {
 	b.AddTxConnFlags = nil
 
 	buildModuleMsgCommand := func(moduleName string, cmdDescriptor *autocliv1.ServiceCommandDescriptor) (*cobra.Command, error) {
-		cmd := topLevelCmd(moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
+		cmd := topLevelCmd(context.Background(), moduleName, fmt.Sprintf("Transactions commands for the %s module", moduleName))
 
 		err := b.AddMsgServiceCommands(cmd, cmdDescriptor)
 		return cmd, err