From 1baee98052423150e8e5657fe9d4ee6e4c5426c0 Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 06:33:47 +0000 Subject: [PATCH 1/6] fix(x/distribution): ensure .CommunityTax is not nil in types.ValidateBasic() --- x/distribution/types/params.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x/distribution/types/params.go b/x/distribution/types/params.go index 395be69d7ea8..ea2da6b9394c 100644 --- a/x/distribution/types/params.go +++ b/x/distribution/types/params.go @@ -1,6 +1,7 @@ package types import ( + "errors" "fmt" "cosmossdk.io/math" @@ -18,6 +19,10 @@ func DefaultParams() Params { // ValidateBasic performs basic validation on distribution parameters. func (p Params) ValidateBasic() error { + if p.CommunityTax.IsNil() { + return errors.New("CommunityTax is nil") + } + 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, From d249af7c330a9bdb4c5e23bb81abad29159ccc57 Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 08:56:38 +0000 Subject: [PATCH 2/6] fix(x/distribution): add test for ValidateBasic --- x/distribution/types/params_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/x/distribution/types/params_test.go b/x/distribution/types/params_test.go index 4ba9167a0494..2390a5e0a54d 100644 --- a/x/distribution/types/params_test.go +++ b/x/distribution/types/params_test.go @@ -47,3 +47,12 @@ func TestParams_ValidateBasic(t *testing.T) { func TestDefaultParams(t *testing.T) { require.NoError(t, types.DefaultParams().ValidateBasic()) } + +func TestNilCommunityTax(t *testing.T) { + var p types.Params + s := `{"community_tax": "", "base_proposer_reward": "", "bonus_proposer_reward": "", "withdraw_addr_enabled": false}` + p.Unmarshal([]byte(s)) + if p.ValidateBasic() == nil { + t.Errorf("ValidateBasic() should return error when CommunityTax is nil: error") + } +} From 0be336a836b1a9b6d11efa723e7c13de3d90a50f Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 09:01:34 +0000 Subject: [PATCH 3/6] feat(distribution): use validateCommunityTax Use function `validateCommunityTax` in `Params.ValidateBasic`. --- x/distribution/types/params.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/x/distribution/types/params.go b/x/distribution/types/params.go index ea2da6b9394c..f7c980284175 100644 --- a/x/distribution/types/params.go +++ b/x/distribution/types/params.go @@ -1,7 +1,6 @@ package types import ( - "errors" "fmt" "cosmossdk.io/math" @@ -19,17 +18,7 @@ func DefaultParams() Params { // ValidateBasic performs basic validation on distribution parameters. func (p Params) ValidateBasic() error { - if p.CommunityTax.IsNil() { - return errors.New("CommunityTax is nil") - } - - 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 { From ecf085349afc14fb2628ecff11081ffac90cbeff Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 09:07:49 +0000 Subject: [PATCH 4/6] fix(distribution): update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1e871464d68..c0f4ac382837 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 From 296df5da9d6e590b4cc4910ca2a4448866461139 Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 09:08:46 +0000 Subject: [PATCH 5/6] fix(distribution): using nil literal in test --- x/distribution/types/params_test.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/x/distribution/types/params_test.go b/x/distribution/types/params_test.go index 2390a5e0a54d..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) { @@ -47,12 +48,3 @@ func TestParams_ValidateBasic(t *testing.T) { func TestDefaultParams(t *testing.T) { require.NoError(t, types.DefaultParams().ValidateBasic()) } - -func TestNilCommunityTax(t *testing.T) { - var p types.Params - s := `{"community_tax": "", "base_proposer_reward": "", "bonus_proposer_reward": "", "withdraw_addr_enabled": false}` - p.Unmarshal([]byte(s)) - if p.ValidateBasic() == nil { - t.Errorf("ValidateBasic() should return error when CommunityTax is nil: error") - } -} From fe52941a464247ee0ce6f2e50599afea99ec25cf Mon Sep 17 00:00:00 2001 From: n0b0dy Date: Tue, 1 Aug 2023 10:00:21 +0000 Subject: [PATCH 6/6] fix(distrubution): update integration test Add test case in integration test `TestMsgUpdateParams` and update related error messages. --- .../distribution/keeper/msg_server_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) 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",