diff --git a/.github/workflows/pr-title-lint.yml b/.github/workflows/pr-title-lint.yml index 1475acf39..9cb426f2b 100644 --- a/.github/workflows/pr-title-lint.yml +++ b/.github/workflows/pr-title-lint.yml @@ -2,6 +2,7 @@ name: PR lint on: pull_request: + types: [opened, reopened, synchronize, edited] jobs: pr-lint: diff --git a/CHANGELOG.md b/CHANGELOG.md index 49d864f23..92ed861ee 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/x/vpool/keeper/keeper.go b/x/vpool/keeper/keeper.go index effb52f91..2200d6cb0 100644 --- a/x/vpool/keeper/keeper.go +++ b/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, }) } diff --git a/x/vpool/keeper/keeper_test.go b/x/vpool/keeper/keeper_test.go index e67a26c0e..da930666c 100644 --- a/x/vpool/keeper/keeper_test.go +++ b/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,