[Uptime Incentives]: Add authorized uptime validation for new NoLock …

…gauges (#7376) * add uptime validation logic and fix pool gauge link * add tests and clean up implementation * add changelog and clean up prints/diff * update changelog entry to be clearer * clean up comments and add additional tests * fix broken genesis tests * extract default NoLock duration to variable and add further comments * switch naming from concentrated to NoLock and move duration setup into appropriate logic branch
osmosis-labs · Jan 30, 2024 · 14078fe · 14078fe
1 parent 8943a4d
commit 14078fe
Show file tree

Hide file tree

Showing 10 changed files with 98 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### State Breaking
 
 * [#7181](https://github.com/osmosis-labs/osmosis/pull/7181) Improve errors for out of gas
+* [#7376](https://github.com/osmosis-labs/osmosis/pull/7376) Add uptime validation logic for `NoLock` (CL) gauges and switch CL gauge to pool ID links to be duration-based
 
 ### Bug Fixes
 

diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go
@@ -74,6 +74,7 @@ var (
 	}
 
 	defaultZeroWeightGaugeRecord = types.InternalGaugeRecord{GaugeId: 1, CurrentWeight: osmomath.ZeroInt(), CumulativeWeight: osmomath.ZeroInt()}
+	defaultNoLockDuration        = time.Nanosecond
 )
 
 type GroupCreationFields struct {
@@ -1139,6 +1140,7 @@ func (s *KeeperTestSuite) CreateNoLockExternalGauges(clPoolId uint64, externalGa
 	clPoolExternalGaugeId, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, numEpochsPaidOver == 1, gaugeCreator, externalGaugeCoins,
 		lockuptypes.QueryCondition{
 			LockQueryType: lockuptypes.NoLock,
+			Duration:      defaultNoLockDuration,
 		},
 		s.Ctx.BlockTime(),
 		numEpochsPaidOver,

diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go
@@ -127,9 +127,42 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 		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 {
+	// If the gauge has no lock, then we currently assume it is a concentrated pool
+	// and ensure the gauge "lock" duration is an authorized uptime.
+	isNoLockGauge := distrTo.LockQueryType == lockuptypes.NoLock
+
+	// If the gauge has an internal gauge denom, it is an internal gauge
+	// and should be run through different validation logic (see below).
+	//
+	// Two important reminders/assumptions:
+	// 1. `NoLock` gauges are required to have empty denoms in `ValidateBasic`, so this
+	// check cannot be controlled by user input.
+	// 2. The safety of this leans on the special-casing of internal gauge logic during
+	// distributions, which should be using the internal incentive duration gov param instead of the duration value.
+	isInternalNoLockGauge := isNoLockGauge && distrTo.Denom == types.NoLockInternalGaugeDenom(poolId)
+	isExternalNoLockGauge := isNoLockGauge && !isInternalNoLockGauge
+
+	// We check durations if the gauge is a regular duration based gauge or if it is a
+	// CL gauge. Note that this excludes time-based gauges and group gauges.
+	if isNoLockGauge || distrTo.LockQueryType == lockuptypes.ByDuration {
+		// Ensure that this gauge's duration is one of the allowed durations on chain
+		// Concentrated pool gauges check against authorized uptimes (if external) or
+		// epoch duration (if internal).
+		//
+		// All other gauges check against the default set of lockable durations.
+		var durations []time.Duration
+		if isExternalNoLockGauge {
+			durations = k.clk.GetParams(ctx).AuthorizedUptimes
+		} else if isInternalNoLockGauge {
+			// Internal CL gauges use epoch time as their duration. This is a legacy
+			// property that does not affect the uptime on created records, which is
+			// determined by the gov param for internal incentive uptimes.
+			durations = []time.Duration{k.GetEpochInfo(ctx).Duration}
+		} else {
+			// This branch is applicable to CFMM pool types such as balancer and stableswap.
+			durations = k.GetLockableDurations(ctx)
+		}
+
 		durationOk := false
 		for _, duration := range durations {
 			if duration == distrTo.Duration {
@@ -146,7 +179,7 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 
 	// For no lock gauges, a pool id must be set.
 	// A pool with such id must exist and be a concentrated pool.
-	if distrTo.LockQueryType == lockuptypes.NoLock {
+	if isNoLockGauge {
 		if poolId == 0 {
 			return 0, fmt.Errorf("'no lock' type gauges must have a pool id")
 		}
@@ -155,7 +188,7 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 		// and get overwritten with the external prefix + pool id
 		// for internal query purposes.
 		distrToDenom := distrTo.Denom
-		if distrToDenom != types.NoLockInternalGaugeDenom(poolId) {
+		if !isInternalNoLockGauge {
 			// If denom is set, then fails.
 			if distrToDenom != "" {
 				return 0, fmt.Errorf("'no lock' type external gauges must have an empty denom set, was %s", distrToDenom)
@@ -177,7 +210,7 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 		// That being said, internal gauges have an additional linking
 		// by duration where duration is the incentives epoch duration.
 		// The internal incentive linking is set in x/pool-incentives CreateConcentratedLiquidityPoolGauge.
-		k.pik.SetPoolGaugeIdNoLock(ctx, poolId, nextGaugeId)
+		k.pik.SetPoolGaugeIdNoLock(ctx, poolId, nextGaugeId, distrTo.Duration)
 	} else {
 		// For all other gauges, pool id must be 0.
 		if poolId != 0 {

diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go
@@ -507,7 +507,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			distrTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
 				// Note: this assumes the gauge is external
-				Denom: "",
+				Denom:    "",
+				Duration: time.Nanosecond,
 			},
 			poolId: concentratedPoolId,
 
@@ -520,7 +521,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			distrTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
 				// Note: this assumes the gauge is internal
-				Denom: types.NoLockInternalGaugeDenom(concentratedPoolId),
+				Denom:    types.NoLockInternalGaugeDenom(concentratedPoolId),
+				Duration: s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration,
 			},
 			poolId: concentratedPoolId,
 
@@ -533,7 +535,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			distrTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
 				// Note: this is invalid for NoLock gauges
-				Denom: "uosmo",
+				Denom:    "uosmo",
+				Duration: time.Nanosecond,
 			},
 			poolId: concentratedPoolId,
 
@@ -543,6 +546,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			name: "fail to create no lock gauge with balancer pool",
 			distrTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
+				Duration:      defaultNoLockDuration,
 			},
 			poolId: balancerPoolId,
 
@@ -552,6 +556,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			name: "fail to create no lock gauge with non-existent pool",
 			distrTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
+				Duration:      defaultNoLockDuration,
 			},
 			poolId: invalidPool,
 
@@ -561,9 +566,46 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			name: "fail to create no lock gauge with zero pool id",
 			distrTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
+				Duration:      defaultNoLockDuration,
 			},
 			poolId: zeroPoolId,
 
+			expectErr: true,
+		},
+		{
+			name: "fail to create external no lock gauge due to unauthorized uptime",
+			distrTo: lockuptypes.QueryCondition{
+				LockQueryType: lockuptypes.NoLock,
+				// Note: this assumes the gauge is external
+				Denom: "",
+				// 1h is a supported uptime that is not authorized
+				Duration: time.Hour,
+			},
+			poolId:    concentratedPoolId,
+			expectErr: true,
+		},
+		{
+			name: "fail to create external no lock gauge due to entirely invalid uptime",
+			distrTo: lockuptypes.QueryCondition{
+				LockQueryType: lockuptypes.NoLock,
+				// Note: this assumes the gauge is external
+				Denom: "",
+				// 2ns is an uptime that isn't supported at all (i.e. can't even be authorized)
+				Duration: 2 * time.Nanosecond,
+			},
+			poolId:    concentratedPoolId,
+			expectErr: true,
+		},
+		{
+			name: "fail to create an internal gauge with an unexpected duration",
+			distrTo: lockuptypes.QueryCondition{
+				LockQueryType: lockuptypes.NoLock,
+				// Note: this assumes the gauge is internal
+				Denom:    types.NoLockInternalGaugeDenom(concentratedPoolId),
+				Duration: time.Nanosecond,
+			},
+			poolId: concentratedPoolId,
+
 			expectErr: true,
 		},
 	}
@@ -589,10 +631,6 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 
 				s.Require().Equal(tc.expectedGaugeId, gaugeId)
 
-				// Assert that pool id and gauge id link meant for internally incentivized gauges is unset.
-				_, err := s.App.PoolIncentivesKeeper.GetPoolGaugeId(s.Ctx, tc.poolId, tc.distrTo.Duration)
-				s.Require().Error(err)
-
 				// Confirm that the general pool id to gauge id link is set.
 				gaugeIds, err := s.App.PoolIncentivesKeeper.GetNoLockGaugeIdsFromPool(s.Ctx, tc.poolId)
 				s.Require().NoError(err)
@@ -774,7 +812,7 @@ func (s *KeeperTestSuite) createGaugeNoRestrictions(isPerpetual bool, coins sdk.
 	}
 
 	if poolID != 0 {
-		s.App.PoolIncentivesKeeper.SetPoolGaugeIdNoLock(s.Ctx, poolID, nextGaugeID)
+		s.App.PoolIncentivesKeeper.SetPoolGaugeIdNoLock(s.Ctx, poolID, nextGaugeID, distrTo.Duration)
 	}
 
 	err := s.App.IncentivesKeeper.SetGauge(s.Ctx, &gauge)

diff --git a/x/incentives/keeper/genesis_test.go b/x/incentives/keeper/genesis_test.go
@@ -27,6 +27,7 @@ var (
 
 	distrToNoLock = lockuptypes.QueryCondition{
 		LockQueryType: lockuptypes.NoLock,
+		Duration:      defaultNoLockDuration,
 	}
 
 	distrToNoLockPool1 = lockuptypes.QueryCondition{

diff --git a/x/incentives/types/expected_keepers.go b/x/incentives/types/expected_keepers.go
@@ -50,6 +50,7 @@ type TxFeesKeeper interface {
 type ConcentratedLiquidityKeeper interface {
 	CreateIncentive(ctx sdk.Context, poolId uint64, sender sdk.AccAddress, incentiveCoin sdk.Coin, emissionRate osmomath.Dec, startTime time.Time, minUptime time.Duration) (cltypes.IncentiveRecord, error)
 	GetConcentratedPoolById(ctx sdk.Context, poolId uint64) (cltypes.ConcentratedPoolExtension, error)
+	GetParams(ctx sdk.Context) (params cltypes.Params)
 }
 
 type AccountKeeper interface {
@@ -59,7 +60,7 @@ type AccountKeeper interface {
 type PoolIncentiveKeeper interface {
 	GetPoolIdFromGaugeId(ctx sdk.Context, gaugeId uint64, lockableDuration time.Duration) (uint64, error)
 	GetInternalGaugeIDForPool(ctx sdk.Context, poolID uint64) (uint64, error)
-	SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64)
+	SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64, uptime time.Duration)
 	GetLongestLockableDuration(ctx sdk.Context) (time.Duration, error)
 }
 

diff --git a/x/pool-incentives/keeper/genesis.go b/x/pool-incentives/keeper/genesis.go
@@ -28,7 +28,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 	}
 	if genState.ConcentratedPoolToNoLockGauges != nil {
 		for _, record := range genState.ConcentratedPoolToNoLockGauges.PoolToGauge {
-			k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId)
+			k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId, record.Duration)
 		}
 	}
 }

diff --git a/x/pool-incentives/keeper/genesis_test.go b/x/pool-incentives/keeper/genesis_test.go
@@ -151,6 +151,7 @@ func (s *KeeperTestSuite) TestImportExportGenesis_ExternalNoLock() {
 	// Create external non-perpetual gauge
 	externalGaugeID, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, false, s.TestAccs[0], defaultCoins.Add(defaultCoins...), lockuptypes.QueryCondition{
 		LockQueryType: lockuptypes.NoLock,
+		Duration:      defaultNoLockDuration,
 	}, s.Ctx.BlockTime(), 2, clPool.GetId())
 	s.Require().NoError(err)
 

diff --git a/x/pool-incentives/keeper/grpc_query_test.go b/x/pool-incentives/keeper/grpc_query_test.go
@@ -16,6 +16,8 @@ import (
 var (
 	isPerpetual  = true
 	notPerpetual = false
+
+	defaultNoLockDuration = time.Nanosecond
 )
 
 func (s *KeeperTestSuite) TestGaugeIds() {
@@ -443,6 +445,7 @@ func (s *KeeperTestSuite) TestExternalIncentiveGauges_NoLock() {
 		defaultNoLockGaugeConfig = gaugeConfig{
 			distributeTo: lockuptypes.QueryCondition{
 				LockQueryType: lockuptypes.NoLock,
+				Duration:      defaultNoLockDuration,
 			},
 			poolId: concentratedPoolId,
 		}

diff --git a/x/pool-incentives/keeper/keeper.go b/x/pool-incentives/keeper/keeper.go
@@ -160,7 +160,7 @@ func (k Keeper) SetPoolGaugeIdInternalIncentive(ctx sdk.Context, poolId uint64,
 
 // SetPoolGaugeIdNoLock sets the link between pool id and gauge id for "NoLock" gauges.
 // CONTRACT: the gauge of the given id must be "NoLock" gauge.
-func (k Keeper) SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64) {
+func (k Keeper) SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64, incentivizedUptime time.Duration) {
 	store := ctx.KVStore(k.storeKey)
 	// maps pool id and gauge id to gauge id.
 	// Note: this could be pool id and gauge id to empty byte array,
@@ -170,10 +170,7 @@ func (k Keeper) SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uin
 	store.Set(key, sdk.Uint64ToBigEndian(gaugeId))
 
 	// Note: this index is used for general linking.
-	// We supply zero for incentivized duration as "NoLock" gauges are not
-	// associated with any lockable duration. Instead, they incentivize
-	// pools directly.
-	key = types.GetPoolIdFromGaugeIdStoreKey(gaugeId, 0)
+	key = types.GetPoolIdFromGaugeIdStoreKey(gaugeId, incentivizedUptime)
 	store.Set(key, sdk.Uint64ToBigEndian(poolId))
 }