fix(cli): Allow max 1 DIRECT signer per tx (#10683)

* fix(cli): Allow max 1 DIRECT signer per tx

* Use sdkerrors

* Update client/tx/tx_test.go

* add more test

* cl

* Fix test

* Update x/auth/client/testutil/suite.go

* Address nits

CHANGELOG.md

@@ -141,6 +141,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#10341](https://github.com/cosmos/cosmos-sdk/pull/10341) Move from `io/ioutil` to `io` and `os` packages.
 * [\#10468](https://github.com/cosmos/cosmos-sdk/pull/10468) Allow futureOps to queue additional operations in simulations
 * [\#10625](https://github.com/cosmos/cosmos-sdk/pull/10625) Add `--fee-payer` CLI flag
+* (cli) [\#10683](https://github.com/cosmos/cosmos-sdk/pull/10683) In CLI, allow 1 SIGN_MODE_DIRECT signer in transactions with multiple signers.
 
 ### Bug Fixes
 

client/tx/tx.go

@@ -170,11 +170,42 @@ func SignWithPrivKey(
 	return sigV2, nil
 }
 
-func checkMultipleSigners(mode signing.SignMode, tx authsigning.Tx) error {
-	if mode == signing.SignMode_SIGN_MODE_DIRECT &&
-		len(tx.GetSigners()) > 1 {
-		return sdkerrors.Wrap(sdkerrors.ErrNotSupported, "Signing in DIRECT mode is only supported for transactions with one signer only")
+// countDirectSigners counts the number of DIRECT signers in a signature data.
+func countDirectSigners(data signing.SignatureData) int {
+	switch data := data.(type) {
+	case *signing.SingleSignatureData:
+		if data.SignMode == signing.SignMode_SIGN_MODE_DIRECT {
+			return 1
+		}
+
+		return 0
+	case *signing.MultiSignatureData:
+		directSigners := 0
+		for _, d := range data.Signatures {
+			directSigners += countDirectSigners(d)
+		}
+
+		return directSigners
+	default:
+		panic("unreachable case")
 	}
+}
+
+// checkMultipleSigners checks that there can be maximum one DIRECT signer in
+// a tx.
+func checkMultipleSigners(tx authsigning.Tx) error {
+	directSigners := 0
+	sigsV2, err := tx.GetSignaturesV2()
+	if err != nil {
+		return err
+	}
+	for _, sig := range sigsV2 {
+		directSigners += countDirectSigners(sig.Data)
+		if directSigners > 1 {
+			return sdkerrors.ErrNotSupported.Wrap("txs signed with CLI can have maximum 1 DIRECT signer")
+		}
+	}
+
 	return nil
 }
 
@@ -194,9 +225,6 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig boo
 		// use the SignModeHandler's default mode if unspecified
 		signMode = txf.txConfig.SignModeHandler().DefaultMode()
 	}
-	if err := checkMultipleSigners(signMode, txBuilder.GetTx()); err != nil {
-		return err
-	}
 
 	k, err := txf.keybase.Key(name)
 	if err != nil {
@@ -246,6 +274,7 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig boo
 		Data:     &sigData,
 		Sequence: txf.Sequence(),
 	}
+
 	var prevSignatures []signing.SignatureV2
 	if !overwriteSig {
 		prevSignatures, err = txBuilder.GetTx().GetSignaturesV2()
@@ -253,7 +282,18 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig boo
 			return err
 		}
 	}
-	if err := txBuilder.SetSignatures(sig); err != nil {
+	// Overwrite or append signer infos.
+	var sigs []signing.SignatureV2
+	if overwriteSig {
+		sigs = []signing.SignatureV2{sig}
+	} else {
+		sigs = append(prevSignatures, sig)
+	}
+	if err := txBuilder.SetSignatures(sigs...); err != nil {
+		return err
+	}
+
+	if err := checkMultipleSigners(txBuilder.GetTx()); err != nil {
 		return err
 	}
 

client/tx/tx_test.go

@@ -210,13 +210,19 @@ func TestSign(t *testing.T) {
 			txfAmino, txb, from2, true, []cryptotypes.PubKey{pubKey2}, []int{1, 0}},
 
 		/**** test double sign Direct mode
-		  signing transaction with more than 2 signers should fail in DIRECT mode ****/
-		{"direct: should fail to append a signature with different mode",
-			txfDirect, txb, from1, false, []cryptotypes.PubKey{}, nil},
-		{"direct: should fail to sign multi-signers tx",
-			txfDirect, txb2, from1, false, []cryptotypes.PubKey{}, nil},
-		{"direct: should fail to overwrite multi-signers tx",
-			txfDirect, txb2, from1, true, []cryptotypes.PubKey{}, nil},
+		  signing transaction with 2 or more DIRECT signers should fail in DIRECT mode ****/
+		{"direct: should  append a DIRECT signature with existing AMINO",
+			// txb already has 1 AMINO signature
+			txfDirect, txb, from1, false, []cryptotypes.PubKey{pubKey2, pubKey1}, nil},
+		{"direct: should add single DIRECT sig in multi-signers tx",
+			txfDirect, txb2, from1, false, []cryptotypes.PubKey{pubKey1}, nil},
+		{"direct: should fail to append 2nd DIRECT sig in multi-signers tx",
+			txfDirect, txb2, from2, false, []cryptotypes.PubKey{}, nil},
+		{"amino: should append 2nd AMINO sig in multi-signers tx with 1 DIRECT sig",
+			// txb2 already has 1 DIRECT signature
+			txfAmino, txb2, from2, false, []cryptotypes.PubKey{}, nil},
+		{"direct: should overwrite multi-signers tx with DIRECT sig",
+			txfDirect, txb2, from1, true, []cryptotypes.PubKey{pubKey1}, nil},
 	}
 	var prevSigs []signingtypes.SignatureV2
 	for _, tc := range testCases {

x/auth/client/testutil/suite.go

@@ -33,6 +33,7 @@ import (
 	bank "github.com/cosmos/cosmos-sdk/x/bank/client/cli"
 	bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
 )
 
 type IntegrationTestSuite struct {
@@ -1364,7 +1365,7 @@ func (s *IntegrationTestSuite) TestTxWithoutPublicKey() {
 	unsignedTxFile := testutil.WriteToNewTempFile(s.T(), string(txJSON))
 
 	// Sign the file with the unsignedTx.
-	signedTx, err := TxSignExec(val1.ClientCtx, val1.Address, unsignedTxFile.Name())
+	signedTx, err := TxSignExec(val1.ClientCtx, val1.Address, unsignedTxFile.Name(), fmt.Sprintf("--%s=true", cli.FlagOverwrite))
 	s.Require().NoError(err)
 
 	// Remove the signerInfo's `public_key` field manually from the signedTx.