Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent adding to a finished gauge #4306

Merged
merged 4 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* [#3715](https://github.com/osmosis-labs/osmosis/pull/3715) Fix x/gamm (golang API) CalculateSpotPrice, balancer.SpotPrice and Stableswap.SpotPrice base and quote asset.
* [#3746](https://github.com/osmosis-labs/osmosis/pull/3746) Make ApplyFuncIfNoErr logic preserve panics for OutOfGas behavior.


* [#4306](https://github.com/osmosis-labs/osmosis/pull/4306) Prevent adding more tokens to an already finished gauge

## v14

Expand Down
4 changes: 4 additions & 0 deletions x/incentives/keeper/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -148,6 +149,9 @@ func (k Keeper) AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins s
if err != nil {
return err
}
if gauge.IsFinishedGauge(ctx.BlockTime()) {
return errors.New("gauge is already completed")
}
if err := k.bk.SendCoinsFromAccountToModule(ctx, owner, types.ModuleName, coins); err != nil {
return err
}
Expand Down
81 changes: 57 additions & 24 deletions x/incentives/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import (

var _ = suite.TestingSuite(nil)

var seventyTokens = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(70000000)))
var tenTokens = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000)))

func (suite *KeeperTestSuite) TestCreateGauge_Fee() {
tests := []struct {
name string
Expand All @@ -30,35 +33,35 @@ func (suite *KeeperTestSuite) TestCreateGauge_Fee() {
{
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))),
gaugeAddition: tenTokens,
},
{
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))),
gaugeAddition: tenTokens,
},
{
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))),
gaugeAddition: tenTokens,
},
{
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))),
gaugeAddition: tenTokens,
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))),
gaugeAddition: tenTokens,
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))),
gaugeAddition: tenTokens,
expectErr: true,
},
{
Expand Down Expand Up @@ -141,40 +144,41 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() {
nonexistentGauge bool
isPerpetual bool
isModuleAccount bool
isGaugeComplete 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(35000000))),
gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))),
gaugeAddition: tenTokens,
},
{
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))),
accountBalanceToFund: seventyTokens,
gaugeAddition: tenTokens,
},
{
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))),
gaugeAddition: tenTokens,
},
{
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))),
gaugeAddition: tenTokens,
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))),
gaugeAddition: tenTokens,
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(20000000))),
gaugeAddition: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000))),
gaugeAddition: tenTokens,
expectErr: true,
},
{
Expand All @@ -183,6 +187,13 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() {
gaugeAddition: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000000))),
expectErr: true,
},
{
name: "user tries to add to a finished gauge",
accountBalanceToFund: seventyTokens,
gaugeAddition: tenTokens,
isGaugeComplete: true,
expectErr: true,
},
}

for _, tc := range tests {
Expand All @@ -191,7 +202,6 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() {
testAccountPubkey := secp256k1.GenPrivKeyFromSecret([]byte("acc")).PubKey()
testAccountAddress := sdk.AccAddress(testAccountPubkey.Address())

ctx := suite.Ctx
bankKeeper := suite.App.BankKeeper
incentivesKeeper := suite.App.IncentivesKeeper
accountKeeper := suite.App.AccountKeeper
Expand All @@ -204,38 +214,61 @@ func (suite *KeeperTestSuite) TestAddToGauge_Fee() {
"module",
"permission",
)
accountKeeper.SetModuleAccount(ctx, modAcc)
accountKeeper.SetModuleAccount(suite.Ctx, modAcc)
}

// System under test.
coins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(500000000)))
gaugeID, _, _, _ := suite.SetupNewGauge(true, coins)
gaugeID, gauge, _, _ := suite.SetupNewGauge(tc.isPerpetual, coins)
if tc.nonexistentGauge {
gaugeID = incentivesKeeper.GetLastGaugeID(ctx) + 1
gaugeID = incentivesKeeper.GetLastGaugeID(suite.Ctx) + 1
}
// simulate times to complete the gauge.
if tc.isGaugeComplete {
suite.completeGauge(gauge, sdk.AccAddress([]byte("a___________________")))
}
msg := &types.MsgAddToGauge{
Owner: testAccountAddress.String(),
GaugeId: gaugeID,
Rewards: tc.gaugeAddition,
}

_, err := msgServer.AddToGauge(sdk.WrapSDKContext(ctx), msg)
_, err := msgServer.AddToGauge(sdk.WrapSDKContext(suite.Ctx), msg)

if tc.expectErr {
suite.Require().Error(err)
suite.Require().Error(err, tc.name)
} else {
suite.Require().NoError(err)
suite.Require().NoError(err, tc.name)
}

bal := bankKeeper.GetAllBalances(ctx, testAccountAddress)
bal := bankKeeper.GetAllBalances(suite.Ctx, testAccountAddress)

if tc.expectErr {
suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name)
} else {
if !tc.expectErr {
fee := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, types.AddToGaugeFee))
accountBalance := tc.accountBalanceToFund.Sub(tc.gaugeAddition)
finalAccountBalance := accountBalance.Sub(fee)
suite.Require().Equal(finalAccountBalance.String(), bal.String(), "test: %v", tc.name)
} else if tc.expectErr && !tc.isGaugeComplete {
suite.Require().Equal(tc.accountBalanceToFund.String(), bal.String(), "test: %v", tc.name)
}
}
}

func (suite *KeeperTestSuite) completeGauge(gauge *types.Gauge, sendingAddress sdk.AccAddress) {
lockCoins := sdk.NewCoin(gauge.DistributeTo.Denom, sdk.NewInt(1000))
suite.FundAcc(sendingAddress, sdk.NewCoins(lockCoins))
suite.LockTokens(sendingAddress, sdk.NewCoins(lockCoins), gauge.DistributeTo.Duration)
epochId := suite.App.IncentivesKeeper.GetEpochInfo(suite.Ctx).Identifier
if suite.Ctx.BlockTime().Before(gauge.StartTime) {
suite.Ctx = suite.Ctx.WithBlockTime(gauge.StartTime.Add(time.Hour))
}
suite.BeginNewBlock(false)
for i := 0; i < int(gauge.NumEpochsPaidOver); i++ {
suite.App.IncentivesKeeper.BeforeEpochStart(suite.Ctx, epochId, int64(i))
suite.App.IncentivesKeeper.AfterEpochEnd(suite.Ctx, epochId, int64(i))
}
suite.BeginNewBlock(false)
gauge2, err := suite.App.IncentivesKeeper.GetGaugeByID(suite.Ctx, gauge.Id)
suite.Require().NoError(err)
suite.Require().True(gauge2.IsFinishedGauge(suite.Ctx.BlockTime()))
}
8 changes: 5 additions & 3 deletions x/incentives/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,16 @@ func SimulateMsgAddToGauge(ak stakingTypes.AccountKeeper, bk stakingTypes.BankKe
if gauge == nil {
return simtypes.NoOpMsg(
types.ModuleName, types.TypeMsgAddToGauge, "No gauge exists"), nil, nil
} else if gauge.IsFinishedGauge(ctx.BlockTime()) {
// TODO: Ideally we'd still run this but expect failure.
return simtypes.NoOpMsg(
types.ModuleName, types.TypeMsgAddToGauge, "Selected a gauge that is finished"), nil, nil
}
gaugeId := RandomGauge(ctx, r, k).Id

rewards := genRewardCoins(r, simCoins, types.AddToGaugeFee)

msg := types.MsgAddToGauge{
Owner: simAccount.Address.String(),
GaugeId: gaugeId,
GaugeId: gauge.Id,
Rewards: rewards,
}

Expand Down
2 changes: 1 addition & 1 deletion x/incentives/types/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (gauge Gauge) IsUpcomingGauge(curTime time.Time) bool {

// IsActiveGauge returns true if the gauge is in an active state during the provided time.
func (gauge Gauge) IsActiveGauge(curTime time.Time) bool {
if curTime.After(gauge.StartTime) || curTime.Equal(gauge.StartTime) && (gauge.IsPerpetual || gauge.FilledEpochs < gauge.NumEpochsPaidOver) {
if (curTime.After(gauge.StartTime) || curTime.Equal(gauge.StartTime)) && (gauge.IsPerpetual || gauge.FilledEpochs < gauge.NumEpochsPaidOver) {
return true
}
return false
Expand Down