diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index 894bff19196c..83a83b4ad813 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -20,8 +20,10 @@ func (f FeePool) ValidateGenesis() error { return fmt.Errorf("negative DecimalPool in distribution fee pool, is %v", f.DecimalPool) } - if f.CommunityPool.IsAnyNegative() { // TODO(@julienrbrt) in v0.53, panic if the community pool is set - return fmt.Errorf("negative CommunityPool in distribution fee pool, is %v", f.CommunityPool) + // Negative values in CommunityPool represent an invalid state that should never occur + // We panic instead of returning an error to prevent the chain from starting with an invalid state + if f.CommunityPool.IsAnyNegative() { + panic(fmt.Sprintf("negative CommunityPool in distribution fee pool, is %v", f.CommunityPool)) } return nil diff --git a/x/distribution/types/fee_pool_test.go b/x/distribution/types/fee_pool_test.go index 86d3c93f6e64..25c5b76bb077 100644 --- a/x/distribution/types/fee_pool_test.go +++ b/x/distribution/types/fee_pool_test.go @@ -12,12 +12,101 @@ import ( ) func TestValidateGenesis(t *testing.T) { - fp := types.InitialFeePool() - require.Nil(t, fp.ValidateGenesis()) + testCases := []struct { + name string + feePool types.FeePool + shouldPanic bool + expectError bool + }{ + { + name: "valid fee pool", + feePool: types.InitialFeePool(), + shouldPanic: false, + expectError: false, + }, + { + name: "negative decimal pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{{Denom: "stake", Amount: math.LegacyNewDec(-1)}}, + CommunityPool: sdk.DecCoins{}, + }, + shouldPanic: false, + expectError: true, + }, + { + name: "negative community pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + CommunityPool: sdk.DecCoins{{Denom: "stake", Amount: math.LegacyNewDec(-1)}}, + }, + shouldPanic: true, + expectError: false, + }, + { + name: "multiple coins in community pool with one negative value", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + CommunityPool: sdk.DecCoins{ + {Denom: "stake", Amount: math.LegacyNewDec(1)}, + {Denom: "atom", Amount: math.LegacyNewDec(-2)}, + }, + }, + shouldPanic: true, + expectError: false, + }, + { + name: "multiple coins in community pool with all positive values", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + CommunityPool: sdk.DecCoins{ + {Denom: "stake", Amount: math.LegacyNewDec(1)}, + {Denom: "atom", Amount: math.LegacyNewDec(2)}, + }, + }, + shouldPanic: false, + expectError: false, + }, + { + name: "nil decimal pool", + feePool: types.FeePool{ + CommunityPool: sdk.DecCoins{}, + }, + shouldPanic: false, + expectError: false, + }, + { + name: "nil community pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + }, + shouldPanic: false, + expectError: false, + }, + { + name: "zero value community pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + CommunityPool: sdk.DecCoins{{Denom: "stake", Amount: math.LegacyNewDec(0)}}, + }, + shouldPanic: false, + expectError: false, + }, + } - fp2 := types.FeePool{CommunityPool: sdk.DecCoins{{Denom: "stake", Amount: math.LegacyNewDec(-1)}}} - require.NotNil(t, fp2.ValidateGenesis()) - - fp3 := types.FeePool{DecimalPool: sdk.DecCoins{{Denom: "stake", Amount: math.LegacyNewDec(-1)}}} - require.NotNil(t, fp3.ValidateGenesis()) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if tc.shouldPanic { + require.Panics(t, func() { + tc.feePool.ValidateGenesis() + }, "ValidateGenesis should panic for test case: %s", tc.name) + } else { + err := tc.feePool.ValidateGenesis() + if tc.expectError { + require.Error(t, err, "ValidateGenesis should return error for test case: %s", tc.name) + } else { + require.NoError(t, err, "ValidateGenesis should not return error for valid FeePool in test case: %s", tc.name) + } + } + }) + } }