[VS Incentives] refactor/test: move group gauge creation to CreateGau…

…ge; address spam concerns

x/incentives/keeper/distribute_test.go

@@ -464,7 +464,7 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() {
 	withGaugeCoins := func(tc test, gaugeCoins sdk.Coins) test {
 		tc.gaugeCoins = gaugeCoins
 		tc.expectedDistributions = gaugeCoins
-		tc.expectedRemainingAmountIncentiveRecord = make([]sdk.Dec, len(gaugeCoins))
+		tc.expectedRemainingAmountIncentiveRecord = make([]osmomath.Dec, len(gaugeCoins))
 		for i := range tc.expectedRemainingAmountIncentiveRecord {
 			tc.expectedRemainingAmountIncentiveRecord[i] = osmomath.NewDec(gaugeCoins[i].Amount.Int64())
 		}
@@ -539,25 +539,20 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() {
 			// can function properly.
 			s.Ctx = s.Ctx.WithBlockTime(oneHourAfterDefault)
 
-			s.FundAcc(s.TestAccs[0], tc.gaugeCoins)
-
 			// Create gauge and get it from state
-			externalGaugeid, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, tc.isPerpertual, s.TestAccs[0], tc.gaugeCoins, tc.distrTo, tc.startTime, tc.numEpochsPaidOver, defaultCLPool)
-			s.Require().NoError(err)
-			externalGauge, err := s.App.IncentivesKeeper.GetGaugeByID(s.Ctx, externalGaugeid)
-			s.Require().NoError(err)
+			externalGauge := s.createGaugeNoRestrictions(tc.isPerpertual, tc.gaugeCoins, tc.distrTo, tc.startTime, tc.numEpochsPaidOver, defaultCLPool)
 
 			// Force gauge's pool id to balancer to trigger error
 			if tc.poolId == defaultBalancerPool {
-				err := s.App.PoolIncentivesKeeper.SetPoolGaugeIdInternalIncentive(s.Ctx, defaultBalancerPool, tc.distrTo.Duration, externalGaugeid)
+				err := s.App.PoolIncentivesKeeper.SetPoolGaugeIdInternalIncentive(s.Ctx, defaultBalancerPool, tc.distrTo.Duration, externalGauge.Id)
 				s.Require().NoError(err)
 			}
 
 			// Activate the gauge.
-			err = s.App.IncentivesKeeper.MoveUpcomingGaugeToActiveGauge(s.Ctx, *externalGauge)
+			err := s.App.IncentivesKeeper.MoveUpcomingGaugeToActiveGauge(s.Ctx, externalGauge)
 			s.Require().NoError(err)
 
-			gauges := []types.Gauge{*externalGauge}
+			gauges := []types.Gauge{externalGauge}
 
 			// System under test.
 			totalDistributedCoins, err := s.App.IncentivesKeeper.Distribute(s.Ctx, gauges)
@@ -568,7 +563,7 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() {
 				s.Require().NoError(err)
 
 				// check the totalAmount of tokens distributed, for both lock gauges and CL pool gauges
-				s.Require().Equal(tc.expectedDistributions, totalDistributedCoins)
+				s.Require().Equal(tc.expectedDistributions.String(), totalDistributedCoins.String())
 
 				incentivesEpochDuration := s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration
 				incentivesEpochDurationSeconds := osmomath.NewDec(incentivesEpochDuration.Milliseconds()).QuoInt(osmomath.NewInt(1000))
@@ -588,7 +583,7 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() {
 				}
 
 				// Check that the gauge's distribution state was updated
-				s.ValidateDistributedGauge(externalGaugeid, 1, tc.expectedDistributions)
+				s.ValidateDistributedGauge(externalGauge.Id, 1, tc.expectedDistributions)
 			}
 		})
 	}

x/incentives/keeper/export_test.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"time"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/osmosis-labs/osmosis/osmomath"
@@ -9,6 +11,8 @@ import (
 
 const PerpetualNumEpochsPaidOver = perpetualNumEpochsPaidOver
 
+var ByGroupQueryCondition = byGroupQueryCondition
+
 // 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)
@@ -75,3 +79,11 @@ func (k Keeper) InitGaugeInfo(ctx sdk.Context, poolIds []uint64) (types.Internal
 func RegularGaugeStoreKey(ID uint64) []byte {
 	return gaugeStoreKey(ID)
 }
+
+func CombineKeys(keys ...[]byte) []byte {
+	return combineKeys(keys...)
+}
+
+func GetTimeKeys(timestamp time.Time) []byte {
+	return getTimeKey(timestamp)
+}

x/incentives/keeper/gauge.go

@@ -16,6 +16,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
+
 	"github.com/osmosis-labs/osmosis/osmomath"
 	"github.com/osmosis-labs/osmosis/v19/x/incentives/types"
 	lockuptypes "github.com/osmosis-labs/osmosis/v19/x/lockup/types"
@@ -26,7 +27,9 @@ import (
 // numEpochPaidOver is the number of epochs that must be given
 // for a gauge to be perpetual. For any other number of epochs
 // other than zero, the gauge is non-perpetual. Zero is invalid.
-const perpetualNumEpochsPaidOver = 1
+const perpetualNumEpochsPaidOver = 0
+
+var byGroupQueryCondition = lockuptypes.QueryCondition{LockQueryType: lockuptypes.ByGroup}
 
 // 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 {
@@ -104,7 +107,7 @@ func (k Keeper) SetGaugeWithRefKey(ctx sdk.Context, gauge *types.Gauge) error {
 }
 
 // CreateGauge creates a gauge with the given parameters and sends coins to the gauge.
-// There can be 2 kinds of gauges for a given set of parameters:
+// There can be 3 kinds of gauges for a given set of parameters:
 // * lockuptypes.ByDuration - a gauge that incentivizes one of the lockable durations.
 // For this gauge, the pool id must be 0. Fails if not.
 //
@@ -116,7 +119,21 @@ func (k Keeper) SetGaugeWithRefKey(ctx sdk.Context, gauge *types.Gauge) error {
 // this is an external gauge, or be equal to types.NoLockInternalGaugeDenom(poolId).
 // If the denom is empty, it will get overwritten to types.NoLockExternalGaugeDenom(poolId).
 // This denom formatting is useful for querying internal vs external gauges associated with a pool.
+// * lockuptypes.Group - a gauge that incentivizes a group of internal pool gauges based on the splitting
+// policy created by a group data structure. It is expected to be created via CreateGroup keeper method.
+// This gauge is the only gauge type that does not have ref keys (active/upcoming/finished) created and
+// associated with it.
+// For this gauge, the pool id must be 0. Fails if not.
+//
+// Returns error if:
+// - attempts to create non-perpetual gauge with numEpochsPaidOver of 0
+//
+// On success, returns the gauge ID.
 func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddress, coins sdk.Coins, distrTo lockuptypes.QueryCondition, startTime time.Time, numEpochsPaidOver uint64, poolId uint64) (uint64, error) {
+	if numEpochsPaidOver == perpetualNumEpochsPaidOver && !isPerpetual {
+		return 0, types.ErrZeroNumEpochsPaidOver
+	}
+
 	// Ensure that this gauge's duration is one of the allowed durations on chain
 	durations := k.GetLockableDurations(ctx)
 	if distrTo.LockQueryType == lockuptypes.ByDuration {
@@ -174,13 +191,16 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 			return 0, fmt.Errorf("pool id must be 0 for gauges with lock")
 		}
 
-		// check if denom this gauge pays out to exists on-chain
-		// N.B.: The reason we check for osmovaloper is to account for gauges that pay out to
-		// superfluid synthetic locks. These locks have the following format:
-		// "cl/pool/1/superbonding/osmovaloper1wcfyglfgjs2xtsyqu7pl60d0mpw5g7f4wh7pnm"
-		// See x/superfluid module README for details.
-		if !k.bk.HasSupply(ctx, distrTo.Denom) && !strings.Contains(distrTo.Denom, "osmovaloper") {
-			return 0, fmt.Errorf("denom does not exist: %s", distrTo.Denom)
+		// Group gauges do not distribute to a denom. skip this check for group gauges.
+		if distrTo.LockQueryType != lockuptypes.ByGroup {
+			// check if denom this gauge pays out to exists on-chain
+			// N.B.: The reason we check for osmovaloper is to account for gauges that pay out to
+			// superfluid synthetic locks. These locks have the following format:
+			// "cl/pool/1/superbonding/osmovaloper1wcfyglfgjs2xtsyqu7pl60d0mpw5g7f4wh7pnm"
+			// See x/superfluid module README for details.
+			if !k.bk.HasSupply(ctx, distrTo.Denom) && !strings.Contains(distrTo.Denom, "osmovaloper") {
+				return 0, fmt.Errorf("denom does not exist: %s", distrTo.Denom)
+			}
 		}
 	}
 
@@ -208,10 +228,15 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 
 	combinedKeys := combineKeys(types.KeyPrefixUpcomingGauges, getTimeKey(gauge.StartTime))
 
-	err = k.CreateGaugeRefKeys(ctx, &gauge, combinedKeys)
-	if err != nil {
-		return 0, err
+	// Only create ref keys (upcoming/active/finished) if gauge is not a group gauge
+	// Group gauges do not follow a similar lifecycle as other gauges.
+	if gauge.DistributeTo.LockQueryType != lockuptypes.ByGroup {
+		err = k.CreateGaugeRefKeys(ctx, &gauge, combinedKeys)
+		if err != nil {
+			return 0, err
+		}
 	}
+
 	k.hooks.AfterCreateGauge(ctx, gauge.Id)
 	return gauge.Id, nil
 }
@@ -222,11 +247,10 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 // Note, that implies that only perpetual pool gauges can be associated with the Group.
 // For Group's own distribution policy, a 1:1 group Gauge is created. This is the Gauge that receives incentives at the end of an epoch
 // in the pool incentives as defined by the DistrRecord. The Group's Gauge can either be perpetual or non-perpetual.
-// If numEpochPaidOver is 1, then the Group's Gauge is perpetual. Otherwise, it is non-perpetual.
+// If numEpochPaidOver is 0, then the Group's Gauge is perpetual. Otherwise, it is non-perpetual.
 // Returns nil on success.
 // Returns error if:
 // - given pool IDs slice is empty or has 1 pool only
-// - numEpochPaidOver is 0
 // - fails to initialize gauge information for every pool ID
 // - fails to send coins from owner to the incentives module for the Group's Gauge
 // - fails to set the Group's Gauge to state
@@ -237,9 +261,6 @@ func (k Keeper) CreateGroup(ctx sdk.Context, coins sdk.Coins, numEpochPaidOver u
 	if len(poolIDs) == 1 {
 		return 0, types.OnePoolIDGroupError{PoolID: poolIDs[0]}
 	}
-	if numEpochPaidOver == 0 {
-		return 0, types.ErrZeroNumEpochsPaidOver
-	}
 
 	// Initialize gauge information for every pool ID.
 	initialInternalGaugeInfo, err := k.initGaugeInfo(ctx, poolIDs)
@@ -258,34 +279,13 @@ func (k Keeper) CreateGroup(ctx sdk.Context, coins sdk.Coins, numEpochPaidOver u
 	// Make sure to update method spec and tests.
 	// https://github.com/osmosis-labs/osmosis/issues/6507
 
-	// TODO: remove gauge creation logic from here.
-	// Instead, call `CreateGauge` directly
-	// Update `CreateGauge` to be able to handle the group type.
-	// Make sure to update method spec and tests.
-	// https://github.com/osmosis-labs/osmosis/issues/6513
-	nextGaugeId := k.GetLastGaugeID(ctx) + 1
-
-	gauge := types.Gauge{
-		Id:          nextGaugeId,
-		IsPerpetual: numEpochPaidOver == perpetualNumEpochsPaidOver,
-		DistributeTo: lockuptypes.QueryCondition{
-			LockQueryType: lockuptypes.ByGroup,
-		},
-		Coins:             coins,
-		StartTime:         ctx.BlockTime(),
-		NumEpochsPaidOver: numEpochPaidOver,
-	}
-
-	if err := k.bk.SendCoinsFromAccountToModule(ctx, owner, types.ModuleName, gauge.Coins); err != nil {
-		return 0, err
-	}
-
-	if err := k.setGauge(ctx, &gauge); err != nil {
+	groupGaugeID, err := k.CreateGauge(ctx, numEpochPaidOver == perpetualNumEpochsPaidOver, owner, coins, byGroupQueryCondition, ctx.BlockTime(), numEpochPaidOver, 0)
+	if err != nil {
 		return 0, err
 	}
 
 	newGroup := types.Group{
-		GroupGaugeId:      nextGaugeId,
+		GroupGaugeId:      groupGaugeID,
 		InternalGaugeInfo: initialInternalGaugeInfo,
 		// Note: only Volume splitting exists today.
 		// We allow for other splitting policies to be added in the future
@@ -294,11 +294,8 @@ func (k Keeper) CreateGroup(ctx sdk.Context, coins sdk.Coins, numEpochPaidOver u
 	}
 
 	k.SetGroup(ctx, newGroup)
-	k.SetLastGaugeID(ctx, gauge.Id)
-
-	k.hooks.AfterCreateGauge(ctx, gauge.Id)
 
-	return nextGaugeId, nil
+	return groupGaugeID, nil
 }
 
 // GetGaugeByID returns gauge from gauge ID.