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

cosmos · Aug 1, 2023 · b9b325e · b9b325e
1 parent c33b4db
commit b9b325e
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
@@ -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
@@ -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) {