fix(x/distribution): add nil check in Params.ValidateBasic (backport #…

…17236) (#17241) Co-authored-by: n0b0dy <n0b0dyCN@users.noreply.github.com>
cosmos · Aug 1, 2023 · 5624447 · 5624447
1 parent ff6a9a6
commit 5624447
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 9 deletions.
diff --git 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
@@ -681,6 +681,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{
@@ -693,7 +707,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",
@@ -707,7 +721,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
@@ -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
@@ -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) {