From b9b325ec991a095cecc719070b7c724ae7cc9134 Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 18:33:10 +0800 Subject: [PATCH] fix(x/distribution): add nil check in Params.ValidateBasic (#17236) --- CHANGELOG.md | 1 + .../distribution/keeper/msg_server_test.go | 18 ++++++++++++++++-- x/distribution/types/params.go | 8 +------- x/distribution/types/params_test.go | 1 + 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7981cdd014a7..57155b5bdf7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (baseapp) [#17159](https://github.com/cosmos/cosmos-sdk/pull/17159) Validators can propose blocks that exceed the gas limit. * (x/group) [#17146](https://github.com/cosmos/cosmos-sdk/pull/17146) Rename x/group legacy ORM package's error codespace from "orm" to "legacy_orm", preventing collisions with ORM's error codespace "orm". * (x/bank) [#17170](https://github.com/cosmos/cosmos-sdk/pull/17170) Avoid empty spendable error message on send coins. +* (x/distribution) [#17236](https://github.com/cosmos/cosmos-sdk/pull/17236) Using "validateCommunityTax" in "Params.ValidateBasic", preventing panic when field "CommunityTax" is nil. ### API Breaking Changes diff --git a/tests/integration/distribution/keeper/msg_server_test.go b/tests/integration/distribution/keeper/msg_server_test.go index 6c17aec04eee..0a32dc847a61 100644 --- a/tests/integration/distribution/keeper/msg_server_test.go +++ b/tests/integration/distribution/keeper/msg_server_test.go @@ -684,6 +684,20 @@ func TestMsgUpdateParams(t *testing.T) { expErr: true, expErrMsg: "invalid authority", }, + { + name: "community tax is nil", + msg: &distrtypes.MsgUpdateParams{ + Authority: f.distrKeeper.GetAuthority(), + Params: distrtypes.Params{ + CommunityTax: math.LegacyDec{}, + WithdrawAddrEnabled: withdrawAddrEnabled, + BaseProposerReward: math.LegacyZeroDec(), + BonusProposerReward: math.LegacyZeroDec(), + }, + }, + expErr: true, + expErrMsg: "community tax must be not nil", + }, { name: "community tax > 1", msg: &distrtypes.MsgUpdateParams{ @@ -696,7 +710,7 @@ func TestMsgUpdateParams(t *testing.T) { }, }, expErr: true, - expErrMsg: "community tax should be non-negative and less than one", + expErrMsg: "community tax too large: 2.000000000000000000", }, { name: "negative community tax", @@ -710,7 +724,7 @@ func TestMsgUpdateParams(t *testing.T) { }, }, expErr: true, - expErrMsg: "community tax should be non-negative and less than one", + expErrMsg: "community tax must be positive: -0.200000000000000000", }, { name: "base proposer reward set", diff --git a/x/distribution/types/params.go b/x/distribution/types/params.go index 395be69d7ea8..f7c980284175 100644 --- a/x/distribution/types/params.go +++ b/x/distribution/types/params.go @@ -18,13 +18,7 @@ func DefaultParams() Params { // ValidateBasic performs basic validation on distribution parameters. func (p Params) ValidateBasic() error { - if p.CommunityTax.IsNegative() || p.CommunityTax.GT(math.LegacyOneDec()) { - return fmt.Errorf( - "community tax should be non-negative and less than one: %s", p.CommunityTax, - ) - } - - return nil + return validateCommunityTax(p.CommunityTax) } func validateCommunityTax(i interface{}) error { diff --git a/x/distribution/types/params_test.go b/x/distribution/types/params_test.go index 4ba9167a0494..dfd68a380ed3 100644 --- a/x/distribution/types/params_test.go +++ b/x/distribution/types/params_test.go @@ -30,6 +30,7 @@ func TestParams_ValidateBasic(t *testing.T) { {"negative bonus proposer reward (must not matter)", fields{toDec("0.1"), toDec("0"), toDec("-0.1"), false}, false}, {"total sum greater than 1 (must not matter)", fields{toDec("0.2"), toDec("0.5"), toDec("0.4"), false}, false}, {"community tax greater than 1", fields{toDec("1.1"), toDec("0"), toDec("0"), false}, true}, + {"community tax nil", fields{sdkmath.LegacyDec{}, toDec("0"), toDec("0"), false}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {