From 08d7022815200aad6a6a32642c5af84417bd976d Mon Sep 17 00:00:00 2001 From: Ben Davison Date: Thu, 30 Jan 2020 16:55:47 +0000 Subject: [PATCH 1/5] introduce a default empty ISymbolOrderBookEntry that is returned buy BestBid or BestAsk if called when the bid or ask lists are empty. This resolves a null reference exception seen during syncronization (specifically when connecting to Kraken). I have also introduced BestOffers which returns both bid and ask in the scope of one lock allowing the caller to be sure that nothing has changed between BestBid and BestAsk request. --- .../SymbolOrderBookTests.cs | 72 +++++++++ CryptoExchange.Net/CryptoExchange.Net.xml | 148 ++++++++++++++++++ .../OrderBook/SymbolOrderBook.cs | 40 +++-- 3 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs diff --git a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs new file mode 100644 index 0000000..efbe657 --- /dev/null +++ b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs @@ -0,0 +1,72 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using CryptoExchange.Net.Logging; +using CryptoExchange.Net.Objects; +using CryptoExchange.Net.OrderBook; +using CryptoExchange.Net.Sockets; +using CryptoExchange.Net.UnitTests.TestImplementations; +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +namespace CryptoExchange.Net.UnitTests +{ + [TestFixture] + public class SymbolOrderBookTests + { + private static OrderBookOptions defaultOrderBookOptions = new OrderBookOptions("Test", true); + + private class TestableSymbolOrderBook : SymbolOrderBook + { + public TestableSymbolOrderBook() : base("BTC/USD", defaultOrderBookOptions) + { + } + + public override void Dispose() + { + throw new NotImplementedException(); + } + + protected override Task> DoResync() + { + throw new NotImplementedException(); + } + + protected override Task> DoStart() + { + throw new NotImplementedException(); + } + } + + [TestCase] + public void GivenEmptyBidList_WhenBestBid_ThenEmptySymbolOrderBookEntry() + { + var symbolOrderBook = new TestableSymbolOrderBook(); + Assert.IsNotNull(symbolOrderBook.BestBid); + Assert.AreEqual(0m, symbolOrderBook.BestBid.Price); + Assert.AreEqual(0m, symbolOrderBook.BestAsk.Quantity); + } + + [TestCase] + public void GivenEmptyAskList_WhenBestAsk_ThenEmptySymbolOrderBookEntry() + { + var symbolOrderBook = new TestableSymbolOrderBook(); + Assert.IsNotNull(symbolOrderBook.BestBid); + Assert.AreEqual(0m, symbolOrderBook.BestBid.Price); + Assert.AreEqual(0m, symbolOrderBook.BestAsk.Quantity); + } + + [TestCase] + public void GivenEmptyBidAndAskList_WhenBestOffers_ThenEmptySymbolOrderBookEntries() + { + var symbolOrderBook = new TestableSymbolOrderBook(); + Assert.IsNotNull(symbolOrderBook.BestOffers); + Assert.IsNotNull(symbolOrderBook.BestOffers.Item1); + Assert.IsNotNull(symbolOrderBook.BestOffers.Item2); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item1.Price); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item1.Quantity); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item2.Price); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item2.Quantity); + } + } +} diff --git a/CryptoExchange.Net/CryptoExchange.Net.xml b/CryptoExchange.Net/CryptoExchange.Net.xml index 62e5487..0954f72 100644 --- a/CryptoExchange.Net/CryptoExchange.Net.xml +++ b/CryptoExchange.Net/CryptoExchange.Net.xml @@ -1819,6 +1819,11 @@ The best ask currently in the order book + + + BestBid/BesAsk returned as a pair + + ctor @@ -2898,5 +2903,148 @@ + + + Specifies that is allowed as an input even if the + corresponding type disallows it. + + + + + Initializes a new instance of the class. + + + + + Specifies that is disallowed as an input even if the + corresponding type allows it. + + + + + Initializes a new instance of the class. + + + + + Specifies that a method that will never return under any circumstance. + + + + + Initializes a new instance of the class. + + + + + Specifies that the method will not return if the associated + parameter is passed the specified value. + + + + + Gets the condition parameter value. + Code after the method is considered unreachable by diagnostics if the argument + to the associated parameter matches this value. + + + + + Initializes a new instance of the + class with the specified parameter value. + + + The condition parameter value. + Code after the method is considered unreachable by diagnostics if the argument + to the associated parameter matches this value. + + + + + Specifies that an output may be even if the + corresponding type disallows it. + + + + + Initializes a new instance of the class. + + + + + Specifies that when a method returns , + the parameter may be even if the corresponding type disallows it. + + + + + Gets the return value condition. + If the method returns this value, the associated parameter may be . + + + + + Initializes the attribute with the specified return value condition. + + + The return value condition. + If the method returns this value, the associated parameter may be . + + + + + Specifies that an output is not even if the + corresponding type allows it. + + + + + Initializes a new instance of the class. + + + + + Specifies that the output will be non- if the + named parameter is non-. + + + + + Gets the associated parameter name. + The output will be non- if the argument to the + parameter specified is non-. + + + + + Initializes the attribute with the associated parameter name. + + + The associated parameter name. + The output will be non- if the argument to the + parameter specified is non-. + + + + + Specifies that when a method returns , + the parameter will not be even if the corresponding type allows it. + + + + + Gets the return value condition. + If the method returns this value, the associated parameter will not be . + + + + + Initializes the attribute with the specified return value condition. + + + The return value condition. + If the method returns this value, the associated parameter will not be . + + diff --git a/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs b/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs index 36e94b8..73bd76f 100644 --- a/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs +++ b/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs @@ -127,6 +127,14 @@ namespace CryptoExchange.Net.OrderBook } } + private class EmptySymbolOrderBookEntry : ISymbolOrderBookEntry + { + public decimal Quantity { get { return 0m; } set {; } } + public decimal Price { get { return 0m; } set {; } } + } + + private static ISymbolOrderBookEntry emptySymbolOrderBookEntry = new EmptySymbolOrderBookEntry(); + /// /// The best bid currently in the order book /// @@ -135,7 +143,7 @@ namespace CryptoExchange.Net.OrderBook get { lock (bookLock) - return bids.FirstOrDefault().Value; + return bids.FirstOrDefault().Value ?? emptySymbolOrderBookEntry; } } @@ -147,7 +155,19 @@ namespace CryptoExchange.Net.OrderBook get { lock (bookLock) - return asks.FirstOrDefault().Value; + return asks.FirstOrDefault().Value ?? emptySymbolOrderBookEntry; + } + } + + /// + /// BestBid/BesAsk returned as a pair + /// + public Tuple BestOffers { + get { + lock (bookLock) + { + return new Tuple(BestBid,BestAsk); + } } } @@ -298,9 +318,10 @@ namespace CryptoExchange.Net.OrderBook private void CheckBestOffersChanged(ISymbolOrderBookEntry prevBestBid, ISymbolOrderBookEntry prevBestAsk) { - if (BestBid.Price != prevBestBid.Price || BestBid.Quantity != prevBestBid.Quantity || - BestAsk.Price != prevBestAsk.Price || BestAsk.Quantity != prevBestAsk.Quantity) - OnBestOffersChanged?.Invoke(BestBid, BestAsk); + var (bestBid, bestAsk) = BestOffers; + if (bestBid.Price != prevBestBid.Price || bestBid.Quantity != prevBestBid.Quantity || + bestAsk.Price != prevBestAsk.Price || bestAsk.Quantity != prevBestAsk.Quantity) + OnBestOffersChanged?.Invoke(bestBid, bestAsk); } /// @@ -329,8 +350,7 @@ namespace CryptoExchange.Net.OrderBook else { CheckProcessBuffer(); - var prevBestBid = BestBid; - var prevBestAsk = BestAsk; + var (prevBestBid, prevBestAsk) = BestOffers; ProcessSingleSequenceUpdates(rangeUpdateId, bids, asks); OnOrderBookUpdate?.Invoke(bids, asks); CheckBestOffersChanged(prevBestBid, prevBestAsk); @@ -366,8 +386,7 @@ namespace CryptoExchange.Net.OrderBook else { CheckProcessBuffer(); - var prevBestBid = BestBid; - var prevBestAsk = BestAsk; + var (prevBestBid, prevBestAsk) = BestOffers; ProcessRangeUpdates(firstUpdateId, lastUpdateId, bids, asks); OnOrderBookUpdate?.Invoke(bids, asks); CheckBestOffersChanged(prevBestBid, prevBestAsk); @@ -396,8 +415,7 @@ namespace CryptoExchange.Net.OrderBook else { CheckProcessBuffer(); - var prevBestBid = BestBid; - var prevBestAsk = BestAsk; + var (prevBestBid, prevBestAsk) = BestOffers; ProcessUpdates(bids, asks); OnOrderBookUpdate?.Invoke(bids, asks); CheckBestOffersChanged(prevBestBid, prevBestAsk); From 635ba1747c181ac6cbe522e57a30bb642fc9a28f Mon Sep 17 00:00:00 2001 From: Ben Davison Date: Thu, 30 Jan 2020 17:59:21 +0000 Subject: [PATCH 2/5] removed not implemented exception from the dispose override in the TestableSymbolOrderBook implementation --- CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs index efbe657..d0ac5c6 100644 --- a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs +++ b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs @@ -22,10 +22,7 @@ namespace CryptoExchange.Net.UnitTests { } - public override void Dispose() - { - throw new NotImplementedException(); - } + public override void Dispose() {} protected override Task> DoResync() { From 4a01c30f3441ebc4152c014c2ac4cd5a1964b668 Mon Sep 17 00:00:00 2001 From: Ben Davison Date: Thu, 30 Jan 2020 21:44:14 +0000 Subject: [PATCH 3/5] Expose BestOffers on ISymbolOrderBook interface --- CryptoExchange.Net/CryptoExchange.Net.xml | 5 +++++ CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/CryptoExchange.Net/CryptoExchange.Net.xml b/CryptoExchange.Net/CryptoExchange.Net.xml index 0954f72..880dd15 100644 --- a/CryptoExchange.Net/CryptoExchange.Net.xml +++ b/CryptoExchange.Net/CryptoExchange.Net.xml @@ -837,6 +837,11 @@ The best ask currently in the order book + + + BestBid/BesAsk returned as a pair + + Start connecting and synchronizing the order book diff --git a/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs b/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs index 9d0b502..10275c3 100644 --- a/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs +++ b/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs @@ -70,6 +70,11 @@ namespace CryptoExchange.Net.Interfaces /// ISymbolOrderBookEntry BestAsk { get; } + /// + /// BestBid/BesAsk returned as a pair + /// + Tuple BestOffers { get; } + /// /// Start connecting and synchronizing the order book /// From 07d0a0159d850b973afb3886d9eae698bd9ff2ad Mon Sep 17 00:00:00 2001 From: Ben Davison Date: Thu, 30 Jan 2020 22:01:14 +0000 Subject: [PATCH 4/5] Use ValueTuple for BestOffers --- CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs | 12 ++++++------ CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs | 2 +- CryptoExchange.Net/OrderBook/SymbolOrderBook.cs | 6 ++---- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs index d0ac5c6..4f1214f 100644 --- a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs +++ b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs @@ -58,12 +58,12 @@ namespace CryptoExchange.Net.UnitTests { var symbolOrderBook = new TestableSymbolOrderBook(); Assert.IsNotNull(symbolOrderBook.BestOffers); - Assert.IsNotNull(symbolOrderBook.BestOffers.Item1); - Assert.IsNotNull(symbolOrderBook.BestOffers.Item2); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item1.Price); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item1.Quantity); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item2.Price); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.Item2.Quantity); + Assert.IsNotNull(symbolOrderBook.BestOffers.BestBid); + Assert.IsNotNull(symbolOrderBook.BestOffers.BestAsk); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestBid.Price); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestBid.Quantity); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestAsk.Price); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestAsk.Quantity); } } } diff --git a/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs b/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs index 10275c3..06321d8 100644 --- a/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs +++ b/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs @@ -73,7 +73,7 @@ namespace CryptoExchange.Net.Interfaces /// /// BestBid/BesAsk returned as a pair /// - Tuple BestOffers { get; } + (ISymbolOrderBookEntry BestBid, ISymbolOrderBookEntry BestAsk) BestOffers { get; } /// /// Start connecting and synchronizing the order book diff --git a/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs b/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs index 73bd76f..143307b 100644 --- a/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs +++ b/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs @@ -162,12 +162,10 @@ namespace CryptoExchange.Net.OrderBook /// /// BestBid/BesAsk returned as a pair /// - public Tuple BestOffers { + public (ISymbolOrderBookEntry BestBid, ISymbolOrderBookEntry BestAsk) BestOffers { get { lock (bookLock) - { - return new Tuple(BestBid,BestAsk); - } + return (BestBid,BestAsk); } } From d384d1ee5babbfd098f27f3b53923627a1e9bf5e Mon Sep 17 00:00:00 2001 From: Ben Davison Date: Thu, 30 Jan 2020 22:03:44 +0000 Subject: [PATCH 5/5] Rename ValueTuple in BestOffers --- CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs | 12 ++++++------ CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs | 2 +- CryptoExchange.Net/OrderBook/SymbolOrderBook.cs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs index 4f1214f..59917c6 100644 --- a/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs +++ b/CryptoExchange.Net.UnitTests/SymbolOrderBookTests.cs @@ -58,12 +58,12 @@ namespace CryptoExchange.Net.UnitTests { var symbolOrderBook = new TestableSymbolOrderBook(); Assert.IsNotNull(symbolOrderBook.BestOffers); - Assert.IsNotNull(symbolOrderBook.BestOffers.BestBid); - Assert.IsNotNull(symbolOrderBook.BestOffers.BestAsk); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestBid.Price); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestBid.Quantity); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestAsk.Price); - Assert.AreEqual(0m, symbolOrderBook.BestOffers.BestAsk.Quantity); + Assert.IsNotNull(symbolOrderBook.BestOffers.Bid); + Assert.IsNotNull(symbolOrderBook.BestOffers.Ask); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Bid.Price); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Bid.Quantity); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Ask.Price); + Assert.AreEqual(0m, symbolOrderBook.BestOffers.Ask.Quantity); } } } diff --git a/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs b/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs index 06321d8..ee24057 100644 --- a/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs +++ b/CryptoExchange.Net/Interfaces/ISymbolOrderBook.cs @@ -73,7 +73,7 @@ namespace CryptoExchange.Net.Interfaces /// /// BestBid/BesAsk returned as a pair /// - (ISymbolOrderBookEntry BestBid, ISymbolOrderBookEntry BestAsk) BestOffers { get; } + (ISymbolOrderBookEntry Bid, ISymbolOrderBookEntry Ask) BestOffers { get; } /// /// Start connecting and synchronizing the order book diff --git a/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs b/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs index 143307b..ea35512 100644 --- a/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs +++ b/CryptoExchange.Net/OrderBook/SymbolOrderBook.cs @@ -162,7 +162,7 @@ namespace CryptoExchange.Net.OrderBook /// /// BestBid/BesAsk returned as a pair /// - public (ISymbolOrderBookEntry BestBid, ISymbolOrderBookEntry BestAsk) BestOffers { + public (ISymbolOrderBookEntry Bid, ISymbolOrderBookEntry Ask) BestOffers { get { lock (bookLock) return (BestBid,BestAsk);