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

x/bank: Refactor CLI & Tests #6525

Merged
merged 27 commits into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
24edb2a
init
alexanderbez Jun 26, 2020
876bcaa
remove old tests
alexanderbez Jun 26, 2020
98b9226
bank/cli: TestGetBalancesCmd
alexanderbez Jun 26, 2020
26eb2e4
default to 1 val
alexanderbez Jun 26, 2020
963e413
add query tests
alexanderbez Jun 26, 2020
a0e5ecd
cli: remove indent and use ReadGetCommandFlags
alexanderbez Jun 27, 2020
6c1f9b4
Merge branch 'master' into bez/6423-x-bank-cli-refactor
alexanderbez Jun 29, 2020
2f9ae75
Updates
alexanderbez Jun 29, 2020
ec870e0
test updates
alexanderbez Jun 29, 2020
8b7a624
Updates
alexanderbez Jun 29, 2020
cfeaf94
Add back missing line
alexanderbez Jun 29, 2020
079019d
more test cases
alexanderbez Jun 29, 2020
e76f949
Merge branch 'master' into bez/6423-x-bank-cli-refactor
alexanderbez Jun 29, 2020
aa4f188
Merge branch 'master' into bez/6423-x-bank-cli-refactor
alexanderbez Jun 30, 2020
ba98321
tests: attempt fix
alexanderbez Jun 30, 2020
dcc5fa7
lint++
alexanderbez Jun 30, 2020
c25fed3
tests: attempt fix
alexanderbez Jun 30, 2020
512b6bc
Merge branch 'master' into bez/6423-x-bank-cli-refactor
alexanderbez Jun 30, 2020
a73f357
cl++
alexanderbez Jun 30, 2020
7dbac20
cl++
alexanderbez Jun 30, 2020
d261237
tests: attempt fix
alexanderbez Jun 30, 2020
8c81821
tests: attempt fix
alexanderbez Jun 30, 2020
be638d3
undo changes
alexanderbez Jun 30, 2020
532b6c4
Merge branch 'master' into bez/6423-x-bank-cli-refactor
alexanderbez Jun 30, 2020
66cb961
fix test
alexanderbez Jun 30, 2020
40b6ec7
Merge branch 'bez/6423-x-bank-cli-refactor' of github.com:cosmos/cosm…
alexanderbez Jun 30, 2020
6ea2632
Merge branch 'master' into bez/6423-x-bank-cli-refactor
alexanderbez Jun 30, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
95 changes: 95 additions & 0 deletions client/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -55,3 +58,95 @@ 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.FlagChainID) {
chainID, _ := flagSet.GetString(flags.FlagChainID)
clientCtx = clientCtx.WithChainID(chainID)
}

if flagSet.Changed(flags.FlagTrustNode) {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
trustNode, _ := flagSet.GetBool(flags.FlagTrustNode)
clientCtx = clientCtx.WithTrustNode(trustNode)
}

if flagSet.Changed(flags.FlagKeyringBackend) {
keyringBackend, _ := flagSet.GetString(flags.FlagKeyringBackend)

kr, err := newKeyringFromFlags(clientCtx, keyringBackend)
if err != nil {
return clientCtx, err
}

clientCtx = clientCtx.WithKeyring(kr)
}

if flagSet.Changed(flags.FlagNode) {
rpcURI, _ := flagSet.GetString(flags.FlagNode)
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)
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
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
}
55 changes: 27 additions & 28 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
45 changes: 0 additions & 45 deletions client/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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))
Expand All @@ -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)")
Expand All @@ -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))
Expand All @@ -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
Expand Down
Loading