Skip to content

Commit

Permalink
fix(perps)!: trade limit ratio check in both directions (#858)
Browse files Browse the repository at this point in the history 
* feat!: make quote trade limit ratio check in either direction

* feat!: add base trade limit check

* fix: check base trading limit ratio in both directions

* test: trading limit ratio

* docs: clarify why base trading limit would never run

* feat!: trading limit ratio check in both directions

* feat!: add quote trade limit check

* fix: error message

* refactor: baseAssetAmount -> baseAmt

* refactor: quoteAssetAmount -> quoteAmt

* refactor: shorten variable names

* Update CHANGELOG.md

* test: trading limit ratio check
  • Loading branch information
@NibiruHeisenberg
NibiruHeisenberg authored Aug 30, 2022
1 parent 5851a55 commit 3b962e4
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 46 deletions.
1 change: 1 addition & 0 deletions .github/workflows/pr-title-lint.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ name: PR lint

on:
pull_request:
types: [opened, reopened, synchronize, edited]

jobs:
pr-lint:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
107 changes: 66 additions & 41 deletions x/vpool/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}

Expand All @@ -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 {
Expand All @@ -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,
})
}

Expand All @@ -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
}
Expand All @@ -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
}

Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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,
})
}

Expand Down
30 changes: 25 additions & 5 deletions x/vpool/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3b962e4

Please sign in to comment.