feat: Enforce MarketParam.Pair uniqueness constraint (#1193)

* Enforce MarketParam.Pair uniqueness constraint

* Fix perps tests

* Fix the rest of the tests

* Prevent updating MarketParam.Pair

* Fix lint

* Fix prices test

* Allow updates again

protocol/testing/e2e/gov/perpetuals_test.go

@@ -1,6 +1,7 @@
 package gov_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/cometbft/cometbft/types"
@@ -276,6 +277,7 @@ func TestUpdatePerpetualsParams(t *testing.T) {
 						for i, marketId := range append(tc.genesisMarketIds, TEST_PERPETUAL_PARAMS.MarketId) {
 							marketParamPrice := pricestest.GenerateMarketParamPrice(
 								pricestest.WithId(marketId),
+								pricestest.WithPair(fmt.Sprintf("%d-%d", i, i)),
 							)
 							marketParams[i] = marketParamPrice.Param
 							marketPrices[i] = marketParamPrice.Price

protocol/testing/e2e/gov/prices_test.go

@@ -28,7 +28,7 @@ var (
 
 	MODIFIED_MARKET_PARAM = pricestypes.MarketParam{
 		Id:                 GENESIS_MARKET_PARAM.Id,
-		Pair:               "eth-adv4tnt",
+		Pair:               GENESIS_MARKET_PARAM.Pair,
 		Exponent:           GENESIS_MARKET_PARAM.Exponent, // exponent cannot be updated
 		MinExchanges:       3,
 		MinPriceChangePpm:  2_002,

protocol/testutil/keeper/perpetuals.go

@@ -237,9 +237,8 @@ func CreateNPerpetuals(
 	allLiquidityTiers := keeper.GetAllLiquidityTiers(ctx)
 	require.Greater(t, len(allLiquidityTiers), 0)
 
+	CreateNMarkets(t, ctx, pricesKeeper, n)
 	for i := range items {
-		CreateNMarkets(t, ctx, pricesKeeper, n)
-
 		var defaultFundingPpm int32
 		marketType := types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS
 		if i%3 == 0 {

protocol/testutil/keeper/prices.go

@@ -171,6 +171,17 @@ func AssertMarketEventsNotInIndexerBlock(
 	require.Equal(t, 0, len(indexerMarketEvents))
 }
 
+// AssertNMarketEventsNotInIndexerBlock verifies that N market events were included in the Indexer block message.
+func AssertNMarketEventsNotInIndexerBlock(
+	t *testing.T,
+	k *keeper.Keeper,
+	ctx sdk.Context,
+	n int,
+) {
+	indexerMarketEvents := getMarketEventsFromIndexerBlock(ctx, k)
+	require.Equal(t, n, len(indexerMarketEvents))
+}
+
 // getMarketEventsFromIndexerBlock returns the market events from the Indexer Block event Kafka message.
 func getMarketEventsFromIndexerBlock(
 	ctx sdk.Context,

protocol/x/perpetuals/keeper/msg_server_update_perpetual_params_test.go

@@ -24,8 +24,8 @@ func TestUpdatePerpetualParams(t *testing.T) {
 		perptest.WithTicker("ETH-USD"),
 		perptest.WithLiquidityTier(1),
 	)
-	testMarket1 := *pricestest.GenerateMarketParamPrice(pricestest.WithId(1))
-	testMarket4 := *pricestest.GenerateMarketParamPrice(pricestest.WithId(4))
+	testMarket1 := *pricestest.GenerateMarketParamPrice(pricestest.WithId(1), pricestest.WithPair("0-0"))
+	testMarket4 := *pricestest.GenerateMarketParamPrice(pricestest.WithId(4), pricestest.WithPair("1-1"))
 
 	tests := map[string]struct {
 		setup             func(*testing.T, sdk.Context, *perpkeeper.Keeper, *priceskeeper.Keeper)

protocol/x/prices/keeper/market.go

@@ -36,6 +36,15 @@ func (k Keeper) CreateMarket(
 	if err := marketPrice.ValidateFromParam(marketParam); err != nil {
 		return types.MarketParam{}, err
 	}
+	// Stateful Validation
+	for _, market := range k.GetAllMarketParams(ctx) {
+		if market.Pair == marketParam.Pair {
+			return types.MarketParam{}, errorsmod.Wrapf(
+				types.ErrMarketParamPairAlreadyExists,
+				marketParam.Pair,
+			)
+		}
+	}
 
 	paramBytes := k.cdc.MustMarshal(&marketParam)
 	priceBytes := k.cdc.MustMarshal(&marketPrice)

protocol/x/prices/keeper/market_param.go

@@ -43,6 +43,11 @@ func (k Keeper) ModifyMarketParam(
 		return types.MarketParam{},
 			errorsmod.Wrapf(types.ErrMarketExponentCannotBeUpdated, lib.UintToString(updatedMarketParam.Id))
 	}
+	for _, market := range k.GetAllMarketParams(ctx) {
+		if market.Pair == updatedMarketParam.Pair && market.Id != updatedMarketParam.Id {
+			return types.MarketParam{}, errorsmod.Wrapf(types.ErrMarketParamPairAlreadyExists, updatedMarketParam.Pair)
+		}
+	}
 
 	// Store the modified market param.
 	marketParamStore := k.getMarketParamStore(ctx)

protocol/x/prices/keeper/market_param_test.go

@@ -26,7 +26,7 @@ func TestModifyMarketParam(t *testing.T) {
 			ctx,
 			types.MarketParam{
 				Id:                 item.Param.Id,
-				Pair:               fmt.Sprintf("foo_%v", i),
+				Pair:               item.Param.Pair,
 				MinExchanges:       uint32(2),
 				Exponent:           item.Param.Exponent,
 				MinPriceChangePpm:  uint32(9_999 - i),
@@ -35,11 +35,11 @@ func TestModifyMarketParam(t *testing.T) {
 		)
 		require.NoError(t, err)
 		require.Equal(t, uint32(i), newItem.Id)
-		require.Equal(t, fmt.Sprintf("foo_%v", i), newItem.Pair)
+		require.Equal(t, fmt.Sprintf("%v-%v", i, i), newItem.Pair)
 		require.Equal(t, item.Param.Exponent, newItem.Exponent)
 		require.Equal(t, uint32(2), newItem.MinExchanges)
 		require.Equal(t, uint32(9999-i), newItem.MinPriceChangePpm)
-		require.Equal(t, fmt.Sprintf("foo_%v", i), metrics.GetMarketPairForTelemetry(item.Param.Id))
+		require.Equal(t, fmt.Sprintf("%v-%v", i, i), metrics.GetMarketPairForTelemetry(item.Param.Id))
 		require.Equal(t, fmt.Sprintf(`{"id":"%v"}`, i), newItem.ExchangeConfigJson)
 		keepertest.AssertMarketModifyEventInIndexerBlock(t, keeper, ctx, newItem)
 	}
@@ -109,13 +109,24 @@ func TestModifyMarketParam_Errors(t *testing.T) {
 				"",
 			).Error(),
 		},
+		"Updating pair fails": {
+			targetId:           0,
+			pair:               "1-1",
+			minExchanges:       uint32(1),
+			minPriceChangePpm:  uint32(50),
+			exchangeConfigJson: validExchangeConfigJson,
+			expectedErr: errorsmod.Wrapf(
+				types.ErrMarketParamPairAlreadyExists,
+				"1-1",
+			).Error(),
+		},
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			ctx, keeper, _, _, _, mockTimeKeeper := keepertest.PricesKeepers(t)
 			mockTimeKeeper.On("Now").Return(constants.TimeT)
 			ctx = ctx.WithTxBytes(constants.TestTxBytes)
-			keepertest.CreateNMarkets(t, ctx, keeper, 1)
+			keepertest.CreateNMarkets(t, ctx, keeper, 2)
 			_, err := keeper.ModifyMarketParam(
 				ctx,
 				types.MarketParam{

protocol/x/prices/keeper/market_price_test.go

@@ -135,19 +135,23 @@ func TestGetMarketIdToValidIndexPrice(t *testing.T) {
 		keeper,
 		[]types.MarketParamPrice{
 			*pricestest.GenerateMarketParamPrice(
+				pricestest.WithPair("0-0"),
 				pricestest.WithId(6),
 				pricestest.WithMinExchanges(2),
 			),
 			*pricestest.GenerateMarketParamPrice(
+				pricestest.WithPair("1-1"),
 				pricestest.WithId(7),
 				pricestest.WithMinExchanges(2),
 			),
 			*pricestest.GenerateMarketParamPrice(
+				pricestest.WithPair("2-2"),
 				pricestest.WithId(8),
 				pricestest.WithMinExchanges(2),
 				pricestest.WithExponent(-8),
 			),
 			*pricestest.GenerateMarketParamPrice(
+				pricestest.WithPair("3-3"),
 				pricestest.WithId(9),
 				pricestest.WithMinExchanges(2),
 				pricestest.WithExponent(-9),

protocol/x/prices/keeper/market_test.go

@@ -158,7 +158,7 @@ func TestCreateMarket_Errors(t *testing.T) {
 			exchangeConfigJson:                    validExchangeConfigJson,
 			expectedErr: errorsmod.Wrap(
 				types.ErrInvalidInput,
-				"market param id 0 does not match market price id 1",
+				"market param id 1 does not match market price id 2",
 			).Error(),
 		},
 		"Market param and price exponents don't match": {
@@ -170,15 +170,29 @@ func TestCreateMarket_Errors(t *testing.T) {
 			exchangeConfigJson: validExchangeConfigJson,
 			expectedErr: errorsmod.Wrap(
 				types.ErrInvalidInput,
-				"market param 0 exponent -6 does not match market price 0 exponent -5",
+				"market param 1 exponent -6 does not match market price 1 exponent -5",
+			).Error(),
+		},
+		"Pair already exists": {
+			pair:               "0-0",
+			minExchanges:       uint32(2),
+			minPriceChangePpm:  uint32(50),
+			price:              constants.FiveBillion,
+			exchangeConfigJson: validExchangeConfigJson,
+			expectedErr: errorsmod.Wrap(
+				types.ErrMarketParamPairAlreadyExists,
+				"0-0",
 			).Error(),
 		},
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			ctx, keeper, _, _, _, _ := keepertest.PricesKeepers(t)
+			ctx, keeper, _, _, _, mockTimeKeeper := keepertest.PricesKeepers(t)
 			ctx = ctx.WithTxBytes(constants.TestTxBytes)
 
+			mockTimeKeeper.On("Now").Return(constants.TimeT)
+			keepertest.CreateNMarkets(t, ctx, keeper, 1)
+
 			marketPriceIdOffset := uint32(0)
 			if tc.marketPriceIdDoesntMatchMarketParamId {
 				marketPriceIdOffset = uint32(1)
@@ -192,31 +206,31 @@ func TestCreateMarket_Errors(t *testing.T) {
 			_, err := keeper.CreateMarket(
 				ctx,
 				types.MarketParam{
-					Id:                 0,
+					Id:                 1,
 					Pair:               tc.pair,
 					Exponent:           int32(-6),
 					MinExchanges:       tc.minExchanges,
 					MinPriceChangePpm:  tc.minPriceChangePpm,
 					ExchangeConfigJson: tc.exchangeConfigJson,
 				},
 				types.MarketPrice{
-					Id:       0 + marketPriceIdOffset,
+					Id:       1 + marketPriceIdOffset,
 					Exponent: int32(-6) + marketPriceExponentOffset,
 					Price:    tc.price,
 				},
 			)
 			require.EqualError(t, err, tc.expectedErr)
 
 			// Verify no new MarketPrice created.
-			_, err = keeper.GetMarketPrice(ctx, 0)
+			_, err = keeper.GetMarketPrice(ctx, 1)
 			require.EqualError(
 				t,
 				err,
-				errorsmod.Wrap(types.ErrMarketPriceDoesNotExist, lib.UintToString(uint32(0))).Error(),
+				errorsmod.Wrap(types.ErrMarketPriceDoesNotExist, lib.UintToString(uint32(1))).Error(),
 			)
 
 			// Verify no new market event.
-			keepertest.AssertMarketEventsNotInIndexerBlock(t, keeper, ctx)
+			keepertest.AssertNMarketEventsNotInIndexerBlock(t, keeper, ctx, 1)
 		})
 	}
 }

protocol/x/prices/keeper/msg_server_update_market_param_test.go

@@ -30,7 +30,7 @@ func TestUpdateMarketParam(t *testing.T) {
 				Authority: lib.GovModuleAddress.String(),
 				MarketParam: pricestypes.MarketParam{
 					Id:                 testMarketParam.Id,
-					Pair:               "PIKACHU-XXX",
+					Pair:               testMarketParam.Pair,
 					Exponent:           testMarketParam.Exponent,
 					MinExchanges:       72,
 					MinPriceChangePpm:  2_023,

protocol/x/prices/types/errors.go

@@ -24,6 +24,7 @@ var (
 	ErrMarketExponentCannotBeUpdated  = errorsmod.Register(ModuleName, 202, "Market exponent cannot be updated")
 	ErrMarketPricesAndParamsDontMatch = errorsmod.Register(ModuleName, 203, "Market prices and params don't match")
 	ErrMarketParamAlreadyExists       = errorsmod.Register(ModuleName, 204, "Market params already exists")
+	ErrMarketParamPairAlreadyExists   = errorsmod.Register(ModuleName, 205, "Market params pair already exists")
 
 	// 300 - 399: Price related errors.
 	ErrIndexPriceNotAvailable = errorsmod.Register(ModuleName, 300, "Index price is not available")

protocol/x/subaccounts/keeper/subaccount_test.go

@@ -2019,10 +2019,11 @@ func TestUpdateSubaccounts(t *testing.T) {
 				),
 			},
 			marketParamPrices: []pricestypes.MarketParamPrice{
-				*pricestest.GenerateMarketParamPrice(pricestest.WithId(100)),
+				*pricestest.GenerateMarketParamPrice(pricestest.WithId(100), pricestest.WithPair("0")),
 				*pricestest.GenerateMarketParamPrice(
 					pricestest.WithId(101),
 					pricestest.WithPriceValue(0),
+					pricestest.WithPair("1"),
 				),
 			},
 			perpetualPositions: []*types.PerpetualPosition{
@@ -3605,10 +3606,11 @@ func TestCanUpdateSubaccounts(t *testing.T) {
 				),
 			},
 			marketParamPrices: []pricestypes.MarketParamPrice{
-				*pricestest.GenerateMarketParamPrice(pricestest.WithId(100)),
+				*pricestest.GenerateMarketParamPrice(pricestest.WithId(100), pricestest.WithPair("0")),
 				*pricestest.GenerateMarketParamPrice(
 					pricestest.WithId(101),
 					pricestest.WithPriceValue(0),
+					pricestest.WithPair("1"),
 				),
 			},
 			perpetualPositions: []*types.PerpetualPosition{