From 07dd3b98bd568ab82c385c932e7bff923482f0b6 Mon Sep 17 00:00:00 2001 From: mbreithecker Date: Fri, 27 Oct 2023 11:48:48 +0200 Subject: [PATCH] chore: add params compatibility check for defund --- x/funders/keeper/logic_funders.go | 64 +++++++++++++++++++ x/funders/keeper/msg_server_defund_pool.go | 8 ++- .../keeper/msg_server_defund_pool_test.go | 15 ++++- x/funders/keeper/msg_server_fund_pool.go | 63 ++---------------- 4 files changed, 87 insertions(+), 63 deletions(-) diff --git a/x/funders/keeper/logic_funders.go b/x/funders/keeper/logic_funders.go index 705db393..7e850d84 100644 --- a/x/funders/keeper/logic_funders.go +++ b/x/funders/keeper/logic_funders.go @@ -97,3 +97,67 @@ func (k Keeper) GetLowestFunding(fundings []types.Funding) (lowestFunding *types } return &fundings[lowestFundingIndex], nil } + +// ensureParamsCompatibility checks compatibility of the provided funding with the pool params. +// i.e. minimum funding per bundle +// +// and minimum funding amount +func (k Keeper) ensureParamsCompatibility(ctx sdk.Context, funding types.Funding) error { + + params := k.GetParams(ctx) + if funding.AmountPerBundle < params.MinFundingAmountPerBundle { + return errors.Wrapf(errorsTypes.ErrInvalidRequest, types.ErrAmountPerBundleTooLow.Error(), params.MinFundingAmountPerBundle) + } + if funding.Amount < params.MinFundingAmount { + return errors.Wrapf(errorsTypes.ErrInvalidRequest, types.ErrMinFundingAmount.Error(), params.MinFundingAmount) + } + return nil +} + +// ensureFreeSlot makes sure that a funder can add funding to a given pool. +// If this is not possible an appropriate error is returned. +// A pool has a fixed amount of funding-slots. If there are still free slots +// a funder can just join (even with the smallest funding possible). +// If all slots are taken, it checks if the new funding has more funds +// than the current lowest funding in that pool. +// If so, the lowest funding gets removed from the pool, so that the +// new funding can be added. +// CONTRACT: no KV Writing on newFunding and fundingState +func (k Keeper) ensureFreeSlot(ctx sdk.Context, newFunding *types.Funding, fundingState *types.FundingState) error { + + activeFundings := k.GetActiveFundings(ctx, *fundingState) + // check if slots are still available + if len(activeFundings) < types.MaxFunders { + return nil + } + + lowestFunding, _ := k.GetLowestFunding(activeFundings) + + if lowestFunding.FunderAddress == newFunding.FunderAddress { + // Funder already has a funding slot + return nil + } + + // Check if lowest funding is lower than new funding based on amount (amount per bundle is ignored) + if newFunding.Amount < lowestFunding.Amount { + return errors.Wrapf(errorsTypes.ErrLogic, types.ErrFundsTooLow.Error(), lowestFunding.Amount) + } + + // Defund lowest funder + if err := util.TransferFromModuleToAddress(k.bankKeeper, ctx, types.ModuleName, lowestFunding.FunderAddress, lowestFunding.Amount); err != nil { + return err + } + + subtracted := lowestFunding.SubtractAmount(lowestFunding.Amount) + fundingState.SetInactive(lowestFunding) + k.SetFunding(ctx, lowestFunding) + + // Emit a defund event. + _ = ctx.EventManager().EmitTypedEvent(&types.EventDefundPool{ + PoolId: fundingState.PoolId, + Address: lowestFunding.FunderAddress, + Amount: subtracted, + }) + + return nil +} diff --git a/x/funders/keeper/msg_server_defund_pool.go b/x/funders/keeper/msg_server_defund_pool.go index c2b413c2..1ad2c10c 100644 --- a/x/funders/keeper/msg_server_defund_pool.go +++ b/x/funders/keeper/msg_server_defund_pool.go @@ -12,9 +12,6 @@ import ( errorsTypes "github.com/cosmos/cosmos-sdk/types/errors" ) -// TODO: Right now it is possible to fund a pool above the minimum funding amount and then defund it below the minimum funding amount. -// This should not be possible. We should probably add a check for this. - // DefundPool handles the logic to defund a pool. // If the user is a funder, it will subtract the provided amount // and send the tokens back. If there are no more funds left, the funding will get inactive. @@ -41,6 +38,11 @@ func (k msgServer) DefundPool(goCtx context.Context, msg *types.MsgDefundPool) ( amount := funding.SubtractAmount(msg.Amount) if funding.Amount == 0 { fundingState.SetInactive(&funding) + } else { + // If funding is not fully revoked, check if updated funding is still compatible with params. + if err := k.ensureParamsCompatibility(ctx, funding); err != nil { + return nil, err + } } // Transfer tokens from this module to sender. diff --git a/x/funders/keeper/msg_server_defund_pool_test.go b/x/funders/keeper/msg_server_defund_pool_test.go index 9bc5a204..4d738442 100644 --- a/x/funders/keeper/msg_server_defund_pool_test.go +++ b/x/funders/keeper/msg_server_defund_pool_test.go @@ -15,9 +15,10 @@ TEST CASES - msg_server_defund_pool.go * Defund 50 KYVE from a funder who has previously funded 100 KYVE * Defund more than actually funded * Defund full funding amount from a funder who has previously funded 100 KYVE -* Defund as highest funder 75 KYVE in order to be the lowest funder afterwards +* Defund as highest funder 75 KYVE in order to be the lowest funder afterward * Try to defund nonexistent fundings * Try to defund a funding twice +* Try to defund below minimum funding params (but not full defund) */ @@ -198,4 +199,16 @@ var _ = Describe("msg_server_defund_pool.go", Ordered, func() { Amount: 100 * i.KYVE, }) }) + + It("Try to defund below minimum funding params (but not full defund)", func() { + // ACT + _, err := s.RunTx(&types.MsgDefundPool{ + Creator: i.ALICE, + PoolId: 0, + Amount: 100*i.KYVE - types.DefaultMinFundingAmount/2, + }) + + // ASSERT + Expect(err.Error()).To(Equal("minimum funding amount of 1000000000kyve not reached: invalid request")) + }) }) diff --git a/x/funders/keeper/msg_server_fund_pool.go b/x/funders/keeper/msg_server_fund_pool.go index edfbac11..82a7736f 100644 --- a/x/funders/keeper/msg_server_fund_pool.go +++ b/x/funders/keeper/msg_server_fund_pool.go @@ -10,54 +10,6 @@ import ( errorsTypes "github.com/cosmos/cosmos-sdk/types/errors" ) -// ensureFreeSlot makes sure that a funder can add funding to a given pool. -// If this is not possible an appropriate error is returned. -// A pool has a fixed amount of funding-slots. If there are still free slots -// a funder can just join (even with the smallest funding possible). -// If all slots are taken, it checks if the new funding has more funds -// than the current lowest funding in that pool. -// If so, the lowest funding gets removed from the pool, so that the -// new funding can be added. -// CONTRACT: no KV Writing on newFunding and fundingState -func (k Keeper) ensureFreeSlot(ctx sdk.Context, newFunding *types.Funding, fundingState *types.FundingState) error { - - activeFundings := k.GetActiveFundings(ctx, *fundingState) - // check if slots are still available - if len(activeFundings) < types.MaxFunders { - return nil - } - - lowestFunding, _ := k.GetLowestFunding(activeFundings) - - if lowestFunding.FunderAddress == newFunding.FunderAddress { - // Funder already has a funding slot - return nil - } - - // Check if lowest funding is lower than new funding based on amount (amount per bundle is ignored) - if newFunding.Amount < lowestFunding.Amount { - return errors.Wrapf(errorsTypes.ErrLogic, types.ErrFundsTooLow.Error(), lowestFunding.Amount) - } - - // Defund lowest funder - if err := util.TransferFromModuleToAddress(k.bankKeeper, ctx, types.ModuleName, lowestFunding.FunderAddress, lowestFunding.Amount); err != nil { - return err - } - - subtracted := lowestFunding.SubtractAmount(lowestFunding.Amount) - fundingState.SetInactive(lowestFunding) - k.SetFunding(ctx, lowestFunding) - - // Emit a defund event. - _ = ctx.EventManager().EmitTypedEvent(&types.EventDefundPool{ - PoolId: fundingState.PoolId, - Address: lowestFunding.FunderAddress, - Amount: subtracted, - }) - - return nil -} - // FundPool handles the logic to fund a pool. // A funder is added to the active funders list with the specified amount // If the funders list is full, it checks if the funder wants to fund @@ -103,15 +55,9 @@ func (k msgServer) FundPool(goCtx context.Context, msg *types.MsgFundPool) (*typ } } - // Check compatibility of updated funding with params - // i.e minimum funding per bundle - // and minimum funding amount - params := k.GetParams(ctx) - if funding.AmountPerBundle < params.MinFundingAmountPerBundle { - return nil, errors.Wrapf(errorsTypes.ErrInvalidRequest, types.ErrAmountPerBundleTooLow.Error(), params.MinFundingAmountPerBundle) - } - if funding.Amount < params.MinFundingAmount { - return nil, errors.Wrapf(errorsTypes.ErrInvalidRequest, types.ErrMinFundingAmount.Error(), params.MinFundingAmount) + // Check if updated (or new) funding is compatible with module params + if err := k.ensureParamsCompatibility(ctx, funding); err != nil { + return nil, err } // Kicks out lowest funder if all slots are taken and new funder is about to fund more. @@ -121,8 +67,7 @@ func (k msgServer) FundPool(goCtx context.Context, msg *types.MsgFundPool) (*typ return nil, err } - // User is allowed to fund - // Let's see if he has enough funds + // All checks passed, transfer funds from funder to module if err := util.TransferFromAddressToModule(k.bankKeeper, ctx, msg.Creator, types.ModuleName, msg.Amount); err != nil { return nil, err }