x/auth: turn sign --validate-sigantures into a standalone command

--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/

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.

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:

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
 }

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),

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
+}

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)
 

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

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) }

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()
+}

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
 }