Merge pull request #624 from neutron-org/chore/reverse-dex-prices

Chore: Flip Dex Price Logic #NTRN-355

proto/neutron/dex/limit_order_tranche.proto

@@ -49,10 +49,19 @@ message LimitOrderTranche {
     (gogoproto.stdtime) = true,
     (gogoproto.nullable) = true
   ];
+  // DEPRECATED: price_taker_to_maker will be removed in future release, `maker_price` should always be used.
   string price_taker_to_maker = 7 [
     (gogoproto.moretags) = "yaml:\"price_taker_to_maker\"",
     (gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
     (gogoproto.nullable) = false,
-    (gogoproto.jsontag) = "price_taker_to_maker"
+    (gogoproto.jsontag) = "price_taker_to_maker",
+    deprecated = true
+  ];
+  // This is the price of the LimitOrder denominated in the opposite token. (ie. 1 TokenA with a maker_price of 10 is worth 10 TokenB )
+  string maker_price = 8 [
+    (gogoproto.moretags) = "yaml:\"maker_price\"",
+    (gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
+    (gogoproto.nullable) = false,
+    (gogoproto.jsontag) = "maker_price"
   ];
 }

proto/neutron/dex/pool_reserves.proto

@@ -20,16 +20,28 @@ message PoolReserves {
     (gogoproto.jsontag) = "reserves_maker_denom",
     (gogoproto.nullable) = false
   ];
+  // DEPRECATED: price_taker_to_maker will be removed in future release, `maker_price` should always be used.
   string price_taker_to_maker = 3 [
     (gogoproto.moretags) = "yaml:\"price_taker_to_maker\"",
     (gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
     (gogoproto.nullable) = false,
-    (gogoproto.jsontag) = "price_taker_to_maker"
+    (gogoproto.jsontag) = "price_taker_to_maker",
+    deprecated = true
   ];
+  // DEPRECATED: price_opposite_taker_maker was an internal implementation detail and will be removed in a future release.
+  // It is being kept strictly for backwards compatibility. The actual field value is unused.
   string price_opposite_taker_to_maker = 4 [
     (gogoproto.moretags) = "yaml:\"price_opposite_taker_to_maker\"",
     (gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
     (gogoproto.nullable) = false,
-    (gogoproto.jsontag) = "price_opposite_taker_to_maker"
+    (gogoproto.jsontag) = "price_opposite_taker_to_maker",
+    deprecated = true
+  ];
+  // This is the price of the PoolReserves denominated in the opposite token. (ie. 1 TokenA with a maker_price of 10 is worth 10 TokenB )
+  string maker_price = 5 [
+    (gogoproto.moretags) = "yaml:\"maker_price\"",
+    (gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
+    (gogoproto.nullable) = false,
+    (gogoproto.jsontag) = "maker_price"
   ];
 }

x/dex/keeper/grpc_query_simulate_place_limit_order.go

@@ -6,6 +6,7 @@ import (
 	"cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	math_utils "github.com/neutron-org/neutron/v5/utils/math"
 	"github.com/neutron-org/neutron/v5/x/dex/types"
 )
 
@@ -36,7 +37,8 @@ func (k Keeper) SimulatePlaceLimitOrder(
 	}
 	tickIndex := msg.TickIndexInToOut
 	if msg.LimitSellPrice != nil {
-		tickIndex, err = types.CalcTickIndexFromPrice(*msg.LimitSellPrice)
+		limitBuyPrice := math_utils.OnePrecDec().Quo(*msg.LimitSellPrice)
+		tickIndex, err = types.CalcTickIndexFromPrice(limitBuyPrice)
 		if err != nil {
 			return nil, errors.Wrapf(err, "invalid LimitSellPrice %s", msg.LimitSellPrice.String())
 		}

x/dex/keeper/integration_cancellimitorder_test.go

@@ -246,8 +246,8 @@ func (s *DexTestSuite) TestCancelPartiallyFilledWithdrawFails() {
 	s.aliceCancelsLimitSell(trancheKey)
 
 	// Then alice gets back remaining ~37 BIGTokenA LO reserves & 10 BIGTokenB taker tokens
-	s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(10000000))
-	s.assertDexBalancesInt(sdkmath.OneInt(), sdkmath.ZeroInt())
+	s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(9999999))
+	s.assertDexBalancesInt(sdkmath.OneInt(), sdkmath.OneInt())
 
 	// Assert that the LimitOrderTrancheUser has been deleted
 	_, found := s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey)
@@ -304,8 +304,8 @@ func (s *DexTestSuite) TestCancelPartiallyFilledMultiUser2() {
 	s.aliceCancelsLimitSell(trancheKey)
 
 	// THEN alice gets back remaining ~38 BIGTokenA LO reserves & 10 BIGTokenB taker tokens
-	s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(10000000))
-	s.assertDexBalancesInt(sdkmath.NewInt(37786096), sdkmath.NewInt(10000000))
+	s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(9999999))
+	s.assertDexBalancesInt(sdkmath.NewInt(37786096), sdkmath.NewInt(10000001))
 
 	// THEN carol swap through more of the limitorder
 	s.carolLimitSells("TokenB", -2001, 20, types.LimitOrderType_FILL_OR_KILL)

x/dex/keeper/limit_order_tranche.go

@@ -10,6 +10,7 @@ import (
 	storetypes "cosmossdk.io/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	math_utils "github.com/neutron-org/neutron/v5/utils/math"
 	"github.com/neutron-org/neutron/v5/x/dex/types"
 	"github.com/neutron-org/neutron/v5/x/dex/utils"
 )
@@ -18,7 +19,7 @@ func NewLimitOrderTranche(
 	limitOrderTrancheKey *types.LimitOrderTrancheKey,
 	goodTil *time.Time,
 ) (*types.LimitOrderTranche, error) {
-	priceTakerToMaker, err := limitOrderTrancheKey.PriceTakerToMaker()
+	price, err := limitOrderTrancheKey.Price()
 	if err != nil {
 		return nil, err
 	}
@@ -29,7 +30,8 @@ func NewLimitOrderTranche(
 		TotalMakerDenom:    math.ZeroInt(),
 		TotalTakerDenom:    math.ZeroInt(),
 		ExpirationTime:     goodTil,
-		PriceTakerToMaker:  priceTakerToMaker,
+		MakerPrice:         price,
+		PriceTakerToMaker:  math_utils.OnePrecDec().Quo(price),
 	}, nil
 }
 

x/dex/keeper/liquidity.go

@@ -37,7 +37,7 @@ func (k Keeper) Swap(
 		}
 
 		// break as soon as we iterated past limitPrice
-		if limitPrice != nil && liq.Price().LT(*limitPrice) {
+		if limitPrice != nil && liq.Price().GT(*limitPrice) {
 			break
 		}
 
@@ -52,11 +52,11 @@ func (k Keeper) Swap(
 		// this avoids unnecessary iteration since outAmount will always be 0 going forward
 		// this also catches the normal exit case where remainingTakerDenom == 0
 
-		// NOTE: In theory this check should be: price * remainingTakerDenom < 1
+		// NOTE: In theory this check should be:  remainingTakerDenom / price  < 1
 		// but due to rounding and inaccuracy of fixed decimal math, it is possible
 		// for liq.swap to use the full the amount of taker liquidity and have a leftover
 		// amount of the taker Denom > than 1 token worth of maker denom
-		if liq.Price().MulInt(remainingTakerDenom).LT(math_utils.NewPrecDec(2)) {
+		if math_utils.NewPrecDecFromInt(remainingTakerDenom).Quo(liq.Price()).LT(math_utils.NewPrecDec(2)) {
 			orderFilled = true
 			break
 		}
@@ -181,7 +181,7 @@ func (k Keeper) MakerLimitOrderSwap(
 
 	if totalInCoin.Amount.IsPositive() {
 		remainingIn := amountIn.Sub(totalInCoin.Amount)
-		expectedOutMakerPortion := limitPrice.MulInt(remainingIn)
+		expectedOutMakerPortion := math_utils.NewPrecDecFromInt(remainingIn).Quo(limitPrice)
 		totalExpectedOut := expectedOutMakerPortion.Add(math_utils.NewPrecDecFromInt(totalOutCoin.Amount))
 		truePrice := totalExpectedOut.QuoInt(amountIn)
 

x/dex/keeper/liquidity_test.go

@@ -483,8 +483,8 @@ func (s *DexTestSuite) TestSwap1To0LOMaxAmountNotUsed() {
 	tokenIn, tokenOut := s.swapWithMaxOut("TokenB", "TokenA", 8, 15)
 
 	// THEN swap should return 8 BIGTokenB in and ~8 BIGTokenA out
-	s.assertSwapOutputInt(tokenIn, sdkmath.NewInt(8_000_000), tokenOut, sdkmath.NewInt(8_000_800))
-	s.assertTickBalancesInt(sdkmath.NewInt(1_999_200), sdkmath.NewInt(8_000_000))
+	s.assertSwapOutputInt(tokenIn, sdkmath.NewInt(8_000_000), tokenOut, sdkmath.NewInt(8_000_799))
+	s.assertTickBalancesInt(sdkmath.NewInt(1_999_201), sdkmath.NewInt(8_000_000))
 }
 
 func (s *DexTestSuite) TestSwap0To1LOMaxAmountUsedMultiTick() {

x/dex/keeper/migrations.go

@@ -5,6 +5,7 @@ import (
 
 	v3 "github.com/neutron-org/neutron/v5/x/dex/migrations/v3"
 	v4 "github.com/neutron-org/neutron/v5/x/dex/migrations/v4"
+	v5 "github.com/neutron-org/neutron/v5/x/dex/migrations/v5"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -26,3 +27,8 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error {
 func (m Migrator) Migrate3to4(ctx sdk.Context) error {
 	return v4.MigrateStore(ctx, m.keeper.cdc, m.keeper.storeKey)
 }
+
+// Migrate4to5 migrates from version 4 to 5.
+func (m Migrator) Migrate4to5(ctx sdk.Context) error {
+	return v5.MigrateStore(ctx, m.keeper.cdc, m.keeper.storeKey)
+}

x/dex/keeper/msg_server.go

@@ -8,6 +8,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
+	math_utils "github.com/neutron-org/neutron/v5/utils/math"
 	"github.com/neutron-org/neutron/v5/x/dex/types"
 )
 
@@ -136,7 +137,8 @@ func (k MsgServer) PlaceLimitOrder(
 	}
 	tickIndex := msg.TickIndexInToOut
 	if msg.LimitSellPrice != nil {
-		tickIndex, err = types.CalcTickIndexFromPrice(*msg.LimitSellPrice)
+		limitBuyPrice := math_utils.OnePrecDec().Quo(*msg.LimitSellPrice)
+		tickIndex, err = types.CalcTickIndexFromPrice(limitBuyPrice)
 		if err != nil {
 			return &types.MsgPlaceLimitOrderResponse{}, errors.Wrapf(err, "invalid LimitSellPrice %s", msg.LimitSellPrice.String())
 		}

x/dex/keeper/multihop_swap.go

@@ -149,7 +149,7 @@ func (k Keeper) HopsToRouteData(
 		if !found {
 			return routeArr, types.ErrLimitPriceNotSatisfied
 		}
-		priceAcc = priceAcc.Mul(price)
+		priceAcc = priceAcc.Quo(price)
 		routeArr[index] = MultihopStep{
 			tradePairID:        tradePairID,
 			RemainingBestPrice: priceAcc,

x/dex/keeper/place_limit_order.go

@@ -109,30 +109,29 @@ func (k Keeper) ExecutePlaceLimitOrder(
 ) (trancheKey string, totalIn math.Int, swapInCoin, swapOutCoin sdk.Coin, sharesIssued math.Int, err error) {
 	amountLeft := amountIn
 
-	var limitPrice math_utils.PrecDec
-	limitPrice, err = types.CalcPrice(tickIndexInToOut)
-
+	limitBuyPrice, err := types.CalcPrice(tickIndexInToOut)
 	if err != nil {
 		return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
 	}
 
-	// Use limitPrice for minAvgSellPrice if it has not be specified
-	minAvgSellPrice := limitPrice
+	// Use limitPrice for minAvgSellPrice if it has not been specified
+	minAvgSellPrice := math_utils.OnePrecDec().Quo(limitBuyPrice)
+
 	if minAvgSellPriceP != nil {
 		minAvgSellPrice = *minAvgSellPriceP
 	}
 
 	// Ensure that after rounding user will get at least 1 token out.
-	err = types.ValidateFairOutput(amountIn, limitPrice)
+	err = types.ValidateFairOutput(amountIn, limitBuyPrice)
 	if err != nil {
 		return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
 	}
 
 	var orderFilled bool
 	if orderType.IsTakerOnly() {
-		swapInCoin, swapOutCoin, err = k.TakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, maxAmountOut, limitPrice, minAvgSellPrice, orderType)
+		swapInCoin, swapOutCoin, err = k.TakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, maxAmountOut, limitBuyPrice, minAvgSellPrice, orderType)
 	} else {
-		swapInCoin, swapOutCoin, orderFilled, err = k.MakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, limitPrice, minAvgSellPrice)
+		swapInCoin, swapOutCoin, orderFilled, err = k.MakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, limitBuyPrice, minAvgSellPrice)
 	}
 	if err != nil {
 		return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
@@ -142,12 +141,12 @@ func (k Keeper) ExecutePlaceLimitOrder(
 	amountLeft = amountLeft.Sub(swapInCoin.Amount)
 
 	makerTradePairID := takerTradePairID.Reversed()
-	makerTickIndexTakerToMaker := tickIndexInToOut * -1
+	tickIndexTakerToMaker := tickIndexInToOut * -1
 	var placeTranche *types.LimitOrderTranche
 	placeTranche, err = k.GetOrInitPlaceTranche(
 		ctx,
 		makerTradePairID,
-		makerTickIndexTakerToMaker,
+		tickIndexTakerToMaker,
 		goodTil,
 		orderType,
 	)
@@ -159,7 +158,7 @@ func (k Keeper) ExecutePlaceLimitOrder(
 	trancheUser := k.GetOrInitLimitOrderTrancheUser(
 		ctx,
 		makerTradePairID,
-		makerTickIndexTakerToMaker,
+		tickIndexTakerToMaker,
 		trancheKey,
 		orderType,
 		receiverAddr.String(),
@@ -173,7 +172,7 @@ func (k Keeper) ExecutePlaceLimitOrder(
 		// NOTE: This does mean that a successful taker leg of the trade will be thrown away since the entire tx will fail.
 		// In most circumstances this seems preferable to executing the taker leg and exiting early before placing a maker
 		// order with the remaining liquidity.
-		err = types.ValidateFairOutput(amountLeft, limitPrice)
+		err = types.ValidateFairOutput(amountLeft, limitBuyPrice)
 		if err != nil {
 			return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
 		}

x/dex/migrations/v4/store.go

@@ -45,12 +45,11 @@ func migrateTickLiquidityPrices(ctx sdk.Context, cdc codec.BinaryCodec, storeKey
 		// Recalculate all prices
 		switch liquidity := tickLiq.Liquidity.(type) {
 		case *types.TickLiquidity_LimitOrderTranche:
-			liquidity.LimitOrderTranche.PriceTakerToMaker = types.MustCalcPrice(liquidity.LimitOrderTranche.Key.TickIndexTakerToMaker)
+			liquidity.LimitOrderTranche.PriceTakerToMaker = types.MustCalcPrice(-liquidity.LimitOrderTranche.Key.TickIndexTakerToMaker)
 			updatedTickLiq = types.TickLiquidity{Liquidity: liquidity}
 		case *types.TickLiquidity_PoolReserves:
 			poolReservesKey := liquidity.PoolReserves.Key
-			liquidity.PoolReserves.PriceTakerToMaker = types.MustCalcPrice(poolReservesKey.TickIndexTakerToMaker)
-			liquidity.PoolReserves.PriceOppositeTakerToMaker = poolReservesKey.Counterpart().MustPriceTakerToMaker()
+			liquidity.PoolReserves.PriceTakerToMaker = types.MustCalcPrice(-poolReservesKey.TickIndexTakerToMaker)
 			updatedTickLiq = types.TickLiquidity{Liquidity: liquidity}
 
 		default:
@@ -90,7 +89,7 @@ func migrateInactiveTranchePrices(ctx sdk.Context, cdc codec.BinaryCodec, storeK
 		var tranche types.LimitOrderTranche
 		cdc.MustUnmarshal(iterator.Value(), &tranche)
 		// Recalculate price
-		tranche.PriceTakerToMaker = types.MustCalcPrice(tranche.Key.TickIndexTakerToMaker)
+		tranche.PriceTakerToMaker = types.MustCalcPrice(-tranche.Key.TickIndexTakerToMaker)
 
 		bz := cdc.MustMarshal(&tranche)
 		ticksToUpdate = append(ticksToUpdate, migrationUpdate{key: iterator.Key(), val: bz})

x/dex/migrations/v4/store_test.go

@@ -49,9 +49,8 @@ func (suite *V4DexMigrationTestSuite) TestPriceUpdates() {
 		Fee:                   1,
 	}
 	poolReserves := &types.PoolReserves{
-		Key:                       poolKey,
-		PriceTakerToMaker:         math.ZeroPrecDec(),
-		PriceOppositeTakerToMaker: math.ZeroPrecDec(),
+		Key:               poolKey,
+		PriceTakerToMaker: math.ZeroPrecDec(),
 	}
 	app.DexKeeper.SetPoolReserves(ctx, poolReserves)
 
@@ -60,6 +59,7 @@ func (suite *V4DexMigrationTestSuite) TestPriceUpdates() {
 
 	// Check LimitOrderTranche has correct price
 	newTranche := app.DexKeeper.GetLimitOrderTranche(ctx, trancheKey)
+
 	suite.True(newTranche.PriceTakerToMaker.Equal(math.MustNewPrecDecFromStr("1.005012269623051203500693815")))
 
 	// check InactiveLimitOrderTranche has correct price
@@ -68,6 +68,5 @@ func (suite *V4DexMigrationTestSuite) TestPriceUpdates() {
 
 	// Check PoolReserves has the correct prices
 	newPool, _ := app.DexKeeper.GetPoolReserves(ctx, poolKey)
-	suite.True(newPool.PriceTakerToMaker.Equal(math.MustNewPrecDecFromStr("0.002479495864288162666675923")))
-	suite.True(newPool.PriceOppositeTakerToMaker.Equal(math.MustNewPrecDecFromStr("403.227141612124702272520931931")))
+	suite.True(newPool.PriceTakerToMaker.Equal(math.MustNewPrecDecFromStr("0.002479495864288162666675934")))
 }