Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use TxSigLimit instead of default constant value #4396

Merged
merged 3 commits into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .pending/bugfixes/sdk/4394-Fix-signature-c
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#4394 Fix signature count check to use the TxSigLimit param instead of
a default.
22 changes: 22 additions & 0 deletions x/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si
}
}()

if res := ValidateSigCount(stdTx, params); !res.IsOK() {
return newCtx, res, true
}

if err := tx.ValidateBasic(); err != nil {
return newCtx, err.Result(), true
}
Expand Down Expand Up @@ -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())
Expand Down
10 changes: 0 additions & 10 deletions x/auth/stdtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
17 changes: 0 additions & 17 deletions x/auth/stdtx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down