fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs…

… (backport #6644) (#6649) * fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs (#6644) * fix: genesis bug in pool incentives * changelog * lint * add clarifying comment * merge main * Revert "merge main" This reverts commit a0ed7a0. --------- Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Adam Tucker <[email protected]> (cherry picked from commit ab93bb2) # Conflicts: # CHANGELOG.md * Update CHANGELOG.md * add default coins --------- Co-authored-by: Roman <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Adam Tucker <[email protected]>
osmosis-labs · Oct 8, 2023 · 43c12f7 · 43c12f7
1 parent 35b252d
commit 43c12f7
Show file tree

Hide file tree

Showing 8 changed files with 474 additions and 132 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Bug Fixes
+* [#6644](https://github.com/osmosis-labs/osmosis/pull/6644) fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs
+
 ## v19.2.0
 
 ### Misc Improvements

diff --git a/proto/osmosis/pool-incentives/v1beta1/genesis.proto b/proto/osmosis/pool-incentives/v1beta1/genesis.proto
@@ -20,8 +20,18 @@ message GenesisState {
     (gogoproto.nullable) = true,
     (gogoproto.moretags) = "yaml:\"distr_info\""
   ];
-  PoolToGauges pool_to_gauges = 4 [
+  // any_pool_to_internal_gauges defines the gauges for any pool to internal
+  // pool. For every pool type (e.g. LP, Concentrated, etc), there is one such
+  // link
+  AnyPoolToInternalGauges any_pool_to_internal_gauges = 4 [
     (gogoproto.nullable) = true,
-    (gogoproto.moretags) = "yaml:\"pool_to_gauges\""
+    (gogoproto.moretags) = "yaml:\"internal_pool_to_gauges\""
+  ];
+  // concentrated_pool_to_no_lock_gauges defines the no lock gauges for
+  // concentrated pool. This only exists between concentrated pool and no lock
+  // gauges. Both external and internal gauges are included.
+  ConcentratedPoolToNoLockGauges concentrated_pool_to_no_lock_gauges = 5 [
+    (gogoproto.nullable) = true,
+    (gogoproto.moretags) = "yaml:\"concentrated_pool_to_no_lock_gauges\""
   ];
 }
diff --git a/proto/osmosis/pool-incentives/v1beta1/incentives.proto b/proto/osmosis/pool-incentives/v1beta1/incentives.proto
@@ -53,6 +53,10 @@ message PoolToGauge {
   ];
 }
 
-message PoolToGauges {
+message AnyPoolToInternalGauges {
   repeated PoolToGauge pool_to_gauge = 2 [ (gogoproto.nullable) = false ];
+}
+
+message ConcentratedPoolToNoLockGauges {
+  repeated PoolToGauge pool_to_gauge = 1 [ (gogoproto.nullable) = false ];
 }
diff --git a/x/pool-incentives/keeper/distr_test.go b/x/pool-incentives/keeper/distr_test.go
@@ -7,6 +7,8 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+var defaultCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, osmomath.NewInt(10000)))
+
 func (s *KeeperTestSuite) TestAllocateAsset() {
 	tests := []struct {
 		name                   string

diff --git a/x/pool-incentives/keeper/genesis.go b/x/pool-incentives/keeper/genesis.go
@@ -19,24 +19,28 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 	} else {
 		k.SetDistrInfo(ctx, *genState.DistrInfo)
 	}
-	if genState.PoolToGauges != nil {
-		for _, record := range genState.PoolToGauges.PoolToGauge {
-			if record.Duration == 0 {
-				k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId)
-			} else {
-				if err := k.SetPoolGaugeIdInternalIncentive(ctx, record.PoolId, record.Duration, record.GaugeId); err != nil {
-					panic(err)
-				}
+	if genState.AnyPoolToInternalGauges != nil {
+		for _, record := range genState.AnyPoolToInternalGauges.PoolToGauge {
+			if err := k.SetPoolGaugeIdInternalIncentive(ctx, record.PoolId, record.Duration, record.GaugeId); err != nil {
+				panic(err)
 			}
 		}
 	}
+	if genState.ConcentratedPoolToNoLockGauges != nil {
+		for _, record := range genState.ConcentratedPoolToNoLockGauges.PoolToGauge {
+			k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId)
+		}
+	}
 }
 
 func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	distrInfo := k.GetDistrInfo(ctx)
 	lastPoolId := k.poolmanagerKeeper.GetNextPoolId(ctx)
 	lockableDurations := k.GetLockableDurations(ctx)
-	var poolToGauges types.PoolToGauges
+	var (
+		anyPoolToInternalGauges        types.AnyPoolToInternalGauges
+		concentratedPoolToNoLockGauges types.ConcentratedPoolToNoLockGauges
+	)
 	for poolId := 1; poolId < int(lastPoolId); poolId++ {
 		pool, err := k.poolmanagerKeeper.GetPool(ctx, uint64(poolId))
 		if err != nil {
@@ -53,6 +57,8 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 
 		isCLPool := pool.GetType() == poolmanagertypes.Concentrated
 		if isCLPool {
+			// This creates a link for the internal pool gauge.
+			// Every CL pool has one such gauge.
 			incParams := k.incentivesKeeper.GetEpochInfo(ctx)
 			gaugeID, err := k.GetPoolGaugeId(ctx, uint64(poolId), incParams.Duration)
 			if err != nil {
@@ -62,7 +68,20 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 			poolToGauge.Duration = incParams.Duration
 			poolToGauge.GaugeId = gaugeID
 			poolToGauge.PoolId = uint64(poolId)
-			poolToGauges.PoolToGauge = append(poolToGauges.PoolToGauge, poolToGauge)
+			anyPoolToInternalGauges.PoolToGauge = append(anyPoolToInternalGauges.PoolToGauge, poolToGauge)
+
+			// All concentrated pools need an additional link for the no-lock gauge.
+			gaugeIDs, err := k.GetNoLockGaugeIdsFromPool(ctx, uint64(poolId))
+			if err != nil {
+				panic(err)
+			}
+			for _, gaugeID := range gaugeIDs {
+				poolToGauge := types.PoolToGauge{
+					GaugeId: gaugeID,
+					PoolId:  uint64(poolId),
+				}
+				concentratedPoolToNoLockGauges.PoolToGauge = append(concentratedPoolToNoLockGauges.PoolToGauge, poolToGauge)
+			}
 		} else {
 			for _, duration := range lockableDurations {
 				gaugeID, err := k.GetPoolGaugeId(ctx, uint64(poolId), duration)
@@ -78,15 +97,16 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 				poolToGauge.Duration = duration
 				poolToGauge.GaugeId = gaugeID
 				poolToGauge.PoolId = uint64(poolId)
-				poolToGauges.PoolToGauge = append(poolToGauges.PoolToGauge, poolToGauge)
+				anyPoolToInternalGauges.PoolToGauge = append(anyPoolToInternalGauges.PoolToGauge, poolToGauge)
 			}
 		}
 	}
 
 	return &types.GenesisState{
-		Params:            k.GetParams(ctx),
-		LockableDurations: k.GetLockableDurations(ctx),
-		DistrInfo:         &distrInfo,
-		PoolToGauges:      &poolToGauges,
+		Params:                         k.GetParams(ctx),
+		LockableDurations:              k.GetLockableDurations(ctx),
+		DistrInfo:                      &distrInfo,
+		AnyPoolToInternalGauges:        &anyPoolToInternalGauges,
+		ConcentratedPoolToNoLockGauges: &concentratedPoolToNoLockGauges,
 	}
 }
diff --git a/x/pool-incentives/keeper/genesis_test.go b/x/pool-incentives/keeper/genesis_test.go
@@ -9,10 +9,9 @@ import (
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 
 	"github.com/osmosis-labs/osmosis/osmomath"
-	pool_incentives "github.com/osmosis-labs/osmosis/v19/x/pool-incentives"
-
 	simapp "github.com/osmosis-labs/osmosis/v19/app"
-
+	lockuptypes "github.com/osmosis-labs/osmosis/v19/x/lockup/types"
+	pool_incentives "github.com/osmosis-labs/osmosis/v19/x/pool-incentives"
 	"github.com/osmosis-labs/osmosis/v19/x/pool-incentives/types"
 )
 
@@ -36,7 +35,7 @@ var (
 				},
 			},
 		},
-		PoolToGauges: &types.PoolToGauges{
+		AnyPoolToInternalGauges: &types.AnyPoolToInternalGauges{
 			PoolToGauge: []types.PoolToGauge{
 				{
 					PoolId:   1,
@@ -48,18 +47,14 @@ var (
 					GaugeId:  2,
 					Duration: time.Second,
 				},
-				// This duplication with zero duration
-				// can happen with "NoLock" gauges
-				// where the link containing the duration
-				// is used to signify that the gauge is internal
-				// while the link without the duration is used
-				// for general purpose. This redundancy is
-				// made for convinience of plugging in the
-				// later added "NoLock" gauge into the existing
-				// logic without having to change majority of the queries.
+			},
+		},
+		ConcentratedPoolToNoLockGauges: &types.ConcentratedPoolToNoLockGauges{
+			PoolToGauge: []types.PoolToGauge{
 				{
-					PoolId:  2,
-					GaugeId: 2,
+					PoolId:   3,
+					GaugeId:  3,
+					Duration: 0,
 				},
 			},
 		},
@@ -124,7 +119,7 @@ func (s *KeeperTestSuite) TestExportGenesis() {
 	s.App.PoolIncentivesKeeper.SetLockableDurations(ctx, durations)
 	savedDurations := s.App.PoolIncentivesKeeper.GetLockableDurations(ctx)
 	s.Equal(savedDurations, durations)
-	var expectedPoolToGauges types.PoolToGauges
+	var expectedPoolToGauges types.AnyPoolToInternalGauges
 	var gauge uint64
 	for _, duration := range durations {
 		gauge++
@@ -139,5 +134,57 @@ func (s *KeeperTestSuite) TestExportGenesis() {
 	s.Equal(genesisExported.Params, genesis.Params)
 	s.Equal(genesisExported.LockableDurations, durations)
 	s.Equal(genesisExported.DistrInfo, genesis.DistrInfo)
-	s.Equal(genesisExported.PoolToGauges, &expectedPoolToGauges)
+	s.Equal(genesisExported.AnyPoolToInternalGauges, &expectedPoolToGauges)
+}
+
+// This test validates that all store indexes are set correctly
+// for NoLock gauges after exporting and then reimporting genesis.
+func (s *KeeperTestSuite) TestImportExportGenesis_ExternalNoLock() {
+	s.SetupTest()
+
+	// Prepare concentrated pool
+	clPool := s.PrepareConcentratedPool()
+
+	// Fund account to create gauge
+	s.FundAcc(s.TestAccs[0], defaultCoins.Add(defaultCoins...))
+
+	// 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,
+	}, s.Ctx.BlockTime(), 2, clPool.GetId())
+	s.Require().NoError(err)
+
+	// We expect internal gauge to be created first
+	internalGaugeID := externalGaugeID - 1
+
+	// Export genesis
+	export := s.App.PoolIncentivesKeeper.ExportGenesis(s.Ctx)
+
+	// Validate that only one link for internal gauges is created
+	s.Require().Equal(1, len(export.AnyPoolToInternalGauges.PoolToGauge))
+
+	// Validate that 2 links, one for external and one for internal gauge, are created
+	s.Require().Equal(2, len(export.ConcentratedPoolToNoLockGauges.PoolToGauge))
+
+	// Reset state
+	s.SetupTest()
+
+	// Import genesis
+	s.App.PoolIncentivesKeeper.InitGenesis(s.Ctx, export)
+
+	// Get the general link between external gauge ID and pool
+	poolID, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, externalGaugeID, 0)
+	s.Require().NoError(err)
+	s.Require().Equal(clPool.GetId(), poolID)
+
+	// Get the general link between internal gauge ID and pool
+	poolID, err = s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, internalGaugeID, 0)
+	s.Require().NoError(err)
+	s.Require().Equal(clPool.GetId(), poolID)
+
+	// Get the internal gauge
+	incentivesEpochDuration := s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration
+	internalGaugeIDAfterImport, err := s.App.PoolIncentivesKeeper.GetPoolGaugeId(s.Ctx, poolID, incentivesEpochDuration)
+	s.Require().NoError(err)
+	s.Require().Equal(internalGaugeID, internalGaugeIDAfterImport)
 }