From 9408cc6f5d6704dc91da1ef6d90060c31e9e03c4 Mon Sep 17 00:00:00 2001 From: King Date: Wed, 5 Jul 2023 13:23:12 +0900 Subject: [PATCH 1/3] fix: fix invariant checks --- x/exchange/keeper/invariants.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x/exchange/keeper/invariants.go b/x/exchange/keeper/invariants.go index 67b1dda1e..42b9d27dd 100644 --- a/x/exchange/keeper/invariants.go +++ b/x/exchange/keeper/invariants.go @@ -39,6 +39,9 @@ func CanCancelOrderInvariant(k Keeper) sdk.Invariant { msg := "" cnt := 0 k.IterateAllOrders(ctx, func(order types.Order) (stop bool) { + if order.MsgHeight == ctx.BlockHeight() { + return false + } if _, err := k.CancelOrder(ctx, order.MustGetOrdererAddress(), order.Id); err != nil { msg += fmt.Sprintf("\tcannot cancel order %d: %v\n", order.Id, err) cnt++ @@ -103,14 +106,14 @@ func OrderBookInvariant(k Keeper) sdk.Invariant { } } marketState := k.MustGetMarketState(ctx, market.Id) - if marketState.LastPrice != nil { - if !bestSellPrice.IsNil() && bestSellPrice.LT(*marketState.LastPrice) { + if marketState.LastPrice != nil && !bestSellPrice.IsNil() && !bestBuyPrice.IsNil() { + if bestSellPrice.LT(*marketState.LastPrice) && bestBuyPrice.GTE(bestSellPrice) { msg += fmt.Sprintf( "\tmarket %d has sell order under the last price: %s < %s\n", market.Id, bestSellPrice, marketState.LastPrice) cnt++ } - if !bestBuyPrice.IsNil() && bestBuyPrice.GT(*marketState.LastPrice) { + if bestBuyPrice.GT(*marketState.LastPrice) && bestSellPrice.LTE(bestBuyPrice) { msg += fmt.Sprintf( "\tmarket %d has buy order above the last price: %s > %s\n", market.Id, bestBuyPrice, marketState.LastPrice) From 394efe073abbfc95f89cabded009309647f6b5f8 Mon Sep 17 00:00:00 2001 From: King Date: Wed, 5 Jul 2023 14:58:32 +0900 Subject: [PATCH 2/3] fix: fix placeLimitOrder nil pointer exception --- app/testutil/test_suite.go | 21 ++++++++ x/exchange/keeper/order.go | 8 ++- x/exchange/keeper/order_test.go | 95 +++++++++++++++++++++++++++++++-- 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/app/testutil/test_suite.go b/app/testutil/test_suite.go index b8fac7043..3508bf5fe 100644 --- a/app/testutil/test_suite.go +++ b/app/testutil/test_suite.go @@ -3,6 +3,7 @@ package testutil import ( "time" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/suite" sdk "github.com/cosmos/cosmos-sdk/types" @@ -76,3 +77,23 @@ func (s *TestSuite) FundedAccount(addrNum int, amt sdk.Coins) sdk.AccAddress { s.FundAccount(addr, amt) return addr } + +func (s *TestSuite) CheckEvent(evtType proto.Message, attrs map[string][]byte) { + s.T().Helper() + evtTypeName := proto.MessageName(evtType) + for _, ev := range s.Ctx.EventManager().ABCIEvents() { + if ev.Type == evtTypeName { + attrMap := make(map[string][]byte) + for _, attr := range ev.Attributes { + attrMap[string(attr.Key)] = attr.Value + } + for k, v := range attrs { + value, ok := attrMap[k] + s.Require().Truef(ok, "key %s not found", k) + s.Require().Equal(v, value) + } + return + } + } + s.FailNowf("CheckEvent failed", "event with type %s not found", evtTypeName) +} diff --git a/x/exchange/keeper/order.go b/x/exchange/keeper/order.go index 9717d592f..002d703a9 100644 --- a/x/exchange/keeper/order.go +++ b/x/exchange/keeper/order.go @@ -155,11 +155,15 @@ func (k Keeper) placeLimitOrder( } } - if isBatch || qty.Sub(res.ExecutedQuantity).IsPositive() { + openQty := qty + if !isBatch { + openQty = openQty.Sub(res.ExecutedQuantity) + } + if isBatch || openQty.IsPositive() { deadline := ctx.BlockTime().Add(lifespan) order, err = k.newOrder( ctx, orderId, typ, market, ordererAddr, isBuy, price, - qty, qty.Sub(res.ExecutedQuantity), deadline, false) + qty, openQty, deadline, false) if err != nil { return } diff --git a/x/exchange/keeper/order_test.go b/x/exchange/keeper/order_test.go index 5eca9dc71..632cbf320 100644 --- a/x/exchange/keeper/order_test.go +++ b/x/exchange/keeper/order_test.go @@ -6,14 +6,99 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" utils "github.com/crescent-network/crescent/v5/types" + "github.com/crescent-network/crescent/v5/x/exchange/keeper" + "github.com/crescent-network/crescent/v5/x/exchange/types" ) -func (s *KeeperTestSuite) TestOrderMatching() { - aliceAddr := utils.TestAddress(1) - bobAddr := utils.TestAddress(2) +func (s *KeeperTestSuite) TestPlaceLimitOrder() { + market := s.CreateMarket(utils.TestAddress(0), "ucre", "uusd", true) + + ordererAddr1 := s.FundedAccount(1, enoughCoins) + ordererAddr2 := s.FundedAccount(2, enoughCoins) + + msgServer := keeper.NewMsgServerImpl(s.keeper) + s.Ctx = s.Ctx.WithEventManager(sdk.NewEventManager()) + resp, err := msgServer.PlaceLimitOrder( + sdk.WrapSDKContext(s.Ctx), types.NewMsgPlaceLimitOrder( + ordererAddr1, market.Id, true, utils.ParseDec("5.1"), sdk.NewInt(10_000000), time.Hour)) + s.Require().NoError(err) + s.Require().EqualValues(1, resp.OrderId) + s.Require().Equal(sdk.NewInt(0), resp.ExecutedQuantity) + s.Require().Equal(sdk.NewInt64Coin("uusd", 0), resp.Paid) + s.Require().Equal(sdk.NewInt64Coin("ucre", 0), resp.Received) + s.CheckEvent(&types.EventPlaceLimitOrder{}, map[string][]byte{ + "executed_quantity": []byte(`"0"`), + "paid": []byte(`{"denom":"uusd","amount":"0"}`), + "received": []byte(`{"denom":"ucre","amount":"0"}`), + }) + + s.Ctx = s.Ctx.WithEventManager(sdk.NewEventManager()) + resp, err = msgServer.PlaceLimitOrder( + sdk.WrapSDKContext(s.Ctx), types.NewMsgPlaceLimitOrder( + ordererAddr2, market.Id, false, utils.ParseDec("5"), sdk.NewInt(5_000000), time.Hour)) + s.Require().NoError(err) + s.Require().EqualValues(2, resp.OrderId) + s.Require().Equal(sdk.NewInt(5_000000), resp.ExecutedQuantity) + s.Require().Equal(sdk.NewInt64Coin("ucre", 5_000000), resp.Paid) + // Matched at 5.1 + s.Require().Equal(sdk.NewInt64Coin("uusd", 25_423500), resp.Received) + s.CheckEvent(&types.EventPlaceLimitOrder{}, map[string][]byte{ + "executed_quantity": []byte(`"5000000"`), + "paid": []byte(`{"denom":"ucre","amount":"5000000"}`), + "received": []byte(`{"denom":"uusd","amount":"25423500"}`), + }) +} + +func (s *KeeperTestSuite) TestPlaceBatchLimitOrder() { + market := s.CreateMarket(utils.TestAddress(0), "ucre", "uusd", true) + + ordererAddr1 := s.FundedAccount(1, enoughCoins) + ordererAddr2 := s.FundedAccount(2, enoughCoins) - s.FundAccount(aliceAddr, utils.ParseCoins("1000000ucre,1000000uusd")) - s.FundAccount(bobAddr, utils.ParseCoins("1000000ucre,1000000uusd")) + ordererBalances1Before := s.GetAllBalances(ordererAddr1) + ordererBalances2Before := s.GetAllBalances(ordererAddr2) + s.PlaceBatchLimitOrder( + market.Id, ordererAddr1, true, utils.ParseDec("5.1"), sdk.NewInt(10_000000), 0) + s.PlaceBatchLimitOrder( + market.Id, ordererAddr2, false, utils.ParseDec("5"), sdk.NewInt(10_000000), 0) + s.Require().NoError(s.keeper.RunBatchMatching(s.Ctx, market)) + s.NextBlock() + + marketState := s.keeper.MustGetMarketState(s.Ctx, market.Id) + s.Require().NotNil(marketState.LastPrice) + s.Require().Equal("5.050000000000000000", marketState.LastPrice.String()) + ordererBalances1After := s.GetAllBalances(ordererAddr1) + ordererBalances2After := s.GetAllBalances(ordererAddr2) + ordererBalances1Diff, _ := ordererBalances1After.SafeSub(ordererBalances1Before) + ordererBalances2Diff, _ := ordererBalances2After.SafeSub(ordererBalances2Before) + // Both taker + s.Require().Equal("9970000ucre,-50500000uusd", ordererBalances1Diff.String()) + s.Require().Equal("-10000000ucre,50348500uusd", ordererBalances2Diff.String()) + + ordererBalances1Before = ordererBalances1After + ordererBalances2Before = ordererBalances2After + s.PlaceBatchLimitOrder( + market.Id, ordererAddr1, true, utils.ParseDec("5.2"), sdk.NewInt(10_000000), 0) + s.PlaceBatchLimitOrder( + market.Id, ordererAddr2, false, utils.ParseDec("5.07"), sdk.NewInt(10_000000), 0) + s.Require().NoError(s.keeper.RunBatchMatching(s.Ctx, market)) + s.NextBlock() + + marketState = s.keeper.MustGetMarketState(s.Ctx, market.Id) + s.Require().NotNil(marketState.LastPrice) + s.Require().Equal("5.070000000000000000", marketState.LastPrice.String()) + ordererBalances1After = s.GetAllBalances(ordererAddr1) + ordererBalances2After = s.GetAllBalances(ordererAddr2) + ordererBalances1Diff, _ = ordererBalances1After.SafeSub(ordererBalances1Before) + ordererBalances2Diff, _ = ordererBalances2After.SafeSub(ordererBalances2Before) + s.Require().Equal("9970000ucre,-50700000uusd", ordererBalances1Diff.String()) + // Maker + s.Require().Equal("-9985000ucre,50700000uusd", ordererBalances2Diff.String()) +} + +func (s *KeeperTestSuite) TestOrderMatching() { + aliceAddr := s.FundedAccount(1, enoughCoins) + bobAddr := s.FundedAccount(2, enoughCoins) market := s.CreateMarket(utils.TestAddress(3), "ucre", "uusd", true) From 21924a7c257276d39edcda525d9fc5884a873db3 Mon Sep 17 00:00:00 2001 From: King Date: Wed, 5 Jul 2023 15:33:49 +0900 Subject: [PATCH 3/3] test: fix test --- x/exchange/keeper/order_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/exchange/keeper/order_test.go b/x/exchange/keeper/order_test.go index 632cbf320..7c94e6023 100644 --- a/x/exchange/keeper/order_test.go +++ b/x/exchange/keeper/order_test.go @@ -97,8 +97,8 @@ func (s *KeeperTestSuite) TestPlaceBatchLimitOrder() { } func (s *KeeperTestSuite) TestOrderMatching() { - aliceAddr := s.FundedAccount(1, enoughCoins) - bobAddr := s.FundedAccount(2, enoughCoins) + aliceAddr := s.FundedAccount(1, utils.ParseCoins("1000000ucre,1000000uusd")) + bobAddr := s.FundedAccount(2, utils.ParseCoins("1000000ucre,1000000uusd")) market := s.CreateMarket(utils.TestAddress(3), "ucre", "uusd", true)