From fc057773a8822383b75238629b231aeb9f862768 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 16:40:52 -0800 Subject: [PATCH 1/8] implement validation, fallback, and wiring for using gauge duration as uptime --- x/incentives/keeper/distribute.go | 26 ++++++++++++- x/incentives/keeper/distribute_test.go | 51 ++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index 22240ff2c46..30896eaa4f4 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -602,6 +602,27 @@ func (k Keeper) distributeInternal( // Get distribution epoch duration. This is used to calculate the emission rate. currentEpoch := k.GetEpochInfo(ctx) + // Validate that the gauge's corresponding uptime is authorized. + authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes + gaugeUptime := gauge.DistributeTo.Duration + isUptimeAuthorized := false + for _, authorizedUptime := range authorizedUptimes { + if gaugeUptime == authorizedUptime { + isUptimeAuthorized = true + } + } + + // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. + // + // This is for two reasons: + // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges + // 2. To avoid having to do a state migration on existing gauges at time of adding + // this change, since prior to this, CL gauges were not required to associate with + // an uptime that was authorized. + if !isUptimeAuthorized { + gaugeUptime = types.DefaultConcentratedUptime + } + // For every coin in the gauge, calculate the remaining reward per epoch // and create a concentrated liquidity incentive record for it that // is supposed to distribute over that epoch. @@ -625,8 +646,9 @@ func (k Keeper) distributeInternal( // Gauge start time should be checked whenever moving between active // and inactive gauges. By the time we get here, the gauge should be active. ctx.BlockTime(), - // Only default uptime is supported at launch. - types.DefaultConcentratedUptime, + // The uptime for each distribution is determined by the gauge's duration field. + // If it is unauthorized, we fall back to a default above. + gaugeUptime, ) ctx.Logger().Info(fmt.Sprintf("distributeInternal CL for pool id %d finished", pool.GetId())) diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index 80df51a630e..b2e74fb9eae 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -436,6 +436,7 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { startTime time.Time numEpochsPaidOver uint64 poolId uint64 + authorizedUptime bool // expected expectErr bool @@ -451,6 +452,7 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { startTime: oneHourAfterDefault, numEpochsPaidOver: 1, poolId: defaultCLPool, + authorizedUptime: false, expectErr: false, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), @@ -508,6 +510,18 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { return tc } + withAuthorizedUptime := func(tc test, duration time.Duration) test { + tc.distrTo.Duration = duration + tc.authorizedUptime = true + return tc + } + + withUnauthorizedUptime := func(tc test, duration time.Duration) test { + tc.distrTo.Duration = duration + tc.authorizedUptime = false + return tc + } + withError := func(tc test) test { tc.expectErr = true return tc @@ -520,8 +534,24 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { "perpetual, 2 coins, paid over 1 epoch": withIsPerpetual(withGaugeCoins(defaultTest, defaultBothCoins), true), "non-perpetual, 1 coin, paid over 2 epochs": withNumEpochs(defaultTest, 2), "non-perpetual, 2 coins, paid over 3 epochs": withNumEpochs(withGaugeCoins(defaultTest, defaultBothCoins), 3), - "error: balancer pool id": withError(withPoolId(defaultTest, defaultBalancerPool)), - "error: inactive gauge": withError(withNumEpochs(defaultTest, 0)), + + // We expect incentives with the set uptime to be created + "non-perpetual, 1 coin, paid over 1 epoch, authorized 1d uptime": withAuthorizedUptime(defaultTest, time.Hour*24), + "non-perpetual, 2 coins, paid over 3 epochs, authorized 7d uptime": withAuthorizedUptime(withNumEpochs(withGaugeCoins(defaultTest, defaultBothCoins), 3), time.Hour*24*7), + "perpetual, 2 coins, authorized 1h uptime": withAuthorizedUptime(withIsPerpetual(withGaugeCoins(defaultTest, defaultBothCoins), true), time.Hour), + + // We expect incentives to fall back to default uptime of 1ns + "non-perpetual, 1 coin, paid over 1 epoch, unauthorized 1d uptime": withUnauthorizedUptime(defaultTest, time.Hour*24), + "non-perpetual, 2 coins, paid over 3 epochs, unauthorized 7d uptime": withUnauthorizedUptime(withNumEpochs(withGaugeCoins(defaultTest, defaultBothCoins), 3), time.Hour*24*7), + "perpetual, 2 coins, unauthorized 1h uptime": withUnauthorizedUptime(withIsPerpetual(withGaugeCoins(defaultTest, defaultBothCoins), true), time.Hour), + + // 3h is not a valid uptime, so we expect this to fall back to 1ns + "non-perpetual, 1 coin, paid over 1 epoch, unauthorized and invalid uptime": withUnauthorizedUptime(defaultTest, time.Hour*3), + "non-perpetual, 2 coins, paid over 3 epochs, unauthorized and invalid uptime": withUnauthorizedUptime(withNumEpochs(withGaugeCoins(defaultTest, defaultBothCoins), 3), time.Hour*3), + "perpetual, 2 coins, unauthorized and invalid uptime": withUnauthorizedUptime(withIsPerpetual(withGaugeCoins(defaultTest, defaultBothCoins), true), time.Hour*3), + + "error: balancer pool id": withError(withPoolId(defaultTest, defaultBalancerPool)), + "error: inactive gauge": withError(withNumEpochs(defaultTest, 0)), } for name, tc := range tests { @@ -540,6 +570,13 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { // can function properly. s.Ctx = s.Ctx.WithBlockTime(oneHourAfterDefault) + // If applicable, authorize gauge's uptime in CL module + if tc.authorizedUptime { + clParams := s.App.ConcentratedLiquidityKeeper.GetParams(s.Ctx) + clParams.AuthorizedUptimes = append(clParams.AuthorizedUptimes, tc.distrTo.Duration) + s.App.ConcentratedLiquidityKeeper.SetParams(s.Ctx, clParams) + } + // Create gauge and get it from state externalGauge := s.createGaugeNoRestrictions(tc.isPerpertual, tc.gaugeCoins, tc.distrTo, tc.startTime, tc.numEpochsPaidOver, defaultCLPool) @@ -569,9 +606,15 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { incentivesEpochDuration := s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration incentivesEpochDurationSeconds := osmomath.NewDec(incentivesEpochDuration.Milliseconds()).QuoInt(osmomath.NewInt(1000)) + // If uptime is not authorized, we expect to fall back to default + expectedUptime := types.DefaultConcentratedUptime + if tc.authorizedUptime { + expectedUptime = tc.distrTo.Duration + } + // Check that incentive records were created for i, coin := range tc.expectedDistributions { - incentiveRecords, err := s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, tc.poolId, time.Nanosecond, uint64(i+1)) + incentiveRecords, err := s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, tc.poolId, expectedUptime, uint64(i+1)) s.Require().NoError(err) expectedEmissionRatePerEpoch := coin.Amount.ToLegacyDec().QuoTruncate(incentivesEpochDurationSeconds) @@ -580,7 +623,7 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { s.Require().Equal(coin.Denom, incentiveRecords.IncentiveRecordBody.RemainingCoin.Denom) s.Require().Equal(tc.expectedRemainingAmountIncentiveRecord[i], incentiveRecords.IncentiveRecordBody.RemainingCoin.Amount) s.Require().Equal(expectedEmissionRatePerEpoch, incentiveRecords.IncentiveRecordBody.EmissionRate) - s.Require().Equal(time.Nanosecond, incentiveRecords.MinUptime) + s.Require().Equal(expectedUptime, incentiveRecords.MinUptime) } // Check that the gauge's distribution state was updated From a855eabbd17c31ea963e7780020918ad61be12fa Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 17:29:30 -0800 Subject: [PATCH 2/8] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b7280917fc..12cb17b9042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#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 +* [#7417](https://github.com/osmosis-labs/osmosis/pull/7417) Update CL gauges to use gauge duration as uptime, falling back to default if unauthorized or invalid ### Bug Fixes From bed822ba555b43fb76a2897bd50dbd0d82f8b75a Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 19:57:52 -0800 Subject: [PATCH 3/8] add param into core CL distr logic and set up sanity tests --- x/incentives/keeper/distribute.go | 40 +++++++++++++++----------- x/incentives/keeper/distribute_test.go | 11 +++++++ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index ea2fb2f2ee0..994775410cc 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -602,25 +602,33 @@ func (k Keeper) distributeInternal( // Get distribution epoch duration. This is used to calculate the emission rate. currentEpoch := k.GetEpochInfo(ctx) - // Validate that the gauge's corresponding uptime is authorized. - authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes + // If internal gauge, use InternalUptime param as the gauge's uptime. + // If external gauge, validate that the gauge's corresponding uptime is authorized. + isInternalGauge := gauge.DistributeTo.Denom == types.NoLockInternalGaugeDenom(pool.GetId()) gaugeUptime := gauge.DistributeTo.Duration - isUptimeAuthorized := false - for _, authorizedUptime := range authorizedUptimes { - if gaugeUptime == authorizedUptime { - isUptimeAuthorized = true + if isInternalGauge { + // CONTRACT: InternalUptime is a supported (but not necessarily authorized) uptime in the CL module. + gaugeUptime = k.GetParams(ctx).InternalUptime + } else { + // Validate that the gauge's corresponding uptime is authorized. + authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes + isUptimeAuthorized := false + for _, authorizedUptime := range authorizedUptimes { + if gaugeUptime == authorizedUptime { + isUptimeAuthorized = true + } } - } - // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. - // - // This is for two reasons: - // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges - // 2. To avoid having to do a state migration on existing gauges at time of adding - // this change, since prior to this, CL gauges were not required to associate with - // an uptime that was authorized. - if !isUptimeAuthorized { - gaugeUptime = types.DefaultConcentratedUptime + // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. + // + // This is for two reasons: + // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges + // 2. To avoid having to do a state migration on existing gauges at time of adding + // this change, since prior to this, CL gauges were not required to associate with + // an uptime that was authorized. + if !isUptimeAuthorized { + gaugeUptime = types.DefaultConcentratedUptime + } } // For every coin in the gauge, calculate the remaining reward per epoch diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index b2e74fb9eae..b75f5038356 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -251,6 +251,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { tokensToAddToGauge sdk.Coins gaugeStartTime time.Time gaugeCoins sdk.Coins + internalUptime time.Duration // expected expectErr bool @@ -260,6 +261,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { numPools: 1, gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), expectErr: false, }, @@ -267,6 +269,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { numPools: 1, gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), + internalUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), expectErr: false, }, @@ -274,6 +277,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { numPools: 3, gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), expectErr: false, }, @@ -281,6 +285,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { numPools: 1, gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), expectErr: false, }, @@ -288,9 +293,12 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { numPools: 1, gaugeStartTime: defaultGaugeStartTime.Add(-5 * time.Hour), gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), expectErr: false, }, + + // 1d authorized, 7d internal } for name, tc := range tests { @@ -301,6 +309,9 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { // We fix blocktime to ensure tests are deterministic s.Ctx = s.Ctx.WithBlockTime(defaultGaugeStartTime) + // Set internal uptime module param as defined in test case + s.App.IncentivesKeeper.SetParam(s.Ctx, types.KeyInternalUptime, tc.internalUptime) + var gauges []types.Gauge // prepare the minting account From 650696b70687de379ca6876be703fac771cb2cd7 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 21:06:35 -0800 Subject: [PATCH 4/8] use new param in core logic and add tests --- x/incentives/keeper/distribute.go | 46 ++++++----- x/incentives/keeper/distribute_test.go | 110 ++++++++++++++++--------- 2 files changed, 97 insertions(+), 59 deletions(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index 994775410cc..57828ba1835 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -603,34 +603,36 @@ func (k Keeper) distributeInternal( currentEpoch := k.GetEpochInfo(ctx) // If internal gauge, use InternalUptime param as the gauge's uptime. - // If external gauge, validate that the gauge's corresponding uptime is authorized. - isInternalGauge := gauge.DistributeTo.Denom == types.NoLockInternalGaugeDenom(pool.GetId()) + // Otherwise, use the gauge's duration. gaugeUptime := gauge.DistributeTo.Duration - if isInternalGauge { - // CONTRACT: InternalUptime is a supported (but not necessarily authorized) uptime in the CL module. + if gauge.DistributeTo.Denom == types.NoLockInternalGaugeDenom(pool.GetId()) { gaugeUptime = k.GetParams(ctx).InternalUptime - } else { - // Validate that the gauge's corresponding uptime is authorized. - authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes - isUptimeAuthorized := false - for _, authorizedUptime := range authorizedUptimes { - if gaugeUptime == authorizedUptime { - isUptimeAuthorized = true - } - } + } - // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. - // - // This is for two reasons: - // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges - // 2. To avoid having to do a state migration on existing gauges at time of adding - // this change, since prior to this, CL gauges were not required to associate with - // an uptime that was authorized. - if !isUptimeAuthorized { - gaugeUptime = types.DefaultConcentratedUptime + // Validate that the gauge's corresponding uptime is authorized. + authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes + isUptimeAuthorized := false + for _, authorizedUptime := range authorizedUptimes { + if gaugeUptime == authorizedUptime { + isUptimeAuthorized = true } } + // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. + // + // This is for two reasons: + // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges + // 2. To avoid having to do a state migration on existing gauges at time of adding + // this change, since prior to this, CL gauges were not required to associate with + // an uptime that was authorized. + // + // Note that this fallback happens even if the gauge is an internal gauge. This is because + // we want to be able to unauthorize internal gauges as well in the event that there is an + // issue found with a previously authorized uptime. + if !isUptimeAuthorized { + gaugeUptime = types.DefaultConcentratedUptime + } + // For every coin in the gauge, calculate the remaining reward per epoch // and create a concentrated liquidity incentive record for it that // is supposed to distribute over that epoch. diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index b75f5038356..0b8f1cf962e 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -256,12 +256,14 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { // expected expectErr bool expectedDistributions sdk.Coins + expectedUptime time.Duration }{ "valid case: one poolId and gaugeId": { numPools: 1, gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins), internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), expectErr: false, }, @@ -270,6 +272,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), expectErr: false, }, @@ -278,6 +281,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins), internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), expectErr: false, }, @@ -286,6 +290,7 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { gaugeStartTime: defaultGaugeStartTime, gaugeCoins: sdk.NewCoins(fiveKRewardCoins), internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), expectErr: false, }, @@ -294,11 +299,36 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { gaugeStartTime: defaultGaugeStartTime.Add(-5 * time.Hour), gaugeCoins: sdk.NewCoins(fiveKRewardCoins), internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), expectErr: false, }, - - // 1d authorized, 7d internal + // We expect incentive record to fall back to the default uptime, since the internal uptime is unauthorized. + "valid case: unauthorized internal uptime": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: time.Hour * 24 * 7, + expectedUptime: types.DefaultConcentratedUptime, + expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), + }, + "valid case: invalid internal uptime": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: time.Hour * 4, + expectedUptime: types.DefaultConcentratedUptime, + expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), + expectErr: false, + }, + "valid case: nil internal uptime": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + expectedUptime: types.DefaultConcentratedUptime, + expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), + expectErr: false, + }, } for name, tc := range tests { @@ -362,56 +392,62 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { poolId, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, gauge.GetId(), currentEpoch.Duration) s.Require().NoError(err) - // check that incentive record wasn't created + // check that incentive record wasn't created on any viable uptime + _, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, types.DefaultConcentratedUptime, uint64(incentiveId)) + s.Require().Error(err) + _, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, tc.internalUptime, uint64(incentiveId)) + s.Require().Error(err) _, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, currentEpoch.Duration, uint64(incentiveId)) s.Require().Error(err) } } - } else { - s.Require().NoError(err) - // check that gauge is not empty - s.Require().NotEqual(len(gauges), 0) + return + } - // check if module amount got deducted correctly - balance := s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName)) - for _, coin := range balance { - actualbalanceAfterDistribution := coinsToMint.AmountOf(coin.Denom).Sub(coin.Amount) - s.Require().Equal(tc.expectedDistributions.AmountOf(coin.Denom).Add(osmomath.ZeroInt()), actualbalanceAfterDistribution.Add(osmomath.ZeroInt())) - } + s.Require().NoError(err) - for i, gauge := range gauges { - for j, coin := range gauge.Coins { - incentiveId := i*len(gauge.Coins) + j + 1 + // check that gauge is not empty + s.Require().NotEqual(len(gauges), 0) - gaugeId := gauge.GetId() + // check if module amount got deducted correctly + balance := s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName)) + for _, coin := range balance { + actualbalanceAfterDistribution := coinsToMint.AmountOf(coin.Denom).Sub(coin.Amount) + s.Require().Equal(tc.expectedDistributions.AmountOf(coin.Denom).Add(osmomath.ZeroInt()), actualbalanceAfterDistribution.Add(osmomath.ZeroInt())) + } - // get poolId from GaugeId - poolId, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, gaugeId, currentEpoch.Duration) - s.Require().NoError(err) + for i, gauge := range gauges { + for j, coin := range gauge.Coins { + incentiveId := i*len(gauge.Coins) + j + 1 - // GetIncentiveRecord to see if pools received incentives properly - incentiveRecord, err := s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, types.DefaultConcentratedUptime, uint64(incentiveId)) - s.Require().NoError(err) + gaugeId := gauge.GetId() - expectedEmissionRate := osmomath.NewDecFromInt(coin.Amount).QuoTruncate(osmomath.NewDec(int64(currentEpoch.Duration.Seconds()))) + // get poolId from GaugeId + poolId, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, gaugeId, currentEpoch.Duration) + s.Require().NoError(err) - // Check that gauge distribution state is updated. - s.ValidateDistributedGauge(gaugeId, 1, tc.gaugeCoins) + // GetIncentiveRecord to see if pools received incentives properly + incentiveRecord, err := s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, tc.expectedUptime, uint64(incentiveId)) + s.Require().NoError(err) - // check every parameter in incentiveRecord so that it matches what we created - incentiveRecordBody := incentiveRecord.GetIncentiveRecordBody() - s.Require().Equal(poolId, incentiveRecord.PoolId) - s.Require().Equal(expectedEmissionRate, incentiveRecordBody.EmissionRate) - s.Require().Equal(s.Ctx.BlockTime().UTC().String(), incentiveRecordBody.StartTime.UTC().String()) - s.Require().Equal(types.DefaultConcentratedUptime, incentiveRecord.MinUptime) - s.Require().Equal(coin.Amount, incentiveRecordBody.RemainingCoin.Amount.TruncateInt()) - s.Require().Equal(coin.Denom, incentiveRecordBody.RemainingCoin.Denom) - } + expectedEmissionRate := osmomath.NewDecFromInt(coin.Amount).QuoTruncate(osmomath.NewDec(int64(currentEpoch.Duration.Seconds()))) + + // Check that gauge distribution state is updated. + s.ValidateDistributedGauge(gaugeId, 1, tc.gaugeCoins) + + // check every parameter in incentiveRecord so that it matches what we created + incentiveRecordBody := incentiveRecord.GetIncentiveRecordBody() + s.Require().Equal(poolId, incentiveRecord.PoolId) + s.Require().Equal(expectedEmissionRate, incentiveRecordBody.EmissionRate) + s.Require().Equal(s.Ctx.BlockTime().UTC().String(), incentiveRecordBody.StartTime.UTC().String()) + s.Require().Equal(tc.expectedUptime, incentiveRecord.MinUptime) + s.Require().Equal(coin.Amount, incentiveRecordBody.RemainingCoin.Amount.TruncateInt()) + s.Require().Equal(coin.Denom, incentiveRecordBody.RemainingCoin.Denom) } - // check the totalAmount of tokens distributed, for both lock gauges and CL pool gauges - s.Require().Equal(tc.expectedDistributions, totalDistributedCoins) } + // check the totalAmount of tokens distributed, for both lock gauges and CL pool gauges + s.Require().Equal(tc.expectedDistributions, totalDistributedCoins) }) } } From 83356f95662ba04fb8ca2ef71cf3cb8c59f1eedb Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 21:10:02 -0800 Subject: [PATCH 5/8] clean up internal gauge distribution tests --- x/incentives/keeper/distribute_test.go | 124 ++++++++++--------------- 1 file changed, 49 insertions(+), 75 deletions(-) diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index 0b8f1cf962e..e131e0e1e20 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -254,80 +254,81 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { internalUptime time.Duration // expected - expectErr bool + // Note: internal gauge distributions should never error, so we don't expect any errors here. expectedDistributions sdk.Coins expectedUptime time.Duration }{ - "valid case: one poolId and gaugeId": { - numPools: 1, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), - internalUptime: types.DefaultConcentratedUptime, + "one poolId and gaugeId": { + numPools: 1, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), - expectErr: false, }, - "valid case: gauge with multiple coins": { - numPools: 1, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), - internalUptime: types.DefaultConcentratedUptime, + "gauge with multiple coins": { + numPools: 1, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), + internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins, fiveKRewardCoinsUosmo), - expectErr: false, }, - "valid case: multiple gaugeId and poolId": { - numPools: 3, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), - internalUptime: types.DefaultConcentratedUptime, + "multiple gaugeId and poolId": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), - expectErr: false, }, - "valid case: one poolId and gaugeId, five 5000 coins": { - numPools: 1, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), - internalUptime: types.DefaultConcentratedUptime, + "one poolId and gaugeId, five 5000 coins": { + numPools: 1, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), - expectErr: false, }, - "valid case: attempt to createIncentiveRecord with start time < currentBlockTime - gets set to block time in incentive record": { - numPools: 1, - gaugeStartTime: defaultGaugeStartTime.Add(-5 * time.Hour), - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), - internalUptime: types.DefaultConcentratedUptime, + "attempt to createIncentiveRecord with start time < currentBlockTime - gets set to block time in incentive record": { + numPools: 1, + gaugeStartTime: defaultGaugeStartTime.Add(-5 * time.Hour), + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: types.DefaultConcentratedUptime, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fiveKRewardCoins), - expectErr: false, }, // We expect incentive record to fall back to the default uptime, since the internal uptime is unauthorized. - "valid case: unauthorized internal uptime": { - numPools: 3, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), - internalUptime: time.Hour * 24 * 7, + "unauthorized internal uptime": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: time.Hour * 24 * 7, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), }, - "valid case: invalid internal uptime": { - numPools: 3, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), - internalUptime: time.Hour * 4, + "invalid internal uptime": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + internalUptime: time.Hour * 4, + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), - expectErr: false, }, - "valid case: nil internal uptime": { - numPools: 3, - gaugeStartTime: defaultGaugeStartTime, - gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + "nil internal uptime": { + numPools: 3, + gaugeStartTime: defaultGaugeStartTime, + gaugeCoins: sdk.NewCoins(fiveKRewardCoins), + expectedUptime: types.DefaultConcentratedUptime, expectedDistributions: sdk.NewCoins(fifteenKRewardCoins), - expectErr: false, }, } @@ -375,35 +376,8 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { gauges = append(gauges, *gauge) } - // Distribute tokens from the gauge + // System under test: Distribute tokens from the gauge totalDistributedCoins, err := s.App.IncentivesKeeper.Distribute(s.Ctx, gauges) - if tc.expectErr { - s.Require().Error(err) - - // module account amount must stay the same - balance := s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName)) - s.Require().Equal(coinsToMint, balance) - - for i, gauge := range gauges { - for j := range gauge.Coins { - incentiveId := i*len(gauge.Coins) + j + 1 - - // get poolId from GaugeId - poolId, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, gauge.GetId(), currentEpoch.Duration) - s.Require().NoError(err) - - // check that incentive record wasn't created on any viable uptime - _, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, types.DefaultConcentratedUptime, uint64(incentiveId)) - s.Require().Error(err) - _, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, tc.internalUptime, uint64(incentiveId)) - s.Require().Error(err) - _, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, currentEpoch.Duration, uint64(incentiveId)) - s.Require().Error(err) - } - } - - return - } s.Require().NoError(err) From 0a97633085c80da19ec24e08196d31337f082572 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 21:18:11 -0800 Subject: [PATCH 6/8] attempt to clean up diff --- x/incentives/keeper/distribute_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index e131e0e1e20..1b53673c648 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -378,7 +378,6 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() { // System under test: Distribute tokens from the gauge totalDistributedCoins, err := s.App.IncentivesKeeper.Distribute(s.Ctx, gauges) - s.Require().NoError(err) // check that gauge is not empty From 3554d1207770f34148851deb3ba452470b5d6baa Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 5 Feb 2024 21:24:54 -0800 Subject: [PATCH 7/8] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c6751aab4a..e76f74c5ec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#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 * [#7357](https://github.com/osmosis-labs/osmosis/pull/7357) Fix: Ensure rate limits are not applied to packets that aren't ics20s * [#7417](https://github.com/osmosis-labs/osmosis/pull/7417) Update CL gauges to use gauge duration as uptime, falling back to default if unauthorized or invalid +* [#7419](https://github.com/osmosis-labs/osmosis/pull/7419) Use new module param for internal incentive uptimes ### Bug Fixes From a91496028ad25f8b74c74ae0a83cc38cd598f852 Mon Sep 17 00:00:00 2001 From: alpo Date: Tue, 6 Feb 2024 20:38:48 -0800 Subject: [PATCH 8/8] extract uptime fetch into helper --- x/incentives/keeper/distribute.go | 56 +++++++++++++++++--------- x/incentives/keeper/distribute_test.go | 38 +++++++++++++++++ x/incentives/keeper/export_test.go | 4 ++ 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index ea2fb2f2ee0..34b957782e2 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -550,6 +550,39 @@ func (k Keeper) syncVolumeSplitGroup(ctx sdk.Context, group types.Group) error { return nil } +// getNoLockGaugeUptime retrieves the uptime corresponding to the passed in gauge. +// For external gauges, it returns the uptime specified in the gauge. +// For internal gauges, it returns the module param for internal gauge uptime. +// +// In either case, if the fetched uptime is invalid or unauthorized, it falls back to a default uptime. +func (k Keeper) getNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge) time.Duration { + // TODO: use module param as uptime for internal gauges once it is implemented. + // (Ref: https://github.com/osmosis-labs/osmosis/issues/7371) + + // Validate that the gauge's corresponding uptime is authorized. + authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes + gaugeUptime := gauge.DistributeTo.Duration + isUptimeAuthorized := false + for _, authorizedUptime := range authorizedUptimes { + if gaugeUptime == authorizedUptime { + isUptimeAuthorized = true + } + } + + // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. + // + // This is for two reasons: + // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges + // 2. To avoid having to do a state migration on existing gauges at time of adding + // this change, since prior to this, CL gauges were not required to associate with + // an uptime that was authorized. + if !isUptimeAuthorized { + gaugeUptime = types.DefaultConcentratedUptime + } + + return gaugeUptime +} + // distributeInternal runs the distribution logic for a gauge, and adds the sends to // the distrInfo struct. It also updates the gauge for the distribution. // It handles any kind of gauges: @@ -602,26 +635,9 @@ func (k Keeper) distributeInternal( // Get distribution epoch duration. This is used to calculate the emission rate. currentEpoch := k.GetEpochInfo(ctx) - // Validate that the gauge's corresponding uptime is authorized. - authorizedUptimes := k.clk.GetParams(ctx).AuthorizedUptimes - gaugeUptime := gauge.DistributeTo.Duration - isUptimeAuthorized := false - for _, authorizedUptime := range authorizedUptimes { - if gaugeUptime == authorizedUptime { - isUptimeAuthorized = true - } - } - - // If the gauge's uptime is not authorized, we fall back to a default instead of erroring. - // - // This is for two reasons: - // 1. To allow uptimes to be unauthorized without entirely freezing existing gauges - // 2. To avoid having to do a state migration on existing gauges at time of adding - // this change, since prior to this, CL gauges were not required to associate with - // an uptime that was authorized. - if !isUptimeAuthorized { - gaugeUptime = types.DefaultConcentratedUptime - } + // Get the uptime for the gauge. Note that if the gauge's uptime is not authorized, + // this falls back to a default value of 1ns. + gaugeUptime := k.getNoLockGaugeUptime(ctx, gauge) // For every coin in the gauge, calculate the remaining reward per epoch // and create a concentrated liquidity incentive record for it that diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index b2e74fb9eae..90649739f4b 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -2451,3 +2451,41 @@ func (s *KeeperTestSuite) TestHandleGroupPostDistribute() { validateLastEpochNonPerpetualPruning(currentGauge.Id, currentGauge.DistributedCoins.Add(defaultCoins...), initialDistributionCoins, s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName))) }) } + +func (s *KeeperTestSuite) TestGetNoLockGaugeUptime() { + tests := map[string]struct { + gauge types.Gauge + authorizedUptimes []time.Duration + expectedUptime time.Duration + }{ + "external gauge with authorized uptime": { + gauge: types.Gauge{ + DistributeTo: lockuptypes.QueryCondition{Duration: time.Hour}, + }, + authorizedUptimes: []time.Duration{types.DefaultConcentratedUptime, time.Hour}, + expectedUptime: time.Hour, + }, + "external gauge with unauthorized uptime": { + gauge: types.Gauge{ + DistributeTo: lockuptypes.QueryCondition{Duration: time.Minute}, + }, + authorizedUptimes: []time.Duration{types.DefaultConcentratedUptime}, + expectedUptime: types.DefaultConcentratedUptime, + }, + } + + for name, tc := range tests { + s.Run(name, func() { + // Setup CL params with authorized uptimes + clParams := s.App.ConcentratedLiquidityKeeper.GetParams(s.Ctx) + clParams.AuthorizedUptimes = tc.authorizedUptimes + s.App.ConcentratedLiquidityKeeper.SetParams(s.Ctx, clParams) + + // System under test + actualUptime := s.App.IncentivesKeeper.GetNoLockGaugeUptime(s.Ctx, tc.gauge) + + // Ensure correct uptime was returned + s.Require().Equal(tc.expectedUptime, actualUptime) + }) + } +} diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index c764f3a1bce..56aa3f4c683 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -97,3 +97,7 @@ func (k Keeper) CreateGroupInternal(ctx sdk.Context, coins sdk.Coins, numEpochPa func (k Keeper) CalculateGroupWeights(ctx sdk.Context, group types.Group) (types.Group, error) { return k.calculateGroupWeights(ctx, group) } + +func (k Keeper) GetNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge) time.Duration { + return k.getNoLockGaugeUptime(ctx, gauge) +}