x/bank: Refactor CLI & Tests

CHANGELOG.md

@@ -63,6 +63,7 @@ older clients.
 
 ### API Breaking Changes
 
+* (client) [\#6525](https://github.com/cosmos/cosmos-sdk/pull/6525) Removed support for `indent` in JSON responses. Clients should consider piping to an external tool such as `jq`.
 * (x/staking) [\#6451](https://github.com/cosmos/cosmos-sdk/pull/6451) `DefaultParamspace` and `ParamKeyTable` in staking module are moved from keeper to types to enforce consistency.
 * [\#6409](https://github.com/cosmos/cosmos-sdk/pull/6409) Rename all IsEmpty methods to Empty across the codebase and enforce consistency.
 * [\#6231](https://github.com/cosmos/cosmos-sdk/pull/6231) Simplify `AppModule` interface, `Route` and `NewHandler` methods become only `Route`

client/cmd.go

@@ -6,6 +6,9 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+
+	"github.com/cosmos/cosmos-sdk/client/flags"
 )
 
 // ValidateCmd returns unknown command error or Help display if help flag set
@@ -55,3 +58,101 @@ func ValidateCmd(cmd *cobra.Command, args []string) error {
 
 	return cmd.Help()
 }
+
+// ReadPersistentCommandFlags returns a Context with fields set for "persistent"
+// flags that do not necessarily change with context. These must be checked if
+// the caller explicitly changed the values.
+func ReadPersistentCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
+	if flagSet.Changed(flags.FlagTrustNode) {
+		trustNode, err := flagSet.GetBool(flags.FlagTrustNode)
+		if err != nil {
+			return clientCtx, err
+		}
+
+		clientCtx = clientCtx.WithTrustNode(trustNode)
+	}
+
+	if flagSet.Changed(flags.FlagKeyringBackend) {
+		keyringBackend, err := flagSet.GetString(flags.FlagKeyringBackend)
+		if err != nil {
+			return clientCtx, err
+		}
+
+		kr, err := newKeyringFromFlags(clientCtx, keyringBackend)
+		if err != nil {
+			return clientCtx, err
+		}
+
+		clientCtx = clientCtx.WithKeyring(kr)
+	}
+
+	if flagSet.Changed(flags.FlagNode) {
+		rpcURI, err := flagSet.GetString(flags.FlagNode)
+		if err != nil {
+			return clientCtx, err
+		}
+
+		clientCtx = clientCtx.WithNodeURI(rpcURI)
+	}
+
+	return clientCtx, nil
+}
+
+// ReadQueryCommandFlags returns an updated Context with fields set based on flags
+// defined in GetCommands. An error is returned if any flag query fails.
+//
+// Certain flags are naturally command and context dependent, so for these flags
+// we do not check if they've been explicitly set by the caller. Other flags can
+// be considered "persistent" (e.g. KeyBase or Client) and these should be checked
+// if the caller explicitly set those.
+func ReadQueryCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
+	height, _ := flagSet.GetInt64(flags.FlagHeight)
+	clientCtx = clientCtx.WithHeight(height)
+
+	useLedger, _ := flagSet.GetBool(flags.FlagUseLedger)
+	clientCtx = clientCtx.WithUseLedger(useLedger)
+
+	return ReadPersistentCommandFlags(clientCtx, flagSet)
+}
+
+// ReadTxCommandFlags returns an updated Context with fields set based on flags
+// defined in PostCommands. An error is returned if any flag query fails.
+//
+// Certain flags are naturally command and context dependent, so for these flags
+// we do not check if they've been explicitly set by the caller. Other flags can
+// be considered "persistent" (e.g. KeyBase or Client) and these should be checked
+// if the caller explicitly set those.
+func ReadTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
+	clientCtx, err := ReadPersistentCommandFlags(clientCtx, flagSet)
+	if err != nil {
+		return clientCtx, err
+	}
+
+	genOnly, _ := flagSet.GetBool(flags.FlagGenerateOnly)
+	clientCtx = clientCtx.WithGenerateOnly(genOnly)
+
+	dryRun, _ := flagSet.GetBool(flags.FlagDryRun)
+	clientCtx = clientCtx.WithSimulation(dryRun)
+
+	offline, _ := flagSet.GetBool(flags.FlagOffline)
+	clientCtx = clientCtx.WithOffline(offline)
+
+	useLedger, _ := flagSet.GetBool(flags.FlagUseLedger)
+	clientCtx = clientCtx.WithUseLedger(useLedger)
+
+	bMode, _ := flagSet.GetString(flags.FlagBroadcastMode)
+	clientCtx = clientCtx.WithBroadcastMode(bMode)
+
+	skipConfirm, _ := flagSet.GetBool(flags.FlagSkipConfirmation)
+	clientCtx = clientCtx.WithSkipConfirmation(skipConfirm)
+
+	from, _ := flagSet.GetString(flags.FlagFrom)
+	fromAddr, fromName, err := GetFromFields(clientCtx.Keyring, from, clientCtx.GenerateOnly)
+	if err != nil {
+		return clientCtx, err
+	}
+
+	clientCtx = clientCtx.WithFrom(from).WithFromAddress(fromAddr).WithFromName(fromName)
+
+	return clientCtx, nil
+}

client/context.go

@@ -43,21 +43,18 @@ type Context struct {
 	Simulate         bool
 	GenerateOnly     bool
 	Offline          bool
-	Indent           bool
 	SkipConfirm      bool
 	TxGenerator      TxGenerator
 	AccountRetriever AccountRetriever
-
-	// TODO: API and CLI interfaces are migrating to a single binary (i.e be part of
-	// the same process of the application). We need to groom through these fields
-	// and remove any that no longer make sense.
-	NodeURI  string
-	Verifier tmlite.Verifier
+	NodeURI          string
+	Verifier         tmlite.Verifier
 
 	// TODO: Deprecated (remove).
 	Codec *codec.Codec
 }
 
+// TODO: Remove all New* and Init* methods.
+
 // NewContextWithInputAndFrom returns a new initialized Context with parameters from the
 // command line using Viper. It takes a io.Reader and and key name or address and populates
 // the FromName and  FromAddress field accordingly. It will also create Tendermint verifier
@@ -127,33 +124,29 @@ func (ctx Context) InitWithInputAndFrom(input io.Reader, from string) Context {
 	ctx.BroadcastMode = viper.GetString(flags.FlagBroadcastMode)
 	ctx.Simulate = viper.GetBool(flags.FlagDryRun)
 	ctx.Offline = offline
-	ctx.Indent = viper.GetBool(flags.FlagIndentResponse)
 	ctx.SkipConfirm = viper.GetBool(flags.FlagSkipConfirmation)
+	ctx.HomeDir = viper.GetString(flags.FlagHome)
+	ctx.GenerateOnly = viper.GetBool(flags.FlagGenerateOnly)
 
-	homedir := viper.GetString(flags.FlagHome)
-	genOnly := viper.GetBool(flags.FlagGenerateOnly)
 	backend := viper.GetString(flags.FlagKeyringBackend)
 	if len(backend) == 0 {
 		backend = keyring.BackendMemory
 	}
 
-	kr, err := newKeyringFromFlags(backend, homedir, input, genOnly)
+	kr, err := newKeyringFromFlags(ctx, backend)
 	if err != nil {
 		panic(fmt.Errorf("couldn't acquire keyring: %v", err))
 	}
 
-	fromAddress, fromName, err := GetFromFields(kr, from, genOnly)
+	fromAddress, fromName, err := GetFromFields(kr, from, ctx.GenerateOnly)
 	if err != nil {
 		fmt.Printf("failed to get from fields: %v\n", err)
 		os.Exit(1)
 	}
 
-	ctx.HomeDir = homedir
-
 	ctx.Keyring = kr
 	ctx.FromAddress = fromAddress
 	ctx.FromName = fromName
-	ctx.GenerateOnly = genOnly
 
 	if offline {
 		return ctx
@@ -290,6 +283,12 @@ func (ctx Context) WithSimulation(simulate bool) Context {
 	return ctx
 }
 
+// WithOffline returns a copy of the context with updated Offline value.
+func (ctx Context) WithOffline(offline bool) Context {
+	ctx.Offline = offline
+	return ctx
+}
+
 // WithFromName returns a copy of the context with an updated from account name.
 func (ctx Context) WithFromName(name string) Context {
 	ctx.FromName = name
@@ -310,6 +309,13 @@ func (ctx Context) WithBroadcastMode(mode string) Context {
 	return ctx
 }
 
+// WithSkipConfirmation returns a copy of the context with an updated SkipConfirm
+// value.
+func (ctx Context) WithSkipConfirmation(skip bool) Context {
+	ctx.SkipConfirm = skip
+	return ctx
+}
+
 // WithTxGenerator returns the context with an updated TxGenerator
 func (ctx Context) WithTxGenerator(generator TxGenerator) Context {
 	ctx.TxGenerator = generator
@@ -335,6 +341,7 @@ func (ctx Context) PrintOutput(toPrint interface{}) error {
 	if ctx.OutputFormat == "text" {
 		// handle text format by decoding and re-encoding JSON as YAML
 		var j interface{}
+
 		err = json.Unmarshal(out, &j)
 		if err != nil {
 			return err
@@ -344,18 +351,9 @@ func (ctx Context) PrintOutput(toPrint interface{}) error {
 		if err != nil {
 			return err
 		}
-	} else if ctx.Indent {
-		// To JSON indent, we re-encode the already encoded JSON given there is no
-		// error. The re-encoded JSON uses the standard library as the initial encoded
-		// JSON should have the correct output produced by ctx.JSONMarshaler.
-		out, err = codec.MarshalIndentFromJSON(out)
-		if err != nil {
-			return err
-		}
 	}
 
 	writer := ctx.Output
-	// default to stdout
 	if writer == nil {
 		writer = os.Stdout
 	}
@@ -409,9 +407,10 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres
 	return info.GetAddress(), info.GetName(), nil
 }
 
-func newKeyringFromFlags(backend, homedir string, input io.Reader, genOnly bool) (keyring.Keyring, error) {
-	if genOnly {
-		return keyring.New(sdk.KeyringServiceName(), keyring.BackendMemory, homedir, input)
+func newKeyringFromFlags(ctx Context, backend string) (keyring.Keyring, error) {
+	if ctx.GenerateOnly {
+		return keyring.New(sdk.KeyringServiceName(), keyring.BackendMemory, ctx.HomeDir, ctx.Input)
 	}
-	return keyring.New(sdk.KeyringServiceName(), backend, homedir, input)
+
+	return keyring.New(sdk.KeyringServiceName(), backend, ctx.HomeDir, ctx.Input)
 }

client/context_test.go

@@ -106,36 +106,16 @@ func TestContext_PrintOutput(t *testing.T) {
 	buf := &bytes.Buffer{}
 	ctx = ctx.WithOutput(buf)
 	ctx.OutputFormat = "json"
-	ctx.Indent = false
 	err = ctx.PrintOutput(hasAnimal)
 	require.NoError(t, err)
 	require.Equal(t,
 		`{"animal":{"@type":"/cosmos_sdk.codec.v1.Dog","size":"big","name":"Spot"},"x":"10"}
 `, string(buf.Bytes()))
 
-	// json indent
-	buf = &bytes.Buffer{}
-	ctx = ctx.WithOutput(buf)
-	ctx.OutputFormat = "json"
-	ctx.Indent = true
-	err = ctx.PrintOutput(hasAnimal)
-	require.NoError(t, err)
-	require.Equal(t,
-		`{
-  "animal": {
-    "@type": "/cosmos_sdk.codec.v1.Dog",
-    "name": "Spot",
-    "size": "big"
-  },
-  "x": "10"
-}
-`, string(buf.Bytes()))
-
 	// yaml
 	buf = &bytes.Buffer{}
 	ctx = ctx.WithOutput(buf)
 	ctx.OutputFormat = "text"
-	ctx.Indent = false
 	err = ctx.PrintOutput(hasAnimal)
 	require.NoError(t, err)
 	require.Equal(t,
@@ -156,41 +136,16 @@ x: "10"
 	buf = &bytes.Buffer{}
 	ctx = ctx.WithOutput(buf)
 	ctx.OutputFormat = "json"
-	ctx.Indent = false
 	err = ctx.PrintOutput(hasAnimal)
 	require.NoError(t, err)
 	require.Equal(t,
 		`{"type":"testdata/HasAnimal","value":{"animal":{"type":"testdata/Dog","value":{"size":"big","name":"Spot"}},"x":"10"}}
 `, string(buf.Bytes()))
 
-	// json indent
-	buf = &bytes.Buffer{}
-	ctx = ctx.WithOutput(buf)
-	ctx.OutputFormat = "json"
-	ctx.Indent = true
-	err = ctx.PrintOutput(hasAnimal)
-	require.NoError(t, err)
-	require.Equal(t,
-		`{
-  "type": "testdata/HasAnimal",
-  "value": {
-    "animal": {
-      "type": "testdata/Dog",
-      "value": {
-        "name": "Spot",
-        "size": "big"
-      }
-    },
-    "x": "10"
-  }
-}
-`, string(buf.Bytes()))
-
 	// yaml
 	buf = &bytes.Buffer{}
 	ctx = ctx.WithOutput(buf)
 	ctx.OutputFormat = "text"
-	ctx.Indent = false
 	err = ctx.PrintOutput(hasAnimal)
 	require.NoError(t, err)
 	require.Equal(t,

client/flags/flags.go

@@ -57,7 +57,6 @@ const (
 	FlagDryRun           = "dry-run"
 	FlagGenerateOnly     = "generate-only"
 	FlagOffline          = "offline"
-	FlagIndentResponse   = "indent"
 	FlagOutputDocument   = "output-document" // inspired by wget -O
 	FlagSkipConfirmation = "yes"
 	FlagProve            = "prove"
@@ -77,13 +76,13 @@ var (
 // GetCommands adds common flags to query commands
 func GetCommands(cmds ...*cobra.Command) []*cobra.Command {
 	for _, c := range cmds {
-		c.Flags().Bool(FlagIndentResponse, false, "Add indent to JSON response")
 		c.Flags().Bool(FlagTrustNode, false, "Trust connected full node (don't verify proofs for responses)")
 		c.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device")
 		c.Flags().String(FlagNode, "tcp://localhost:26657", "<host>:<port> to Tendermint RPC interface for this chain")
 		c.Flags().Int64(FlagHeight, 0, "Use a specific height to query state at (this can error if the node is pruning state)")
 		c.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")
 
+		// TODO: REMOVE VIPER CALLS!
 		viper.BindPFlag(FlagTrustNode, c.Flags().Lookup(FlagTrustNode))
 		viper.BindPFlag(FlagUseLedger, c.Flags().Lookup(FlagUseLedger))
 		viper.BindPFlag(FlagNode, c.Flags().Lookup(FlagNode))
@@ -100,7 +99,6 @@ func GetCommands(cmds ...*cobra.Command) []*cobra.Command {
 // PostCommands adds common flags for commands to post tx
 func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 	for _, c := range cmds {
-		c.Flags().Bool(FlagIndentResponse, false, "Add indent to JSON response")
 		c.Flags().String(FlagFrom, "", "Name or address of private key with which to sign")
 		c.Flags().Uint64P(FlagAccountNumber, "a", 0, "The account number of the signing account (offline mode only)")
 		c.Flags().Uint64P(FlagSequence, "s", 0, "The sequence number of the signing account (offline mode only)")
@@ -120,10 +118,15 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 		c.Flags().String(FlagSignMode, "", "Choose sign mode (direct|amino-json), this is an advanced feature")
 
 		// --gas can accept integers and "simulate"
+		//
+		// TODO: Remove usage of var in favor of string as this is technical creating
+		// a singleton usage pattern and can cause issues in parallel tests.
 		c.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(
 			"gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)",
 			GasFlagAuto, DefaultGasLimit,
 		))
+
+		// TODO: REMOVE VIPER CALLS!
 		viper.BindPFlag(FlagTrustNode, c.Flags().Lookup(FlagTrustNode))
 		viper.BindPFlag(FlagUseLedger, c.Flags().Lookup(FlagUseLedger))
 		viper.BindPFlag(FlagNode, c.Flags().Lookup(FlagNode))
@@ -137,8 +140,6 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 	return cmds
 }
 
-// Gas flag parsing functions
-
 // GasSetting encapsulates the possible values passed through the --gas flag.
 type GasSetting struct {
 	Simulate bool