fix: check denom routes at time of gauge creation (#8136)

* check denoms for gauge creation

* changelog

* extract logic

* initial push of test fixes

* tidy

* add comments

* add test cases

* Update CHANGELOG.md

* chore: don't use hardcoded "uosmo" string (#8141)

* use uosmo var

* fix lint

* tidy

* Revert "chore: don't use hardcoded "uosmo" string (#8141)"

This reverts commit c25e4e0.

* Update CHANGELOG.md

* Update CHANGELOG.md

CHANGELOG.md

@@ -45,11 +45,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### State Breaking
 
 * [#7005](https://github.com/osmosis-labs/osmosis/pull/7005) Adding deactivated smart account module.
+* [#8106](https://github.com/osmosis-labs/osmosis/pull/8106) Enable ProtoRev distro on epoch
 * [#8053](https://github.com/osmosis-labs/osmosis/pull/8053) Reset validator signing info missed blocks counter
-* [#8106](https://github.com/osmosis-labs/osmosis/pull/8106) Enable ProtoRev distro on epoch.
-* [#8030](https://github.com/osmosis-labs/osmosis/pull/8030) Delete legacy behavior where lockups could not unbond at very small block heights on a testnet.
 * [#8073](https://github.com/osmosis-labs/osmosis/pull/8073) Speedup CL spread factor calculations, but mildly changes rounding behavior in the final decimal place.
-* [#8125](https://github.com/osmosis-labs/osmosis/pull/8125) When using smart accounts, fees are deducted directly after the feePayer is authenticated. Regardless of the authentication of other signers.
+* [#8125](https://github.com/osmosis-labs/osmosis/pull/8125) When using smart accounts, fees are deducted directly after the feePayer is authenticated. Regardless of the authentication of other signers
+* [#8136](https://github.com/osmosis-labs/osmosis/pull/8136) Don't allow gauge creation/addition with rewards that have no protorev route (i.e. no way to determine if rewards meet minimum epoch value distribution requirements)
 * [#8144](https://github.com/osmosis-labs/osmosis/pull/8144) IBC wasm clients can now make stargate queries and support abort.
 
 ### State Compatible

x/incentives/keeper/distribute_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/osmosis-labs/osmosis/osmomath"
 	"github.com/osmosis-labs/osmosis/osmoutils/coinutil"
 	appParams "github.com/osmosis-labs/osmosis/v24/app/params"
+	appparams "github.com/osmosis-labs/osmosis/v24/app/params"
 	"github.com/osmosis-labs/osmosis/v24/x/incentives/types"
 	incentivetypes "github.com/osmosis-labs/osmosis/v24/x/incentives/types"
 	lockuptypes "github.com/osmosis-labs/osmosis/v24/x/lockup/types"
@@ -1196,6 +1197,12 @@ func (s *KeeperTestSuite) TestFunctionalInternalExternalCLGauge() {
 		halfOfExternalGaugeCoins = sdk.NewCoins(sdk.NewCoin("eth", osmomath.NewInt(defaultExternalGaugeValue/numEpochsPaidOverGaugeTwo)), sdk.NewCoin("usdc", osmomath.NewInt(defaultExternalGaugeValue/numEpochsPaidOverGaugeTwo))) // distributed at each epoch for non-perp gauge with numEpoch = 2
 	)
 
+	// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+	// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+	for _, coin := range append(externalGaugeCoins, internalGaugeCoins...) {
+		s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+	}
+
 	s.FundAcc(s.TestAccs[1], requiredBalances)
 	s.FundAcc(s.TestAccs[2], requiredBalances)
 	s.FundModuleAcc(incentivetypes.ModuleName, requiredBalances)
@@ -1346,6 +1353,12 @@ func (s *KeeperTestSuite) TestFunctionalInternalExternalCLGauge() {
 }
 
 func (s *KeeperTestSuite) CreateNoLockExternalGauges(clPoolId uint64, externalGaugeCoins sdk.Coins, gaugeCreator sdk.AccAddress, numEpochsPaidOver uint64) uint64 {
+	// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+	// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+	for _, coin := range externalGaugeCoins {
+		s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+	}
+
 	// Create 1 external no-lock gauge perpetual over 1 epochs MsgCreateGauge
 	clPoolExternalGaugeId, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, numEpochsPaidOver == 1, gaugeCreator, externalGaugeCoins,
 		lockuptypes.QueryCondition{

x/incentives/keeper/export_test.go

@@ -106,3 +106,7 @@ func (k Keeper) GetNoLockGaugeUptime(ctx sdk.Context, gauge types.Gauge, poolId
 func (k Keeper) SkipSpamGaugeDistribute(ctx sdk.Context, locks []*lockuptypes.PeriodLock, gauge types.Gauge, totalDistrCoins sdk.Coins, remainCoins sdk.Coins) (bool, sdk.Coins, error) {
 	return k.skipSpamGaugeDistribute(ctx, locks, gauge, totalDistrCoins, remainCoins)
 }
+
+func (k Keeper) CheckIfDenomsAreDistributable(ctx sdk.Context, coins sdk.Coins) error {
+	return k.checkIfDenomsAreDistributable(ctx, coins)
+}

x/incentives/keeper/gauge.go

@@ -16,6 +16,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/osmosis-labs/osmosis/osmomath"
+	appparams "github.com/osmosis-labs/osmosis/v24/app/params"
 	"github.com/osmosis-labs/osmosis/v24/x/incentives/types"
 	lockuptypes "github.com/osmosis-labs/osmosis/v24/x/lockup/types"
 	poolmanagertypes "github.com/osmosis-labs/osmosis/v24/x/poolmanager/types"
@@ -127,6 +128,15 @@ func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddr
 		return 0, types.ErrZeroNumEpochsPaidOver
 	}
 
+	// Check that the coins being sent to the gauge exist as a skip hot route
+	// This is used to determine the underlying value of the rewards per user at epoch,
+	// since we don't distribute tokens values under a certain threshold.
+	// If the denom doesn't exist in the skip hot route, we would never distribute rewards
+	// from this gauge.
+	if err := k.checkIfDenomsAreDistributable(ctx, coins); err != nil {
+		return 0, err
+	}
+
 	// 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
@@ -413,6 +423,10 @@ func (k Keeper) AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins s
 // Notes: does not do token transfers since it is used internally for token transferring value within the
 // incentives module or by higher level functions that do transfer.
 func (k Keeper) addToGaugeRewards(ctx sdk.Context, coins sdk.Coins, gaugeID uint64) error {
+	if err := k.checkIfDenomsAreDistributable(ctx, coins); err != nil {
+		return err
+	}
+
 	gauge, err := k.GetGaugeByID(ctx, gaugeID)
 	if err != nil {
 		return err
@@ -457,3 +471,19 @@ func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sd
 	}
 	return nil
 }
+
+// checkIfDenomsAreDistributable checks if the denoms in the provided coins are registered in protorev.
+// It iterates over the coins and for each coin, it tries to get the pool for the denom pair with "uosmo".
+// If the pool does not exist, it returns an error indicating that the denom does not exist as a protorev hot route.
+// If all denoms are valid, it returns nil.
+func (k Keeper) checkIfDenomsAreDistributable(ctx sdk.Context, coins sdk.Coins) error {
+	for _, coin := range coins {
+		if coin.Denom != appparams.BaseCoinUnit {
+			_, err := k.prk.GetPoolForDenomPairNoOrder(ctx, coin.Denom, appparams.BaseCoinUnit)
+			if err != nil {
+				return types.NoRouteForDenomError{Denom: coin.Denom}
+			}
+		}
+	}
+	return nil
+}

x/incentives/keeper/gauge_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/osmosis-labs/osmosis/osmomath"
 	"github.com/osmosis-labs/osmosis/osmoutils"
+	appparams "github.com/osmosis-labs/osmosis/v24/app/params"
 	incentiveskeeper "github.com/osmosis-labs/osmosis/v24/x/incentives/keeper"
 	"github.com/osmosis-labs/osmosis/v24/x/incentives/types"
 	lockuptypes "github.com/osmosis-labs/osmosis/v24/x/lockup/types"
@@ -54,6 +55,12 @@ var (
 func (s *KeeperTestSuite) TestInvalidDurationGaugeCreationValidation() {
 	s.SetupTest()
 
+	// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+	// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+	for _, coin := range defaultLiquidTokens {
+		s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+	}
+
 	addrs := s.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration)
 	distrTo := lockuptypes.QueryCondition{
 		LockQueryType: lockuptypes.ByDuration,
@@ -72,6 +79,12 @@ func (s *KeeperTestSuite) TestInvalidDurationGaugeCreationValidation() {
 func (s *KeeperTestSuite) TestNonExistentDenomGaugeCreation() {
 	s.SetupTest()
 
+	// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+	// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+	for _, coin := range defaultLiquidTokens {
+		s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+	}
+
 	addrNoSupply := sdk.AccAddress([]byte("Gauge_Creation_Addr_"))
 	addrs := s.SetupManyLocks(1, defaultLiquidTokens, defaultLPTokens, defaultLockDuration)
 	distrTo := lockuptypes.QueryCondition{
@@ -397,7 +410,8 @@ func (s *KeeperTestSuite) TestAddToGaugeRewards() {
 		gaugeId            uint64
 		minimumGasConsumed uint64
 
-		expectErr bool
+		skipSettingRoute bool
+		expectErr        bool
 	}{
 		{
 			name:  "valid case: valid gauge",
@@ -441,11 +455,33 @@ func (s *KeeperTestSuite) TestAddToGaugeRewards() {
 
 			expectErr: true,
 		},
+		{
+			name:  "invalid case: valid gauge, but errors due to no protorev route",
+			owner: s.TestAccs[0],
+			coinsToAdd: sdk.NewCoins(
+				sdk.NewCoin("uosmo", osmomath.NewInt(100000)),
+				sdk.NewCoin("atom", osmomath.NewInt(99999)),
+			),
+			gaugeId:            1,
+			minimumGasConsumed: uint64(2 * types.BaseGasFeeForAddRewardToGauge),
+
+			skipSettingRoute: true,
+			expectErr:        true,
+		},
 	}
 
 	for _, tc := range testCases {
 		s.Run(tc.name, func() {
 			s.SetupTest()
+
+			// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+			// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+			if !tc.skipSettingRoute {
+				for _, coin := range tc.coinsToAdd {
+					s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+				}
+			}
+
 			_, _, existingGaugeCoins, _ := s.SetupNewGauge(true, defaultCoins)
 
 			s.FundAcc(tc.owner, tc.coinsToAdd)
@@ -500,6 +536,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 
 		expectedGaugeId  uint64
 		expectedDenomSet string
+		skipSettingRoute bool
 		expectErr        bool
 	}{
 		{
@@ -516,6 +553,21 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 			expectedDenomSet: types.NoLockExternalGaugeDenom(concentratedPoolId),
 			expectErr:        false,
 		},
+		{
+			name: "create valid no lock gauge with CL pool, but errors due to no protorev route",
+			distrTo: lockuptypes.QueryCondition{
+				LockQueryType: lockuptypes.NoLock,
+				// Note: this assumes the gauge is external
+				Denom:    "",
+				Duration: time.Nanosecond,
+			},
+			poolId: concentratedPoolId,
+
+			expectedGaugeId:  defaultExpectedGaugeId,
+			expectedDenomSet: types.NoLockExternalGaugeDenom(concentratedPoolId),
+			skipSettingRoute: true,
+			expectErr:        true,
+		},
 		{
 			name: "create valid no lock gauge with CL pool (denom set to no lock internal prefix)",
 			distrTo: lockuptypes.QueryCondition{
@@ -615,6 +667,14 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() {
 		s.Run(tc.name, func() {
 			s.SetupTest()
 
+			// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+			// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+			if !tc.skipSettingRoute {
+				for _, coin := range defaultGaugeCreationCoins {
+					s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+				}
+			}
+
 			s.PrepareBalancerPool()
 			s.PrepareConcentratedPool()
 
@@ -667,6 +727,7 @@ func (s *KeeperTestSuite) TestCreateGauge_Group() {
 
 		expectedGaugeId  uint64
 		expectedDenomSet string
+		skipSettingRoute bool
 		expectErr        error
 	}{
 		{
@@ -699,13 +760,31 @@ func (s *KeeperTestSuite) TestCreateGauge_Group() {
 
 			expectErr: types.ErrZeroNumEpochsPaidOver,
 		},
+		{
+			name:              "create valid non-perpetual group gauge, but errors due to no protorev route",
+			distrTo:           incentiveskeeper.ByGroupQueryCondition,
+			poolId:            zeroPoolId,
+			isPerpetual:       false,
+			numEpochsPaidOver: types.PerpetualNumEpochsPaidOver + 1,
+
+			skipSettingRoute: true,
+			expectErr:        types.NoRouteForDenomError{Denom: "atom"},
+		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		s.Run(tc.name, func() {
 			s.SetupTest()
 
+			// Since this test creates or adds to a gauge, we need to ensure a route exists in protorev hot routes.
+			// The pool doesn't need to actually exist for this test, so we can just ensure the denom pair has some entry.
+			if !tc.skipSettingRoute {
+				for _, coin := range defaultGaugeCreationCoins {
+					s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+				}
+			}
+
 			s.PrepareBalancerPool()
 			s.PrepareConcentratedPool()
 
@@ -834,3 +913,45 @@ func (s *KeeperTestSuite) validateNoGaugeIDInSlice(slice []types.Gauge, gaugeID
 	// No gauge matched ID.
 	s.Require().Empty(gaugeMatch)
 }
+
+func (s *KeeperTestSuite) TestCheckIfDenomsAreDistributable() {
+	s.SetupTest()
+
+	coinWithRouteA := sdk.NewCoin("denom1", sdk.NewInt(100))
+	coinWithRouteB := sdk.NewCoin("denom2", sdk.NewInt(100))
+	coinWithoutRouteC := sdk.NewCoin("denom3", sdk.NewInt(100))
+
+	for _, coin := range []sdk.Coin{coinWithRouteA, coinWithRouteB} {
+		s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+	}
+
+	testCases := []struct {
+		name        string
+		coins       sdk.Coins
+		expectedErr error
+	}{
+		{
+			name:  "valid case: all denoms are distributable",
+			coins: sdk.NewCoins(coinWithRouteA, coinWithRouteB),
+		},
+		{
+			name:        "invalid case: one denom is not distributable",
+			coins:       sdk.NewCoins(coinWithRouteA, coinWithoutRouteC),
+			expectedErr: types.NoRouteForDenomError{Denom: coinWithoutRouteC.Denom},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		s.Run(tc.name, func() {
+			// System under test
+			err := s.App.IncentivesKeeper.CheckIfDenomsAreDistributable(s.Ctx, tc.coins)
+
+			if tc.expectedErr != nil {
+				s.Require().Error(err)
+			} else {
+				s.Require().NoError(err)
+			}
+		})
+	}
+}

x/incentives/keeper/genesis_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/osmosis-labs/osmosis/osmomath"
 	osmoapp "github.com/osmosis-labs/osmosis/v24/app"
+	appparams "github.com/osmosis-labs/osmosis/v24/app/params"
 
 	"github.com/osmosis-labs/osmosis/v24/x/concentrated-liquidity/model"
 	"github.com/osmosis-labs/osmosis/v24/x/incentives/types"
@@ -230,6 +231,9 @@ func TestIncentivesInitGenesis(t *testing.T) {
 }
 
 func createAllGaugeTypes(t *testing.T, app *osmoapp.OsmosisApp, ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins, startTime time.Time) {
+	for _, coin := range coins {
+		app.ProtoRevKeeper.SetPoolForDenomPair(ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
+	}
 	// create a byDuration gauge
 	_, err := app.IncentivesKeeper.CreateGauge(ctx, true, addr, coins, distrToByDuration, startTime, 1, 0)
 	require.NoError(t, err)