fix(perps)!: trade limit ratio check in both directions

.github/workflows/pr-title-lint.yml

@@ -2,6 +2,7 @@ name: PR lint
 
 on:
   pull_request:
+    types: [opened, reopened, synchronize, edited]
 
 jobs:
   pr-lint:

CHANGELOG.md

@@ -41,6 +41,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Improvements
+
+* [#858](https://github.com/NibiruChain/nibiru/pull/858) - fix trading limit ratio check; checks in both directions on both quote and base assets
+
 ### Features
 
 * [#852](https://github.com/NibiruChain/nibiru/pull/852) - feat(genesis): add cli command to add pairs at genesis

x/vpool/keeper/keeper.go

@@ -54,10 +54,10 @@ func (k Keeper) SwapBaseForQuote(
 	ctx sdk.Context,
 	pair common.AssetPair,
 	dir types.Direction,
-	baseAssetAmount sdk.Dec,
-	quoteAmountLimit sdk.Dec,
+	baseAmt sdk.Dec,
+	quoteLimit sdk.Dec,
 	skipFluctuationLimitCheck bool,
-) (quoteAssetAmount sdk.Dec, err error) {
+) (quoteAmt sdk.Dec, err error) {
 	if !k.ExistsPool(ctx, pair) {
 		return sdk.Dec{}, types.ErrPairNotSupported
 	}
@@ -66,7 +66,7 @@ func (k Keeper) SwapBaseForQuote(
 		return sdk.Dec{}, types.ErrNoValidPrice.Wrapf("%s", pair.String())
 	}
 
-	if baseAssetAmount.IsZero() {
+	if baseAmt.IsZero() {
 		return sdk.ZeroDec(), nil
 	}
 
@@ -75,39 +75,50 @@ func (k Keeper) SwapBaseForQuote(
 		return sdk.Dec{}, err
 	}
 
-	if dir == types.Direction_REMOVE_FROM_POOL && !pool.HasEnoughBaseReserve(baseAssetAmount) {
+	if !pool.HasEnoughBaseReserve(baseAmt) {
 		return sdk.Dec{}, types.ErrOverTradingLimit
 	}
 
-	quoteAssetAmount, err = pool.GetQuoteAmountByBaseAmount(dir, baseAssetAmount)
+	quoteAmt, err = pool.GetQuoteAmountByBaseAmount(dir, baseAmt)
 	if err != nil {
 		return sdk.Dec{}, err
 	}
 
-	if !quoteAmountLimit.IsZero() {
+	if !pool.HasEnoughQuoteReserve(quoteAmt) {
+		// in reality, this if statement should never run because a perturbation in quote reserve assets
+		// greater than trading limit ratio would have happened when checking for a perturbation in
+		// base assets, due to x*y=k
+		//
+		// e.g. a 10% change in quote asset reserves would have triggered a >10% change in
+		// base asset reserves
+		return sdk.Dec{}, types.ErrOverTradingLimit.Wrapf(
+			"quote amount %s is over trading limit", quoteAmt)
+	}
+
+	if !quoteLimit.IsZero() {
 		// if going long and the base amount retrieved from the pool is less than the limit
-		if dir == types.Direction_ADD_TO_POOL && quoteAssetAmount.LT(quoteAmountLimit) {
+		if dir == types.Direction_ADD_TO_POOL && quoteAmt.LT(quoteLimit) {
 			return sdk.Dec{}, types.ErrAssetFailsUserLimit.Wrapf(
 				"quote amount (%s) is less than selected limit (%s)",
-				quoteAssetAmount.String(),
-				quoteAmountLimit.String(),
+				quoteAmt.String(),
+				quoteLimit.String(),
 			)
 			// if going short and the base amount retrieved from the pool is greater than the limit
-		} else if dir == types.Direction_REMOVE_FROM_POOL && quoteAssetAmount.GT(quoteAmountLimit) {
+		} else if dir == types.Direction_REMOVE_FROM_POOL && quoteAmt.GT(quoteLimit) {
 			return sdk.Dec{}, types.ErrAssetFailsUserLimit.Wrapf(
 				"quote amount (%s) is greater than selected limit (%s)",
-				quoteAssetAmount.String(),
-				quoteAmountLimit.String(),
+				quoteAmt.String(),
+				quoteLimit.String(),
 			)
 		}
 	}
 
 	if dir == types.Direction_ADD_TO_POOL {
-		pool.IncreaseBaseAssetReserve(baseAssetAmount)
-		pool.DecreaseQuoteAssetReserve(quoteAssetAmount)
+		pool.IncreaseBaseAssetReserve(baseAmt)
+		pool.DecreaseQuoteAssetReserve(quoteAmt)
 	} else if dir == types.Direction_REMOVE_FROM_POOL {
-		pool.DecreaseBaseAssetReserve(baseAssetAmount)
-		pool.IncreaseQuoteAssetReserve(quoteAssetAmount)
+		pool.DecreaseBaseAssetReserve(baseAmt)
+		pool.IncreaseQuoteAssetReserve(quoteAmt)
 	}
 
 	if err = k.updatePool(ctx, pool, skipFluctuationLimitCheck); err != nil {
@@ -127,10 +138,10 @@ func (k Keeper) SwapBaseForQuote(
 		return sdk.Dec{}, err
 	}
 
-	return quoteAssetAmount, ctx.EventManager().EmitTypedEvent(&types.SwapBaseForQuoteEvent{
+	return quoteAmt, ctx.EventManager().EmitTypedEvent(&types.SwapBaseForQuoteEvent{
 		Pair:        pair.String(),
-		QuoteAmount: quoteAssetAmount,
-		BaseAmount:  baseAssetAmount,
+		QuoteAmount: quoteAmt,
+		BaseAmount:  baseAmt,
 	})
 }
 
@@ -155,10 +166,10 @@ func (k Keeper) SwapQuoteForBase(
 	ctx sdk.Context,
 	pair common.AssetPair,
 	dir types.Direction,
-	quoteAssetAmount sdk.Dec,
-	baseAmountLimit sdk.Dec,
+	quoteAmt sdk.Dec,
+	baseLimit sdk.Dec,
 	skipFluctuationLimitCheck bool,
-) (baseAssetAmount sdk.Dec, err error) {
+) (baseAmt sdk.Dec, err error) {
 	if !k.ExistsPool(ctx, pair) {
 		return sdk.Dec{}, types.ErrPairNotSupported
 	}
@@ -167,7 +178,7 @@ func (k Keeper) SwapQuoteForBase(
 		return sdk.Dec{}, types.ErrNoValidPrice.Wrapf("%s", pair.String())
 	}
 
-	if quoteAssetAmount.IsZero() {
+	if quoteAmt.IsZero() {
 		return sdk.ZeroDec(), nil
 	}
 
@@ -176,40 +187,53 @@ func (k Keeper) SwapQuoteForBase(
 		return sdk.Dec{}, err
 	}
 
-	if dir == types.Direction_REMOVE_FROM_POOL && !pool.HasEnoughQuoteReserve(quoteAssetAmount) {
-		return sdk.Dec{}, types.ErrOverTradingLimit
+	// check trade limit ratio on quote in either direction
+	if !pool.HasEnoughQuoteReserve(quoteAmt) {
+		return sdk.Dec{}, types.ErrOverTradingLimit.Wrapf(
+			"quote amount %s is over trading limit", quoteAmt)
 	}
 
-	baseAssetAmount, err = pool.GetBaseAmountByQuoteAmount(dir, quoteAssetAmount)
+	baseAmt, err = pool.GetBaseAmountByQuoteAmount(dir, quoteAmt)
 	if err != nil {
 		return sdk.Dec{}, err
 	}
 
+	if !pool.HasEnoughBaseReserve(baseAmt) {
+		// in reality, this if statement should never run because a perturbation in base reserve assets
+		// greater than trading limit ratio would have happened when checking for a perturbation in
+		// quote assets, due to x*y=k
+		//
+		// e.g. a 10% change in base asset reserves would have triggered a >10% change in
+		// quote asset reserves
+		return sdk.Dec{}, types.ErrOverTradingLimit.Wrapf(
+			"base amount %s is over trading limit", baseAmt)
+	}
+
 	// check if base asset limit is violated
-	if !baseAmountLimit.IsZero() {
+	if !baseLimit.IsZero() {
 		// if going long and the base amount retrieved from the pool is less than the limit
-		if dir == types.Direction_ADD_TO_POOL && baseAssetAmount.LT(baseAmountLimit) {
+		if dir == types.Direction_ADD_TO_POOL && baseAmt.LT(baseLimit) {
 			return sdk.Dec{}, types.ErrAssetFailsUserLimit.Wrapf(
 				"base amount (%s) is less than selected limit (%s)",
-				baseAssetAmount.String(),
-				baseAmountLimit.String(),
+				baseAmt.String(),
+				baseLimit.String(),
 			)
 			// if going short and the base amount retrieved from the pool is greater than the limit
-		} else if dir == types.Direction_REMOVE_FROM_POOL && baseAssetAmount.GT(baseAmountLimit) {
+		} else if dir == types.Direction_REMOVE_FROM_POOL && baseAmt.GT(baseLimit) {
 			return sdk.Dec{}, types.ErrAssetFailsUserLimit.Wrapf(
 				"base amount (%s) is greater than selected limit (%s)",
-				baseAssetAmount.String(),
-				baseAmountLimit.String(),
+				baseAmt.String(),
+				baseLimit.String(),
 			)
 		}
 	}
 
 	if dir == types.Direction_ADD_TO_POOL {
-		pool.DecreaseBaseAssetReserve(baseAssetAmount)
-		pool.IncreaseQuoteAssetReserve(quoteAssetAmount)
+		pool.DecreaseBaseAssetReserve(baseAmt)
+		pool.IncreaseQuoteAssetReserve(quoteAmt)
 	} else if dir == types.Direction_REMOVE_FROM_POOL {
-		pool.IncreaseBaseAssetReserve(baseAssetAmount)
-		pool.DecreaseQuoteAssetReserve(quoteAssetAmount)
+		pool.IncreaseBaseAssetReserve(baseAmt)
+		pool.DecreaseQuoteAssetReserve(quoteAmt)
 	}
 
 	if err = k.updatePool(ctx, pool, skipFluctuationLimitCheck); err != nil {
@@ -220,6 +244,7 @@ func (k Keeper) SwapQuoteForBase(
 	if err != nil {
 		return sdk.Dec{}, err
 	}
+
 	if err := ctx.EventManager().EmitTypedEvent(&types.MarkPriceChanged{
 		Pair:      pair.String(),
 		Price:     spotPrice,
@@ -228,10 +253,10 @@ func (k Keeper) SwapQuoteForBase(
 		return sdk.Dec{}, err
 	}
 
-	return baseAssetAmount, ctx.EventManager().EmitTypedEvent(&types.SwapQuoteForBaseEvent{
+	return baseAmt, ctx.EventManager().EmitTypedEvent(&types.SwapQuoteForBaseEvent{
 		Pair:        pair.String(),
-		QuoteAmount: quoteAssetAmount,
-		BaseAmount:  baseAssetAmount,
+		QuoteAmount: quoteAmt,
+		BaseAmount:  baseAmt,
 	})
 }
 

x/vpool/keeper/keeper_test.go

@@ -95,11 +95,21 @@ func TestSwapQuoteForBase(t *testing.T) {
 			expectedErr: types.ErrAssetFailsUserLimit,
 		},
 		{
-			name:                      "quote input bigger than trade limit ratio",
+			name:                      "over trading limit when removing quote",
 			pair:                      common.PairBTCStable,
 			direction:                 types.Direction_REMOVE_FROM_POOL,
-			quoteAmount:               sdk.NewDec(10_000_000),
-			baseLimit:                 sdk.NewDec(10),
+			quoteAmount:               sdk.NewDec(9_000_001),
+			baseLimit:                 sdk.ZeroDec(),
+			skipFluctuationLimitCheck: false,
+
+			expectedErr: types.ErrOverTradingLimit,
+		},
+		{
+			name:                      "over trading limit when adding quote",
+			pair:                      common.PairBTCStable,
+			direction:                 types.Direction_ADD_TO_POOL,
+			quoteAmount:               sdk.NewDec(9_000_001),
+			baseLimit:                 sdk.ZeroDec(),
 			skipFluctuationLimitCheck: false,
 
 			expectedErr: types.ErrOverTradingLimit,
@@ -282,11 +292,21 @@ func TestSwapBaseForQuote(t *testing.T) {
 			expectedErr: types.ErrAssetFailsUserLimit,
 		},
 		{
-			name:                      "base input bigger than trade limit ratio",
+			name:                      "over trading limit when removing base",
 			pair:                      common.PairBTCStable,
 			direction:                 types.Direction_REMOVE_FROM_POOL,
 			baseAmt:                   sdk.NewDec(4_500_001),
-			quoteLimit:                sdk.NewDec(10),
+			quoteLimit:                sdk.ZeroDec(),
+			skipFluctuationLimitCheck: false,
+
+			expectedErr: types.ErrOverTradingLimit,
+		},
+		{
+			name:                      "over trading limit when adding base",
+			pair:                      common.PairBTCStable,
+			direction:                 types.Direction_ADD_TO_POOL,
+			baseAmt:                   sdk.NewDec(4_500_001),
+			quoteLimit:                sdk.ZeroDec(),
 			skipFluctuationLimitCheck: false,
 
 			expectedErr: types.ErrOverTradingLimit,