diff --git a/CHANGELOG.md b/CHANGELOG.md index 90673485c90..7294176ad9c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/client/tx/tx.go b/client/tx/tx.go index db787a774c5..1d3fcb3af01 100644 --- a/client/tx/tx.go +++ b/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 } diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index 8c3ed0ddf07..a05c9310bd4 100644 --- a/client/tx/tx_test.go +++ b/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 { diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index b7e527271bb..9fbb3b31d7f 100644 --- a/x/auth/client/testutil/suite.go +++ b/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.