fix: prevent unfair deposit and LO

Due to rounding in favor of the make when trading against an LP or LO position
small amounts can unfairly benefit the maker at the expense of the taker.
This is solved by calculating the real price of a position for the taker when
the position is created and throwing an error if a position creates an effective
spread > .5% between the expected price and the true price.

x/dex/keeper/core.go

@@ -64,14 +64,20 @@ func (k Keeper) DepositCore(
 
 		existingShares := k.bankKeeper.GetSupply(ctx, pool.GetPoolDenom()).Amount
 
-		inAmount0, inAmount1, outShares := pool.Deposit(amount0, amount1, existingShares, autoswap)
+		inAmount0, inAmount1, outShares, err := pool.Deposit(amount0, amount1, existingShares, autoswap)
+		if err != nil {
+			return nil, nil, nil, err
+		}
 
 		k.SetPool(ctx, pool)
 
 		if inAmount0.IsZero() && inAmount1.IsZero() {
 			return nil, nil, nil, types.ErrZeroTrueDeposit
 		}
 
+		// NOTE: This check should be impossible since it is subsumed by the tick.ensureFairTruePrice() check
+		// but it should be left in place as an extra safeguard and to prevent regression should the FairTruePrice check
+		// change in the future
 		if outShares.IsZero() {
 			return nil, nil, nil, types.ErrDepositShareUnderflow
 		}
@@ -391,7 +397,11 @@ func (k Keeper) PlaceLimitOrderCore(
 	// FOR GTC, JIT & GoodTil try to place a maker limitOrder with remaining Amount
 	if amountLeft.IsPositive() &&
 		(orderType.IsGTC() || orderType.IsJIT() || orderType.IsGoodTil()) {
-		placeTranche.PlaceMakerLimitOrder(amountLeft)
+		err = placeTranche.PlaceMakerLimitOrder(amountLeft)
+		if err != nil {
+			return
+		}
+
 		trancheUser.SharesOwned = trancheUser.SharesOwned.Add(amountLeft)
 
 		if orderType.HasExpiration() {

x/dex/keeper/integration_deposit_singlesided_test.go

@@ -425,6 +425,34 @@ func (s *DexTestSuite) TestDepositSingleSidedZeroTrueAmountsFail() {
 // 	)
 // }
 
+func (s *DexTestSuite) TestDepositSingleTokenAUnfairSpreadFails() {
+	s.fundAliceBalances(20, 0)
+
+	// GIVEN
+	// deposit 20 of token A at tick -20 fee 1
+	// the taker would get an effective price of .95 (tick 512)
+
+	// THEN deposit should fail
+	s.assertAliceDepositFails(
+		types.ErrInvalidPositionSpread,
+		NewDepositInt(sdkmath.NewInt(20), sdkmath.ZeroInt(), -20, 1),
+	)
+}
+
+func (s *DexTestSuite) TestDepositSingleTokenBUnfairSpreadFails() {
+	s.fundAliceBalances(0, 70)
+
+	// GIVEN
+	// deposit 70 of token B at tick 0 fee 1
+	// the taker would get an effective price of .985 (tick 140)
+
+	// THEN deposit should fail
+	s.assertAliceDepositFails(
+		types.ErrInvalidPositionSpread,
+		NewDepositInt(sdkmath.ZeroInt(), sdkmath.NewInt(70), 0, 1),
+	)
+}
+
 func (s *DexTestSuite) TestDepositSingleInvalidFeeFails() {
 	s.fundAliceBalances(0, 50)
 

x/dex/keeper/integration_placelimitorder_test.go

@@ -658,3 +658,27 @@ func (s *DexTestSuite) TestPlaceLimitOrderGoodTilAlreadyExpiredFails() {
 	})
 	s.Assert().ErrorIs(err, types.ErrExpirationTimeInPast)
 }
+
+func (s *DexTestSuite) TestPlaceLimitOrderTokenAUnfairSpreadFails() {
+	s.fundAliceBalances(20, 0)
+
+	// GIVEN
+	// alice place a LO of 20 of TokenA at tick -20
+	// the taker would get an effective price of .95 (tick 512)
+
+	// THEN LO should fail
+	_, err := s.limitSellsInt(s.alice, "TokenA", -20, sdkmath.NewInt(20))
+	s.ErrorIs(err, types.ErrInvalidPositionSpread)
+}
+
+func (s *DexTestSuite) TestPlaceLimitOrderTokenBUnfairSpreadFails() {
+	s.fundAliceBalances(0, 70)
+
+	// GIVEN
+	// alice place a LO of 70 of TokenB at tick 1
+	// the taker would get an effective price of .985 (tick 140)
+
+	// THEN deposit should fail
+	_, err := s.limitSellsInt(s.alice, "TokenB", 1, sdkmath.NewInt(70))
+	s.ErrorIs(err, types.ErrInvalidPositionSpread)
+}

x/dex/keeper/msg_server_test.go

@@ -422,6 +422,34 @@ func (s *DexTestSuite) limitSells(
 	return msg.TrancheKey, err
 }
 
+func (s *DexTestSuite) limitSellsInt(
+	account sdk.AccAddress,
+	tokenIn string,
+	tickIndexNormalized int, amountIn sdkmath.Int,
+	orderTypeOpt ...types.LimitOrderType,
+) (string, error) {
+	var orderType types.LimitOrderType
+	if len(orderTypeOpt) == 0 {
+		orderType = types.LimitOrderType_GOOD_TIL_CANCELLED
+	} else {
+		orderType = orderTypeOpt[0]
+	}
+
+	tradePairID := types.NewTradePairIDFromTaker(defaultPairID, tokenIn)
+	tickIndexTakerToMaker := tradePairID.TickIndexTakerToMaker(int64(tickIndexNormalized))
+	msg, err := s.msgServer.PlaceLimitOrder(s.GoCtx, &types.MsgPlaceLimitOrder{
+		Creator:          account.String(),
+		Receiver:         account.String(),
+		TokenIn:          tradePairID.TakerDenom,
+		TokenOut:         tradePairID.MakerDenom,
+		TickIndexInToOut: tickIndexTakerToMaker,
+		AmountIn:         amountIn,
+		OrderType:        orderType,
+	})
+
+	return msg.TrancheKey, err
+}
+
 func (s *DexTestSuite) limitSellsGoodTil(
 	account sdk.AccAddress,
 	tokenIn string,
@@ -467,15 +495,24 @@ type DepositWithOptions struct {
 	Options   DepositOptions
 }
 
-func NewDeposit(amountA, amountB, tickIndex, fee int) *Deposit {
+func NewDepositInt(amountA, amountB sdkmath.Int, tickIndex, fee int) *Deposit {
 	return &Deposit{
-		AmountA:   sdkmath.NewInt(int64(amountA)).Mul(denomMultiple),
-		AmountB:   sdkmath.NewInt(int64(amountB)).Mul(denomMultiple),
+		AmountA:   amountA,
+		AmountB:   amountB,
 		TickIndex: int64(tickIndex),
 		Fee:       uint64(fee),
 	}
 }
 
+func NewDeposit(amountA, amountB, tickIndex, fee int) *Deposit {
+	return NewDepositInt(
+		sdkmath.NewInt(int64(amountA)).Mul(denomMultiple),
+		sdkmath.NewInt(int64(amountB)).Mul(denomMultiple),
+		tickIndex,
+		fee,
+	)
+}
+
 func NewDepositWithOptions(
 	amountA, amountB, tickIndex, fee int,
 	options DepositOptions,

x/dex/keeper/pool_test.go

@@ -18,7 +18,10 @@ func createNPools(k *keeper.Keeper, ctx sdk.Context, n int) []*types.Pool {
 		if err != nil {
 			panic("failed to create pool")
 		}
-		pool.Deposit(math.NewInt(10), math.NewInt(0), math.ZeroInt(), true)
+		_, _, _, err = pool.Deposit(math.NewInt(10), math.NewInt(0), math.ZeroInt(), true)
+		if err != nil {
+			panic("pool deposit failed")
+		}
 		k.SetPool(ctx, pool)
 		items[i] = pool
 	}
@@ -31,7 +34,8 @@ func TestPoolInit(t *testing.T) {
 
 	pool, err := keeper.InitPool(ctx, defaultPairID, 0, 1)
 	require.NoError(t, err)
-	pool.Deposit(math.NewInt(1000), math.NewInt(1000), math.NewInt(0), true)
+	_, _, _, err = pool.Deposit(math.NewInt(1000), math.NewInt(1000), math.NewInt(0), true)
+	require.NoError(t, err)
 	keeper.SetPool(ctx, pool)
 
 	dbPool, found := keeper.GetPool(ctx, defaultPairID, 0, 1)

x/dex/types/errors.go

@@ -164,4 +164,9 @@ var (
 		1149,
 		"Invalid Address",
 	)
+	ErrInvalidPositionSpread = sdkerrors.Register(
+		ModuleName,
+		1151,
+		"Invalid position; creates unfair spread for taker",
+	)
 )

x/dex/types/limit_order_tranche.go

@@ -157,7 +157,31 @@ func (t *LimitOrderTranche) Swap(maxAmountTakerIn math.Int, maxAmountMakerOut *m
 	return inAmount, outAmount
 }
 
-func (t *LimitOrderTranche) PlaceMakerLimitOrder(amountIn math.Int) {
+func (t *LimitOrderTranche) PlaceMakerLimitOrder(amountIn math.Int) error {
 	t.ReservesMakerDenom = t.ReservesMakerDenom.Add(amountIn)
 	t.TotalMakerDenom = t.TotalMakerDenom.Add(amountIn)
+
+	return t.ensureFairTruePrice()
+}
+
+func (t *LimitOrderTranche) truePriceTaker() math_utils.PrecDec {
+	// If tranche is empty don't try to calculate price
+	if !t.HasTokenIn() {
+		return math_utils.ZeroPrecDec()
+	}
+
+	takerAmountIn := math_utils.NewPrecDecFromInt(t.ReservesMakerDenom).Quo(t.PriceTakerToMaker).Ceil()
+
+	return takerAmountIn.QuoInt(t.ReservesMakerDenom)
+}
+
+func (t *LimitOrderTranche) ensureFairTruePrice() error {
+	priceMakerToTaker := math_utils.OnePrecDec().Quo(t.PriceTakerToMaker)
+	priceDiffFromExpected := t.truePriceTaker().Sub(priceMakerToTaker)
+	pctDiff := priceDiffFromExpected.Quo(priceMakerToTaker)
+
+	if pctDiff.GT(MaxTrueMakerSpread) {
+		return ErrInvalidPositionSpread
+	}
+	return nil
 }

x/dex/types/pool.go

@@ -113,7 +113,7 @@ func (p *Pool) Deposit(
 	maxAmount1,
 	existingShares math.Int,
 	autoswap bool,
-) (inAmount0, inAmount1 math.Int, outShares sdk.Coin) {
+) (inAmount0, inAmount1 math.Int, outShares sdk.Coin, err error) {
 	lowerReserve0 := &p.LowerTick0.ReservesMakerDenom
 	upperReserve1 := &p.UpperTick1.ReservesMakerDenom
 
@@ -125,7 +125,7 @@ func (p *Pool) Deposit(
 	)
 
 	if inAmount0.Equal(math.ZeroInt()) && inAmount1.Equal(math.ZeroInt()) {
-		return math.ZeroInt(), math.ZeroInt(), sdk.Coin{Denom: p.GetPoolDenom()}
+		return math.ZeroInt(), math.ZeroInt(), sdk.Coin{Denom: p.GetPoolDenom()}, nil
 	}
 
 	outShares = p.CalcSharesMinted(inAmount0, inAmount1, existingShares)
@@ -137,7 +137,10 @@ func (p *Pool) Deposit(
 		// NOTE: Currently not doing anything with the error,
 		// but added error handling to all of the new functions for autoswap.
 		// Open to changing it however.
-		residualShares, _ := p.CalcResidualSharesMinted(residualAmount0, residualAmount1)
+		residualShares, err := p.CalcResidualSharesMinted(residualAmount0, residualAmount1)
+		if err != nil {
+			return sdk.ZeroInt(), sdk.ZeroInt(), sdk.Coin{}, err
+		}
 
 		outShares = outShares.Add(residualShares)
 
@@ -148,7 +151,15 @@ func (p *Pool) Deposit(
 	*lowerReserve0 = lowerReserve0.Add(inAmount0)
 	*upperReserve1 = upperReserve1.Add(inAmount1)
 
-	return inAmount0, inAmount1, outShares
+	if err := p.LowerTick0.ensureFairTruePrice(); err != nil {
+		return sdk.ZeroInt(), sdk.ZeroInt(), sdk.Coin{}, err
+	}
+
+	if err := p.UpperTick1.ensureFairTruePrice(); err != nil {
+		return sdk.ZeroInt(), sdk.ZeroInt(), sdk.Coin{}, err
+	}
+
+	return inAmount0, inAmount1, outShares, nil
 }
 
 func (p *Pool) GetPoolDenom() string {

x/dex/types/pool_reserves.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"cosmossdk.io/math"
+	math_utils "github.com/neutron-org/neutron/utils/math"
 )
 
 func (p PoolReserves) HasToken() bool {
@@ -50,3 +51,25 @@ func MustNewPoolReserves(
 	}
 	return poolReserves
 }
+
+func (p *PoolReserves) truePriceTaker() math_utils.PrecDec {
+	// If pool is empty don't try to calculate price
+	if !p.HasToken() {
+		return math_utils.ZeroPrecDec()
+	}
+
+	takerAmountIn := math_utils.NewPrecDecFromInt(p.ReservesMakerDenom).Quo(p.PriceTakerToMaker).Ceil()
+
+	return takerAmountIn.QuoInt(p.ReservesMakerDenom)
+}
+
+func (p *PoolReserves) ensureFairTruePrice() error {
+	priceMakerToTaker := math_utils.OnePrecDec().Quo(p.PriceTakerToMaker)
+	priceDiffFromExpected := p.truePriceTaker().Sub(priceMakerToTaker)
+	pctDiff := priceDiffFromExpected.Quo(priceMakerToTaker)
+
+	if pctDiff.GT(MaxTrueMakerSpread) {
+		return ErrInvalidPositionSpread
+	}
+	return nil
+}

x/dex/types/tick_liquidity.go

@@ -4,6 +4,8 @@ import (
 	math_utils "github.com/neutron-org/neutron/utils/math"
 )
 
+var MaxTrueMakerSpread = math_utils.MustNewPrecDecFromStr("0.005")
+
 // NOTE: These methods should be avoided if possible.
 // Generally default to dealing with LimitOrderTranche or PoolReserves explicitly