From 84c5ad618ca903ffbbb6e6ed02fc7fe00cf30f4d Mon Sep 17 00:00:00 2001 From: Sishir Giri Date: Fri, 21 Apr 2023 10:16:31 -0700 Subject: [PATCH] [Incentives Module][bugfix] Scale gas costs by denoms in gauge (#4830) * added gas cost for add reward to gauge * added more test * nit * more nit * fixed calc * added changelog --------- Co-authored-by: Roman --- CHANGELOG.md | 1 + x/incentives/keeper/gauge.go | 4 ++ x/incentives/keeper/gauge_test.go | 97 +++++++++++++++++++++++++++++++ x/incentives/types/constants.go | 5 ++ 4 files changed, 107 insertions(+) create mode 100644 x/incentives/types/constants.go diff --git a/CHANGELOG.md b/CHANGELOG.md index dc3fad7c8e8..4db26e9dbe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#4659](https://github.com/osmosis-labs/osmosis/pull/4659) implement AllPools query in x/poolmanager. * [#4783](https://github.com/osmosis-labs/osmosis/pull/4783) Update wasmd to 0.31.0 + * [#4830](https://github.com/osmosis-labs/osmosis/pull/4830) Add gas cost when we AddToGaugeRewards, linearly increase with coins to add * [#4886](https://github.com/osmosis-labs/osmosis/pull/4886) Implement MsgSplitRouteSwapExactAmountIn and MsgSplitRouteSwapExactAmountOut that supports route splitting. ### Misc Improvements diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index c69727b1405..4df9b666feb 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -152,6 +152,10 @@ func (k Keeper) AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins s if gauge.IsFinishedGauge(ctx.BlockTime()) { return errors.New("gauge is already completed") } + + // Fixed gas consumption adding reward to gauges based on the number of coins to add + ctx.GasMeter().ConsumeGas(uint64(types.BaseGasFeeForAddRewardToGauge*(len(coins)+len(gauge.Coins))), "scaling gas cost for adding to gauge rewards") + if err := k.bk.SendCoinsFromAccountToModule(ctx, owner, types.ModuleName, coins); err != nil { return err } diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index bc8a0c4f25b..ef672054e85 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "time" "github.com/stretchr/testify/suite" @@ -332,3 +333,99 @@ func (suite *KeeperTestSuite) TestChargeFeeIfSufficientFeeDenomBalance() { }) } } + +func (suite *KeeperTestSuite) TestAddToGaugeRewards() { + testCases := []struct { + name string + owner sdk.AccAddress + coinsToAdd sdk.Coins + gaugeId uint64 + minimumGasConsumed uint64 + + expectErr bool + }{ + { + name: "valid case: valid gauge", + owner: suite.TestAccs[0], + coinsToAdd: sdk.NewCoins( + sdk.NewCoin("uosmo", sdk.NewInt(100000)), + sdk.NewCoin("atom", sdk.NewInt(99999)), + ), + gaugeId: 1, + minimumGasConsumed: uint64(2 * types.BaseGasFeeForAddRewardToGauge), + + expectErr: false, + }, + { + name: "valid case: valid gauge with >4 denoms", + owner: suite.TestAccs[0], + coinsToAdd: sdk.NewCoins( + sdk.NewCoin("uosmo", sdk.NewInt(100000)), + sdk.NewCoin("atom", sdk.NewInt(99999)), + sdk.NewCoin("mars", sdk.NewInt(88888)), + sdk.NewCoin("akash", sdk.NewInt(77777)), + sdk.NewCoin("eth", sdk.NewInt(6666)), + sdk.NewCoin("usdc", sdk.NewInt(555)), + sdk.NewCoin("dai", sdk.NewInt(4444)), + sdk.NewCoin("ust", sdk.NewInt(3333)), + ), + gaugeId: 1, + minimumGasConsumed: uint64(8 * types.BaseGasFeeForAddRewardToGauge), + + expectErr: false, + }, + { + name: "invalid case: gauge Id is not valid", + owner: suite.TestAccs[0], + coinsToAdd: sdk.NewCoins( + sdk.NewCoin("uosmo", sdk.NewInt(100000)), + sdk.NewCoin("atom", sdk.NewInt(99999)), + ), + gaugeId: 0, + minimumGasConsumed: uint64(0), + + expectErr: true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.SetupTest() + _, _, existingGaugeCoins, _ := suite.SetupNewGauge(true, sdk.Coins{sdk.NewInt64Coin("stake", 12)}) + + suite.FundAcc(tc.owner, tc.coinsToAdd) + + existingGasConsumed := suite.Ctx.GasMeter().GasConsumed() + + err := suite.App.IncentivesKeeper.AddToGaugeRewards(suite.Ctx, tc.owner, tc.coinsToAdd, tc.gaugeId) + if tc.expectErr { + suite.Require().Error(err) + + // balance shouldn't change in the module + balance := suite.App.BankKeeper.GetAllBalances(suite.Ctx, suite.App.AccountKeeper.GetModuleAddress(types.ModuleName)) + suite.Require().Equal(existingGaugeCoins, balance) + + } else { + suite.Require().NoError(err) + + // Ensure that at least the minimum amount of gas was charged (based on number of additional gauge coins) + gasConsumed := suite.Ctx.GasMeter().GasConsumed() - existingGasConsumed + fmt.Println(gasConsumed, tc.minimumGasConsumed) + suite.Require().True(gasConsumed >= tc.minimumGasConsumed) + + // existing coins gets added to the module when we create gauge and add to gauge + expectedCoins := existingGaugeCoins.Add(tc.coinsToAdd...) + + // check module account balance, should go up + balance := suite.App.BankKeeper.GetAllBalances(suite.Ctx, suite.App.AccountKeeper.GetModuleAddress(types.ModuleName)) + suite.Require().Equal(expectedCoins, balance) + + // check gauge coins should go up + gauge, err := suite.App.IncentivesKeeper.GetGaugeByID(suite.Ctx, tc.gaugeId) + suite.Require().NoError(err) + + suite.Require().Equal(expectedCoins, gauge.Coins) + } + }) + } +} diff --git a/x/incentives/types/constants.go b/x/incentives/types/constants.go new file mode 100644 index 00000000000..eab79b004b9 --- /dev/null +++ b/x/incentives/types/constants.go @@ -0,0 +1,5 @@ +package types + +var ( + BaseGasFeeForAddRewardToGauge = 10_000 +)