From 9f5c71702df38d674f18cd3097200259cb51906f Mon Sep 17 00:00:00 2001 From: Troy Kessler Date: Mon, 6 May 2024 14:07:59 +0200 Subject: [PATCH 1/2] chore: only charge coins which are whitelisted --- x/bundles/types/expected_keepers.go | 2 + x/funders/keeper/logic_funders.go | 58 ++++++++++++++----- x/funders/keeper/logic_funders_test.go | 37 ++++++------ .../keeper/msg_server_defund_pool_test.go | 14 ++--- x/funders/keeper/msg_server_fund_pool_test.go | 18 +++--- x/funders/spec/06_exported.md | 3 +- x/funders/types/funders.go | 22 +++---- x/funders/types/funders_test.go | 50 +++++++++++++--- x/pool/spec/07_exported.md | 1 + x/query/keeper/grpc_query_funders.go | 12 ++-- x/query/keeper/grpc_query_fundings.go | 10 ++-- 11 files changed, 146 insertions(+), 81 deletions(-) diff --git a/x/bundles/types/expected_keepers.go b/x/bundles/types/expected_keepers.go index ae16e50b..f074c129 100644 --- a/x/bundles/types/expected_keepers.go +++ b/x/bundles/types/expected_keepers.go @@ -3,6 +3,7 @@ package types import ( "cosmossdk.io/math" delegationTypes "github.com/KYVENetwork/chain/x/delegation/types" + "github.com/KYVENetwork/chain/x/funders/types" pooltypes "github.com/KYVENetwork/chain/x/pool/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -48,6 +49,7 @@ type DelegationKeeper interface { } type FundersKeeper interface { + GetCoinWhitelist(ctx sdk.Context) (whitelist []types.WhitelistCoinEntry) ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient string) (payout sdk.Coins, err error) } diff --git a/x/funders/keeper/logic_funders.go b/x/funders/keeper/logic_funders.go index eaff409d..6390ef27 100644 --- a/x/funders/keeper/logic_funders.go +++ b/x/funders/keeper/logic_funders.go @@ -31,27 +31,54 @@ func (k Keeper) GetTotalActiveFunding(ctx sdk.Context, poolId uint64) (amounts s return } +// GetCoinWhitelist gets the coin whitelist from the params of the funding module +func (k Keeper) GetCoinWhitelist(ctx sdk.Context) (whitelist []types.WhitelistCoinEntry) { + params := k.GetParams(ctx) + + for _, entry := range params.CoinWhitelist { + whitelist = append(whitelist, *entry) + } + + return +} + +// GetCoinWhitelistMap gets the coin whitelist as a map with the denom as key for easier lookup. +// WARNING: Don't use this for setter functions since go maps are non-deterministic! +func (k Keeper) GetCoinWhitelistMap(ctx sdk.Context) (whitelist map[string]types.WhitelistCoinEntry) { + whitelist = make(map[string]types.WhitelistCoinEntry) + + w := k.GetCoinWhitelist(ctx) + for _, entry := range w { + whitelist[entry.CoinDenom] = entry + } + + return +} + // ChargeFundersOfPool charges all funders of a pool with their amount_per_bundle // If the amount is lower than the amount_per_bundle, // the max amount is charged and the funder is removed from the active funders list. // The amount is transferred from the funders to the recipient module account. -// If there are no more active funders, an event is emitted. +// If there are no more active funders, an event is emitted. This method only charges +// coins which are whitelisted. func (k Keeper) ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient string) (payouts sdk.Coins, err error) { // Get funding state for pool fundingState, found := k.GetFundingState(ctx, poolId) if !found { - return sdk.NewCoins(), errors.Wrapf(errorsTypes.ErrNotFound, types.ErrFundingStateDoesNotExist.Error(), poolId) + return payouts, errors.Wrapf(errorsTypes.ErrNotFound, types.ErrFundingStateDoesNotExist.Error(), poolId) } // If there are no active fundings we immediately return activeFundings := k.GetActiveFundings(ctx, fundingState) if len(activeFundings) == 0 { - return sdk.NewCoins(), nil + return } - // This is the amount every funding will be charged + whitelist := k.GetCoinWhitelistMap(ctx) + + // Charge every active funder and collect payouts for _, funding := range activeFundings { - payouts = payouts.Add(funding.ChargeOneBundle()...) + payouts = payouts.Add(funding.ChargeOneBundle(whitelist)...) if funding.Amounts.IsZero() { fundingState.SetInactive(&funding) } @@ -68,24 +95,25 @@ func (k Keeper) ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient st }) } - // Move funds to pool module account - if !payouts.IsZero() { - if err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, types.ModuleName, recipient, payouts); err != nil { - return sdk.NewCoins(), err - } + if payouts.IsZero() { + return } + // Move funds to recipient module + err = k.bankKeeper.SendCoinsFromModuleToModule(ctx, types.ModuleName, recipient, payouts) return } // GetLowestFunding returns the funding with the lowest amount // Precondition: len(fundings) > 0 -func (k Keeper) GetLowestFunding(fundings []types.Funding, whitelist []*types.WhitelistCoinEntry) (lowestFunding *types.Funding, err error) { +func (k Keeper) GetLowestFunding(ctx sdk.Context, fundings []types.Funding) (lowestFunding *types.Funding, err error) { if len(fundings) == 0 { return nil, fmt.Errorf("no active fundings") } + whitelist := k.GetCoinWhitelistMap(ctx) lowestFundingIndex := 0 + for i := range fundings { if fundings[i].GetScore(whitelist) < fundings[lowestFundingIndex].GetScore(whitelist) { lowestFundingIndex = i @@ -167,9 +195,9 @@ func (k Keeper) ensureFreeSlot(ctx sdk.Context, newFunding *types.Funding, fundi return nil } - params := k.GetParams(ctx) + whitelist := k.GetCoinWhitelistMap(ctx) - lowestFunding, err := k.GetLowestFunding(activeFundings, params.CoinWhitelist) + lowestFunding, err := k.GetLowestFunding(ctx, activeFundings) if err != nil { return err } @@ -180,8 +208,8 @@ func (k Keeper) ensureFreeSlot(ctx sdk.Context, newFunding *types.Funding, fundi } // Check if lowest funding is lower than new funding based on amount (amount per bundle is ignored) - if newFunding.GetScore(params.CoinWhitelist) < lowestFunding.GetScore(params.CoinWhitelist) { - return errors.Wrapf(errorsTypes.ErrLogic, types.ErrFundsTooLow.Error(), lowestFunding.GetScore(params.CoinWhitelist)) + if newFunding.GetScore(whitelist) < lowestFunding.GetScore(whitelist) { + return errors.Wrapf(errorsTypes.ErrLogic, types.ErrFundsTooLow.Error(), lowestFunding.GetScore(whitelist)) } // Defund lowest funder diff --git a/x/funders/keeper/logic_funders_test.go b/x/funders/keeper/logic_funders_test.go index abf5030e..b56ca74c 100644 --- a/x/funders/keeper/logic_funders_test.go +++ b/x/funders/keeper/logic_funders_test.go @@ -368,7 +368,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { It("Charge funder that has coins which are not in the whitelist", func() { // ARRANGE - whitelist = []*funderstypes.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), funderstypes.NewParams([]*funderstypes.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -387,25 +387,24 @@ var _ = Describe("logic_funders.go", Ordered, func() { MinFundingAmountPerBundle: 1 * i.KYVE, CoinWeight: math.LegacyNewDec(3), }, - } - s.App().FundersKeeper.SetParams(s.Ctx(), funderstypes.NewParams(whitelist, 20)) + }, 20)) // ACT payout, err := s.App().FundersKeeper.ChargeFundersOfPool(s.Ctx(), 0, pooltypes.ModuleName) Expect(err).NotTo(HaveOccurred()) // ASSERT - Expect(payout.String()).To(Equal(i.ACoins(11 * i.T_KYVE).String())) + Expect(payout).To(BeEmpty()) fundingAlice, foundAlice := s.App().FundersKeeper.GetFunding(s.Ctx(), i.ALICE, 0) Expect(foundAlice).To(BeTrue()) - Expect(fundingAlice.Amounts.String()).To(Equal(i.ACoins(99 * i.T_KYVE).String())) - Expect(fundingAlice.TotalFunded.String()).To(Equal(i.ACoins(1 * i.T_KYVE).String())) + Expect(fundingAlice.Amounts.String()).To(Equal(i.ACoins(100 * i.T_KYVE).String())) + Expect(fundingAlice.TotalFunded).To(BeEmpty()) fundingBob, foundBob := s.App().FundersKeeper.GetFunding(s.Ctx(), i.BOB, 0) Expect(foundBob).To(BeTrue()) - Expect(fundingBob.Amounts.String()).To(Equal(i.ACoins(40 * i.T_KYVE).String())) - Expect(fundingBob.TotalFunded.String()).To(Equal(i.ACoins(10 * i.T_KYVE).String())) + Expect(fundingBob.Amounts.String()).To(Equal(i.ACoins(50 * i.T_KYVE).String())) + Expect(fundingBob.TotalFunded).To(BeEmpty()) fundingState, _ := s.App().FundersKeeper.GetFundingState(s.Ctx(), 0) Expect(fundingState.ActiveFunderAddresses).To(HaveLen(2)) @@ -414,8 +413,8 @@ var _ = Describe("logic_funders.go", Ordered, func() { fundersBalance := s.GetCoinsFromModule(funderstypes.ModuleName) poolBalance := s.GetCoinsFromModule(pooltypes.ModuleName) - Expect(fundersBalance.String()).To(Equal(i.ACoins(139 * i.T_KYVE).String())) - Expect(poolBalance.String()).To(Equal(i.ACoins(11 * i.T_KYVE).String())) + Expect(fundersBalance.String()).To(Equal(i.ACoins(150 * i.T_KYVE).String())) + Expect(poolBalance).To(BeEmpty()) }) It("Charge without fundings", func() { @@ -458,7 +457,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { }) It("Check if the lowest funding is returned correctly with one coin", func() { - whitelist := []*funderstypes.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), funderstypes.NewParams([]*funderstypes.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -477,7 +476,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { CoinDenom: i.C_DENOM, CoinWeight: math.LegacyNewDec(3), }, - } + }, 20)) fundings := []funderstypes.Funding{ { @@ -500,13 +499,13 @@ var _ = Describe("logic_funders.go", Ordered, func() { }, } - getLowestFunding, err := s.App().FundersKeeper.GetLowestFunding(fundings, whitelist) + getLowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), fundings) Expect(err).NotTo(HaveOccurred()) Expect(getLowestFunding.FunderAddress).To(Equal(i.DUMMY[2])) }) It("Check if the lowest funding is returned correctly with multiple coins", func() { - whitelist := []*funderstypes.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), funderstypes.NewParams([]*funderstypes.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -525,7 +524,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { CoinDenom: i.C_DENOM, CoinWeight: math.LegacyNewDec(3), }, - } + }, 20)) fundings := []funderstypes.Funding{ { @@ -548,13 +547,13 @@ var _ = Describe("logic_funders.go", Ordered, func() { }, } - getLowestFunding, err := s.App().FundersKeeper.GetLowestFunding(fundings, whitelist) + getLowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), fundings) Expect(err).NotTo(HaveOccurred()) Expect(getLowestFunding.FunderAddress).To(Equal(i.DUMMY[1])) }) It("Check if the lowest funding is returned correctly with coins which are not whitelisted", func() { - whitelist := []*funderstypes.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), funderstypes.NewParams([]*funderstypes.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -569,7 +568,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { CoinDenom: i.B_DENOM, CoinWeight: math.LegacyNewDec(2), }, - } + }, 20)) fundings := []funderstypes.Funding{ { @@ -592,7 +591,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { }, } - getLowestFunding, err := s.App().FundersKeeper.GetLowestFunding(fundings, whitelist) + getLowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), fundings) Expect(err).NotTo(HaveOccurred()) Expect(getLowestFunding.FunderAddress).To(Equal(i.DUMMY[2])) }) diff --git a/x/funders/keeper/msg_server_defund_pool_test.go b/x/funders/keeper/msg_server_defund_pool_test.go index 97dfce39..582da49d 100644 --- a/x/funders/keeper/msg_server_defund_pool_test.go +++ b/x/funders/keeper/msg_server_defund_pool_test.go @@ -214,7 +214,7 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { fundingState, _ := s.App().FundersKeeper.GetFundingState(s.Ctx(), 0) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.BOB)) @@ -228,7 +228,7 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { // ASSERT fundingState, _ = s.App().FundersKeeper.GetFundingState(s.Ctx(), 0) activeFundings = s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err = s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err = s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) @@ -304,7 +304,7 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { It("Try to partially defund after a coin has been removed from the whitelist", func() { // ARRANGE - whitelist = []*types.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams([]*types.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -323,8 +323,7 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { MinFundingAmountPerBundle: 1 * i.KYVE, CoinWeight: math.LegacyNewDec(3), }, - } - s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams(whitelist, 20)) + }, 20)) // ACT _, err := s.RunTx(&types.MsgDefundPool{ @@ -340,7 +339,7 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { It("Try to fully defund after a coin has been removed from the whitelist", func() { // ARRANGE - whitelist = []*types.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams([]*types.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -359,8 +358,7 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { MinFundingAmountPerBundle: 1 * i.KYVE, CoinWeight: math.LegacyNewDec(3), }, - } - s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams(whitelist, 20)) + }, 20)) // ACT s.RunTxFundersSuccess(&types.MsgDefundPool{ diff --git a/x/funders/keeper/msg_server_fund_pool_test.go b/x/funders/keeper/msg_server_fund_pool_test.go index dc262927..140fb58e 100644 --- a/x/funders/keeper/msg_server_fund_pool_test.go +++ b/x/funders/keeper/msg_server_fund_pool_test.go @@ -182,7 +182,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { Expect(fundingState.ActiveFunderAddresses[0]).To(Equal(i.ALICE)) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) }) @@ -222,7 +222,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { Expect(fundingState.ActiveFunderAddresses[0]).To(Equal(i.ALICE)) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) }) @@ -262,7 +262,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { Expect(fundingState.ActiveFunderAddresses[0]).To(Equal(i.ALICE)) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) }) @@ -302,7 +302,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { Expect(fundingState.ActiveFunderAddresses[0]).To(Equal(i.ALICE)) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) }) @@ -384,7 +384,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { Expect(len(fundingState.ActiveFunderAddresses)).To(Equal(2)) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.BOB)) }) @@ -420,7 +420,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { Expect(len(fundingState.ActiveFunderAddresses)).To(Equal(2)) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) }) @@ -672,7 +672,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) Expect(activeFundings).To(HaveLen(50)) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) }) @@ -718,7 +718,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) Expect(activeFundings).To(HaveLen(50)) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.BOB)) @@ -755,7 +755,7 @@ var _ = Describe("msg_server_fund_pool.go", Ordered, func() { fundingState, _ := s.App().FundersKeeper.GetFundingState(s.Ctx(), 0) activeFundings := s.App().FundersKeeper.GetActiveFundings(s.Ctx(), fundingState) Expect(activeFundings).To(HaveLen(50)) - lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(activeFundings, whitelist) + lowestFunding, err := s.App().FundersKeeper.GetLowestFunding(s.Ctx(), activeFundings) Expect(err).To(BeNil()) Expect(lowestFunding.FunderAddress).To(Equal(i.ALICE)) diff --git a/x/funders/spec/06_exported.md b/x/funders/spec/06_exported.md index 2364838e..8b0a83ca 100644 --- a/x/funders/spec/06_exported.md +++ b/x/funders/spec/06_exported.md @@ -13,7 +13,8 @@ type FundersKeeper interface { // If the amount is lower than the amount_per_bundle, // the max amount is charged and the funder is removed from the active funders list. // The amount is transferred from the funders to the recipient module account. - // If there are no more active funders, an event is emitted. + // If there are no more active funders, an event is emitted. This method only charges + // coins which are whitelisted. ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient string) (payout sdk.Coins, err error) } ``` diff --git a/x/funders/types/funders.go b/x/funders/types/funders.go index a85a8427..f8df0a21 100644 --- a/x/funders/types/funders.go +++ b/x/funders/types/funders.go @@ -2,15 +2,9 @@ package types import sdk "github.com/cosmos/cosmos-sdk/types" -func (f *Funding) GetScore(whitelist []*WhitelistCoinEntry) (score uint64) { - // create map for easier lookup - w := make(map[string]WhitelistCoinEntry) - for _, entry := range whitelist { - w[entry.CoinDenom] = *entry - } - +func (f *Funding) GetScore(whitelist map[string]WhitelistCoinEntry) (score uint64) { for _, coin := range f.Amounts { - if entry, found := w[coin.Denom]; found { + if entry, found := whitelist[coin.Denom]; found { score += uint64(entry.CoinWeight.MulInt64(coin.Amount.Int64()).TruncateInt64()) } } @@ -32,8 +26,16 @@ func (f *Funding) CleanAmountsPerBundle() { f.AmountsPerBundle = amountsPerBundle } -func (f *Funding) ChargeOneBundle() (payouts sdk.Coins) { - payouts = f.Amounts.Min(f.AmountsPerBundle) +func (f *Funding) ChargeOneBundle(whitelist map[string]WhitelistCoinEntry) (payouts sdk.Coins) { + chargable := f.Amounts.Min(f.AmountsPerBundle) + + // only charge coins which are whitelisted + for _, coin := range chargable { + if _, found := whitelist[coin.Denom]; found { + payouts = payouts.Add(coin) + } + } + f.TotalFunded = f.TotalFunded.Add(payouts...) f.Amounts = f.Amounts.Sub(payouts...) f.CleanAmountsPerBundle() diff --git a/x/funders/types/funders_test.go b/x/funders/types/funders_test.go index e766bd14..e79e4668 100644 --- a/x/funders/types/funders_test.go +++ b/x/funders/types/funders_test.go @@ -18,6 +18,7 @@ TEST CASES - funders.go * Funding.ChargeOneBundle - charge more than available * Funding.ChargeOneBundle - charge with multiple coins * Funding.ChargeOneBundle - charge with no coins +* Funding.ChargeOneBundle - charge coins which are not in the whitelist * Funding.CleanAmountsPerBundle - same coins are present in amounts and in amounts per bundle * Funding.CleanAmountsPerBundle - more coins are present in amounts per bundle than in amounts * Funding.CleanAmountsPerBundle - coins are present in amounts per bundle but not in amounts @@ -33,11 +34,10 @@ var _ = Describe("logic_funders.go", Ordered, func() { funding := types.Funding{} fundingState := types.FundingState{} - var whitelist []*types.WhitelistCoinEntry BeforeEach(func() { // set whitelist - whitelist = []*types.WhitelistCoinEntry{ + s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams([]*types.WhitelistCoinEntry{ { CoinDenom: globaltypes.Denom, MinFundingAmount: 10 * i.KYVE, @@ -62,8 +62,7 @@ var _ = Describe("logic_funders.go", Ordered, func() { MinFundingAmountPerBundle: 1 * i.KYVE, CoinWeight: math.LegacyNewDec(3), }, - } - s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams(whitelist, 20)) + }, 20)) funding = types.Funding{ FunderAddress: i.ALICE, @@ -79,8 +78,11 @@ var _ = Describe("logic_funders.go", Ordered, func() { }) It("Funding.ChargeOneBundle", func() { + // ARRANGE + whitelist := s.App().FundersKeeper.GetCoinWhitelistMap(s.Ctx()) + // ACT - payouts := funding.ChargeOneBundle() + payouts := funding.ChargeOneBundle(whitelist) // ASSERT Expect(payouts.String()).To(Equal(i.ACoins(1 * i.T_KYVE).String())) @@ -90,10 +92,11 @@ var _ = Describe("logic_funders.go", Ordered, func() { It("Funding.ChargeOneBundle - charge more than available", func() { // ARRANGE + whitelist := s.App().FundersKeeper.GetCoinWhitelistMap(s.Ctx()) funding.Amounts = i.ACoins(1 * i.T_KYVE / 2) // ACT - payouts := funding.ChargeOneBundle() + payouts := funding.ChargeOneBundle(whitelist) // ASSERT Expect(payouts.String()).To(Equal(i.ACoins(1 * i.T_KYVE / 2).String())) @@ -103,11 +106,12 @@ var _ = Describe("logic_funders.go", Ordered, func() { It("Funding.ChargeOneBundle - charge with multiple coins", func() { // ARRANGE + whitelist := s.App().FundersKeeper.GetCoinWhitelistMap(s.Ctx()) funding.Amounts = sdk.NewCoins(i.ACoin(100*i.T_KYVE), i.BCoin(100*i.T_KYVE)) funding.AmountsPerBundle = sdk.NewCoins(i.ACoin(1*i.T_KYVE), i.BCoin(2*i.T_KYVE)) // ACT - payouts := funding.ChargeOneBundle() + payouts := funding.ChargeOneBundle(whitelist) // ASSERT Expect(payouts.String()).To(Equal(sdk.NewCoins(i.ACoin(1*i.T_KYVE), i.BCoin(2*i.T_KYVE)).String())) @@ -117,11 +121,12 @@ var _ = Describe("logic_funders.go", Ordered, func() { It("Funding.ChargeOneBundle - charge with no coins", func() { // ARRANGE + whitelist := s.App().FundersKeeper.GetCoinWhitelistMap(s.Ctx()) funding.Amounts = sdk.NewCoins() funding.AmountsPerBundle = sdk.NewCoins() // ACT - payouts := funding.ChargeOneBundle() + payouts := funding.ChargeOneBundle(whitelist) // ASSERT Expect(payouts.IsZero()).To(BeTrue()) @@ -129,6 +134,35 @@ var _ = Describe("logic_funders.go", Ordered, func() { Expect(funding.TotalFunded.IsZero()).To(BeTrue()) }) + It("Funding.ChargeOneBundle - charge with coins which are not in the whitelist", func() { + // ARRANGE + s.App().FundersKeeper.SetParams(s.Ctx(), types.NewParams([]*types.WhitelistCoinEntry{ + { + CoinDenom: globaltypes.Denom, + MinFundingAmount: 10 * i.KYVE, + MinFundingAmountPerBundle: 1 * i.KYVE, + CoinWeight: math.LegacyNewDec(1), + }, + { + CoinDenom: i.A_DENOM, + MinFundingAmount: 10 * i.KYVE, + MinFundingAmountPerBundle: 1 * i.KYVE, + CoinWeight: math.LegacyNewDec(1), + }, + }, 20)) + whitelist := s.App().FundersKeeper.GetCoinWhitelistMap(s.Ctx()) + funding.Amounts = sdk.NewCoins(i.ACoin(100*i.T_KYVE), i.BCoin(100*i.T_KYVE), i.CCoin(100*i.T_KYVE)) + funding.AmountsPerBundle = sdk.NewCoins(i.ACoin(1*i.T_KYVE), i.BCoin(1*i.T_KYVE), i.CCoin(1*i.T_KYVE)) + + // ACT + payouts := funding.ChargeOneBundle(whitelist) + + // ASSERT + Expect(payouts.String()).To(Equal(i.ACoins(1 * i.T_KYVE).String())) + Expect(funding.Amounts.String()).To(Equal(sdk.NewCoins(i.ACoin(99*i.T_KYVE), i.BCoin(100*i.T_KYVE), i.CCoin(100*i.T_KYVE)).String())) + Expect(funding.TotalFunded.String()).To(Equal(i.ACoins(1 * i.T_KYVE).String())) + }) + It("Funding.CleanAmountsPerBundle - same coins are present in amounts and in amounts per bundle", func() { // ARRANGE funding.Amounts = sdk.NewCoins(i.ACoin(100*i.T_KYVE), i.BCoin(80*i.T_KYVE)) diff --git a/x/pool/spec/07_exported.md b/x/pool/spec/07_exported.md index 3ea145aa..ea40ad68 100644 --- a/x/pool/spec/07_exported.md +++ b/x/pool/spec/07_exported.md @@ -29,6 +29,7 @@ type PoolKeeper interface { // the appropriate amount from each funder. // All funders who can't afford the amount, are kicked out. // The method returns the payout amount the pool was able to charge from the funders. + // This method only charges coins which are whitelisted. ChargeFundersOfPool(ctx sdk.Context, poolId uint64, amount uint64, recipient string) (payout uint64, err error) } ``` diff --git a/x/query/keeper/grpc_query_funders.go b/x/query/keeper/grpc_query_funders.go index 968fb851..5ca2a774 100644 --- a/x/query/keeper/grpc_query_funders.go +++ b/x/query/keeper/grpc_query_funders.go @@ -23,12 +23,12 @@ func (k Keeper) Funders(c context.Context, req *types.QueryFundersRequest) (*typ return nil, err } - params := k.fundersKeeper.GetParams(ctx) + whitelist := k.fundersKeeper.GetCoinWhitelistMap(ctx) data := make([]types.Funder, 0) for _, funder := range funders { fundings := k.fundersKeeper.GetFundingsOfFunder(ctx, funder.Address) - data = append(data, k.parseFunder(&funder, fundings, params.CoinWhitelist)) + data = append(data, k.parseFunder(&funder, fundings, whitelist)) } return &types.QueryFundersResponse{Funders: data, Pagination: pageRes}, nil @@ -48,9 +48,9 @@ func (k Keeper) Funder(c context.Context, req *types.QueryFunderRequest) (*types allFundings := k.fundersKeeper.GetFundingsOfFunder(ctx, funder.Address) fundings := k.filterFundingsOnStatus(allFundings, req.Status) - params := k.fundersKeeper.GetParams(ctx) - funderData := k.parseFunder(&funder, allFundings, params.CoinWhitelist) - fundingsData := k.parseFundings(fundings, params.CoinWhitelist) + whitelist := k.fundersKeeper.GetCoinWhitelistMap(ctx) + funderData := k.parseFunder(&funder, allFundings, whitelist) + fundingsData := k.parseFundings(fundings, whitelist) return &types.QueryFunderResponse{ Funder: &funderData, @@ -75,7 +75,7 @@ func (k Keeper) filterFundingsOnStatus(fundings []fundersTypes.Funding, fundingS return filtered } -func (k Keeper) parseFunder(funder *fundersTypes.Funder, fundings []fundersTypes.Funding, whitelist []*fundersTypes.WhitelistCoinEntry) types.Funder { +func (k Keeper) parseFunder(funder *fundersTypes.Funder, fundings []fundersTypes.Funding, whitelist map[string]fundersTypes.WhitelistCoinEntry) types.Funder { stats := types.FundingStats{ TotalUsedFunds: sdk.NewCoins(), TotalAllocatedFunds: sdk.NewCoins(), diff --git a/x/query/keeper/grpc_query_fundings.go b/x/query/keeper/grpc_query_fundings.go index 9a3d40af..b0617eb2 100644 --- a/x/query/keeper/grpc_query_fundings.go +++ b/x/query/keeper/grpc_query_fundings.go @@ -21,8 +21,8 @@ func (k Keeper) FundingsByFunder(c context.Context, req *types.QueryFundingsByFu return nil, err } - params := k.fundersKeeper.GetParams(ctx) - data := k.parseFundings(fundings, params.CoinWhitelist) + whitelist := k.fundersKeeper.GetCoinWhitelistMap(ctx) + data := k.parseFundings(fundings, whitelist) return &types.QueryFundingsByFunderResponse{Fundings: data, Pagination: pageRes}, nil } @@ -38,13 +38,13 @@ func (k Keeper) FundingsByPool(c context.Context, req *types.QueryFundingsByPool return nil, err } - params := k.fundersKeeper.GetParams(ctx) - data := k.parseFundings(fundings, params.CoinWhitelist) + whitelist := k.fundersKeeper.GetCoinWhitelistMap(ctx) + data := k.parseFundings(fundings, whitelist) return &types.QueryFundingsByPoolResponse{Fundings: data, Pagination: pageRes}, nil } -func (k Keeper) parseFundings(fundings []fundersTypes.Funding, whitelist []*fundersTypes.WhitelistCoinEntry) []types.Funding { +func (k Keeper) parseFundings(fundings []fundersTypes.Funding, whitelist map[string]fundersTypes.WhitelistCoinEntry) []types.Funding { fundingsData := make([]types.Funding, 0) for _, funding := range fundings { fundingsData = append(fundingsData, types.Funding{ From 1a71d2e5881cdc2709f1e85cf57dfbef10fd2de6 Mon Sep 17 00:00:00 2001 From: Troy Kessler Date: Wed, 8 May 2024 14:17:41 +0200 Subject: [PATCH 2/2] chore: implemented feedback --- x/funders/keeper/logic_funders.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/x/funders/keeper/logic_funders.go b/x/funders/keeper/logic_funders.go index 6390ef27..9f023f5b 100644 --- a/x/funders/keeper/logic_funders.go +++ b/x/funders/keeper/logic_funders.go @@ -61,20 +61,21 @@ func (k Keeper) GetCoinWhitelistMap(ctx sdk.Context) (whitelist map[string]types // The amount is transferred from the funders to the recipient module account. // If there are no more active funders, an event is emitted. This method only charges // coins which are whitelisted. -func (k Keeper) ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient string) (payouts sdk.Coins, err error) { +func (k Keeper) ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient string) (sdk.Coins, error) { // Get funding state for pool fundingState, found := k.GetFundingState(ctx, poolId) if !found { - return payouts, errors.Wrapf(errorsTypes.ErrNotFound, types.ErrFundingStateDoesNotExist.Error(), poolId) + return sdk.NewCoins(), errors.Wrapf(errorsTypes.ErrNotFound, types.ErrFundingStateDoesNotExist.Error(), poolId) } // If there are no active fundings we immediately return activeFundings := k.GetActiveFundings(ctx, fundingState) if len(activeFundings) == 0 { - return + return sdk.NewCoins(), nil } whitelist := k.GetCoinWhitelistMap(ctx) + payouts := sdk.NewCoins() // Charge every active funder and collect payouts for _, funding := range activeFundings { @@ -96,12 +97,15 @@ func (k Keeper) ChargeFundersOfPool(ctx sdk.Context, poolId uint64, recipient st } if payouts.IsZero() { - return + return payouts, nil } // Move funds to recipient module - err = k.bankKeeper.SendCoinsFromModuleToModule(ctx, types.ModuleName, recipient, payouts) - return + if err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, types.ModuleName, recipient, payouts); err != nil { + return sdk.NewCoins(), err + } + + return payouts, nil } // GetLowestFunding returns the funding with the lowest amount @@ -195,8 +199,6 @@ func (k Keeper) ensureFreeSlot(ctx sdk.Context, newFunding *types.Funding, fundi return nil } - whitelist := k.GetCoinWhitelistMap(ctx) - lowestFunding, err := k.GetLowestFunding(ctx, activeFundings) if err != nil { return err @@ -207,6 +209,8 @@ func (k Keeper) ensureFreeSlot(ctx sdk.Context, newFunding *types.Funding, fundi return nil } + whitelist := k.GetCoinWhitelistMap(ctx) + // Check if lowest funding is lower than new funding based on amount (amount per bundle is ignored) if newFunding.GetScore(whitelist) < lowestFunding.GetScore(whitelist) { return errors.Wrapf(errorsTypes.ErrLogic, types.ErrFundsTooLow.Error(), lowestFunding.GetScore(whitelist))