refactor: Add guard clauses to OpenPosition

CHANGELOG.md

@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Improvements
 
 - [#715](https://github.com/NibiruChain/nibiru/pull/715) - remove redundant perp.Keeper.SetPosition parameters
+- [#718](https://github.com/NibiruChain/nibiru/pull/718) - add guard clauses on OpenPosition (leverage and quote amount != 0)
 
 ### API Breaking
 

x/perp/keeper/clearing_house.go

@@ -11,7 +11,22 @@ import (
 	vpooltypes "github.com/NibiruChain/nibiru/x/vpool/types"
 )
 
-// TODO test: OpenPosition | https://github.com/NibiruChain/nibiru/issues/299
+/*
+OpenPosition opens a position on the selected pair.
+
+args:
+  - ctx: cosmos-sdk context
+  - pair: the pair where the position will be opened
+  - side: whether the position in the BUY or SELL direction
+  - traderAddr: the address of the trader who opens the position
+  - quoteAssetAmount: the amount of quote asset
+  - leverage: the amount of leverage to take, as sdk.Dec
+  - baseAmtLimit: the limit on the base asset amount to make sure the trader doesn't get screwed, in base asset units
+
+ret:
+  - positionResp: contains the result of the open position and the new position
+  - err: error
+*/
 func (k Keeper) OpenPosition(
 	ctx sdk.Context,
 	pair common.AssetPair,
@@ -21,16 +36,16 @@ func (k Keeper) OpenPosition(
 	leverage sdk.Dec,
 	baseAmtLimit sdk.Dec,
 ) (positionResp *types.PositionResp, err error) {
-	if err = k.requireVpool(ctx, pair); err != nil {
+	err = k.checkOpenPositionRequirements(ctx, pair, quoteAssetAmount, leverage)
+	if err != nil {
 		return nil, err
 	}
 
 	// require params
 	params := k.GetParams(ctx)
-	// TODO: missing checks
 
 	position, err := k.PositionsState(ctx).Get(pair, traderAddr)
-	var isNewPosition bool = errors.Is(err, types.ErrPositionNotFound)
+	isNewPosition := errors.Is(err, types.ErrPositionNotFound)
 	if isNewPosition {
 		position = types.ZeroPosition(ctx, pair, traderAddr)
 		k.PositionsState(ctx).Set(position)
@@ -75,6 +90,28 @@ func (k Keeper) OpenPosition(
 	return positionResp, nil
 }
 
+// checkOpenPositionRequirements checks the minimum requirements to open a position.
+//
+// - Checks that the VPool exists.
+// - Checks that quote asset is not zero.
+// - Checks that leverage is not zero.
+//
+func (k Keeper) checkOpenPositionRequirements(ctx sdk.Context, pair common.AssetPair, quoteAssetAmount sdk.Int, leverage sdk.Dec) error {
+	if err := k.requireVpool(ctx, pair); err != nil {
+		return err
+	}
+
+	if quoteAssetAmount.IsZero() {
+		return types.ErrQuoteAmountIsZero
+	}
+
+	if leverage.IsZero() {
+		return types.ErrLeverageIsZero
+	}
+
+	return nil
+}
+
 // afterPositionUpdate is called when a position has been updated.
 func (k Keeper) afterPositionUpdate(
 	ctx sdk.Context,
@@ -606,7 +643,7 @@ func (k Keeper) closePositionEntirely(
 	return positionResp, nil
 }
 
-/**
+/*
 ClosePosition closes a position entirely and transfers the remaining margin back to the user.
 Errors if the position has bad debt.
 

x/perp/keeper/clearing_house_integration_test.go

@@ -346,6 +346,26 @@ func TestOpenPositionError(t *testing.T) {
 			baseLimit:       sdk.NewDec(10_000),
 			expectedErr:     vpooltypes.ErrAssetFailsUserLimit,
 		},
+		{
+			name:            "quote asset amount is zero",
+			traderFunds:     sdk.NewCoins(sdk.NewInt64Coin(common.DenomStable, 1020)),
+			initialPosition: nil,
+			side:            types.Side_SELL,
+			margin:          sdk.NewInt(0),
+			leverage:        sdk.NewDec(10),
+			baseLimit:       sdk.NewDec(10_000),
+			expectedErr:     types.ErrQuoteAmountIsZero,
+		},
+		{
+			name:            "leverage amount is zero",
+			traderFunds:     sdk.NewCoins(sdk.NewInt64Coin(common.DenomStable, 1020)),
+			initialPosition: nil,
+			side:            types.Side_SELL,
+			margin:          sdk.NewInt(1000),
+			leverage:        sdk.NewDec(0),
+			baseLimit:       sdk.NewDec(10_000),
+			expectedErr:     types.ErrLeverageIsZero,
+		},
 	}
 
 	for _, testCase := range testCases {

x/perp/keeper/liquidate_test.go

@@ -16,110 +16,6 @@ import (
 	"github.com/NibiruChain/nibiru/x/testutil/testapp"
 )
 
-func TestExecuteFullLiquidation_EmptyPosition(t *testing.T) {
-	testCases := []struct {
-		name           string
-		side           types.Side
-		quote          sdk.Int
-		leverage       sdk.Dec
-		baseLimit      sdk.Dec
-		liquidationFee sdk.Dec
-		traderFunds    sdk.Coin
-	}{
-		{
-			name:           "liquidateEmptyPositionBUY",
-			side:           types.Side_BUY,
-			quote:          sdk.NewInt(0),
-			leverage:       sdk.OneDec(),
-			baseLimit:      sdk.ZeroDec(),
-			liquidationFee: sdk.MustNewDecFromStr("0.1"),
-			traderFunds:    sdk.NewInt64Coin("NUSD", 60),
-		},
-		{
-			name:           "liquidateEmptyPositionSELL",
-			side:           types.Side_SELL,
-			quote:          sdk.NewInt(0),
-			leverage:       sdk.OneDec(),
-			baseLimit:      sdk.ZeroDec(),
-			liquidationFee: sdk.MustNewDecFromStr("0.1"),
-			traderFunds:    sdk.NewInt64Coin("NUSD", 60),
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			nibiruApp, ctx := testapp.NewNibiruAppAndContext(true)
-			pair := common.MustNewAssetPair("BTC:NUSD")
-
-			t.Log("Set vpool defined by pair on VpoolKeeper")
-			vpoolKeeper := &nibiruApp.VpoolKeeper
-			vpoolKeeper.CreatePool(
-				ctx,
-				pair,
-				/* tradeLimitRatio */ sdk.MustNewDecFromStr("0.9"),
-				/* quoteAssetReserves */ sdk.NewDec(10_000_000),
-				/* baseAssetReserves */ sdk.NewDec(5_000_000),
-				/* fluctuationLimitRatio */ sdk.MustNewDecFromStr("1"),
-				/* maxOracleSpreadRatio */ sdk.MustNewDecFromStr("0.1"),
-			)
-			require.True(t, vpoolKeeper.ExistsPool(ctx, pair))
-
-			t.Log("Set vpool defined by pair on PerpKeeper")
-			perpKeeper := &nibiruApp.PerpKeeper
-			params := types.DefaultParams()
-
-			perpKeeper.SetParams(ctx, types.NewParams(
-				params.Stopped,
-				params.MaintenanceMarginRatio,
-				params.FeePoolFeeRatio,
-				params.EcosystemFundFeeRatio,
-				tc.liquidationFee,
-				params.PartialLiquidationRatio,
-				"hour",
-				15*time.Minute,
-			))
-
-			perpKeeper.PairMetadataState(ctx).Set(&types.PairMetadata{
-				Pair:                       pair,
-				CumulativePremiumFractions: []sdk.Dec{sdk.OneDec()},
-			})
-
-			t.Log("Fund trader account with sufficient quote")
-			var err error
-			trader := sample.AccAddress()
-			err = simapp.FundAccount(nibiruApp.BankKeeper, ctx, trader,
-				sdk.NewCoins(tc.traderFunds))
-			require.NoError(t, err)
-
-			t.Log("Open position")
-			positionResp, err := nibiruApp.PerpKeeper.OpenPosition(
-				ctx, pair, tc.side, trader, tc.quote, tc.leverage, tc.baseLimit)
-			require.NoError(t, err)
-
-			t.Log("Artificially populate Vault and PerpEF to prevent BankKeeper errors")
-			startingModuleFunds := sdk.NewCoins(sdk.NewInt64Coin(
-				pair.GetQuoteTokenDenom(), 1_000_000))
-			assert.NoError(t, simapp.FundModuleAccount(
-				nibiruApp.BankKeeper, ctx, types.VaultModuleAccount, startingModuleFunds))
-			assert.NoError(t, simapp.FundModuleAccount(
-				nibiruApp.BankKeeper, ctx, types.PerpEFModuleAccount, startingModuleFunds))
-
-			t.Log("Liquidate the position")
-			liquidator := sample.AccAddress()
-			_, err = nibiruApp.PerpKeeper.ExecuteFullLiquidation(ctx, liquidator, positionResp.Position)
-
-			require.Error(t, err)
-
-			// No change in the position
-			newPosition, _ := nibiruApp.PerpKeeper.PositionsState(ctx).Get(pair, trader)
-			assert.Equal(t, positionResp.Position.Size_, newPosition.Size_)
-			assert.Equal(t, positionResp.Position.Margin, newPosition.Margin)
-			assert.Equal(t, positionResp.Position.OpenNotional, newPosition.OpenNotional)
-		})
-	}
-}
-
 func TestExecuteFullLiquidation(t *testing.T) {
 	// constants for this suite
 	tokenPair := common.MustNewAssetPair("BTC:NUSD")
@@ -320,99 +216,6 @@ func TestExecuteFullLiquidation(t *testing.T) {
 	}
 }
 
-func TestExecutePartialLiquidation_EmptyPosition(t *testing.T) {
-	testCases := []struct {
-		name           string
-		side           types.Side
-		quote          sdk.Int
-		leverage       sdk.Dec
-		baseLimit      sdk.Dec
-		liquidationFee sdk.Dec
-		traderFunds    sdk.Coin
-	}{
-		{
-			name:           "liquidateEmptyPositionBUY",
-			side:           types.Side_BUY,
-			quote:          sdk.NewInt(0),
-			leverage:       sdk.OneDec(),
-			baseLimit:      sdk.ZeroDec(),
-			liquidationFee: sdk.MustNewDecFromStr("0.1"),
-			traderFunds:    sdk.NewInt64Coin("yyy", 60),
-		},
-		{
-			name:           "liquidateEmptyPositionSELL",
-			side:           types.Side_SELL,
-			quote:          sdk.NewInt(0),
-			leverage:       sdk.OneDec(),
-			baseLimit:      sdk.ZeroDec(),
-			liquidationFee: sdk.MustNewDecFromStr("0.1"),
-			traderFunds:    sdk.NewInt64Coin("yyy", 60),
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			t.Log("Initialize keepers, pair, and liquidator")
-			nibiruApp, ctx := testapp.NewNibiruAppAndContext(true)
-			pair := common.MustNewAssetPair("xxx:yyy")
-			vpoolKeeper := &nibiruApp.VpoolKeeper
-			perpKeeper := &nibiruApp.PerpKeeper
-
-			t.Log("Create vpool")
-			vpoolKeeper.CreatePool(
-				ctx,
-				pair,
-				/* tradeLimitRatio */ sdk.MustNewDecFromStr("0.9"),
-				/* quoteAssetReserves */ sdk.NewDec(10_000_000),
-				/* baseAssetReserves */ sdk.NewDec(5_000_000),
-				/* fluctuationLimitRatio */ sdk.MustNewDecFromStr("1"),
-				/* maxOracleSpreadRatio */ sdk.MustNewDecFromStr("0.1"),
-			)
-			require.True(t, vpoolKeeper.ExistsPool(ctx, pair))
-
-			t.Log("Set perp params")
-			params := types.DefaultParams()
-			params.LiquidationFeeRatio = tc.liquidationFee
-			perpKeeper.SetParams(ctx, params)
-			perpKeeper.PairMetadataState(ctx).Set(&types.PairMetadata{
-				Pair:                       pair,
-				CumulativePremiumFractions: []sdk.Dec{sdk.ZeroDec()},
-			})
-
-			t.Log("Fund trader account with sufficient quote")
-			trader := sample.AccAddress()
-			require.NoError(t, simapp.FundAccount(nibiruApp.BankKeeper, ctx, trader,
-				sdk.NewCoins(tc.traderFunds)))
-
-			t.Log("Open position")
-			positionResp, err := nibiruApp.PerpKeeper.OpenPosition(
-				ctx, pair, tc.side, trader, tc.quote, tc.leverage, tc.baseLimit)
-			require.NoError(t, err)
-
-			t.Log("Artificially populate Vault and PerpEF to prevent BankKeeper errors")
-			startingModuleFunds := sdk.NewCoins(sdk.NewInt64Coin(
-				pair.GetQuoteTokenDenom(), 1_000_000))
-			assert.NoError(t, simapp.FundModuleAccount(
-				nibiruApp.BankKeeper, ctx, types.VaultModuleAccount, startingModuleFunds))
-			assert.NoError(t, simapp.FundModuleAccount(
-				nibiruApp.BankKeeper, ctx, types.PerpEFModuleAccount, startingModuleFunds))
-
-			t.Log("Liquidate the position")
-			liquidator := sample.AccAddress()
-			_, err = nibiruApp.PerpKeeper.ExecutePartialLiquidation(ctx, liquidator, positionResp.Position)
-
-			require.Error(t, err)
-
-			// No change in the position
-			newPosition, _ := nibiruApp.PerpKeeper.PositionsState(ctx).Get(pair, trader)
-			require.Equal(t, positionResp.Position.Size_, newPosition.Size_)
-			require.Equal(t, positionResp.Position.Margin, newPosition.Margin)
-			require.Equal(t, positionResp.Position.OpenNotional, newPosition.OpenNotional)
-		})
-	}
-}
-
 func TestExecutePartialLiquidation(t *testing.T) {
 	// constants for this suite
 	tokenPair := common.MustNewAssetPair("xxx:yyy")

x/perp/types/types.go

@@ -23,8 +23,9 @@ var (
 	ErrPairNotFound                      = sdkerrors.Register(ModuleName, 3, "pair doesn't have live vpool")
 	ErrPairMetadataNotFound              = sdkerrors.Register(ModuleName, 4, "pair doesn't have metadata")
 	ErrPositionZero                      = sdkerrors.Register(ModuleName, 5, "position is zero")
-	ErrExchangeStopped                   = sdkerrors.Register(ModuleName, 6, "exchange is stopped")
 	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")
 )
 
 func ZeroPosition(ctx sdk.Context, tokenPair common.AssetPair, traderAddr sdk.AccAddress) *Position {