From cdbc67717bdeabb39d9d61b47b0b172989335905 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 11:49:36 -0700 Subject: [PATCH 01/27] feat(incentives)!: create gauge and add to gauge fee charge --- app/keepers/keepers.go | 2 ++ x/incentives/keeper/export_test.go | 5 ++- x/incentives/keeper/gauge.go | 24 +++++++++++++++ x/incentives/keeper/gauge_test.go | 42 +++++++++++++++++++++++++- x/incentives/keeper/keeper.go | 6 +++- x/incentives/types/expected_keepers.go | 10 ++++++ 6 files changed, 86 insertions(+), 3 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index aeecb95a0e5..4845e5fe035 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -267,6 +267,8 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appKeepers.BankKeeper, appKeepers.LockupKeeper, appKeepers.EpochsKeeper, + appKeepers.DistrKeeper, + appKeepers.TxFeesKeeper, ) appKeepers.SuperfluidKeeper = superfluidkeeper.NewKeeper( diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index ac465da4fe9..bae1f28ef50 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -11,7 +11,6 @@ func (k Keeper) AddGaugeRefByKey(ctx sdk.Context, key []byte, guageID uint64) er return k.addGaugeRefByKey(ctx, key, guageID) } -// DeleteGaugeRefByKey removes the provided gauge ID from an array associated with the provided key. func (k Keeper) DeleteGaugeRefByKey(ctx sdk.Context, key []byte, guageID uint64) error { return k.deleteGaugeRefByKey(ctx, key, guageID) } @@ -35,3 +34,7 @@ func (k Keeper) MoveUpcomingGaugeToActiveGauge(ctx sdk.Context, gauge types.Gaug func (k Keeper) MoveActiveGaugeToFinishedGauge(ctx sdk.Context, gauge types.Gauge) error { return k.moveActiveGaugeToFinishedGauge(ctx, gauge) } + +func (k Keeper) ChargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64) error { + return k.chargeFee(ctx, address, fee) +} diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index 412ef436f69..f924705f076 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -17,6 +17,11 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +const ( + createGaugeMinBaseFee = 50 * 1_000_000 + addToGaugeMinBaseFee = 25 * 1_000_000 +) + // getGaugesFromIterator iterates over everything in a gauge's iterator, until it reaches the end. Return all gauges iterated over. func (k Keeper) getGaugesFromIterator(ctx sdk.Context, iterator db.Iterator) []types.Gauge { gauges := []types.Gauge{} @@ -91,6 +96,9 @@ func (k Keeper) SetGaugeWithRefKey(ctx sdk.Context, gauge *types.Gauge) error { // CreateGauge creates a gauge and sends coins to the gauge. func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddress, coins sdk.Coins, distrTo lockuptypes.QueryCondition, startTime time.Time, numEpochsPaidOver uint64) (uint64, error) { + if err := k.chargeFee(ctx, owner, createGaugeMinBaseFee); err != nil { + return 0, err + } // Ensure that this gauge's duration is one of the allowed durations on chain durations := k.GetLockableDurations(ctx) if distrTo.LockQueryType == lockuptypes.ByDuration { @@ -143,6 +151,9 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr // AddToGaugeRewards adds coins to gauge. func (k Keeper) AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins sdk.Coins, gaugeID uint64) error { + if err := k.chargeFee(ctx, owner, addToGaugeMinBaseFee); err != nil { + return err + } gauge, err := k.GetGaugeByID(ctx, gaugeID) if err != nil { return err @@ -278,3 +289,16 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { params := k.GetParams(ctx) return k.ek.GetEpochInfo(ctx, params.DistrEpochIdentifier) } + +func (k Keeper) chargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64) (err error) { + // Send creation fee to community pool + feeDenom, err := k.txfk.GetBaseDenom(ctx) + if err != nil { + return err + } + creationFee := sdk.NewCoins(sdk.NewCoin(feeDenom, sdk.NewInt(fee))) + if err := k.dk.FundCommunityPool(ctx, creationFee, address); err != nil { + return err + } + return nil +} diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 45579973715..24a21360247 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "testing" "time" "github.com/stretchr/testify/suite" @@ -11,7 +12,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -var _ = suite.TestingSuite(nil) +func TestGaugeTestSuite(t *testing.T) { + suite.Run(t, new(KeeperTestSuite)) +} // TestInvalidDurationGaugeCreationValidation tests error handling for creating a gauge with an invalid duration. func (suite *KeeperTestSuite) TestInvalidDurationGaugeCreationValidation() { @@ -237,3 +240,40 @@ func (suite *KeeperTestSuite) TestGaugeOperations() { } } } + +// func (suite *KeeperTestSuite) TestChargeFee() { + +// testcases := map[string]struct { +// accountBalanceToFund sdk.Coin +// feeToCharge int64 + +// expectErr bool +// }{ +// "charge fee": { +// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), +// feeToCharge: 100, +// }, +// } + +// for name, tc := range testcases { +// suite.Run(name, func() { +// suite.SetupTest() + +// testAccount := suite.TestAccs[0] + +// ctx := suite.Ctx +// incentivesKeepers := suite.App.IncentivesKeeper + +// suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) + +// // System under test. +// err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge) + +// if tc.expectErr { +// suite.Require().Error(err) +// } else { +// suite.Require().NoError(err) +// } +// }) +// } +// } diff --git a/x/incentives/keeper/keeper.go b/x/incentives/keeper/keeper.go index 09c80d185a5..c9d12b4f89c 100644 --- a/x/incentives/keeper/keeper.go +++ b/x/incentives/keeper/keeper.go @@ -22,10 +22,12 @@ type Keeper struct { bk types.BankKeeper lk types.LockupKeeper ek types.EpochKeeper + dk types.DistrKeeper + txfk types.TxFeesKeeper } // NewKeeper returns a new instance of the incentive module keeper struct. -func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper) *Keeper { +func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper, txfk types.TxFeesKeeper) *Keeper { if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) } @@ -37,6 +39,8 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub bk: bk, lk: lk, ek: ek, + dk: dk, + txfk: txfk, } } diff --git a/x/incentives/types/expected_keepers.go b/x/incentives/types/expected_keepers.go index e377908df79..2098c3d0959 100644 --- a/x/incentives/types/expected_keepers.go +++ b/x/incentives/types/expected_keepers.go @@ -36,3 +36,13 @@ type LockupKeeper interface { type EpochKeeper interface { GetEpochInfo(ctx sdk.Context, identifier string) epochstypes.EpochInfo } + +// DistrKeeper defines the contract needed to be fulfilled for distribution keeper. +type DistrKeeper interface { + FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error +} + +// TxFeesKeeper defines the expected interface needed to managing transaction fees. +type TxFeesKeeper interface { + GetBaseDenom(ctx sdk.Context) (denom string, err error) +} From 2895645c61eb69385ce597f0375cb3d9f5e3d49b Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 12:07:00 -0700 Subject: [PATCH 02/27] initialize txfees keeper before incentives --- app/keepers/keepers.go | 26 +++++------ x/incentives/keeper/gauge_test.go | 72 +++++++++++++++---------------- 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 4845e5fe035..b88217fde6b 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -260,6 +260,19 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appKeepers.EpochsKeeper = epochskeeper.NewKeeper(appCodec, appKeepers.keys[epochstypes.StoreKey]) + txFeesKeeper := txfeeskeeper.NewKeeper( + appCodec, + appKeepers.AccountKeeper, + appKeepers.BankKeeper, + appKeepers.EpochsKeeper, + appKeepers.keys[txfeestypes.StoreKey], + appKeepers.GAMMKeeper, + appKeepers.GAMMKeeper, + txfeestypes.FeeCollectorName, + txfeestypes.NonNativeFeeCollectorName, + ) + appKeepers.TxFeesKeeper = &txFeesKeeper + appKeepers.IncentivesKeeper = incentiveskeeper.NewKeeper( appCodec, appKeepers.keys[incentivestypes.StoreKey], @@ -301,19 +314,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers( ) appKeepers.PoolIncentivesKeeper = &poolIncentivesKeeper - txFeesKeeper := txfeeskeeper.NewKeeper( - appCodec, - appKeepers.AccountKeeper, - appKeepers.BankKeeper, - appKeepers.EpochsKeeper, - appKeepers.keys[txfeestypes.StoreKey], - appKeepers.GAMMKeeper, - appKeepers.GAMMKeeper, - txfeestypes.FeeCollectorName, - txfeestypes.NonNativeFeeCollectorName, - ) - appKeepers.TxFeesKeeper = &txFeesKeeper - tokenFactoryKeeper := tokenfactorykeeper.NewKeeper( appCodec, appKeepers.keys[tokenfactorytypes.StoreKey], diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 24a21360247..fd7a3086cd9 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -241,39 +241,39 @@ func (suite *KeeperTestSuite) TestGaugeOperations() { } } -// func (suite *KeeperTestSuite) TestChargeFee() { - -// testcases := map[string]struct { -// accountBalanceToFund sdk.Coin -// feeToCharge int64 - -// expectErr bool -// }{ -// "charge fee": { -// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), -// feeToCharge: 100, -// }, -// } - -// for name, tc := range testcases { -// suite.Run(name, func() { -// suite.SetupTest() - -// testAccount := suite.TestAccs[0] - -// ctx := suite.Ctx -// incentivesKeepers := suite.App.IncentivesKeeper - -// suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) - -// // System under test. -// err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge) - -// if tc.expectErr { -// suite.Require().Error(err) -// } else { -// suite.Require().NoError(err) -// } -// }) -// } -// } +func (suite *KeeperTestSuite) TestChargeFee() { + + testcases := map[string]struct { + accountBalanceToFund sdk.Coin + feeToCharge int64 + + expectErr bool + }{ + "charge fee": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), + feeToCharge: 100, + }, + } + + for name, tc := range testcases { + suite.Run(name, func() { + suite.SetupTest() + + testAccount := suite.TestAccs[0] + + ctx := suite.Ctx + incentivesKeepers := suite.App.IncentivesKeeper + + suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) + + // System under test. + err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge) + + if tc.expectErr { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + } + }) + } +} From 82ae999d311277f54ce4722b0cc7aebf278224c7 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 12:13:22 -0700 Subject: [PATCH 03/27] finish TestChargeFee --- x/incentives/keeper/gauge_test.go | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index fd7a3086cd9..18984dc2c3a 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -242,16 +242,37 @@ func (suite *KeeperTestSuite) TestGaugeOperations() { } func (suite *KeeperTestSuite) TestChargeFee() { + const baseFee = int64(100) testcases := map[string]struct { accountBalanceToFund sdk.Coin feeToCharge int64 - expectErr bool + expectError bool }{ - "charge fee": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), - feeToCharge: 100, + "charge fee, fee == acount balance, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + feeToCharge: baseFee, + }, + "charge fee - fee < acount balance, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + feeToCharge: baseFee - 1, + }, + "charge fee - fee > acount balance, error": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + feeToCharge: baseFee + 1, + + expectError: true, + }, + "charge fee - fee < acount balance, custom values, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), + feeToCharge: 55, + }, + "charge fee, account funded with coins other than base denom, error": { + accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), + feeToCharge: baseFee, + + expectError: true, }, } @@ -269,7 +290,7 @@ func (suite *KeeperTestSuite) TestChargeFee() { // System under test. err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge) - if tc.expectErr { + if tc.expectError { suite.Require().Error(err) } else { suite.Require().NoError(err) From bec30235a4e16f3e7c09abaf6e1ec218f9d77d5f Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 14:05:22 -0700 Subject: [PATCH 04/27] refactor to charge fee in message server --- app/keepers/keepers.go | 1 + x/incentives/keeper/export_test.go | 4 +- x/incentives/keeper/gauge.go | 29 +++++---- x/incentives/keeper/gauge_test.go | 83 +++++++++++++++++++++++--- x/incentives/keeper/keeper.go | 4 +- x/incentives/keeper/msg_server.go | 8 +++ x/incentives/types/expected_keepers.go | 6 ++ 7 files changed, 113 insertions(+), 22 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index b88217fde6b..4f84ee735c9 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -277,6 +277,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appCodec, appKeepers.keys[incentivestypes.StoreKey], appKeepers.GetSubspace(incentivestypes.ModuleName), + appKeepers.AccountKeeper, appKeepers.BankKeeper, appKeepers.LockupKeeper, appKeepers.EpochsKeeper, diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index bae1f28ef50..e31366c69e0 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -35,6 +35,6 @@ func (k Keeper) MoveActiveGaugeToFinishedGauge(ctx sdk.Context, gauge types.Gaug return k.moveActiveGaugeToFinishedGauge(ctx, gauge) } -func (k Keeper) ChargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64) error { - return k.chargeFee(ctx, address, fee) +func (k Keeper) ChargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) error { + return k.chargeFee(ctx, address, fee, gaugeCoins) } diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index f924705f076..094912fa5fa 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -7,6 +7,7 @@ import ( "strings" "time" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/gogo/protobuf/proto" db "github.com/tendermint/tm-db" @@ -18,8 +19,10 @@ import ( ) const ( - createGaugeMinBaseFee = 50 * 1_000_000 - addToGaugeMinBaseFee = 25 * 1_000_000 + // CreateGaugeFee is the fee required to create a new gauge. + createGaugeFee = 50 * 1_000_000 + // AddToGagugeFee is the fee required to add to gauge. + addToGaugeFee = 25 * 1_000_000 ) // getGaugesFromIterator iterates over everything in a gauge's iterator, until it reaches the end. Return all gauges iterated over. @@ -96,9 +99,6 @@ func (k Keeper) SetGaugeWithRefKey(ctx sdk.Context, gauge *types.Gauge) error { // CreateGauge creates a gauge and sends coins to the gauge. func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddress, coins sdk.Coins, distrTo lockuptypes.QueryCondition, startTime time.Time, numEpochsPaidOver uint64) (uint64, error) { - if err := k.chargeFee(ctx, owner, createGaugeMinBaseFee); err != nil { - return 0, err - } // Ensure that this gauge's duration is one of the allowed durations on chain durations := k.GetLockableDurations(ctx) if distrTo.LockQueryType == lockuptypes.ByDuration { @@ -151,9 +151,9 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr // AddToGaugeRewards adds coins to gauge. func (k Keeper) AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins sdk.Coins, gaugeID uint64) error { - if err := k.chargeFee(ctx, owner, addToGaugeMinBaseFee); err != nil { - return err - } + // if err := k.chargeFee(ctx, owner, types.AddToGaugeFee); err != nil { + // return err + // } gauge, err := k.GetGaugeByID(ctx, gaugeID) if err != nil { return err @@ -290,14 +290,21 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { return k.ek.GetEpochInfo(ctx, params.DistrEpochIdentifier) } -func (k Keeper) chargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64) (err error) { +func (k Keeper) chargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) (err error) { // Send creation fee to community pool feeDenom, err := k.txfk.GetBaseDenom(ctx) if err != nil { return err } - creationFee := sdk.NewCoins(sdk.NewCoin(feeDenom, sdk.NewInt(fee))) - if err := k.dk.FundCommunityPool(ctx, creationFee, address); err != nil { + feeInt := sdk.NewInt(fee) + + totalCost := gaugeCoins.AmountOf(feeDenom).Add(feeInt) + accountBalance := k.bk.GetBalance(ctx, address, feeDenom).Amount + if accountBalance.LT(totalCost) { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "account's balance of %s (%s) is less than the total cost of the message (%s)", feeDenom, accountBalance, totalCost) + } + + if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(feeDenom, feeInt)), address); err != nil { return err } return nil diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 18984dc2c3a..7007b391091 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -247,30 +247,36 @@ func (suite *KeeperTestSuite) TestChargeFee() { testcases := map[string]struct { accountBalanceToFund sdk.Coin feeToCharge int64 + gaugeCoins sdk.Coins expectError bool }{ - "charge fee, fee == acount balance, success": { + "charge fee, fee + base denom gauge coin == acount balance, success": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee, + feeToCharge: baseFee / 2, + gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, - "charge fee - fee < acount balance, success": { + "charge fee - fee + base denom gauge coin < acount balance, success": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee - 1, + feeToCharge: baseFee/2 - 1, + gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, - "charge fee - fee > acount balance, error": { + "charge fee - fee + base denom gauge coin > acount balance, error": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee + 1, + feeToCharge: baseFee/2 + 1, + gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), expectError: true, }, - "charge fee - fee < acount balance, custom values, success": { + "charge fee - fee + base denom gauge coin < acount balance, custom values, success": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), feeToCharge: 55, + gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(328812))), }, "charge fee, account funded with coins other than base denom, error": { accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), feeToCharge: baseFee, + gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), expectError: true, }, @@ -285,10 +291,11 @@ func (suite *KeeperTestSuite) TestChargeFee() { ctx := suite.Ctx incentivesKeepers := suite.App.IncentivesKeeper + // Pre-fund account. suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) // System under test. - err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge) + err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) if tc.expectError { suite.Require().Error(err) @@ -298,3 +305,63 @@ func (suite *KeeperTestSuite) TestChargeFee() { }) } } + +// func (suite *KeeperTestSuite) TestAddToGauge_ChargeFee() { +// const baseFee = int64(100) + +// testcases := map[string]struct { +// accountBalanceToFund sdk.Coin +// feeToCharge int64 + +// expectError bool +// }{ +// "charge fee, fee == acount balance, success": { +// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), +// feeToCharge: baseFee, +// }, +// "charge fee - fee < acount balance, success": { +// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), +// feeToCharge: baseFee - 1, +// }, +// "charge fee - fee > acount balance, error": { +// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), +// feeToCharge: baseFee + 1, + +// expectError: true, +// }, +// "charge fee - fee < acount balance, custom values, success": { +// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), +// feeToCharge: 55, +// }, +// "charge fee, account funded with coins other than base denom, error": { +// accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), +// feeToCharge: baseFee, + +// expectError: true, +// }, +// } + +// for name, tc := range testcases { +// suite.Run(name, func() { +// suite.SetupTest() + +// testAccount := suite.TestAccs[0] + +// ctx := suite.Ctx +// incentivesKeepers := suite.App.IncentivesKeeper + +// suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) + +// // Pre + +// // System under test. +// err := incentivesKeepers.AddToGaugeRewards(ctx, testAccount, tc.feeToCharge) + +// if tc.expectError { +// suite.Require().Error(err) +// } else { +// suite.Require().NoError(err) +// } +// }) +// } +// } diff --git a/x/incentives/keeper/keeper.go b/x/incentives/keeper/keeper.go index c9d12b4f89c..92890874182 100644 --- a/x/incentives/keeper/keeper.go +++ b/x/incentives/keeper/keeper.go @@ -19,6 +19,7 @@ type Keeper struct { storeKey sdk.StoreKey paramSpace paramtypes.Subspace hooks types.IncentiveHooks + ak types.AccountKeeper bk types.BankKeeper lk types.LockupKeeper ek types.EpochKeeper @@ -27,7 +28,7 @@ type Keeper struct { } // NewKeeper returns a new instance of the incentive module keeper struct. -func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper, txfk types.TxFeesKeeper) *Keeper { +func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak types.AccountKeeper, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper, txfk types.TxFeesKeeper) *Keeper { if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) } @@ -36,6 +37,7 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub cdc: cdc, storeKey: storeKey, paramSpace: paramSpace, + ak: ak, bk: bk, lk: lk, ek: ek, diff --git a/x/incentives/keeper/msg_server.go b/x/incentives/keeper/msg_server.go index bfcffaf99e0..abb2d521eca 100644 --- a/x/incentives/keeper/msg_server.go +++ b/x/incentives/keeper/msg_server.go @@ -33,6 +33,10 @@ func (server msgServer) CreateGauge(goCtx context.Context, msg *types.MsgCreateG return nil, err } + if err := server.keeper.chargeFee(ctx, owner, createGaugeFee, msg.Coins); err != nil { + return nil, err + } + gaugeID, err := server.keeper.CreateGauge(ctx, msg.IsPerpetual, owner, msg.Coins, msg.DistributeTo, msg.StartTime, msg.NumEpochsPaidOver) if err != nil { return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) @@ -56,6 +60,10 @@ func (server msgServer) AddToGauge(goCtx context.Context, msg *types.MsgAddToGau if err != nil { return nil, err } + + if err := server.keeper.chargeFee(ctx, owner, addToGaugeFee, msg.Rewards); err != nil { + return nil, err + } err = server.keeper.AddToGaugeRewards(ctx, owner, msg.Rewards, msg.GaugeId) if err != nil { return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) diff --git a/x/incentives/types/expected_keepers.go b/x/incentives/types/expected_keepers.go index 2098c3d0959..04b1750dbab 100644 --- a/x/incentives/types/expected_keepers.go +++ b/x/incentives/types/expected_keepers.go @@ -7,11 +7,17 @@ import ( lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) +type AccountKeeper interface { + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI +} + // BankKeeper defines the expected interface needed to retrieve account balances. type BankKeeper interface { GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins + GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin HasSupply(ctx sdk.Context, denom string) bool From 4fd5c719be1afeba22c6fc8a9b78efd170d3b3e9 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 14:16:50 -0700 Subject: [PATCH 05/27] more tests --- x/incentives/keeper/gauge.go | 3 --- x/incentives/keeper/gauge_test.go | 28 +++++++++++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index 094912fa5fa..e68cf978199 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -151,9 +151,6 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr // AddToGaugeRewards adds coins to gauge. func (k Keeper) AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins sdk.Coins, gaugeID uint64) error { - // if err := k.chargeFee(ctx, owner, types.AddToGaugeFee); err != nil { - // return err - // } gauge, err := k.GetGaugeByID(ctx, gaugeID) if err != nil { return err diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 7007b391091..52cf498a02b 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -251,35 +251,53 @@ func (suite *KeeperTestSuite) TestChargeFee() { expectError bool }{ - "charge fee, fee + base denom gauge coin == acount balance, success": { + "fee + base denom gauge coin == acount balance, success": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee / 2, gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, - "charge fee - fee + base denom gauge coin < acount balance, success": { + "fee + base denom gauge coin < acount balance, success": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee/2 - 1, gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, - "charge fee - fee + base denom gauge coin > acount balance, error": { + "fee + base denom gauge coin > acount balance, error": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee/2 + 1, gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), expectError: true, }, - "charge fee - fee + base denom gauge coin < acount balance, custom values, success": { + "fee + base denom gauge coin < acount balance, custom values, success": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), feeToCharge: 55, gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(328812))), }, - "charge fee, account funded with coins other than base denom, error": { + "account funded with coins other than base denom, error": { accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), feeToCharge: baseFee, gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), expectError: true, }, + "fee == account balance, no gauge coins, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + feeToCharge: baseFee, + }, + "gauge coins == account balance, no fee, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee))), + }, + "fee == account balance, gauge coins in denom other than base, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + feeToCharge: baseFee, + gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2))), + }, + "fee + gauge coins == account balance, multiple gauge coins, one in denom other than base, success": { + accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + feeToCharge: baseFee / 2, + gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2)), sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), + }, } for name, tc := range testcases { From ccf9daac08f9d9d729f1e4ef67c60cd44efb0639 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 14:17:21 -0700 Subject: [PATCH 06/27] clean up --- x/incentives/keeper/gauge_test.go | 60 ------------------------------- 1 file changed, 60 deletions(-) diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 52cf498a02b..c5814de6563 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -323,63 +323,3 @@ func (suite *KeeperTestSuite) TestChargeFee() { }) } } - -// func (suite *KeeperTestSuite) TestAddToGauge_ChargeFee() { -// const baseFee = int64(100) - -// testcases := map[string]struct { -// accountBalanceToFund sdk.Coin -// feeToCharge int64 - -// expectError bool -// }{ -// "charge fee, fee == acount balance, success": { -// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), -// feeToCharge: baseFee, -// }, -// "charge fee - fee < acount balance, success": { -// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), -// feeToCharge: baseFee - 1, -// }, -// "charge fee - fee > acount balance, error": { -// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), -// feeToCharge: baseFee + 1, - -// expectError: true, -// }, -// "charge fee - fee < acount balance, custom values, success": { -// accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), -// feeToCharge: 55, -// }, -// "charge fee, account funded with coins other than base denom, error": { -// accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), -// feeToCharge: baseFee, - -// expectError: true, -// }, -// } - -// for name, tc := range testcases { -// suite.Run(name, func() { -// suite.SetupTest() - -// testAccount := suite.TestAccs[0] - -// ctx := suite.Ctx -// incentivesKeepers := suite.App.IncentivesKeeper - -// suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) - -// // Pre - -// // System under test. -// err := incentivesKeepers.AddToGaugeRewards(ctx, testAccount, tc.feeToCharge) - -// if tc.expectError { -// suite.Require().Error(err) -// } else { -// suite.Require().NoError(err) -// } -// }) -// } -// } From ec5fbea5030cf7e01b8ba1252b2edef916112fba Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 14:46:28 -0700 Subject: [PATCH 07/27] test balances --- x/incentives/keeper/gauge.go | 2 +- x/incentives/keeper/gauge_test.go | 12 ++++++++++++ x/incentives/keeper/keeper.go | 4 ++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index e68cf978199..0e4751c5976 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -289,7 +289,7 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { func (k Keeper) chargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) (err error) { // Send creation fee to community pool - feeDenom, err := k.txfk.GetBaseDenom(ctx) + feeDenom, err := k.tk.GetBaseDenom(ctx) if err != nil { return err } diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index c5814de6563..3bdfbcd9a41 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -308,17 +308,29 @@ func (suite *KeeperTestSuite) TestChargeFee() { ctx := suite.Ctx incentivesKeepers := suite.App.IncentivesKeeper + bankKeeper := suite.App.BankKeeper // Pre-fund account. suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) + oldBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount + // System under test. err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) + // Assertions. + newBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount if tc.expectError { suite.Require().Error(err) + + // check account balance unchanged + suite.Require().Equal(oldBalanceAmount, newBalanceAmount) } else { suite.Require().NoError(err) + + // check account balance changed. + expectedNewBalanceAmount := oldBalanceAmount.Sub(sdk.NewInt(tc.feeToCharge)) + suite.Require().Equal(expectedNewBalanceAmount.String(), newBalanceAmount.String()) } }) } diff --git a/x/incentives/keeper/keeper.go b/x/incentives/keeper/keeper.go index 92890874182..c2019b4ccdf 100644 --- a/x/incentives/keeper/keeper.go +++ b/x/incentives/keeper/keeper.go @@ -24,7 +24,7 @@ type Keeper struct { lk types.LockupKeeper ek types.EpochKeeper dk types.DistrKeeper - txfk types.TxFeesKeeper + tk types.TxFeesKeeper } // NewKeeper returns a new instance of the incentive module keeper struct. @@ -42,7 +42,7 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub lk: lk, ek: ek, dk: dk, - txfk: txfk, + tk: txfk, } } From 590349fb25082401486420a8f2f7b7ef1b8ac79f Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Sun, 24 Jul 2022 17:02:17 -0500 Subject: [PATCH 08/27] test create gauge fees (#2228) * test create gauge fees * add mod account to test * test create gauge fees * add mod account to test * move to msg server * merge --- x/incentives/keeper/export_test.go | 15 +- x/incentives/keeper/gauge_test.go | 206 +++++++++++--------- x/incentives/keeper/msg_server_test.go | 256 +++++++++++++++++++++++++ 3 files changed, 379 insertions(+), 98 deletions(-) create mode 100644 x/incentives/keeper/msg_server_test.go diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index e31366c69e0..fc102b21340 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -6,13 +6,20 @@ import ( "github.com/osmosis-labs/osmosis/v10/x/incentives/types" ) +const ( + // CreateGaugeFee is the fee required to create a new gauge. + CreateGaugeFee = createGaugeFee + // AddToGagugeFee is the fee required to add to gauge. + AddToGaugeFee = addToGaugeFee +) + // AddGaugeRefByKey appends the provided gauge ID into an array associated with the provided key. -func (k Keeper) AddGaugeRefByKey(ctx sdk.Context, key []byte, guageID uint64) error { - return k.addGaugeRefByKey(ctx, key, guageID) +func (k Keeper) AddGaugeRefByKey(ctx sdk.Context, key []byte, gaugeID uint64) error { + return k.addGaugeRefByKey(ctx, key, gaugeID) } -func (k Keeper) DeleteGaugeRefByKey(ctx sdk.Context, key []byte, guageID uint64) error { - return k.deleteGaugeRefByKey(ctx, key, guageID) +func (k Keeper) DeleteGaugeRefByKey(ctx sdk.Context, key []byte, gaugeID uint64) error { + return k.deleteGaugeRefByKey(ctx, key, gaugeID) } // GetGaugeRefs returns the gauge IDs specified by the provided key. diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 3bdfbcd9a41..0810edf86c7 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -241,97 +241,115 @@ func (suite *KeeperTestSuite) TestGaugeOperations() { } } -func (suite *KeeperTestSuite) TestChargeFee() { - const baseFee = int64(100) - - testcases := map[string]struct { - accountBalanceToFund sdk.Coin - feeToCharge int64 - gaugeCoins sdk.Coins - - expectError bool - }{ - "fee + base denom gauge coin == acount balance, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee / 2, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), - }, - "fee + base denom gauge coin < acount balance, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee/2 - 1, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), - }, - "fee + base denom gauge coin > acount balance, error": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee/2 + 1, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), - - expectError: true, - }, - "fee + base denom gauge coin < acount balance, custom values, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), - feeToCharge: 55, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(328812))), - }, - "account funded with coins other than base denom, error": { - accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), - feeToCharge: baseFee, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), - - expectError: true, - }, - "fee == account balance, no gauge coins, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee, - }, - "gauge coins == account balance, no fee, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee))), - }, - "fee == account balance, gauge coins in denom other than base, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee, - gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2))), - }, - "fee + gauge coins == account balance, multiple gauge coins, one in denom other than base, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - feeToCharge: baseFee / 2, - gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2)), sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), - }, - } - - for name, tc := range testcases { - suite.Run(name, func() { - suite.SetupTest() - - testAccount := suite.TestAccs[0] - - ctx := suite.Ctx - incentivesKeepers := suite.App.IncentivesKeeper - bankKeeper := suite.App.BankKeeper - - // Pre-fund account. - suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) - - oldBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount - - // System under test. - err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) - - // Assertions. - newBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount - if tc.expectError { - suite.Require().Error(err) - - // check account balance unchanged - suite.Require().Equal(oldBalanceAmount, newBalanceAmount) - } else { - suite.Require().NoError(err) - - // check account balance changed. - expectedNewBalanceAmount := oldBalanceAmount.Sub(sdk.NewInt(tc.feeToCharge)) - suite.Require().Equal(expectedNewBalanceAmount.String(), newBalanceAmount.String()) - } - }) - } -} +// func (suite *KeeperTestSuite) TestCreateGaugeFee() { + +// tests := []struct { +// name string +// accountBalanceToFund sdk.Coins +// gaugeAddition sdk.Coins +// expectedEndBalance sdk.Coins +// isPerpetual bool +// isModuleAccount bool +// expectErr bool +// }{ +// { +// name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), +// }, +// { +// name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// }, +// { +// name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), +// }, +// { +// name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), +// isPerpetual: true, +// isModuleAccount: true, +// }, +// { +// name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), +// isPerpetual: true, +// }, +// { +// name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), +// expectErr: true, +// }, +// { +// name: "user tries to create a non-perpetual gauge but does not have the correct fee denom", +// accountBalanceToFund: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), +// gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), +// expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), +// expectErr: true, +// }, +// // TODO: This is unexpected behavior +// // We need validation to not charge fee if user doesn't have enough funds +// // { +// // name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", +// // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), +// // gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), +// // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), +// // expectErr: true, +// // }, + +// } + +// for _, tc := range tests { +// suite.SetupTest() + +// //testAccount := suite.TestAccs[0] +// testAccountPubkey := secp256k1.GenPrivKeyFromSecret([]byte("acc")).PubKey() +// testAccountAddress := sdk.AccAddress(testAccountPubkey.Address()) + +// ctx := suite.Ctx +// incentivesKeepers := suite.App.IncentivesKeeper + +// suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) + +// if tc.isModuleAccount { +// modAcc := authtypes.NewModuleAccount(authtypes.NewBaseAccount(testAccountAddress, testAccountPubkey, 1, 0), +// "module", +// "permission", +// ) +// suite.App.AccountKeeper.SetModuleAccount(ctx, modAcc) +// } + +// suite.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration) +// distrTo := lockuptypes.QueryCondition{ +// LockQueryType: lockuptypes.ByDuration, +// Denom: defaultLPDenom, +// Duration: defaultLockDuration, +// } + +// // System under test. + +// _, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) + +// if tc.expectErr { +// suite.Require().Error(err) +// } else { +// suite.Require().NoError(err) +// } + +// bal := suite.App.BankKeeper.GetAllBalances(suite.Ctx, testAccountAddress) +// suite.Require().Equal(tc.expectedEndBalance.String(), bal.String(), "test: %v", tc.name) + +// } +// } diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go new file mode 100644 index 00000000000..49ac1dcb384 --- /dev/null +++ b/x/incentives/keeper/msg_server_test.go @@ -0,0 +1,256 @@ +package keeper_test + +import ( + "time" + + "github.com/stretchr/testify/suite" + + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + + incentiveskeeper "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" + lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +var _ = suite.TestingSuite(nil) + +func (suite *KeeperTestSuite) TestCreateGaugeFee() { + + tests := []struct { + name string + accountBalanceToFund sdk.Coins + gaugeAddition sdk.Coins + expectedEndBalance sdk.Coins + isPerpetual bool + isModuleAccount bool + expectErr bool + }{ + { + name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), + }, + { + name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + }, + { + name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + }, + { + name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + isPerpetual: true, + isModuleAccount: true, + }, + { + name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + isPerpetual: true, + }, + { + name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + expectErr: true, + }, + { + name: "user tries to create a non-perpetual gauge but does not have the correct fee denom", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), + expectErr: true, + }, + // TODO: This is unexpected behavior + // We need validation to not charge fee if user doesn't have enough funds + // { + // name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", + // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + // gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), + // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + // expectErr: true, + // }, + + } + + for _, tc := range tests { + suite.SetupTest() + + testAccountPubkey := secp256k1.GenPrivKeyFromSecret([]byte("acc")).PubKey() + testAccountAddress := sdk.AccAddress(testAccountPubkey.Address()) + + ctx := suite.Ctx + incentivesKeepers := suite.App.IncentivesKeeper + + suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) + + if tc.isModuleAccount { + modAcc := authtypes.NewModuleAccount(authtypes.NewBaseAccount(testAccountAddress, testAccountPubkey, 1, 0), + "module", + "permission", + ) + suite.App.AccountKeeper.SetModuleAccount(ctx, modAcc) + } + + suite.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration) + distrTo := lockuptypes.QueryCondition{ + LockQueryType: lockuptypes.ByDuration, + Denom: defaultLPDenom, + Duration: defaultLockDuration, + } + + // System under test. + + _, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) + + if tc.expectErr { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + } + + bal := suite.App.BankKeeper.GetAllBalances(suite.Ctx, testAccountAddress) + suite.Require().Equal(tc.expectedEndBalance.String(), bal.String(), "test: %v", tc.name) + + if tc.expectErr { + suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) + } else { + finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(incentiveskeeper.CreateGaugeFee)))) + suite.Require().Equal(finalAccountBalalance.String(), bal.String(), "test: %v", tc.name) + } + + } +} + +func (suite *KeeperTestSuite) TestAddToGaugeFee() { + + tests := []struct { + name string + accountBalanceToFund sdk.Coins + gaugeAddition sdk.Coins + expectedEndBalance sdk.Coins + isPerpetual bool + isModuleAccount bool + expectErr bool + }{ + { + name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), + }, + { + name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + }, + { + name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + }, + { + name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + isPerpetual: true, + isModuleAccount: true, + }, + { + name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + isPerpetual: true, + }, + { + name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + expectErr: true, + }, + { + name: "user tries to create a non-perpetual gauge but does not have the correct fee denom", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), + expectErr: true, + }, + // TODO: This is unexpected behavior + // We need validation to not charge fee if user doesn't have enough funds + // { + // name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", + // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + // gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), + // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + // expectErr: true, + // }, + + } + + for _, tc := range tests { + suite.SetupTest() + + testAccountPubkey := secp256k1.GenPrivKeyFromSecret([]byte("acc")).PubKey() + testAccountAddress := sdk.AccAddress(testAccountPubkey.Address()) + + ctx := suite.Ctx + incentivesKeepers := suite.App.IncentivesKeeper + + suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) + + if tc.isModuleAccount { + modAcc := authtypes.NewModuleAccount(authtypes.NewBaseAccount(testAccountAddress, testAccountPubkey, 1, 0), + "module", + "permission", + ) + suite.App.AccountKeeper.SetModuleAccount(ctx, modAcc) + } + + suite.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration) + distrTo := lockuptypes.QueryCondition{ + LockQueryType: lockuptypes.ByDuration, + Denom: defaultLPDenom, + Duration: defaultLockDuration, + } + + // System under test. + + _, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) + //incentivesKeepers.AddToGaugeRewards(ctx, testAccountAddress, tc.gaugeAddition) + + if tc.expectErr { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + } + + bal := suite.App.BankKeeper.GetAllBalances(suite.Ctx, testAccountAddress) + suite.Require().Equal(tc.expectedEndBalance.String(), bal.String(), "test: %v", tc.name) + + if tc.expectErr { + suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) + } else { + finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(incentiveskeeper.CreateGaugeFee)))) + suite.Require().Equal(finalAccountBalalance.String(), bal.String(), "test: %v", tc.name) + } + + } +} From 023fc86410bb74fd3bb11ec8d9a83f8cf43b9f03 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 15:12:26 -0700 Subject: [PATCH 09/27] add comments --- x/incentives/keeper/export_test.go | 6 ++++-- x/incentives/keeper/gauge.go | 6 +++++- x/incentives/keeper/gauge_test.go | 2 +- x/incentives/keeper/msg_server.go | 4 ++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index e31366c69e0..524f4e8431a 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -11,6 +11,7 @@ func (k Keeper) AddGaugeRefByKey(ctx sdk.Context, key []byte, guageID uint64) er return k.addGaugeRefByKey(ctx, key, guageID) } +// DeleteGaugeRefByKey removes the provided gauge ID from an array associated with the provided key. func (k Keeper) DeleteGaugeRefByKey(ctx sdk.Context, key []byte, guageID uint64) error { return k.deleteGaugeRefByKey(ctx, key, guageID) } @@ -35,6 +36,7 @@ func (k Keeper) MoveActiveGaugeToFinishedGauge(ctx sdk.Context, gauge types.Gaug return k.moveActiveGaugeToFinishedGauge(ctx, gauge) } -func (k Keeper) ChargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) error { - return k.chargeFee(ctx, address, fee, gaugeCoins) +// ChargeFeeIfSufficientFeeDenomBalance see chargeFeeIfSufficientFeeDenomBalance spec. +func (k Keeper) ChargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) error { + return k.chargeFeeIfSufficientFeeDenomBalance(ctx, address, fee, gaugeCoins) } diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index 0e4751c5976..e21b8fdf82b 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -287,7 +287,11 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { return k.ek.GetEpochInfo(ctx, params.DistrEpochIdentifier) } -func (k Keeper) chargeFee(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) (err error) { +// chargeFeeIfSufficientFeeDenomBalance charges fee in the base tx fee denom on the address if the address has +// balance that is less than fee + amount of the coin from gaugeCoins that is of base tx fee denom. +// gaugeCoins might not have a coin of tx base fee denom. In that case, only fee is compared to balance. +// Returns nil on success, error otherwise. +func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) (err error) { // Send creation fee to community pool feeDenom, err := k.tk.GetBaseDenom(ctx) if err != nil { diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 3bdfbcd9a41..1d80aad0c1e 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -316,7 +316,7 @@ func (suite *KeeperTestSuite) TestChargeFee() { oldBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount // System under test. - err := incentivesKeepers.ChargeFee(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) + err := incentivesKeepers.ChargeFeeIfSufficientFeeDenomBalance(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) // Assertions. newBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount diff --git a/x/incentives/keeper/msg_server.go b/x/incentives/keeper/msg_server.go index abb2d521eca..f936c19f967 100644 --- a/x/incentives/keeper/msg_server.go +++ b/x/incentives/keeper/msg_server.go @@ -33,7 +33,7 @@ func (server msgServer) CreateGauge(goCtx context.Context, msg *types.MsgCreateG return nil, err } - if err := server.keeper.chargeFee(ctx, owner, createGaugeFee, msg.Coins); err != nil { + if err := server.keeper.chargeFeeIfSufficientFeeDenomBalance(ctx, owner, createGaugeFee, msg.Coins); err != nil { return nil, err } @@ -61,7 +61,7 @@ func (server msgServer) AddToGauge(goCtx context.Context, msg *types.MsgAddToGau return nil, err } - if err := server.keeper.chargeFee(ctx, owner, addToGaugeFee, msg.Rewards); err != nil { + if err := server.keeper.chargeFeeIfSufficientFeeDenomBalance(ctx, owner, addToGaugeFee, msg.Rewards); err != nil { return nil, err } err = server.keeper.AddToGaugeRewards(ctx, owner, msg.Rewards, msg.GaugeId) From 1b4eb6c7fb33b3121926a7261ae01bb3bcc708f2 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 15:16:20 -0700 Subject: [PATCH 10/27] account keeper comment --- x/incentives/types/expected_keepers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/incentives/types/expected_keepers.go b/x/incentives/types/expected_keepers.go index 04b1750dbab..72d81333018 100644 --- a/x/incentives/types/expected_keepers.go +++ b/x/incentives/types/expected_keepers.go @@ -10,6 +10,7 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) +// AccountKeeper defines the expected interface needed for managing accounts. type AccountKeeper interface { GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI } From 86e8b5e4ad334bae019da676cb25fb17f6c79bdb Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 18:26:18 -0700 Subject: [PATCH 11/27] fix TestCreateGaugeFee --- x/incentives/keeper/msg_server_test.go | 71 +++++++++++++++----------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index 49ac1dcb384..1cf59c7dfa9 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "testing" "time" "github.com/stretchr/testify/suite" @@ -8,13 +9,17 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" incentiveskeeper "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" + "github.com/osmosis-labs/osmosis/v10/x/incentives/types" lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" sdk "github.com/cosmos/cosmos-sdk/types" ) -var _ = suite.TestingSuite(nil) +func TestMsgServerTestSuite(t *testing.T) { + suite.Run(t, new(KeeperTestSuite)) +} func (suite *KeeperTestSuite) TestCreateGaugeFee() { @@ -31,59 +36,56 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), }, { name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), }, { name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), }, { name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - isPerpetual: true, - isModuleAccount: true, + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + isPerpetual: true, + isModuleAccount: true, }, { name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - isPerpetual: true, + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + isPerpetual: true, }, { name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), - expectErr: true, + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + expectErr: true, }, { name: "user tries to create a non-perpetual gauge but does not have the correct fee denom", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), - expectErr: true, + expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), + expectErr: true, + }, + { + name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + expectErr: true, }, - // TODO: This is unexpected behavior - // We need validation to not charge fee if user doesn't have enough funds - // { - // name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", - // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), - // gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), - // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), - // expectErr: true, - // }, - } for _, tc := range tests { @@ -93,7 +95,8 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { testAccountAddress := sdk.AccAddress(testAccountPubkey.Address()) ctx := suite.Ctx - incentivesKeepers := suite.App.IncentivesKeeper + bankKeeper := suite.App.BankKeeper + msgServer := keeper.NewMsgServerImpl(suite.App.IncentivesKeeper) suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) @@ -112,9 +115,16 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { Duration: defaultLockDuration, } + msg := &types.MsgCreateGauge{ + IsPerpetual: tc.isPerpetual, + Owner: testAccountAddress.String(), + DistributeTo: distrTo, + Coins: tc.gaugeAddition, + StartTime: time.Now(), + NumEpochsPaidOver: 1, + } // System under test. - - _, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) + _, err := msgServer.CreateGauge(sdk.WrapSDKContext(ctx), msg) if tc.expectErr { suite.Require().Error(err) @@ -122,14 +132,13 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { suite.Require().NoError(err) } - bal := suite.App.BankKeeper.GetAllBalances(suite.Ctx, testAccountAddress) - suite.Require().Equal(tc.expectedEndBalance.String(), bal.String(), "test: %v", tc.name) + balanceAmount := bankKeeper.GetAllBalances(ctx, testAccountAddress) + suite.Require().Equal(tc.expectedEndBalance.String(), balanceAmount.String(), "test: %v", tc.name) if tc.expectErr { - suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) + suite.Require().Equal(tc.accountBalanceToFund.String(), balanceAmount.String(), "test: %v", tc.name) } else { - finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(incentiveskeeper.CreateGaugeFee)))) - suite.Require().Equal(finalAccountBalalance.String(), bal.String(), "test: %v", tc.name) + suite.Require().Equal(tc.expectedEndBalance.String(), balanceAmount.String(), "test: %v", tc.name) } } From 100b693c51cb28e2da6e23954f7960b881485546 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 18:47:05 -0700 Subject: [PATCH 12/27] apply appparams.BaseCoinUnit --- x/incentives/keeper/gauge.go | 17 ++--- x/incentives/keeper/gauge_test.go | 33 +++++----- x/incentives/keeper/msg_server_test.go | 87 +++++++++++++------------- 3 files changed, 67 insertions(+), 70 deletions(-) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index e21b8fdf82b..a261d4d175c 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -11,6 +11,7 @@ import ( "github.com/gogo/protobuf/proto" db "github.com/tendermint/tm-db" + appparams "github.com/osmosis-labs/osmosis/v10/app/params" epochtypes "github.com/osmosis-labs/osmosis/v10/x/epochs/types" "github.com/osmosis-labs/osmosis/v10/x/incentives/types" lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" @@ -290,22 +291,16 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { // chargeFeeIfSufficientFeeDenomBalance charges fee in the base tx fee denom on the address if the address has // balance that is less than fee + amount of the coin from gaugeCoins that is of base tx fee denom. // gaugeCoins might not have a coin of tx base fee denom. In that case, only fee is compared to balance. +// The fee is sent to the community pool. // Returns nil on success, error otherwise. func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) (err error) { - // Send creation fee to community pool - feeDenom, err := k.tk.GetBaseDenom(ctx) - if err != nil { - return err - } feeInt := sdk.NewInt(fee) - - totalCost := gaugeCoins.AmountOf(feeDenom).Add(feeInt) - accountBalance := k.bk.GetBalance(ctx, address, feeDenom).Amount + totalCost := gaugeCoins.AmountOf(appparams.BaseCoinUnit).Add(feeInt) + accountBalance := k.bk.GetBalance(ctx, address, appparams.BaseCoinUnit).Amount if accountBalance.LT(totalCost) { - return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "account's balance of %s (%s) is less than the total cost of the message (%s)", feeDenom, accountBalance, totalCost) + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "account's balance of %s (%s) is less than the total cost of the message (%s)", appparams.BaseCoinUnit, accountBalance, totalCost) } - - if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(feeDenom, feeInt)), address); err != nil { + if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, feeInt)), address); err != nil { return err } return nil diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 1d80aad0c1e..73d8b2abf5d 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/suite" + appparams "github.com/osmosis-labs/osmosis/v10/app/params" "github.com/osmosis-labs/osmosis/v10/x/incentives/types" lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" @@ -252,51 +253,51 @@ func (suite *KeeperTestSuite) TestChargeFee() { expectError bool }{ "fee + base denom gauge coin == acount balance, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee / 2, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, "fee + base denom gauge coin < acount balance, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee/2 - 1, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, "fee + base denom gauge coin > acount balance, error": { accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee/2 + 1, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee/2))), expectError: true, }, "fee + base denom gauge coin < acount balance, custom values, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(11793193112)), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(11793193112)), feeToCharge: 55, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(328812))), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(328812))), }, "account funded with coins other than base denom, error": { accountBalanceToFund: sdk.NewCoin("usdc", sdk.NewInt(baseFee)), feeToCharge: baseFee, - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee/2))), expectError: true, }, "fee == account balance, no gauge coins, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee, }, "gauge coins == account balance, no fee, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), - gaugeCoins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee))), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee)), + gaugeCoins: sdk.NewCoins(sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee))), }, "fee == account balance, gauge coins in denom other than base, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee, gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2))), }, "fee + gauge coins == account balance, multiple gauge coins, one in denom other than base, success": { - accountBalanceToFund: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee)), + accountBalanceToFund: sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee)), feeToCharge: baseFee / 2, - gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2)), sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseFee/2))), + gaugeCoins: sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(baseFee*2)), sdk.NewCoin(appparams.DefaultBondDenom, sdk.NewInt(baseFee/2))), }, } @@ -313,13 +314,13 @@ func (suite *KeeperTestSuite) TestChargeFee() { // Pre-fund account. suite.FundAcc(testAccount, sdk.NewCoins(tc.accountBalanceToFund)) - oldBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount + oldBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, appparams.DefaultBondDenom).Amount // System under test. err := incentivesKeepers.ChargeFeeIfSufficientFeeDenomBalance(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) // Assertions. - newBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, sdk.DefaultBondDenom).Amount + newBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, appparams.DefaultBondDenom).Amount if tc.expectError { suite.Require().Error(err) diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index 1cf59c7dfa9..cd84d84246a 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + appparams "github.com/osmosis-labs/osmosis/v10/app/params" "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" incentiveskeeper "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" "github.com/osmosis-labs/osmosis/v10/x/incentives/types" @@ -34,42 +35,42 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { }{ { name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(0))), }, { name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), }, { name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), }, { name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), isPerpetual: true, isModuleAccount: true, }, { name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), isPerpetual: true, }, { name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), expectErr: true, }, { @@ -81,9 +82,9 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { }, { name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(30000000))), + expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), expectErr: true, }, } @@ -157,42 +158,42 @@ func (suite *KeeperTestSuite) TestAddToGaugeFee() { }{ { name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(0))), }, { name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), }, { name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), }, { name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), isPerpetual: true, isModuleAccount: true, }, { name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), isPerpetual: true, }, { name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), - gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(40000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), + gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), expectErr: true, }, { @@ -206,9 +207,9 @@ func (suite *KeeperTestSuite) TestAddToGaugeFee() { // We need validation to not charge fee if user doesn't have enough funds // { // name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", - // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), - // gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(30000000))), - // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(60000000))), + // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), + // gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(30000000))), + // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), // expectErr: true, // }, @@ -257,7 +258,7 @@ func (suite *KeeperTestSuite) TestAddToGaugeFee() { if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) } else { - finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(incentiveskeeper.CreateGaugeFee)))) + finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(incentiveskeeper.CreateGaugeFee)))) suite.Require().Equal(finalAccountBalalance.String(), bal.String(), "test: %v", tc.name) } From 915e9ff99a4a615ee4a5de2c74a06bf6368dc94a Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 18:49:46 -0700 Subject: [PATCH 13/27] remove txfees keeper from incentives and revert order --- app/keepers/keepers.go | 27 +++++++++++++------------- x/incentives/keeper/keeper.go | 4 +--- x/incentives/keeper/msg_server_test.go | 1 - 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 4f84ee735c9..fa33d5dddbf 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -260,19 +260,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appKeepers.EpochsKeeper = epochskeeper.NewKeeper(appCodec, appKeepers.keys[epochstypes.StoreKey]) - txFeesKeeper := txfeeskeeper.NewKeeper( - appCodec, - appKeepers.AccountKeeper, - appKeepers.BankKeeper, - appKeepers.EpochsKeeper, - appKeepers.keys[txfeestypes.StoreKey], - appKeepers.GAMMKeeper, - appKeepers.GAMMKeeper, - txfeestypes.FeeCollectorName, - txfeestypes.NonNativeFeeCollectorName, - ) - appKeepers.TxFeesKeeper = &txFeesKeeper - appKeepers.IncentivesKeeper = incentiveskeeper.NewKeeper( appCodec, appKeepers.keys[incentivestypes.StoreKey], @@ -282,7 +269,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appKeepers.LockupKeeper, appKeepers.EpochsKeeper, appKeepers.DistrKeeper, - appKeepers.TxFeesKeeper, ) appKeepers.SuperfluidKeeper = superfluidkeeper.NewKeeper( @@ -315,6 +301,19 @@ func (appKeepers *AppKeepers) InitNormalKeepers( ) appKeepers.PoolIncentivesKeeper = &poolIncentivesKeeper + txFeesKeeper := txfeeskeeper.NewKeeper( + appCodec, + appKeepers.AccountKeeper, + appKeepers.BankKeeper, + appKeepers.EpochsKeeper, + appKeepers.keys[txfeestypes.StoreKey], + appKeepers.GAMMKeeper, + appKeepers.GAMMKeeper, + txfeestypes.FeeCollectorName, + txfeestypes.NonNativeFeeCollectorName, + ) + appKeepers.TxFeesKeeper = &txFeesKeeper + tokenFactoryKeeper := tokenfactorykeeper.NewKeeper( appCodec, appKeepers.keys[tokenfactorytypes.StoreKey], diff --git a/x/incentives/keeper/keeper.go b/x/incentives/keeper/keeper.go index c2019b4ccdf..8718aa0ca65 100644 --- a/x/incentives/keeper/keeper.go +++ b/x/incentives/keeper/keeper.go @@ -24,11 +24,10 @@ type Keeper struct { lk types.LockupKeeper ek types.EpochKeeper dk types.DistrKeeper - tk types.TxFeesKeeper } // NewKeeper returns a new instance of the incentive module keeper struct. -func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak types.AccountKeeper, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper, txfk types.TxFeesKeeper) *Keeper { +func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak types.AccountKeeper, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper) *Keeper { if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) } @@ -42,7 +41,6 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub lk: lk, ek: ek, dk: dk, - tk: txfk, } } diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index cd84d84246a..a6c1f5f111a 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -23,7 +23,6 @@ func TestMsgServerTestSuite(t *testing.T) { } func (suite *KeeperTestSuite) TestCreateGaugeFee() { - tests := []struct { name string accountBalanceToFund sdk.Coins From 70d9347b3bf4dd40edf2db885a8d2d4a9a17d142 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 18:56:52 -0700 Subject: [PATCH 14/27] clean up --- x/incentives/keeper/export_test.go | 4 ++-- x/incentives/keeper/gauge.go | 13 ++++++------- x/incentives/keeper/gauge_test.go | 2 +- x/incentives/keeper/msg_server_test.go | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index ae8f1dc9822..47602af7137 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -6,7 +6,7 @@ import ( "github.com/osmosis-labs/osmosis/v10/x/incentives/types" ) -const ( +var ( // CreateGaugeFee is the fee required to create a new gauge. CreateGaugeFee = createGaugeFee // AddToGagugeFee is the fee required to add to gauge. @@ -44,6 +44,6 @@ func (k Keeper) MoveActiveGaugeToFinishedGauge(ctx sdk.Context, gauge types.Gaug } // ChargeFeeIfSufficientFeeDenomBalance see chargeFeeIfSufficientFeeDenomBalance spec. -func (k Keeper) ChargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) error { +func (k Keeper) ChargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee sdk.Int, gaugeCoins sdk.Coins) error { return k.chargeFeeIfSufficientFeeDenomBalance(ctx, address, fee, gaugeCoins) } diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index a261d4d175c..cfb84823524 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -19,11 +19,11 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -const ( +var ( // CreateGaugeFee is the fee required to create a new gauge. - createGaugeFee = 50 * 1_000_000 + createGaugeFee = sdk.NewInt(50 * 1_000_000) // AddToGagugeFee is the fee required to add to gauge. - addToGaugeFee = 25 * 1_000_000 + addToGaugeFee = sdk.NewInt(25 * 1_000_000) ) // getGaugesFromIterator iterates over everything in a gauge's iterator, until it reaches the end. Return all gauges iterated over. @@ -293,14 +293,13 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { // gaugeCoins might not have a coin of tx base fee denom. In that case, only fee is compared to balance. // The fee is sent to the community pool. // Returns nil on success, error otherwise. -func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee int64, gaugeCoins sdk.Coins) (err error) { - feeInt := sdk.NewInt(fee) - totalCost := gaugeCoins.AmountOf(appparams.BaseCoinUnit).Add(feeInt) +func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee sdk.Int, gaugeCoins sdk.Coins) (err error) { + totalCost := gaugeCoins.AmountOf(appparams.BaseCoinUnit).Add(fee) accountBalance := k.bk.GetBalance(ctx, address, appparams.BaseCoinUnit).Amount if accountBalance.LT(totalCost) { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "account's balance of %s (%s) is less than the total cost of the message (%s)", appparams.BaseCoinUnit, accountBalance, totalCost) } - if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, feeInt)), address); err != nil { + if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, fee)), address); err != nil { return err } return nil diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 73d8b2abf5d..d51d16cd03f 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -317,7 +317,7 @@ func (suite *KeeperTestSuite) TestChargeFee() { oldBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, appparams.DefaultBondDenom).Amount // System under test. - err := incentivesKeepers.ChargeFeeIfSufficientFeeDenomBalance(ctx, testAccount, tc.feeToCharge, tc.gaugeCoins) + err := incentivesKeepers.ChargeFeeIfSufficientFeeDenomBalance(ctx, testAccount, sdk.NewInt(tc.feeToCharge), tc.gaugeCoins) // Assertions. newBalanceAmount := bankKeeper.GetBalance(ctx, testAccount, appparams.DefaultBondDenom).Amount diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index a6c1f5f111a..088aeb49364 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -257,7 +257,7 @@ func (suite *KeeperTestSuite) TestAddToGaugeFee() { if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) } else { - finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(incentiveskeeper.CreateGaugeFee)))) + finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(appparams.BaseCoinUnit, incentiveskeeper.CreateGaugeFee))) suite.Require().Equal(finalAccountBalalance.String(), bal.String(), "test: %v", tc.name) } From b91c221fad3bb4a91e22307cfa15a37a502cfdb0 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 19:13:17 -0700 Subject: [PATCH 15/27] remove unused keepers fromm incentives keeper --- app/keepers/keepers.go | 1 - x/incentives/keeper/keeper.go | 4 +--- x/incentives/types/expected_keepers.go | 11 ----------- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index fa33d5dddbf..3369228ba26 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -264,7 +264,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appCodec, appKeepers.keys[incentivestypes.StoreKey], appKeepers.GetSubspace(incentivestypes.ModuleName), - appKeepers.AccountKeeper, appKeepers.BankKeeper, appKeepers.LockupKeeper, appKeepers.EpochsKeeper, diff --git a/x/incentives/keeper/keeper.go b/x/incentives/keeper/keeper.go index 8718aa0ca65..d21abad3579 100644 --- a/x/incentives/keeper/keeper.go +++ b/x/incentives/keeper/keeper.go @@ -19,7 +19,6 @@ type Keeper struct { storeKey sdk.StoreKey paramSpace paramtypes.Subspace hooks types.IncentiveHooks - ak types.AccountKeeper bk types.BankKeeper lk types.LockupKeeper ek types.EpochKeeper @@ -27,7 +26,7 @@ type Keeper struct { } // NewKeeper returns a new instance of the incentive module keeper struct. -func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak types.AccountKeeper, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper) *Keeper { +func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper) *Keeper { if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) } @@ -36,7 +35,6 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub cdc: cdc, storeKey: storeKey, paramSpace: paramSpace, - ak: ak, bk: bk, lk: lk, ek: ek, diff --git a/x/incentives/types/expected_keepers.go b/x/incentives/types/expected_keepers.go index 72d81333018..5319039c3fd 100644 --- a/x/incentives/types/expected_keepers.go +++ b/x/incentives/types/expected_keepers.go @@ -7,14 +7,8 @@ import ( lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" sdk "github.com/cosmos/cosmos-sdk/types" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) -// AccountKeeper defines the expected interface needed for managing accounts. -type AccountKeeper interface { - GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI -} - // BankKeeper defines the expected interface needed to retrieve account balances. type BankKeeper interface { GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins @@ -48,8 +42,3 @@ type EpochKeeper interface { type DistrKeeper interface { FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error } - -// TxFeesKeeper defines the expected interface needed to managing transaction fees. -type TxFeesKeeper interface { - GetBaseDenom(ctx sdk.Context) (denom string, err error) -} From f9fd23ca8f9d430b795acc57a1d2d42f34db7df6 Mon Sep 17 00:00:00 2001 From: Roman Date: Sun, 24 Jul 2022 19:16:04 -0700 Subject: [PATCH 16/27] Update x/incentives/keeper/gauge.go --- x/incentives/keeper/gauge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index cfb84823524..5d3acc76b9f 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -20,7 +20,7 @@ import ( ) var ( - // CreateGaugeFee is the fee required to create a new gauge. + // createGaugeFee is the fee required to create a new gauge. createGaugeFee = sdk.NewInt(50 * 1_000_000) // AddToGagugeFee is the fee required to add to gauge. addToGaugeFee = sdk.NewInt(25 * 1_000_000) From ff28eb42757e538b00556961e97961d038e48003 Mon Sep 17 00:00:00 2001 From: Roman Date: Sun, 24 Jul 2022 19:16:10 -0700 Subject: [PATCH 17/27] Update x/incentives/keeper/gauge.go --- x/incentives/keeper/gauge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index 5d3acc76b9f..dd9e141481f 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -22,7 +22,7 @@ import ( var ( // createGaugeFee is the fee required to create a new gauge. createGaugeFee = sdk.NewInt(50 * 1_000_000) - // AddToGagugeFee is the fee required to add to gauge. + // addToGagugeFee is the fee required to add to gauge. addToGaugeFee = sdk.NewInt(25 * 1_000_000) ) From 3da8d5a817ce305d8e3503a97b6d6b36d8f7a0a5 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 19:20:19 -0700 Subject: [PATCH 18/27] clean up --- x/incentives/keeper/gauge_test.go | 5 +---- x/incentives/keeper/msg_server_test.go | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index d51d16cd03f..31ad9bffd08 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "testing" "time" "github.com/stretchr/testify/suite" @@ -13,9 +12,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -func TestGaugeTestSuite(t *testing.T) { - suite.Run(t, new(KeeperTestSuite)) -} +var _ = suite.TestingSuite(nil) // TestInvalidDurationGaugeCreationValidation tests error handling for creating a gauge with an invalid duration. func (suite *KeeperTestSuite) TestInvalidDurationGaugeCreationValidation() { diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index 088aeb49364..5de46f41828 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "testing" "time" "github.com/stretchr/testify/suite" @@ -18,9 +17,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -func TestMsgServerTestSuite(t *testing.T) { - suite.Run(t, new(KeeperTestSuite)) -} +var _ = suite.TestingSuite(nil) func (suite *KeeperTestSuite) TestCreateGaugeFee() { tests := []struct { From f841b4979a94114f11581c754a30cb60c7b37210 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 19:25:34 -0700 Subject: [PATCH 19/27] fixture names --- x/incentives/keeper/msg_server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index 5de46f41828..03adec30042 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -19,7 +19,7 @@ import ( var _ = suite.TestingSuite(nil) -func (suite *KeeperTestSuite) TestCreateGaugeFee() { +func (suite *KeeperTestSuite) TestCreateGauge_Fee() { tests := []struct { name string accountBalanceToFund sdk.Coins @@ -141,7 +141,7 @@ func (suite *KeeperTestSuite) TestCreateGaugeFee() { } } -func (suite *KeeperTestSuite) TestAddToGaugeFee() { +func (suite *KeeperTestSuite) TestAddToGauge_Fee() { tests := []struct { name string From 0503797db15c398ea914c32a5f15927309f3f056 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 19:27:30 -0700 Subject: [PATCH 20/27] chargeFeeIfSufficientFeeDenomBalance test name --- x/incentives/keeper/gauge_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 31ad9bffd08..2e36c881846 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -239,7 +239,7 @@ func (suite *KeeperTestSuite) TestGaugeOperations() { } } -func (suite *KeeperTestSuite) TestChargeFee() { +func (suite *KeeperTestSuite) TestChargeFeeIfSufficientFeeDenomBalance() { const baseFee = int64(100) testcases := map[string]struct { From 0f4a821f81ce2253ac3424b41cfd2584fdd6bc65 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 19:29:18 -0700 Subject: [PATCH 21/27] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70f47dada78..95766a487b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#2016](https://github.com/osmosis-labs/osmosis/pull/2016) Add fixed 10000 gas cost for each Balancer swap * [#2147](https://github.com/osmosis-labs/osmosis/pull/2147) Set MaxAgeNumBlocks in v11 Upgrade Handler to two weeks. * [#2193](https://github.com/osmosis-labs/osmosis/pull/2193) Add TwapKeeper to the Osmosis app +* [#2227](https://github.com/osmosis-labs/osmosis/pull/2227) Enable charging fee in base denom for `CreateGauge` and `AddToGauge`. #### Golang API breaks From 5c431266aacde60a7755fdd58cfdbc4950d94521 Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 19:31:58 -0700 Subject: [PATCH 22/27] comment --- x/incentives/keeper/gauge.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index dd9e141481f..cf4dc318c4a 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -288,9 +288,9 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context) epochtypes.EpochInfo { return k.ek.GetEpochInfo(ctx, params.DistrEpochIdentifier) } -// chargeFeeIfSufficientFeeDenomBalance charges fee in the base tx fee denom on the address if the address has -// balance that is less than fee + amount of the coin from gaugeCoins that is of base tx fee denom. -// gaugeCoins might not have a coin of tx base fee denom. In that case, only fee is compared to balance. +// chargeFeeIfSufficientFeeDenomBalance charges fee in the base denom on the address if the address has +// balance that is less than fee + amount of the coin from gaugeCoins that is of base denom. +// gaugeCoins might not have a coin of tx base denom. In that case, fee is only compared to balance. // The fee is sent to the community pool. // Returns nil on success, error otherwise. func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sdk.AccAddress, fee sdk.Int, gaugeCoins sdk.Coins) (err error) { From a16ba32174dc1c69c9c6b52cd474c0350fc5c258 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Sun, 24 Jul 2022 22:18:58 -0500 Subject: [PATCH 23/27] finished tests (#2230) * finished tests * use keeper instead of math * remove unused tests --- x/incentives/keeper/msg_server_test.go | 82 +++++++++++--------------- 1 file changed, 36 insertions(+), 46 deletions(-) diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index 03adec30042..c328e3ba290 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -33,25 +33,21 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(0))), }, { name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), }, { name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), }, { name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), isPerpetual: true, isModuleAccount: true, }, @@ -59,28 +55,24 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), isPerpetual: true, }, { name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), expectErr: true, }, { name: "user tries to create a non-perpetual gauge but does not have the correct fee denom", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), expectErr: true, }, { name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(30000000))), - expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), expectErr: true, }, } @@ -130,12 +122,14 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { } balanceAmount := bankKeeper.GetAllBalances(ctx, testAccountAddress) - suite.Require().Equal(tc.expectedEndBalance.String(), balanceAmount.String(), "test: %v", tc.name) if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), balanceAmount.String(), "test: %v", tc.name) } else { - suite.Require().Equal(tc.expectedEndBalance.String(), balanceAmount.String(), "test: %v", tc.name) + fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, incentiveskeeper.CreateGaugeFee)) + accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition) + finalAccountBalance := accountBalance.Sub(fee) + suite.Require().Equal(finalAccountBalance.String(), balanceAmount.String(), "test: %v", tc.name) } } @@ -147,68 +141,59 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { name string accountBalanceToFund sdk.Coins gaugeAddition sdk.Coins - expectedEndBalance sdk.Coins + nonexistentGauge bool isPerpetual bool isModuleAccount bool expectErr bool }{ { name: "user creates a non-perpetual gauge and fills gauge with all remaining tokens", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(35000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(0))), }, { name: "user creates a non-perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), }, { name: "user with multiple denoms creates a non-perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), }, { name: "module account creates a perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - isPerpetual: true, - isModuleAccount: true, + isPerpetual: true, + isModuleAccount: true, }, { name: "user with multiple denoms creates a perpetual gauge and fills gauge with some remaining tokens", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(70000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(70000000))), - isPerpetual: true, + isPerpetual: true, }, { name: "user tries to create a non-perpetual gauge but does not have enough funds to pay for the create gauge fee", - accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), + accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(20000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(40000000))), - expectErr: true, + expectErr: true, }, { - name: "user tries to create a non-perpetual gauge but does not have the correct fee denom", + name: "user tries to add to a non-perpetual gauge but does not have the correct fee denom", accountBalanceToFund: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), - //expectedEndBalance: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(60000000))), - expectErr: true, + expectErr: true, }, - // TODO: This is unexpected behavior - // We need validation to not charge fee if user doesn't have enough funds // { - // name: "one user tries to create a gauge, has enough funds to pay for the create gauge fee but not enough to fill the gauge", + // name: "user tries to add to a perpetual gauge but the gauge does not exist", // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), - // gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(30000000))), - // expectedEndBalance: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), + // gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), + // nonexistentGauge: true, + // isPerpetual: true, // expectErr: true, // }, - } for _, tc := range tests { @@ -218,7 +203,8 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { testAccountAddress := sdk.AccAddress(testAccountPubkey.Address()) ctx := suite.Ctx - incentivesKeepers := suite.App.IncentivesKeeper + bankKeeper := suite.App.BankKeeper + msgServer := keeper.NewMsgServerImpl(suite.App.IncentivesKeeper) suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) @@ -230,17 +216,20 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { suite.App.AccountKeeper.SetModuleAccount(ctx, modAcc) } - suite.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration) - distrTo := lockuptypes.QueryCondition{ - LockQueryType: lockuptypes.ByDuration, - Denom: defaultLPDenom, - Duration: defaultLockDuration, - } - // System under test. + coins := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(500000000))) + gaugeID, _, _, _ := suite.SetupNewGauge(true, coins) + if tc.nonexistentGauge { + gaugeID = suite.App.IncentivesKeeper.GetLastGaugeID(ctx) + 1 + } + msg := &types.MsgAddToGauge{ + Owner: testAccountAddress.String(), + GaugeId: gaugeID, + Rewards: tc.gaugeAddition, + } - _, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) - //incentivesKeepers.AddToGaugeRewards(ctx, testAccountAddress, tc.gaugeAddition) + //_, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) + _, err := msgServer.AddToGauge(sdk.WrapSDKContext(ctx), msg) if tc.expectErr { suite.Require().Error(err) @@ -248,14 +237,15 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { suite.Require().NoError(err) } - bal := suite.App.BankKeeper.GetAllBalances(suite.Ctx, testAccountAddress) - suite.Require().Equal(tc.expectedEndBalance.String(), bal.String(), "test: %v", tc.name) + bal := bankKeeper.GetAllBalances(ctx, testAccountAddress) if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) } else { - finalAccountBalalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition.Add(sdk.NewCoin(appparams.BaseCoinUnit, incentiveskeeper.CreateGaugeFee))) - suite.Require().Equal(finalAccountBalalance.String(), bal.String(), "test: %v", tc.name) + fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, incentiveskeeper.AddToGaugeFee)) + accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition) + finalAccountBalance := accountBalance.Sub(fee) + suite.Require().Equal(finalAccountBalance.String(), bal.String(), "test: %v", tc.name) } } From eba45d24082fc3386bba1894acc421a07026f2aa Mon Sep 17 00:00:00 2001 From: Roman Akhtariev Date: Sun, 24 Jul 2022 20:28:03 -0700 Subject: [PATCH 24/27] clean up --- x/incentives/keeper/msg_server_test.go | 27 +++++++++----------------- 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index c328e3ba290..f744a01a580 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -10,7 +10,6 @@ import ( appparams "github.com/osmosis-labs/osmosis/v10/app/params" "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" - incentiveskeeper "github.com/osmosis-labs/osmosis/v10/x/incentives/keeper" "github.com/osmosis-labs/osmosis/v10/x/incentives/types" lockuptypes "github.com/osmosis-labs/osmosis/v10/x/lockup/types" @@ -85,6 +84,7 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { ctx := suite.Ctx bankKeeper := suite.App.BankKeeper + accountKeeper := suite.App.AccountKeeper msgServer := keeper.NewMsgServerImpl(suite.App.IncentivesKeeper) suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) @@ -94,7 +94,7 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { "module", "permission", ) - suite.App.AccountKeeper.SetModuleAccount(ctx, modAcc) + accountKeeper.SetModuleAccount(ctx, modAcc) } suite.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration) @@ -126,12 +126,11 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), balanceAmount.String(), "test: %v", tc.name) } else { - fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, incentiveskeeper.CreateGaugeFee)) + fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, keeper.CreateGaugeFee)) accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition) finalAccountBalance := accountBalance.Sub(fee) suite.Require().Equal(finalAccountBalance.String(), balanceAmount.String(), "test: %v", tc.name) } - } } @@ -186,14 +185,6 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))), expectErr: true, }, - // { - // name: "user tries to add to a perpetual gauge but the gauge does not exist", - // accountBalanceToFund: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(60000000))), - // gaugeAddition: sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(10000000))), - // nonexistentGauge: true, - // isPerpetual: true, - // expectErr: true, - // }, } for _, tc := range tests { @@ -204,7 +195,9 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { ctx := suite.Ctx bankKeeper := suite.App.BankKeeper - msgServer := keeper.NewMsgServerImpl(suite.App.IncentivesKeeper) + incentivesKeeper := suite.App.IncentivesKeeper + accountKeeper := suite.App.AccountKeeper + msgServer := keeper.NewMsgServerImpl(incentivesKeeper) suite.FundAcc(testAccountAddress, tc.accountBalanceToFund) @@ -213,14 +206,14 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { "module", "permission", ) - suite.App.AccountKeeper.SetModuleAccount(ctx, modAcc) + accountKeeper.SetModuleAccount(ctx, modAcc) } // System under test. coins := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, sdk.NewInt(500000000))) gaugeID, _, _, _ := suite.SetupNewGauge(true, coins) if tc.nonexistentGauge { - gaugeID = suite.App.IncentivesKeeper.GetLastGaugeID(ctx) + 1 + gaugeID = incentivesKeeper.GetLastGaugeID(ctx) + 1 } msg := &types.MsgAddToGauge{ Owner: testAccountAddress.String(), @@ -228,7 +221,6 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { Rewards: tc.gaugeAddition, } - //_, err := incentivesKeepers.CreateGauge(ctx, tc.isPerpetual, testAccountAddress, tc.gaugeAddition, distrTo, time.Time{}, 1) _, err := msgServer.AddToGauge(sdk.WrapSDKContext(ctx), msg) if tc.expectErr { @@ -242,11 +234,10 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) } else { - fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, incentiveskeeper.AddToGaugeFee)) + fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, keeper.AddToGaugeFee)) accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition) finalAccountBalance := accountBalance.Sub(fee) suite.Require().Equal(finalAccountBalance.String(), bal.String(), "test: %v", tc.name) } - } } From 87dd1fe8423b4b680826375138ddfd4d2337489d Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Sun, 24 Jul 2022 23:26:34 -0500 Subject: [PATCH 25/27] sim only allow accounts with enough to pay fee --- x/incentives/simulation/operations.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/x/incentives/simulation/operations.go b/x/incentives/simulation/operations.go index 10b281f4a09..58864220819 100644 --- a/x/incentives/simulation/operations.go +++ b/x/incentives/simulation/operations.go @@ -18,6 +18,8 @@ import ( simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/simulation" stakingTypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + appparams "github.com/osmosis-labs/osmosis/v10/app/params" ) // Simulation operation weights constants. @@ -28,6 +30,13 @@ const ( OpWeightMsgAddToGauge = "op_weight_msg_add_to_gauge" ) +var ( + // createGaugeFee is the fee required to create a new gauge. + createGaugeFee = sdk.NewInt(50 * 1_000_000) + // addToGagugeFee is the fee required to add to gauge. + addToGaugeFee = sdk.NewInt(25 * 1_000_000) +) + // WeightedOperations returns all the operations from the module with their respective weights. func WeightedOperations( appParams simtypes.AppParams, cdc codec.JSONCodec, ak stakingTypes.AccountKeeper, @@ -115,7 +124,7 @@ func SimulateMsgCreateGauge(ak stakingTypes.AccountKeeper, bk stakingTypes.BankK ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { simAccount, _ := simtypes.RandomAcc(r, accs) simCoins := bk.SpendableCoins(ctx, simAccount.Address) - if simCoins.Len() <= 0 { + if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(createGaugeFee) { return simtypes.NoOpMsg( types.ModuleName, types.TypeMsgCreateGauge, "Account have no coin"), nil, nil } @@ -154,7 +163,7 @@ func SimulateMsgAddToGauge(ak stakingTypes.AccountKeeper, bk stakingTypes.BankKe ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { simAccount, _ := simtypes.RandomAcc(r, accs) simCoins := bk.SpendableCoins(ctx, simAccount.Address) - if simCoins.Len() <= 0 { + if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(createGaugeFee) { return simtypes.NoOpMsg( types.ModuleName, types.TypeMsgAddToGauge, "Account have no coin"), nil, nil } From 86988760b0b90be423821f3db3ec692e0109a014 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Sun, 24 Jul 2022 23:31:34 -0500 Subject: [PATCH 26/27] lint --- x/incentives/simulation/operations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/incentives/simulation/operations.go b/x/incentives/simulation/operations.go index 58864220819..f989028dbee 100644 --- a/x/incentives/simulation/operations.go +++ b/x/incentives/simulation/operations.go @@ -163,7 +163,7 @@ func SimulateMsgAddToGauge(ak stakingTypes.AccountKeeper, bk stakingTypes.BankKe ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { simAccount, _ := simtypes.RandomAcc(r, accs) simCoins := bk.SpendableCoins(ctx, simAccount.Address) - if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(createGaugeFee) { + if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(addToGaugeFee) { return simtypes.NoOpMsg( types.ModuleName, types.TypeMsgAddToGauge, "Account have no coin"), nil, nil } From b2d64371c5ce39b8eeb937eda0119ae9b4c3a807 Mon Sep 17 00:00:00 2001 From: Roman Date: Mon, 25 Jul 2022 20:56:57 -0400 Subject: [PATCH 27/27] move fee to types and reuse in simulation (#2234) --- x/incentives/keeper/export_test.go | 7 ------- x/incentives/keeper/gauge.go | 7 ------- x/incentives/keeper/msg_server.go | 4 ++-- x/incentives/keeper/msg_server_test.go | 4 ++-- x/incentives/simulation/operations.go | 11 ++--------- x/incentives/types/gauge.go | 7 +++++++ 6 files changed, 13 insertions(+), 27 deletions(-) diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index 47602af7137..9eeb06ac191 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -6,13 +6,6 @@ import ( "github.com/osmosis-labs/osmosis/v10/x/incentives/types" ) -var ( - // CreateGaugeFee is the fee required to create a new gauge. - CreateGaugeFee = createGaugeFee - // AddToGagugeFee is the fee required to add to gauge. - AddToGaugeFee = addToGaugeFee -) - // AddGaugeRefByKey appends the provided gauge ID into an array associated with the provided key. func (k Keeper) AddGaugeRefByKey(ctx sdk.Context, key []byte, gaugeID uint64) error { return k.addGaugeRefByKey(ctx, key, gaugeID) diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index cf4dc318c4a..8a8638bc243 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -19,13 +19,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -var ( - // createGaugeFee is the fee required to create a new gauge. - createGaugeFee = sdk.NewInt(50 * 1_000_000) - // addToGagugeFee is the fee required to add to gauge. - addToGaugeFee = sdk.NewInt(25 * 1_000_000) -) - // getGaugesFromIterator iterates over everything in a gauge's iterator, until it reaches the end. Return all gauges iterated over. func (k Keeper) getGaugesFromIterator(ctx sdk.Context, iterator db.Iterator) []types.Gauge { gauges := []types.Gauge{} diff --git a/x/incentives/keeper/msg_server.go b/x/incentives/keeper/msg_server.go index f936c19f967..c966ab5f4bb 100644 --- a/x/incentives/keeper/msg_server.go +++ b/x/incentives/keeper/msg_server.go @@ -33,7 +33,7 @@ func (server msgServer) CreateGauge(goCtx context.Context, msg *types.MsgCreateG return nil, err } - if err := server.keeper.chargeFeeIfSufficientFeeDenomBalance(ctx, owner, createGaugeFee, msg.Coins); err != nil { + if err := server.keeper.chargeFeeIfSufficientFeeDenomBalance(ctx, owner, types.CreateGaugeFee, msg.Coins); err != nil { return nil, err } @@ -61,7 +61,7 @@ func (server msgServer) AddToGauge(goCtx context.Context, msg *types.MsgAddToGau return nil, err } - if err := server.keeper.chargeFeeIfSufficientFeeDenomBalance(ctx, owner, addToGaugeFee, msg.Rewards); err != nil { + if err := server.keeper.chargeFeeIfSufficientFeeDenomBalance(ctx, owner, types.AddToGaugeFee, msg.Rewards); err != nil { return nil, err } err = server.keeper.AddToGaugeRewards(ctx, owner, msg.Rewards, msg.GaugeId) diff --git a/x/incentives/keeper/msg_server_test.go b/x/incentives/keeper/msg_server_test.go index f744a01a580..497a5d2d92b 100644 --- a/x/incentives/keeper/msg_server_test.go +++ b/x/incentives/keeper/msg_server_test.go @@ -126,7 +126,7 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() { if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), balanceAmount.String(), "test: %v", tc.name) } else { - fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, keeper.CreateGaugeFee)) + fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, types.CreateGaugeFee)) accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition) finalAccountBalance := accountBalance.Sub(fee) suite.Require().Equal(finalAccountBalance.String(), balanceAmount.String(), "test: %v", tc.name) @@ -234,7 +234,7 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() { if tc.expectErr { suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name) } else { - fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, keeper.AddToGaugeFee)) + fee := sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, types.AddToGaugeFee)) accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition) finalAccountBalance := accountBalance.Sub(fee) suite.Require().Equal(finalAccountBalance.String(), bal.String(), "test: %v", tc.name) diff --git a/x/incentives/simulation/operations.go b/x/incentives/simulation/operations.go index f989028dbee..81d4e0854c7 100644 --- a/x/incentives/simulation/operations.go +++ b/x/incentives/simulation/operations.go @@ -30,13 +30,6 @@ const ( OpWeightMsgAddToGauge = "op_weight_msg_add_to_gauge" ) -var ( - // createGaugeFee is the fee required to create a new gauge. - createGaugeFee = sdk.NewInt(50 * 1_000_000) - // addToGagugeFee is the fee required to add to gauge. - addToGaugeFee = sdk.NewInt(25 * 1_000_000) -) - // WeightedOperations returns all the operations from the module with their respective weights. func WeightedOperations( appParams simtypes.AppParams, cdc codec.JSONCodec, ak stakingTypes.AccountKeeper, @@ -124,7 +117,7 @@ func SimulateMsgCreateGauge(ak stakingTypes.AccountKeeper, bk stakingTypes.BankK ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { simAccount, _ := simtypes.RandomAcc(r, accs) simCoins := bk.SpendableCoins(ctx, simAccount.Address) - if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(createGaugeFee) { + if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(types.CreateGaugeFee) { return simtypes.NoOpMsg( types.ModuleName, types.TypeMsgCreateGauge, "Account have no coin"), nil, nil } @@ -163,7 +156,7 @@ func SimulateMsgAddToGauge(ak stakingTypes.AccountKeeper, bk stakingTypes.BankKe ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { simAccount, _ := simtypes.RandomAcc(r, accs) simCoins := bk.SpendableCoins(ctx, simAccount.Address) - if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(addToGaugeFee) { + if simCoins.Len() <= 0 || simCoins.AmountOf(appparams.BaseCoinUnit).LT(types.AddToGaugeFee) { return simtypes.NoOpMsg( types.ModuleName, types.TypeMsgAddToGauge, "Account have no coin"), nil, nil } diff --git a/x/incentives/types/gauge.go b/x/incentives/types/gauge.go index bb0f97d087c..ec3b112d31f 100644 --- a/x/incentives/types/gauge.go +++ b/x/incentives/types/gauge.go @@ -8,6 +8,13 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +var ( + // CreateGaugeFee is the fee required to create a new gauge. + CreateGaugeFee = sdk.NewInt(50 * 1_000_000) + // AddToGagugeFee is the fee required to add to gauge. + AddToGaugeFee = sdk.NewInt(25 * 1_000_000) +) + // NewGauge creates a new gauge struct given the required gauge parameters. func NewGauge(id uint64, isPerpetual bool, distrTo lockuptypes.QueryCondition, coins sdk.Coins, startTime time.Time, numEpochsPaidOver uint64, filledEpochs uint64, distrCoins sdk.Coins) Gauge { return Gauge{