refactor(perp): Organize Keeper methods (#684)

* Reorganize keeper methods * Remove GetPosition and SetPosition * Remove ClearPosition * Remove unused Params state * Remove unnecessary Logger statements * Update CHANGELOG.md Co-authored-by: AgentSmithMatrix <[email protected]>
NibiruChain · Jul 8, 2022 · 28eb87e · 28eb87e
1 parent 712ea3c
commit 28eb87e
Show file tree

Hide file tree

Showing 16 changed files with 775 additions and 881 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,7 +42,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
-### Improvements
-* [#686](https://github.com/NibiruChain/nibiru/pull/686) Add changelog enforcer to github actions.
-
+- [#686](https://github.com/NibiruChain/nibiru/pull/686) Add changelog enforcer to github actions.
 
+- [#686](https://github.com/NibiruChain/nibiru/pull/686) Reorganize PerpKeeper methods
diff --git a/x/perp/keeper/clearing_house.go b/x/perp/keeper/clearing_house.go
@@ -29,11 +29,11 @@ func (k Keeper) OpenPosition(
 	params := k.GetParams(ctx)
 	// TODO: missing checks
 
-	position, err := k.GetPosition(ctx, pair, traderAddr)
+	position, err := k.PositionsState(ctx).Get(pair, traderAddr)
 	var isNewPosition bool = errors.Is(err, types.ErrPositionNotFound)
 	if isNewPosition {
 		position = types.ZeroPosition(ctx, pair, traderAddr)
-		k.SetPosition(ctx, pair, traderAddr, position)
+		k.PositionsState(ctx).Set(pair, traderAddr, position)
 	} else if err != nil && !isNewPosition {
 		return err
 	}
@@ -86,7 +86,7 @@ func (k Keeper) afterPositionUpdate(
 ) (err error) {
 	// update position in state
 	if !positionResp.Position.Size_.IsZero() {
-		k.SetPosition(ctx, pair, traderAddr, positionResp.Position)
+		k.PositionsState(ctx).Set(pair, traderAddr, positionResp.Position)
 	}
 
 	if !isNewPosition && !positionResp.Position.Size_.IsZero() {
@@ -252,119 +252,6 @@ func (k Keeper) increasePosition(
 	return positionResp, nil
 }
 
-// getLatestCumulativePremiumFraction returns the last cumulative premium fraction recorded for the
-// specific pair.
-func (k Keeper) getLatestCumulativePremiumFraction(
-	ctx sdk.Context, pair common.AssetPair,
-) (sdk.Dec, error) {
-	pairMetadata, err := k.PairMetadataState(ctx).Get(pair)
-	if err != nil {
-		k.Logger(ctx).Error(
-			err.Error(),
-			"pair",
-			pair.String(),
-		)
-		return sdk.Dec{}, err
-	}
-	// this should never fail
-	return pairMetadata.CumulativePremiumFractions[len(pairMetadata.CumulativePremiumFractions)-1], nil
-}
-
-/*
-Calculates position notional value and unrealized PnL. Lets the caller pick
-either spot price, TWAP, or ORACLE to use for calculation.
-
-args:
-  - ctx: cosmos-sdk context
-  - position: the trader's position
-  - pnlCalcOption: SPOT or TWAP or ORACLE
-
-Returns:
-  - positionNotional: the position's notional value as sdk.Dec (signed)
-  - unrealizedPnl: the position's unrealized profits and losses (PnL) as sdk.Dec (signed)
-		For LONG positions, this is positionNotional - openNotional
-		For SHORT positions, this is openNotional - positionNotional
-*/
-func (k Keeper) getPositionNotionalAndUnrealizedPnL(
-	ctx sdk.Context,
-	currentPosition types.Position,
-	pnlCalcOption types.PnLCalcOption,
-) (positionNotional sdk.Dec, unrealizedPnL sdk.Dec, err error) {
-	positionSizeAbs := currentPosition.Size_.Abs()
-	if positionSizeAbs.IsZero() {
-		return sdk.ZeroDec(), sdk.ZeroDec(), nil
-	}
-
-	var baseAssetDirection vpooltypes.Direction
-	if currentPosition.Size_.IsPositive() {
-		// LONG
-		baseAssetDirection = vpooltypes.Direction_ADD_TO_POOL
-	} else {
-		// SHORT
-		baseAssetDirection = vpooltypes.Direction_REMOVE_FROM_POOL
-	}
-
-	switch pnlCalcOption {
-	case types.PnLCalcOption_TWAP:
-		positionNotional, err = k.VpoolKeeper.GetBaseAssetTWAP(
-			ctx,
-			currentPosition.Pair,
-			baseAssetDirection,
-			positionSizeAbs,
-			/*lookbackInterval=*/ k.GetParams(ctx).TwapLookbackWindow,
-		)
-		if err != nil {
-			k.Logger(ctx).Error(err.Error(), "calc_option", pnlCalcOption.String())
-			return sdk.ZeroDec(), sdk.ZeroDec(), err
-		}
-	case types.PnLCalcOption_SPOT_PRICE:
-		positionNotional, err = k.VpoolKeeper.GetBaseAssetPrice(
-			ctx,
-			currentPosition.Pair,
-			baseAssetDirection,
-			positionSizeAbs,
-		)
-		if err != nil {
-			k.Logger(ctx).Error(err.Error(), "calc_option", pnlCalcOption.String())
-			return sdk.ZeroDec(), sdk.ZeroDec(), err
-		}
-	case types.PnLCalcOption_ORACLE:
-		oraclePrice, err := k.VpoolKeeper.GetUnderlyingPrice(
-			ctx, currentPosition.Pair)
-		if err != nil {
-			k.Logger(ctx).Error(err.Error(), "calc_option", pnlCalcOption.String())
-			return sdk.ZeroDec(), sdk.ZeroDec(), err
-		}
-		positionNotional = oraclePrice.Mul(positionSizeAbs)
-	default:
-		panic("unrecognized pnl calc option: " + pnlCalcOption.String())
-	}
-
-	if positionNotional.Equal(currentPosition.OpenNotional) {
-		// if position notional and open notional are the same, then early return
-		return positionNotional, sdk.ZeroDec(), nil
-	}
-
-	if currentPosition.Size_.IsPositive() {
-		// LONG
-		unrealizedPnL = positionNotional.Sub(currentPosition.OpenNotional)
-	} else {
-		// SHORT
-		unrealizedPnL = currentPosition.OpenNotional.Sub(positionNotional)
-	}
-
-	k.Logger(ctx).Debug("get_position_notional_and_unrealized_pnl",
-		"position",
-		currentPosition.String(),
-		"position_notional",
-		positionNotional.String(),
-		"unrealized_pnl",
-		unrealizedPnL.String(),
-	)
-
-	return positionNotional, unrealizedPnL, nil
-}
-
 // TODO test: openReversePosition | https://github.com/NibiruChain/nibiru/issues/299
 func (k Keeper) openReversePosition(
 	ctx sdk.Context,
@@ -516,11 +403,6 @@ func (k Keeper) decreasePosition(
 		BlockNumber:                         ctx.BlockHeight(),
 	}
 
-	k.Logger(ctx).Debug("decrease_position",
-		"positionResp",
-		positionResp.String(),
-	)
-
 	return positionResp, nil
 }
 
@@ -628,11 +510,6 @@ func (k Keeper) closeAndOpenReversePosition(
 		positionResp = closePositionResp
 	}
 
-	k.Logger(ctx).Debug("close_and_open_reverse_position",
-		"positionResp",
-		positionResp.String(),
-	)
-
 	return positionResp, nil
 }
 
@@ -720,25 +597,19 @@ func (k Keeper) closePositionEntirely(
 		BlockNumber:                         ctx.BlockHeight(),
 	}
 
-	if err = k.ClearPosition(
-		ctx,
+	if err = k.PositionsState(ctx).Delete(
 		currentPosition.Pair,
 		trader,
 	); err != nil {
 		return nil, err
 	}
 
-	k.Logger(ctx).Debug("close_position_entirely",
-		"positionResp",
-		positionResp.String(),
-	)
-
 	return positionResp, nil
 }
 
 // ClosePosition gets the current position, and calls OpenPosition to open a reverse position with amount equal to the current open notional.
 func (k Keeper) ClosePosition(ctx sdk.Context, pair common.AssetPair, addr sdk.AccAddress) (*types.PositionResp, error) {
-	position, err := k.GetPosition(ctx, pair, addr)
+	position, err := k.PositionsState(ctx).Get(pair, addr)
 	if err != nil {
 		return nil, err
 	}
@@ -818,72 +689,6 @@ func (k Keeper) transferFee(
 	return feeToFeePool.Add(feeToEcosystemFund), nil
 }
 
-/*
-Calculates both position notional value and unrealized PnL based on
-both spot price and TWAP, and lets the caller pick which one based on MAX or MIN.
-
-args:
-  - ctx: cosmos-sdk context
-  - position: the trader's position
-  - pnlPreferenceOption: MAX or MIN
-
-Returns:
-  - positionNotional: the position's notional value as sdk.Dec (signed)
-  - unrealizedPnl: the position's unrealized profits and losses (PnL) as sdk.Dec (signed)
-		For LONG positions, this is positionNotional - openNotional
-		For SHORT positions, this is openNotional - positionNotional
-*/
-func (k Keeper) getPreferencePositionNotionalAndUnrealizedPnL(
-	ctx sdk.Context,
-	position types.Position,
-	pnLPreferenceOption types.PnLPreferenceOption,
-) (positionNotional sdk.Dec, unrealizedPnl sdk.Dec, err error) {
-	spotPositionNotional, spotPricePnl, err := k.getPositionNotionalAndUnrealizedPnL(
-		ctx,
-		position,
-		types.PnLCalcOption_SPOT_PRICE,
-	)
-	if err != nil {
-		k.Logger(ctx).Error(
-			err.Error(),
-			"calc_option",
-			types.PnLCalcOption_SPOT_PRICE.String(),
-			"preference_option",
-			pnLPreferenceOption.String(),
-		)
-		return sdk.Dec{}, sdk.Dec{}, err
-	}
-
-	twapPositionNotional, twapPricePnL, err := k.getPositionNotionalAndUnrealizedPnL(
-		ctx,
-		position,
-		types.PnLCalcOption_TWAP,
-	)
-	if err != nil {
-		k.Logger(ctx).Error(
-			err.Error(),
-			"calc_option",
-			types.PnLCalcOption_TWAP.String(),
-			"preference_option",
-			pnLPreferenceOption.String(),
-		)
-		return sdk.Dec{}, sdk.Dec{}, err
-	}
-
-	switch pnLPreferenceOption {
-	case types.PnLPreferenceOption_MAX:
-		positionNotional = sdk.MaxDec(spotPositionNotional, twapPositionNotional)
-		unrealizedPnl = sdk.MaxDec(spotPricePnl, twapPricePnL)
-	case types.PnLPreferenceOption_MIN:
-		positionNotional = sdk.MinDec(spotPositionNotional, twapPositionNotional)
-		unrealizedPnl = sdk.MinDec(spotPricePnl, twapPricePnL)
-	default:
-		panic("invalid pnl preference option " + pnLPreferenceOption.String())
-	}
-
-	return positionNotional, unrealizedPnl, nil
-}
-
 /*
 Trades quoteAssets in exchange for baseAssets.
 The quote asset is a stablecoin like NUSD.
@@ -920,27 +725,9 @@ func (k Keeper) swapQuoteForBase(
 	baseAmount, err = k.VpoolKeeper.SwapQuoteForBase(
 		ctx, pair, quoteAssetDirection, quoteAssetAmount, baseAssetLimit)
 	if err != nil {
-		k.Logger(ctx).Error(
-			err.Error(),
-			"pair",
-			pair.String(),
-			"side",
-			side.String(),
-			"quoteAssetAmount",
-			quoteAssetAmount.String(),
-			"baseAssetLimit",
-			baseAssetLimit.String(),
-		)
 		return sdk.Dec{}, err
 	}
 
-	k.Logger(ctx).Debug("swap_quote_for_base",
-		"side",
-		side.String(),
-		"baseAmt",
-		baseAmount.Abs(),
-	)
-
 	if side == types.Side_BUY {
 		return baseAmount, nil
 	} else {