From b79de8e3cf6a7cd563951fc9c6488be4bc35bc3a Mon Sep 17 00:00:00 2001 From: Ragnar Date: Sat, 25 Jan 2025 02:34:46 +0100 Subject: [PATCH 1/5] Update fee_pool.go --- x/distribution/types/fee_pool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index 894bff19196c..6d5469b662f1 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -20,8 +20,8 @@ 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) + if f.CommunityPool.IsAnyNegative() { + panic(fmt.Sprintf("negative CommunityPool in distribution fee pool, is %v", f.CommunityPool)) } return nil From be36e1bfebb8fa744721601cc4da26b44e5f5d20 Mon Sep 17 00:00:00 2001 From: Ragnar Date: Mon, 3 Feb 2025 17:38:28 +0100 Subject: [PATCH 2/5] Update fee_pool.go --- x/distribution/types/fee_pool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index 6d5469b662f1..0cf974608e4b 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -20,8 +20,8 @@ func (f FeePool) ValidateGenesis() error { return fmt.Errorf("negative DecimalPool in distribution fee pool, is %v", f.DecimalPool) } - if f.CommunityPool.IsAnyNegative() { - panic(fmt.Sprintf("negative CommunityPool in distribution fee pool, is %v", f.CommunityPool)) + if !f.CommunityPool.IsZero() { + panic(fmt.Sprintf("CommunityPool must be zero in distribution fee pool as it should be specified in protocolpool, current value: %v", f.CommunityPool)) } return nil From 2d8b2d8180ab8d4a0554cbbe53a1a7ebeaf44aed Mon Sep 17 00:00:00 2001 From: Ragnar Date: Tue, 4 Feb 2025 20:44:36 +0100 Subject: [PATCH 3/5] Update fee_pool_test.go --- x/distribution/types/fee_pool_test.go | 57 +++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/x/distribution/types/fee_pool_test.go b/x/distribution/types/fee_pool_test.go index 86d3c93f6e64..c7cc151d7e8e 100644 --- a/x/distribution/types/fee_pool_test.go +++ b/x/distribution/types/fee_pool_test.go @@ -12,12 +12,55 @@ import ( ) func TestValidateGenesis(t *testing.T) { - fp := types.InitialFeePool() - require.Nil(t, fp.ValidateGenesis()) + testCases := []struct { + name string + feePool types.FeePool + shouldPanic bool + }{ + { + name: "valid fee pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + CommunityPool: sdk.DecCoins{}, + }, + shouldPanic: false, + }, + { + name: "negative decimal pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{ + sdk.DecCoin{Denom: "stake", Amount: sdk.NewDec(-1)}, + }, + CommunityPool: sdk.DecCoins{}, + }, + shouldPanic: false, + }, + { + name: "non-zero community pool", + feePool: types.FeePool{ + DecimalPool: sdk.DecCoins{}, + CommunityPool: sdk.DecCoins{ + sdk.DecCoin{Denom: "stake", Amount: sdk.NewDec(1)}, + }, + }, + shouldPanic: true, + }, + } - 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() + }, "expected ValidateGenesis to panic with non-zero community pool") + } else { + err := tc.feePool.ValidateGenesis() + if tc.feePool.DecimalPool.IsAnyNegative() { + require.Error(t, err) + } else { + require.NoError(t, err) + } + } + }) + } } From 71d28413cf3dda30aa3db05196856f349df534a3 Mon Sep 17 00:00:00 2001 From: Ragnar Date: Wed, 26 Feb 2025 02:06:30 +0100 Subject: [PATCH 4/5] Update fee_pool_test.go --- x/distribution/types/fee_pool_test.go | 74 ++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/x/distribution/types/fee_pool_test.go b/x/distribution/types/fee_pool_test.go index c7cc151d7e8e..25c5b76bb077 100644 --- a/x/distribution/types/fee_pool_test.go +++ b/x/distribution/types/fee_pool_test.go @@ -16,34 +16,80 @@ func TestValidateGenesis(t *testing.T) { name string feePool types.FeePool shouldPanic bool + expectError bool }{ { - name: "valid fee pool", - feePool: types.FeePool{ - DecimalPool: sdk.DecCoins{}, - CommunityPool: sdk.DecCoins{}, - }, + name: "valid fee pool", + feePool: types.InitialFeePool(), shouldPanic: false, + expectError: false, }, { name: "negative decimal pool", feePool: types.FeePool{ - DecimalPool: sdk.DecCoins{ - sdk.DecCoin{Denom: "stake", Amount: sdk.NewDec(-1)}, - }, + DecimalPool: sdk.DecCoins{{Denom: "stake", Amount: math.LegacyNewDec(-1)}}, CommunityPool: sdk.DecCoins{}, }, shouldPanic: false, + expectError: true, }, { - name: "non-zero community pool", + 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{ - sdk.DecCoin{Denom: "stake", Amount: sdk.NewDec(1)}, + {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, }, } @@ -52,13 +98,13 @@ func TestValidateGenesis(t *testing.T) { if tc.shouldPanic { require.Panics(t, func() { tc.feePool.ValidateGenesis() - }, "expected ValidateGenesis to panic with non-zero community pool") + }, "ValidateGenesis should panic for test case: %s", tc.name) } else { err := tc.feePool.ValidateGenesis() - if tc.feePool.DecimalPool.IsAnyNegative() { - require.Error(t, err) + if tc.expectError { + require.Error(t, err, "ValidateGenesis should return error for test case: %s", tc.name) } else { - require.NoError(t, err) + require.NoError(t, err, "ValidateGenesis should not return error for valid FeePool in test case: %s", tc.name) } } }) From 298c204d2b00b937692e214937b49c636dae843d Mon Sep 17 00:00:00 2001 From: Ragnar Date: Wed, 26 Feb 2025 02:06:41 +0100 Subject: [PATCH 5/5] Update fee_pool.go --- x/distribution/types/fee_pool.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index 0cf974608e4b..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.IsZero() { - panic(fmt.Sprintf("CommunityPool must be zero in distribution fee pool as it should be specified in protocolpool, current value: %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