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

… (backport #6644) (#6646) * 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 # x/pool-incentives/keeper/genesis_test.go # x/pool-incentives/types/genesis.pb.go # x/pool-incentives/types/incentives.pb.go * fix backport * regen protos * attempt to fix e2e --------- Co-authored-by: Roman <[email protected]> Co-authored-by: Adam Tucker <[email protected]>
osmosis-labs · Oct 8, 2023 · 1f5f188 · 1f5f188
1 parent 7f88292
commit 1f5f188
Show file tree

Hide file tree

Showing 11 changed files with 475 additions and 133 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Bug Fixes
+* [#6644](https://github.com/osmosis-labs/osmosis/pull/6644) fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs
+
 ### Misc Improvements
 
 * [#6161](https://github.com/osmosis-labs/osmosis/pull/6161) Reduce CPU time of epochs

diff --git a/go.mod b/go.mod
@@ -6,7 +6,7 @@ require (
 	cosmossdk.io/errors v1.0.0
 	github.com/CosmWasm/wasmd v0.31.0
 	github.com/cosmos/cosmos-proto v1.0.0-beta.2
-	github.com/cosmos/cosmos-sdk v0.47.3
+	github.com/cosmos/cosmos-sdk v0.47.5
 	github.com/cosmos/go-bip39 v1.0.0
 	github.com/cosmos/ibc-apps/modules/async-icq/v4 v4.0.0-20230524151648-c02fa46c2860
 	github.com/cosmos/ibc-go/v4 v4.3.1

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/tests/cl-genesis-positions/convert.go b/tests/cl-genesis-positions/convert.go
@@ -111,7 +111,7 @@ func ConvertSubgraphToOsmosisGenesis(positionCreatorAddresses []sdk.AccAddress,
 	// Initialize first position to be 1:1 price
 	// this is because the first position must have non-zero token0 and token1 to initialize the price
 	// however, our data has first position with non-zero amount.
-	_, _, _, _, err = osmosis.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(osmosis.Ctx, pool.GetId(), osmosis.TestAccs[0], sdk.NewCoins(sdk.NewCoin(msgCreatePool.Denom0, sdk.NewInt(100)), sdk.NewCoin(msgCreatePool.Denom1, sdk.NewInt(100))))
+	_, err = osmosis.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(osmosis.Ctx, pool.GetId(), osmosis.TestAccs[0], sdk.NewCoins(sdk.NewCoin(msgCreatePool.Denom0, sdk.NewInt(100)), sdk.NewCoin(msgCreatePool.Denom1, sdk.NewInt(100))))
 	if err != nil {
 		panic(err)
 	}

diff --git a/tests/e2e/initialization/config.go b/tests/e2e/initialization/config.go
@@ -409,7 +409,7 @@ func updatePoolIncentiveGenesis(pooliGenState *poolitypes.GenesisState) {
 	}
 
 	// START: CAN REMOVE POST v17 UPGRADE
-	poolToGauges := poolitypes.PoolToGauges{}
+	poolToGauges := poolitypes.AnyPoolToInternalGauges{}
 	currentGaugeId := uint64(1)
 	for _, assetPair := range AssetPairs {
 		for _, duration := range pooliGenState.LockableDurations {
@@ -422,7 +422,7 @@ func updatePoolIncentiveGenesis(pooliGenState *poolitypes.GenesisState) {
 			poolToGauges.PoolToGauge = append(poolToGauges.PoolToGauge, poolToGauge)
 		}
 	}
-	pooliGenState.PoolToGauges = &poolToGauges
+	pooliGenState.AnyPoolToInternalGauges = &poolToGauges
 	// END: CAN REMOVE POST v17 UPGRADE
 }
 

diff --git a/x/pool-incentives/keeper/distr_test.go b/x/pool-incentives/keeper/distr_test.go
@@ -6,6 +6,8 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+var defaultCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.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
@@ -18,31 +18,37 @@ 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 {
 			panic(err)
 		}
 		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 {
@@ -52,7 +58,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)
@@ -63,15 +82,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,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 
+	lockuptypes "github.com/osmosis-labs/osmosis/v16/x/lockup/types"
 	pool_incentives "github.com/osmosis-labs/osmosis/v16/x/pool-incentives"
 
 	simapp "github.com/osmosis-labs/osmosis/v16/app"
@@ -36,7 +37,7 @@ var (
 				},
 			},
 		},
-		PoolToGauges: &types.PoolToGauges{
+		AnyPoolToInternalGauges: &types.AnyPoolToInternalGauges{
 			PoolToGauge: []types.PoolToGauge{
 				{
 					PoolId:   1,
@@ -48,18 +49,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 +121,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 +136,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)
 }