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

feat(x/incentives): min value param for epoch distribution #7615

Merged
merged 12 commits into from
Mar 14, 2024
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#7555](https://github.com/osmosis-labs/osmosis/pull/7555) Refactor taker fees, distribute via a single module account, track once at epoch
* [#7562](https://github.com/osmosis-labs/osmosis/pull/7562) Speedup Protorev estimation logic by removing unnecessary taker fee simulations.
* [#7595](https://github.com/osmosis-labs/osmosis/pull/7595) Fix cosmwasm pool model code ID migration.
* [#7615](https://github.com/osmosis-labs/osmosis/pull/7615) Min value param for epoch distribution.
* [#7619](https://github.com/osmosis-labs/osmosis/pull/7619) Slight speedup/gas improvement to CL GetTotalPoolLiquidity queries
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Remove duplicate CL accumulator update logic.


### State Compatible

* [#7590](https://github.com/osmosis-labs/osmosis/pull/7590) fix cwpool migration prop disallowing only one of code id or bytecode.
Expand Down
1 change: 1 addition & 0 deletions app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
appKeepers.ConcentratedLiquidityKeeper,
appKeepers.PoolManagerKeeper,
appKeepers.PoolIncentivesKeeper,
appKeepers.ProtoRevKeeper,
)
appKeepers.ConcentratedLiquidityKeeper.SetIncentivesKeeper(appKeepers.IncentivesKeeper)
appKeepers.GAMMKeeper.SetIncentivesKeeper(appKeepers.IncentivesKeeper)
Expand Down
6 changes: 6 additions & 0 deletions app/upgrades/v24/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (

"github.com/osmosis-labs/osmosis/v23/app/keepers"
"github.com/osmosis-labs/osmosis/v23/app/upgrades"

incentivestypes "github.com/osmosis-labs/osmosis/v23/x/incentives/types"
)

func CreateUpgradeHandler(
Expand Down Expand Up @@ -39,6 +41,10 @@ func CreateUpgradeHandler(
// since we only need the pool indexed TWAPs.
keepers.TwapKeeper.DeleteAllHistoricalTimeIndexedTWAPs(ctx)

// Set the new min value for distribution for the incentives module.
// https://www.mintscan.io/osmosis/proposals/733
keepers.IncentivesKeeper.SetParam(ctx, incentivestypes.KeyMinValueForDistr, incentivestypes.DefaultMinValueForDistr)

return migrations, nil
}
}
8 changes: 8 additions & 0 deletions app/upgrades/v24/upgrades_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/osmosis-labs/osmosis/osmoutils"
"github.com/osmosis-labs/osmosis/v23/app/apptesting"

incentivestypes "github.com/osmosis-labs/osmosis/v23/x/incentives/types"
protorevtypes "github.com/osmosis-labs/osmosis/v23/x/protorev/types"
"github.com/osmosis-labs/osmosis/v23/x/twap/types"
twaptypes "github.com/osmosis-labs/osmosis/v23/x/twap/types"
Expand Down Expand Up @@ -131,6 +132,13 @@ func (s *UpgradeTestSuite) TestUpgrade() {
oldBaseDenoms, err = s.App.ProtoRevKeeper.DeprecatedGetAllBaseDenoms(s.Ctx)
s.Require().NoError(err)
s.Require().Empty(oldBaseDenoms)

// INCENTIVES Tests
//

// Check that the new min value for distribution has been set
params := s.App.IncentivesKeeper.GetParams(s.Ctx)
s.Require().Equal(incentivestypes.DefaultMinValueForDistr, params.MinValueForDistribution)
}

func dummyUpgrade(s *UpgradeTestSuite) {
Expand Down
7 changes: 7 additions & 0 deletions proto/osmosis/incentives/params.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,11 @@ message Params {
(gogoproto.stdduration) = true,
(gogoproto.moretags) = "yaml:\"internal_uptime\""
];
// min_value_for_distribution is the minimum amount a token must be worth
// in order to be eligible for distribution. If the token is worth
// less than this amount (or the route between the two denoms is not
// registered), it will not be distributed and is forfeited to the remaining
// distributees that are eligible.
mattverse marked this conversation as resolved.
Show resolved Hide resolved
cosmos.base.v1beta1.Coin min_value_for_distribution = 5
[ (gogoproto.nullable) = false ];
}
3 changes: 3 additions & 0 deletions x/cosmwasmpool/cosmwasm/msg/transmuter/transmuter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ func (s *TransmuterSuite) TestFunctionalTransmuter() {
expectedDenomSuffix = "/transmuter/poolshare"
)

// Set base denom
s.App.IncentivesKeeper.SetParam(s.Ctx, incentivetypes.KeyMinValueForDistr, sdk.NewCoin("uosmo", osmomath.NewInt(10000)))
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

The asset being distributed via gauges either needs to be the denom set in the param store OR have a pool linking the denom to the denom set in the param store. So the option here is to either set the param to the denom in the test or create a pool linking the denom in the test to the denom in the param store.


// Create Transmuter pool
transmuter := s.PrepareCosmWasmPool()

Expand Down
79 changes: 73 additions & 6 deletions x/incentives/keeper/distribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@ import (

var (
millisecondsInSecDec = osmomath.NewDec(1000)
zeroInt = osmomath.ZeroInt()
)

// DistributionValueCache is a cache for when we calculate the minimum value
// an underlying token must be to be distributed.
type DistributionValueCache struct {
denomToMinValueMap map[string]osmomath.Int
}

// AllocateAcrossGauges for every gauge in the input, it updates the weights according to the splitting
// policy and allocates the coins to the underlying gauges per the updated weights.
// Note, every group is associated with a group gauge. The distribution to regular gauges
Expand Down Expand Up @@ -388,7 +395,7 @@ func (k Keeper) doDistributionSends(ctx sdk.Context, distrs *distributionInfo) e
// the distrInfo struct. It also updates the gauge for the distribution.
// locks is expected to be the correct set of lock recipients for this gauge.
func (k Keeper) distributeSyntheticInternal(
ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo,
ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo, minDistrValueCache *DistributionValueCache,
) (sdk.Coins, error) {
qualifiedLocks := k.lk.GetLocksLongerThanDurationDenom(ctx, gauge.DistributeTo.Denom, gauge.DistributeTo.Duration)

Expand Down Expand Up @@ -425,7 +432,7 @@ func (k Keeper) distributeSyntheticInternal(
sortedAndTrimmedQualifiedLocks[v.index] = &v.lock
}

return k.distributeInternal(ctx, gauge, sortedAndTrimmedQualifiedLocks, distrInfo)
return k.distributeInternal(ctx, gauge, sortedAndTrimmedQualifiedLocks, distrInfo, minDistrValueCache)
}

// syncGroupWeights updates the individual and total weights of the group records based on the splitting policy.
Expand Down Expand Up @@ -454,7 +461,7 @@ func (k Keeper) syncGroupWeights(ctx sdk.Context, group types.Group) error {
// calculateGroupWeights calculates the updated weights of the group records based on the pool volumes.
// It returns the updated group and an error if any. It does not mutate the passed in object.
func (k Keeper) calculateGroupWeights(ctx sdk.Context, group types.Group) (types.Group, error) {
totalWeight := sdk.ZeroInt()
totalWeight := zeroInt

// We operate on a deep copy of the given group because we expect to handle specific errors quietly
// and want to avoid the scenario where the original group gauge is partially mutated in such cases.
Expand Down Expand Up @@ -611,10 +618,14 @@ func (k Keeper) getNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge, poolId
//
// CONTRACT: gauge passed in as argument must be an active gauge.
func (k Keeper) distributeInternal(
ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo,
ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo, minDistrValueCache *DistributionValueCache,
) (sdk.Coins, error) {
totalDistrCoins := sdk.NewCoins()

// Retrieve the min value for distribution.
// If any distribution amount is valued less than what the param is set, it will be skipped.
minValueForDistr := k.GetParams(ctx).MinValueForDistribution

remainCoins := gauge.Coins.Sub(gauge.DistributedCoins...)

// if its a perpetual gauge, we set remaining epochs to 1.
Expand Down Expand Up @@ -722,6 +733,54 @@ func (k Keeper) distributeInternal(
// which is bounded to an Int. So we can safely skip this.

amtIntBi.Quo(amtIntBi, lockSumTimesRemainingEpochsBi)

// Determine if the value to distribute is worth enough in minValueForDistr denom to be distributed.
if coin.Denom == minValueForDistr.Denom {
// If the denom is the same as the minValueForDistr param, no transformation is needed.
if amtInt.LT(minValueForDistr.Amount) {
continue
}
} else {
// If the denom is not the minValueForDistr denom, we need to transform the underlying to it.
// Check if the denom exists in the cached values
value, ok := minDistrValueCache.denomToMinValueMap[coin.Denom]
if !ok {
// Cache miss, figure out the value and add it to the cache
poolId, err := k.prk.GetPoolForDenomPairNoOrder(ctx, minValueForDistr.Denom, coin.Denom)
if err != nil {
// If the pool denom pair pool route does not exist in protorev, we add a zero value to cache to avoid
// querying the pool again.
minDistrValueCache.denomToMinValueMap[coin.Denom] = zeroInt
continue
}
swapModule, pool, err := k.pmk.GetPoolModuleAndPool(ctx, poolId)
if err != nil {
return nil, err
}

minTokenRequiredForDistr, err := swapModule.CalcOutAmtGivenIn(ctx, pool, minValueForDistr, coin.Denom, sdk.ZeroDec())
if err != nil {
return nil, err
}

// Add min token required for distribution to the cache
minDistrValueCache.denomToMinValueMap[coin.Denom] = minTokenRequiredForDistr.Amount

// Check if the value is worth enough in the token to be distributed.
if amtInt.LT(minTokenRequiredForDistr.Amount) {
// The value is not worth enough, continue
continue
}
} else {
// Cache hit, use the value

// Check if the underlying is worth enough in the token to be distributed.
if amtInt.LT(value) || value.IsZero() {
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved
continue
}
}
}

if amtInt.Sign() == 1 {
newlyDistributedCoin := sdk.Coin{Denom: coin.Denom, Amount: amtInt}
distrCoins = distrCoins.Add(newlyDistributedCoin)
Expand Down Expand Up @@ -858,21 +917,29 @@ func (k Keeper) Distribute(ctx sdk.Context, gauges []types.Gauge) (sdk.Coins, er
totalDistributedCoins := sdk.NewCoins()
scratchSlice := make([]*lockuptypes.PeriodLock, 0, 50000)

// Instead of re-fetching the minimum value an underlying token must be to meet the minimum
// requirement for distribution, we cache the values here.
// While this isn't precise as it doesn't account for price impact, it is good enough for the sole
// purpose of determining if we should distribute the token or not.
minDistrValueCache := &DistributionValueCache{
denomToMinValueMap: make(map[string]osmomath.Int),
}

for _, gauge := range gauges {
var gaugeDistributedCoins sdk.Coins
filteredLocks := k.getDistributeToBaseLocks(ctx, gauge, locksByDenomCache, &scratchSlice)
// send based on synthetic lockup coins if it's distributing to synthetic lockups
var err error
if lockuptypes.IsSyntheticDenom(gauge.DistributeTo.Denom) {
ctx.Logger().Debug("distributeSyntheticInternal, gauge id %d, %d", "module", types.ModuleName, "gaugeId", gauge.Id, "height", ctx.BlockHeight())
gaugeDistributedCoins, err = k.distributeSyntheticInternal(ctx, gauge, filteredLocks, &distrInfo)
gaugeDistributedCoins, err = k.distributeSyntheticInternal(ctx, gauge, filteredLocks, &distrInfo, minDistrValueCache)
} else {
// Do not distribute if LockQueryType = Group, because if we distribute here we will be double distributing.
if gauge.DistributeTo.LockQueryType == lockuptypes.ByGroup {
continue
}

gaugeDistributedCoins, err = k.distributeInternal(ctx, gauge, filteredLocks, &distrInfo)
gaugeDistributedCoins, err = k.distributeInternal(ctx, gauge, filteredLocks, &distrInfo, minDistrValueCache)
}
if err != nil {
return nil, err
Expand Down
Loading