feat(perp): close position #511
Conversation
# Conflicts: # proto/perp/v1/tx.proto # x/perp/types/tx.pb.go
| if err != nil { | ||
| return nil, sdkerrors.Wrap(vpooltypes.ErrClosingPosition, err.Error()) |
There was a problem hiding this comment.
was curious why we are removing the wrap?
There was a problem hiding this comment.
It breaks module boundary responsibilities for the x/perp module to throw a vpool error type.
| if err != nil { | ||
| return nil, sdkerrors.Wrap(vpooltypes.ErrClosingPosition, err.Error()) |
There was a problem hiding this comment.
It breaks module boundary responsibilities for the x/perp module to throw a vpool error type.
| return err | ||
| } | ||
|
|
||
| currentOpenNotional, _, err := k.getPositionNotionalAndUnrealizedPnL(ctx, *position, types.PnLCalcOption_SPOT_PRICE) |
There was a problem hiding this comment.
| currentOpenNotional, _, err := k.getPositionNotionalAndUnrealizedPnL(ctx, *position, types.PnLCalcOption_SPOT_PRICE) | |
| currentPositionNotional, _, err := k.getPositionNotionalAndUnrealizedPnL(ctx, *position, types.PnLCalcOption_SPOT_PRICE) |
| if err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
We need the check over fluctuation limit.
NibiruHeisenberg
requested review from
kai0x,
NibiruHeisenberg,
AgentSmithMatrix and
Unique-Divine
x/perp/keeper/perp_test.go
Outdated
| ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1). | ||
| WithBlockTime(ctx.BlockTime().Add(1 * time.Minute)) | ||
|
|
||
| _, err = nibiruApp.PerpKeeper.ClosePosition(ctx, pair, alice) |
There was a problem hiding this comment.
Can you also add assertions on the positionResp so that we can verify the funding payment and bad debt?
There was a problem hiding this comment.
@godismercilex if you can add this little assertions.
There was a problem hiding this comment.
I feel like adding exact calculation assertions is redundant since ClosePosition is a subset of OpenPosition with no custom functionality, those assertions should be covered in the other functions that close position calls, I've anyways added some basic coverage on the expected output, not pure math.
|
I opened issue #603 that is related to this one in order to merge this issue. |
NibiruHeisenberg
added
CLI
type: tests
No description provided.