Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: genesis bug in pool incentives linking NoLock gauges and PoolIDs (backport #6644) #6647

Merged
merged 3 commits into from
Oct 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions proto/osmosis/pool-incentives/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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\""
];
}
6 changes: 5 additions & 1 deletion proto/osmosis/pool-incentives/v1beta1/incentives.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 ];
}
2 changes: 2 additions & 0 deletions x/pool-incentives/keeper/distr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 35 additions & 15 deletions x/pool-incentives/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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,
}
}
77 changes: 63 additions & 14 deletions x/pool-incentives/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

lockuptypes "github.com/osmosis-labs/osmosis/v17/x/lockup/types"
pool_incentives "github.com/osmosis-labs/osmosis/v17/x/pool-incentives"

simapp "github.com/osmosis-labs/osmosis/v17/app"
Expand Down Expand Up @@ -36,7 +37,7 @@ var (
},
},
},
PoolToGauges: &types.PoolToGauges{
AnyPoolToInternalGauges: &types.AnyPoolToInternalGauges{
PoolToGauge: []types.PoolToGauge{
{
PoolId: 1,
Expand All @@ -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,
},
},
},
Expand Down Expand Up @@ -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++
Expand All @@ -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)
}
Loading