From d96c89e78bf779893c27e43b8d91e8c21cc2bd83 Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 13 Nov 2024 19:06:43 +0100 Subject: [PATCH 1/2] feat: ported initia changes --- x/rewards/types/dec_pool.go | 31 +++- x/rewards/types/dec_pool_test.go | 310 +++++++++++++++++++++++++++++++ x/rewards/types/pool.go | 105 ++++++----- x/rewards/types/pool_test.go | 243 ++++++++++++++++++++++++ 4 files changed, 633 insertions(+), 56 deletions(-) create mode 100644 x/rewards/types/dec_pool_test.go create mode 100644 x/rewards/types/pool_test.go diff --git a/x/rewards/types/dec_pool.go b/x/rewards/types/dec_pool.go index 434e3f4d4..0419a533e 100644 --- a/x/rewards/types/dec_pool.go +++ b/x/rewards/types/dec_pool.go @@ -19,7 +19,7 @@ func NewDecPoolsFromPools(pools Pools) DecPools { decPools = append(decPools, NewDecPool(p.Denom, sdk.NewDecCoinsFromCoins(p.Coins...))) } - return decPools + return decPools.Sort() } // Sum returns sum of pool tokens @@ -38,6 +38,13 @@ func (pools DecPools) Add(poolsB ...DecPool) DecPools { // Add will perform addition of two DecPools sets. func (pools DecPools) safeAdd(poolsB DecPools) DecPools { + if !pools.isSorted() { + panic("Pools (self) must be sorted") + } + if !poolsB.isSorted() { + panic("Wrong argument: Pools must be sorted") + } + sum := ([]DecPool)(nil) indexA, indexB := 0, 0 lenA, lenB := len(pools), len(poolsB) @@ -256,16 +263,30 @@ var _ sort.Interface = DecPools{} // Sort is a helper function to sort the set of p in-place func (pools DecPools) Sort() DecPools { - sort.Sort(pools) + // sort.Sort does a costly runtime copy as part of `runtime.convTSlice` + // So we avoid this heap allocation if len(dec pools) <= 1. In the future, we should hopefully find + // a strategy to always avoid this. + if len(pools) > 1 { + sort.Sort(pools) + } return pools } +func (pools DecPools) isSorted() bool { + for i := 1; i < len(pools); i++ { + if pools[i-1].Denom > pools[i].Denom { + return false + } + } + return true +} + //----------------------------------------------------------------------------- // DecPool functions // NewDecPool return new pool instance func NewDecPool(denom string, coins sdk.DecCoins) DecPool { - return DecPool{denom, coins} + return DecPool{denom, sdk.NewDecCoins(coins...)} } // IsEmpty returns wether the pool coins are empty or not @@ -301,7 +322,7 @@ func (pool DecPool) TruncateDecimal() (Pool, DecPool) { } func removeZeroDecPools(pools DecPools) DecPools { - result := make([]DecPool, 0, len(pools)) + result := make(DecPools, 0, len(pools)) for _, pool := range pools { if !pool.IsEmpty() { @@ -315,7 +336,7 @@ func removeZeroDecPools(pools DecPools) DecPools { // IsEqual returns true if the two sets of DecPools have the same value. func (pool DecPool) IsEqual(other DecPool) bool { if pool.Denom != other.Denom { - panic(fmt.Sprintf("invalid pool denominations; %s, %s", pool.Denom, other.Denom)) + return false } return pool.DecCoins.Equal(other.DecCoins) diff --git a/x/rewards/types/dec_pool_test.go b/x/rewards/types/dec_pool_test.go new file mode 100644 index 000000000..9fc05cf19 --- /dev/null +++ b/x/rewards/types/dec_pool_test.go @@ -0,0 +1,310 @@ +package types_test + +import ( + "testing" + + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/initia-labs/initia/x/distribution/types" + "github.com/stretchr/testify/suite" +) + +type decpoolTestSuite struct { + suite.Suite + ca0, ca1, ca2, ca4, cm0, cm1, cm2, cm4 sdk.Coin + emptyCoins sdk.Coins + pool0, pool1, pool2, pool4 types.Pool + pool000, pool111, pool222, pool444 types.Pool + pool101, pool110, pool122, pool211, pool244 types.Pool + emptyPools types.Pools + decpool0, decpool1, decpool2, decpool4 types.DecPool + decpool000, decpool111, decpool222, decpool444 types.DecPool + decpool101, decpool110, decpool122, decpool211, decpool244 types.DecPool + emptyDecPools types.DecPools +} + +func TestDecPoolTestSuite(t *testing.T) { + suite.Run(t, new(decpoolTestSuite)) +} + +func (s *decpoolTestSuite) SetupSuite() { + zero := math.NewInt(0) + one := math.OneInt() + two := math.NewInt(2) + four := math.NewInt(4) + s.ca0, s.ca1, s.ca2, s.ca4 = sdk.NewCoin(testCoinDenom1, zero), sdk.NewCoin(testCoinDenom1, one), sdk.NewCoin(testCoinDenom1, two), sdk.NewCoin(testCoinDenom1, four) + s.cm0, s.cm1, s.cm2, s.cm4 = sdk.NewCoin(testCoinDenom2, zero), sdk.NewCoin(testCoinDenom2, one), sdk.NewCoin(testCoinDenom2, two), sdk.NewCoin(testCoinDenom2, four) + s.emptyCoins = sdk.Coins{} + s.pool0 = types.NewPool(testPoolDenom0, sdk.Coins{}) + s.pool1 = types.NewPool(testPoolDenom1, sdk.Coins{}) + s.pool2 = types.NewPool(testPoolDenom2, sdk.Coins{}) + s.pool4 = types.NewPool(testPoolDenom4, sdk.Coins{}) + s.pool000 = types.NewPool(testPoolDenom0, sdk.NewCoins(s.ca0, s.cm0)) + s.pool111 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca1, s.cm1)) + s.pool222 = types.NewPool(testPoolDenom2, sdk.NewCoins(s.ca2, s.cm2)) + s.pool444 = types.NewPool(testPoolDenom4, sdk.NewCoins(s.ca4, s.cm4)) + s.pool101 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.cm1)) + s.pool110 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca1)) + s.pool122 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca2, s.cm2)) + s.pool211 = types.NewPool(testPoolDenom2, sdk.NewCoins(s.ca1, s.cm1)) + s.pool244 = types.NewPool(testPoolDenom2, sdk.NewCoins(s.ca4, s.cm4)) + s.emptyPools = types.Pools{} + s.decpool0 = types.NewDecPool(testPoolDenom0, sdk.DecCoins{}) + s.decpool1 = types.NewDecPool(testPoolDenom1, sdk.DecCoins{}) + s.decpool2 = types.NewDecPool(testPoolDenom2, sdk.DecCoins{}) + s.decpool4 = types.NewDecPool(testPoolDenom4, sdk.DecCoins{}) + s.decpool000 = types.NewDecPool(testPoolDenom0, sdk.NewDecCoinsFromCoins(s.ca0, s.cm0)) + s.decpool111 = types.NewDecPool(testPoolDenom1, sdk.NewDecCoinsFromCoins(s.ca1, s.cm1)) + s.decpool222 = types.NewDecPool(testPoolDenom2, sdk.NewDecCoinsFromCoins(s.ca2, s.cm2)) + s.decpool444 = types.NewDecPool(testPoolDenom4, sdk.NewDecCoinsFromCoins(s.ca4, s.cm4)) + s.decpool101 = types.NewDecPool(testPoolDenom1, sdk.NewDecCoinsFromCoins(s.cm1)) + s.decpool110 = types.NewDecPool(testPoolDenom1, sdk.NewDecCoinsFromCoins(s.ca1)) + s.decpool122 = types.NewDecPool(testPoolDenom1, sdk.NewDecCoinsFromCoins(s.ca2, s.cm2)) + s.decpool211 = types.NewDecPool(testPoolDenom2, sdk.NewDecCoinsFromCoins(s.ca1, s.cm1)) + s.decpool244 = types.NewDecPool(testPoolDenom2, sdk.NewDecCoinsFromCoins(s.ca4, s.cm4)) + s.emptyDecPools = types.DecPools{} +} + +func (s *decpoolTestSuite) TestIsEqualPool() { + coins11 := sdk.NewDecCoins(sdk.NewInt64DecCoin(testCoinDenom1, 1), sdk.NewInt64DecCoin(testCoinDenom2, 1)) + coins12 := sdk.NewDecCoins(sdk.NewInt64DecCoin(testCoinDenom1, 1), sdk.NewInt64DecCoin(testCoinDenom2, 2)) + cases := []struct { + inputOne types.DecPool + inputTwo types.DecPool + expected bool + }{ + {types.NewDecPool(testPoolDenom1, sdk.NewDecCoins(coins11...)), types.NewDecPool(testPoolDenom1, sdk.NewDecCoins(coins11...)), true}, + {types.NewDecPool(testPoolDenom1, sdk.NewDecCoins(coins11...)), types.NewDecPool(testPoolDenom2, sdk.NewDecCoins(coins11...)), false}, + {types.NewDecPool(testPoolDenom1, sdk.NewDecCoins(coins11...)), types.NewDecPool(testPoolDenom1, sdk.NewDecCoins(coins12...)), false}, + } + for tcIndex, tc := range cases { + res := tc.inputOne.IsEqual(tc.inputTwo) + s.Require().Equal(tc.expected, res, "pool equality relation is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestIsEmptyPool() { + cases := []struct { + input types.DecPool + expected bool + }{ + {types.NewDecPool(testPoolDenom1, sdk.DecCoins{}), true}, + {types.NewDecPool(testPoolDenom1, sdk.NewDecCoins(sdk.NewDecCoinFromCoin(s.ca1), sdk.NewDecCoinFromCoin(s.cm1))), false}, + } + for tcIndex, tc := range cases { + res := tc.input.IsEmpty() + s.Require().Equal(tc.expected, res, "pool emptiness is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestAddPool() { + cases := []struct { + inputOne types.DecPool + inputTwo types.DecPool + expected types.DecPool + shouldPanic bool + }{ + {s.decpool111, s.decpool111, s.decpool122, false}, + {s.decpool101, s.decpool110, s.decpool111, false}, + {types.NewDecPool(testPoolDenom1, sdk.DecCoins{}), s.decpool111, s.decpool111, false}, + {s.decpool111, s.decpool211, s.decpool111, true}, + } + for tcIndex, tc := range cases { + if tc.shouldPanic { + s.Require().Panics(func() { tc.inputOne.Add(tc.inputTwo) }) + } else { + res := tc.inputOne.Add(tc.inputTwo) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } + } +} + +func (s *decpoolTestSuite) TestSubPool() { + cases := []struct { + inputOne types.DecPool + inputTwo types.DecPool + expected types.DecPool + shouldPanic bool + }{ + {s.decpool122, s.decpool111, s.decpool111, false}, + {s.decpool111, s.decpool110, s.decpool101, false}, + {s.decpool111, s.decpool111, types.DecPool{testPoolDenom1, sdk.DecCoins(nil)}, false}, + {s.decpool111, s.decpool211, s.decpool111, true}, + } + for tcIndex, tc := range cases { + if tc.shouldPanic { + s.Require().Panics(func() { tc.inputOne.Sub(tc.inputTwo) }) + } else { + res := tc.inputOne.Sub(tc.inputTwo) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } + } +} + +func (s *decpoolTestSuite) TestNewPoolsSorted() { + cases := []struct { + input types.DecPools + expected types.DecPools + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.DecPools{s.decpool111, s.decpool222}}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool444, s.pool111)), types.DecPools{s.decpool111, s.decpool444}}, + } + for tcIndex, tc := range cases { + s.Require().Equal(tc.input.IsEqual(tc.expected), true, "pools are not sorted, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestAddPools() { + cases := []struct { + inputOne types.DecPools + inputTwo types.DecPools + expected types.DecPools + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.DecPools{}, types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222))}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool122, s.pool244))}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool222, s.pool111)), types.NewDecPoolsFromPools(types.NewPools(s.pool122, s.pool244))}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool444, s.pool000)), types.NewDecPoolsFromPools(types.NewPools(s.pool000, s.pool222, s.pool111, s.pool444))}, + } + for tcIndex, tc := range cases { + res := tc.inputOne.Add(tc.inputTwo...) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestSubPools() { + cases := []struct { + inputOne types.DecPools + inputTwo types.DecPools + expected types.DecPools + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.DecPools(nil)}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool122, s.pool244)), types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222))}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool122, s.pool244)), types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool222, s.pool111))}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool000, s.pool222, s.pool111, s.pool444)), types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool444, s.pool000))}, + } + for tcIndex, tc := range cases { + res := tc.inputOne.Sub(tc.inputTwo) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestIsAnyNegativePools() { + cases := []struct { + input types.DecPools + expected bool + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222, s.pool444)), false}, + {types.DecPools{types.DecPool{"test", sdk.DecCoins{sdk.DecCoin{"testdenom", math.LegacyNewDecFromInt(math.NewInt(-10))}}}}, true}, + } + for tcIndex, tc := range cases { + res := tc.input.IsAnyNegative() + s.Require().Equal(tc.expected, res, "negative pool coins check is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestCoinsOfPools() { + pools := types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222, s.pool444)) + cases := []struct { + input string + expected sdk.DecCoins + }{ + {testPoolDenom1, sdk.NewDecCoinsFromCoins(s.ca1, s.cm1)}, + {testPoolDenom2, sdk.NewDecCoinsFromCoins(s.ca2, s.cm2)}, + {testPoolDenom4, sdk.NewDecCoinsFromCoins(s.ca4, s.cm4)}, + } + for tcIndex, tc := range cases { + res := pools.CoinsOf(tc.input) + s.Require().True(tc.expected.Equal(res), "pool coins retrieval is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestIsEmptyPools() { + cases := []struct { + input types.DecPools + expected bool + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool0)), true}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111)), false}, + } + for tcIndex, tc := range cases { + res := tc.input.IsEmpty() + s.Require().Equal(tc.expected, res, "pool emptiness is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestIsEqualPools() { + cases := []struct { + inputOne types.DecPools + inputTwo types.DecPools + expected bool + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool000, s.pool111)), types.NewDecPoolsFromPools(types.NewPools(s.pool111)), true}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.NewPools(s.pool111)), false}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), types.NewDecPoolsFromPools(types.Pools{s.pool111, s.pool222, s.pool000}), false}, // should we delete empty pool? + } + for tcIndex, tc := range cases { + res := tc.inputOne.IsEqual(tc.inputTwo) + s.Require().Equal(tc.expected, res, "pools equality relation is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestSumPools() { + cases := []struct { + input types.DecPools + expected sdk.DecCoins + }{ + {types.NewDecPoolsFromPools(types.NewPools(s.pool122, s.pool222)), sdk.NewDecCoinsFromCoins(s.ca4, s.cm4)}, + {types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool111)), sdk.NewDecCoinsFromCoins(s.ca2, s.cm2)}, + } + for tcIndex, tc := range cases { + res := tc.input.Sum() + s.Require().True(tc.expected.Equal(res), "sum of pools is incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestTruncatePools() { + cases := []struct { + input types.DecPools + expected1 types.Pools + expected2 types.DecPools + }{ + { + types.DecPools{types.DecPool{"test1", sdk.DecCoins{sdk.DecCoin{"testdenom1", math.LegacyNewDecFromIntWithPrec(math.NewInt(10500), 3)}}}}, + types.Pools{types.Pool{"test1", sdk.Coins{sdk.Coin{"testdenom1", math.NewInt(10)}}}}, + types.DecPools{types.DecPool{"test1", sdk.DecCoins{sdk.DecCoin{"testdenom1", math.LegacyNewDecFromIntWithPrec(math.NewInt(500), 3)}}}}, + }, + { + types.DecPools{types.DecPool{"test1", sdk.DecCoins{sdk.DecCoin{"testdenom1", math.LegacyNewDecFromIntWithPrec(math.NewInt(10000), 3)}}}}, + types.Pools{types.Pool{"test1", sdk.Coins{sdk.Coin{"testdenom1", math.NewInt(10)}}}}, + types.DecPools{}, + }, + } + for tcIndex, tc := range cases { + res1, res2 := tc.input.TruncateDecimal() + s.Require().True(tc.expected1.IsEqual(res1), "truncated pools are incorrect, tc #%d", tcIndex) + s.Require().True(tc.expected2.IsEqual(res2), "change pools are incorrect, tc #%d", tcIndex) + } +} + +func (s *decpoolTestSuite) TestIntersectPools() { + cases := []struct { + inputOne types.DecPools + inputTwo types.DecPools + expected types.DecPools + }{ + { + types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), + types.NewDecPoolsFromPools(types.NewPools(s.pool110)), + types.NewDecPoolsFromPools(types.NewPools(s.pool110)), + }, + { + types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool222)), + types.NewDecPoolsFromPools(types.NewPools(s.pool111, s.pool444)), + types.NewDecPoolsFromPools(types.NewPools(s.pool111)), + }, + } + for tcIndex, tc := range cases { + res := tc.inputOne.Intersect(tc.inputTwo) + s.Require().True(tc.expected.IsEqual(res), "intersection of pools is incorrect, tc #%d", tcIndex) + } +} diff --git a/x/rewards/types/pool.go b/x/rewards/types/pool.go index aeb54d0b6..5c725a59e 100644 --- a/x/rewards/types/pool.go +++ b/x/rewards/types/pool.go @@ -3,7 +3,6 @@ package types import ( "fmt" "sort" - "strings" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -12,6 +11,11 @@ import ( // rewards pools for multi-token staking type Pools []Pool +// NewPools creates a new Pools instance +func NewPools(pools ...Pool) Pools { + return removeZeroPools(pools).Sort() +} + // Sum returns sum of pool tokens func (pools Pools) Sum() (coins sdk.Coins) { for _, p := range pools { @@ -27,52 +31,37 @@ func (pools Pools) Add(poolsB ...Pool) Pools { } // Add will perform addition of two Pools sets. -func (pools Pools) safeAdd(poolsB Pools) Pools { - sum := ([]Pool)(nil) - indexA, indexB := 0, 0 - lenA, lenB := len(pools), len(poolsB) - - for { - if indexA == lenA { - if indexB == lenB { - // return nil pools if both sets are empty - return sum - } - - // return set B (excluding zero pools) if set A is empty - return append(sum, removeZeroPools(poolsB[indexB:])...) - } else if indexB == lenB { - // return set A (excluding zero pools) if set B is empty - return append(sum, removeZeroPools(pools[indexA:])...) - } - - poolA, poolB := pools[indexA], poolsB[indexB] - - switch strings.Compare(poolA.Denom, poolB.Denom) { - case -1: // pool A denom < pool B denom - if !poolA.IsEmpty() { - sum = append(sum, poolA) - } - - indexA++ - - case 0: // pool A denom == pool B denom - res := poolA.Add(poolB) - if !res.IsEmpty() { - sum = append(sum, res) - } - - indexA++ - indexB++ +func (pools Pools) safeAdd(poolsB Pools) (coalesced Pools) { + // probably the best way will be to make Pools and interface and hide the structure + // definition (type alias) + if !pools.isSorted() { + panic("Pools (self) must be sorted") + } + if !poolsB.isSorted() { + panic("Wrong argument: Pools must be sorted") + } - case 1: // pool A denom > pool B denom - if !poolB.IsEmpty() { - sum = append(sum, poolB) - } + uniqPools := make(map[string]Pools, len(pools)+len(poolsB)) + // Traverse all the pools for each of the pools and poolsB. + for _, pL := range []Pools{pools, poolsB} { + for _, c := range pL { + uniqPools[c.Denom] = append(uniqPools[c.Denom], c) + } + } - indexB++ + for denom, pL := range uniqPools { //#nosec + comboPool := Pool{Denom: denom, Coins: sdk.Coins{}} + for _, p := range pL { + comboPool = comboPool.Add(p) } + if !comboPool.IsEmpty() { + coalesced = append(coalesced, comboPool) + } + } + if coalesced == nil { + return Pools{} } + return coalesced.Sort() } // Sub subtracts a set of Pools from another (adds the inverse). @@ -140,13 +129,13 @@ func (pools Pools) CoinsOf(denom string) sdk.Coins { default: midIdx := len(pools) / 2 // 2:1, 3:1, 4:2 - coin := pools[midIdx] + pool := pools[midIdx] switch { - case denom < coin.Denom: + case denom < pool.Denom: return pools[:midIdx].CoinsOf(denom) - case denom == coin.Denom: - return coin.Coins + case denom == pool.Denom: + return pool.Coins default: return pools[midIdx+1:].CoinsOf(denom) } @@ -212,16 +201,30 @@ var _ sort.Interface = Pools{} // Sort is a helper function to sort the set of p in-place func (pools Pools) Sort() Pools { - sort.Sort(pools) + // sort.Sort does a costly runtime copy as part of `runtime.convTSlice` + // So we avoid this heap allocation if len(pools) <= 1. In the future, we should hopefully find + // a strategy to always avoid this. + if len(pools) > 1 { + sort.Sort(pools) + } return pools } +func (pools Pools) isSorted() bool { + for i := 1; i < len(pools); i++ { + if pools[i-1].Denom > pools[i].Denom { + return false + } + } + return true +} + //----------------------------------------------------------------------------- // Pool functions // NewPool return new pool instance func NewPool(denom string, coins sdk.Coins) Pool { - return Pool{denom, coins} + return Pool{denom, sdk.NewCoins(coins...)} } // IsEmpty returns wether the pool coins are empty or not @@ -250,7 +253,7 @@ func (pool Pool) Sub(poolB Pool) Pool { } func removeZeroPools(pools Pools) Pools { - result := make([]Pool, 0, len(pools)) + result := make(Pools, 0, len(pools)) for _, pool := range pools { if !pool.IsEmpty() { @@ -264,7 +267,7 @@ func removeZeroPools(pools Pools) Pools { // IsEqual returns true if the two sets of Pools have the same value. func (pool Pool) IsEqual(other Pool) bool { if pool.Denom != other.Denom { - panic(fmt.Sprintf("invalid pool denominations; %s, %s", pool.Denom, other.Denom)) + return false } return pool.Coins.Equal(other.Coins) diff --git a/x/rewards/types/pool_test.go b/x/rewards/types/pool_test.go new file mode 100644 index 000000000..2fc8111ac --- /dev/null +++ b/x/rewards/types/pool_test.go @@ -0,0 +1,243 @@ +package types_test + +import ( + "testing" + + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/initia-labs/initia/x/distribution/types" + "github.com/stretchr/testify/suite" +) + +var ( + testPoolDenom0 = "pool0" + testPoolDenom1 = "pool1" + testPoolDenom2 = "pool2" + testPoolDenom4 = "pool4" +) + +var ( + testCoinDenom1 = "coin1" + testCoinDenom2 = "coin2" +) + +type poolTestSuite struct { + suite.Suite + ca0, ca1, ca2, ca4, cm0, cm1, cm2, cm4 sdk.Coin + emptyCoins sdk.Coins + pool0, pool1, pool2, pool4 types.Pool + pool000, pool111, pool222, pool444 types.Pool + pool101, pool110, pool122, pool211, pool244 types.Pool + emptyPools types.Pools +} + +func TestPoolTestSuite(t *testing.T) { + suite.Run(t, new(poolTestSuite)) +} + +func (s *poolTestSuite) SetupSuite() { + zero := math.NewInt(0) + one := math.OneInt() + two := math.NewInt(2) + four := math.NewInt(4) + s.ca0, s.ca1, s.ca2, s.ca4 = sdk.NewCoin(testCoinDenom1, zero), sdk.NewCoin(testCoinDenom1, one), sdk.NewCoin(testCoinDenom1, two), sdk.NewCoin(testCoinDenom1, four) + s.cm0, s.cm1, s.cm2, s.cm4 = sdk.NewCoin(testCoinDenom2, zero), sdk.NewCoin(testCoinDenom2, one), sdk.NewCoin(testCoinDenom2, two), sdk.NewCoin(testCoinDenom2, four) + s.emptyCoins = sdk.Coins{} + s.pool0 = types.NewPool(testPoolDenom0, sdk.Coins{}) + s.pool1 = types.NewPool(testPoolDenom1, sdk.Coins{}) + s.pool2 = types.NewPool(testPoolDenom2, sdk.Coins{}) + s.pool4 = types.NewPool(testPoolDenom4, sdk.Coins{}) + s.pool000 = types.NewPool(testPoolDenom0, sdk.NewCoins(s.ca0, s.cm0)) + s.pool111 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca1, s.cm1)) + s.pool222 = types.NewPool(testPoolDenom2, sdk.NewCoins(s.ca2, s.cm2)) + s.pool444 = types.NewPool(testPoolDenom4, sdk.NewCoins(s.ca4, s.cm4)) + s.pool101 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.cm1)) + s.pool110 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca1)) + s.pool122 = types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca2, s.cm2)) + s.pool211 = types.NewPool(testPoolDenom2, sdk.NewCoins(s.ca1, s.cm1)) + s.pool244 = types.NewPool(testPoolDenom2, sdk.NewCoins(s.ca4, s.cm4)) + s.emptyPools = types.Pools{} +} + +func (s *poolTestSuite) TestIsEqualPool() { + coins11 := sdk.NewCoins(sdk.NewInt64Coin(testCoinDenom1, 1), sdk.NewInt64Coin(testCoinDenom2, 1)) + coins12 := sdk.NewCoins(sdk.NewInt64Coin(testCoinDenom1, 1), sdk.NewInt64Coin(testCoinDenom2, 2)) + cases := []struct { + inputOne types.Pool + inputTwo types.Pool + expected bool + }{ + {types.NewPool(testPoolDenom1, sdk.NewCoins(coins11...)), types.NewPool(testPoolDenom1, sdk.NewCoins(coins11...)), true}, + {types.NewPool(testPoolDenom1, sdk.NewCoins(coins11...)), types.NewPool(testPoolDenom2, sdk.NewCoins(coins11...)), false}, + {types.NewPool(testPoolDenom1, sdk.NewCoins(coins11...)), types.NewPool(testPoolDenom1, sdk.NewCoins(coins12...)), false}, + } + for tcIndex, tc := range cases { + res := tc.inputOne.IsEqual(tc.inputTwo) + s.Require().Equal(tc.expected, res, "pool equality relation is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestIsEmptyPool() { + cases := []struct { + input types.Pool + expected bool + }{ + {types.NewPool(testPoolDenom1, s.emptyCoins), true}, + {types.NewPool(testPoolDenom1, sdk.NewCoins(s.ca1, s.cm1)), false}, + } + for tcIndex, tc := range cases { + res := tc.input.IsEmpty() + s.Require().Equal(tc.expected, res, "pool emptiness is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestAddPool() { + cases := []struct { + inputOne types.Pool + inputTwo types.Pool + expected types.Pool + shouldPanic bool + }{ + {s.pool111, s.pool111, s.pool122, false}, + {s.pool101, s.pool110, s.pool111, false}, + {types.NewPool(testPoolDenom1, sdk.Coins{}), s.pool111, s.pool111, false}, + {s.pool111, s.pool211, s.pool111, true}, + } + for tcIndex, tc := range cases { + if tc.shouldPanic { + s.Require().Panics(func() { tc.inputOne.Add(tc.inputTwo) }) + } else { + res := tc.inputOne.Add(tc.inputTwo) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } + } +} + +func (s *poolTestSuite) TestSubPool() { + cases := []struct { + inputOne types.Pool + inputTwo types.Pool + expected types.Pool + shouldPanic bool + }{ + {s.pool122, s.pool111, s.pool111, false}, + {s.pool111, s.pool110, s.pool101, false}, + {s.pool111, s.pool111, types.NewPool(testPoolDenom1, sdk.Coins{}), false}, + {s.pool111, s.pool211, s.pool111, true}, + } + for tcIndex, tc := range cases { + if tc.shouldPanic { + s.Require().Panics(func() { tc.inputOne.Sub(tc.inputTwo) }) + } else { + res := tc.inputOne.Sub(tc.inputTwo) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } + } +} + +func (s *poolTestSuite) TestNewPoolsSorted() { + cases := []struct { + input types.Pools + expected types.Pools + }{ + {types.NewPools(s.pool111, s.pool222), types.Pools{s.pool111, s.pool222}}, + {types.NewPools(s.pool444, s.pool111), types.Pools{s.pool111, s.pool444}}, + } + for tcIndex, tc := range cases { + s.Require().Equal(tc.input.IsEqual(tc.expected), true, "pools are not sorted, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestAddPools() { + cases := []struct { + inputOne types.Pools + inputTwo types.Pools + expected types.Pools + }{ + {types.NewPools(s.pool111, s.pool222), types.Pools{}, types.NewPools(s.pool111, s.pool222)}, + {types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool122, s.pool244)}, + {types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool222, s.pool111), types.NewPools(s.pool122, s.pool244)}, + {types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool444, s.pool000), types.NewPools(s.pool000, s.pool222, s.pool111, s.pool444)}, + } + for tcIndex, tc := range cases { + res := tc.inputOne.Add(tc.inputTwo...) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestSubPools() { + cases := []struct { + inputOne types.Pools + inputTwo types.Pools + expected types.Pools + }{ + {types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool111, s.pool222), types.Pools{}}, + {types.NewPools(s.pool122, s.pool244), types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool111, s.pool222)}, + {types.NewPools(s.pool122, s.pool244), types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool222, s.pool111)}, + {types.NewPools(s.pool000, s.pool222, s.pool111, s.pool444), types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool444, s.pool000)}, + } + for tcIndex, tc := range cases { + res := tc.inputOne.Sub(tc.inputTwo) + s.Require().Equal(tc.expected, res, "sum of pools is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestIsAnyNegativePools() { + cases := []struct { + input types.Pools + expected bool + }{ + {types.NewPools(s.pool111, s.pool222, s.pool444), false}, + {types.NewPools(types.Pool{"test", sdk.Coins{sdk.Coin{"testdenom", math.NewInt(-10)}}}), true}, + } + for tcIndex, tc := range cases { + res := tc.input.IsAnyNegative() + s.Require().Equal(tc.expected, res, "negative pool coins check is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestCoinsOfPools() { + pools := types.NewPools(s.pool111, s.pool222, s.pool444) + cases := []struct { + input string + expected sdk.Coins + }{ + {testPoolDenom1, sdk.NewCoins(s.ca1, s.cm1)}, + {testPoolDenom2, sdk.NewCoins(s.ca2, s.cm2)}, + {testPoolDenom4, sdk.NewCoins(s.ca4, s.cm4)}, + } + for tcIndex, tc := range cases { + res := pools.CoinsOf(tc.input) + s.Require().True(tc.expected.Equal(res), "pool coins retrieval is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestIsEmptyPools() { + cases := []struct { + input types.Pools + expected bool + }{ + {types.NewPools(s.pool0), true}, + {types.NewPools(s.pool111), false}, + } + for tcIndex, tc := range cases { + res := tc.input.IsEmpty() + s.Require().Equal(tc.expected, res, "pool emptiness is incorrect, tc #%d", tcIndex) + } +} + +func (s *poolTestSuite) TestIsEqualPools() { + cases := []struct { + inputOne types.Pools + inputTwo types.Pools + expected bool + }{ + {types.NewPools(s.pool000, s.pool111), types.NewPools(s.pool111), true}, + {types.NewPools(s.pool111, s.pool222), types.NewPools(s.pool111), false}, + {types.NewPools(s.pool111, s.pool222), types.Pools{s.pool111, s.pool222, s.pool000}, false}, // should we delete empty pool? + } + for tcIndex, tc := range cases { + res := tc.inputOne.IsEqual(tc.inputTwo) + s.Require().Equal(tc.expected, res, "pools equality relation is incorrect, tc #%d", tcIndex) + } +} From a8df0ed3d2fc3a23e613bcd13e129a06493d33e3 Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 13 Nov 2024 19:14:12 +0100 Subject: [PATCH 2/2] fix: use add instead of append --- x/rewards/keeper/delegation.go | 2 +- x/rewards/keeper/target.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/rewards/keeper/delegation.go b/x/rewards/keeper/delegation.go index 3306729cf..2a8c5e1dd 100644 --- a/x/rewards/keeper/delegation.go +++ b/x/rewards/keeper/delegation.go @@ -77,7 +77,7 @@ func (k *Keeper) calculateDelegationRewardsBetween( } for _, diff := range differences { - rewards = append(rewards, types.NewDecPool( + rewards = rewards.Add(types.NewDecPool( diff.Denom, diff.DecCoins.MulDecTruncate(stakes.AmountOf(diff.Denom)), )) diff --git a/x/rewards/keeper/target.go b/x/rewards/keeper/target.go index 1ded465a2..3a671d44d 100644 --- a/x/rewards/keeper/target.go +++ b/x/rewards/keeper/target.go @@ -88,9 +88,9 @@ func (k *Keeper) IncrementDelegationTargetPeriod(ctx context.Context, target res // can't calculate ratio for zero-token targets // ergo we instead add to the community pool communityFunding = communityFunding.Add(types.NewDecPool(token.Denom, rewardCoins)) - current = append(current, types.NewDecPool(token.Denom, sdk.DecCoins{})) + current = current.Add(types.NewDecPool(token.Denom, sdk.DecCoins{})) } else { - current = append(current, + current = current.Add( types.NewDecPool(token.Denom, rewardCoins.QuoDecTruncate(math.LegacyNewDecFromInt(token.Amount)))) } }