diff --git a/CHANGELOG.md b/CHANGELOG.md index 19fbd5b858b..e6728bc81f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### State Breaking * [#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 ### Bug Fixes diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index 0c4eeb81ab9..80df51a630e 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -74,6 +74,7 @@ var ( } defaultZeroWeightGaugeRecord = types.InternalGaugeRecord{GaugeId: 1, CurrentWeight: osmomath.ZeroInt(), CumulativeWeight: osmomath.ZeroInt()} + defaultNoLockDuration = time.Nanosecond ) type GroupCreationFields struct { @@ -1139,6 +1140,7 @@ func (s *KeeperTestSuite) CreateNoLockExternalGauges(clPoolId uint64, externalGa clPoolExternalGaugeId, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, numEpochsPaidOver == 1, gaugeCreator, externalGaugeCoins, lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, }, s.Ctx.BlockTime(), numEpochsPaidOver, diff --git a/x/incentives/keeper/gauge.go b/x/incentives/keeper/gauge.go index 872b0572811..893c2fa5708 100644 --- a/x/incentives/keeper/gauge.go +++ b/x/incentives/keeper/gauge.go @@ -127,9 +127,42 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr return 0, types.ErrZeroNumEpochsPaidOver } - // Ensure that this gauge's duration is one of the allowed durations on chain - durations := k.GetLockableDurations(ctx) - if distrTo.LockQueryType == lockuptypes.ByDuration { + // If the gauge has no lock, then we currently assume it is a concentrated pool + // and ensure the gauge "lock" duration is an authorized uptime. + isNoLockGauge := distrTo.LockQueryType == lockuptypes.NoLock + + // If the gauge has an internal gauge denom, it is an internal gauge + // and should be run through different validation logic (see below). + // + // Two important reminders/assumptions: + // 1. `NoLock` gauges are required to have empty denoms in `ValidateBasic`, so this + // check cannot be controlled by user input. + // 2. The safety of this leans on the special-casing of internal gauge logic during + // distributions, which should be using the internal incentive duration gov param instead of the duration value. + isInternalNoLockGauge := isNoLockGauge && distrTo.Denom == types.NoLockInternalGaugeDenom(poolId) + isExternalNoLockGauge := isNoLockGauge && !isInternalNoLockGauge + + // We check durations if the gauge is a regular duration based gauge or if it is a + // CL gauge. Note that this excludes time-based gauges and group gauges. + if isNoLockGauge || distrTo.LockQueryType == lockuptypes.ByDuration { + // Ensure that this gauge's duration is one of the allowed durations on chain + // Concentrated pool gauges check against authorized uptimes (if external) or + // epoch duration (if internal). + // + // All other gauges check against the default set of lockable durations. + var durations []time.Duration + if isExternalNoLockGauge { + durations = k.clk.GetParams(ctx).AuthorizedUptimes + } else if isInternalNoLockGauge { + // Internal CL gauges use epoch time as their duration. This is a legacy + // property that does not affect the uptime on created records, which is + // determined by the gov param for internal incentive uptimes. + durations = []time.Duration{k.GetEpochInfo(ctx).Duration} + } else { + // This branch is applicable to CFMM pool types such as balancer and stableswap. + durations = k.GetLockableDurations(ctx) + } + durationOk := false for _, duration := range durations { if duration == distrTo.Duration { @@ -146,7 +179,7 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr // For no lock gauges, a pool id must be set. // A pool with such id must exist and be a concentrated pool. - if distrTo.LockQueryType == lockuptypes.NoLock { + if isNoLockGauge { if poolId == 0 { return 0, fmt.Errorf("'no lock' type gauges must have a pool id") } @@ -155,7 +188,7 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr // and get overwritten with the external prefix + pool id // for internal query purposes. distrToDenom := distrTo.Denom - if distrToDenom != types.NoLockInternalGaugeDenom(poolId) { + if !isInternalNoLockGauge { // If denom is set, then fails. if distrToDenom != "" { return 0, fmt.Errorf("'no lock' type external gauges must have an empty denom set, was %s", distrToDenom) @@ -177,7 +210,7 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr // That being said, internal gauges have an additional linking // by duration where duration is the incentives epoch duration. // The internal incentive linking is set in x/pool-incentives CreateConcentratedLiquidityPoolGauge. - k.pik.SetPoolGaugeIdNoLock(ctx, poolId, nextGaugeId) + k.pik.SetPoolGaugeIdNoLock(ctx, poolId, nextGaugeId, distrTo.Duration) } else { // For all other gauges, pool id must be 0. if poolId != 0 { diff --git a/x/incentives/keeper/gauge_test.go b/x/incentives/keeper/gauge_test.go index 5cc545b74a9..1ec234fc5b7 100644 --- a/x/incentives/keeper/gauge_test.go +++ b/x/incentives/keeper/gauge_test.go @@ -507,7 +507,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { distrTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, // Note: this assumes the gauge is external - Denom: "", + Denom: "", + Duration: time.Nanosecond, }, poolId: concentratedPoolId, @@ -520,7 +521,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { distrTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, // Note: this assumes the gauge is internal - Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), + Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), + Duration: s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration, }, poolId: concentratedPoolId, @@ -533,7 +535,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { distrTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, // Note: this is invalid for NoLock gauges - Denom: "uosmo", + Denom: "uosmo", + Duration: time.Nanosecond, }, poolId: concentratedPoolId, @@ -543,6 +546,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { name: "fail to create no lock gauge with balancer pool", distrTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, }, poolId: balancerPoolId, @@ -552,6 +556,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { name: "fail to create no lock gauge with non-existent pool", distrTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, }, poolId: invalidPool, @@ -561,9 +566,46 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { name: "fail to create no lock gauge with zero pool id", distrTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, }, poolId: zeroPoolId, + expectErr: true, + }, + { + name: "fail to create external no lock gauge due to unauthorized uptime", + distrTo: lockuptypes.QueryCondition{ + LockQueryType: lockuptypes.NoLock, + // Note: this assumes the gauge is external + Denom: "", + // 1h is a supported uptime that is not authorized + Duration: time.Hour, + }, + poolId: concentratedPoolId, + expectErr: true, + }, + { + name: "fail to create external no lock gauge due to entirely invalid uptime", + distrTo: lockuptypes.QueryCondition{ + LockQueryType: lockuptypes.NoLock, + // Note: this assumes the gauge is external + Denom: "", + // 2ns is an uptime that isn't supported at all (i.e. can't even be authorized) + Duration: 2 * time.Nanosecond, + }, + poolId: concentratedPoolId, + expectErr: true, + }, + { + name: "fail to create an internal gauge with an unexpected duration", + distrTo: lockuptypes.QueryCondition{ + LockQueryType: lockuptypes.NoLock, + // Note: this assumes the gauge is internal + Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), + Duration: time.Nanosecond, + }, + poolId: concentratedPoolId, + expectErr: true, }, } @@ -589,10 +631,6 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { s.Require().Equal(tc.expectedGaugeId, gaugeId) - // Assert that pool id and gauge id link meant for internally incentivized gauges is unset. - _, err := s.App.PoolIncentivesKeeper.GetPoolGaugeId(s.Ctx, tc.poolId, tc.distrTo.Duration) - s.Require().Error(err) - // Confirm that the general pool id to gauge id link is set. gaugeIds, err := s.App.PoolIncentivesKeeper.GetNoLockGaugeIdsFromPool(s.Ctx, tc.poolId) s.Require().NoError(err) @@ -774,7 +812,7 @@ func (s *KeeperTestSuite) createGaugeNoRestrictions(isPerpetual bool, coins sdk. } if poolID != 0 { - s.App.PoolIncentivesKeeper.SetPoolGaugeIdNoLock(s.Ctx, poolID, nextGaugeID) + s.App.PoolIncentivesKeeper.SetPoolGaugeIdNoLock(s.Ctx, poolID, nextGaugeID, distrTo.Duration) } err := s.App.IncentivesKeeper.SetGauge(s.Ctx, &gauge) diff --git a/x/incentives/keeper/genesis_test.go b/x/incentives/keeper/genesis_test.go index 1809e0c0d8a..ea642ab049b 100644 --- a/x/incentives/keeper/genesis_test.go +++ b/x/incentives/keeper/genesis_test.go @@ -27,6 +27,7 @@ var ( distrToNoLock = lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, } distrToNoLockPool1 = lockuptypes.QueryCondition{ diff --git a/x/incentives/types/expected_keepers.go b/x/incentives/types/expected_keepers.go index 05d8e44ddf3..16487b87ed4 100644 --- a/x/incentives/types/expected_keepers.go +++ b/x/incentives/types/expected_keepers.go @@ -50,6 +50,7 @@ type TxFeesKeeper interface { type ConcentratedLiquidityKeeper interface { CreateIncentive(ctx sdk.Context, poolId uint64, sender sdk.AccAddress, incentiveCoin sdk.Coin, emissionRate osmomath.Dec, startTime time.Time, minUptime time.Duration) (cltypes.IncentiveRecord, error) GetConcentratedPoolById(ctx sdk.Context, poolId uint64) (cltypes.ConcentratedPoolExtension, error) + GetParams(ctx sdk.Context) (params cltypes.Params) } type AccountKeeper interface { @@ -59,7 +60,7 @@ type AccountKeeper interface { type PoolIncentiveKeeper interface { GetPoolIdFromGaugeId(ctx sdk.Context, gaugeId uint64, lockableDuration time.Duration) (uint64, error) GetInternalGaugeIDForPool(ctx sdk.Context, poolID uint64) (uint64, error) - SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64) + SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64, uptime time.Duration) GetLongestLockableDuration(ctx sdk.Context) (time.Duration, error) } diff --git a/x/pool-incentives/keeper/genesis.go b/x/pool-incentives/keeper/genesis.go index 82925b82976..43bfdd73d56 100644 --- a/x/pool-incentives/keeper/genesis.go +++ b/x/pool-incentives/keeper/genesis.go @@ -28,7 +28,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { } if genState.ConcentratedPoolToNoLockGauges != nil { for _, record := range genState.ConcentratedPoolToNoLockGauges.PoolToGauge { - k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId) + k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId, record.Duration) } } } diff --git a/x/pool-incentives/keeper/genesis_test.go b/x/pool-incentives/keeper/genesis_test.go index 29e33786465..9ad88e4d9cc 100644 --- a/x/pool-incentives/keeper/genesis_test.go +++ b/x/pool-incentives/keeper/genesis_test.go @@ -151,6 +151,7 @@ func (s *KeeperTestSuite) TestImportExportGenesis_ExternalNoLock() { // Create external non-perpetual gauge externalGaugeID, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, false, s.TestAccs[0], defaultCoins.Add(defaultCoins...), lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, }, s.Ctx.BlockTime(), 2, clPool.GetId()) s.Require().NoError(err) diff --git a/x/pool-incentives/keeper/grpc_query_test.go b/x/pool-incentives/keeper/grpc_query_test.go index 0e956fe200c..86415c6f737 100644 --- a/x/pool-incentives/keeper/grpc_query_test.go +++ b/x/pool-incentives/keeper/grpc_query_test.go @@ -16,6 +16,8 @@ import ( var ( isPerpetual = true notPerpetual = false + + defaultNoLockDuration = time.Nanosecond ) func (s *KeeperTestSuite) TestGaugeIds() { @@ -443,6 +445,7 @@ func (s *KeeperTestSuite) TestExternalIncentiveGauges_NoLock() { defaultNoLockGaugeConfig = gaugeConfig{ distributeTo: lockuptypes.QueryCondition{ LockQueryType: lockuptypes.NoLock, + Duration: defaultNoLockDuration, }, poolId: concentratedPoolId, } diff --git a/x/pool-incentives/keeper/keeper.go b/x/pool-incentives/keeper/keeper.go index fd2edbfec45..395fa96c421 100644 --- a/x/pool-incentives/keeper/keeper.go +++ b/x/pool-incentives/keeper/keeper.go @@ -160,7 +160,7 @@ func (k Keeper) SetPoolGaugeIdInternalIncentive(ctx sdk.Context, poolId uint64, // SetPoolGaugeIdNoLock sets the link between pool id and gauge id for "NoLock" gauges. // CONTRACT: the gauge of the given id must be "NoLock" gauge. -func (k Keeper) SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64) { +func (k Keeper) SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uint64, incentivizedUptime time.Duration) { store := ctx.KVStore(k.storeKey) // maps pool id and gauge id to gauge id. // Note: this could be pool id and gauge id to empty byte array, @@ -170,10 +170,7 @@ func (k Keeper) SetPoolGaugeIdNoLock(ctx sdk.Context, poolId uint64, gaugeId uin store.Set(key, sdk.Uint64ToBigEndian(gaugeId)) // Note: this index is used for general linking. - // We supply zero for incentivized duration as "NoLock" gauges are not - // associated with any lockable duration. Instead, they incentivize - // pools directly. - key = types.GetPoolIdFromGaugeIdStoreKey(gaugeId, 0) + key = types.GetPoolIdFromGaugeIdStoreKey(gaugeId, incentivizedUptime) store.Set(key, sdk.Uint64ToBigEndian(poolId)) }