feat: scaling factor for pool uptime accumulator

app/apptesting/test_suite.go

@@ -696,3 +696,11 @@ func (s *KeeperTestHelper) SetupVolumeForPools(poolIDs []uint64, volumesForEachP
 func (s *KeeperTestHelper) IncreaseVolumeForPools(poolIDs []uint64, volumesForEachPool []osmomath.Int) {
 	s.SetupVolumeForPools(poolIDs, volumesForEachPool, map[uint64]osmomath.Int{})
 }
+
+// CompareDecCoinsSlice compares two slices of DecCoins
+func (s *KeeperTestHelper) CompareDecCoinsSlice(expected, actual []sdk.DecCoins) {
+	s.Require().Equal(len(expected), len(actual))
+	for i := range actual {
+		s.Require().Equal(expected[i].String(), actual[i].String())
+	}
+}

x/concentrated-liquidity/export_test.go

@@ -18,11 +18,12 @@ const (
 )
 
 var (
-	EmptyCoins         = emptyCoins
-	HundredFooCoins    = sdk.NewDecCoin("foo", osmomath.NewInt(100))
-	HundredBarCoins    = sdk.NewDecCoin("bar", osmomath.NewInt(100))
-	TwoHundredFooCoins = sdk.NewDecCoin("foo", osmomath.NewInt(200))
-	TwoHundredBarCoins = sdk.NewDecCoin("bar", osmomath.NewInt(200))
+	EmptyCoins              = emptyCoins
+	HundredFooCoins         = sdk.NewDecCoin("foo", osmomath.NewInt(100))
+	HundredBarCoins         = sdk.NewDecCoin("bar", osmomath.NewInt(100))
+	TwoHundredFooCoins      = sdk.NewDecCoin("foo", osmomath.NewInt(200))
+	TwoHundredBarCoins      = sdk.NewDecCoin("bar", osmomath.NewInt(200))
+	PerUnitLiqScalingFactor = perUnitLiqScalingFactor
 )
 
 func (k Keeper) SetPool(ctx sdk.Context, pool types.ConcentratedPoolExtension) error {

x/concentrated-liquidity/incentives.go

@@ -19,6 +19,8 @@ import (
 	"github.com/osmosis-labs/osmosis/v22/x/concentrated-liquidity/types"
 )
 
+var perUnitLiqScalingFactor = osmomath.NewDec(1e18)
+
 // createUptimeAccumulators creates accumulator objects in store for each supported uptime for the given poolId.
 // The accumulators are initialized with the default (zero) values.
 func (k Keeper) createUptimeAccumulators(ctx sdk.Context, poolId uint64) error {
@@ -240,16 +242,19 @@ func calcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, l
 		}
 
 		// Total amount emitted = time elapsed * emission
-		totalEmittedAmount := timeElapsed.Mul(incentiveRecordBody.EmissionRate)
+		totalEmittedAmount := timeElapsed.MulTruncate(incentiveRecordBody.EmissionRate)
+		// We scale up the remaining rewards to avoid truncation to zero
+		// when dividing by the liquidity in the accumulator.
+		scaledTotalEmittedAmount := totalEmittedAmount.MulTruncate(perUnitLiqScalingFactor)
 
 		// Incentives to emit per unit of qualifying liquidity = total emitted / liquidityInAccum
 		// Note that we truncate to ensure we do not overdistribute incentives
-		incentivesPerLiquidity := totalEmittedAmount.QuoTruncate(liquidityInAccum)
+		incentivesPerLiquidity := scaledTotalEmittedAmount.QuoTruncate(liquidityInAccum)
 
 		// If truncation occurs, we emit events to alert us of the issue.
-		if incentivesPerLiquidity.IsZero() && !totalEmittedAmount.IsZero() {
+		if incentivesPerLiquidity.IsZero() && !scaledTotalEmittedAmount.IsZero() {
 			telemetry.IncrCounter(1, types.IncentiveTruncationPlaceholderName)
-			ctx.Logger().Error(types.IncentiveTruncationPlaceholderName, "pool_id", poolID, "total_liq", liquidityInAccum, "per_unit_liq", incentivesPerLiquidity, "total_amt", totalEmittedAmount)
+			ctx.Logger().Error(types.IncentiveTruncationPlaceholderName, "pool_id", poolID, "total_liq", liquidityInAccum, "per_unit_liq", incentivesPerLiquidity, "total_amt", totalEmittedAmount, "total_amt_scaled", scaledTotalEmittedAmount)
 		}
 
 		emittedIncentivesPerLiquidity := sdk.NewDecCoinFromDec(incentiveRecordBody.RemainingCoin.Denom, incentivesPerLiquidity)
@@ -269,7 +274,18 @@ func calcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, l
 		} else {
 			// If there are not enough incentives remaining to be emitted, we emit the remaining rewards.
 			// When the returned records are set in state, all records with remaining rewards of zero will be cleared.
-			remainingIncentivesPerLiquidity := remainingRewards.QuoTruncate(liquidityInAccum)
+
+			// We scale up the remaining rewards to avoid truncation to zero
+			// when dividing by the liquidity in the accumulator.
+			remainingRewardsScaled := remainingRewards.MulTruncate(perUnitLiqScalingFactor)
+			remainingIncentivesPerLiquidity := remainingRewardsScaled.QuoTruncateMut(liquidityInAccum)
+
+			// Detect truncation and emit telemetry to alert us of the issue.
+			if remainingIncentivesPerLiquidity.IsZero() && !remainingRewardsScaled.IsZero() {
+				telemetry.IncrCounter(1, types.IncentiveTruncationPlaceholderName)
+				ctx.Logger().Error(types.IncentiveTruncationPlaceholderName, "pool_id", poolID, "total_liq", liquidityInAccum, "per_unit_liq", incentivesPerLiquidity, "total_amt", remainingRewards, "total_amt_scaled", remainingRewardsScaled)
+			}
+
 			emittedIncentivesPerLiquidity = sdk.NewDecCoinFromDec(incentiveRecordBody.RemainingCoin.Denom, remainingIncentivesPerLiquidity)
 			incentivesToAddToCurAccum = incentivesToAddToCurAccum.Add(emittedIncentivesPerLiquidity)
 
@@ -704,11 +720,22 @@ func (k Keeper) prepareClaimAllIncentivesForPosition(ctx sdk.Context, positionId
 
 		// If the accumulator contains the position, claim the position's incentives.
 		if hasPosition {
-			collectedIncentivesForUptime, _, err := updateAccumAndClaimRewards(uptimeAccum, positionName, uptimeGrowthOutside[uptimeIndex])
+			collectedIncentivesForUptimeScaled, _, err := updateAccumAndClaimRewards(uptimeAccum, positionName, uptimeGrowthOutside[uptimeIndex])
 			if err != nil {
 				return sdk.Coins{}, sdk.Coins{}, err
 			}
 
+			// We scale the uptime per-unit of liquidity accumulator up to avoid truncation to zero.
+			// However, once we compute the total for the liquidity entitlement, we must scale it back down.
+			// We always truncate down in the pool's favor.
+			collectedIncentivesForUptime := sdk.NewCoins()
+			for _, incentiveCoin := range collectedIncentivesForUptimeScaled {
+				incentiveCoin.Amount = incentiveCoin.Amount.ToLegacyDec().QuoTruncateMut(perUnitLiqScalingFactor).TruncateInt()
+				if incentiveCoin.Amount.IsPositive() {
+					collectedIncentivesForUptime = append(collectedIncentivesForUptime, incentiveCoin)
+				}
+			}
+
 			if positionAge < supportedUptimes[uptimeIndex] {
 				// If the age of the position is less than the current uptime we are iterating through, then the position's
 				// incentives are forfeited to the community pool. The parent function does the actual bank send.

x/concentrated-liquidity/incentives_test.go

@@ -172,7 +172,7 @@ func wrapUptimeTrackers(accumValues []sdk.DecCoins) []model.UptimeTracker {
 
 // expectedIncentivesFromRate calculates the amount of incentives we expect to accrue based on the rate and time elapsed
 func expectedIncentivesFromRate(denom string, rate osmomath.Dec, timeElapsed time.Duration, qualifyingLiquidity osmomath.Dec) sdk.DecCoin {
-	timeInSec := osmomath.NewDec(int64(timeElapsed)).Quo(osmomath.MustNewDecFromStr("1000000000"))
+	timeInSec := osmomath.NewDec(int64(timeElapsed)).Quo(osmomath.MustNewDecFromStr("1000000000")).MulTruncateMut(cl.PerUnitLiqScalingFactor)
 	amount := rate.Mul(timeInSec).QuoTruncate(qualifyingLiquidity)
 
 	return sdk.NewDecCoinFromDec(denom, amount)
@@ -214,7 +214,7 @@ func addToUptimeAccums(ctx sdk.Context, poolId uint64, clKeeper *cl.Keeper, addV
 	}
 
 	for uptimeIndex, uptimeAccum := range poolUptimeAccumulators {
-		uptimeAccum.AddToAccumulator(addValues[uptimeIndex])
+		uptimeAccum.AddToAccumulator(addValues[uptimeIndex].MulDecTruncate(cl.PerUnitLiqScalingFactor))
 	}
 
 	return nil
@@ -568,7 +568,7 @@ func (s *KeeperTestSuite) TestCalcAccruedIncentivesForAccum() {
 
 			// We expect the fully incentive amount to be emitted
 			expectedResult: sdk.DecCoins{
-				sdk.NewDecCoinFromDec(incentiveRecordOne.IncentiveRecordBody.RemainingCoin.Denom, incentiveRecordOne.IncentiveRecordBody.RemainingCoin.Amount.QuoTruncate(osmomath.NewDec(123))),
+				sdk.NewDecCoinFromDec(incentiveRecordOne.IncentiveRecordBody.RemainingCoin.Denom, incentiveRecordOne.IncentiveRecordBody.RemainingCoin.Amount.MulTruncate(cl.PerUnitLiqScalingFactor).QuoTruncate(osmomath.NewDec(123))),
 			},
 
 			// Incentive record should have zero remaining amountosmomath.ZeroDec
@@ -1372,6 +1372,12 @@ func (s *KeeperTestSuite) TestGetUptimeGrowthRange() {
 				pool := s.PrepareConcentratedPool()
 				currentTick := pool.GetCurrentTick()
 
+				// Note: we scale all these values up as addToUptimeAccums(...) does the same for the global uptime accums.
+				tc.lowerTickUptimeGrowthOutside = s.scaleUptimeAccumulators(tc.lowerTickUptimeGrowthOutside)
+				tc.upperTickUptimeGrowthOutside = s.scaleUptimeAccumulators(tc.upperTickUptimeGrowthOutside)
+				tc.expectedUptimeGrowthInside = s.scaleUptimeAccumulators(tc.expectedUptimeGrowthInside)
+				tc.expectedUptimeGrowthOutside = s.scaleUptimeAccumulators(tc.expectedUptimeGrowthOutside)
+
 				// Update global uptime accums
 				err := addToUptimeAccums(s.Ctx, pool.GetId(), s.App.ConcentratedLiquidityKeeper, tc.globalUptimeGrowth)
 				s.Require().NoError(err)
@@ -1388,13 +1394,13 @@ func (s *KeeperTestSuite) TestGetUptimeGrowthRange() {
 				s.Require().NoError(err)
 
 				// check if returned uptime growth inside has correct value
-				s.Require().Equal(tc.expectedUptimeGrowthInside, uptimeGrowthInside)
+				s.CompareDecCoinsSlice(tc.expectedUptimeGrowthInside, uptimeGrowthInside)
 
 				uptimeGrowthOutside, err := s.App.ConcentratedLiquidityKeeper.GetUptimeGrowthOutsideRange(s.Ctx, pool.GetId(), tc.lowerTick, tc.upperTick)
 				s.Require().NoError(err)
 
 				// check if returned uptime growth inside has correct value
-				s.Require().Equal(tc.expectedUptimeGrowthOutside, uptimeGrowthOutside)
+				s.CompareDecCoinsSlice(tc.expectedUptimeGrowthOutside, uptimeGrowthOutside)
 			})
 		}
 	})
@@ -1439,13 +1445,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 
 		"(lower < curr < upper) nonzero uptime trackers": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			positionId:               DefaultPositionId,
 			currentTickIndex:         0,
@@ -1455,13 +1462,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"(lower < curr < upper) non-zero uptime trackers (position already existing)": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			existingPosition:  true,
 			addToGlobalAccums: uptimeHelper.threeHundredTokensMultiDenom,
@@ -1478,13 +1486,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"(lower < upper < curr) nonzero uptime trackers": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.threeHundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.threeHundredTokensMultiDenom)),
 			},
 			positionId:               DefaultPositionId,
 			currentTickIndex:         51,
@@ -1494,13 +1503,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"(lower < upper < curr) non-zero uptime trackers (position already existing)": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.threeHundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.threeHundredTokensMultiDenom)),
 			},
 			existingPosition:  true,
 			addToGlobalAccums: uptimeHelper.threeHundredTokensMultiDenom,
@@ -1517,13 +1527,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"(curr < lower < upper) nonzero uptime trackers": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.threeHundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.threeHundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			positionId:               DefaultPositionId,
 			currentTickIndex:         -51,
@@ -1533,13 +1544,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"(curr < lower < upper) nonzero uptime trackers (position already existing)": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.threeHundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.threeHundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			existingPosition:  true,
 			addToGlobalAccums: uptimeHelper.threeHundredTokensMultiDenom,
@@ -1556,13 +1568,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"(lower < curr < upper) nonzero and variable uptime trackers": {
 			positionLiquidity: DefaultLiquidityAmt,
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.varyingTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.varyingTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			positionId:       DefaultPositionId,
 			currentTickIndex: 0,
@@ -1613,13 +1626,14 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 		},
 		"error: negative liquidity for first position": {
 			positionLiquidity: DefaultLiquidityAmt.Neg(),
+			// Note: that we scale uptime tracker values up as addToUptimeAccums(...) does the same for the global uptime accums.
 			lowerTick: tick{
 				tickIndex:      -50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			upperTick: tick{
 				tickIndex:      50,
-				uptimeTrackers: wrapUptimeTrackers(uptimeHelper.hundredTokensMultiDenom),
+				uptimeTrackers: wrapUptimeTrackers(s.scaleUptimeAccumulators(uptimeHelper.hundredTokensMultiDenom)),
 			},
 			positionId:              DefaultPositionId,
 			currentTickIndex:        0,
@@ -1637,6 +1651,10 @@ func (s *KeeperTestSuite) TestInitOrUpdatePositionUptimeAccumulators() {
 				// Init suite for each test.
 				s.SetupTest()
 
+				// Note: that we scale all these values up as addToUptimeAccums(...) does the same for the global uptime accums.
+				test.expectedInitAccumValue = s.scaleUptimeAccumulators(test.expectedInitAccumValue)
+				test.expectedUnclaimedRewards = s.scaleUptimeAccumulators(test.expectedUnclaimedRewards)
+
 				// Set blocktime to fixed UTC value for consistency
 				s.Ctx = s.Ctx.WithBlockTime(DefaultJoinTime)
 
@@ -2898,7 +2916,7 @@ func (s *KeeperTestSuite) TestQueryAndClaimAllIncentives() {
 					for _, growthInside := range tc.growthInside {
 						expectedCoins = expectedCoins.Add(sdk.NormalizeCoins(growthInside)...)
 					}
-					s.Require().Equal(expectedCoins, amountClaimed)
+					s.Require().Equal(expectedCoins.String(), amountClaimed.String())
 					s.Require().Equal(sdk.Coins{}, amountForfeited)
 				}
 
@@ -3022,6 +3040,12 @@ func (s *KeeperTestSuite) TestPrepareClaimAllIncentivesForPosition() {
 						s.Require().NoError(err)
 
 						outstandingRewards := accum.GetTotalRewards(uptimeAccum, position)
+
+						// Scale down outstanding rewards
+						for _, reward := range outstandingRewards {
+							reward.Amount = reward.Amount.QuoTruncateMut(cl.PerUnitLiqScalingFactor)
+						}
+
 						collectedIncentivesForUptime, _ := outstandingRewards.TruncateDecimal()
 
 						for _, coin := range collectedIncentivesForUptime {
@@ -3612,11 +3636,26 @@ func (s *KeeperTestSuite) TestIncentiveTruncation() {
 	s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(time.Minute * 50))
 	incentives, _, err := s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, s.TestAccs[0], positionData.ID)
 	s.Require().NoError(err)
-	s.Require().True(incentives.IsZero())
+	s.Require().False(incentives.IsZero())
 
 	// Incentives should now be claimed due to lack of truncation
 	s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(time.Minute * 51))
 	incentives, _, err = s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, s.TestAccs[0], positionData.ID)
 	s.Require().NoError(err)
 	s.Require().False(incentives.IsZero())
 }
+
+// scaleUptimeAccumulators scales the uptime accumulators by the scaling factor.
+// This is to avoid truncation to zero in core logic when the liquidity is large.
+func (s *KeeperTestSuite) scaleUptimeAccumulators(uptimeAccumulatorsToScale []sdk.DecCoins) []sdk.DecCoins {
+	growthCopy := make([]sdk.DecCoins, len(uptimeAccumulatorsToScale))
+	for i, growth := range uptimeAccumulatorsToScale {
+		growthCopy[i] = make(sdk.DecCoins, len(growth))
+		for j, coin := range growth {
+			growthCopy[i][j].Denom = coin.Denom
+			growthCopy[i][j].Amount = coin.Amount.MulTruncate(cl.PerUnitLiqScalingFactor)
+		}
+	}
+
+	return growthCopy
+}