ArjunVachhani/order-matcher

Need Test method for Match 1000 to 1000

P9avel opened this issue · 11 comments

Hello, i am need method which will matched 100% and removed orders. I am not found equal and I am try write themself. Please look code.

[Fact]
public void Empty_Market_After_Match_Enters_Pending_With_Limit()
{
Order order1 = new Order { IsBuy = true, OpenQuantity = 1000, Price = 10, OrderId = 1 };
OrderMatchingResult acceptanceResult = matchingEngine.AddOrder(order1, 1);

        mockTradeListener.Verify(x => x.OnAccept(order1.OrderId));
        mockTradeListener.VerifyNoOtherCalls();
        Assert.Equal(OrderMatchingResult.OrderAccepted, acceptanceResult);
        Assert.Contains(order1, matchingEngine.CurrentOrders.Select(x => x.Value));
        Assert.Contains(order1.OrderId, matchingEngine.AcceptedOrders);
        Assert.Contains(order1, matchingEngine.Book.BidSide.SelectMany(x => x.Value));
        Assert.Single(matchingEngine.Book.BidSide);
        Assert.Empty(matchingEngine.Book.AskSide);
        Assert.Empty(matchingEngine.Book.StopAskSide);
        Assert.Empty(matchingEngine.Book.StopBidSide);
        Assert.Equal(1000, order1.OpenQuantity);
        Assert.Equal((ulong)1, order1.Sequnce);

        Order order2 = new Order { IsBuy = false, OpenQuantity = 1000, Price = 0, OrderId = 2 };
        OrderMatchingResult result2 = matchingEngine.AddOrder(order2, 2);

        mockTradeListener.Verify(x => x.OnAccept(order2.OrderId));
        mockTradeListener.Verify(x => x.OnTrade(2, 1, 10, 1000, 0, 50, 10000, 20));
        mockTradeListener.VerifyNoOtherCalls();
        
        Assert.False(matchingEngine.AcceptedOrders.Any());
    }

But i am have exception Assert.False(matchingEngine.AcceptedOrders.Any()); I think very bad when Orders not cleaned in AcceptedOrders after trading. Possible can you to check and modify behivior and include (if need renamed) in *.Tests project of source codes?

@P9avel MatchingEngine keeps track of accepted order id. it rejects order if order id has appeared before. This behavior can restrict same order entering MatchingEngine multiple times.

I understood you. but the problem is that the hash table with OrderId will constantly grow until the memory runs out. This solution cannot be used in production. Possible can you add to MatchingEngine new option and saved last OrderId in int variable. When user resent less value (newOrderId <= LastOrderId) then throw exception?

@P9avel I agree with your concern. (newOrderId <= LastOrderId) check works only if orders are in sequence, if orders are not in sequence then it will reject legitimate order. I think we can have a data structure like sliding window, which can compact if all the previous orders has been seen.

You using one-thread solution. And external code generated OrderId. The sequence can only be broken if the Orders were sent in the wrong order from outside. I.e. external client generated OrderIds but violates them. I think the external client must monitor the correct order sending. Think enought (newOrderId <= LastOrderId) checking in your code. And need options for this checking Off, because in common cases client can generated OrderId as Guid because int can ends.

for various reasons its hard to maintain order sequence. It seems to me that support for orders in out of sequence might be the right choice.

Main problem here what never clearing old OrderIds

Possible simple add options for OrderId checking Off. And then i am set this options and not need saved old OrderIds ?

@P9avel agree, we could have option to disable accepted order tracking.

@P9avel we have added support to disable accepted order tracking.

matchingEngine.AcceptedOrderTrackingEnabled = false

@P9avel Now we are using custom data structure that uses BitArray, also it has logic to compact if all the orderIds for a range has been processed. Benefits of the data structure are

  1. It will take less memory compared to HashSet
  2. It will compact if all the orderIds has been processed for a given range. basically it will not grow if the orders are processed in a range
P9avel commented

cool news, but not actual now for me. Good luck in your project