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

[Uptime Incentives]: Incorporate gov param in distribution logic #7419

Merged
merged 12 commits into from
Feb 7, 2024
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 8 additions & 5 deletions x/incentives/keeper/distribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,13 +555,16 @@ func (k Keeper) syncVolumeSplitGroup(ctx sdk.Context, group types.Group) error {
// 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)
func (k Keeper) getNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge, poolId uint64) time.Duration {
// If internal gauge, use InternalUptime param as the gauge's uptime.
// Otherwise, use the gauge's duration.
gaugeUptime := gauge.DistributeTo.Duration
if gauge.DistributeTo.Denom == types.NoLockInternalGaugeDenom(poolId) {
gaugeUptime = k.GetParams(ctx).InternalUptime
}

// 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 {
Expand Down Expand Up @@ -637,7 +640,7 @@ func (k Keeper) distributeInternal(

// 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)
gaugeUptime := k.getNoLockGaugeUptime(ctx, gauge, pool.GetId())

// For every coin in the gauge, calculate the remaining reward per epoch
// and create a concentrated liquidity incentive record for it that
Expand Down
216 changes: 134 additions & 82 deletions x/incentives/keeper/distribute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,45 +251,84 @@ func (s *KeeperTestSuite) TestDistribute_InternalIncentives_NoLock() {
tokensToAddToGauge sdk.Coins
gaugeStartTime time.Time
gaugeCoins sdk.Coins
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": {
Copy link
Contributor Author

@AlpinYukseloglu AlpinYukseloglu Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: diff is a bit messy due to driveby test case renaming/removal of expectErr: falses. Happy to undo either of these if anyone has strong qualms.

numPools: 1,
gaugeStartTime: defaultGaugeStartTime,
gaugeCoins: sdk.NewCoins(fiveKRewardCoins),
"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),
"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),
"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),
"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),
"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.
"unauthorized internal uptime": {
numPools: 3,
gaugeStartTime: defaultGaugeStartTime,
gaugeCoins: sdk.NewCoins(fiveKRewardCoins),
internalUptime: time.Hour * 24 * 7,

expectedUptime: types.DefaultConcentratedUptime,
expectedDistributions: sdk.NewCoins(fifteenKRewardCoins),
},
"invalid internal uptime": {
numPools: 3,
gaugeStartTime: defaultGaugeStartTime,
gaugeCoins: sdk.NewCoins(fiveKRewardCoins),
internalUptime: time.Hour * 4,

expectedUptime: types.DefaultConcentratedUptime,
expectedDistributions: sdk.NewCoins(fifteenKRewardCoins),
},
"nil internal uptime": {
numPools: 3,
gaugeStartTime: defaultGaugeStartTime,
gaugeCoins: sdk.NewCoins(fiveKRewardCoins),

expectedUptime: types.DefaultConcentratedUptime,
expectedDistributions: sdk.NewCoins(fifteenKRewardCoins),
},
}

Expand All @@ -301,6 +340,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
Expand Down Expand Up @@ -334,73 +376,51 @@ 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
_, err = s.App.ConcentratedLiquidityKeeper.GetIncentiveRecord(s.Ctx, poolId, currentEpoch.Duration, uint64(incentiveId))
s.Require().Error(err)
}
}
} else {
Copy link
Contributor Author

@AlpinYukseloglu AlpinYukseloglu Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a couple cleanup attempts, it seems a messed up diff is unavoidable here. The core changes are:

  1. Removed error branch, as the logic covered by these cases should never be erroring (also there were no error cases)
  2. Moved the passing logic out of the else block, which caused the indentation-related diff below
  3. Updated uptime checks to be against tc.expectedUptime instead of a static default value

Note: i made the changes for 1 in a separate commit in case we want to revert. I find this version of the tests to be much less verbose & more readable, but on the fence on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine to me imo

s.Require().NoError(err)
s.Require().NoError(err)

// check that gauge is not empty
s.Require().NotEqual(len(gauges), 0)
// check that gauge is not empty
s.Require().NotEqual(len(gauges), 0)

// 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()))
}
// 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()))
}

for i, gauge := range gauges {
for j, coin := range gauge.Coins {
incentiveId := i*len(gauge.Coins) + j + 1
for i, gauge := range gauges {
for j, coin := range gauge.Coins {
incentiveId := i*len(gauge.Coins) + j + 1

gaugeId := gauge.GetId()
gaugeId := gauge.GetId()

// get poolId from GaugeId
poolId, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, gaugeId, currentEpoch.Duration)
s.Require().NoError(err)
// get poolId from GaugeId
poolId, err := s.App.PoolIncentivesKeeper.GetPoolIdFromGaugeId(s.Ctx, gaugeId, currentEpoch.Duration)
s.Require().NoError(err)

// 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)
// 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)

expectedEmissionRate := osmomath.NewDecFromInt(coin.Amount).QuoTruncate(osmomath.NewDec(int64(currentEpoch.Duration.Seconds())))
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 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(types.DefaultConcentratedUptime, incentiveRecord.MinUptime)
s.Require().Equal(coin.Amount, incentiveRecordBody.RemainingCoin.Amount.TruncateInt())
s.Require().Equal(coin.Denom, incentiveRecordBody.RemainingCoin.Denom)
}
// 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)
})
}
}
Expand Down Expand Up @@ -2453,9 +2473,11 @@ func (s *KeeperTestSuite) TestHandleGroupPostDistribute() {
}

func (s *KeeperTestSuite) TestGetNoLockGaugeUptime() {
defaultPoolId := uint64(1)
tests := map[string]struct {
gauge types.Gauge
authorizedUptimes []time.Duration
internalUptime time.Duration
expectedUptime time.Duration
}{
"external gauge with authorized uptime": {
Expand All @@ -2472,17 +2494,47 @@ func (s *KeeperTestSuite) TestGetNoLockGaugeUptime() {
authorizedUptimes: []time.Duration{types.DefaultConcentratedUptime},
expectedUptime: types.DefaultConcentratedUptime,
},
"internal gauge with authorized uptime": {
gauge: types.Gauge{
DistributeTo: lockuptypes.QueryCondition{
Denom: types.NoLockInternalGaugeDenom(defaultPoolId),
Duration: time.Hour,
},
},
authorizedUptimes: []time.Duration{types.DefaultConcentratedUptime, time.Hour},
internalUptime: time.Hour,
expectedUptime: time.Hour,
},
"internal gauge with unauthorized uptime": {
gauge: types.Gauge{
DistributeTo: lockuptypes.QueryCondition{
Denom: types.NoLockInternalGaugeDenom(defaultPoolId),
Duration: time.Hour,
},
},
authorizedUptimes: []time.Duration{types.DefaultConcentratedUptime},
internalUptime: time.Hour,
expectedUptime: types.DefaultConcentratedUptime,
},
}

for name, tc := range tests {
s.Run(name, func() {
// Prepare CL pool
clPool := s.PrepareConcentratedPool()

// Setup CL params with authorized uptimes
clParams := s.App.ConcentratedLiquidityKeeper.GetParams(s.Ctx)
clParams.AuthorizedUptimes = tc.authorizedUptimes
s.App.ConcentratedLiquidityKeeper.SetParams(s.Ctx, clParams)

// Set internal uptime module param if applicable
if tc.internalUptime != time.Duration(0) {
s.App.IncentivesKeeper.SetParam(s.Ctx, types.KeyInternalUptime, tc.internalUptime)
}

// System under test
actualUptime := s.App.IncentivesKeeper.GetNoLockGaugeUptime(s.Ctx, tc.gauge)
actualUptime := s.App.IncentivesKeeper.GetNoLockGaugeUptime(s.Ctx, tc.gauge, clPool.GetId())

// Ensure correct uptime was returned
s.Require().Equal(tc.expectedUptime, actualUptime)
Expand Down
4 changes: 2 additions & 2 deletions x/incentives/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,6 @@ func (k Keeper) CalculateGroupWeights(ctx sdk.Context, group types.Group) (types
return k.calculateGroupWeights(ctx, group)
}

func (k Keeper) GetNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge) time.Duration {
return k.getNoLockGaugeUptime(ctx, gauge)
func (k Keeper) GetNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge, poolId uint64) time.Duration {
return k.getNoLockGaugeUptime(ctx, gauge, poolId)
}