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 <alexanderbez@users.noreply.github.com>
This commit is contained in:
Alessio Treglia
2020-05-04 13:55:16 +00:00
committed by GitHub
co-authored by Alexander Bezobchuk
parent 54ecf7d629
commit 2414e5bdd4
20 changed files with 352 additions and 181 deletions
+68
View File
@@ -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()
}
+1
View File
@@ -20,6 +20,7 @@ func GetTxCmd(cdc *codec.Codec) *cobra.Command {
txCmd.AddCommand(
GetMultiSignCommand(cdc),
GetSignCommand(cdc),
GetValidateSignaturesCommand(cdc),
)
return txCmd
}
+1 -111
View File
@@ -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
}
+149
View File
@@ -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
}