Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(perp): Block users from opening infinite leverage positions #776

Merged
merged 9 commits into from
Aug 3, 2022
Merged

fix(perp): Block users from opening infinite leverage positions #776

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3ede370
feat: Add check of margin ratio when opening a new position
Aug 1, 2022
f12e289
Merge branch 'master' into mat/fix-infinite-leverage
matthiasmatt Aug 1, 2022
90f84a8
fix: Update changelog
Aug 1, 2022
b5907b8
merge: Merge with remote
Aug 1, 2022
be04b4b
Merge branch 'master' into mat/fix-infinite-leverage
matthiasmatt Aug 2, 2022
cb80922
fix: Fix changelog
Aug 2, 2022
dc9a1e5
Merge branch 'mat/fix-infinite-leverage' of github.com:NibiruChain/ni…
Aug 3, 2022
c469645
fix: Remove redundent bad debt test in liquidate_test
Aug 3, 2022
1f5947e
merge: Merge with remote
Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Bug Fixes

* [#746](https://github.com/NibiruChain/nibiru/pull/746) - Pin cosmos-sdk version to v0.45 for proto generation.
* [#776](https://github.com/NibiruChain/nibiru/pull/776) - Fix a bug where the user could open infinite leverage positions
matthiasmatt marked this conversation as resolved.
Show resolved Hide resolved

## [v0.10.0](https://github.com/NibiruChain/nibiru/releases/tag/v0.10.0) - 2022-07-18

Expand Down
4 changes: 2 additions & 2 deletions x/perp/keeper/clearing_house.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (k Keeper) afterPositionUpdate(
return fmt.Errorf("bad debt must be zero to prevent attacker from leveraging it")
}

if !isNewPosition && !positionResp.Position.Size_.IsZero() {
if !positionResp.Position.Size_.IsZero() {
marginRatio, err := k.GetMarginRatio(
ctx,
*positionResp.Position,
Expand All @@ -142,7 +142,7 @@ func (k Keeper) afterPositionUpdate(

maintenanceMarginRatio := k.VpoolKeeper.GetMaintenanceMarginRatio(ctx, pair)
if err = requireMoreMarginRatio(marginRatio, maintenanceMarginRatio, true); err != nil {
return err
return types.ErrMarginRatioTooLow
}
}

Expand Down
23 changes: 21 additions & 2 deletions x/perp/keeper/clearing_house_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package keeper_test

import (
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -325,7 +324,7 @@ func TestOpenPositionError(t *testing.T) {
margin: sdk.NewInt(1),
leverage: sdk.NewDec(1),
baseLimit: sdk.ZeroDec(),
expectedErr: fmt.Errorf("margin ratio did not meet criteria"),
expectedErr: types.ErrMarginRatioTooLow,
},
{
name: "new long position not over base limit",
Expand Down Expand Up @@ -367,6 +366,26 @@ func TestOpenPositionError(t *testing.T) {
baseLimit: sdk.NewDec(10_000),
expectedErr: types.ErrLeverageIsZero,
},
{
name: "leverage amount is too high - SELL",
traderFunds: sdk.NewCoins(sdk.NewInt64Coin(common.DenomStable, 1020)),
initialPosition: nil,
side: types.Side_SELL,
margin: sdk.NewInt(100),
leverage: sdk.NewDec(100),
baseLimit: sdk.NewDec(11_000),
expectedErr: types.ErrMarginRatioTooLow,
},
{
name: "leverage amount is too high - BUY",
traderFunds: sdk.NewCoins(sdk.NewInt64Coin(common.DenomStable, 1020)),
initialPosition: nil,
side: types.Side_BUY,
margin: sdk.NewInt(100),
leverage: sdk.NewDec(100),
baseLimit: sdk.NewDec(0),
expectedErr: types.ErrMarginRatioTooLow,
},
}

for _, testCase := range testCases {
Expand Down
20 changes: 12 additions & 8 deletions x/perp/keeper/liquidate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,34 +81,34 @@ func TestExecuteFullLiquidation(t *testing.T) {
*/
positionSide: types.Side_BUY,
quoteAmount: sdk.NewInt(50),
leverage: sdk.MustNewDecFromStr("10000"),
leverage: sdk.MustNewDecFromStr("10"),
baseAssetLimit: sdk.ZeroDec(),
liquidationFee: sdk.MustNewDecFromStr("0.1"),
traderFunds: sdk.NewInt64Coin("NUSD", 1150),
// feeToLiquidator
// = positionResp.ExchangedNotionalValue * liquidationFee / 2
// = 500_000 * 0.1 / 2 = 25_000
expectedLiquidatorBalance: sdk.NewInt64Coin("NUSD", 25_000),
// = 500 * 0.1 / 2 = 25
expectedLiquidatorBalance: sdk.NewInt64Coin("NUSD", 25),
// startingBalance = 1_000_000
// perpEFBalance = startingBalance + openPositionDelta + liquidateDelta
expectedPerpEFBalance: sdk.NewInt64Coin("NUSD", 975_550),
expectedPerpEFBalance: sdk.NewInt64Coin("NUSD", 1000025),
expectedBadDebt: sdk.MustNewDecFromStr("24950"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you double-check this test? How can the bad debt be positive but the PerpEF balance go up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no bad debt created there. Changing the liquidation ratio from 100 to 10 removed it. To create bad debt I need to change the mark price after opening the position so the margin ratio is below the liquidation fee.

In this case, that means mocking the vpool GetSpotPrice function, which is exactly what's being done in the liquidate_unit_test.go functions

Should we remove those 2 bad debt tests? Since they are already checked there: liquidate_unit_test.go:374 / TestLiquidateIntoFullLiquidationWithBadDebt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, yeah in that case these two tests are redundant since TestLiquidateIntoFullLiquidationWithBadDebt should be covering them.

},
"happy path - bad debt, short": {
// Same as above case but for shorts
positionSide: types.Side_SELL,
quoteAmount: sdk.NewInt(50),
leverage: sdk.MustNewDecFromStr("10000"),
leverage: sdk.MustNewDecFromStr("10"),
baseAssetLimit: sdk.ZeroDec(),
liquidationFee: sdk.MustNewDecFromStr("0.1"),
traderFunds: sdk.NewInt64Coin("NUSD", 1150),
// feeToLiquidator
// = positionResp.ExchangedNotionalValue * liquidationFee / 2
// = 500_000 * 0.1 / 2 = 25_000
expectedLiquidatorBalance: sdk.NewInt64Coin("NUSD", 25_000),
// = 500 * 0.1 / 2 = 25
expectedLiquidatorBalance: sdk.NewInt64Coin("NUSD", 25),
// startingBalance = 1_000_000
// perpEFBalance = startingBalance + openPositionDelta + liquidateDelta
expectedPerpEFBalance: sdk.NewInt64Coin("NUSD", 975_550),
expectedPerpEFBalance: sdk.NewInt64Coin("NUSD", 1000025),
expectedBadDebt: sdk.MustNewDecFromStr("24950"),
},
}
Expand Down Expand Up @@ -156,6 +156,10 @@ func TestExecuteFullLiquidation(t *testing.T) {
sdk.NewCoins(tc.traderFunds))
require.NoError(t, err)

t.Log("increment block height and time for TWAP calculation")
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1).
WithBlockTime(time.Now().Add(time.Minute))

t.Log("Open position")
positionResp, err := nibiruApp.PerpKeeper.OpenPosition(
ctx, tokenPair, tc.positionSide, traderAddr, tc.quoteAmount, tc.leverage, tc.baseAssetLimit)
Expand Down
4 changes: 4 additions & 0 deletions x/perp/keeper/msg_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ func TestMsgServerOpenPosition(t *testing.T) {
require.NoError(t, simapp.FundAccount(app.BankKeeper, ctx, traderAddr, tc.traderFunds))
}

t.Log("increment block height and time for TWAP calculation")
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1).
WithBlockTime(time.Now().Add(time.Minute))

resp, err := msgServer.OpenPosition(sdk.WrapSDKContext(ctx), &types.MsgOpenPosition{
Sender: tc.sender,
TokenPair: tc.pair,
Expand Down
2 changes: 2 additions & 0 deletions x/perp/keeper/perp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ func TestKeeperClosePosition(t *testing.T) {
)

t.Log("open position for alice - long")
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1).WithBlockTime(time.Now().Add(time.Minute))

alice := sample.AccAddress()
err := simapp.FundAccount(nibiruApp.BankKeeper, ctx, alice,
Expand Down Expand Up @@ -206,6 +207,7 @@ func TestKeeperClosePosition(t *testing.T) {
bobQuote := sdk.NewInt(60)
bobLeverage := sdk.NewDec(10)
bobBaseLimit := sdk.NewDec(150)

_, err = nibiruApp.PerpKeeper.OpenPosition(
ctx, pair, bobSide, bob, bobQuote, bobLeverage, bobBaseLimit)
require.NoError(t, err)
Expand Down
1 change: 1 addition & 0 deletions x/perp/types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var (
ErrFailedRemoveMarginCanCauseBadDebt = sdkerrors.Register(ModuleName, 7, "failed to remove margin; position would have bad debt if removed")
ErrQuoteAmountIsZero = sdkerrors.Register(ModuleName, 8, "quote amount cannot be zero")
ErrLeverageIsZero = sdkerrors.Register(ModuleName, 9, "leverage cannot be zero")
ErrMarginRatioTooLow = sdkerrors.Register(ModuleName, 10, "margin ratio did not meet maintenance margin ratio")
)

func ZeroPosition(ctx sdk.Context, tokenPair common.AssetPair, traderAddr sdk.AccAddress) *Position {
Expand Down