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

test/feat: incentive accumulator truncation #7395

Merged
merged 2 commits into from
Feb 1, 2024
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
2 changes: 1 addition & 1 deletion x/concentrated-liquidity/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (k Keeper) CreateUptimeAccumulators(ctx sdk.Context, poolId uint64) error {
}

func CalcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, qualifyingLiquidity osmomath.Dec, timeElapsed osmomath.Dec, poolIncentiveRecords []types.IncentiveRecord) (sdk.DecCoins, []types.IncentiveRecord, error) {
return calcAccruedIncentivesForAccum(ctx, accumUptime, qualifyingLiquidity, timeElapsed, poolIncentiveRecords)
return calcAccruedIncentivesForAccum(ctx, accumUptime, qualifyingLiquidity, timeElapsed, poolIncentiveRecords, 0)
}

func (k Keeper) UpdateGivenPoolUptimeAccumulatorsToNow(ctx sdk.Context, pool types.ConcentratedPoolExtension, uptimeAccums []*accum.AccumulatorObject) error {
Expand Down
20 changes: 18 additions & 2 deletions x/concentrated-liquidity/incentives.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

sdkprefix "github.com/cosmos/cosmos-sdk/store/prefix"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/query"
"golang.org/x/exp/slices"
Expand Down Expand Up @@ -189,7 +190,7 @@ func (k Keeper) updateGivenPoolUptimeAccumulatorsToNow(ctx sdk.Context, pool typ
for uptimeIndex := range uptimeAccums {
// Get relevant uptime-level values
curUptimeDuration := types.SupportedUptimes[uptimeIndex]
incentivesToAddToCurAccum, updatedPoolRecords, err := calcAccruedIncentivesForAccum(ctx, curUptimeDuration, qualifyingLiquidity, timeElapsedSec, poolIncentiveRecords)
incentivesToAddToCurAccum, updatedPoolRecords, err := calcAccruedIncentivesForAccum(ctx, curUptimeDuration, qualifyingLiquidity, timeElapsedSec, poolIncentiveRecords, poolId)
if err != nil {
return err
}
Expand Down Expand Up @@ -222,7 +223,7 @@ func (k Keeper) updateGivenPoolUptimeAccumulatorsToNow(ctx sdk.Context, pool typ
// Returns the IncentivesPerLiquidity value and an updated list of IncentiveRecords that
// reflect emitted incentives
// Returns error if the qualifying liquidity/time elapsed are zero.
func calcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, liquidityInAccum osmomath.Dec, timeElapsed osmomath.Dec, poolIncentiveRecords []types.IncentiveRecord) (sdk.DecCoins, []types.IncentiveRecord, error) {
func calcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, liquidityInAccum osmomath.Dec, timeElapsed osmomath.Dec, poolIncentiveRecords []types.IncentiveRecord, poolID uint64) (sdk.DecCoins, []types.IncentiveRecord, error) {
if !liquidityInAccum.IsPositive() || !timeElapsed.IsPositive() {
return sdk.DecCoins{}, []types.IncentiveRecord{}, types.QualifyingLiquidityOrTimeElapsedNotPositiveError{QualifyingLiquidity: liquidityInAccum, TimeElapsed: timeElapsed}
}
Expand All @@ -244,6 +245,21 @@ func calcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, l
// 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)

// If truncation occurs, we emit events to alert us of the issue.
if incentivesPerLiquidity.IsZero() && !totalEmittedAmount.IsZero() {
ctx.EventManager().EmitEvent(sdk.NewEvent(
types.IncentiveTruncationPlaceholderName,
sdk.NewAttribute("pool_id", strconv.FormatUint(poolID, 10)),
sdk.NewAttribute("total_liq", liquidityInAccum.String()),
sdk.NewAttribute("per_unit_liq", incentivesPerLiquidity.String()),
sdk.NewAttribute("total_amt", totalEmittedAmount.String()),
))

telemetry.IncrCounter(1, types.IncentiveTruncationPlaceholderName)
ctx.Logger().Error(types.IncentiveTruncationPlaceholderName, "pool_id", poolID, "total_liq", liquidityInAccum, "per_unit_liq", incentivesPerLiquidity, "total_amt", totalEmittedAmount)
Comment on lines +251 to +260
Copy link
Member Author

Choose a reason for hiding this comment

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

We should create an alert for these and a Grafana dashboard cc: @niccoloraspa @alessandrolomanto

}

emittedIncentivesPerLiquidity := sdk.NewDecCoinFromDec(incentiveRecordBody.RemainingCoin.Denom, incentivesPerLiquidity)

// Ensure that we only emit if there are enough incentives remaining to be emitted
Expand Down
89 changes: 89 additions & 0 deletions x/concentrated-liquidity/incentives_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package concentrated_liquidity_test

import (
"errors"
"fmt"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/osmoutils/accum"
cl "github.com/osmosis-labs/osmosis/v22/x/concentrated-liquidity"
"github.com/osmosis-labs/osmosis/v22/x/concentrated-liquidity/math"
"github.com/osmosis-labs/osmosis/v22/x/concentrated-liquidity/model"
"github.com/osmosis-labs/osmosis/v22/x/concentrated-liquidity/types"
"github.com/osmosis-labs/osmosis/v22/x/gamm/pool-models/balancer"
Expand Down Expand Up @@ -3548,3 +3550,90 @@ func (s *KeeperTestSuite) TestGetIncentiveRecordSerialized() {
})
}
}

// This PR shows that there is a chance of incentives being truncated due to large liquidity value.
// We observed this in pool 1423 where both tokens have 18 decimal precision.
//
// It has been determined that no funds are at risk. The incentives are eventually distributed if either:
// a) Long time without an update to the pool state occurs (at least 51 minute with the current configuration)
// b) current tick liquidity becomes smaller
func (s *KeeperTestSuite) TestIncentiveTruncation() {
s.SetupTest()

// Create a pool
pool := s.PrepareConcentratedPool()

// osmosisd q concentratedliquidity incentive-records 1423 --node https://osmosis-rpc.polkachu.com:443
// incentive_records:
// - incentive_id: "5833"
// incentive_record_body:
// emission_rate: "9645.061724537037037037"
// remaining_coin:
// amount: "518549443.513510006462246574"
// denom: ibc/A8CA5EE328FA10C9519DF6057DA1F69682D28F7D0F5CCC7ECB72E3DCA2D157A4
// start_time: "2024-01-31T17:16:11.187417702Z"
// min_uptime: 0.000000001s
// pool_id: "1423"
// pagination:
// next_key: null
// total: "0"
// 24 * 60 * 60 * 9645.061724537037037037
// 833333333.0 -<------ Initial incentives in recorrd
incentiveCoin := sdk.NewCoin("ibc/A8CA5EE328FA10C9519DF6057DA1F69682D28F7D0F5CCC7ECB72E3DCA2D157A4", sdk.NewInt(833333333))

// Create a pool state simulating pool 1423. The only difference is that we force the pool state given 1 position as
// opposed to many.
// osmosisd q poolmanager pool 1423 --height 13559864 --node https://osmosis-rpc.polkachu.com:443
desiredLiquidity := osmomath.MustNewBigDecFromStr("28968940108516957474488782.253893404842148631")
desiredCurrentTick := int64(596)
desiredCurrentSqrtPrice, err := math.TickToSqrtPrice(desiredCurrentTick)
s.Require().NoError(err)

amount0 := math.CalcAmount0Delta(desiredLiquidity, desiredCurrentSqrtPrice, types.MaxSqrtPriceBigDec, true).Dec().TruncateInt()
amount1 := math.CalcAmount1Delta(desiredLiquidity, types.MinSqrtPriceBigDec, desiredCurrentSqrtPrice, true).Dec().TruncateInt()

lpCoins := sdk.NewCoins(sdk.NewCoin(ETH, amount0), sdk.NewCoin(USDC, amount1))
s.FundAcc(s.TestAccs[0], lpCoins)

// LP
positionData, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, pool.GetId(), s.TestAccs[0], lpCoins, osmomath.ZeroInt(), osmomath.ZeroInt(), types.MinInitializedTick, types.MaxTick)
s.Require().NoError(err)

fmt.Println("initial liquidity", positionData.Liquidity)

// Fund the account with the incentive coin
s.FundAcc(s.TestAccs[0], sdk.NewCoins(incentiveCoin))

// Set incentives for pool to ensure accumulators work correctly
_, err = s.App.ConcentratedLiquidityKeeper.CreateIncentive(s.Ctx, pool.GetId(), s.TestAccs[0], incentiveCoin, osmomath.MustNewDecFromStr("9645.061724537037037037"), s.Ctx.BlockTime(), time.Nanosecond)
s.Require().NoError(err)

// Reset events
s.Ctx = s.Ctx.WithEventManager(sdk.NewEventManager())

// The check below shows that the incentive is not claimed due to truncation
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())

// Truncation happens
events := s.Ctx.EventManager().Events()
s.Require().Equal(1, len(events))
s.Require().Equal(types.IncentiveTruncationPlaceholderName, events[0].Type)

// Reset events
s.Ctx = s.Ctx.WithEventManager(sdk.NewEventManager())

s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(time.Minute * 51))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we only need to add 1 minute here? Since we already went ahead 50 minutes above?

incentives, _, err = s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, s.TestAccs[0], positionData.ID)
s.Require().NoError(err)
s.Require().False(incentives.IsZero())

// Truncation event does not occur
events = s.Ctx.EventManager().Events()
s.Require().NotEqual(0, len(events))
for _, event := range events {
s.Require().NotEqual(types.IncentiveTruncationPlaceholderName, event.Type)
}
}
2 changes: 2 additions & 0 deletions x/concentrated-liquidity/types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,6 @@ const (
AttributeKeySpreadRewardGrowthOppositeDirectionOfLastTraversal = "spread_reward_growth"
AttributeKeyUptimeGrowthOppositeDirectionOfLastTraversal = "uptime_growth"
AttributeNewOwner = "new_owner"

IncentiveTruncationPlaceholderName = "concentrated_liquidity_incentive_truncation"
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove placeholder name?

)