From 4a88694810a5ccd71c86a325149a3e815df29aa1 Mon Sep 17 00:00:00 2001 From: codchen Date: Thu, 20 Oct 2022 12:24:01 +0800 Subject: [PATCH 1/2] Fix cancel tests --- .../msg_server_cancel_orders_test.go | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 x/dex/keeper/msgserver/msg_server_cancel_orders_test.go diff --git a/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go b/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go new file mode 100644 index 0000000000..22ff2c5197 --- /dev/null +++ b/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go @@ -0,0 +1,101 @@ +package msgserver_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + keepertest "github.com/sei-protocol/sei-chain/testutil/keeper" + "github.com/sei-protocol/sei-chain/x/dex/keeper/msgserver" + "github.com/sei-protocol/sei-chain/x/dex/types" + typesutils "github.com/sei-protocol/sei-chain/x/dex/types/utils" + dexutils "github.com/sei-protocol/sei-chain/x/dex/utils" + "github.com/stretchr/testify/require" +) + +func TestCancelOrder(t *testing.T) { + // store a long limit order to the orderbook + keeper, ctx := keepertest.DexKeeper(t) + keeper.SetLongBook(ctx, keepertest.TestContract, types.LongBook{ + Price: sdk.OneDec(), + Entry: &types.OrderEntry{ + Price: sdk.OneDec(), + Quantity: sdk.MustNewDecFromStr("2"), + PriceDenom: keepertest.TestPriceDenom, + AssetDenom: keepertest.TestAssetDenom, + Allocations: []*types.Allocation{ + { + Account: keepertest.TestAccount, + OrderId: 1, + Quantity: sdk.MustNewDecFromStr("2"), + }, + }, + }, + }) + + // cancel order + msg := &types.MsgCancelOrders{ + Creator: keepertest.TestAccount, + ContractAddr: keepertest.TestContract, + Cancellations: []*types.Cancellation{ + { + Price: sdk.OneDec(), + PositionDirection: types.PositionDirection_LONG, + PriceDenom: keepertest.TestPriceDenom, + AssetDenom: keepertest.TestAssetDenom, + Id: 1, + }, + }, + } + keeper.AddRegisteredPair(ctx, keepertest.TestContract, keepertest.TestPair) + keeper.SetTickSizeForPair(ctx, keepertest.TestContract, keepertest.TestPair, *keepertest.TestPair.Ticksize) + wctx := sdk.WrapSDKContext(ctx) + server := msgserver.NewMsgServerImpl(*keeper) + _, err := server.CancelOrders(wctx, msg) + + pairBlockCancellations := dexutils.GetMemState(ctx.Context()).GetBlockCancels(ctx, keepertest.TestContract, typesutils.GetPairString(&keepertest.TestPair)) + require.Nil(t, err) + require.Equal(t, 1, len(pairBlockCancellations.Get())) + require.Equal(t, uint64(1), pairBlockCancellations.Get()[0].Id) + require.Equal(t, keepertest.TestAccount, pairBlockCancellations.Get()[0].Creator) +} + +func TestInvalidCancels(t *testing.T) { + // nil cancel price + keeper, ctx := keepertest.DexKeeper(t) + keeper.AddRegisteredPair(ctx, keepertest.TestContract, keepertest.TestPair) + keeper.SetTickSizeForPair(ctx, keepertest.TestContract, keepertest.TestPair, *keepertest.TestPair.Ticksize) + keeper.SetLongBook(ctx, keepertest.TestContract, types.LongBook{ + Price: sdk.OneDec(), + Entry: &types.OrderEntry{ + Price: sdk.OneDec(), + Quantity: sdk.MustNewDecFromStr("2"), + PriceDenom: keepertest.TestPriceDenom, + AssetDenom: keepertest.TestAssetDenom, + Allocations: []*types.Allocation{ + { + Account: keepertest.TestAccount, + OrderId: 1, + Quantity: sdk.MustNewDecFromStr("2"), + }, + }, + }, + }) + wctx := sdk.WrapSDKContext(ctx) + server := msgserver.NewMsgServerImpl(*keeper) + + // nil creator + msg := &types.MsgCancelOrders{ + ContractAddr: keepertest.TestContract, + Cancellations: []*types.Cancellation{ + { + PositionDirection: types.PositionDirection_LONG, + PriceDenom: keepertest.TestPriceDenom, + AssetDenom: keepertest.TestAssetDenom, + Id: 1, + Price: sdk.OneDec(), + }, + }, + } + _, err := server.CancelOrders(wctx, msg) + require.NotNil(t, err) +} From ebd585f9dfcaa8ff08419a9b101a2f70bd9669c2 Mon Sep 17 00:00:00 2001 From: codchen Date: Thu, 20 Oct 2022 12:29:24 +0800 Subject: [PATCH 2/2] fix tests --- .../msgserver/msg_server_cancel_orders.go | 40 ++++++++- .../msg_server_cancel_orders_test.go | 86 +++++++++++++++---- 2 files changed, 105 insertions(+), 21 deletions(-) diff --git a/x/dex/keeper/msgserver/msg_server_cancel_orders.go b/x/dex/keeper/msgserver/msg_server_cancel_orders.go index b6be562375..d0dfbbe382 100644 --- a/x/dex/keeper/msgserver/msg_server_cancel_orders.go +++ b/x/dex/keeper/msgserver/msg_server_cancel_orders.go @@ -3,6 +3,7 @@ package msgserver import ( "context" "errors" + "fmt" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/sei-protocol/sei-chain/x/dex/types" @@ -13,6 +14,11 @@ import ( func (k msgServer) CancelOrders(goCtx context.Context, msg *types.MsgCancelOrders) (*types.MsgCancelOrdersResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + // validate cancellation requests + if err := k.validateCancels(msg); err != nil { + return nil, err + } + for _, cancellation := range msg.GetCancellations() { var allocation *types.Allocation var found bool @@ -40,9 +46,14 @@ func (k msgServer) CancelOrders(goCtx context.Context, msg *types.MsgCancelOrder if !cancelledInCurrentBlock { // only cancel if it's not cancelled in a previous tx in the same block cancel := types.Cancellation{ - Id: cancellation.Id, - Initiator: types.CancellationInitiator_USER, - Creator: msg.Creator, + Id: cancellation.Id, + Initiator: types.CancellationInitiator_USER, + Creator: msg.Creator, + ContractAddr: msg.ContractAddr, + Price: cancellation.Price, + AssetDenom: cancellation.AssetDenom, + PriceDenom: cancellation.PriceDenom, + PositionDirection: cancellation.PositionDirection, } pairBlockCancellations.Add(&cancel) } @@ -50,3 +61,26 @@ func (k msgServer) CancelOrders(goCtx context.Context, msg *types.MsgCancelOrder return &types.MsgCancelOrdersResponse{}, nil } + +func (k msgServer) validateCancels(cancels *types.MsgCancelOrders) error { + if len(cancels.Creator) == 0 { + return fmt.Errorf("invalid cancellation, creator cannot be empty") + } + if len(cancels.ContractAddr) == 0 { + return fmt.Errorf("invalid cancellation, contract address cannot be empty") + } + + for _, cancellation := range cancels.GetCancellations() { + if cancellation.Price.IsNil() { + return fmt.Errorf("invalid cancellation price: %s", cancellation.Price) + } + if len(cancellation.AssetDenom) == 0 { + return fmt.Errorf("invalid cancellation, asset denom is empty") + } + if len(cancellation.PriceDenom) == 0 { + return fmt.Errorf("invalid cancellation, price denom is empty") + } + } + + return nil +} diff --git a/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go b/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go index 22ff2c5197..eb1656c273 100644 --- a/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go +++ b/x/dex/keeper/msgserver/msg_server_cancel_orders_test.go @@ -56,35 +56,37 @@ func TestCancelOrder(t *testing.T) { require.Nil(t, err) require.Equal(t, 1, len(pairBlockCancellations.Get())) require.Equal(t, uint64(1), pairBlockCancellations.Get()[0].Id) + require.Equal(t, sdk.OneDec(), pairBlockCancellations.Get()[0].Price) + require.Equal(t, "atom", pairBlockCancellations.Get()[0].AssetDenom) + require.Equal(t, "usdc", pairBlockCancellations.Get()[0].PriceDenom) require.Equal(t, keepertest.TestAccount, pairBlockCancellations.Get()[0].Creator) + require.Equal(t, keepertest.TestContract, pairBlockCancellations.Get()[0].ContractAddr) } func TestInvalidCancels(t *testing.T) { // nil cancel price keeper, ctx := keepertest.DexKeeper(t) - keeper.AddRegisteredPair(ctx, keepertest.TestContract, keepertest.TestPair) - keeper.SetTickSizeForPair(ctx, keepertest.TestContract, keepertest.TestPair, *keepertest.TestPair.Ticksize) - keeper.SetLongBook(ctx, keepertest.TestContract, types.LongBook{ - Price: sdk.OneDec(), - Entry: &types.OrderEntry{ - Price: sdk.OneDec(), - Quantity: sdk.MustNewDecFromStr("2"), - PriceDenom: keepertest.TestPriceDenom, - AssetDenom: keepertest.TestAssetDenom, - Allocations: []*types.Allocation{ - { - Account: keepertest.TestAccount, - OrderId: 1, - Quantity: sdk.MustNewDecFromStr("2"), - }, + msg := &types.MsgCancelOrders{ + Creator: keepertest.TestAccount, + ContractAddr: keepertest.TestContract, + Cancellations: []*types.Cancellation{ + { + PositionDirection: types.PositionDirection_LONG, + PriceDenom: keepertest.TestPriceDenom, + AssetDenom: keepertest.TestAssetDenom, + Id: 1, }, }, - }) + } + keeper.AddRegisteredPair(ctx, keepertest.TestContract, keepertest.TestPair) + keeper.SetTickSizeForPair(ctx, keepertest.TestContract, keepertest.TestPair, *keepertest.TestPair.Ticksize) wctx := sdk.WrapSDKContext(ctx) server := msgserver.NewMsgServerImpl(*keeper) + _, err := server.CancelOrders(wctx, msg) + require.NotNil(t, err) // nil creator - msg := &types.MsgCancelOrders{ + msg = &types.MsgCancelOrders{ ContractAddr: keepertest.TestContract, Cancellations: []*types.Cancellation{ { @@ -96,6 +98,54 @@ func TestInvalidCancels(t *testing.T) { }, }, } - _, err := server.CancelOrders(wctx, msg) + _, err = server.CancelOrders(wctx, msg) + require.NotNil(t, err) + + // nil contract address + msg = &types.MsgCancelOrders{ + Creator: keepertest.TestAccount, + Cancellations: []*types.Cancellation{ + { + Price: sdk.OneDec(), + PositionDirection: types.PositionDirection_LONG, + PriceDenom: keepertest.TestPriceDenom, + AssetDenom: keepertest.TestAssetDenom, + Id: 1, + }, + }, + } + _, err = server.CancelOrders(wctx, msg) + require.NotNil(t, err) + + // nil price denom + msg = &types.MsgCancelOrders{ + Creator: keepertest.TestAccount, + ContractAddr: keepertest.TestContract, + Cancellations: []*types.Cancellation{ + { + Price: sdk.OneDec(), + PositionDirection: types.PositionDirection_LONG, + AssetDenom: keepertest.TestAssetDenom, + Id: 1, + }, + }, + } + _, err = server.CancelOrders(wctx, msg) + require.NotNil(t, err) + + // nil asset denom + msg = &types.MsgCancelOrders{ + Creator: keepertest.TestAccount, + ContractAddr: keepertest.TestContract, + Cancellations: []*types.Cancellation{ + { + Price: sdk.OneDec(), + PositionDirection: types.PositionDirection_LONG, + PriceDenom: keepertest.TestPriceDenom, + Id: 1, + }, + }, + } + _, err = server.CancelOrders(wctx, msg) require.NotNil(t, err) }