From 9f2d1c280235a44acb6468c5de1bd34779b04744 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 22 May 2019 11:04:23 -0400 Subject: [PATCH 1/3] Use TxSigLimit instead of default --- .pending/bugfixes/sdk/4394-Fix-signature-c | 2 ++ x/auth/ante.go | 22 ++++++++++++++++++++++ x/auth/stdtx.go | 10 ---------- 3 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 .pending/bugfixes/sdk/4394-Fix-signature-c diff --git a/.pending/bugfixes/sdk/4394-Fix-signature-c b/.pending/bugfixes/sdk/4394-Fix-signature-c new file mode 100644 index 000000000000..9beebf1fb8ae --- /dev/null +++ b/.pending/bugfixes/sdk/4394-Fix-signature-c @@ -0,0 +1,2 @@ +#4394 Fix signature count check to use the TxSigLimit param instead of +a default. diff --git a/x/auth/ante.go b/x/auth/ante.go index 57b566d04f7d..6c43d8676ef5 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -86,6 +86,10 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si } }() + if res := ValidateSigCount(stdTx, params); !res.IsOK() { + return + } + if err := tx.ValidateBasic(); err != nil { return newCtx, err.Result(), true } @@ -154,6 +158,24 @@ func GetSignerAcc(ctx sdk.Context, ak AccountKeeper, addr sdk.AccAddress) (Accou return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)).Result() } +// ValidateSigCount validates that the transaction has a valid cumulative total +// amount of signatures. +func ValidateSigCount(stdTx StdTx, params Params) sdk.Result { + stdSigs := stdTx.GetSignatures() + + sigCount := 0 + for i := 0; i < len(stdSigs); i++ { + sigCount += countSubKeys(stdSigs[i].PubKey) + if uint64(sigCount) > params.TxSigLimit { + return sdk.ErrTooManySignatures( + fmt.Sprintf("signatures: %d, limit: %d", sigCount, params.TxSigLimit), + ).Result() + } + } + + return sdk.Result{} +} + // ValidateMemo validates the memo size. func ValidateMemo(stdTx StdTx, params Params) sdk.Result { memoLength := len(stdTx.GetMemo()) diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index 20efaae424f2..2233752621c3 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -56,16 +56,6 @@ func (tx StdTx) ValidateBasic() sdk.Error { return sdk.ErrUnauthorized("wrong number of signers") } - sigCount := 0 - for i := 0; i < len(stdSigs); i++ { - sigCount += countSubKeys(stdSigs[i].PubKey) - if uint64(sigCount) > DefaultTxSigLimit { - return sdk.ErrTooManySignatures( - fmt.Sprintf("signatures: %d, limit: %d", sigCount, DefaultTxSigLimit), - ) - } - } - return nil } From 556c6e028fe839b30dea4eb5a559fe9a94903647 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 22 May 2019 11:06:04 -0400 Subject: [PATCH 2/3] Fix return --- x/auth/ante.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index 6c43d8676ef5..32042bbd0b24 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -87,7 +87,7 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si }() if res := ValidateSigCount(stdTx, params); !res.IsOK() { - return + return newCtx, res, true } if err := tx.ValidateBasic(); err != nil { From 8740d20061d7ab7807189cccbff2544cbf782b28 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 22 May 2019 11:13:17 -0400 Subject: [PATCH 3/3] Remove TestTxValidateBasic --- x/auth/stdtx_test.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/x/auth/stdtx_test.go b/x/auth/stdtx_test.go index a1fbf4a34ba4..1139407ad293 100644 --- a/x/auth/stdtx_test.go +++ b/x/auth/stdtx_test.go @@ -63,12 +63,6 @@ func TestTxValidateBasic(t *testing.T) { // keys and addresses priv1, _, addr1 := keyPubAddr() priv2, _, addr2 := keyPubAddr() - priv3, _, addr3 := keyPubAddr() - priv4, _, addr4 := keyPubAddr() - priv5, _, addr5 := keyPubAddr() - priv6, _, addr6 := keyPubAddr() - priv7, _, addr7 := keyPubAddr() - priv8, _, addr8 := keyPubAddr() // msg and signatures msg1 := newTestMsg(addr1, addr2) @@ -101,17 +95,6 @@ func TestTxValidateBasic(t *testing.T) { require.Error(t, err) require.Equal(t, sdk.CodeUnauthorized, err.Result().Code) - // require to fail validation when there are too many signatures - privs = []crypto.PrivKey{priv1, priv2, priv3, priv4, priv5, priv6, priv7, priv8} - accNums, seqs = []uint64{0, 0, 0, 0, 0, 0, 0, 0}, []uint64{0, 0, 0, 0, 0, 0, 0, 0} - badMsg := newTestMsg(addr1, addr2, addr3, addr4, addr5, addr6, addr7, addr8) - badMsgs := []sdk.Msg{badMsg} - tx = newTestTx(ctx, badMsgs, privs, accNums, seqs, fee) - - err = tx.ValidateBasic() - require.Error(t, err) - require.Equal(t, sdk.CodeTooManySignatures, err.Result().Code) - // require to fail with invalid gas supplied badFee = newStdFee() badFee.Gas = 9223372036854775808