Use Context in Command instead of Argument + Util (#6572)

* Use context

* use PersistentPreRunE

* undo

* use init context

* Update types

* update tests

* implement tests

* Update simapp/cmd/simcli/main.go

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update simapp/cmd/simcli/main.go

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update x/bank/client/cli/tx.go

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* fix build

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

client/cmd.go

@@ -11,6 +11,23 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 )
 
+type contextKey string
+
+// ClientContextKey defines the context key used to retrieve a client.Context from
+// a command's Context.
+const ClientContextKey = contextKey("client.context")
+
+// SetCmdClientContextHandler is to be used in a command pre-hook execution to
+// read flags that populate a Context and sets that to the command's Context.
+func SetCmdClientContextHandler(clientCtx Context, cmd *cobra.Command) (err error) {
+	clientCtx, err = ReadPersistentCommandFlags(clientCtx, cmd.Flags())
+	if err != nil {
+		return err
+	}
+
+	return SetCmdClientContext(cmd, clientCtx)
+}
+
 // ValidateCmd returns unknown command error or Help display if help flag set
 func ValidateCmd(cmd *cobra.Command, args []string) error {
 	var unknownCmd string
@@ -155,3 +172,27 @@ func ReadTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err
 
 	return clientCtx, nil
 }
+
+// GetClientContextFromCmd returns a Context from a command or an empty Context
+// if it has not been set.
+func GetClientContextFromCmd(cmd *cobra.Command) Context {
+	if v := cmd.Context().Value(ClientContextKey); v != nil {
+		clientCtxPtr := v.(*Context)
+		return *clientCtxPtr
+	}
+
+	return Context{}
+}
+
+// SetCmdClientContext sets a command's Context value to the provided argument.
+func SetCmdClientContext(cmd *cobra.Command, clientCtx Context) error {
+	v := cmd.Context().Value(ClientContextKey)
+	if v == nil {
+		return errors.New("client context not set")
+	}
+
+	clientCtxPtr := v.(*Context)
+	*clientCtxPtr = clientCtx
+
+	return nil
+}

client/cmd_test.go

@@ -1,12 +1,16 @@
 package client_test
 
 import (
+	"context"
+	"fmt"
+	"io/ioutil"
 	"testing"
 
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/client/flags"
 )
 
 func TestValidateCmd(t *testing.T) {
@@ -50,3 +54,66 @@ func TestValidateCmd(t *testing.T) {
 		require.Equal(t, tt.wantErr, err != nil, tt.reason)
 	}
 }
+
+func TestSetCmdClientContextHandler(t *testing.T) {
+	initClientCtx := client.Context{}.WithHomeDir("/foo/bar").WithChainID("test-chain")
+
+	newCmd := func() *cobra.Command {
+		c := &cobra.Command{
+			PreRunE: func(cmd *cobra.Command, args []string) error {
+				return client.SetCmdClientContextHandler(initClientCtx, cmd)
+			},
+			RunE: func(cmd *cobra.Command, _ []string) error {
+				clientCtx := client.GetClientContextFromCmd(cmd)
+				_, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags())
+				if err != nil {
+					return err
+				}
+
+				return nil
+			},
+		}
+
+		c.Flags().String(flags.FlagChainID, "", "network chain ID")
+
+		return c
+	}
+
+	testCases := []struct {
+		name            string
+		expectedContext client.Context
+		args            []string
+	}{
+		{
+			"no flags set",
+			initClientCtx,
+			[]string{},
+		},
+		{
+			"flags set",
+			initClientCtx.WithChainID("new-chain-id"),
+			[]string{
+				fmt.Sprintf("--%s=new-chain-id", flags.FlagChainID),
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			ctx = context.WithValue(ctx, client.ClientContextKey, &client.Context{})
+
+			cmd := newCmd()
+			cmd.SetOut(ioutil.Discard)
+			cmd.SetErr(ioutil.Discard)
+			cmd.SetArgs(tc.args)
+
+			require.NoError(t, cmd.ExecuteContext(ctx))
+
+			clientCtx := client.GetClientContextFromCmd(cmd)
+			require.Equal(t, tc.expectedContext, clientCtx)
+		})
+	}
+}

simapp/cmd/simcli/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"os"
 
@@ -33,8 +34,10 @@ func init() {
 	authclient.Codec = encodingConfig.Marshaler
 }
 
+// TODO: setup keybase, viper object, etc. to be passed into
+// the below functions and eliminate global vars, like we do
+// with the cdc
 func main() {
-	// Configure cobra to sort commands
 	cobra.EnableCommandSorting = false
 
 	// Read in the configuration file for the sdk
@@ -44,19 +47,13 @@ func main() {
 	config.SetBech32PrefixForConsensusNode(sdk.Bech32PrefixConsAddr, sdk.Bech32PrefixConsPub)
 	config.Seal()
 
-	// TODO: setup keybase, viper object, etc. to be passed into
-	// the below functions and eliminate global vars, like we do
-	// with the cdc
-
 	rootCmd := &cobra.Command{
 		Use:   "simcli",
 		Short: "Command line interface for interacting with simapp",
 	}
 
-	// Add --chain-id to persistent flags and mark it required
 	rootCmd.PersistentFlags().String(flags.FlagChainID, "", "network chain ID")
 
-	// Construct Root Command
 	rootCmd.AddCommand(
 		rpc.StatusCommand(),
 		queryCmd(),
@@ -71,9 +68,11 @@ func main() {
 	// Add flags and prefix all env exposed with GA
 	executor := cli.PrepareMainCmd(rootCmd, "GA", simapp.DefaultCLIHome)
 
-	err := executor.Execute()
+	ctx := context.Background()
-	if err != nil {
+	ctx = context.WithValue(ctx, client.ClientContextKey, &client.Context{})
-		fmt.Printf("Failed executing CLI command: %s, exiting...\n", err)
+
+	if err := executor.ExecuteContext(ctx); err != nil {
+		fmt.Printf("failed execution: %s, exiting...\n", err)
 		os.Exit(1)
 	}
 }
@@ -85,7 +84,10 @@ func queryCmd() *cobra.Command {
 		Short:                      "Querying subcommands",
 		DisableFlagParsing:         true,
 		SuggestionsMinimumDistance: 2,
-		RunE:                       client.ValidateCmd,
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
+			return client.SetCmdClientContextHandler(initClientCtx, cmd)
+		},
+		RunE: client.ValidateCmd,
 	}
 
 	queryCmd.AddCommand(
@@ -109,11 +111,14 @@ func txCmd() *cobra.Command {
 		Short:                      "Transactions subcommands",
 		DisableFlagParsing:         true,
 		SuggestionsMinimumDistance: 2,
-		RunE:                       client.ValidateCmd,
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
+			return client.SetCmdClientContextHandler(initClientCtx, cmd)
+		},
+		RunE: client.ValidateCmd,
 	}
 
 	txCmd.AddCommand(
-		bankcmd.NewSendTxCmd(initClientCtx),
+		bankcmd.NewSendTxCmd(),
 		flags.LineBreak,
 		authcmd.GetSignCommand(initClientCtx),
 		authcmd.GetSignBatchCommand(encodingConfig.Amino),

types/module/module.go

@@ -105,7 +105,10 @@ func (bm BasicManager) RegisterRESTRoutes(clientCtx client.Context, rtr *mux.Rou
 	}
 }
 
-// AddTxCommands adds all tx commands to the rootTxCmd
+// AddTxCommands adds all tx commands to the rootTxCmd.
+//
+// TODO: Remove clientCtx argument.
+// REF: https://github.com/cosmos/cosmos-sdk/issues/6571
 func (bm BasicManager) AddTxCommands(rootTxCmd *cobra.Command, ctx client.Context) {
 	for _, b := range bm {
 		if cmd := b.GetTxCmd(ctx); cmd != nil {
@@ -114,7 +117,10 @@ func (bm BasicManager) AddTxCommands(rootTxCmd *cobra.Command, ctx client.Contex
 	}
 }
 
-// AddQueryCommands adds all query commands to the rootQueryCmd
+// AddQueryCommands adds all query commands to the rootQueryCmd.
+//
+// TODO: Remove clientCtx argument.
+// REF: https://github.com/cosmos/cosmos-sdk/issues/6571
 func (bm BasicManager) AddQueryCommands(rootQueryCmd *cobra.Command, clientCtx client.Context) {
 	for _, b := range bm {
 		if cmd := b.GetQueryCmd(clientCtx); cmd != nil {

x/bank/client/cli/cli_test.go

@@ -2,16 +2,19 @@ package cli_test
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
 
+	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/testutil"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/bank/client/cli"
+	banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
 )
 
 type IntegrationTestSuite struct {
@@ -44,6 +47,9 @@ func (s *IntegrationTestSuite) TestGetBalancesCmd() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx.WithOutput(buf)
 
+	ctx := context.Background()
+	ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
+
 	testCases := []struct {
 		name      string
 		args      []string
@@ -96,7 +102,7 @@ func (s *IntegrationTestSuite) TestGetBalancesCmd() {
 			cmd.SetOut(buf)
 			cmd.SetArgs(tc.args)
 
-			err := cmd.Execute()
+			err := cmd.ExecuteContext(ctx)
 			if tc.expectErr {
 				s.Require().Error(err)
 			} else {
@@ -113,6 +119,9 @@ func (s *IntegrationTestSuite) TestGetCmdQueryTotalSupply() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx.WithOutput(buf)
 
+	ctx := context.Background()
+	ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
+
 	testCases := []struct {
 		name      string
 		args      []string
@@ -163,7 +172,7 @@ func (s *IntegrationTestSuite) TestGetCmdQueryTotalSupply() {
 			cmd.SetOut(buf)
 			cmd.SetArgs(tc.args)
 
-			err := cmd.Execute()
+			err := cmd.ExecuteContext(ctx)
 			if tc.expectErr {
 				s.Require().Error(err)
 			} else {
@@ -180,8 +189,13 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx.WithOutput(buf)
 
+	ctx := context.Background()
+	ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
+
 	testCases := []struct {
 		name         string
+		from, to     sdk.AccAddress
+		amount       sdk.Coins
 		args         []string
 		expectErr    bool
 		respType     fmt.Stringer
@@ -189,13 +203,13 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
 	}{
 		{
 			"valid transaction (gen-only)",
+			val.Address,
+			val.Address,
+			sdk.NewCoins(
+				sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
+				sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
+			),
 			[]string{
-				val.Address.String(),
-				val.Address.String(),
-				sdk.NewCoins(
-					sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
-					sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
-				).String(),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
@@ -207,13 +221,13 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
 		},
 		{
 			"valid transaction",
+			val.Address,
+			val.Address,
+			sdk.NewCoins(
+				sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
+				sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
+			),
 			[]string{
-				val.Address.String(),
-				val.Address.String(),
-				sdk.NewCoins(
-					sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
-					sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
-				).String(),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
@@ -224,13 +238,13 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
 		},
 		{
 			"not enough fees",
+			val.Address,
+			val.Address,
+			sdk.NewCoins(
+				sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
+				sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
+			),
 			[]string{
-				val.Address.String(),
-				val.Address.String(),
-				sdk.NewCoins(
-					sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
-					sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
-				).String(),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(1))).String()),
@@ -241,13 +255,13 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
 		},
 		{
 			"not enough gas",
+			val.Address,
+			val.Address,
+			sdk.NewCoins(
+				sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
+				sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
+			),
 			[]string{
-				val.Address.String(),
-				val.Address.String(),
-				sdk.NewCoins(
-					sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)),
-					sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)),
-				).String(),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
@@ -265,17 +279,17 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
 		s.Run(tc.name, func() {
 			buf.Reset()
 
-			cmd := cli.NewSendTxCmd(clientCtx)
+			cmd := cli.NewSendTxCmd()
 			cmd.SetErr(buf)
 			cmd.SetOut(buf)
 			cmd.SetArgs(tc.args)
 
-			err := cmd.Execute()
+			out, err := banktestutil.MsgSendExec(clientCtx, tc.from, tc.to, tc.amount, tc.args...)
 			if tc.expectErr {
 				s.Require().Error(err)
 			} else {
 				s.Require().NoError(err)
-				s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(buf.Bytes(), tc.respType), buf.String())
+				s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out, tc.respType), string(out))
 
 				txResp := tc.respType.(*sdk.TxResponse)
 				s.Require().Equal(tc.expectedCode, txResp.Code)

x/bank/client/cli/tx.go

@@ -11,7 +11,7 @@ import (
 )
 
 // NewTxCmd returns a root CLI command handler for all x/bank transaction commands.
-func NewTxCmd(clientCtx client.Context) *cobra.Command {
+func NewTxCmd() *cobra.Command {
 	txCmd := &cobra.Command{
 		Use:                        types.ModuleName,
 		Short:                      "Bank transaction subcommands",
@@ -20,19 +20,21 @@ func NewTxCmd(clientCtx client.Context) *cobra.Command {
 		RunE:                       client.ValidateCmd,
 	}
 
-	txCmd.AddCommand(NewSendTxCmd(clientCtx))
+	txCmd.AddCommand(NewSendTxCmd())
 
 	return txCmd
 }
 
 // NewSendTxCmd returns a CLI command handler for creating a MsgSend transaction.
-func NewSendTxCmd(clientCtx client.Context) *cobra.Command {
+func NewSendTxCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "send [from_key_or_address] [to_address] [amount]",
 		Short: "Create and/or sign and broadcast a MsgSend transaction",
 		Args:  cobra.ExactArgs(3),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			cmd.Flags().Set(flags.FlagFrom, args[0])
+
+			clientCtx := client.GetClientContextFromCmd(cmd)
 			clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags())
 			if err != nil {
 				return err

x/bank/client/testutil/cli_helpers.go

@@ -1,19 +1,49 @@
 package testutil
 
 import (
+	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/cosmos/cosmos-sdk/client"
 	clientkeys "github.com/cosmos/cosmos-sdk/client/keys"
 	"github.com/cosmos/cosmos-sdk/tests"
 	"github.com/cosmos/cosmos-sdk/tests/cli"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/cli"
 )
 
-// TODO: REMOVE OR COMPLETELY REFACTOR THIS FILE.
+func MsgSendExec(clientCtx client.Context, from, to, amount fmt.Stringer, extraArgs ...string) ([]byte, error) {
+	buf := new(bytes.Buffer)
+	clientCtx = clientCtx.WithOutput(buf)
+
+	ctx := context.Background()
+	ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
+
+	args := []string{from.String(), to.String(), amount.String()}
+	args = append(args, extraArgs...)
+
+	cmd := bankcli.NewSendTxCmd()
+	cmd.SetErr(buf)
+	cmd.SetOut(buf)
+	cmd.SetArgs(args)
+
+	if err := cmd.ExecuteContext(ctx); err != nil {
+		return nil, err
+	}
+
+	return buf.Bytes(), nil
+}
+
+// ----------------------------------------------------------------------------
+// TODO: REMOVE ALL FUNCTIONS BELOW.
+//
+// REF: https://github.com/cosmos/cosmos-sdk/issues/6571
+// ----------------------------------------------------------------------------
 
 // TxSend is simcli tx send
 func TxSend(f *cli.Fixtures, from string, to sdk.AccAddress, amount sdk.Coin, flags ...string) (bool, string, string) {

x/bank/module.go

@@ -64,8 +64,8 @@ func (AppModuleBasic) RegisterRESTRoutes(clientCtx client.Context, rtr *mux.Rout
 }
 
 // GetTxCmd returns the root tx command for the bank module.
-func (AppModuleBasic) GetTxCmd(clientCtx client.Context) *cobra.Command {
+func (AppModuleBasic) GetTxCmd(_ client.Context) *cobra.Command {
-	return cli.NewTxCmd(clientCtx)
+	return cli.NewTxCmd()
 }
 
 // GetQueryCmd returns no root query command for the bank module.