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
osmosis-labs · Oct 8, 2023 · 9e7103e · 9e7103e
1 parent 1406f1d
commit 9e7103e
Show file tree

Hide file tree

Showing 7 changed files with 546 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,73 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+<<<<<<< HEAD
+=======
+### Features
+
+* [#6468](https://github.com/osmosis-labs/osmosis/pull/6468) feat: remove osmo multihop discount
+* [#6420](https://github.com/osmosis-labs/osmosis/pull/6420) feat[CL]: Creates a governance set whitelist of addresses that can bypass the normal pool creation restrictions on concentrated liquidity pools
+* [#6632](https://github.com/osmosis-labs/osmosis/pull/6632) Taker fee bypass whitelist
+
+### State Breaking
+
+* [#6344](https://github.com/osmosis-labs/osmosis/pull/6344) fix: set name, display and symbol of denom metadata in tokenfactory's CreateDenom
+* [#6279](https://github.com/osmosis-labs/osmosis/pull/6279) fix prop-597 introduced issue
+* [#6282](https://github.com/osmosis-labs/osmosis/pull/6282) Fix CreateCanonicalConcentratedLiquidityPoolAndMigrationLink overriding migration records
+* [#6309](https://github.com/osmosis-labs/osmosis/pull/6309) Add  Cosmwasm Pool Queries to Stargate Query
+* [#6493](https://github.com/osmosis-labs/osmosis/pull/6493) Add PoolManager Params query to Stargate Whitelist
+* [#6421](https://github.com/osmosis-labs/osmosis/pull/6421) Moves ValidatePermissionlessPoolCreationEnabled out of poolmanager module
+* [#6627](https://github.com/osmosis-labs/osmosis/pull/6627) Limit pow iterations in osmomath.
+* [#6586](https://github.com/osmosis-labs/osmosis/pull/6586) add auth.moduleaccounts to the stargate whitelist
+
+### 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
+
+* [#6476](https://github.com/osmosis-labs/osmosis/pull/6476) band-aid state export fix for cwpool gauges
+* [#6492](https://github.com/osmosis-labs/osmosis/pull/6492) bump IAVL version to v0.19.7
+
+
+### Features
+
+* [#6427](https://github.com/osmosis-labs/osmosis/pull/6427) sdk.Coins Mul and Quo helpers in osmoutils
+* [#6437](https://github.com/osmosis-labs/osmosis/pull/6437) mutative version for QuoRoundUp
+* [#6261](https://github.com/osmosis-labs/osmosis/pull/6261) mutative and efficient BigDec truncations with arbitrary decimals
+* [#6416](https://github.com/osmosis-labs/osmosis/pull/6416) feat[CL]: add num initialized ticks query
+* [#6488](https://github.com/osmosis-labs/osmosis/pull/6488) v2 SpotPrice CLI and GRPC query with 36 decimals in poolmanager
+
+### API Breaks
+
+* [#6487](https://github.com/osmosis-labs/osmosis/pull/6487) make PoolModuleI CalculateSpotPrice API return BigDec
+* [#6511](https://github.com/osmosis-labs/osmosis/pull/6511) remove redundant param from CreateGaugeRefKeys in incentives
+* [#6510](https://github.com/osmosis-labs/osmosis/pull/6510) remove redundant ctx param from DeleteAllKeysFromPrefix in osmoutils
+
+
+## v19.1.0
+
+### Features
+
+* [#6427](https://github.com/osmosis-labs/osmosis/pull/6427) sdk.Coins Mul and Quo helpers in osmoutils
+* [#6428](https://github.com/osmosis-labs/osmosis/pull/6428) osmomath: QuoTruncateMut
+* [#6437](https://github.com/osmosis-labs/osmosis/pull/6437) mutative version for QuoRoundUp
+* [#6261](https://github.com/osmosis-labs/osmosis/pull/6261) mutative and efficient BigDec truncations with arbitrary decimals
+* [#6416](https://github.com/osmosis-labs/osmosis/pull/6416) feat[CL]: add num initialized ticks query
+
+### Misc Improvements
+
+* [#6392](https://github.com/osmosis-labs/osmosis/pull/6392) Speedup fractional exponentiation
+
+### Bug Fixes
+
+* [#6334](https://github.com/osmosis-labs/osmosis/pull/6334) fix: enable taker fee cli
+* [#6352](https://github.com/osmosis-labs/osmosis/pull/6352) Reduce error blow-up in CalcAmount0Delta by changing the order of math operations.
+* [#6368](https://github.com/osmosis-labs/osmosis/pull/6368) Stricter rounding behavior in CL math's CalcAmount0Delta and GetNextSqrtPriceFromAmount0InRoundingUp
+* [#6409](https://github.com/osmosis-labs/osmosis/pull/6409) CL math: Convert Int to BigDec
+
+>>>>>>> ab93bb25 (fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs (#6644))
 ### API Breaks
 
 * [#6256](https://github.com/osmosis-labs/osmosis/pull/6256) Refactor CalcPriceToTick to operate on BigDec price to support new price range.

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/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,11 +9,19 @@ import (
 	"github.com/stretchr/testify/require"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 
+<<<<<<< HEAD
 	pool_incentives "github.com/osmosis-labs/osmosis/v18/x/pool-incentives"
 
 	simapp "github.com/osmosis-labs/osmosis/v18/app"
 
 	"github.com/osmosis-labs/osmosis/v18/x/pool-incentives/types"
+=======
+	"github.com/osmosis-labs/osmosis/osmomath"
+	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"
+>>>>>>> ab93bb25 (fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs (#6644))
 )
 
 var (
@@ -36,7 +44,7 @@ var (
 				},
 			},
 		},
-		PoolToGauges: &types.PoolToGauges{
+		AnyPoolToInternalGauges: &types.AnyPoolToInternalGauges{
 			PoolToGauge: []types.PoolToGauge{
 				{
 					PoolId:   1,
@@ -48,18 +56,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 +128,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 +143,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)
 }