From 2414e5bdd45fbec9b0c832e47568039a9ea5eec8 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Mon, 4 May 2020 14:55:16 +0100 Subject: [PATCH] x/auth: turn sign --validate-sigantures into a standalone command (#6108) --validate-signatures should not be a flag of the sign command as the operation performed (transaction signatures verification) is logically distinct. cli_test is and has always been an horrible name for package directory as it's very much Go anti-idiomatic - _test is the suffix used by test packages, not directories. Plus, CLI test cases can and should live alongside other testcases that don't require binaries to be built beforehand. Thus: x/module/client/cli_test/*.go -> x/module/client/cli/ Test files that require sim{cli,d} shall be tagged with // +build cli_test With regard to cli test auxiliary functions, they should live in: x/module/client/testutil/ Co-authored-by: Alexander Bezobchuk --- CHANGELOG.md | 1 + Makefile | 8 +- client/flags/flags.go | 2 + simapp/cmd/simcli/main.go | 1 + tests/cli/helpers.go | 10 +- tests/cli/simd_test.go | 20 ++- tests/io.go | 14 ++ tests/{knownValues.go => known_values.go} | 0 tests/util.go | 8 +- x/auth/client/cli/cli_test.go | 68 ++++++++ x/auth/client/cli/tx.go | 1 + x/auth/client/cli/tx_sign.go | 112 +------------ x/auth/client/cli/validate_sigs.go | 149 ++++++++++++++++++ x/auth/client/testutil/helpers.go | 31 ++++ .../send_test.go => cli/cli_test.go} | 60 +++---- .../client/{cli_test => testutil}/helpers.go | 2 +- .../distr_test.go => cli/cli_test.go} | 16 +- .../client/{cli_test => testutil}/helpers.go | 2 +- .../staking_test.go => cli/cli_test.go} | 26 +-- .../client/{cli_test => testutil}/helpers.go | 2 +- 20 files changed, 352 insertions(+), 181 deletions(-) rename tests/{knownValues.go => known_values.go} (100%) create mode 100644 x/auth/client/cli/cli_test.go create mode 100644 x/auth/client/cli/validate_sigs.go create mode 100644 x/auth/client/testutil/helpers.go rename x/bank/client/{cli_test/send_test.go => cli/cli_test.go} (67%) rename x/bank/client/{cli_test => testutil}/helpers.go (99%) rename x/distribution/client/{cli_test/distr_test.go => cli/cli_test.go} (80%) rename x/distribution/client/{cli_test => testutil}/helpers.go (98%) rename x/staking/client/{cli_test/staking_test.go => cli/cli_test.go} (63%) rename x/staking/client/{cli_test => testutil}/helpers.go (99%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2f45b63a5..6605072889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ that parse log messages. * (client) [\#5799](https://github.com/cosmos/cosmos-sdk/pull/5799) The `tx encode/decode` commands, due to change on encoding break compatibility with older clients. * (x/auth) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) `tx sign` command now returns an error when signing is attempted with offline/multisig keys. +* (x/auth) [\#6108](https://github.com/cosmos/cosmos-sdk/pull/6108) `tx sign` command's `--validate-signatures` flag is migrated into a `tx validate-signatures` standalone command. * (client/keys) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Remove `keys update` command. * (x/evidence) [\#5952](https://github.com/cosmos/cosmos-sdk/pull/5952) Remove CLI and REST handlers for querying `x/evidence` parameters. * (server) [\#5982](https://github.com/cosmos/cosmos-sdk/pull/5982) `--pruning` now must be set to `custom` if you want to customise the granular options. diff --git a/Makefile b/Makefile index dae876965a..bc1a2daf4a 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ PACKAGES_NOSIMULATION=$(shell go list ./... | grep -v '/simulation') PACKAGES_SIMTEST=$(shell go list ./... | grep '/simulation') -VERSION := $(shell echo $(shell git describe --tags) | sed 's/^v//') +VERSION := $(shell echo $(shell git describe --tags --always) | sed 's/^v//') COMMIT := $(shell git log -1 --format='%H') LEDGER_ENABLED ?= true BINDIR ?= $(GOPATH)/bin @@ -124,8 +124,7 @@ test-race: @VERSION=$(VERSION) go test -mod=readonly -race $(PACKAGES_NOSIMULATION) test-integration: build-sim - BUILDDIR=$(BUILDDIR) go test -mod=readonly -p 4 `go list ./tests/cli/...` -tags=-tags='ledger test_ledger_mock cli_test' - BUILDDIR=$(BUILDDIR) go test -mod=readonly -p 4 `go list ./x/.../client/cli_test/...` -tags=-tags='ledger test_ledger_mock cli_test' + BUILDDIR=$(BUILDDIR) go test -mod=readonly -p 4 -tags=-tags='ledger test_ledger_mock cli_test' -run ^TestCLI `go list ./.../cli/...` .PHONY: test test-all test-ledger-mock test-ledger test-unit test-race @@ -207,9 +206,8 @@ benchmark: ############################################################################### lint: - golangci-lint run + golangci-lint run --out-format=tab --issues-exit-code=0 find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" | xargs gofmt -d -s - go mod verify .PHONY: lint format: diff --git a/client/flags/flags.go b/client/flags/flags.go index cf67c64ee5..9544e4baf3 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -97,6 +97,7 @@ func GetCommands(cmds ...*cobra.Command) []*cobra.Command { c.MarkFlagRequired(FlagChainID) c.SetErr(c.ErrOrStderr()) + c.SetOut(c.OutOrStdout()) } return cmds } @@ -135,6 +136,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { c.MarkFlagRequired(FlagChainID) c.SetErr(c.ErrOrStderr()) + c.SetOut(c.OutOrStdout()) } return cmds } diff --git a/simapp/cmd/simcli/main.go b/simapp/cmd/simcli/main.go index fb61c810d7..a4086c8f2f 100644 --- a/simapp/cmd/simcli/main.go +++ b/simapp/cmd/simcli/main.go @@ -124,6 +124,7 @@ func txCmd(cdc *amino.Codec) *cobra.Command { flags.LineBreak, authcmd.GetSignCommand(cdc), authcmd.GetMultiSignCommand(cdc), + authcmd.GetValidateSignaturesCommand(cdc), flags.LineBreak, authcmd.GetBroadcastCommand(cdc), authcmd.GetEncodeCommand(cdc), diff --git a/tests/cli/helpers.go b/tests/cli/helpers.go index a67d33fa53..0350a6836f 100644 --- a/tests/cli/helpers.go +++ b/tests/cli/helpers.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "strings" - "testing" "github.com/stretchr/testify/require" @@ -182,7 +181,14 @@ func AddFlags(cmd string, flags []string) string { return strings.TrimSpace(cmd) } -func UnmarshalStdTx(t *testing.T, c *codec.Codec, s string) (stdTx auth.StdTx) { +func UnmarshalStdTx(t require.TestingT, c *codec.Codec, s string) (stdTx auth.StdTx) { require.Nil(t, c.UnmarshalJSON([]byte(s), &stdTx)) return } + +func MarshalStdTx(t require.TestingT, c *codec.Codec, stdTx auth.StdTx) []byte { + bz, err := c.MarshalBinaryBare(stdTx) + require.NoError(t, err) + + return bz +} diff --git a/tests/cli/simd_test.go b/tests/cli/simd_test.go index b0860fdf0d..a17e848c26 100644 --- a/tests/cli/simd_test.go +++ b/tests/cli/simd_test.go @@ -1,20 +1,24 @@ +// +build cli_test + package cli_test import ( "fmt" + "io/ioutil" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + tmtypes "github.com/tendermint/tendermint/types" + "github.com/cosmos/cosmos-sdk/std" "github.com/cosmos/cosmos-sdk/tests/cli" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/bank" - "github.com/stretchr/testify/require" - tmtypes "github.com/tendermint/tendermint/types" - "io/ioutil" - "path/filepath" - "testing" ) -func TestSimdCollectGentxs(t *testing.T) { +func TestCLISimdCollectGentxs(t *testing.T) { t.Parallel() var customMaxBytes, customMaxGas int64 = 99999999, 1234567 f := cli.NewFixtures(t) @@ -62,7 +66,7 @@ func TestSimdCollectGentxs(t *testing.T) { f.Cleanup(gentxDir) } -func TestSimdAddGenesisAccount(t *testing.T) { +func TestCLISimdAddGenesisAccount(t *testing.T) { t.Parallel() f := cli.NewFixtures(t) @@ -113,7 +117,7 @@ func TestSimdAddGenesisAccount(t *testing.T) { f.Cleanup() } -func TestValidateGenesis(t *testing.T) { +func TestCLIValidateGenesis(t *testing.T) { t.Parallel() f := cli.InitFixtures(t) diff --git a/tests/io.go b/tests/io.go index b1aa30634a..a513985719 100644 --- a/tests/io.go +++ b/tests/io.go @@ -2,9 +2,12 @@ package tests import ( "bytes" + "io/ioutil" + "os" "strings" "github.com/spf13/cobra" + "github.com/stretchr/testify/require" ) // ApplyMockIO replaces stdin/out/err with buffers that can be used during testing. @@ -18,4 +21,15 @@ func ApplyMockIO(c *cobra.Command) (*strings.Reader, *bytes.Buffer, *bytes.Buffe return mockIn, mockOut, mockErr } +// Write the given string to a new temporary file +func WriteToNewTempFile(t require.TestingT, s string) (*os.File, func()) { + fp, err := ioutil.TempFile(os.TempDir(), "cosmos_cli_test_") + require.Nil(t, err) + + _, err = fp.WriteString(s) + require.Nil(t, err) + + return fp, func() { os.Remove(fp.Name()) } +} + // DONTCOVER diff --git a/tests/knownValues.go b/tests/known_values.go similarity index 100% rename from tests/knownValues.go rename to tests/known_values.go diff --git a/tests/util.go b/tests/util.go index 2bc7ee4358..4435d77d41 100644 --- a/tests/util.go +++ b/tests/util.go @@ -6,7 +6,6 @@ import ( "net/http" "os" "strings" - "testing" "time" "github.com/stretchr/testify/require" @@ -213,10 +212,15 @@ func ExtractPortFromAddress(listenAddress string) string { return stringList[2] } +type NamedTestingT interface { + require.TestingT + Name() string +} + // NewTestCaseDir creates a new temporary directory for a test case. // Returns the directory path and a cleanup function. // nolint: errcheck -func NewTestCaseDir(t *testing.T) (string, func()) { +func NewTestCaseDir(t NamedTestingT) (string, func()) { dir, err := ioutil.TempDir("", t.Name()+"_") require.NoError(t, err) return dir, func() { os.RemoveAll(dir) } diff --git a/x/auth/client/cli/cli_test.go b/x/auth/client/cli/cli_test.go new file mode 100644 index 0000000000..e1d33a0a37 --- /dev/null +++ b/x/auth/client/cli/cli_test.go @@ -0,0 +1,68 @@ +// +build cli_test + +package cli_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/tests" + "github.com/cosmos/cosmos-sdk/tests/cli" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/client/testutil" +) + +func TestCLIValidateSignatures(t *testing.T) { + t.Parallel() + f := cli.InitFixtures(t) + + // start simd server + proc := f.SDStart() + t.Cleanup(func() { proc.Stop(false) }) + + f.ValidateGenesis() + + fooAddr := f.KeyAddress(cli.KeyFoo) + barAddr := f.KeyAddress(cli.KeyBar) + + // generate sendTx with default gas + success, stdout, stderr := testutil.TxSend(f, fooAddr.String(), barAddr, sdk.NewInt64Coin("stake", 10), "--generate-only") + require.True(t, success) + require.Empty(t, stderr) + + // write unsigned tx to file + unsignedTxFile, cleanup := tests.WriteToNewTempFile(t, stdout) + t.Cleanup(cleanup) + + // validate we can successfully sign + success, stdout, _ = testutil.TxSign(f, cli.KeyFoo, unsignedTxFile.Name()) + require.True(t, success) + + stdTx := cli.UnmarshalStdTx(t, f.Cdc, stdout) + + require.Equal(t, len(stdTx.Msgs), 1) + require.Equal(t, 1, len(stdTx.GetSignatures())) + require.Equal(t, fooAddr.String(), stdTx.GetSigners()[0].String()) + + // write signed tx to file + signedTxFile, cleanup := tests.WriteToNewTempFile(t, stdout) + t.Cleanup(cleanup) + + // validate signatures + success, _, _ = testutil.TxValidateSignatures(f, signedTxFile.Name()) + require.True(t, success) + + // modify the transaction + stdTx.Memo = "MODIFIED-ORIGINAL-TX-BAD" + bz := cli.MarshalStdTx(t, f.Cdc, stdTx) + modSignedTxFile, cleanup := tests.WriteToNewTempFile(t, string(bz)) + t.Cleanup(cleanup) + + // validate signature validation failure due to different transaction sig bytes + success, _, _ = testutil.TxValidateSignatures(f, modSignedTxFile.Name()) + require.False(t, success) + + // Cleanup testing directories + f.Cleanup() +} diff --git a/x/auth/client/cli/tx.go b/x/auth/client/cli/tx.go index 7ee246c38e..742ab35471 100644 --- a/x/auth/client/cli/tx.go +++ b/x/auth/client/cli/tx.go @@ -20,6 +20,7 @@ func GetTxCmd(cdc *codec.Codec) *cobra.Command { txCmd.AddCommand( GetMultiSignCommand(cdc), GetSignCommand(cdc), + GetValidateSignaturesCommand(cdc), ) return txCmd } diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 62100b0233..cb49bdeb73 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -1,16 +1,12 @@ package cli import ( - "bufio" "fmt" "os" - "strings" "github.com/spf13/cobra" "github.com/spf13/viper" - "github.com/tendermint/tendermint/crypto/multisig" - "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -37,12 +33,6 @@ It will read a transaction from [file], sign it, and print its JSON encoding. If the flag --signature-only flag is set, it will output a JSON representation of the generated signature only. -If the flag --validate-signatures is set, then the command would check whether all required -signers have signed the transactions, whether the signatures were collected in the right -order, and if the signature is valid over the given transaction. If the --offline -flag is also set, signature validation over the transaction will be not be -performed as that will require RPC communication with a full node. - The --offline flag makes sure that the client will not reach out to full node. As a result, the account and sequence number queries will not be performed and it is required to set such parameters manually. Note, invalid values will cause @@ -65,10 +55,6 @@ be generated via the 'multisign' command. flagAppend, true, "Append the signature to the existing ones. If disabled, old signatures would be overwritten. Ignored if --multisig is on", ) - cmd.Flags().Bool( - flagValidateSigs, false, - "Print the addresses that must sign the transaction, those who have already signed it, and make sure that signatures are in the correct order", - ) cmd.Flags().Bool(flagSigOnly, false, "Print only the generated signature, then exit") cmd.Flags().String(flagOutfile, "", "The document will be written to the given file instead of STDOUT") cmd = flags.PostCommands(cmd)[0] @@ -88,23 +74,11 @@ func preSignCmd(cmd *cobra.Command, _ []string) { func makeSignCmd(cdc *codec.Codec) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - stdTx, err := client.ReadStdTxFromFile(cdc, args[0]) + cliCtx, txBldr, stdTx, err := readStdTxAndInitContexts(cdc, cmd, args[0]) if err != nil { return err } - inBuf := bufio.NewReader(cmd.InOrStdin()) - cliCtx := context.NewCLIContextWithInput(inBuf).WithCodec(cdc) - txBldr := types.NewTxBuilderFromCLI(inBuf) - - if viper.GetBool(flagValidateSigs) { - if !printAndValidateSigs(cliCtx, txBldr.ChainID(), stdTx, cliCtx.Offline) { - return fmt.Errorf("signatures validation failed") - } - - return nil - } - // if --signature-only is on, then override --append var newTx types.StdTx generateSignatureOnly := viper.GetBool(flagSigOnly) @@ -174,87 +148,3 @@ func getSignatureJSON(cdc *codec.Codec, newTx types.StdTx, indent, generateSigna } } } - -// printAndValidateSigs will validate the signatures of a given transaction over -// its expected signers. In addition, if offline has not been supplied, the -// signature is verified over the transaction sign bytes. -func printAndValidateSigs( - cliCtx context.CLIContext, chainID string, stdTx types.StdTx, offline bool, -) bool { - - fmt.Println("Signers:") - - signers := stdTx.GetSigners() - for i, signer := range signers { - fmt.Printf(" %v: %v\n", i, signer.String()) - } - - success := true - sigs := stdTx.Signatures - - fmt.Println("") - fmt.Println("Signatures:") - - if len(sigs) != len(signers) { - success = false - } - - for i, sig := range sigs { - sigAddr := sdk.AccAddress(sig.GetPubKey().Address()) - sigSanity := "OK" - - var ( - multiSigHeader string - multiSigMsg string - ) - - if i >= len(signers) || !sigAddr.Equals(signers[i]) { - sigSanity = "ERROR: signature does not match its respective signer" - success = false - } - - // Validate the actual signature over the transaction bytes since we can - // reach out to a full node to query accounts. - if !offline && success { - acc, err := types.NewAccountRetriever(client.Codec, cliCtx).GetAccount(sigAddr) - if err != nil { - fmt.Printf("failed to get account: %s\n", sigAddr) - return false - } - - sigBytes := types.StdSignBytes( - chainID, acc.GetAccountNumber(), acc.GetSequence(), - stdTx.Fee, stdTx.GetMsgs(), stdTx.GetMemo(), - ) - - if ok := sig.GetPubKey().VerifyBytes(sigBytes, sig.Signature); !ok { - sigSanity = "ERROR: signature invalid" - success = false - } - } - - multiPK, ok := sig.GetPubKey().(multisig.PubKeyMultisigThreshold) - if ok { - var multiSig multisig.Multisignature - cliCtx.Codec.MustUnmarshalBinaryBare(sig.Signature, &multiSig) - - var b strings.Builder - b.WriteString("\n MultiSig Signatures:\n") - - for i := 0; i < multiSig.BitArray.Size(); i++ { - if multiSig.BitArray.GetIndex(i) { - addr := sdk.AccAddress(multiPK.PubKeys[i].Address().Bytes()) - b.WriteString(fmt.Sprintf(" %d: %s (weight: %d)\n", i, addr, 1)) - } - } - - multiSigHeader = fmt.Sprintf(" [multisig threshold: %d/%d]", multiPK.K, len(multiPK.PubKeys)) - multiSigMsg = b.String() - } - - fmt.Printf(" %d: %s\t\t\t[%s]%s%s\n", i, sigAddr.String(), sigSanity, multiSigHeader, multiSigMsg) - } - - fmt.Println("") - return success -} diff --git a/x/auth/client/cli/validate_sigs.go b/x/auth/client/cli/validate_sigs.go new file mode 100644 index 0000000000..a7b87d3878 --- /dev/null +++ b/x/auth/client/cli/validate_sigs.go @@ -0,0 +1,149 @@ +package cli + +import ( + "bufio" + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/tendermint/tendermint/crypto/multisig" + + "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/client" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func GetValidateSignaturesCommand(codec *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "validate-signatures [file]", + Short: "Validate transactions signatures", + Long: `Print the addresses that must sign the transaction, those who have already +signed it, and make sure that signatures are in the correct order. + +The command would check whether all required signers have signed the transactions, whether +the signatures were collected in the right order, and if the signature is valid over the +given transaction. If the --offline flag is also set, signature validation over the +transaction will be not be performed as that will require RPC communication with a full node. +`, + PreRun: preSignCmd, + RunE: makeValidateSignaturesCmd(codec), + Args: cobra.ExactArgs(1), + } + + return flags.PostCommands(cmd)[0] +} + +func makeValidateSignaturesCmd(cdc *codec.Codec) func(cmd *cobra.Command, args []string) error { + return func(cmd *cobra.Command, args []string) error { + cliCtx, txBldr, stdTx, err := readStdTxAndInitContexts(cdc, cmd, args[0]) + if err != nil { + return err + } + + if !printAndValidateSigs(cmd, cliCtx, txBldr.ChainID(), stdTx, cliCtx.Offline) { + return fmt.Errorf("signatures validation failed") + } + + return nil + } +} + +// printAndValidateSigs will validate the signatures of a given transaction over its +// expected signers. In addition, if offline has not been supplied, the signature is +// verified over the transaction sign bytes. Returns false if the validation fails. +func printAndValidateSigs( + cmd *cobra.Command, cliCtx context.CLIContext, chainID string, stdTx types.StdTx, offline bool, +) bool { + cmd.Println("Signers:") + signers := stdTx.GetSigners() + + for i, signer := range signers { + cmd.Printf(" %v: %v\n", i, signer.String()) + } + + success := true + sigs := stdTx.Signatures + cmd.Println("") + cmd.Println("Signatures:") + + if len(sigs) != len(signers) { + success = false + } + + for i, sig := range sigs { + var ( + multiSigHeader string + multiSigMsg string + sigAddr = sdk.AccAddress(sig.GetPubKey().Address()) + sigSanity = "OK" + ) + + if i >= len(signers) || !sigAddr.Equals(signers[i]) { + sigSanity = "ERROR: signature does not match its respective signer" + success = false + } + + // Validate the actual signature over the transaction bytes since we can + // reach out to a full node to query accounts. + if !offline && success { + acc, err := types.NewAccountRetriever(client.Codec, cliCtx).GetAccount(sigAddr) + if err != nil { + cmd.Printf("failed to get account: %s\n", sigAddr) + return false + } + + sigBytes := types.StdSignBytes( + chainID, acc.GetAccountNumber(), acc.GetSequence(), + stdTx.Fee, stdTx.GetMsgs(), stdTx.GetMemo(), + ) + + if ok := sig.GetPubKey().VerifyBytes(sigBytes, sig.Signature); !ok { + sigSanity = "ERROR: signature invalid" + success = false + } + } + + multiPK, ok := sig.GetPubKey().(multisig.PubKeyMultisigThreshold) + if ok { + var multiSig multisig.Multisignature + cliCtx.Codec.MustUnmarshalBinaryBare(sig.Signature, &multiSig) + + var b strings.Builder + b.WriteString("\n MultiSig Signatures:\n") + + for i := 0; i < multiSig.BitArray.Size(); i++ { + if multiSig.BitArray.GetIndex(i) { + addr := sdk.AccAddress(multiPK.PubKeys[i].Address().Bytes()) + b.WriteString(fmt.Sprintf(" %d: %s (weight: %d)\n", i, addr, 1)) + } + } + + multiSigHeader = fmt.Sprintf(" [multisig threshold: %d/%d]", multiPK.K, len(multiPK.PubKeys)) + multiSigMsg = b.String() + } + + cmd.Printf(" %d: %s\t\t\t[%s]%s%s\n", i, sigAddr.String(), sigSanity, multiSigHeader, multiSigMsg) + } + + cmd.Println("") + + return success +} + +func readStdTxAndInitContexts(cdc *codec.Codec, cmd *cobra.Command, filename string) ( + context.CLIContext, types.TxBuilder, types.StdTx, error, +) { + stdTx, err := client.ReadStdTxFromFile(cdc, filename) + if err != nil { + return context.CLIContext{}, types.TxBuilder{}, types.StdTx{}, err + } + + inBuf := bufio.NewReader(cmd.InOrStdin()) + cliCtx := context.NewCLIContextWithInput(inBuf).WithCodec(cdc) + txBldr := types.NewTxBuilderFromCLI(inBuf) + + return cliCtx, txBldr, stdTx, nil +} diff --git a/x/auth/client/testutil/helpers.go b/x/auth/client/testutil/helpers.go new file mode 100644 index 0000000000..c1e3edf55a --- /dev/null +++ b/x/auth/client/testutil/helpers.go @@ -0,0 +1,31 @@ +package testutil + +import ( + "fmt" + + clientkeys "github.com/cosmos/cosmos-sdk/client/keys" + "github.com/cosmos/cosmos-sdk/tests/cli" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// TxSign is simcli sign +func TxSign(f *cli.Fixtures, signer, fileName string, flags ...string) (bool, string, string) { + cmd := fmt.Sprintf("%s tx sign %v --keyring-backend=test --from=%s %v", f.SimcliBinary, f.Flags(), signer, fileName) + + return cli.ExecuteWriteRetStdStreams(f.T, cli.AddFlags(cmd, flags), clientkeys.DefaultKeyPass) +} + +// TxSend is simcli tx send +func TxSend(f *cli.Fixtures, from string, to sdk.AccAddress, amount sdk.Coin, flags ...string) (bool, string, string) { + cmd := fmt.Sprintf("%s tx send --keyring-backend=test %s %s %s %v", f.SimcliBinary, from, to, amount, f.Flags()) + + return cli.ExecuteWriteRetStdStreams(f.T, cli.AddFlags(cmd, flags), clientkeys.DefaultKeyPass) +} + +// TxValidateSignatures is simcli tx validate-signatures +func TxValidateSignatures(f *cli.Fixtures, fileName string, flags ...string) (bool, string, string) { + cmd := fmt.Sprintf("%s tx validate-signatures %v --keyring-backend=test %v", f.SimcliBinary, + f.Flags(), fileName) + + return cli.ExecuteWriteRetStdStreams(f.T, cli.AddFlags(cmd, flags), clientkeys.DefaultKeyPass) +} diff --git a/x/bank/client/cli_test/send_test.go b/x/bank/client/cli/cli_test.go similarity index 67% rename from x/bank/client/cli_test/send_test.go rename to x/bank/client/cli/cli_test.go index 153f6245d8..cc100e8a9e 100644 --- a/x/bank/client/cli_test/send_test.go +++ b/x/bank/client/cli/cli_test.go @@ -11,7 +11,7 @@ import ( "github.com/cosmos/cosmos-sdk/tests" "github.com/cosmos/cosmos-sdk/tests/cli" sdk "github.com/cosmos/cosmos-sdk/types" - bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/cli_test" + "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" ) func TestCLISend(t *testing.T) { @@ -20,37 +20,37 @@ func TestCLISend(t *testing.T) { // start simd server proc := f.SDStart() - defer proc.Stop(false) + t.Cleanup(func() { proc.Stop(false) }) // Save key addresses for later uspackage testse fooAddr := f.KeyAddress(cli.KeyFoo) barAddr := f.KeyAddress(cli.KeyBar) startTokens := sdk.TokensFromConsensusPower(50) - require.Equal(t, startTokens, bankcli.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) + require.Equal(t, startTokens, testutil.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) sendTokens := sdk.TokensFromConsensusPower(10) // It does not allow to send in offline mode - success, _, stdErr := bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y", "--offline") + success, _, stdErr := testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y", "--offline") require.Contains(t, stdErr, "no RPC client is defined in offline mode") require.False(f.T, success) tests.WaitForNextNBlocksTM(1, f.Port) // Send some tokens from one account to the other - bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y") + testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y") tests.WaitForNextNBlocksTM(1, f.Port) // Ensure account balances match expected - require.Equal(t, sendTokens, bankcli.QueryBalances(f, barAddr).AmountOf(cli.Denom)) - require.Equal(t, startTokens.Sub(sendTokens), bankcli.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) + require.Equal(t, sendTokens, testutil.QueryBalances(f, barAddr).AmountOf(cli.Denom)) + require.Equal(t, startTokens.Sub(sendTokens), testutil.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) // Test --dry-run - success, _, _ = bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "--dry-run") + success, _, _ = testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "--dry-run") require.True(t, success) // Test --generate-only - success, stdout, stderr := bankcli.TxSend( + success, stdout, stderr := testutil.TxSend( f, fooAddr.String(), barAddr, sdk.NewCoin(cli.Denom, sendTokens), "--generate-only=true", ) require.Empty(t, stderr) @@ -62,23 +62,23 @@ func TestCLISend(t *testing.T) { require.Len(t, msg.GetSignatures(), 0) // Check state didn't change - require.Equal(t, startTokens.Sub(sendTokens), bankcli.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) + require.Equal(t, startTokens.Sub(sendTokens), testutil.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) // test autosequencing - bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y") + testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y") tests.WaitForNextNBlocksTM(1, f.Port) // Ensure account balances match expected - require.Equal(t, sendTokens.MulRaw(2), bankcli.QueryBalances(f, barAddr).AmountOf(cli.Denom)) - require.Equal(t, startTokens.Sub(sendTokens.MulRaw(2)), bankcli.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) + require.Equal(t, sendTokens.MulRaw(2), testutil.QueryBalances(f, barAddr).AmountOf(cli.Denom)) + require.Equal(t, startTokens.Sub(sendTokens.MulRaw(2)), testutil.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) // test memo - bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "--memo='testmemo'", "-y") + testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "--memo='testmemo'", "-y") tests.WaitForNextNBlocksTM(1, f.Port) // Ensure account balances match expected - require.Equal(t, sendTokens.MulRaw(3), bankcli.QueryBalances(f, barAddr).AmountOf(cli.Denom)) - require.Equal(t, startTokens.Sub(sendTokens.MulRaw(3)), bankcli.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) + require.Equal(t, sendTokens.MulRaw(3), testutil.QueryBalances(f, barAddr).AmountOf(cli.Denom)) + require.Equal(t, startTokens.Sub(sendTokens.MulRaw(3)), testutil.QueryBalances(f, fooAddr).AmountOf(cli.Denom)) f.Cleanup() } @@ -95,25 +95,25 @@ func TestCLIMinimumFees(t *testing.T) { sdk.NewDecCoinFromDec(cli.Fee2Denom, minGasPrice), ) proc := f.SDStart(fees) - defer proc.Stop(false) + t.Cleanup(func() { proc.Stop(false) }) barAddr := f.KeyAddress(cli.KeyBar) // Send a transaction that will get rejected - success, stdOut, _ := bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.Fee2Denom, 10), "-y") + success, stdOut, _ := testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.Fee2Denom, 10), "-y") require.Contains(t, stdOut, "insufficient fees") require.True(f.T, success) tests.WaitForNextNBlocksTM(1, f.Port) // Ensure tx w/ correct fees pass txFees := fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(cli.FeeDenom, 2)) - success, _, _ = bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.Fee2Denom, 10), txFees, "-y") + success, _, _ = testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.Fee2Denom, 10), txFees, "-y") require.True(f.T, success) tests.WaitForNextNBlocksTM(1, f.Port) // Ensure tx w/ improper fees fails txFees = fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(cli.FeeDenom, 1)) - success, _, _ = bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.FooDenom, 10), txFees, "-y") + success, _, _ = testutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.FooDenom, 10), txFees, "-y") require.Contains(t, stdOut, "insufficient fees") require.True(f.T, success) @@ -128,13 +128,13 @@ func TestCLIGasPrices(t *testing.T) { // start simd server with minimum fees minGasPrice, _ := sdk.NewDecFromStr("0.000006") proc := f.SDStart(fmt.Sprintf("--minimum-gas-prices=%s", sdk.NewDecCoinFromDec(cli.FeeDenom, minGasPrice))) - defer proc.Stop(false) + t.Cleanup(func() { proc.Stop(false) }) barAddr := f.KeyAddress(cli.KeyBar) // insufficient gas prices (tx fails) badGasPrice, _ := sdk.NewDecFromStr("0.000003") - success, stdOut, _ := bankcli.TxSend( + success, stdOut, _ := testutil.TxSend( f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.FooDenom, 50), fmt.Sprintf("--gas-prices=%s", sdk.NewDecCoinFromDec(cli.FeeDenom, badGasPrice)), "-y") require.Contains(t, stdOut, "insufficient fees") @@ -144,7 +144,7 @@ func TestCLIGasPrices(t *testing.T) { tests.WaitForNextNBlocksTM(1, f.Port) // sufficient gas prices (tx passes) - success, _, _ = bankcli.TxSend( + success, _, _ = testutil.TxSend( f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.FooDenom, 50), fmt.Sprintf("--gas-prices=%s", sdk.NewDecCoinFromDec(cli.FeeDenom, minGasPrice)), "-y") require.True(t, success) @@ -162,16 +162,16 @@ func TestCLIFeesDeduction(t *testing.T) { // start simd server with minimum fees minGasPrice, _ := sdk.NewDecFromStr("0.000006") proc := f.SDStart(fmt.Sprintf("--minimum-gas-prices=%s", sdk.NewDecCoinFromDec(cli.FeeDenom, minGasPrice))) - defer proc.Stop(false) + t.Cleanup(func() { proc.Stop(false) }) // Save key addresses for later use fooAddr := f.KeyAddress(cli.KeyFoo) barAddr := f.KeyAddress(cli.KeyBar) - fooAmt := bankcli.QueryBalances(f, fooAddr).AmountOf(cli.FooDenom) + fooAmt := testutil.QueryBalances(f, fooAddr).AmountOf(cli.FooDenom) // test simulation - success, _, _ := bankcli.TxSend( + success, _, _ := testutil.TxSend( f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.FooDenom, 1000), fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(cli.FeeDenom, 2)), "--dry-run") require.True(t, success) @@ -180,11 +180,11 @@ func TestCLIFeesDeduction(t *testing.T) { tests.WaitForNextNBlocksTM(1, f.Port) // ensure state didn't change - require.Equal(t, fooAmt.Int64(), bankcli.QueryBalances(f, fooAddr).AmountOf(cli.FooDenom).Int64()) + require.Equal(t, fooAmt.Int64(), testutil.QueryBalances(f, fooAddr).AmountOf(cli.FooDenom).Int64()) // insufficient funds (coins + fees) tx fails largeCoins := sdk.TokensFromConsensusPower(10000000) - success, stdOut, _ := bankcli.TxSend( + success, stdOut, _ := testutil.TxSend( f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.FooDenom, largeCoins), fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(cli.FeeDenom, 2)), "-y") require.Contains(t, stdOut, "insufficient funds") @@ -194,10 +194,10 @@ func TestCLIFeesDeduction(t *testing.T) { tests.WaitForNextNBlocksTM(1, f.Port) // ensure state didn't change - require.Equal(t, fooAmt.Int64(), bankcli.QueryBalances(f, fooAddr).AmountOf(cli.FooDenom).Int64()) + require.Equal(t, fooAmt.Int64(), testutil.QueryBalances(f, fooAddr).AmountOf(cli.FooDenom).Int64()) // test success (transfer = coins + fees) - success, _, _ = bankcli.TxSend( + success, _, _ = testutil.TxSend( f, cli.KeyFoo, barAddr, sdk.NewInt64Coin(cli.FooDenom, 500), fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(cli.FeeDenom, 2)), "-y") require.True(t, success) diff --git a/x/bank/client/cli_test/helpers.go b/x/bank/client/testutil/helpers.go similarity index 99% rename from x/bank/client/cli_test/helpers.go rename to x/bank/client/testutil/helpers.go index 5722e5abed..49c4611f66 100644 --- a/x/bank/client/cli_test/helpers.go +++ b/x/bank/client/testutil/helpers.go @@ -1,4 +1,4 @@ -package cli +package testutil import ( "encoding/json" diff --git a/x/distribution/client/cli_test/distr_test.go b/x/distribution/client/cli/cli_test.go similarity index 80% rename from x/distribution/client/cli_test/distr_test.go rename to x/distribution/client/cli/cli_test.go index e3dbc4b516..d2c63acd69 100644 --- a/x/distribution/client/cli_test/distr_test.go +++ b/x/distribution/client/cli/cli_test.go @@ -1,19 +1,21 @@ +// +build cli_test + package cli_test import ( - "github.com/cosmos/cosmos-sdk/tests/cli" "path/filepath" "testing" "github.com/stretchr/testify/require" tmtypes "github.com/tendermint/tendermint/types" + "github.com/cosmos/cosmos-sdk/tests/cli" sdk "github.com/cosmos/cosmos-sdk/types" - distrcli "github.com/cosmos/cosmos-sdk/x/distribution/client/cli_test" + "github.com/cosmos/cosmos-sdk/x/distribution/client/testutil" "github.com/cosmos/cosmos-sdk/x/mint" ) -func TestCliWithdrawRewards(t *testing.T) { +func TestCLIWithdrawRewards(t *testing.T) { t.Parallel() f := cli.InitFixtures(t) @@ -36,18 +38,18 @@ func TestCliWithdrawRewards(t *testing.T) { // start simd server proc := f.SDStart() - defer proc.Stop(false) + t.Cleanup(func() { proc.Stop(false) }) fooAddr := f.KeyAddress(cli.KeyFoo) - rewards := distrcli.QueryRewards(f, fooAddr) + rewards := testutil.QueryRewards(f, fooAddr) require.Equal(t, 1, len(rewards.Rewards)) require.NotNil(t, rewards.Total) fooVal := sdk.ValAddress(fooAddr) - success := distrcli.TxWithdrawRewards(f, fooVal, fooAddr.String(), "-y") + success := testutil.TxWithdrawRewards(f, fooVal, fooAddr.String(), "-y") require.True(t, success) - rewards = distrcli.QueryRewards(f, fooAddr) + rewards = testutil.QueryRewards(f, fooAddr) require.Equal(t, 1, len(rewards.Rewards)) require.Nil(t, rewards.Total) diff --git a/x/distribution/client/cli_test/helpers.go b/x/distribution/client/testutil/helpers.go similarity index 98% rename from x/distribution/client/cli_test/helpers.go rename to x/distribution/client/testutil/helpers.go index 5322165f4c..975d31674a 100644 --- a/x/distribution/client/cli_test/helpers.go +++ b/x/distribution/client/testutil/helpers.go @@ -1,4 +1,4 @@ -package cli +package testutil import ( "fmt" diff --git a/x/staking/client/cli_test/staking_test.go b/x/staking/client/cli/cli_test.go similarity index 63% rename from x/staking/client/cli_test/staking_test.go rename to x/staking/client/cli/cli_test.go index 380d6011e6..599974c5a7 100644 --- a/x/staking/client/cli_test/staking_test.go +++ b/x/staking/client/cli/cli_test.go @@ -11,8 +11,8 @@ import ( "github.com/cosmos/cosmos-sdk/tests" "github.com/cosmos/cosmos-sdk/tests/cli" sdk "github.com/cosmos/cosmos-sdk/types" - bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/cli_test" - stakingcli "github.com/cosmos/cosmos-sdk/x/staking/client/cli_test" + bankclienttestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" + "github.com/cosmos/cosmos-sdk/x/staking/client/testutil" ) func TestCLICreateValidator(t *testing.T) { @@ -29,13 +29,13 @@ func TestCLICreateValidator(t *testing.T) { consPubKey := sdk.MustBech32ifyPubKey(sdk.Bech32PubKeyTypeConsPub, ed25519.GenPrivKey().PubKey()) sendTokens := sdk.TokensFromConsensusPower(10) - bankcli.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y") + bankclienttestutil.TxSend(f, cli.KeyFoo, barAddr, sdk.NewCoin(cli.Denom, sendTokens), "-y") tests.WaitForNextNBlocksTM(1, f.Port) - require.Equal(t, sendTokens, bankcli.QueryBalances(f, barAddr).AmountOf(cli.Denom)) + require.Equal(t, sendTokens, bankclienttestutil.QueryBalances(f, barAddr).AmountOf(cli.Denom)) //Generate a create validator transaction and ensure correctness - success, stdout, stderr := stakingcli.TxStakingCreateValidator(f, barAddr.String(), consPubKey, sdk.NewInt64Coin(cli.Denom, 2), "--generate-only") + success, stdout, stderr := testutil.TxStakingCreateValidator(f, barAddr.String(), consPubKey, sdk.NewInt64Coin(cli.Denom, 2), "--generate-only") require.True(f.T, success) require.Empty(f.T, stderr) @@ -46,39 +46,39 @@ func TestCLICreateValidator(t *testing.T) { // Test --dry-run newValTokens := sdk.TokensFromConsensusPower(2) - success, _, _ = stakingcli.TxStakingCreateValidator(f, barAddr.String(), consPubKey, sdk.NewCoin(cli.Denom, newValTokens), "--dry-run") + success, _, _ = testutil.TxStakingCreateValidator(f, barAddr.String(), consPubKey, sdk.NewCoin(cli.Denom, newValTokens), "--dry-run") require.True(t, success) // Create the validator - stakingcli.TxStakingCreateValidator(f, cli.KeyBar, consPubKey, sdk.NewCoin(cli.Denom, newValTokens), "-y") + testutil.TxStakingCreateValidator(f, cli.KeyBar, consPubKey, sdk.NewCoin(cli.Denom, newValTokens), "-y") tests.WaitForNextNBlocksTM(1, f.Port) // Ensure funds were deducted properly - require.Equal(t, sendTokens.Sub(newValTokens), bankcli.QueryBalances(f, barAddr).AmountOf(cli.Denom)) + require.Equal(t, sendTokens.Sub(newValTokens), bankclienttestutil.QueryBalances(f, barAddr).AmountOf(cli.Denom)) // Ensure that validator state is as expected - validator := stakingcli.QueryStakingValidator(f, barVal) + validator := testutil.QueryStakingValidator(f, barVal) require.Equal(t, validator.OperatorAddress, barVal) require.True(sdk.IntEq(t, newValTokens, validator.Tokens)) // Query delegations to the validator - validatorDelegations := stakingcli.QueryStakingDelegationsTo(f, barVal) + validatorDelegations := testutil.QueryStakingDelegationsTo(f, barVal) require.Len(t, validatorDelegations, 1) require.NotZero(t, validatorDelegations[0].Shares) // unbond a single share unbondAmt := sdk.NewCoin(sdk.DefaultBondDenom, sdk.TokensFromConsensusPower(1)) - success = stakingcli.TxStakingUnbond(f, cli.KeyBar, unbondAmt.String(), barVal, "-y") + success = testutil.TxStakingUnbond(f, cli.KeyBar, unbondAmt.String(), barVal, "-y") require.True(t, success) tests.WaitForNextNBlocksTM(1, f.Port) // Ensure bonded staking is correct remainingTokens := newValTokens.Sub(unbondAmt.Amount) - validator = stakingcli.QueryStakingValidator(f, barVal) + validator = testutil.QueryStakingValidator(f, barVal) require.Equal(t, remainingTokens, validator.Tokens) // Get unbonding delegations from the validator - validatorUbds := stakingcli.QueryStakingUnbondingDelegationsFrom(f, barVal) + validatorUbds := testutil.QueryStakingUnbondingDelegationsFrom(f, barVal) require.Len(t, validatorUbds, 1) require.Len(t, validatorUbds[0].Entries, 1) require.Equal(t, remainingTokens.String(), validatorUbds[0].Entries[0].Balance.String()) diff --git a/x/staking/client/cli_test/helpers.go b/x/staking/client/testutil/helpers.go similarity index 99% rename from x/staking/client/cli_test/helpers.go rename to x/staking/client/testutil/helpers.go index 63e28d51fb..b9694d2e93 100644 --- a/x/staking/client/cli_test/helpers.go +++ b/x/staking/client/testutil/helpers.go @@ -1,4 +1,4 @@ -package cli +package testutil import ( "fmt"